Uh oh!
There was an error while loading. Please reload this page.
fix(cli): secrets set tolerates a malformed config.toml (Go parity) - #5796
Conversation
## Current Behavior Go's `secrets set` (`internal/secrets/set/set.go:20-24`) swallows a malformed `supabase/config.toml` parse error to the debug logger and proceeds with empty `EdgeRuntime.Secrets` — env-file and positional-arg secrets still apply. `secrets set` has no `--linked`/`--local`/`--db-url` flag, so the root PreRun never loads the config first either; this is the only load. The TS port instead let the parse error propagate as a fatal `LegacySecretsConfigParseError`, aborting the whole command even when the user only wanted to set a secret via CLI arg or env-file. ## Expected Behavior - Catches `ProjectConfigParseError`, logs it to the debug logger, and continues with no config-declared secrets, matching Go. - The logged message is truncated before any blank-line-separated source snippet: `smol-toml`'s `TomlError` (and some schema-decode errors) embed a codeblock of the surrounding file lines in their message, which for this file's `[edge_runtime.secrets]` section can include literal secret values. Go's equivalent log line (`DecodeError.Error()`) is a short, content-free message — its verbose `.String()` with the snippet is never called by `set.go`. Truncating avoids echoing a user's own secret value into `--debug` output next to an unrelated syntax error. - Deletes the now-unused `LegacySecretsConfigParseError` (no remaining references). - Threads `LegacyDebugLogger` into the shared `legacyManagementApiRuntimeLayer` (used by many legacy commands) since the layer was already building the same instance internally for `cliConfig`/`httpClient`/`credentials` but never exposed it at the top level — the file's own doc comment describes this as the sanctioned way to add a new commonly-needed top-level service. Fixes CLI-1867
Coly010
commented
Jul 6, 2026
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit:923815ff72
ℹ️ 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".
Uh oh!
There was an error while loading. Please reload this page.
…ors (review: #5796) Go's viper+mapstructure decode (pkg/config/config.go:749) mutates the target struct field-by-field, so a type error on an unrelated key (e.g. analytics.port) doesn't stop EdgeRuntime.Secrets from landing on utils.Config — confirmed empirically against pkg/config. Effect Schema.decodeUnknownSync is all-or-nothing, so `secrets set` was silently discarding a valid [edge_runtime.secrets] section whenever any other field in config.toml failed schema decode. ProjectConfigParseError now carries the pre-decode document when the failure is a schema-decode error (not a raw parse error) so secrets set can re-decode just the edge_runtime subtree against the full schema and recover its secrets, without loosening decode semantics for other @supabase/config callers. A genuine parse failure still has no recoverable structure in either implementation.
Coly010
commented
Jul 6, 2026
@codex review |
1 similar comment
Coly010
commented
Jul 6, 2026
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit:19dc72c044
ℹ️ 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".
Uh oh!
There was an error while loading. Please reload this page.
… (review: #5796) Re-decoding the whole edge_runtime subtree still failed recovery when the schema error came from a sibling field in the same table (e.g. edge_runtime.inspector_port), discarding a valid [edge_runtime.secrets] block. Verified against pkg/config directly that Go's mapstructure decode tolerates this case and still populates EdgeRuntime.Secrets, so recoverEdgeRuntimeConfig now re-slices just edge_runtime.secrets instead of the whole edge_runtime table.
The previous commit's cast (`edgeRuntime as Record<string, unknown>`) violates this repo's no-`as`-casts policy; a local `isRecord` type predicate narrows `unknown` without it. Also fixes an oxfmt formatting drift in SIDE_EFFECTS.md left over from an earlier commit in this PR.
Coly010
commented
Jul 6, 2026
@codex review |
Codex Review: Didn't find any major issues. Hooray! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
Supabase CLI previewnpx --yes https://pkg.pr.new/supabase/cli/supabase@3153365d1d092a739b2d68b1a00fc9b76aaee6cbPreview package for commit |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit:a2b843d298
ℹ️ 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".
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
…only (review: #5796) Several callers of loadProjectConfig (gen types, next start, functions dev/serve/deploy) don't catch ProjectConfigParseError, so the attached document propagates uncaught. Only secrets set's recoverEdgeRuntimeConfig reads it, and only edge_runtime.secrets, so attaching the whole post-interpolation document (including literal edge_runtime.secrets values) is unnecessary blast radius for a TS-only telemetry surface with no Go equivalent to weigh against.
…5796) Regression test for a mixed valid/invalid edge_runtime.secrets map (GOOD = "ok" alongside BAD = 123). recoverEdgeRuntimeConfig's per-entry filtering (landed alongside the ProjectConfigParseError.document narrowing) now keeps GOOD instead of discarding the whole map, matching Go's mapstructure map decode which decodes each entry independently.
…review: #5796) loadProjectConfig resolves env(VAR) references against .env/.env.local before schema decode, so a malformed dotenv line fails with ProjectEnvParseError rather than ProjectConfigParseError, escaping the existing catchTag and aborting the command. Go's Load() (pkg/config/config.go:788-791) calls loadNestedEnv first too and swallows any resulting error the same way internal/secrets/set/set.go swallows a bad config.toml, so secrets set must stay non-fatal here as well and keep working from env-file/positional args.
jgoux
left a comment
There was a problem hiding this comment.
I found three remaining issues in the current head. The first two are Go-parity behavior gaps in secrets set; the third is a sensitive-data hardening issue in the config error shape.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
…ocument (review: #5796) Several loadProjectConfig callers (gen types, next start, functions dev/serve/deploy) don't catch ProjectConfigParseError, so its document field can propagate uncaught. Wrap edge_runtime.secrets values in Redacted before attaching, matching the x-secret treatment secret() already gets elsewhere in this package, so an uncaught error can't carry a resolved secret in plaintext. secrets set's recoverEdgeRuntimeConfig/filterDecodableSecrets unwrap via Redacted.value before re-decoding.
Coly010
commented
Jul 7, 2026
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit:4f58020207
ℹ️ 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".
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
…s (review: #5796) isRecord() treated arrays as records, so `[edge_runtime] secrets = [...]` fell through filterDecodableSecrets and Object.entries turned array indices into fabricated secret names (e.g. "0"). Go's mapstructure never sets WeaklyTypedInput, so a slice source for a map-typed field hits UnconvertibleTypeError and the whole field is left empty instead.
…eview: #5796) loadProjectConfig() omitted { projectRef: ref }, so a schema-decode recovery on `secrets set --project-ref <ref>` used the base [edge_runtime.secrets] instead of a matching [remotes.*] override. Go seeds Config.ProjectId before Load() and merges the override in loadFromFile before mapstructure ever runs, so the recovery must load against the same resolved ref. Also swallow DuplicateRemoteProjectIdError non-fatally, matching Go's LoadConfig catch-all.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit:5ff3823e87
ℹ️ 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".
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
…5796) Codex flagged that redactEdgeRuntimeSecrets only wrapped string secret values in Redacted before attaching them to ProjectConfigParseError.document, so a malformed entry with a non-string shape (e.g. a TOML array or inline table) still carried the secret in plaintext. Redacted.make accepts any value and always stringifies to <redacted:...>, so wrap every entry unconditionally instead of gating on typeof.
…ew: #5796) Go's DecryptSecretHookFunc (pkg/config/secret.go:98) never hashes an empty secret value, and ListSecrets (internal/secrets/set/set.go:48-52) only includes entries with a non-empty SHA256, so an empty config-declared secret is never sent and can't overwrite a same-named remote secret with an empty string. The TS merge loop only checked Redacted.isRedacted, which missed the empty-value case for both the happy-path config load and the malformed-config recovery path (both flow through the same loop). Require a non-empty value too so both of Go's zero-hash cases match.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit:de9c5ffcf9
ℹ️ 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".
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
…rrors (review: #5796) `redactEdgeRuntimeSecrets` only wrapped per-entry values inside an `edge_runtime.secrets` table; a malformed `secrets` field that is itself non-object (e.g. `secrets = ["actual-secret"]`) fell through its early return and reached `ProjectConfigParseError.document` unredacted. Wrap the whole field in `Redacted` in that case too, and unwrap it in `secrets set`'s `recoverEdgeRuntimeConfig` before the `isRecord` check so the wrapper object itself isn't misread as a one-entry secrets map.
…rrors (review: #5796) `secrets set --debug` stringified the raw schema-decode error when a malformed `[edge_runtime.secrets]` entry failed decode, which embeds the rejected value inline (e.g. `Expected string, actual ["actual-secret"]`) with no blank-line separator for the existing truncation to catch. Go's pinned mapstructure decode-error types never include the rejected value, only type names, so use a fixed message for schema-decode failures and keep the truncation for genuine TOML syntax errors, where it already matches Go.
Coly010
commented
Jul 7, 2026
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit:75e09e27c2
ℹ️ 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".
Uh oh!
There was an error while loading. Please reload this page.
Go's loadFromFile prints "Loading config override: [remotes.<name>]" to stderr unconditionally when a [remotes.*] block matches the resolved ref (pkg/config/config.go:605), before mapstructure decode ever runs. The ref-aware loadProjectConfig call added to secrets set mapped the loaded result straight down to .config, discarding appliedRemote and silently dropping the notice for remote-specific configs, unlike config push and the storage/seed handlers which already surface it.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit:7cbf5f303d
ℹ️ 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".
Uh oh!
There was an error while loading. Please reload this page.
…crets set (review: #5796) Go's loadFromFile prints "Loading config override: [remotes.<name>]" to stderr before mapstructure decode runs, so the notice still fires even when the subsequent decode fails and secrets set recovers non-fatally. ProjectConfigParseError now carries appliedRemote through the failed decode so the catch path can surface it, matching the success path.
Coly010
commented
Jul 7, 2026
@codex review |
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Coly010
commented
Jul 7, 2026
@codex review |
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Uh oh!
There was an error while loading. Please reload this page.
Current Behavior
Go's
secrets set(internal/secrets/set/set.go:20-24) swallows a malformedsupabase/config.tomlparse error to the debug logger and proceeds with emptyEdgeRuntime.Secrets— env-file and positional-arg secrets still apply.secrets sethas no--linked/--local/--db-urlflag, so the root PreRun never loads the config first either; this is the only load. The TS port instead let the parse error propagate as a fatalLegacySecretsConfigParseError, aborting the whole command even when the user only wanted to set a secret via CLI arg or env-file.Expected Behavior
ProjectConfigParseError, logs it to the debug logger, and continues with no config-declared secrets, matching Go.smol-toml'sTomlError(and some schema-decode errors) embed a codeblock of the surrounding file lines in their message, which for this file's[edge_runtime.secrets]section can include literal secret values. Go's equivalent log line (DecodeError.Error()) is a short, content-free message — its verbose.String()with the snippet is never called byset.go. Truncating avoids echoing a user's own secret value into--debugoutput next to an unrelated syntax error.LegacySecretsConfigParseError(no remaining references).LegacyDebugLoggerinto the sharedlegacyManagementApiRuntimeLayer(used by many legacy commands) since the layer was already building the same instance internally forcliConfig/httpClient/credentialsbut never exposed it at the top level — the file's own doc comment describes this as the sanctioned way to add a new commonly-needed top-level service.Fixes CLI-1867