fix(cli): preserve flag/env channel when delegating to the Go proxy (#5546)

Coly010 · web-flow · commit bd39bcf5e613 · 2026-06-11T11:42:17.000Z
## What changed

Ported legacy commands delegate to the bundled Go binary ("the proxy")
via `LegacyGoProxy.exec`. The proxy must be called **1:1 with the user's
input channel**: a value supplied as a CLI flag must reach the proxy as
a flag, and a value supplied as an environment variable must reach the
proxy as an environment variable. Neither should be cross-mapped.

`bootstrap` violated this for the DB password. It resolved the password
from `--password`, the `SUPABASE_DB_PASSWORD` env var, **or** an
interactive prompt, then always re-emitted it to the delegated `db push`
subprocess as a `--password` flag — mapping an env-sourced value onto a
flag.

## Why this is the correct mapping

In Go, bootstrap's `-p` flag is `BindPFlag`'d to viper `DB_PASSWORD`
(`cmd/bootstrap.go:67`); `create.Run` funnels the resolved value into
the same viper key (`create.go:31`); and `db push` reads it back via
`viper.GetString("DB_PASSWORD")` (`flags/db_url.go:128`). With
`SetEnvPrefix("SUPABASE") + AutomaticEnv()` (`root.go:320-322`), the
standalone `db push` subprocess resolves `DB_PASSWORD` from **either**
its own `-p` flag **or** the `SUPABASE_DB_PASSWORD` env var. So the
faithful mapping is flag→flag, env→env.

## Changes

- **`go-proxy.service.ts` / `go-proxy.layer.ts`** — add an optional
per-call `{ env }` overlay to `LegacyGoProxy.exec`, merged over the
construction-time env on top of the inherited process env (`extendEnv:
true`). Backward-compatible; all other call sites are unchanged.
- **`bootstrap.handler.ts`** (step L) — `--password` flag is forwarded
as `--password` (flag→flag); an env- or prompt-sourced password is
forwarded as the `SUPABASE_DB_PASSWORD` env var (env→env).
- **`bootstrap.integration.test.ts`** — the proxy mock now captures env;
the existing flag test asserts flag-only, plus two new tests cover the
env-source and prompt-source channels.
- **`SIDE_EFFECTS.md`** — documents the channel-preserving behavior of
the delegated `db push`.

## Audit

An inventory of all ~74 proxy handlers under
`apps/cli/src/legacy/commands/` found `bootstrap`'s password to be the
**only** cross-mapping instance. Other commands that forward
`--password` only do so from their own `--password` flag (flag→flag),
none write `process.env` before `exec`, and global flags in
`legacy/cli/root.ts` are all flag→flag. No other command needed changes.

Resolves CLI-1617.
diff --git a/apps/cli/src/legacy/commands/bootstrap/SIDE_EFFECTS.md b/apps/cli/src/legacy/commands/bootstrap/SIDE_EFFECTS.md
@@ -97,17 +97,20 @@ Human banners are suppressed; a single structured result is emitted:
 ## Notes
 
 - **Interim Go-proxy delegation for migration push.** The push step shells out to the bundled
-  Go binary (`db push --include-roles --include-seed [--password …]`) until `db push` gets its
-  own native port (separate Linear issue). The sub-step is **not** instrumentation-wrapped (the
-  subprocess fires its own push telemetry). Known divergence: `LegacyGoProxy.exec` propagates the
-  exit code, so Go's push backoff is **not** reproduced (single attempt) — to be restored when
-  `db push` is natively ported. (`LegacyGoProxy.exec` exits the process on a non-zero exit rather
-  than returning a failure, so the step cannot be wrapped in `Effect.retry`.)
-- **Interim credential exposure.** Because the push step runs in a subprocess, the DB password is
-  passed as `--password <value>` and is therefore briefly visible in the OS process table for the
-  lifetime of that subprocess (Go runs `push.Run` in-process and never exposes it). The same
-  password is already written in plaintext to `<workdir>/.env` in the same directory, so the
-  incremental exposure is small; it is eliminated when `db push` is natively ported (no subprocess).
+  Go binary (`db push --include-roles --include-seed`) until `db push` gets its own native port
+  (separate Linear issue). The sub-step is **not** instrumentation-wrapped (the subprocess fires
+  its own push telemetry). Known divergence: `LegacyGoProxy.exec` propagates the exit code, so Go's
+  push backoff is **not** reproduced (single attempt) — to be restored when `db push` is natively
+  ported. (`LegacyGoProxy.exec` exits the process on a non-zero exit rather than returning a
+  failure, so the step cannot be wrapped in `Effect.retry`.)
+- **DB password is forwarded on the same channel the user supplied it (CLI-1617).** The proxy must
+  be called 1:1 with the user's input: a flag stays a flag, an env var stays an env var. So when the
+  user passed `-p/--password`, the push sub-step receives `--password <value>` (flag → flag); when
+  the password came from the `SUPABASE_DB_PASSWORD` env var **or** the interactive prompt, it is
+  forwarded as the `SUPABASE_DB_PASSWORD` env var instead (env → env), matching Go, which binds `-p`
+  to viper `DB_PASSWORD` and reads it back from viper in `db push`. A consequence is that an
+  env-/prompt-sourced password is no longer placed in the OS process table; only an explicit
+  `--password` flag is (the same password is already written in plaintext to `<workdir>/.env`).
 - The api-keys and health retries use the full Go `utils.NewBackoffPolicy` policy: exponential
   backoff, 3s initial interval, multiplier 1.5, 60s max interval (capped before jitter), ±50% jitter
   (randomization factor 0.5), 15m max-elapsed cap, and 8 retries (9 total attempts). The per-attempt
diff --git a/apps/cli/src/legacy/commands/bootstrap/bootstrap.handler.ts b/apps/cli/src/legacy/commands/bootstrap/bootstrap.handler.ts
@@ -253,9 +253,33 @@ export const legacyBootstrap = Effect.fn("legacy.bootstrap")(function* (
     // No instrumentation wrap: the subprocess fires its own push telemetry.
     // `bootstrap.go:122-127` -> push.Run(..., includeRoles, includeSeed) =>
     // `--include-roles --include-seed` (no `--include-all`).
+    //
+    // Channel parity (CLI-1617): the proxy must be called 1:1 with the user's
+    // input — a flag stays a flag, an env var stays an env var. Go binds
+    // bootstrap's `-p` to viper `DB_PASSWORD` and `db push` reads it from viper
+    // (== the `SUPABASE_DB_PASSWORD` env var for the subprocess), so only a
+    // flag-sourced password travels as `--password`; an env-/prompt-sourced one
+    // travels as the env var.
+    //
+    // The flag branch keys on a *non-empty* flag value: an explicit `--password ""`
+    // (e.g. an unset `$SUPABASE_DB_PASSWORD` expanded by the shell) leaves viper
+    // `DB_PASSWORD` empty in Go too, so `create.promptMissingParams` prompts and
+    // `viper.Set`s the resolved value — which in-process `db push` then reads.
+    // Forwarding the literal empty flag would lose that prompted password, so an
+    // empty flag falls through to the resolved `created.dbPassword` (which carries
+    // the env- or prompt-sourced value) on the env channel.
     const pushArgs = ["db", "push", "--include-roles", "--include-seed"];
-    if (created.dbPassword.length > 0) pushArgs.push("--password", created.dbPassword);
-    yield* proxy.exec(pushArgs);
+    if (Option.isSome(flags.password) && flags.password.value.length > 0) {
+      pushArgs.push("--password", flags.password.value);
+      yield* proxy.exec(pushArgs);
+    } else {
+      yield* proxy.exec(
+        pushArgs,
+        created.dbPassword.length > 0
+          ? { env: { SUPABASE_DB_PASSWORD: created.dbPassword } }
+          : undefined,
+      );
+    }
 
     // M. Start suggestion. `bootstrap.go:128-130`.
     if (isText) {
diff --git a/apps/cli/src/legacy/commands/bootstrap/bootstrap.integration.test.ts b/apps/cli/src/legacy/commands/bootstrap/bootstrap.integration.test.ts
@@ -80,13 +80,15 @@ interface SetupOpts {
   readonly health?: { readonly status: number; readonly body: unknown };
   readonly promptTextResponses?: ReadonlyArray<string>;
   readonly promptConfirmResponses?: ReadonlyArray<boolean>;
+  readonly promptPasswordResponses?: ReadonlyArray<string>;
 }
 
 function setup(opts: SetupOpts = {}) {
   const out = mockOutput({
     format: opts.format ?? "text",
     promptTextResponses: opts.promptTextResponses,
     promptConfirmResponses: opts.promptConfirmResponses,
+    promptPasswordResponses: opts.promptPasswordResponses,
   });
   const telemetry = mockLegacyTelemetryStateTracked();
   const linkedCache = mockLegacyLinkedProjectCacheTracked();
@@ -136,11 +138,11 @@ function setup(opts: SetupOpts = {}) {
       }),
   });
 
-  const proxyCalls: Array<ReadonlyArray<string>> = [];
+  const proxyCalls: Array<{ args: ReadonlyArray<string>; env?: Record<string, string> }> = [];
   const proxyLayer = Layer.succeed(LegacyGoProxy, {
-    exec: (args: ReadonlyArray<string>) =>
+    exec: (args: ReadonlyArray<string>, execOpts?: { env?: Record<string, string> }) =>
       Effect.sync(() => {
-        proxyCalls.push(args);
+        proxyCalls.push({ args, env: execOpts?.env });
       }),
   });
 
@@ -394,25 +396,103 @@ describe("legacy bootstrap integration", () => {
     }).pipe(Effect.provide(s.layer));
   });
 
-  it.live("delegates the db push step to the Go proxy with the resolved password", () => {
+  it.live("forwards a --password flag to the Go proxy as a flag (flag stays a flag)", () => {
     const s = setup();
     return Effect.gen(function* () {
       yield* legacyBootstrap(
         flags({ template: Option.some("scratch"), password: Option.some("pw123") }),
         FAST_BACKOFF,
       );
       expect(s.proxyCalls).toHaveLength(1);
-      expect(s.proxyCalls[0]).toEqual([
+      // Flag-sourced password travels as a flag, never re-mapped to an env var.
+      expect(s.proxyCalls[0]?.args).toEqual([
         "db",
         "push",
         "--include-roles",
         "--include-seed",
         "--password",
         "pw123",
       ]);
+      expect(s.proxyCalls[0]?.env).toBeUndefined();
     }).pipe(Effect.provide(s.layer));
   });
 
+  it.live("forwards the prompted password (not the empty flag) when --password is empty", () => {
+    // An explicit `--password ""` (e.g. unset `$SUPABASE_DB_PASSWORD` expanded by
+    // the shell) leaves the password empty, so the create step prompts. Go reads
+    // the prompted value from viper for the in-process push, so the subprocess
+    // must receive the prompted password — never the literal empty flag value.
+    const s = setup({ promptPasswordResponses: ["prompted-pw"] });
+    const prev = process.env["SUPABASE_DB_PASSWORD"];
+    delete process.env["SUPABASE_DB_PASSWORD"];
+    return Effect.gen(function* () {
+      yield* legacyBootstrap(
+        flags({ template: Option.some("scratch"), password: Option.some("") }),
+        FAST_BACKOFF,
+      );
+      expect(s.proxyCalls).toHaveLength(1);
+      expect(s.proxyCalls[0]?.args).toEqual(["db", "push", "--include-roles", "--include-seed"]);
+      expect(s.proxyCalls[0]?.env).toEqual({ SUPABASE_DB_PASSWORD: "prompted-pw" });
+    }).pipe(
+      Effect.provide(s.layer),
+      Effect.ensuring(
+        Effect.sync(() => {
+          if (prev === undefined) delete process.env["SUPABASE_DB_PASSWORD"];
+          else process.env["SUPABASE_DB_PASSWORD"] = prev;
+        }),
+      ),
+    );
+  });
+
+  it.live("forwards a SUPABASE_DB_PASSWORD env var to the proxy as an env var", () => {
+    const s = setup();
+    const prev = process.env["SUPABASE_DB_PASSWORD"];
+    process.env["SUPABASE_DB_PASSWORD"] = "env-pw";
+    return Effect.gen(function* () {
+      yield* legacyBootstrap(
+        flags({ template: Option.some("scratch"), password: Option.none() }),
+        FAST_BACKOFF,
+      );
+      expect(s.proxyCalls).toHaveLength(1);
+      // Env-sourced password stays an env var — no --password flag mapping (CLI-1617).
+      expect(s.proxyCalls[0]?.args).toEqual(["db", "push", "--include-roles", "--include-seed"]);
+      expect(s.proxyCalls[0]?.env).toEqual({ SUPABASE_DB_PASSWORD: "env-pw" });
+    }).pipe(
+      Effect.provide(s.layer),
+      Effect.ensuring(
+        Effect.sync(() => {
+          if (prev === undefined) delete process.env["SUPABASE_DB_PASSWORD"];
+          else process.env["SUPABASE_DB_PASSWORD"] = prev;
+        }),
+      ),
+    );
+  });
+
+  it.live("forwards a prompted password to the proxy as an env var, not a flag", () => {
+    const s = setup({ promptPasswordResponses: ["prompted-pw"] });
+    const prev = process.env["SUPABASE_DB_PASSWORD"];
+    delete process.env["SUPABASE_DB_PASSWORD"];
+    return Effect.gen(function* () {
+      yield* legacyBootstrap(
+        flags({ template: Option.some("scratch"), password: Option.none() }),
+        FAST_BACKOFF,
+      );
+      expect(s.proxyCalls).toHaveLength(1);
+      // Go funnels the prompted value into viper DB_PASSWORD; the subprocess
+      // equivalent is the SUPABASE_DB_PASSWORD env var.
+      expect(s.proxyCalls[0]?.args).toEqual(["db", "push", "--include-roles", "--include-seed"]);
+      expect(s.proxyCalls[0]?.env).toEqual({ SUPABASE_DB_PASSWORD: "prompted-pw" });
+    }).pipe(
+      Effect.provide(s.layer),
+      Effect.ensuring(
+        Effect.sync(() => {
+          if (prev === undefined) delete process.env["SUPABASE_DB_PASSWORD"];
+          else process.env["SUPABASE_DB_PASSWORD"] = prev;
+        }),
+      ),
+    );
+  });
+
   it.live("flushes telemetry and caches the linked project via ensuring", () => {
     const s = setup();
     return Effect.gen(function* () {
diff --git a/apps/cli/src/shared/legacy/go-proxy.layer.ts b/apps/cli/src/shared/legacy/go-proxy.layer.ts
@@ -197,9 +197,13 @@ export function makeGoProxyLayer(opts?: {
               // Scoped via `Effect.scoped` so listeners are always removed on
               // normal completion, failure, or fiber interruption.
               yield* processControl.holdSignals(["SIGINT", "SIGTERM", "SIGHUP"]);
+              // Per-call env (execOpts.env) overlays the construction-time env;
+              // `extendEnv: true` keeps both on top of the inherited process env.
+              const env =
+                opts?.env || execOpts?.env ? { ...opts?.env, ...execOpts?.env } : undefined;
               const command = ChildProcess.make(binary, [...globalArgs, ...args], {
                 cwd: execOpts?.cwd ?? opts?.cwd,
-                env: opts?.env,
+                env,
                 extendEnv: true,
                 stdin: "inherit",
                 stdout: "inherit",
diff --git a/apps/cli/src/shared/legacy/go-proxy.service.ts b/apps/cli/src/shared/legacy/go-proxy.service.ts
@@ -6,10 +6,17 @@ interface LegacyGoProxyShape {
    * Forward the given args to the Go binary, inheriting stdin/stdout/stderr
    * and propagating the exit code. On a non-zero exit the process exits with
    * the same code — callers do not need to handle the failure case.
+   *
+   * `opts.cwd` overrides the working directory for this call (falls back to the
+   * layer's construction-time cwd). `opts.env` overlays extra environment
+   * variables onto the subprocess (merged on top of the inherited process env);
+   * use it to pass values the user supplied as environment variables back to the
+   * proxy as environment variables, rather than cross-mapping them onto CLI
+   * flags (CLI-1617).
    */
   readonly exec: (
     args: ReadonlyArray<string>,
-    opts?: { readonly cwd?: string },
+    opts?: { readonly cwd?: string; readonly env?: Record<string, string> },
   ) => Effect.Effect<void>;
 }
 
