diff --git a/apps/cli/src/legacy/commands/secrets/secrets.errors.ts b/apps/cli/src/legacy/commands/secrets/secrets.errors.ts index 7094cf2336..385c0911fe 100644 --- a/apps/cli/src/legacy/commands/secrets/secrets.errors.ts +++ b/apps/cli/src/legacy/commands/secrets/secrets.errors.ts @@ -82,9 +82,3 @@ export class LegacySecretsUnsetCancelledError extends Data.TaggedError( )<{ readonly message: string; }> {} - -export class LegacySecretsConfigParseError extends Data.TaggedError( - "LegacySecretsConfigParseError", -)<{ - readonly message: string; -}> {} diff --git a/apps/cli/src/legacy/commands/secrets/set/SIDE_EFFECTS.md b/apps/cli/src/legacy/commands/secrets/set/SIDE_EFFECTS.md index c99884485e..215c28de17 100644 --- a/apps/cli/src/legacy/commands/secrets/set/SIDE_EFFECTS.md +++ b/apps/cli/src/legacy/commands/secrets/set/SIDE_EFFECTS.md @@ -2,17 +2,17 @@ ## Files Read -| Path | Format | When | -| ----------------------------------------- | ------------------------- | --------------------------------------------------------------------------------------------- | -| `/proc/sys/kernel/osrelease` (Linux) | plain text | once on layer init — disables keyring on WSL (`WSL` / `Microsoft` substring match) | -| keyring `"Supabase CLI"` / `` | OS keychain | when `SUPABASE_ACCESS_TOKEN` unset and keyring available; account = `LegacyCliConfig.profile` | -| keyring `"Supabase CLI"` / `access-token` | OS keychain | legacy-key fallback when the profile-keyed lookup misses | -| `~/.supabase/access-token` | plain text (token string) | last-resort fallback after env + keyring miss | -| `/supabase/.temp/project-ref` | plain text | when `--project-ref` and `SUPABASE_PROJECT_ID` are both unset | -| `/supabase/config.toml` | TOML | always (for `[edge_runtime.secrets]`) — via `@supabase/config`'s `loadProjectConfig` | -| `/.env` | dotenv | always — context for `env(VAR)` interpolation in `[edge_runtime.secrets]` values | -| `/.env.local` | dotenv | always — overrides `.env` for `env(VAR)` interpolation context | -| `` (absolute or CWD-relative) | dotenv | when `--env-file` flag is provided | +| Path | Format | When | +| ----------------------------------------- | ------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | +| `/proc/sys/kernel/osrelease` (Linux) | plain text | once on layer init — disables keyring on WSL (`WSL` / `Microsoft` substring match) | +| keyring `"Supabase CLI"` / `` | OS keychain | when `SUPABASE_ACCESS_TOKEN` unset and keyring available; account = `LegacyCliConfig.profile` | +| keyring `"Supabase CLI"` / `access-token` | OS keychain | legacy-key fallback when the profile-keyed lookup misses | +| `~/.supabase/access-token` | plain text (token string) | last-resort fallback after env + keyring miss | +| `/supabase/.temp/project-ref` | plain text | when `--project-ref` and `SUPABASE_PROJECT_ID` are both unset | +| `/supabase/config.toml` | TOML | always (for `[edge_runtime.secrets]`) — via `@supabase/config`'s `loadProjectConfig`; a parse failure is logged to the debug logger and tolerated (Go parity), not fatal | +| `/.env` | dotenv | always — context for `env(VAR)` interpolation in `[edge_runtime.secrets]` values | +| `/.env.local` | dotenv | always — overrides `.env` for `env(VAR)` interpolation context | +| `` (absolute or CWD-relative) | dotenv | when `--env-file` flag is provided | ## Files Written @@ -52,7 +52,6 @@ | `1` | `LegacyInvalidSecretPairError` — positional argument missing `=` | | `1` | `LegacySecretsEnvFileOpenError` — `--env-file` cannot be opened | | `1` | `LegacySecretsEnvFileParseError` — `--env-file` cannot be parsed | -| `1` | `LegacySecretsConfigParseError` — `supabase/config.toml` cannot be parsed | | `1` | `LegacySecretsSetUnexpectedStatusError` — non-2xx response from POST | | `1` | `LegacySecretsSetNetworkError` — transport-level network failure | @@ -85,4 +84,5 @@ One `result` NDJSON event on success containing `{project_ref, count}`. - Source order for merging entries: `[edge_runtime.secrets]` from `config.toml` (only resolved entries — see below) → `--env-file` (overrides config) → CLI args (overrides env-file). - `SUPABASE_`-prefixed entries are skipped post-merge with a stderr warning. - `[edge_runtime.secrets]` from config.toml is read via `@supabase/config`'s `loadProjectConfig` + `resolveProjectSubtree`. Resolved secret values arrive wrapped in `Redacted`; unresolved `env(VAR)` literals (env var unset) stay as plain strings and are filtered out at the handler — matches Go's `set.go:48-52` which filters by `len(secret.SHA256) > 0` (the SHA256 is empty when `DecryptSecretHookFunc` sees a still-literal `env(VAR)`). +- A malformed `config.toml` does **not** abort the command — matches Go's `set.go:20-24`, which logs the `LoadConfig` error to the debug logger and proceeds. `--env-file` and positional `NAME=VALUE` secrets always still apply. What happens to config-declared secrets depends on the failure class, matching Go's `viper`+`mapstructure` decode (`pkg/config/config.go:749`), which mutates the target struct field-by-field: a raw TOML/JSON syntax error drops everything (no `EdgeRuntime.Secrets`), but a schema-type error on an _unrelated_ field (e.g. `analytics.port` being a string) still leaves a valid `[edge_runtime.secrets]` section usable — the handler recovers it by re-decoding just that subtree. Pass `--debug` to see the logged parse error. - Sends `User-Agent: SupabaseCLI/` and Bearer auth. No `X-Supabase-Command` headers — Go parity. diff --git a/apps/cli/src/legacy/commands/secrets/set/set.handler.ts b/apps/cli/src/legacy/commands/secrets/set/set.handler.ts index 422f503070..5ecfe9b69a 100644 --- a/apps/cli/src/legacy/commands/secrets/set/set.handler.ts +++ b/apps/cli/src/legacy/commands/secrets/set/set.handler.ts @@ -1,9 +1,17 @@ -import { loadProjectConfig, loadProjectEnvironment, resolveProjectSubtree } from "@supabase/config"; +import { + loadProjectConfig, + loadProjectEnvironment, + ProjectConfigSchema, + resolveProjectSubtree, + type ProjectConfig, + type ProjectConfigParseError, +} from "@supabase/config"; import { parse as parseDotenv } from "dotenv"; -import { Effect, FileSystem, Option, Path, Redacted } from "effect"; +import { Effect, FileSystem, Option, Path, Redacted, Schema } from "effect"; import { LegacyPlatformApi } from "../../../auth/legacy-platform-api.service.ts"; import { LegacyProjectRefResolver } from "../../../config/legacy-project-ref.service.ts"; +import { LegacyDebugLogger } from "../../../shared/legacy-debug-logger.service.ts"; import { LegacyLinkedProjectCache } from "../../../telemetry/legacy-linked-project-cache.service.ts"; import { LegacyTelemetryState } from "../../../telemetry/legacy-telemetry-state.service.ts"; import { Output } from "../../../../shared/output/output.service.ts"; @@ -11,7 +19,6 @@ import { RuntimeInfo } from "../../../../shared/runtime/runtime-info.service.ts" import { mapLegacyHttpError } from "../../../shared/legacy-http-errors.ts"; import { LegacyInvalidSecretPairError, - LegacySecretsConfigParseError, LegacySecretsEnvFileOpenError, LegacySecretsEnvFileParseError, LegacySecretsNoArgumentsError, @@ -27,12 +34,108 @@ const mapSetError = mapLegacyHttpError({ statusMessage: (_status, body) => `Unexpected error setting project secrets: ${body}`, }); +const decodeProjectConfig = Schema.decodeUnknownSync(ProjectConfigSchema); + +// Excludes arrays, matching `packages/config/src/io.ts`'s `isObject` (the +// identical "is this a table" check used when merging `[remotes.*]`). A TOML +// array for a map-typed field (e.g. `[edge_runtime] secrets = ["actual-secret"]`) +// is not a recoverable table: `Object.entries` on an array yields index keys +// ("0", "1", ...), which would otherwise fabricate spurious secret names. Go's +// mapstructure decoder never does this either — `UnmarshalExact` +// (`apps/cli-go/pkg/config/config.go:749`) never sets `WeaklyTypedInput`, so a +// slice source for a map-typed field hits `UnconvertibleTypeError` in +// `decodeMap` rather than the index-as-key `decodeMapFromSlice` path, and the +// whole field is left empty. +function isRecord(value: unknown): value is Record { + return typeof value === "object" && value !== null && !Array.isArray(value); +} + +/** + * Best-effort recovery for a schema-decode failure (as opposed to a raw + * TOML/JSON parse failure) on `supabase/config.toml`. Go's `viper`+ + * `mapstructure` decode (`apps/cli-go/pkg/config/config.go:749`) mutates the + * target struct field-by-field: a type error anywhere — an unrelated + * top-level table (`analytics.port`), a sibling field inside the same + * `edge_runtime` table (`edge_runtime.inspector_port`), *or* a single bad + * entry inside the `edge_runtime.secrets` map itself (`BAD = 123`) — does not + * stop the rest of `edge_runtime.secrets` from landing in `utils.Config`. + * `UnmarshalExact` still populates every field (and every map entry) it *can* + * decode before aggregating errors: `mapstructure`'s map decoder + * (`decodeMapFromMap`) iterates each key independently, appends a per-entry + * error and `continue`s rather than aborting, then still calls `val.Set` with + * whatever entries succeeded. Confirmed empirically against this repo's + * actual `pkg/config`: a TOML with both a malformed `edge_runtime.inspector_port` + * and a valid `[edge_runtime.secrets]` block still yields a populated + * `EdgeRuntime.Secrets` (`InspectorPort` is left at its zero value), and a + * `[edge_runtime.secrets]` block with one bad entry alongside a good one + * still yields the good entry. + * `Schema.decodeUnknownSync` has no such tolerance; a single bad field + * anywhere discards the whole decode — re-decoding the *entire* `edge_runtime` + * subtree would still fail in the sibling-field case (`inspector_port` comes + * along for the ride), and re-decoding the whole `secrets` map atomically + * would still fail when just one entry in that map is bad. To keep + * `secrets set` at parity without loosening `packages/config`'s decode + * semantics for every caller: re-slice `edge_runtime.secrets` out of the + * pre-decode document (`cause.document` — only set when the document itself + * parsed fine and the *schema* decode is what failed, see + * `ProjectConfigParseError`), decode each entry independently and keep only + * the ones that succeed (mirroring `decodeMapFromMap`'s per-key tolerance), + * then decode the filtered map against the full schema, where every other + * field (including the rest of `edge_runtime`) defaults cleanly. A true parse + * failure (`cause.document` undefined) has no recoverable structure in either + * implementation — Go's own `viper.MergeConfig` also fails the whole load + * before `mapstructure` ever runs in that case. + */ +function recoverEdgeRuntimeConfig(cause: ProjectConfigParseError): ProjectConfig | null { + if (cause.document === undefined) { + return null; + } + const edgeRuntime = cause.document.edge_runtime; + const secrets = isRecord(edgeRuntime) ? edgeRuntime.secrets : undefined; + const decodableSecrets = isRecord(secrets) ? filterDecodableSecrets(secrets) : undefined; + try { + return decodeProjectConfig({ + edge_runtime: decodableSecrets !== undefined ? { secrets: decodableSecrets } : {}, + }); + } catch { + return null; + } +} + +/** + * Mirrors mapstructure's per-entry map decode tolerance + * (`decodeMapFromMap`, invoked via `v.UnmarshalExact` in + * `apps/cli-go/pkg/config/config.go:749`): a decode error on one secret + * value doesn't discard the whole `[edge_runtime.secrets]` map — only that + * entry is dropped, and every other entry is still recovered. + * + * Each value arrives as `Redacted` — `ProjectConfigParseError.document` + * wraps `edge_runtime.secrets` values so an uncaught parse error can't leak a + * resolved secret into a log or trace (see the field doc on `.document`). + * Unwrap before re-decoding: `secret()`'s schema is a plain `Schema.String`, + * not `Redacted`. + */ +function filterDecodableSecrets(secrets: Record): Record { + const kept: Record = {}; + for (const [name, value] of Object.entries(secrets)) { + const plainValue = Redacted.isRedacted(value) ? Redacted.value(value) : value; + try { + decodeProjectConfig({ edge_runtime: { secrets: { [name]: plainValue } } }); + kept[name] = plainValue; + } catch { + // Drop this entry only, matching mapstructure's per-key error handling. + } + } + return kept; +} + export const legacySecretsSet = Effect.fn("legacy.secrets.set")(function* ( flags: LegacySecretsSetFlags, ) { const output = yield* Output; const api = yield* LegacyPlatformApi; const resolver = yield* LegacyProjectRefResolver; + const debugLogger = yield* LegacyDebugLogger; const linkedProjectCache = yield* LegacyLinkedProjectCache; const telemetryState = yield* LegacyTelemetryState; const runtimeInfo = yield* RuntimeInfo; @@ -53,23 +156,70 @@ export const legacySecretsSet = Effect.fn("legacy.secrets.set")(function* ( // literals stay as plain strings, so `Redacted.isRedacted(...)` is the // equivalent guard. const merged = new Map(); - const loaded = yield* loadProjectConfig(runtimeInfo.cwd).pipe( - Effect.catchTag("ProjectConfigParseError", (cause) => - Effect.fail( - new LegacySecretsConfigParseError({ - message: `failed to parse supabase/config.toml: ${String(cause.cause)}`, - }), - ), + // Go swallows a malformed config.toml (or a malformed `.env`/`.env.local` + // sibling — see the `ProjectEnvParseError` catch below) here + // (`internal/secrets/set/set.go:20-24`: `fmt.Fprintln(utils.GetDebugLogger(), err)`) + // and proceeds with an empty `EdgeRuntime.Secrets` — env-file and + // positional-arg secrets still work. `secrets set` has no + // `--linked`/`--local`/`--db-url` flag, so (unlike most commands) the root + // `PreRun` never loads the config first either; this is the only load, and + // it must not be fatal. + // + // Pass `ref` so a matching `[remotes.*]` block is merged over the base + // config before decode, mirroring Go's `flags.LoadConfig` + // (`internal/utils/flags/config_path.go:11-12`: `utils.Config.ProjectId = + // ProjectRef` before `Load()`) merging the override in `loadFromFile` + // (`pkg/config/config.go:604-609`) ahead of the tolerant decode below. + // Without this, a schema-decode error on `--project-ref ` + // would recover the *base* `[edge_runtime.secrets]` instead of the + // explicitly selected remote's override. + const loadedConfig = yield* loadProjectConfig(runtimeInfo.cwd, { projectRef: ref }).pipe( + Effect.map((loaded) => loaded?.config ?? null), + Effect.catchTag("ProjectConfigParseError", (cause) => { + // `smol-toml`'s `TomlError` (and some schema-decode errors) embed a + // source codeblock after a blank-line separator — literal file content, + // which for this file's `[edge_runtime.secrets]` section can include + // real secret values. Go's equivalent log line (`DecodeError.Error()`) + // is a short, content-free message; only its unused `.String()` method + // includes a snippet, and Go's `set.go:20-24` never calls it. Truncate + // before the separator so a syntax error next to a secret line can't + // echo that secret to `--debug` output. + const shortMessage = String(cause.cause).split("\n\n")[0]; + return debugLogger + .debug(`failed to parse supabase/config.toml: ${shortMessage}`) + .pipe(Effect.as(recoverEdgeRuntimeConfig(cause))); + }), + // `loadProjectConfig` resolves `env(VAR)` references against + // `.env`/`.env.local` (`loadProjectEnvironment` inside + // `loadProjectConfigFile`) *before* schema decode, so a malformed dotenv + // line fails with this distinct tag rather than `ProjectConfigParseError`. + // Go's `Load()` (`pkg/config/config.go:788-791`) calls `loadNestedEnv` + // first too and returns immediately on error, before `loadFromFile` (the + // TOML parse) ever runs — so `EdgeRuntime.Secrets` never gets populated + // in this failure path, unlike the schema-decode-only case above. Recover + // to `null`, not `recoverEdgeRuntimeConfig`: there is no parsed document + // to recover a subtree from. + Effect.catchTag("ProjectEnvParseError", (cause) => + debugLogger.debug(`failed to parse ${cause.path}:${cause.line}`).pipe(Effect.as(null)), + ), + // Two `[remotes.*]` blocks declare the same `project_id` as `ref` — Go's + // `flags.LoadConfig` swallows *any* `Load()` error non-fatally + // (`internal/secrets/set/set.go:22-24`), including this one, which + // `loadFromFile` raises before `mapstructure` ever runs + // (`pkg/config/config.go:601`). `cause.message` already matches Go's + // string verbatim (see `DuplicateRemoteProjectIdError`'s field doc). + Effect.catchTag("DuplicateRemoteProjectIdError", (cause) => + debugLogger.debug(cause.message).pipe(Effect.as(null)), ), ); - if (loaded !== null) { + if (loadedConfig !== null) { const projectEnv = yield* loadProjectEnvironment({ cwd: runtimeInfo.cwd, baseEnv: process.env, }); if (projectEnv !== null) { const resolved = yield* resolveProjectSubtree( - loaded.config.edge_runtime, + loadedConfig.edge_runtime, projectEnv, "edge_runtime", ); diff --git a/apps/cli/src/legacy/commands/secrets/set/set.integration.test.ts b/apps/cli/src/legacy/commands/secrets/set/set.integration.test.ts index 524ee4f3f7..4e69972eb7 100644 --- a/apps/cli/src/legacy/commands/secrets/set/set.integration.test.ts +++ b/apps/cli/src/legacy/commands/secrets/set/set.integration.test.ts @@ -16,8 +16,23 @@ import { mockLegacyPlatformApi, useLegacyTempWorkdir, } from "../../../../../tests/helpers/legacy-mocks.ts"; +import { LegacyDebugLogger } from "../../../shared/legacy-debug-logger.service.ts"; import { legacySecretsSet } from "./set.handler.ts"; +function mockLegacyDebugLoggerTracked() { + const messages: Array = []; + return { + messages, + layer: Layer.succeed(LegacyDebugLogger, { + debug: (message) => + Effect.sync(() => { + messages.push(message); + }), + http: () => Effect.void, + }), + }; +} + // --------------------------------------------------------------------------- // Setup // --------------------------------------------------------------------------- @@ -40,6 +55,7 @@ function setup(opts: SetupOpts = {}) { network: opts.network, }); const cliConfig = mockLegacyCliConfig({ workdir: tempRoot.current }); + const debugLogger = mockLegacyDebugLoggerTracked(); const layer = Layer.mergeAll( buildLegacyTestRuntime({ out, @@ -49,8 +65,9 @@ function setup(opts: SetupOpts = {}) { }), mockRuntimeInfo({ cwd: tempRoot.current }), processEnvLayer(opts.env ?? {}), + debugLogger.layer, ); - return { layer, out, api }; + return { layer, out, api, debugLogger }; } function writeConfig(content: string) { @@ -58,6 +75,11 @@ function writeConfig(content: string) { writeFileSync(join(tempRoot.current, "supabase", "config.toml"), content); } +function writeSupabaseDotEnv(content: string) { + mkdirSync(join(tempRoot.current, "supabase"), { recursive: true }); + writeFileSync(join(tempRoot.current, "supabase", ".env"), content); +} + function parsePostBody(body: unknown): Array<{ name: string; value: string }> { // `mockLegacyPlatformApi` JSON-decodes the request body when it parses; this // helper just narrows the type for the test assertions. @@ -317,23 +339,316 @@ FOO = "literal-foo" }).pipe(Effect.provide(layer)); }); - it.live("fails with LegacySecretsConfigParseError when config.toml is malformed", () => { - writeConfig("this is not valid = = toml [[[\n"); - const { layer } = setup(); - return Effect.gen(function* () { - const exit = yield* Effect.exit( - legacySecretsSet({ + it.live( + "tolerates a malformed config.toml, logs it to the debug logger, and still sets CLI-arg secrets", + () => { + writeConfig("this is not valid = = toml [[[\n"); + const { layer, api, debugLogger } = setup(); + return Effect.gen(function* () { + yield* legacySecretsSet({ projectRef: Option.none(), envFile: Option.none(), secrets: ["FOO=bar"], - }), + }); + expect(api.requests).toHaveLength(1); + expect(parsePostBody(api.requests[0]?.body)).toEqual([{ name: "FOO", value: "bar" }]); + expect(debugLogger.messages).toHaveLength(1); + expect(debugLogger.messages[0]).toContain("failed to parse supabase/config.toml"); + }).pipe(Effect.provide(layer)); + }, + ); + + it.live( + "recovers [edge_runtime.secrets] when an unrelated field fails schema decode (CLI-1867 Go parity)", + () => { + // Valid TOML syntax throughout, but `analytics.port` has the wrong type + // for its schema field. Go's viper+mapstructure decode + // (`pkg/config/config.go:749`) mutates the target struct field-by-field, + // so an unrelated type error doesn't stop `EdgeRuntime.Secrets` from + // landing on `utils.Config` — `secrets set` still reads it. Effect + // Schema's `decodeUnknownSync` is atomic and would otherwise discard the + // whole document, silently dropping `FROM_CONFIG` too. + writeConfig( + `[edge_runtime.secrets] +FROM_CONFIG = "config-value" + +[analytics] +port = "not-a-number" +`, ); - expect(Exit.isFailure(exit)).toBe(true); - if (Exit.isFailure(exit)) { - expect(JSON.stringify(exit.cause)).toContain("LegacySecretsConfigParseError"); - } - }).pipe(Effect.provide(layer)); - }); + const { layer, api, debugLogger } = setup(); + return Effect.gen(function* () { + yield* legacySecretsSet({ + projectRef: Option.none(), + envFile: Option.none(), + secrets: [], + }); + expect(parsePostBody(api.requests[0]?.body)).toEqual([ + { name: "FROM_CONFIG", value: "config-value" }, + ]); + expect(debugLogger.messages).toHaveLength(1); + expect(debugLogger.messages[0]).toContain("failed to parse supabase/config.toml"); + }).pipe(Effect.provide(layer)); + }, + ); + + it.live( + "recovers [edge_runtime.secrets] when a sibling field in the same edge_runtime table fails schema decode (CLI-1867 Go parity)", + () => { + // Valid TOML syntax throughout, but `edge_runtime.inspector_port` has + // the wrong type for its schema field — a SIBLING of `secrets` inside + // the same `edge_runtime` table, not an unrelated top-level table. Go's + // viper+mapstructure decode (`pkg/config/config.go:749`) mutates the + // target struct field-by-field even within the same table, so + // `EdgeRuntime.Secrets` still lands on `utils.Config` while + // `InspectorPort` is left at its zero value — verified empirically + // against `pkg/config` directly. The recovery must therefore re-decode + // `secrets` on its own rather than the whole `edge_runtime` subtree. + writeConfig( + `[edge_runtime] +inspector_port = "not-a-number" + +[edge_runtime.secrets] +FROM_CONFIG = "config-value" +`, + ); + const { layer, api, debugLogger } = setup(); + return Effect.gen(function* () { + yield* legacySecretsSet({ + projectRef: Option.none(), + envFile: Option.none(), + secrets: [], + }); + expect(parsePostBody(api.requests[0]?.body)).toEqual([ + { name: "FROM_CONFIG", value: "config-value" }, + ]); + expect(debugLogger.messages).toHaveLength(1); + expect(debugLogger.messages[0]).toContain("failed to parse supabase/config.toml"); + }).pipe(Effect.provide(layer)); + }, + ); + + it.live( + "tolerates a malformed supabase/.env, logs it to the debug logger, and still sets CLI-arg secrets (CLI-1867 Go parity)", + () => { + // `loadProjectConfig` resolves `env(VAR)` references against + // `supabase/.env`/`.env.local` *before* schema decode, so a malformed + // dotenv line fails with `ProjectEnvParseError` rather than + // `ProjectConfigParseError`. Go's `Load()` (`pkg/config/config.go:788-791`) + // calls `loadNestedEnv` first too and swallows any error the same way + // `flags.LoadConfig` does in `internal/secrets/set/set.go:20-24` — so this + // must not abort the command either. `.env` is only read once a + // `supabase/config.toml`/`.json` is found (`findProjectPaths`), so a + // config.toml must exist here too. + writeConfig( + `[edge_runtime.secrets] +FROM_CONFIG = "config-value" +`, + ); + writeSupabaseDotEnv("THIS IS NOT A VALID DOTENV LINE\n"); + const { layer, api, debugLogger } = setup(); + return Effect.gen(function* () { + yield* legacySecretsSet({ + projectRef: Option.none(), + envFile: Option.none(), + secrets: ["FOO=bar"], + }); + expect(api.requests).toHaveLength(1); + expect(parsePostBody(api.requests[0]?.body)).toEqual([{ name: "FOO", value: "bar" }]); + expect(debugLogger.messages).toHaveLength(1); + expect(debugLogger.messages[0]).toContain("failed to parse"); + }).pipe(Effect.provide(layer)); + }, + ); + + it.live( + "recovers valid [edge_runtime.secrets] entries when a sibling entry in the same map fails schema decode (CLI-1867 Go parity)", + () => { + // `GOOD` is a valid secret value; `BAD` is not (a non-string TOML value + // for a field whose schema expects a string-like secret). Go's + // mapstructure decodes `map[string]Secret` entry-by-entry + // (`decodeMapFromMap`), appending a per-entry error and continuing + // rather than discarding the whole map, so `GOOD` still lands on + // `utils.Config.EdgeRuntime.Secrets` even with `BAD` present. Effect + // Schema's `decodeUnknownSync` is atomic per record and would otherwise + // discard `GOOD` too when re-decoding the whole `secrets` map at once. + writeConfig( + `[edge_runtime.secrets] +GOOD = "config-value" +BAD = 123 +`, + ); + const { layer, api, debugLogger } = setup(); + return Effect.gen(function* () { + yield* legacySecretsSet({ + projectRef: Option.none(), + envFile: Option.none(), + secrets: [], + }); + const body = parsePostBody(api.requests[0]?.body); + expect(body).toEqual([{ name: "GOOD", value: "config-value" }]); + expect(debugLogger.messages).toHaveLength(1); + expect(debugLogger.messages[0]).toContain("failed to parse supabase/config.toml"); + }).pipe(Effect.provide(layer)); + }, + ); + + it.live( + "does not fabricate a secret named 0 when [edge_runtime.secrets] is an array (CLI-1867 Go parity)", + () => { + // `edge_runtime.secrets` as an array (instead of a table) is not + // recoverable structure: Go's mapstructure decoder never sets + // `WeaklyTypedInput`, so a slice source for a map-typed field hits + // `UnconvertibleTypeError` in `decodeMap` rather than the index-as-key + // `decodeMapFromSlice` path, and the whole field is left empty. Before + // the `isRecord` fix, `Object.entries(["actual-secret"])` would turn + // this into a spurious `{ "0": "actual-secret" }` entry. + writeConfig( + `[analytics] +port = "not-a-number" + +[edge_runtime] +secrets = ["actual-secret"] +`, + ); + const { layer, api, debugLogger } = setup(); + return Effect.gen(function* () { + yield* legacySecretsSet({ + projectRef: Option.none(), + envFile: Option.none(), + secrets: ["FOO=bar"], + }); + const body = parsePostBody(api.requests[0]?.body); + expect(body).toEqual([{ name: "FOO", value: "bar" }]); + expect(body.find((entry) => entry.name === "0")).toBeUndefined(); + expect(debugLogger.messages).toHaveLength(1); + }).pipe(Effect.provide(layer)); + }, + ); + + it.live( + "recovers the selected remote's [edge_runtime.secrets] override, not the base, on schema-decode error (CLI-1867 Go parity)", + () => { + // `analytics.port` is an unrelated schema-decode error that triggers the + // recovery path. `remotes.staging.project_id` matches the ref the + // resolver defaults to (`mockLegacyCliConfig`'s `LEGACY_VALID_REF`), so + // Go seeds `Config.ProjectId` before `Load()` + // (`internal/utils/flags/config_path.go:11-12`) and merges the remote + // override in `loadFromFile` (`pkg/config/config.go:604-609`) before the + // tolerant decode this PR models — the recovered secret must reflect the + // remote's override value, not the base document's. + writeConfig( + `[edge_runtime.secrets] +FROM_CONFIG = "base-value" + +[analytics] +port = "not-a-number" + +[remotes.staging] +project_id = "${LEGACY_VALID_REF}" + +[remotes.staging.edge_runtime.secrets] +FROM_CONFIG = "remote-value" +`, + ); + const { layer, api, debugLogger } = setup(); + return Effect.gen(function* () { + yield* legacySecretsSet({ + projectRef: Option.none(), + envFile: Option.none(), + secrets: [], + }); + expect(parsePostBody(api.requests[0]?.body)).toEqual([ + { name: "FROM_CONFIG", value: "remote-value" }, + ]); + expect(debugLogger.messages).toHaveLength(1); + expect(debugLogger.messages[0]).toContain("failed to parse supabase/config.toml"); + }).pipe(Effect.provide(layer)); + }, + ); + + it.live( + "tolerates two [remotes.*] blocks sharing the target project_id, logs it, and still sets CLI-arg secrets (CLI-1867 Go parity)", + () => { + // Go's `flags.LoadConfig` swallows *any* `Load()` error non-fatally + // (`internal/secrets/set/set.go:22-24`), including the duplicate- + // `project_id` error `loadFromFile` raises before `mapstructure` ever + // runs (`pkg/config/config.go:601`). There is no parsed document to + // recover a subtree from, so config-sourced secrets are dropped + // entirely — only CLI-arg secrets survive. + writeConfig( + `[edge_runtime.secrets] +FROM_CONFIG = "config-value" + +[remotes.a] +project_id = "dupe-project-id" + +[remotes.b] +project_id = "dupe-project-id" +`, + ); + const { layer, api, debugLogger } = setup(); + return Effect.gen(function* () { + yield* legacySecretsSet({ + projectRef: Option.none(), + envFile: Option.none(), + secrets: ["FOO=bar"], + }); + expect(parsePostBody(api.requests[0]?.body)).toEqual([{ name: "FOO", value: "bar" }]); + expect(debugLogger.messages).toHaveLength(1); + expect(debugLogger.messages[0]).toContain("duplicate project_id for [remotes."); + }).pipe(Effect.provide(layer)); + }, + ); + + it.live( + "does not echo a literal secret value from config.toml into the debug log on a syntax error", + () => { + // `smol-toml`'s `TomlError` embeds a source codeblock (the offending line ±1) + // in its message; the planted secret sits directly above the syntax error so + // it would land inside that codeblock if the handler logged the raw message. + writeConfig( + [ + "[edge_runtime.secrets]", + 'PLANTED_SECRET = "sk_live_TOTALLY_REAL_SECRET_VALUE"', + "BROKEN = = invalid[[[", + ].join("\n"), + ); + const { layer, debugLogger } = setup(); + return Effect.gen(function* () { + yield* legacySecretsSet({ + projectRef: Option.none(), + envFile: Option.none(), + secrets: ["FOO=bar"], + }); + expect(debugLogger.messages).toHaveLength(1); + expect(debugLogger.messages[0]).not.toContain("PLANTED_SECRET"); + expect(debugLogger.messages[0]).not.toContain("sk_live_TOTALLY_REAL_SECRET_VALUE"); + }).pipe(Effect.provide(layer)); + }, + ); + + it.live( + "still fails with LegacySecretsNoArgumentsError when a malformed config leaves zero secret sources", + () => { + writeConfig("this is not valid = = toml [[[\n"); + const { layer, api } = setup(); + return Effect.gen(function* () { + const exit = yield* Effect.exit( + legacySecretsSet({ + projectRef: Option.none(), + envFile: Option.none(), + secrets: [], + }), + ); + expect(Exit.isFailure(exit)).toBe(true); + if (Exit.isFailure(exit)) { + expect(JSON.stringify(exit.cause)).toContain("LegacySecretsNoArgumentsError"); + } + expect(api.requests).toHaveLength(0); + }).pipe(Effect.provide(layer)); + }, + ); it.live("fails with LegacySecretsSetNetworkError on transport failure", () => { const { layer } = setup({ network: "fail" }); diff --git a/apps/cli/src/legacy/shared/legacy-management-api-runtime.layer.ts b/apps/cli/src/legacy/shared/legacy-management-api-runtime.layer.ts index 9b0ab6b18d..2b75bcd08a 100644 --- a/apps/cli/src/legacy/shared/legacy-management-api-runtime.layer.ts +++ b/apps/cli/src/legacy/shared/legacy-management-api-runtime.layer.ts @@ -15,6 +15,7 @@ import { LegacyCliConfig } from "../config/legacy-cli-config.service.ts"; import { legacyCliConfigLayer } from "../config/legacy-cli-config.layer.ts"; import { LegacyProjectRefResolver } from "../config/legacy-project-ref.service.ts"; import { legacyProjectRefLayer } from "../config/legacy-project-ref.layer.ts"; +import { LegacyDebugLogger } from "./legacy-debug-logger.service.ts"; import { legacyDebugLoggerLayer } from "./legacy-debug-logger.layer.ts"; import { legacyDohFetchLayer } from "./legacy-http-dns.ts"; import { LegacyIdentityStitch, legacyIdentityStitchLayer } from "./legacy-identity-stitch.ts"; @@ -104,6 +105,10 @@ export function legacyManagementApiRuntimeLayer(subcommand: ReadonlyArray {} export class ProjectEnvParseError extends Data.TaggedError("ProjectEnvParseError")<{ diff --git a/packages/config/src/io.ts b/packages/config/src/io.ts index 85f9ca6201..5b5d79efc6 100644 --- a/packages/config/src/io.ts +++ b/packages/config/src/io.ts @@ -1,4 +1,4 @@ -import { Console, Effect, FileSystem, Path, Schema } from "effect"; +import { Console, Effect, FileSystem, Path, Redacted, Schema } from "effect"; import * as SmolToml from "smol-toml"; import { ProjectConfigSchema, type ProjectConfig } from "./base.ts"; import { DuplicateRemoteProjectIdError, ProjectConfigParseError } from "./errors.ts"; @@ -317,6 +317,37 @@ function normalizeDeprecatedSMTPSections(document: unknown): NormalizedSMTPDocum return { document: normalized, deprecatedSections }; } +/** + * Wraps every `edge_runtime.secrets` value in `Redacted` before it's attached + * to `ProjectConfigParseError.document`. By this point `secrets` values are + * real, resolved secrets (post `env()` interpolation, see + * `interpolateEnvReferencesAgainstSchema` in `loadProjectConfigFile`) — the + * same values `secret()` (`lib/env.ts`) annotates `x-secret` for elsewhere in + * this package (`resolveProjectValue`'s `redactValue`). Several callers of + * `loadProjectConfig` (`gen types`, `next start`, `functions dev/serve/deploy`) + * don't catch `ProjectConfigParseError` at all, so this keeps the same + * accidental-leak protection `Redacted` already gives every other secret path + * in this package, in case an uncaught error's `document` ever reaches a log + * or trace. `secrets set`'s `recoverEdgeRuntimeConfig`/`filterDecodableSecrets` + * unwrap via `Redacted.isRedacted`/`Redacted.value` before re-decoding. + */ +function redactEdgeRuntimeSecrets(edgeRuntime: unknown): unknown { + if (!isObject(edgeRuntime) || !isObject(edgeRuntime.secrets)) { + return edgeRuntime; + } + return { + ...edgeRuntime, + secrets: Object.fromEntries( + Object.entries(edgeRuntime.secrets).map(([name, value]) => [ + name, + typeof value === "string" + ? Redacted.make(value, { label: `edge_runtime.secrets.${name}` }) + : value, + ]), + ), + }; +} + function getSchemaRef(document: unknown): string | undefined { if (!isObject(document)) { return undefined; @@ -333,7 +364,27 @@ function parseProjectConfig( ): Effect.Effect { return Effect.try({ try: () => decodeProjectConfig(document), - catch: (cause) => new ProjectConfigParseError({ path, format, cause }), + // `document` always parsed successfully by this point (raw parse failures + // are caught earlier, in `loadProjectConfigFile`), so any error here is a + // schema-decode failure — attach it so callers can attempt a narrower, + // Go-tolerant re-decode of an unaffected subtree. See the field doc on + // `ProjectConfigParseError.document`. Only the `edge_runtime` subtree is + // retained (not the whole document): it's the only slice any caller + // re-decodes today (`secrets set`'s `recoverEdgeRuntimeConfig`), and several + // callers of `loadProjectConfig` (e.g. `gen types`, `next start`, + // `functions dev/serve/deploy`) don't catch `ProjectConfigParseError` at + // all, so this error can propagate with whatever we attach here — no + // reason to carry unrelated sections (db credentials, other + // `[remotes.*]` blocks, etc.) along for the ride. + catch: (cause) => + new ProjectConfigParseError({ + path, + format, + cause, + document: isObject(document) + ? { edge_runtime: redactEdgeRuntimeSecrets(document.edge_runtime) } + : undefined, + }), }); } diff --git a/packages/config/src/io.unit.test.ts b/packages/config/src/io.unit.test.ts index d22d4eaf85..ffe8b7db20 100644 --- a/packages/config/src/io.unit.test.ts +++ b/packages/config/src/io.unit.test.ts @@ -5,7 +5,7 @@ import { mkdir, readFile, rm, writeFile } from "node:fs/promises"; import { tmpdir } from "node:os"; import { dirname, join } from "node:path"; import { fileURLToPath } from "node:url"; -import { Cause, Effect, Exit, FileSystem, Layer, Option, Path, Schema } from "effect"; +import { Cause, Effect, Exit, FileSystem, Layer, Option, Path, Redacted, Schema } from "effect"; import { ProjectConfigSchema } from "./base.ts"; import { loadProjectConfig as loadProjectConfigFromBun } from "./bun.ts"; import { @@ -430,6 +430,60 @@ major_version = 16 } }); + test("redacts edge_runtime.secrets on the ProjectConfigParseError document", async () => { + const cwd = makeTempProject(); + const tomlPath = await runConfigEffect(configTomlPath(cwd)); + + try { + await mkdir(join(cwd, "supabase"), { recursive: true }); + // `analytics.port` fails schema decode (expects a number), which is + // enough to fail the whole `Schema.decodeUnknownSync` call while + // `edge_runtime.secrets` parses fine on its own — the scenario + // `recoverEdgeRuntimeConfig` (apps/cli's `secrets set`) exists to + // recover from. `MY_SUPER_SECRET_VALUE` stands in for a real secret so + // the assertion below can confirm it never appears in plaintext. + await writeFile( + tomlPath, + `[analytics] +port = "not-a-number" + +[edge_runtime.secrets] +FOO = "MY_SUPER_SECRET_VALUE" +`, + ); + + const exit = await Effect.runPromiseExit( + loadProjectConfigFile(tomlPath).pipe(Effect.provide(BunServices.layer)), + ); + + expect(Exit.isFailure(exit)).toBe(true); + if (!Exit.isFailure(exit)) { + return; + } + const error = Cause.findErrorOption(exit.cause); + expect(Option.isSome(error)).toBe(true); + if (!Option.isSome(error) || error.value._tag !== "ProjectConfigParseError") { + return; + } + + const edgeRuntime = error.value.document?.edge_runtime; + const secrets = + edgeRuntime !== null && typeof edgeRuntime === "object" && edgeRuntime !== undefined + ? (edgeRuntime as Record).secrets + : undefined; + expect(secrets).toBeDefined(); + const foo = (secrets as Record).FOO; + expect(Redacted.isRedacted(foo)).toBe(true); + expect(Redacted.value(foo as Redacted.Redacted)).toBe("MY_SUPER_SECRET_VALUE"); + // The whole point: a caller that doesn't know to unwrap `Redacted` + // (e.g. an uncaught error serialized into a log) never sees the raw + // secret, even via JSON.stringify. + expect(JSON.stringify(error.value.document)).not.toContain("MY_SUPER_SECRET_VALUE"); + } finally { + await rm(cwd, { recursive: true, force: true }); + } + }); + test("preserves TOML as the active format on save", async () => { const cwd = makeTempProject(); const tomlPath = await runConfigEffect(configTomlPath(cwd));