From 19535b918c6c2439d2756b0506cf4a16b0a38c46 Mon Sep 17 00:00:00 2001 From: Colum Ferry Date: Fri, 22 May 2026 16:44:54 +0100 Subject: [PATCH] fix(config): interpolate env() refs before schema decode MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `env(VAR)` in `supabase/config.toml` numeric/boolean fields crashed `loadProjectConfig` with `ProjectConfigParseError: Expected number` because the strict Effect Schema decode ran immediately after raw TOML parse — `interpolateValue` in `project.ts` exists but only fires post-decode via `resolveProjectValue`, so it never got the chance to substitute `"env(SUPABASE_ANALYTICS_PORT)"` on `analytics.port`. `loadProjectConfigFile` now loads the project environment and runs a schema-aware pre-decode walker that traverses the parsed document and `ProjectConfigSchema.ast` in parallel: - Substitutes `env(VAR)` string leaves against `.env` / `.env.local` / ambient env. - For unset vars, preserves the literal verbatim — matches Go's `apps/cli-go/pkg/config/decode_hooks.go:14-21` (LoadEnvHook). - After substitution, if the schema at that path expects Number or Boolean, coerces the substituted string to the primitive — mirroring Go's mapstructure chain where LoadEnvHook returns a string and subsequent hooks convert it to the target type. - Fields declared with the `env()` helper opt out via a marker annotation; they continue to flow through post-decode resolution. `interpolateLeafValue` in `project.ts` now returns the literal verbatim when the referenced env var is missing instead of throwing `MissingProjectEnvVarError` (also Go parity). The error class is removed; `resolveProjectValue` / `resolveProjectSubtree` no longer have a failure channel. No schema-file edits — coercion lives in the walker, so future schemas that add numeric/boolean fields automatically work with env() refs. Fixes CLI-1489 --- packages/config/src/errors.ts | 5 - packages/config/src/index.ts | 1 - packages/config/src/io.ts | 33 +++- packages/config/src/io.unit.test.ts | 137 +++++++++++++ packages/config/src/lib/env.ts | 233 ++++++++++++++++++++++- packages/config/src/project.ts | 66 +++---- packages/config/src/project.unit.test.ts | 35 ++-- 7 files changed, 441 insertions(+), 69 deletions(-) diff --git a/packages/config/src/errors.ts b/packages/config/src/errors.ts index f1b6b5ea5..1cc1c4da1 100644 --- a/packages/config/src/errors.ts +++ b/packages/config/src/errors.ts @@ -12,11 +12,6 @@ export class ProjectEnvParseError extends Data.TaggedError("ProjectEnvParseError readonly line: number; }> {} -export class MissingProjectEnvVarError extends Data.TaggedError("MissingProjectEnvVarError")<{ - readonly configPath: string; - readonly envName: string; -}> {} - export class MissingProjectConfigValueError extends Data.TaggedError( "MissingProjectConfigValueError", )<{ diff --git a/packages/config/src/index.ts b/packages/config/src/index.ts index aca28d8be..788b00813 100644 --- a/packages/config/src/index.ts +++ b/packages/config/src/index.ts @@ -1,7 +1,6 @@ export { ProjectConfigSchema, type ProjectConfig, type ProjectConfigJson } from "./base.ts"; export { MissingProjectConfigValueError, - MissingProjectEnvVarError, ProjectConfigParseError, ProjectEnvParseError, } from "./errors.ts"; diff --git a/packages/config/src/io.ts b/packages/config/src/io.ts index 07fbfb883..b981a4bd8 100644 --- a/packages/config/src/io.ts +++ b/packages/config/src/io.ts @@ -2,7 +2,9 @@ import { Effect, FileSystem, Path, Schema } from "effect"; import * as SmolToml from "smol-toml"; import { ProjectConfigSchema, type ProjectConfig } from "./base.ts"; import { ProjectConfigParseError } from "./errors.ts"; +import { interpolateEnvReferencesAgainstSchema } from "./lib/env.ts"; import { findProjectPaths } from "./paths.ts"; +import { loadProjectEnvironment } from "./project.ts"; const projectConfigSchemaKey = "$schema"; @@ -203,18 +205,37 @@ function encodeProjectConfigToTomlDocument( return `${SmolToml.stringify(toConfigDocument(config, schemaRef))}\n`; } -export const loadProjectConfigFile = Effect.fnUntraced(function* (path: string) { +export const loadProjectConfigFile = Effect.fnUntraced(function* (filePath: string) { const fs = yield* FileSystem.FileSystem; - const format = path.endsWith(".json") ? "json" : "toml"; - const content = yield* fs.readFileString(path); + const path = yield* Path.Path; + const format = filePath.endsWith(".json") ? "json" : "toml"; + const content = yield* fs.readFileString(filePath); const document = yield* Effect.try({ try: () => parseProjectConfigDocument(content, format), - catch: (cause) => new ProjectConfigParseError({ path, format, cause }), + catch: (cause) => new ProjectConfigParseError({ path: filePath, format, cause }), + }); + + // Substitute `env(VAR)` references against `.env`/`.env.local`/ambient env + // before schema decode. Required for numeric/boolean fields, which would + // otherwise crash the strict decoder with `Expected number` (CLI-1489). + // The config file lives at `/supabase/config.{toml,json}`, so + // walking two directories up gives us the project root that + // `loadProjectEnvironment` expects. + const projectRoot = path.dirname(path.dirname(filePath)); + const projectEnv = yield* loadProjectEnvironment({ + cwd: projectRoot, + baseEnv: process.env, }); - const config = yield* parseProjectConfig(document, format, path); + const interpolated = interpolateEnvReferencesAgainstSchema( + document, + projectEnv?.values ?? {}, + ProjectConfigSchema, + ); + + const config = yield* parseProjectConfig(interpolated, format, filePath); return { - path, + path: filePath, format, config, schemaRef: getSchemaRef(document), diff --git a/packages/config/src/io.unit.test.ts b/packages/config/src/io.unit.test.ts index 4b934885c..e3154e9e0 100644 --- a/packages/config/src/io.unit.test.ts +++ b/packages/config/src/io.unit.test.ts @@ -675,4 +675,141 @@ major_version = 16 expect(schemaString).toContain("env"); expect(schemaString).not.toContain("versions"); }); + + test("resolves env() on numeric port fields (CLI-1489)", async () => { + const cwd = makeTempProject(); + + try { + await mkdir(join(cwd, "supabase"), { recursive: true }); + await writeFile( + join(cwd, "supabase", "config.toml"), + `project_id = "ref_123" + +[api] +port = "env(SUPABASE_API_PORT)" + +[db] +port = "env(SUPABASE_DB_PORT)" + +[analytics] +port = "env(SUPABASE_ANALYTICS_PORT)" +`, + ); + await writeFile( + join(cwd, "supabase", ".env"), + "SUPABASE_API_PORT=54321\nSUPABASE_DB_PORT=54322\nSUPABASE_ANALYTICS_PORT=54327\n", + ); + + const loaded = await runConfigEffect(loadProjectConfig(cwd)); + + expect(loaded).not.toBeNull(); + expect(loaded!.config.api.port).toBe(54321); + expect(loaded!.config.db.port).toBe(54322); + expect(loaded!.config.analytics.port).toBe(54327); + } finally { + await rm(cwd, { recursive: true, force: true }); + } + }); + + test("resolves env() on boolean fields", async () => { + const cwd = makeTempProject(); + + try { + await mkdir(join(cwd, "supabase"), { recursive: true }); + await writeFile( + join(cwd, "supabase", "config.toml"), + `project_id = "ref_123" + +[analytics] +enabled = "env(SUPABASE_ANALYTICS_ENABLED)" +`, + ); + await writeFile(join(cwd, "supabase", ".env"), "SUPABASE_ANALYTICS_ENABLED=false\n"); + + const loaded = await runConfigEffect(loadProjectConfig(cwd)); + expect(loaded!.config.analytics.enabled).toBe(false); + } finally { + await rm(cwd, { recursive: true, force: true }); + } + }); + + test("preserves env() literals on string fields when the var is unset (Go parity)", async () => { + const cwd = makeTempProject(); + + try { + await mkdir(join(cwd, "supabase"), { recursive: true }); + await writeFile( + join(cwd, "supabase", "config.toml"), + `project_id = "ref_123" + +[auth] +jwt_secret = "env(MISSING_SECRET)" +`, + ); + + const loaded = await runConfigEffect(loadProjectConfig(cwd)); + expect(loaded!.config.auth.jwt_secret).toBe("env(MISSING_SECRET)"); + } finally { + await rm(cwd, { recursive: true, force: true }); + } + }); + + test("fails to decode a numeric field when env var is unset", async () => { + const cwd = makeTempProject(); + + try { + await mkdir(join(cwd, "supabase"), { recursive: true }); + await writeFile( + join(cwd, "supabase", "config.toml"), + `project_id = "ref_123" + +[analytics] +port = "env(MISSING_PORT)" +`, + ); + + const exit = await Effect.runPromiseExit( + loadProjectConfig(cwd).pipe(Effect.provide(BunServices.layer)), + ); + + expect(Exit.isFailure(exit)).toBe(true); + if (Exit.isFailure(exit)) { + const failure = Cause.findErrorOption(exit.cause); + expect(Option.isSome(failure)).toBe(true); + if (Option.isSome(failure)) { + expect((failure.value as { _tag: string })._tag).toBe("ProjectConfigParseError"); + } + } + } finally { + await rm(cwd, { recursive: true, force: true }); + } + }); + + test("falls back to ambient process.env when .env is missing", async () => { + const cwd = makeTempProject(); + const previous = process.env.SUPABASE_DB_PORT_TEST; + process.env.SUPABASE_DB_PORT_TEST = "55555"; + + try { + await mkdir(join(cwd, "supabase"), { recursive: true }); + await writeFile( + join(cwd, "supabase", "config.toml"), + `project_id = "ref_123" + +[db] +port = "env(SUPABASE_DB_PORT_TEST)" +`, + ); + + const loaded = await runConfigEffect(loadProjectConfig(cwd)); + expect(loaded!.config.db.port).toBe(55555); + } finally { + if (previous === undefined) { + delete process.env.SUPABASE_DB_PORT_TEST; + } else { + process.env.SUPABASE_DB_PORT_TEST = previous; + } + await rm(cwd, { recursive: true, force: true }); + } + }); }); diff --git a/packages/config/src/lib/env.ts b/packages/config/src/lib/env.ts index 0339142f3..bd1681929 100644 --- a/packages/config/src/lib/env.ts +++ b/packages/config/src/lib/env.ts @@ -1,4 +1,4 @@ -import { Schema } from "effect"; +import { Schema, SchemaAST } from "effect"; export const ENV_PATTERN = "^env\\([A-Z_][A-Z0-9_]*\\)$"; export const ENV_CAPTURE_REGEX = /^env\(([A-Z_][A-Z0-9_]*)\)$/; @@ -12,10 +12,16 @@ interface EnvAnnotations extends Schema.Annotations.Documentation { readonly secret?: true; } +// Marker annotation: this field requires the `env(VAR)` literal form and is +// resolved post-decode via `resolveProjectValue` / `resolveProjectSubtree`. +// The pre-decode walker honors this and leaves the literal untouched. +const X_ENV_DEFERRED = "x-env-deferred" as const; + export const env = (annotations?: EnvAnnotations) => { const { secret, ...rest } = annotations ?? {}; return Schema.String.check(Schema.isPattern(envRegex)).annotate({ ...rest, + [X_ENV_DEFERRED]: true, ...(secret ? { "x-secret": true } : {}), }); }; @@ -27,3 +33,228 @@ export const secret = (annotations?: SecretAnnotations) => ...annotations, "x-secret": true, }); + +// --------------------------------------------------------------------------- +// Pre-decode env() interpolation with schema-aware type coercion +// --------------------------------------------------------------------------- +// +// TOML/JSON parsers turn `port = "env(SUPABASE_ANALYTICS_PORT)"` into a string +// at `analytics.port`, but the schema declares `port: Schema.Number`. Without +// pre-decode handling the strict decoder rejects the string and crashes +// `supabase db start` (CLI-1489). +// +// `interpolateEnvReferencesAgainstSchema` walks the parsed document and the +// schema AST in parallel: +// - For string leaves matching `env(VAR)`: substitute `env[VAR]` if set, or +// preserve the literal verbatim if unset (matches Go's +// `apps/cli-go/pkg/config/decode_hooks.go:14-21`). +// - After substitution, if the schema at that path expects Number or Boolean +// and the value is still a string, coerce it. This mirrors Go's +// mapstructure chain where `LoadEnvHook` returns a string and subsequent +// hooks convert it to the target type. +// - Coercion is only attempted on strings produced by env() substitution. +// Pre-existing string literals at non-string paths are left untouched — +// they'll surface as schema errors at decode time with their original +// value, preserving error clarity. + +type ExpectedType = "number" | "boolean" | "string" | "unknown"; + +// Unwrap Suspend (lazy AST refs from recursive schemas). Other transformation +// wrappers expose the target type via `.ast` directly, so no additional +// unwrapping is needed at this layer. +function unwrapAst(ast: SchemaAST.AST): SchemaAST.AST { + if (ast._tag === "Suspend") { + return unwrapAst(ast.thunk()); + } + return ast; +} + +function leafExpectedType(ast: SchemaAST.AST): ExpectedType { + const node = unwrapAst(ast); + switch (node._tag) { + case "Number": + return "number"; + case "Boolean": + return "boolean"; + case "String": + return "string"; + case "Union": { + // Walk Union branches in declared order; first concrete primitive wins. + // For unions like `Schema.Union(Schema.Number, Schema.Null)` this picks + // the meaningful side. If the union mixes Number and String we err on + // the side of the first match — the schema decode will still validate + // membership after coercion. + for (const variant of node.types) { + const t = leafExpectedType(variant); + if (t !== "unknown") { + return t; + } + } + return "unknown"; + } + default: + return "unknown"; + } +} + +function descendAst(ast: SchemaAST.AST, segment: string): SchemaAST.AST | null { + const node = unwrapAst(ast); + + if (node._tag === "Objects") { + const ps = node.propertySignatures.find((p) => p.name === segment); + if (ps !== undefined) { + return ps.type; + } + // Record-like sections (e.g. `[edge_runtime.secrets]`, `[remotes.]`) + // express their value shape via index signatures. + if (node.indexSignatures.length > 0) { + return node.indexSignatures[0]!.type; + } + return null; + } + + if (node._tag === "Arrays") { + const index = Number.parseInt(segment, 10); + if (Number.isInteger(index)) { + if (index >= 0 && index < node.elements.length) { + return node.elements[index]!; + } + if (node.rest.length > 0) { + return node.rest[0]!; + } + } + return null; + } + + if (node._tag === "Union") { + // Pick the first branch whose descent succeeds. + for (const variant of node.types) { + const next = descendAst(variant, segment); + if (next !== null) { + return next; + } + } + return null; + } + + return null; +} + +function coerceLeaf(value: unknown, expected: ExpectedType): unknown { + if (typeof value !== "string") { + return value; + } + if (expected === "number") { + const trimmed = value.trim(); + if (trimmed === "") { + return value; + } + const n = Number(trimmed); + if (Number.isFinite(n)) { + return n; + } + return value; + } + if (expected === "boolean") { + if (value === "true") return true; + if (value === "false") return false; + return value; + } + return value; +} + +function substituteEnvLeaf(value: string, env: Readonly>): string { + const match = ENV_CAPTURE_REGEX.exec(value); + if (match === null) { + return value; + } + const envName = match[1]; + if (envName === undefined || !Object.prototype.hasOwnProperty.call(env, envName)) { + return value; + } + return env[envName] ?? value; +} + +function isDeferredEnvField(ast: SchemaAST.AST): boolean { + const node = unwrapAst(ast); + if (node.annotations?.[X_ENV_DEFERRED] === true) { + return true; + } + // The env() helper threads its annotation through `.check(isPattern(...))`, + // which attaches the metadata to the Filter rather than the base AST. + for (const check of node.checks ?? []) { + if ( + (check as { annotations?: Record }).annotations?.[X_ENV_DEFERRED] === true + ) { + return true; + } + } + return false; +} + +function walk( + document: unknown, + env: Readonly>, + ast: SchemaAST.AST | null, +): unknown { + if (Array.isArray(document)) { + return document.map((item, index) => { + const child = ast === null ? null : descendAst(ast, String(index)); + return walk(item, env, child); + }); + } + + if (typeof document === "object" && document !== null) { + const result: Record = {}; + for (const [key, value] of Object.entries(document)) { + const child = ast === null ? null : descendAst(ast, key); + result[key] = walk(value, env, child); + } + return result; + } + + if (typeof document === "string") { + // Fields declared with the `env()` helper require the literal `env(VAR)` + // form for post-decode resolution. Skip substitution there so the schema + // pattern check still matches. + if (ast !== null && isDeferredEnvField(ast)) { + return document; + } + // Substitute env() then coerce based on the schema's expected type at this + // path. Only the substituted form is fed to coercion — literal strings at + // non-string paths are left untouched so the decoder can report them with + // their original value. + const substituted = substituteEnvLeaf(document, env); + if (substituted === document) { + return document; + } + if (ast === null) { + return substituted; + } + return coerceLeaf(substituted, leafExpectedType(ast)); + } + + return document; +} + +/** + * Pre-decode env() substitution + schema-aware coercion. + * + * Walks the raw parsed document and the schema AST in parallel. For every + * string leaf matching `env(VAR)`: + * 1. Substitutes `env[VAR]` if set, else preserves the literal verbatim + * (Go-parity with `apps/cli-go/pkg/config/decode_hooks.go:14-21`). + * 2. If the schema at that path expects Number or Boolean, coerces the + * substituted string to the expected primitive — mirroring Go's + * mapstructure chain where `LoadEnvHook` returns a string that the next + * hook converts to the target type. + * + * Returns a new structure; does not mutate the input. + */ +export function interpolateEnvReferencesAgainstSchema( + document: unknown, + env: Readonly>, + schema: { readonly ast: SchemaAST.AST }, +): unknown { + return walk(document, env, schema.ast); +} diff --git a/packages/config/src/project.ts b/packages/config/src/project.ts index 7db85b782..68953f2b1 100644 --- a/packages/config/src/project.ts +++ b/packages/config/src/project.ts @@ -1,6 +1,6 @@ import { Effect, FileSystem, Redacted } from "effect"; import { ProjectConfigSchema } from "./base.ts"; -import { MissingProjectEnvVarError, ProjectEnvParseError } from "./errors.ts"; +import { ProjectEnvParseError } from "./errors.ts"; import { ENV_CAPTURE_REGEX, isEnvReference } from "./lib/env.ts"; import { findProjectPaths, type ProjectPaths } from "./paths.ts"; @@ -216,11 +216,7 @@ function isSecretPath(path: ReadonlyArray): boolean { return secretPathPatterns.some((pattern) => matchesPathPattern(pattern, path)); } -function interpolateLeafValue( - value: string, - env: Readonly>, - configPath: ReadonlyArray, -): string { +function interpolateLeafValue(value: string, env: Readonly>): string { const match = envReferencePattern.exec(value); const envName = match?.[1]; @@ -228,11 +224,10 @@ function interpolateLeafValue( return value; } + // Preserve the literal `env(VAR)` verbatim when VAR is unset. Matches Go's + // `apps/cli-go/pkg/config/decode_hooks.go:14-21` (LoadEnvHook). if (!Object.prototype.hasOwnProperty.call(env, envName)) { - throw new MissingProjectEnvVarError({ - configPath: configPath.join("."), - envName, - }); + return value; } return env[envName] ?? value; @@ -246,27 +241,23 @@ function toPathSegments(path: string): ReadonlyArray { return path.split(".").filter((segment) => segment.length > 0); } -function interpolateValue( - value: unknown, - env: Readonly>, - path: ReadonlyArray = [], -): unknown { +function interpolateValue(value: unknown, env: Readonly>): unknown { if (Array.isArray(value)) { - return value.map((item, index) => interpolateValue(item, env, [...path, String(index)])); + return value.map((item) => interpolateValue(item, env)); } if (typeof value === "object" && value !== null) { const result: Record = {}; for (const [key, child] of Object.entries(value)) { - result[key] = interpolateValue(child, env, [...path, key]); + result[key] = interpolateValue(child, env); } return result; } if (typeof value === "string") { - return interpolateLeafValue(value, env, path); + return interpolateLeafValue(value, env); } return value; @@ -298,34 +289,37 @@ function resolveProjectValueAtPath( value: unknown, projectEnv: ProjectEnvironment, path: ReadonlyArray, -): Effect.Effect { - try { - const interpolated = interpolateValue(value, projectEnv.values, path); - return Effect.succeed(redactValue(interpolated, path)); - } catch (error) { - if (error instanceof MissingProjectEnvVarError) { - return Effect.fail(error); - } - throw error; - } +): unknown { + const interpolated = interpolateValue(value, projectEnv.values); + return redactValue(interpolated, path); } export function resolveProjectValue( value: T, projectEnv: ProjectEnvironment, configPath: string, -): Effect.Effect, MissingProjectEnvVarError> { - return Effect.suspend(() => - resolveProjectValueAtPath(value, projectEnv, toPathSegments(configPath)), - ).pipe(Effect.map((resolved) => resolved as ResolvedProjectValue)); +): Effect.Effect> { + return Effect.sync( + () => + resolveProjectValueAtPath( + value, + projectEnv, + toPathSegments(configPath), + ) as ResolvedProjectValue, + ); } export function resolveProjectSubtree( value: T, projectEnv: ProjectEnvironment, pathPrefix: string, -): Effect.Effect, MissingProjectEnvVarError> { - return Effect.suspend(() => - resolveProjectValueAtPath(value, projectEnv, toPathSegments(pathPrefix)), - ).pipe(Effect.map((resolved) => resolved as ResolvedProjectValue)); +): Effect.Effect> { + return Effect.sync( + () => + resolveProjectValueAtPath( + value, + projectEnv, + toPathSegments(pathPrefix), + ) as ResolvedProjectValue, + ); } diff --git a/packages/config/src/project.unit.test.ts b/packages/config/src/project.unit.test.ts index a0b4ffbdd..aae33d171 100644 --- a/packages/config/src/project.unit.test.ts +++ b/packages/config/src/project.unit.test.ts @@ -220,7 +220,7 @@ jwt_secret = "env(PREVIEW_JWT_SECRET)" } }); - test("resolveProjectValue fails when an explicit env() reference is missing", async () => { + test("resolveProjectValue preserves env() literal when the env var is missing (Go parity)", async () => { const cwd = makeTempProject(); const projectRoot = join(cwd, "repo"); @@ -238,21 +238,20 @@ jwt_secret = "env(MISSING_SECRET)" const loaded = await runConfigEffect(loadProjectConfig(projectRoot)); const projectEnv = await runConfigEffect(loadProjectEnvironment({ cwd: projectRoot })); - await expect( - runConfigEffect( - resolveProjectValue(loaded!.config.auth.jwt_secret, projectEnv!, "auth.jwt_secret"), - ), - ).rejects.toMatchObject({ - _tag: "MissingProjectEnvVarError", - configPath: "auth.jwt_secret", - envName: "MISSING_SECRET", - }); + const resolved = await runConfigEffect( + resolveProjectValue(loaded!.config.auth.jwt_secret, projectEnv!, "auth.jwt_secret"), + ); + + // Secret paths are normally redacted, but unresolved env() literals pass + // through as plain strings so callers can see the missing reference. + expect(Redacted.isRedacted(resolved)).toBe(false); + expect(resolved).toBe("env(MISSING_SECRET)"); } finally { await rm(cwd, { recursive: true, force: true }); } }); - test("resolveProjectSubtree fails when the selected subtree contains a missing env()", async () => { + test("resolveProjectSubtree preserves env() literals nested inside the selected subtree", async () => { const cwd = makeTempProject(); const projectRoot = join(cwd, "repo"); @@ -271,15 +270,11 @@ auth_token = "env(MISSING_SECRET)" const loaded = await runConfigEffect(loadProjectConfig(projectRoot)); const projectEnv = await runConfigEffect(loadProjectEnvironment({ cwd: projectRoot })); - await expect( - runConfigEffect( - resolveProjectSubtree(loaded!.config.auth.sms.twilio, projectEnv!, "auth.sms.twilio"), - ), - ).rejects.toMatchObject({ - _tag: "MissingProjectEnvVarError", - configPath: "auth.sms.twilio.auth_token", - envName: "MISSING_SECRET", - }); + const resolved = await runConfigEffect( + resolveProjectSubtree(loaded!.config.auth.sms.twilio, projectEnv!, "auth.sms.twilio"), + ); + + expect(resolved.auth_token).toBe("env(MISSING_SECRET)"); } finally { await rm(cwd, { recursive: true, force: true }); }