feat(cli): port supabase stop and status commands to native TypeScript#5765
feat(cli): port supabase stop and status commands to native TypeScript#5765Coly010 wants to merge 76 commits into
Conversation
Replaces the Go-proxy stubs for `stop`/`status` with native Effect implementations that talk directly to Docker/Podman via subprocess, replicating Go's label-filtering and container-naming scheme byte-for-byte. Legacy `start` is still Go-proxied, so this intentionally does not go through `@supabase/stack/effect`'s daemon-based orchestration model — that substrate is incompatible with the real containers Go's binary creates. Adds shared Docker/config infrastructure (`legacy-docker-lifecycle`, `legacy-go-jwt`, `legacy-local-config-values`, `legacy-api-url`) used by both commands, and fixes several correctness issues found during review: stdout colorization keyed off the wrong stream's TTY status, `--override-name` incorrectly leaking into pretty-mode output (Go ignores it there), and the `--backup`/`--no-backup` formula not matching Go's actual (dead-flag) behavior. Fixes CLI-1324
…e.test.ts Documents the *.live.test.ts convention in AGENTS.md (a 4th test category alongside unit/integration/e2e): black-box CLI subprocess tests executed by the cli-e2e-ci harness against a real supabox stack. Clarifies how local-dev-stack commands like stop/status fit this pattern despite never calling the Management API — they only need the real Docker daemon the cli-e2e-ci runner also provides, so they reuse the existing describeLive gate rather than a dedicated one. Adds stop.live.test.ts and status.live.test.ts, each spinning up a real local Docker stack (init -> start, excluding the heaviest services) in an isolated temp directory and verifying the command under test against the real containers, with best-effort cleanup on every path.
|
@codex review |
![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
Avoid colliding with backup's negated flag
Effect boolean flags register both the positive and negated token, so Flag.boolean("backup") above already claims --no-backup as the false form of the hidden backup flag. Defining a separate noBackup boolean on the same token can route supabase stop --no-backup to backup=false or otherwise collide during parsing, leaving flags.noBackup false and skipping the volume-prune path even though the user requested data deletion; model the hidden --backup=false flag without generating a --no-backup alias.
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
Thanks for flagging this — I dug into it and don't think there's a collision here, though the concern was reasonable to raise given how the two flags look side by side. Effect's flag registry ( I confirmed this empirically by running the exact flag config through
This also matches the Go CLI's own structure: |
…5765) Three accepted findings from PR review, each verified against Go source before implementing: - Sanitize the config/env-derived project id before building the Docker label filter in both `stop` and `status`. Go's `Config.Validate` sanitizes `Config.ProjectId` once at config-load time (pkg/config/config.go:938-944), and every later reader — including the Docker label `start` writes (internal/utils/docker.go:375) — sees that same sanitized string. Without this, a dirty `project_id` (spaces, leading punctuation) would filter on a value `start` never labeled, silently matching nothing. The explicit `--project-id` bypass on `stop` stays raw, matching Go's stop.go:19-20. - `status --override-name` with an unrecognized field key is now silently ignored instead of a hard error, matching Go's `go-env` Unmarshal, which never checks its input map for unmatched keys (verified against the vendored go-env@v0.1.2 source). - `status` no longer hard-fails when `supabase/config.toml` is absent; Go's `flags.LoadConfig` treats a missing file as a no-op and proceeds with template defaults (pkg/config/config.go:655-656), so this now decodes an empty document through the shared config schema for its defaults instead of erroring. (The broader gap — Go's automatic `SUPABASE_<FIELD>` env-var binding via viper, which `@supabase/config` doesn't have an equivalent for — is a larger, cross-cutting `@supabase/config` feature affecting every ported command, called out as a follow-up rather than folded into this fix.) Two other findings were investigated and rejected with cited evidence (directly on the PR): a claimed `--backup`/`--no-backup` flag collision (empirically disproven against Effect's parser), and image-name-based `--exclude` matching (the underlying config.toml schema has no field to check, on either CLI).
…0O86N3Lyo) Go's status.toValues() gates each service on `--exclude` matching either the container id or the Docker image's short name (ShortContainerImageName). The TS port only checked container ids. Port the short-name extraction and check it against the same default images the embedded Dockerfile manifest already provides, since the relevant Go config fields (KongImage, Image, etc.) all carry `toml:"-"` and are never user-overridable.
…us-commands # Conflicts: # apps/cli/docs/go-cli-porting-status.md
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1b42381e79
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@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: 1b42381e79
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…OErm0O86N4wLX) Go's stop.Run checks len(projectId) > 0 (internal/stop/stop.go:18), not just whether --project-id was set, so an explicit but empty value falls through to config.toml resolution. The TS port only checked Option.isSome, so `--project-id ""` resolved to the bare all-projects label filter instead.
…6N4wLa, PRRT_kwDOErm0O86N4wLu) Go registers both --override-name and --exclude as pflag StringSliceVar (cmd/status.go:36-37), which CSV-splits each occurrence and accumulates across repeats. The TS flags only handled repetition, so a single comma-separated value like `--exclude kong,auth` produced one malformed entry instead of two. Reuses the shared legacyParseStringSliceFlag already applied to sso/postgres-config for the same StringSlice parity gap.
…6N4wL1) Go's Config.Validate fails config-load when auth.jwt_secret is set but shorter than 16 characters (pkg/config/apikeys.go:45-47), before any command can render output. The TS resolver accepted any non-empty value and signed ANON_KEY/SERVICE_ROLE_KEY with it, letting `status -o env/json` succeed and print keys for a config the Go CLI and local stack both reject.
… PRRT_kwDOErm0O86N4wLx)
Go's config loader binds Viper with SetEnvPrefix("SUPABASE") + AutomaticEnv()
(pkg/config/config.go:529-535), so SUPABASE_AUTH_JWT_SECRET/PUBLISHABLE_KEY/
SECRET_KEY/ANON_KEY/SERVICE_ROLE_KEY override the corresponding config.toml
value at higher precedence. legacyResolveLocalConfigValues only read the
decoded config object, so a local stack started with those env overrides
had `status` print keys that didn't match the running Auth service. Scoped
to exactly the 5 auth fields this module reads, not a general
@supabase/config port of Viper's AutomaticEnv.
…ed (review: PRRT_kwDOErm0O86N4wLk) Go's generateJWT signs anon/service_role with the first key in auth.signing_keys_path (RS256/ES256) instead of HMAC when that file resolves to a non-empty JWK array (pkg/config/apikeys.go:76-113). The TS resolver only ever HMAC-signed with jwt_secret. Ports GenerateAsymmetricJWT via legacyGenerateAsymmetricGoJwt (RFC 7517 JWK -> Node crypto private key, ieee-p1363 signature encoding for ES256's raw r||s format) and wires it into legacyResolveLocalConfigValues, which now needs workdir to resolve a relative signing_keys_path against <workdir>/supabase.
Go's jwkToRSAPrivateKey/jwkToECDSAPrivateKey reject a JWK whose kty doesn't match its claimed alg, and an EC key whose curve isn't P-256 (pkg/config/auth.go). legacyGenerateAsymmetricGoJwt only checked alg, so an EC key forged with alg: RS256 (or a non-P-256 curve claimed as ES256) signed "successfully" and produced a spec-invalid token that silently fails verification instead of raising an error. Found independently by the security and DX reviewers in review-changes.
…tusValues twice status.handler.ts called legacyStatusValues twice per invocation (real values, then pretty-mode values with an empty override map) — only the first call was wrapped in Effect.try. With signing_keys_path support this doubled file I/O + JWT signing on every text-mode run, and left the second call able to throw an uncaught exception if it ever diverged from the first. Splits legacyStatusValues into legacyResolveStatusState (the throwing half: local config resolution + gating) and legacyStatusValuesFromState (pure name remapping), so the handler resolves state once, guards it once, and reuses it for both value maps.
…ration coverage Go's Config.Validate fails a bad auth.signing_keys_path with "failed to read signing keys: %w" (open failure) or "failed to decode signing keys: %w" (parse failure) (pkg/config/config.go:1059-1062). The TS port let readFileSync/JSON.parse's raw Node error text through unwrapped instead. Also adds status.integration.test.ts coverage for the SUPABASE_AUTH_* env override and asymmetric-signing-key behaviors, which previously only had unit-level coverage on the pure resolver.
Supabase CLI previewnpx --yes https://pkg.pr.new/supabase/cli/supabase@09e218a2e474da77ab4d410a15bbfe8813dd817cPreview package for commit |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 60877ba497
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…spection (ci: e2e shard 1/3)
Go's assertContainerHealthy never special-cases a missing container — it
wraps whatever ContainerInspect returns, so the real daemon error text
("No such container: ...") flows through. The TS port collapsed that case
into a hardcoded "no such container" string via an "absent" sentinel,
discarding the real text.
Separately, legacyInspectContainerState's Effect.all([exitCode, stdout,
stderr]) ran sequentially by default, awaiting exitCode (Node's "exit"
event) before ever subscribing to the stdout/stderr streams. Node's "exit"
can fire before a fast process's stdio pipes are drained, so the real
Docker CLI's stderr was silently lost in the real subprocess environment
even after removing the hardcoded string — reproduced against a real
docker CLI subprocess, confirmed via runParity's stderr comparison in
apps/cli-e2e. Same fix applied to the two other call sites in this file
that share the pattern (legacyListContainersByLabel, legacyListVolumesByLabel).
…e2e shard 1/3) Go prints this line via an unconditional, immediate fmt.Fprintln before any Docker call runs (docker.go:97), routed straight to stdout in non-interactive mode (tea.go's fakeProgram). The TS port gated it behind the shared output.task spinner's 200ms debounce, so the line was silently dropped whenever the underlying Docker calls resolved faster than that threshold — exactly what happens against the mocked/replayed Docker CLI in the e2e harness. Printing it directly via output.raw removes the race entirely.
…ity request logs (ci: e2e shard 1/3) stop/status parity comparisons were failing on request-log mismatches that reflect nothing about CLI behavior: the real docker CLI issues a fresh HEAD /_ping handshake before every subprocess invocation (Go's SDK pings once per command via a persistent client), and negotiates its own API version segment into the URL path (/v1.51/ vs /v1.53/) based on the installed docker CLI/daemon, not anything the command controls. Strip both before comparing so parity reflects the actual Docker operations performed rather than client plumbing, mirroring the equivalent normalization already used for fixture matching in apps/cli-e2e/src/server/placeholder.ts.
…s (review: PRRT_kwDOErm0O86N7ctR)
Go's Viper binds SetEnvPrefix("SUPABASE") + AutomaticEnv() over every
config field via UnmarshalExact's struct walk (pkg/config/config.go:531-535,
698-705), including the plain-string Auth.SigningKeysPath field
(pkg/config/auth.go:164) — not just the 5 auth fields this module already
wrapped with envOverride. Load() resolves that override before Validate()
opens and parses the JWK file (config.go:735-745, 1059-1062), so an
env-only SUPABASE_AUTH_SIGNING_KEYS_PATH was silently ignored and status
fell back to HMAC-signed keys the running Auth service would reject.
… status (review: PRRT_kwDOErm0O86N7ctY) Go's Config.Load runs loadNestedEnv (supabase/.env and .env.local via godotenv.Load, which never overrides an already-set var) before loadFromFile wires up Viper's AutomaticEnv (pkg/config/config.go:735-745, 528-535) — so an env-file-only SUPABASE_PROJECT_ID overrides config.toml's project_id too, not just an ambient shell export. Both handlers only read process.env directly, missing that middle step: an env-named stack could be left running by stop, or status could target the wrong container. @supabase/config's loadProjectEnvironment already implements the same "ambient wins over .env/.env.local" layering (used today only for env() interpolation inside config.toml), so both handlers now resolve SUPABASE_PROJECT_ID through it instead of process.env directly, and pass the same resolved environment into loadProjectConfig so the .env files aren't parsed twice. Falls back to process.env directly when no supabase/config.toml exists anywhere (loadProjectEnvironment resolves to null in that case), preserving the existing ambient-only behavior for that scenario. The identical gap in LegacyCliConfig.projectId (used by several other already-ported commands) is out of scope here and left for a follow-up.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fb101635cd
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…erred resolution interpolateLeafValue (used by resolveProjectValue/resolveProjectSubtree for consumer-side lookups like edge_runtime.secrets) only guarded against a fully unset env var, not a present-but-empty one, unlike substituteEnvLeaf in lib/env.ts. Go's LoadEnvHook only substitutes a non-empty value (apps/cli-go/pkg/config/decode_hooks.go:19-24, `len(env) > 0`), so a dotenv line like `EMPTY=` should preserve the `env(EMPTY)` literal. Without this fix, a present-but-empty secret resolved to "", got redacted as a real value, and `secrets set` uploaded a blank secret Go would have skipped. review: #5765
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d3838ed4ea
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…tus/stop (review: #5765) Go's Config.Validate rejects an enabled auth.captcha section missing provider/secret, and an enabled analytics.backend="bigquery" section missing gcp_project_id/gcp_project_number/gcp_jwt_path, before status or stop ever touch Docker (pkg/config/config.go:1099-1109,1174-1187). legacyResolveLocalConfigValues ported the site_url/jwt_secret/port checks from the same Validate pass but not these two, so native status/stop could succeed against a config the Go CLI refuses to load. Port both checks, matching the exact Go error wording, and add unit coverage mirroring the existing site_url/deno_version test blocks.
…ted booleans (review: #5765) Go's LoadEnvHook substitutes an env() reference into a raw string, and mapstructure's weakly-typed decodeBool then parses it with strconv.ParseBool, accepting 1/t/T/TRUE/true/True and 0/f/F/FALSE/false/False (plus "" as false). coerceLeaf's boolean branch only recognized the literal strings "true"/"false", so a config boolean set via e.g. `enabled = "env(FOO)"` with FOO=1 failed TS schema decoding for a config Go loads fine. Mirror the same acceptance set legacyParseGoBool already uses for SUPABASE_*_ENABLED overrides, duplicated locally so packages/config doesn't depend on apps/cli.
…ment (review: #5765) parseDotEnv split a .env file into physical lines before matching each one against dotEnvLinePattern, so a godotenv-valid multiline quoted value (e.g. a PEM block) never got a chance to match as a single value — the continuation line(s) then failed the KEY=VALUE pattern outright. Go's loadNestedEnv goes straight to joho/godotenv, which natively supports this, so native status/stop (both of which call loadProjectEnvironment before their own Go-compatible dotenv pass) could reject a supabase/.env Go accepts, solely because of an unrelated multiline secret. Detect a line that opens a quote without closing it on that same physical line, and only then accumulate subsequent lines into the match candidate before running the existing per-line pattern — every other line continues to match exactly as before.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 88aa0fa223
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // Go's `Config.Validate` checks `edge_runtime.deno_version` after the auth | ||
| // block and the functions loop (`pkg/config/config.go:1158-1173`), and — | ||
| // unlike `studio.port`/`local_smtp.port` above — unconditionally, with no | ||
| // `edge_runtime.enabled` gate. | ||
| const denoVersion = envOverrideDenoVersion(config.edge_runtime.deno_version, projectEnvValues); | ||
| validateDenoVersion(denoVersion); |
There was a problem hiding this comment.
When auth is enabled and a nested Viper override such as SUPABASE_AUTH_SMS_TWILIO_ENABLED=true is supplied without the required Twilio fields, Go applies that env value before c.Auth.Sms.validate() and fails config loading. This path only handles a few top-level SUPABASE_*_ENABLED gates and never validates the post-env SMS provider state, so native status/default stop can succeed for an env-overridden auth config the Go CLI rejects.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Thanks for digging into this — you're right about the Go behavior: loadFromFile binds SUPABASE_* env vars to every nested struct field via viper.ExperimentalBindStruct()/AutomaticEnv() (config.go:582-586) and unmarshals them into c before Config.Load calls c.Validate → c.Auth.Sms.validate() (config.go:882,1145,1348-1417), so a bare SUPABASE_AUTH_SMS_TWILIO_ENABLED=true without the required fields does fail Go's config load.
Two things worth separating out, though:
- For the common case — a misconfigured
[auth.sms.*]section written directly inconfig.toml— this is already Go-parity today.packages/config/src/auth/sms.tshas schema-level checks for all five providers with the exact same error strings assms.validate()(verified this throws"Missing required field in config: auth.sms.twilio.account_sid"for an enabled-but-incomplete Twilio block, enforced duringloadProjectConfig's decode before this resolver even runs). - The remaining gap is specifically ambient env overrides of nested fields that aren't referenced via the TOML's explicit
env(VAR)syntax.@supabase/config's env mechanism only implements Go'sLoadEnvHook(explicitenv(VAR)substitution), not Viper's implicit binding — and that's a systemic limitation shared by MFA, hooks, third-party, and webauthn validation in this same file (see theauth.captchacomment a few dozen lines up, which calls out the same "no env-override precedent for nested auth subsections" boundary explicitly).
Given that, I don't think it's right to bolt nested Viper-style env-override support onto SMS alone while leaving the identical gap in MFA/hooks/third-party — that'd be inconsistent scope, and it's more of an architecture/consistency call than something Go parity alone settles for this one thread. @Coly010, would appreciate your call on whether to (a) add this generically across all nested auth subsections as a follow-up, or (b) leave it as a documented, accepted limitation. Leaving this thread open for that input.
…tRef (review: #5765) Go's loadFromFile builds the project_id -> [remotes.*] duplicate map unconditionally on every config load (config.go:594-602), independent of whether a remote ends up matching Config.ProjectId. loadProjectConfigFile only ran that check when a caller passed a projectRef, so native status/stop (which never select a remote) could load a config.toml with duplicate [remotes.*].project_id values that Go would reject outright.
…ig.Validate (review: #5765) Go's Config.Validate checks every [remotes.*].project_id against a 20-lowercase-letter ref pattern unconditionally on every config load (config.go:996-1001), regardless of whether that remote is selected. The shared @supabase/config loader never validated this, so native status/stop could render or prune for a config Go would refuse to load. Test fixtures using short human-readable fake refs (previewref, override-project, etc.) are updated to valid 20-letter refs to satisfy the new check.
…directory (review: #5765) Go's ChangeWorkDir os.Chdir()s the resolved workdir in PersistentPreRunE, before any command runs (apps/cli-go/internal/utils/misc.go:231-250) — a missing path or a path that isn't a directory fails immediately, before config load or Docker access. status/stop previously only resolved the string and continued, so stop could fall through to the workdir-basename default and prune the wrong project, and status would surface a Docker/ config error instead of the workdir error. Scoped to status/stop's own handlers (legacy-workdir-validation.ts) rather than the shared legacy-cli-config.layer.ts: the default walk-up-for-config resolution always returns a real, already-existing directory, so only the explicit branch can ever fail this check, and validating it centrally in the config layer used by every legacy command would ripple the new error type into unrelated command families (db config, management API runtime) far beyond this review's scope.
…alidate (review: #5765) Go's Config.Validate checks ProjectId first, before any other field (pkg/config/config.go:990-991). The workdir-basename default is merged in as a viper default before config.toml is merged, so an explicit project_id = "" in the file overwrites that default with the literal empty string rather than being treated as absent. legacyResolveLocalProjectId previously collapsed "absent" and "present but empty" into the same fallback, so status and stop's default path could inspect or stop a different project (the workdir basename) instead of rejecting the invalid config. SUPABASE_PROJECT_ID still takes precedence, matching Viper's AutomaticEnv ordering.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a4aa01719b
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
… (review: #5765) Go's Config.Validate runs several checks legacyResolveLocalConfigValues skipped, so native status/default stop could succeed on configs the Go CLI rejects: ValidateBucketName over storage.buckets (config.go:1063-1068, ungated), Auth.Hook.validate()'s URI/secret rules (config.go:1136,1453-1521), Auth.MFA.validate()'s enroll-implies-verify rule for TOTP/phone/WebAuthn (config.go:1139,1523-1534), and ValidateFunctionSlug over functions (config.go:1158-1163, ungated). The bucket-name/function-slug patterns and hook-secret pattern are hoisted from legacy-db-config.toml-read.ts (now exported) instead of duplicating them a third time.
… (review: #5765) Go's Config.Validate calls ThirdParty.validate() unconditionally inside if c.Auth.Enabled (pkg/config/config.go:1151), rejecting an enabled provider missing its required field (e.g. firebase without project_id) or more than one provider enabled at once. legacyResolveLocalConfigValues never read config.auth.third_party at all, so native status/default stop could succeed on a config Go rejects. Hoists the clerk domain pattern already ported for the db/migration path instead of duplicating it.
#5765) Go's Config.Validate calls Experimental.validate() right after the analytics block, rejecting a present [experimental.webhooks] section whose enabled isn't explicitly true (pkg/config/config.go:1846-1854), and invalid JSON in experimental.pgdelta.format_options. The webhooks check hinges on config.toml SECTION PRESENCE, not the decoded value: @supabase/config's schema decode-fills experimental.webhooks = {enabled: false} even when the TOML section is absent, so checking the decoded ProjectConfig directly would reject every default project. Threads LoadedProjectConfig.document (the raw, pre-default TOML) through legacyResolveLocalConfigValues as a new optional parameter so this (and a future passkey/webauthn check that needs the same raw access) can inspect presence directly.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a0e5cb6242
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…solver (review: #5765) Go's Config.Validate calls Auth.Email.validate(fsys) right after Auth.MFA.validate(), reading every template's content_path unconditionally and every enabled notification's content_path, hard-failing on a missing/unreadable file (pkg/config/config.go:1293- 1313). legacyResolveLocalConfigValues never read config.auth.email at all, so native status/default stop could succeed on a config Go rejects. Matches Go's asymmetric relative-path bases (config.go:900-916, its own FIXME comment): template paths resolve against the workdir itself, notification paths against <workdir>/supabase.
…(review: #5765) Go's Config.Validate parses studio.api_url with net/url.Parse right after the studio.port check, still inside if c.Studio.Enabled (pkg/config/config.go:1074-1078), rejecting malformed hosts such as an unterminated IPv6 literal (http://[::1). legacyResolveLocalConfigValues never checked studio.api_url at all, so native status/default stop could accept a config Go rejects. Extends legacy-storage-url.ts's legacyGoUrlParse (previously Scheme/Path only, since storage ss:// URLs never have a host) with a faithful port of net/url.parseHost's bracket/port validation, verified byte-for-byte against the real Go stdlib.
…r (review: #5765) Go's Config.Validate rejects [auth.passkey] enabled = true configured without a complete [auth.webauthn] (rp_id/rp_origins), right after the signing-keys read and before Auth.Hook.validate() (pkg/config/config.go:1117-1129). legacyResolveLocalConfigValues never read this at all, so native status/default stop could continue for a config Go refuses. @supabase/config's auth schema has no passkey/webauthn fields, so this threads LoadedProjectConfig.document (the raw, pre-schema-default TOML) through legacyResolveLocalConfigValues -> legacyResolveStatusLocalState -> status.handler.ts/stop.handler.ts, the same document-based approach already used for the equivalent check on the db/migration config-load path (legacy-db-config.toml-read.ts's legacyValidateAuthConfig, A6).
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6b8d4e9f73
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| projectEnvValues: Readonly<Record<string, string>> | undefined, | ||
| ): number { | ||
| const value = envOverride(name, undefined, projectEnvValues); | ||
| if (value === undefined) return configuredPort; |
There was a problem hiding this comment.
The current helper still returns the decoded configuredPort unchecked whenever there is no direct SUPABASE_*_PORT override, and the shared config schema declares ports as plain numbers. When the value comes from config.toml or an env() substitution, such as db.port = 70000 or -1, Go rejects it while decoding the uint16 field, but this path can continue and emit URLs with an invalid port; apply the same integer/range validation to the configured value too.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
This is the same claim already raised and addressed on this PR (see the resolved discussion on envOverridePort, same file/function): Go's db.port/api.port/etc. decode as uint16 via viper.UnmarshalExact → mapstructure's decodeUint. For a literal TOML integer like 70000, that decode does not hard-fail — reflect.Value.SetUint's truncating conversion silently wraps it to 4464 (70000 mod 65536), and Go proceeds with the wrapped value (verified by building and running that decoder against the identical input). The hard-fail path only exists for the string-sourced SUPABASE_*_PORT env-var override, which goes through mapstructure's WeaklyTypedInput string branch (strconv.ParseUint(str, 0, 16)) — exactly the case envOverridePort's existing MAX_PORT check already covers.
Adding range validation to the plain configuredPort value (as proposed here) would make native status/stop reject literal TOML ports that the real Go CLI silently accepts (with wraparound) — a step away from parity, not toward it. Not making this change, for the same reason as before.
| new LegacyStatusConfigLoadError({ message: `failed to read config: ${String(cause)}` }), | ||
| }); | ||
|
|
||
| const loaded = yield* loadProjectConfig(cliConfig.workdir, { |
There was a problem hiding this comment.
When [auth] enabled = false, Go skips Auth.Sms.validate() and Auth.External.validate() entirely, so stale nested blocks such as [auth.sms.twilio] enabled = true without credentials or [auth.external.github] enabled = true without a secret do not block status/default stop. This load goes through @supabase/config, whose nested requiredWhenEnabled checks run without the parent auth.enabled gate, so native status can reject a disabled auth configuration that the Go CLI accepts; defer those auth sub-section checks to the post-load Go-parity validator.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Confirmed as a real gap, not a no-op: Go's Config.Validate nests Auth.Sms.validate()/Auth.External.validate() entirely inside if c.Auth.Enabled (apps/cli-go/pkg/config/config.go:1087,1145,1148), so a disabled auth section with a stale enabled-but-uncredentialed [auth.sms.twilio]/[auth.external.github] block validates fine in Go. In @supabase/config, requiredWhenEnabled (packages/config/src/auth/sms.ts, packages/config/src/auth/providers.ts) only checks each nested provider's own enabled flag, never the parent auth.enabled, so the schema rejects that same config unconditionally during decode — this repo's own legacy-local-config-values.ts:1229-1232 doc comment already flags Sms/External as the two Go-gated sub-checks not yet ported into the (correctly authEnabled-gated) post-load validator.
Holding off on making this change in this run, though: the only place that actually stops the unconditional check from firing is @supabase/config's schema itself (sms.ts/providers.ts), since a nested struct's own .check() fails during decode before any caller-side post-load validator gets a chance to run. That schema is shared by every native command that calls loadProjectConfig — config push, gen types, seed buckets, functions new, functions serve/deploy, not just status/stop — so removing the check there and re-adding it only in status/stop's own legacy-local-config-values.ts would silently drop this validation for every other command, including the auth.enabled=true case those commands correctly reject today. The parity-correct fix is to relocate the check to a cross-field .check() on the top-level auth struct (gated on auth.enabled, still enforced for every caller), not a status/stop-local change — that's real surgery on shared, widely-consumed schema code well beyond this port's stated scope, and I'd rather not make it unilaterally without the coverage/verification pass across every other loadProjectConfig consumer it deserves.
Flagging for @Coly010's call: happy to take this on as a follow-up against @supabase/config if you'd like it tracked separately, or to attempt it inline here if you'd rather have it in this PR.
| new LegacyStatusConfigLoadError({ message: `failed to read config: ${String(cause)}` }), | ||
| ), | ||
| ); | ||
| const config = loaded?.config ?? Schema.decodeUnknownSync(ProjectConfigSchema)({}); |
There was a problem hiding this comment.
The shared config schema accepts [functions.<slug>.env] records, but the Go function config struct has no env field and v.UnmarshalExact rejects that TOML before status or default stop reaches Docker. By trusting this decoded ProjectConfig without an exact Go-shape check, native status can proceed for a config the Go CLI refuses; add a raw-document validation for function keys that are not in the Go struct before continuing.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Thanks for digging into this — you're right about Go's behavior: v.UnmarshalExact (apps/cli-go/pkg/config/config.go:749) does hard-fail on an unknown key like [functions.<slug>.env] (Go's function struct has no env field, config.go:290-296). I reproduced this directly against the pinned viper/mapstructure versions and confirmed Go rejects it with 'functions[foo]' has invalid keys: env.
That said, env isn't an accidental leftover in @supabase/config's schema — it's the intentional per-function allowlist for the newer environments/config.json system (docs/environments-management.md), and the permissive decode (no Go-struct-exactness check on unknown keys) is a property of @supabase/config shared by every legacy command that calls loadProjectConfig — stop, config push, secrets set, gen types, functions new, etc. all go through the identical tomlOnly fallback path and none of them do Go-shape validation either. Fixing this only in status.handler.ts would leave the identical gap open in all of those and produce inconsistent strictness rather than real parity.
The right fix is a TOML-vs-JSON-aware "reject keys Go's struct doesn't have" guard centralized in packages/config, applied uniformly wherever a tomlOnly load happens — not a per-command patch, and given it needs to distinguish the TOML-only Go-compat path from the JSON/config.json path (which legitimately allows env), it's more involved than this thread's scope. Not making a status/stop-local change here; happy to see this tracked as a follow-up against @supabase/config instead.
| const jwtSecret = resolveJwtSecret( | ||
| envOverride("SUPABASE_AUTH_JWT_SECRET", config.auth.jwt_secret, projectEnvValues), | ||
| ); |
There was a problem hiding this comment.
Go runs DecryptSecretHookFunc for every config.Secret during UnmarshalExact, so auth.jwt_secret = "encrypted:..." is either decrypted with DOTENV_PRIVATE_KEY* or fails config loading before status/stop continues. This resolver passes the raw string straight into JWT-secret/key generation, so valid encrypted secrets produce the wrong local keys and malformed encrypted secrets can still let native status/default stop proceed; mirror the Go decrypt-or-fail step for secret fields before using them.
Useful? React with 👍 / 👎.
…NALYTICS_BACKEND overrides (review: #5765) Two Go-parity gaps in the status/stop config resolver: - Go defaults `auth.email.smtp.enabled = true` when `[auth.email.smtp]` is present but omits `enabled` (pkg/config/config.go:743-748), a presence-based default set on the raw viper map before the struct decodes. The shared schema always decodes a missing `enabled` key to `false`, erasing that presence signal, so this reads the raw TOML document instead (same approach already used for passkey/webauthn and experimental.webhooks). Mirrors the equivalent, already-correct check on the db/migration config-load path (legacy-db-config.toml-read.ts). - `SUPABASE_ANALYTICS_BACKEND` env overrides weren't validated against Go's `LogflareBackend` enum (config.go:60-65), so a bad override (e.g. "mysql") would continue where Go's `UnmarshalText` hard-fails config load. Validates the override-or-configured value with a single check, matching how viper merges both sources into one string before UnmarshalExact runs UnmarshalText exactly once.
…chema decode (review: #5765) Go's `(e external) validate()` (pkg/config/config.go:1418-1423) unconditionally deletes the deprecated `linkedin`/`slack` provider keys before the required-field loop runs, warning on stderr only when the deleted provider was enabled — a bare `[auth.external.slack] enabled = true` with no client_id/secret loads fine in Go. The shared schema still modeled `slack` as a fully-required provider (and had no `slack_oidc` entry at all, unlike `linkedin_oidc`), so `loadProjectConfig` rejected a config Go accepts. This affected every command that loads project config (status, stop, push, pull), not just status/stop. Adds `normalizeDeprecatedExternalProviders` in io.ts, mirroring the existing `normalizeDeprecatedSMTPSections`/`[inbucket]` precedent, and renames the `slack` provider to `slack_oidc` in the schema (config-sync/auth.sync.ts already had unreachable `slack_oidc` cases, confirming this was the intended id). Runs on the post-remote-merge document, not pre-merge like the SMTP normalizer, since Go's external.validate() runs once on the final effective config rather than eagerly over every [remotes.*] entry.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 46916aac3d
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (h.uri === undefined || h.uri.length === 0) { | ||
| throw new Error(`Missing required field in config: auth.hook.${hookType}.uri`); | ||
| } | ||
| const scheme = (/^([a-zA-Z][a-zA-Z0-9+.-]*):/u.exec(h.uri)?.[1] ?? "").toLowerCase(); |
There was a problem hiding this comment.
Fresh evidence: the new implementation only regex-extracts the URI scheme here, so an enabled hook with a malformed HTTP(S) URI such as http://[::1 and otherwise valid secrets is accepted. Go's hookConfig.validate calls url.Parse before the scheme switch and rejects that config during status/default stop config loading, before any Docker access; parse the full URI here rather than treating any http:/https: prefix as valid.
Useful? React with 👍 / 👎.
| ); | ||
| } | ||
|
|
||
| const config = yield* parseProjectConfig(normalizedForDecode, format, filePath); |
There was a problem hiding this comment.
When no projectRef is selected, remotes is still present in normalizedForDecode, so ProjectConfigSchema validates every nested remote block. That rejects configs Go accepts, e.g. an unselected [remotes.prod.auth.external.github] enabled = true stub without a secret, because Go's Config.Validate only checks remotes.*.project_id for remote blocks unless one is merged into the effective base config; status/default stop can therefore fail before Docker on a remote subtree Go ignores.
Useful? React with 👍 / 👎.
| for (const [name, tmpl] of Object.entries(email.template)) { | ||
| if (tmpl.content_path.length === 0) continue; | ||
| readEmailContentPath("template", name, tmpl.content_path, workdir); |
There was a problem hiding this comment.
When auth is enabled and a template uses the Go-modeled content field (for example [auth.email.template.invite] content = "..."), Go's Auth.Email.validate rejects it before status/default stop reaches Docker and asks the user to use content_path instead. The shared schema strips that unknown content key, so this loop just sees an empty content_path and accepts the config; add a raw-document check for template content (and enabled notification content) before continuing.
Useful? React with 👍 / 👎.
| // `Auth.Email.validate()`, then (skipping the unported Sms/External steps) | ||
| // `Auth.ThirdParty.validate()`, all right after the signing-keys read and | ||
| // still inside `if c.Auth.Enabled` (`pkg/config/config.go:1117-1153`). | ||
| // Sms/External (`config.go:1145-1150`) aren't ported. |
There was a problem hiding this comment.
When auth is enabled and config.toml declares an arbitrary OAuth provider such as [auth.external.custom] enabled = true without client_id/secret, Go decodes auth.external as a map and validates every enabled provider, so config loading fails before Docker access. The fixed TS schema drops provider keys it does not model and this block explicitly skips External, letting native status/default stop accept configs the Go CLI rejects; validate enabled raw auth.external.* entries under the same authEnabled gate.
Useful? React with 👍 / 👎.
…rify (review: #PRRT_kwDOErm0O86OqN9V)
Go's jwkToRSAPrivateKey (pkg/config/apikeys.go:132-168) builds the RSA
private key from n/e/d/p/q alone and its stdlib derives dp/dq/qi lazily
when absent, so a signing-keys JWK missing those CRT exponents still
signs successfully in Go. Node's createPrivateKey({ format: "jwk" })
has no such fallback and hard-rejects the same key, so native status
failed to generate local anon/service keys for a file the Go CLI
accepts. Compute the missing dp/dq/qi via BigInt before handing the
JWK to Node, matching Go's derivation.
…ew: #PRRT_kwDOErm0O86OqN9Z) Go's LoadEnvHook is the first mapstructure decode hook composed into UnmarshalExact (config.go:749-753,769-772), so it resolves a nested env(VAR) reference on any string mapstructure decodes into the struct, including a SUPABASE_* AutomaticEnv override string itself, not just a config.toml literal. envOverride returned that raw override string verbatim, so e.g. SUPABASE_API_ENABLED=env(API_ENABLED) was rejected as a malformed bool instead of resolving to the referenced value. Resolve one level of env(VAR) indirection with the same projectEnvValues/ process.env precedence and non-empty gate as the outer lookup, reusing packages/config's ENV_CAPTURE_REGEX (now re-exported) rather than duplicating the pattern.
…iew: #PRRT_kwDOErm0O86OrVR-, #PRRT_kwDOErm0O86OrzAH) Go's loadFromFile duplicate-check/selection loop over [remotes.*] reads viper's raw string values (config.go:596-610) and only resolves env(...) afterward, inside the struct decode's LoadEnvHook (config.go:611,749-753, decode_hooks.go:13-26). loadProjectConfigFile instead ran the match/merge against the already-interpolated document, so a [remotes.x] project_id = "env(REF)" would match a caller-supplied, already-resolved REF even though Go compares the literal env(REF) string and never selects that remote. Run the duplicate-check and match/merge against the raw, pre-interpolation document, then interpolate the merged result for the final decode. The project_id FORMAT check (Go's post-decode Config.Validate) still uses the interpolated remotes, since that check genuinely runs after env() resolution in Go.
… #PRRT_kwDOErm0O86OrVSC) Go's Docker.DaemonHost() (internal/utils/docker.go:41-54) comes from a client built via cli.Initialize(), whose endpoint resolution walks DOCKER_HOST -> DOCKER_CONTEXT -> the config file's currentContext -> the context store (docker/cli's getDockerEndPoint/resolveContextName) - not just DOCKER_HOST. legacyGetHostname only checked SUPABASE_SERVICES_HOSTNAME/DOCKER_HOST and fell back to 127.0.0.1, so with an active non-default Docker context and no DOCKER_HOST set, native status could correctly inspect a remote daemon (the docker/ podman binary its ps/inspect calls shell out to already honors the context) while printing unusable localhost API/DB/Studio URLs for it. Resolve the active context's tcp:// endpoint from $DOCKER_CONFIG/config.json and the context store's meta.json before falling back to the loopback default, mirroring Go's precedence.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b15c7dd935
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| Command.withHandler((flags) => | ||
| legacyStop(flags).pipe(withLegacyCommandInstrumentation({ flags }), withJsonErrorHandling), |
There was a problem hiding this comment.
When supabase stop -o csv or -o table is used, the TS global flag parser accepts those db-query-only values and this command proceeds into legacyStop, so containers can be stopped even though the Go CLI would reject the flag before stop.RunE runs. I checked the Go wiring: cmd/root.go:330 registers the root --output enum, and internal/utils/output.go:30-37 allows only env|pretty|json|toml|yaml; table|csv are not valid for stop. Please reject unsupported LegacyOutputFlag values for this native command before invoking the handler.
Useful? React with 👍 / 👎.
| if (goFmt === "pretty") { | ||
| yield* renderPretty(); | ||
| return; | ||
| } |
There was a problem hiding this comment.
When supabase status -o csv or -o table is used, the TS root parser accepts these db-query-only values, but this branch does not reject them and falls through to the normal pretty/status path after config loading and Docker inspection. In Go, the root --output enum registered at apps/cli-go/cmd/root.go:330 only allows env|pretty|json|toml|yaml (apps/cli-go/internal/utils/output.go:30-37), so those invocations fail during flag parsing before status.Run touches Docker; add the same command-level rejection here for unsupported LegacyOutputFlag values.
Useful? React with 👍 / 👎.
| const loaded = yield* loadProjectConfig(cliConfig.workdir, { | ||
| projectEnv: projectEnv !== null ? { ...projectEnv, values: projectEnvValues } : undefined, |
There was a problem hiding this comment.
When auth is enabled and multiple SMS providers are present, Go's sms.validate() uses a switch over providers, so if [auth.sms.twilio] is enabled and complete, a later incomplete [auth.sms.messagebird] enabled = true block is not inspected and status/default stop continue. This load path decodes through @supabase/config, whose per-provider requiredWhenEnabled checks validate every enabled SMS provider, so native status and default stop can reject a stale secondary SMS block that the Go CLI accepts before reaching Docker.
Useful? React with 👍 / 👎.
…o (review: #PRRT_kwDOErm0O86OrzAK) Go's loadDefaultEnv (apps/cli-go/pkg/config/config.go:1243-1250) omits .env.local from its candidate dotenv list whenever SUPABASE_ENV=test, so a malformed or intentionally non-test supabase/.env.local is invisible to Go in that mode. status/stop's preliminary loadProjectEnvironment call had no such gate and its parse failure propagates as a real command failure, unlike the later legacyResolveProjectEnvironmentValues pass which already re-derives the file list per SUPABASE_ENV. Add an opt-in skipEnvLocal option to loadProjectEnvironment (default off, so next/ and secrets set are unaffected) and pass it from both status.handler.ts and stop.handler.ts when SUPABASE_ENV=test.
…ields (review: #PRRT_kwDOErm0O86OrzAR)
Go's newDecodeHook (apps/cli-go/pkg/config/config.go:775-784) wires
mapstructure.StringToSliceHookFunc(",") unconditionally into config
decode, so a plain string value for a []string field (e.g.
auth.additional_redirect_urls = "http://a,http://b") decodes fine in
Go, whether the string is a literal TOML value or an env()-substituted
one. @supabase/config's schema had no equivalent and rejected any
string value for an array-typed field outright.
Extend the existing pre-decode env() coercion walker
(interpolateEnvReferencesAgainstSchema) with an "array" ExpectedType
for homogeneous string arrays, and apply it to any string leaf at that
path (not just env()-substituted ones, since Go's hook fires
regardless of value source) by splitting on "," -- matching Go's
StringToSliceHookFunc exactly, including its empty-string-to-empty-
slice case.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 09e218a2e4
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // `legacyCliProjectFilterValue`'s doc comment). | ||
| const projectId = legacySanitizeProjectId( | ||
| legacyResolveLocalProjectId( | ||
| projectEnvValues["SUPABASE_PROJECT_ID"] ?? process.env["SUPABASE_PROJECT_ID"], |
There was a problem hiding this comment.
When SUPABASE_PROJECT_ID=env(PROJECT_REF) is set and PROJECT_REF contains the real local project id, Go resolves that indirection during Config.Load before status builds utils.Config.ProjectId, so Docker inspect/list uses the resolved label and container ids. This line bypasses the envOverride path that was added for other SUPABASE_* fields and feeds the literal env(PROJECT_REF) into legacyResolveLocalProjectId, so native status looks for supabase_db_env_PROJECT_REF_ / com.supabase.cli.project=env_PROJECT_REF_ and can report a running Go-started stack as missing; the mirrored default stop path has the same raw lookup.
Useful? React with 👍 / 👎.
| const authDocument = asRecord(document?.["auth"]); | ||
| validatePasskeyWebauthn(authDocument); | ||
| validateAuthHooks(config.auth.hook); | ||
| validateMfaConfig(config.auth.mfa); |
There was a problem hiding this comment.
When auth is enabled and a nested Viper override such as SUPABASE_AUTH_MFA_TOTP_ENROLL_ENABLED=true is supplied without the matching SUPABASE_AUTH_MFA_TOTP_VERIFY_ENABLED=true, Go's ExperimentalBindStruct/AutomaticEnv applies that value before Auth.MFA.validate() and rejects the config. This call validates only the TOML-decoded config.auth.mfa, so env-only nested MFA settings remain at their defaults and native status/default stop can continue for a config the Go CLI refuses; apply the same nested SUPABASE_AUTH_MFA_* overrides before running this validator.
Useful? React with 👍 / 👎.
| const loaded = yield* loadProjectConfig(cliConfig.workdir, { | ||
| projectEnv: projectEnv !== null ? { ...projectEnv, values: projectEnvValues } : undefined, | ||
| search: false, |
There was a problem hiding this comment.
When the workdir basename is a valid project ref and [remotes.<name>].project_id matches it, Go mutates Config.ProjectId to that basename in Eject() before the remote-selection loop and merges the matching remote before validation/output. This loadProjectConfig call never passes that implicit ref, so applyRemoteOverride cannot select the remote and native status (and the mirrored default stop path) can validate and report values from the base config while the Go CLI would use the remote-specific config.
Useful? React with 👍 / 👎.
| if (substituted === document) { | ||
| return document; |
There was a problem hiding this comment.
When a TOML scalar is quoted for a number or boolean field, such as db.port = "54322" or api.enabled = "false", Go's Viper/mapstructure UnmarshalExact weakly coerces it before validation, so status and the default stop path continue. This branch returns the original string whenever it was not produced by env(), leaving the strict schema to reject configs the Go CLI accepts; apply the same scalar coercion for literal strings at number/boolean schema paths.
Useful? React with 👍 / 👎.
| validateMfaConfig(config.auth.mfa); | ||
| validateAuthEmailTemplates(config.auth.email, workdir); | ||
| validateAuthEmailSmtp(asRecord(authDocument?.["email"])); | ||
| validateThirdPartyAuth(config.auth.third_party); |
There was a problem hiding this comment.
When auth is enabled and SUPABASE_AUTH_THIRD_PARTY_FIREBASE_ENABLED=true is provided without SUPABASE_AUTH_THIRD_PARTY_FIREBASE_PROJECT_ID, Go's Viper ExperimentalBindStruct/AutomaticEnv applies the nested env value before Auth.ThirdParty.validate() and rejects config loading. This call only sees config.auth.third_party decoded from TOML, so env-only third-party providers remain disabled and native status/default stop can continue for a config Go refuses; apply nested SUPABASE_AUTH_THIRD_PARTY_* overrides before validating.
Useful? React with 👍 / 👎.
| ); | ||
| } | ||
|
|
||
| const config = yield* parseProjectConfig(normalizedForDecode, format, filePath); |
There was a problem hiding this comment.
When config.toml contains an invalid Go-decoded scalar such as db.health_timeout = "nope", auth.email.max_frequency = "soon", or storage.file_size_limit = "bogus", Go's mapstructure hook chain (StringToTimeDurationHookFunc/TextUnmarshallerHookFunc) fails UnmarshalExact before status/default stop touches Docker. This parse path feeds the normalized document into the permissive TS schema, where those fields are just strings (or string/number) and no Go duration/size parser runs, so native status/default stop can accept configs the Go CLI rejects; validate the text-unmarshalled scalar formats before returning the config.
Useful? React with 👍 / 👎.
1 participant
Summary
Ports
supabase stopandsupabase statusfrom Go-proxy stubs to native TypeScript in the legacy CLI shell (CLI-1324).startis still Go-proxied, so this intentionally does not route through@supabase/stack/effect's daemon-based orchestration model — that substrate manages a different set of containers than the ones Go's binary actually creates, and using it would silently no-op against a real running stack.legacy-docker-lifecycle,legacy-go-jwt,legacy-local-config-values,legacy-api-url) is reused by both commands, matching Go's local-dev defaults exactly — including a Go-byte-exact JWT signer, since@supabase/stack's own JWT generator uses a different issuer/claim order than what Go prints for local dev keys.*.live.test.tsas a documented test category (AGENTS.md) alongside unit/integration/e2e: black-box subprocess tests run by thecli-e2e-ciharness against a real platform.stop/statusdon't call the Management API, so their live tests spin up a real local Docker stack instead and verify against it directly (e.g. confirming Docker itself has no containers left afterstop, not just trusting the CLI's exit code).Notable review findings fixed along the way
stderr's TTY status while writing tostdout— piping stdout while stderr stayed a TTY (supabase status | less) would have corrupted output with ANSI escapes (the same bug class CLI-1546 fixed once before).--override-namewas leaking into pretty-mode output; Go'sPrettyPrintrebuilds a fresh, un-overridden view and ignores it there.--backup/--no-backup: Go's--backupflag is dead code (declared, never bound to a variable incmd/stop.go) — the port now matches that exactly instead of an intended-but-never-true semantic.CLOSES CLI-1324