feat(cli): port supabase test db and test new#5522
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 617bc838ce
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Supabase CLI previewnpx --yes https://pkg.pr.new/supabase@5522Preview package for commit |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: da955478ae
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 24b8d5a3fc
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Coly010
force-pushed
the
cli/port-test-db
branch
from
24b8d5a to
c9e5648
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c9e5648bf5
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: eea9b56858
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
![depthfirst-app[bot]](https://avatars.githubusercontent.com/in/1021002?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5dfc7e6504
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a94be70ba5
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e41db48dc0
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2de496615f
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6e2499722d
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 66f758e5b9
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d1217db13e
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9c8a8bbdf9
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 30d0759f76
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4899ae7c83
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 563c9095d1
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 85ba357228
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dc95ef8998
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 70ac327d88
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: df51f5e5bf
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bca0cc10c4
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cc4ca5005b
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cb62fb9a00
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bc004087ec
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ecd9b83fb1
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
… DSNs with options connect (review: socket-options-url)
… pgconn (review: pgservice)
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 85dc98ce9e
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
…role step-down, connect timeout) - Probe the final fallback attempt too, so connect returns a live session — Go always dials eagerly (pgx.ConnectConfig / pgconn.ConnectConfig), and the linked temp-role wait relies on it (review: temp-role-probe). - Gate SET SESSION ROLE postgres on !isLocal: Go installs the step-down hook only in ConnectByConfigStream, not ConnectLocalPostgres (review: local-role-stepdown). - Apply Go's connect_timeout default (10s remote, 2s local; DSN/PGCONNECT_TIMEOUT override) via PgClient.connectTimeout (review: connect-timeout).
…timeout, empty query overrides) - Preserve backslashes before ordinary chars in keyword/value DSNs; unescape only \\ and \' like pgconn parseDSNSettings (review: dsn-backslash). - Parse connect_timeout / PGCONNECT_TIMEOUT into connectTimeoutSeconds (review: connect-timeout). - A present-but-empty URL query (?dbname=/?user=) overrides the structural value verbatim, matching pgconn parseURLSettings (review: empty-query-override).
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b780c3766a
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| // Go walks `supabase/` first, then the repo root; first writer wins. | ||
| const dirs = [path.join(workdir, "supabase"), workdir]; |
There was a problem hiding this comment.
When --workdir points at a project nested below the invocation directory, this only loads .env* from <workdir>/supabase and <workdir>. The Go loader walks from the supabase/ directory up through every parent until the original cwd, so a repo-level .env such as <repo>/.env can provide SUPABASE_DB_PASSWORD, PGPORT, or other PG* defaults; here those files are skipped and test db --local/--db-url can connect with defaults or an empty password instead.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Thanks for the careful read — I dug into the Go loader and this isn't a divergence; the two-dir walk is at exact parity, so I'm leaving it as is.
The key detail is that Go chdir's into workdir before any command runs: the root command calls utils.ChangeWorkDir (internal/utils/misc.go:231-250) from PersistentPreRunE (cmd/root.go:104), which does os.Chdir(workdir) (misc.go:243). So by the time loadNestedEnv executes, os.Getwd() already returns workdir, not the original invocation directory.
Then in loadNestedEnv (pkg/config/config.go:1050-1071):
repoDir, _ := os.Getwd() // == workdir (already chdir'd)
basePath := filepath.Join(repoDir, "supabase") // SupabaseDirPath is relative "supabase"
for cwd := basePath; cwd != filepath.Dir(repoDir); cwd = filepath.Dir(cwd) {
loadDefaultEnv(env)
}The loop starts at workdir/supabase, walks up via filepath.Dir, and stops at filepath.Dir(workdir). So it processes exactly workdir/supabase and workdir, then stops — it never ascends above workdir. A <repo>/.env sitting above a nested --workdir is skipped by the Go CLI too, so loading it here would be a divergence, not a fix.
For completeness: when --workdir is omitted, Go resolves it via getProjectRoot (misc.go:209-224), which walks up from the invocation cwd to the nearest ancestor containing supabase/config.toml and chdir's there. That project-root discovery is the chdir step; the env walk itself is always anchored to the resolved root. In the port, workdir is the already-resolved project root (resolved in LegacyCliConfig), so [workdir/supabase, workdir] mirrors Go's post-chdir behaviour precisely, including the nested case.
(The doc comment's "up to the repo root" phrasing is a bit loose — I'll tighten it separately — but the behaviour is correct.) Leaving this open for visibility.
…e, PGUSER, connect_timeout) - Accept comma-separated ?port= lists for multi-host URLs (review: comma-ports). - Reject ports outside 1..65535 like pgconn parsePort (review: port-range). - Treat a present-but-empty connString service= as explicit: it overrides PGSERVICE and fails resolution, rather than silently using the env service (review: empty-service). - Ignore empty PGUSER, falling through to the OS account (review: empty-pguser). - A present-but-empty connString connect_timeout= is a parse error; an empty PGCONNECT_TIMEOUT env var stays unset (review: empty-connect-timeout).
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fc600624f7
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
…d password redaction - Accept the `database` key as an alias for `dbname` (URL query + DSN keyword), matching pgconn's nameMap (review: database-alias). - Merge present-but-empty pgservice values (pgconn doesn't empty-skip service settings like env), so an empty service password= suppresses PGPASSWORD → .pgpass and an empty connect_timeout= is a parse error (review: empty-service). - Redact malformed --db-url passwords containing literal @ or / (anchor the redaction boundary on the last @ before the authority terminator) to close a CWE-209 leak (review: redaction-leak).
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 070c6aceb0
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| // prefer / require / unset → TLS without verification (pgx default). | ||
| return { rejectUnauthorized: false, ...sni }; |
There was a problem hiding this comment.
🟡 Severity: MEDIUM
For remote connections with sslmode=prefer, require, or unset (the default), TLS is used but server certificate validation is explicitly disabled. A network-adjacent attacker can perform a MITM attack against --linked or --db-url sessions, intercepting temporary login credentials in transit.
Helpful? Add 👍 / 👎
💡 Fix Suggestion
Suggestion: For sslmode=require, Node.js's built-in trust store (Mozilla/system CAs) should be used to validate the server certificate, protecting against MITM attacks. Only the truly opportunistic modes (prefer and unset) should skip verification. Since legacySslConfigsFor (line 208) already promotes require + caCert to verify-ca before calling legacySslOptionFor, the remaining require path here only occurs when no CA cert is pinned — so using the system trust store is the correct behavior. Change the final return to conditionally set rejectUnauthorized: true for require and false for prefer/unset. This is a targeted one-line fix at the exact location of the vulnerability.
⚠️ Experimental Feature: This code suggestion is automatically generated. Please review carefully.
| // prefer / require / unset → TLS without verification (pgx default). | |
| return { rejectUnauthorized: false, ...sni }; | |
| // prefer / unset → opportunistic TLS without verification; require → validate system CAs. | |
| return { rejectUnauthorized: sslmode === "require", ...sni }; |
There was a problem hiding this comment.
Thanks for flagging this — rejectUnauthorized: false on a remote TLS connection does look alarming on its face. I checked the pgconn parity baseline before deciding, and I'm keeping the current behaviour; here's the reasoning.
This file is a strict 1:1 port of the Go CLI's connection layer (jackc/pgconn v1.14.3). pgconn's configTLS handles sslmode=require with no sslrootcert by deliberately disabling verification:
// config.go:641-650
case "require":
// if a root CA file exists, require behaves like verify-ca
if sslrootcert != "" { goto nextCase }
tlsConfig.InsecureSkipVerify = true
breakVerification is performed only for verify-ca (config.go:653-686) and verify-full (config.go:687-688). This is documented libpq semantics, not a pgconn quirk: require means "encrypt but trust the endpoint," and only upgrades to verifying behaviour when a root cert is pinned.
Our legacySslConfigsFor already replicates that promotion — require + caCert is rewritten to verify-ca before legacySslOptionFor is called — so the require branch here is only ever reached with no pinned CA, which is exactly pgconn's InsecureSkipVerify = true path. Changing it to rejectUnauthorized: true would make the TS CLI reject certs the Go CLI accepts under the same DSN (self-signed / private-CA endpoints), diverging from the command we're porting.
The honest tradeoff: require over a hostile network is MITM-able in both CLIs, and the supported remedy is identical — use sslmode=verify-full (or pin a CA via sslrootcert), both of which the parser already honors. Tightening only the TS side would silently diverge the two CLIs, so I'm keeping parity here. Leaving this open rather than resolving it, since it's a security-vs-strict-parity call a maintainer may want to weigh in on.
…efile, port-only URLs, hostaddr, empty keys) - A present-but-empty ?host= overrides the structural host verbatim (literal empty host, no PGHOST re-fallback) (review: empty-host). - A present-but-empty servicefile= overrides PGSERVICEFILE and fails resolution (review: empty-servicefile). - Accept port-only URLs (postgres://:5433/db) via the hand-split path; host falls back to PGHOST/default, port kept (review: port-only-url). - Stop using hostaddr as a host fallback — pgconn dials defaultHost for a hostaddr-only DSN (review: hostaddr-fallback). - Reject keyword DSNs with an empty key like '=ignored ...' (review: empty-key).
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f280cd3383
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
…-wins, host-gateway, trailing backslash, redaction) - Load sslrootcert when any dial target is non-socket, not just the primary, so a TCP fallback behind a socket primary still gets the CA (review: cacert-fallback). - DSN dbname/database aliases use last-wins (pgconn remaps at parse time) (review: dsn-alias-order). - Add host.docker.internal:host-gateway (--add-host) on Linux, matching Go's DockerStart ExtraHosts (review: docker-host-gateway). - Reject an unquoted DSN value ending in a lone backslash (pgconn 'invalid backslash') (review: trailing-backslash). - Fully redact an unterminated quoted password in error output (review: redaction-unterminated).
…ew: dotenv-multiline) godotenv v1.5.1 scans quoted values across newlines over the whole buffer (parseBytes), so a PEM/private key spanning lines parses as one value. The line-by-line parser threw 'unterminated quoted value' and aborted test db config loading. Reworked parseDotEnv to a cursor model (skip-statement-start → key → extractVarValue returning the remaining buffer), preserving comments, export prefix, =/: separators, escape + $VAR expansion, and single vs double semantics.
…s (review: tap-stdout) test db has no machine-format envelope (Go has no --output-format for it; output is the streamed pg_prove TAP on stdout). On a run failure, withJsonErrorHandling appended a JSON error to stdout in json/stream-json mode, corrupting the TAP that already streamed. Catch LegacyTestDbRunError before that wrapper: in machine modes route the diagnostic to stderr and exit 1 (matching Go's recoverAndExit); text mode keeps its existing rendering, and pre-stream errors still get the JSON envelope.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 319307b5e0
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| const lastProbed = attempts[lastIndex]!.client.pipe( | ||
| Effect.tap((candidate) => candidate`select 1`), | ||
| ); |
There was a problem hiding this comment.
When a direct HA DSN includes target_session_attrs=read-write (or primary) and the first host is a standby, this probe accepts the first host as soon as select 1 succeeds, so the command proceeds to CREATE EXTENSION/pgTAP on a read-only server instead of falling through to the next host. The Go path uses pgconn.ParseConfig, which installs ValidateConnect for target_session_attrs, so these DSNs should validate the selected host before returning the session.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Rejecting on parity grounds — this is the same question raised earlier on this PR (and rejected for the same reason): the Go test db path does not honor target_session_attrs, so validating it here would diverge from the reference.
pgconn keeps target_session_attrs only as a non-serializable ValidateConnect function pointer (it's in notRuntimeParams, so it never lands in RuntimeParams; config.go:303,318-321,381-391). But test db connects by re-serializing the parsed config through ToPostgresURL (internal/utils/connect.go:23-45) and re-parsing — and ToPostgresURL emits only host/port/user/password/database + RuntimeParams. So target_session_attrs (and the multi-host fallback list) are dropped before the connection is made: Go connects to the first host with no read-write validation. Enforcing it in TS would add HA/standby-rejection behavior the Go command demonstrably doesn't have.
Leaving open (not resolving) so a maintainer can weigh in if they'd want to intentionally improve on Go here — but that would be a deliberate divergence tracked separately, not a parity fix.
| const sslrootcert = | ||
| url.searchParams.get("sslrootcert") ?? | ||
| svc("sslrootcert") ?? | ||
| libpqEnv(env, "PGSSLROOTCERT") ?? | ||
| null; |
There was a problem hiding this comment.
When --db-url (or PG env/service settings) supplies sslcert and sslkey for a database that requires client-certificate auth, this parser only retains sslrootcert, so the TLS keypair never reaches legacySslConfigsFor/PgClient.make and the connection fails. The Go CLI's pgconn path recognizes PGSSLCERT/PGSSLKEY and loads the certificate pair during TLS configuration, so client-cert DSNs that worked there regress here.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Rejecting on parity grounds — same as the earlier client-cert thread on this PR: the Go test db path does not forward sslcert/sslkey either, so adding it would diverge.
Although pgconn.ParseConfig loads sslcert/sslkey into config.TLSConfig.Certificates, that TLSConfig is never used to connect. test db connects via ConnectByConfigStream → ConnectByUrl(ToPostgresURL(config)) (internal/utils/connect.go:223), and ToPostgresURL (:23-45) serializes only user/password/host/port/database + RuntimeParams — and sslcert/sslkey/sslrootcert are in pgconn's notRuntimeParams set (config.go:287-321), so they're excluded. pgxv5.Connect then re-parses that cert-less URL, rebuilding TLS with no client certificate. So a cert-authenticated endpoint would equally fail under the Go CLI's test db; forwarding the keypair here would let the TS CLI authenticate where Go can't.
(sslrootcert is forwarded in this port as a deliberate, separately-decided superset, but the client keypair tracks Go's drop.) Leaving open for a maintainer; client-cert support in test db would be a behaviour change worth making on both CLIs together.
…redirect (review: doh-ip-validation) Go only uses FallbackLookupIP results as pgconn LookupFunc dial targets; the TS port additionally feeds the resolved host into legacyBuildConnectionUrl, so a tampered DoH answer like 1.2.3.4@attacker.com could become the URL authority and redirect the credentialed connection. Gate parseResolvedIps on net.isIP(data) !== 0, matching Go's effective IP-only semantics (CWE-20/CWE-350).
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 461bba3343
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| /** Whether a resolved sslmode is present and not one pgconn accepts. */ | ||
| function isInvalidSslmode(sslmode: string | null | undefined): boolean { | ||
| return ( | ||
| sslmode !== null && sslmode !== undefined && sslmode.length > 0 && !VALID_SSLMODES.has(sslmode) |
There was a problem hiding this comment.
When a connection string or pg_service entry explicitly sets an empty SSL mode, e.g. postgres://host/db?sslmode= or sslmode=, this predicate treats it as valid and the parser later omits it, so the connection falls back to the default TLS behavior. pgconn preserves present empty settings and configTLS rejects them with sslmode is invalid, so typoed DSNs that the Go CLI fails fast can now proceed to connect with a different SSL mode.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Thanks for the careful read, but holding the current behaviour here — it already matches pgconn v1.14.3, and the premise that pgconn rejects an empty sslmode doesn't hold.
configTLS reads sslmode := settings["sslmode"] and normalizes an empty value back to the libpq default before the switch:
// config.go:627-629
if sslmode == "" {
sslmode = "prefer"
}So a present-but-empty ?sslmode= / sslmode= never reaches the default: return errors.New("sslmode is invalid") branch (config.go:690) — it's handled exactly like prefer, and the second switch (config.go:772-776) likewise returns the prefer fallback list. The "sslmode is invalid" error is only reachable for a non-empty unrecognized value (e.g. sslmode=verfy-full); configTLS is the sole sslmode consumer.
So the sslmode.length > 0 guard is intentional and correct: present-empty → not-invalid → omitted → driver applies prefer, exactly what the Go CLI does. Dropping it would make us reject DSNs the Go CLI happily connects with — a divergence.
This is also why sslmode differs from the dbname/service/passfile/connect_timeout present-empty cases on this PR: for those, pgconn's downstream handling of an empty value genuinely diverges from absent (empty service fails GetService(""), empty connect_timeout fails to parse). sslmode is the one setting pgconn folds an empty value back into the default, so the override pattern doesn't carry over. Leaving open for a maintainer.
| const rootcertPath = cfg.sslrootcert; | ||
| const anyTcpTarget = dialTargets.some(({ dialHost }) => !legacyIsUnixSocketHost(dialHost)); | ||
| const caCert = | ||
| rootcertPath !== undefined && rootcertPath.length > 0 && !isLocal && anyTcpTarget |
There was a problem hiding this comment.
When sslmode=disable is combined with sslrootcert/PGSSLROOTCERT pointing at a missing or unreadable file, this branch still reads the CA bundle for any TCP target and fails before connecting. pgconn's TLS setup returns immediately for disable and never reads sslrootcert, so plaintext DSNs that the Go CLI accepts can now be rejected solely because a stale CA path is present.
Useful? React with 👍 / 👎.
| // both appear (Go's map iteration has no defined precedence). Only an absent | ||
| // param falls back to the path → service → PGDATABASE → resolved user. | ||
| const dbnameQuery = query.get("dbname") ?? query.get("database"); | ||
| const structuralDb = decodeURIComponent(url.pathname.replace(/^\//, "")); |
There was a problem hiding this comment.
When a URL path accidentally or intentionally contains more than one leading slash, such as postgres://host//postgres, Go's pgconn uses strings.TrimLeft(url.Path, "/") and still targets postgres. This removes only the first slash, so the port sets PGDATABASE to /postgres and runs the setup connection/pg_prove against a different (usually nonexistent) database.
Useful? React with 👍 / 👎.
| */ | ||
| function libpqPort(raw: string | undefined): number | undefined { | ||
| if (raw === undefined) return DIRECT_PORT; | ||
| return /^\d+$/.test(raw) ? Number(raw) : undefined; |
There was a problem hiding this comment.
When the port list comes from PGPORT or a pg_service entry, for example PGHOST=h1,h2 PGPORT=5432,5433, this helper rejects the comma even though pgconn stores the env/service value verbatim and later splits settings["port"] per host. HA configurations that the Go CLI would try in order now fail URL/DSN parsing before pgTAP can run.
Useful? React with 👍 / 👎.
| // comment); resume at that newline so the next statement is parsed cleanly. | ||
| const afterQuote = value.slice(end + 1); | ||
| const nl = afterQuote.indexOf("\n"); | ||
| const rest = nl === -1 ? "" : afterQuote.slice(nl); |
There was a problem hiding this comment.
When a project .env has non-comment garbage after a quoted value, such as SUPABASE_DB_PASSWORD="secret" typo, this discards everything up to the newline and accepts the file. godotenv resumes parsing immediately after the closing quote, so only whitespace plus a # comment is harmless; other trailing text raises a parse error instead of silently loading credentials from a malformed env file.
Useful? React with 👍 / 👎.
|
|
||
| let url: URL; | ||
| try { | ||
| url = new URL(normalized); |
There was a problem hiding this comment.
When a direct URL uses an unbracketed IPv6 literal that Go's net/url accepts, e.g. postgres://::1/postgres, this still routes through WHATWG new URL() and returns a parse error before the connection is attempted. pgconn treats that authority as an IP-only host and applies the default port, so IPv6 DSNs accepted by the Go CLI now fail during parsing.
Useful? React with 👍 / 👎.
…bsolutePath (review: windows-socket) pgconn v1.14.3 isAbsolutePath (config.go:112-129) treats an uppercase drive-letter path (A-Z + ':' + '\') as a unix socket, like POSIX '/'. legacyIsUnixSocketHost only matched '/', so a host=C:\pgsql DSN got TCP/TLS/DoH handling instead of socket plaintext. Match pgconn exactly (uppercase drive only; lowercase c:\ stays TCP).
…ide (review: os-account-user, empty-passfile)
- Default libpq user now resolves PGUSER (env) else the OS account via
os.userInfo().username (mirroring user.Current().Username), not $USER/$USERNAME
— pgconn never reads those (only PGUSER is an env override).
- A present-but-empty passfile= overrides PGPASSFILE/default and resolves to no
.pgpass (pgconn's ReadPassfile("") fails → empty password), instead of falling
back to the env/default passfile.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bf4c5382d5
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| } catch { | ||
| return ""; | ||
| } | ||
| const matchHost = host.startsWith("/") ? "localhost" : host; |
There was a problem hiding this comment.
When a DSN uses a Windows absolute socket directory (for example host=C:\\pgsql) and omits the password, this still looks up .pgpass using the literal socket path because only POSIX /... hosts are rewritten. The connection layer now treats the same A:\... form as a Unix socket, and pgconn also maps any Unix-socket host to localhost before FindPassword, so Windows socket connections that rely on a standard localhost:*:*:*:password passfile entry fail authentication in the TS port.
Useful? React with 👍 / 👎.
| value: string, | ||
| env: LegacyParseEnv = processEnv, | ||
| ): LegacyPgConnInput | undefined { | ||
| const trimmed = value.trim(); |
There was a problem hiding this comment.
When --db-url has leading/trailing whitespace, especially an all-whitespace value from a bad env expansion, this trim accepts inputs that pgconn rejects (non-empty strings are dispatched without trimming). A value like --db-url ' ' now becomes an empty keyword DSN and falls through to PGHOST/libpq defaults, so pgTAP can run against an unintended database instead of failing fast like the Go CLI.
Useful? React with 👍 / 👎.
| while (i < n && !isSpace(value[i]!)) { | ||
| // pgconn's unquoted scan advances past any `\`, then errors with | ||
| // "invalid backslash" when the escape has no following char | ||
| // (`config.go:539-543`), so a lone trailing backslash is a parse error. | ||
| if (!isEscapedChar(i) && value[i] === "\\" && i + 1 >= n) return undefined; | ||
| if (isEscapedChar(i)) i++; |
There was a problem hiding this comment.
When a keyword DSN value contains an escaped space or tab, such as password=sec\ ret host=db.example.com, pgconn's scanner skips the escaped whitespace while keeping the backslash in the value, so the DSN is valid. This loop stops on the space before considering a general backslash escape (only \\ and \' are treated specially), so the same connection string is rejected or split into a bogus next key, breaking libpq-style DSNs that worked in the Go CLI.
Useful? React with 👍 / 👎.
Summary: open Codex review threads we're intentionally not addressingThese threads are all left open on purpose. Each is a real observation, but in every case implementing the suggestion would make the TypeScript 1. Dropped by Go's
|
2 participants
What changed
Native TypeScript port of
supabase test dbandsupabase test newinto the legacy shell (stable channel), replacing the Phase-0 Go proxies.test new— writessupabase/tests/<name>_test.sqlfrom the embedded pgtap template; matches Go's relative-path success message, file location, and exit codes.--template(pgtap).test db—--db-url/--local/--linked+ variadic paths. Connects via@effect/sql-pgto enable/disable the pgTAP extension, then runssupabase/pg_prove:3.36throughdocker run(read-only volume mounts,--security-opt label:disable, local docker network or host networking). Honors--network-id, and thedb-url/linked/localmutual-exclusivity is byte-for-byte identical to Go's cobra error.New shared infrastructure (for upcoming
db reset/db dumpports)LegacyDbConnection— Postgres connection seam (single swap point for the driver).LegacyDbConfigResolver—--db-url/--local/--linkedresolution, including the linked sub-flow (temp login-role viaV1CreateLoginRole, pooler fallback with a public-suffix MITM domain check, network-ban unban, backoff). Ports Go'sflags.ParseDatabaseConfig+NewDbConfigWithPassword.LegacyDockerRun— one-shotdocker runrunner.The Management API stack is built lazily on the
--linkedbranch only, so--local/--db-urlnever resolve an access token (auth-free, matching Go).Reviewer notes
@effect/sql-pg(4.0.0-beta.75, pure-JSpg); verified it bundles and round-trips underbun build --compile.PgClientexposes noOnNoticehook, so "already installed" is detected with apg_extensionpre-check before enabling — equivalent observable behavior to Go's notice-code 42710 callback.SIDE_EFFECTS.md):test dbhas no--output-formatmachine envelope (Go has none; TAP streams to stdout in all modes); the[images] pgproveconfig override is not modeled by the TS config schema.Known follow-up
The
--linkedsub-flow (login-role / pooler / unban / backoff) is implemented and type-checked but lacks a dedicated integration test — it requires the real management runtime with a mocked HTTP transport and a real linked project to verify faithfully. The local/db-url resolver paths andtomlparsing are covered.Closes CLI-1318