From a6c6cb942bb08c65a762397c2e10b161b21deabe Mon Sep 17 00:00:00 2001 From: Colum Ferry Date: Wed, 5 Aug 2026 14:04:16 +0100 Subject: [PATCH 01/19] fix(functions): port functions download to native TypeScript (CLI-1963) Ports `supabase functions download`'s default Docker-unbundle path (`--use-docker`, default true) from wholesale Go-binary delegation to native TypeScript, in both the legacy and next shells. `--use-api` was already native; `--legacy-bundle` (hidden, deprecated pre-1.120.0 fallback requiring a host Deno-binary install with no precedent elsewhere in this codebase) is deliberately left delegating to the Go binary, per the parity-audit rationale recorded on the Linear issue. Hoists the Docker-orchestration primitives `download.ts` needs (`runChildProcess`, `isDockerRunning`, `ensureDockerNetwork`, `ensureDockerNamedVolume`, `localDockerId`, `resolveEdgeRuntimeVersion`, etc.) out of `deploy.ts` into a new `functions-docker.ts`, and deduplicates the `edge-runtime-version` pin file lookup that was copy-pasted across all four `deploy`/`download` handler files into a single `resolveEdgeRuntimeVersionPin` helper. Along the way, fixes: - CLI-1891-class validation gap: slugs sourced from the Management API's function list weren't validated before the new Docker path's temp-file write, reopening a path-traversal vector Go's own `downloadAll` already guards against. - The `next` shell's `--use-docker` flag was missing `Flag.withDefault(true)`, a real default-value divergence from both `legacy` and Go. - A brotli-decompression bug: this CLI's HTTP transport already auto-decodes `Content-Encoding: br` responses (confirmed empirically), so re-running `brotliDecompressSync` on the eszip body threw on already-decoded bytes. - Temp eszip cleanup only ran after a successful Docker run; wrapped in `Effect.ensuring` so it also runs on network/volume/spawn failures, matching Go's `defer`. - The `.suggestion` field's leading newline (needed to reproduce Go's blank separator line before the `--legacy-bundle` hint) was being trimmed away by the generic CLI error normalizer. --- apps/cli/docs/go-cli-porting-status.md | 2 +- .../functions/deploy/deploy.handler.ts | 11 +- .../functions/download/SIDE_EFFECTS.md | 128 +-- .../functions/download/download.handler.ts | 11 +- .../download/download.integration.test.ts | 776 +++++++++++++++--- .../functions/serve/serve.integration.test.ts | 10 +- .../commands/start/lib/container-lifecycle.ts | 14 +- .../functions/deploy/deploy.handler.ts | 12 +- .../functions/download/download.command.ts | 1 + .../functions/download/download.handler.ts | 8 +- .../download/download.integration.test.ts | 355 ++++++-- apps/cli/src/shared/cli/cobra-flag-groups.ts | 25 + .../src/shared/cli/hidden-flag.unit.test.ts | 28 +- apps/cli/src/shared/functions/deploy.ts | 226 +---- apps/cli/src/shared/functions/download.ts | 441 +++++++++- .../src/shared/functions/functions-docker.ts | 188 +++++ .../src/shared/functions/functions.shared.ts | 22 + apps/cli/src/shared/functions/serve.ts | 12 +- apps/cli/src/shared/output/normalize-error.ts | 10 +- 19 files changed, 1783 insertions(+), 497 deletions(-) create mode 100644 apps/cli/src/shared/functions/functions-docker.ts diff --git a/apps/cli/docs/go-cli-porting-status.md b/apps/cli/docs/go-cli-porting-status.md index 5f461d0990..2c8eaa667a 100644 --- a/apps/cli/docs/go-cli-porting-status.md +++ b/apps/cli/docs/go-cli-porting-status.md @@ -287,7 +287,7 @@ Legend: | `gen keys` | `wrapped` | [`../src/legacy/commands/gen/keys/keys.command.ts`](../src/legacy/commands/gen/keys/keys.command.ts) | | `functions list` | `wrapped` | [`../src/legacy/commands/functions/list/list.command.ts`](../src/legacy/commands/functions/list/list.command.ts) | | `functions delete` | `ported` | [`../src/legacy/commands/functions/delete/delete.command.ts`](../src/legacy/commands/functions/delete/delete.command.ts) | -| `functions download` | `ported` | [`../src/legacy/commands/functions/download/download.command.ts`](../src/legacy/commands/functions/download/download.command.ts) | +| `functions download` | `ported` | [`../src/legacy/commands/functions/download/download.command.ts`](../src/legacy/commands/functions/download/download.command.ts) — native for `--use-api` and the default Docker-unbundle path (`--use-docker`, CLI-1963); hidden `--legacy-bundle` still delegates to the Go binary (pre-1.120.0 fallback requiring a host Deno-binary install with no precedent elsewhere in this codebase — tracked separately, see CLI-1963) | | `functions deploy` | `ported` | [`../src/legacy/commands/functions/deploy/deploy.command.ts`](../src/legacy/commands/functions/deploy/deploy.command.ts) | | `functions new` | `ported` | [`../src/legacy/commands/functions/new/new.command.ts`](../src/legacy/commands/functions/new/new.command.ts) | | `functions serve` | `ported` | [`../src/legacy/commands/functions/serve/serve.command.ts`](../src/legacy/commands/functions/serve/serve.command.ts) | diff --git a/apps/cli/src/legacy/commands/functions/deploy/deploy.handler.ts b/apps/cli/src/legacy/commands/functions/deploy/deploy.handler.ts index 00bb20b7ba..59bec4fd78 100644 --- a/apps/cli/src/legacy/commands/functions/deploy/deploy.handler.ts +++ b/apps/cli/src/legacy/commands/functions/deploy/deploy.handler.ts @@ -1,8 +1,7 @@ -import { DEFAULT_VERSIONS } from "@supabase/stack/effect"; -import { readFile } from "node:fs/promises"; import { join } from "node:path"; import { Effect, Option, Stdio } from "effect"; import { deployFunctions } from "../../../../shared/functions/deploy.ts"; +import { resolveEdgeRuntimeVersionPin } from "../../../../shared/functions/functions.shared.ts"; import { legacyAqua, legacyBold } from "../../../shared/legacy-colors.ts"; import { LegacyPlatformApi } from "../../../auth/legacy-platform-api.service.ts"; import { LegacyCliConfig } from "../../../config/legacy-cli-config.service.ts"; @@ -29,12 +28,8 @@ export const legacyFunctionsDeploy = Effect.fn("legacy.functions.deploy")(functi const runtimeInfo = yield* RuntimeInfo; const stdio = yield* Stdio.Stdio; const rawArgs = yield* stdio.args; - const edgeRuntimeVersion = yield* Effect.tryPromise(() => - readFile(join(cliConfig.workdir, "supabase", ".temp", "edge-runtime-version"), "utf8"), - ).pipe( - Effect.map((version) => version.trim()), - Effect.catch(() => Effect.succeed("")), - Effect.map((version) => version || DEFAULT_VERSIONS["edge-runtime"]), + const edgeRuntimeVersion = yield* resolveEdgeRuntimeVersionPin( + join(cliConfig.workdir, "supabase"), ); let resolvedProjectRef = Option.none(); diff --git a/apps/cli/src/legacy/commands/functions/download/SIDE_EFFECTS.md b/apps/cli/src/legacy/commands/functions/download/SIDE_EFFECTS.md index 9c9c6a1845..a32ee01293 100644 --- a/apps/cli/src/legacy/commands/functions/download/SIDE_EFFECTS.md +++ b/apps/cli/src/legacy/commands/functions/download/SIDE_EFFECTS.md @@ -2,45 +2,53 @@ ## Files Read -| Path | Format | When | -| ----------------------------------------------- | ---------- | ------------------------------------------------------------- | -| `/access-token` | plain text | when `SUPABASE_ACCESS_TOKEN` unset and keyring unavailable | -| `/profile` | plain text | when `--profile` and `SUPABASE_PROFILE` are both unset | -| `.yaml` | YAML | when `SUPABASE_PROFILE` or `--profile` points to a file | -| `/supabase/.temp/project-ref` | plain text | when `--project-ref` and `SUPABASE_PROJECT_ID` are both unset | -| `/telemetry.json` | JSON | when present, before post-run telemetry state is refreshed | +| Path | Format | When | +| --------------------------------------------------- | ---------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `/access-token` | plain text | when `SUPABASE_ACCESS_TOKEN` unset and keyring unavailable | +| `/profile` | plain text | when `--profile` and `SUPABASE_PROFILE` are both unset | +| `.yaml` | YAML | when `SUPABASE_PROFILE` or `--profile` points to a file | +| `/supabase/.temp/project-ref` | plain text | when `--project-ref` and `SUPABASE_PROJECT_ID` are both unset | +| `/supabase/.temp/edge-runtime-version` | plain text | Docker-unbundle path: overrides the default edge-runtime image tag when present | +| `/supabase/config.toml` (or `config.json`) | TOML/JSON | Docker-unbundle path: resolves `edge_runtime.deno_version` and `project_id` (`loadProjectConfig`) — a new file-read surface versus the `--use-api` path, which reads no project config | +| `/telemetry.json` | JSON | when present, before post-run telemetry state is refreshed | ## Files Written -| Path | Format | When | -| --------------------------------------------------- | ------ | ----------------------------------------------------------------------- | -| `/supabase/functions//` | bytes | for each source file returned by the API | -| `/supabase/.temp/linked-project.json` | JSON | after resolving a project ref, cached on both success and failure paths | -| `/telemetry.json` | JSON | after command completion, flushed on both success and failure paths | +| Path | Format | When | +| --------------------------------------------------- | ------ | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `/supabase/functions//` | bytes | for each source file returned by the API (`--use-api`, or the Docker-unbundle fallback when Docker isn't running) | +| `/supabase/.temp/output_.eszip` | bytes | Docker-unbundle path (default): downloaded eszip, extracted into `supabase/functions//...` by the edge-runtime container; removed after the attempt unless `--debug` is set | +| `/supabase/.temp/linked-project.json` | JSON | after resolving a project ref, cached on both success and failure paths | +| `/telemetry.json` | JSON | after command completion, flushed on both success and failure paths | ## API Routes -| Method | Path | Auth | Request body | Response (used fields) | -| ------ | ------------------------------------------ | ------------ | ------------ | ----------------------------------------------------- | -| `GET` | `/v1/projects/{ref}/functions` | Bearer token | none | function slugs, when downloading all | -| `GET` | `/v1/projects/{ref}/functions/{slug}` | Bearer token | none | entrypoint path, when absent from metadata | -| `GET` | `/v1/projects/{ref}/functions/{slug}/body` | Bearer token | none | multipart function source | -| `GET` | `/v1/projects` | Bearer token | none | project picker options when no ref is supplied in TTY | -| `GET` | `/v1/projects/{ref}` | Bearer token | none | linked project metadata used by the post-run cache | +| Method | Path | Auth | Request body | Response (used fields) | +| ------ | ------------------------------------------ | ------------ | ------------ | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `GET` | `/v1/projects/{ref}/functions` | Bearer token | none | function slugs, when downloading all | +| `GET` | `/v1/projects/{ref}/functions/{slug}` | Bearer token | none | entrypoint path, when absent from multipart metadata (`--use-api` path only) | +| `GET` | `/v1/projects/{ref}/functions/{slug}/body` | Bearer token | none | `--use-api`: multipart function source (`Accept: multipart/form-data`). Docker-unbundle: raw eszip bytes; a `Content-Encoding: br` response is decoded transparently by the HTTP transport, not by this command | +| `GET` | `/v1/projects` | Bearer token | none | project picker options when no ref is supplied in TTY | +| `GET` | `/v1/projects/{ref}` | Bearer token | none | linked project metadata used by the post-run cache | ## Subprocesses -| Command | When | Purpose | -| ------------------------------------ | ----------------------------------------------------------------- | ----------------------------------- | -| `supabase-go functions download ...` | `--use-docker` (default) or `--legacy-bundle`, unless `--use-api` | preserve hidden compatibility modes | - -The delegated call runs with `SUPABASE_TELEMETRY_DISABLED=1` so the Go child's -own `cli_command_executed` doesn't double-count on top of this command's own -telemetry (mirrors `db pull`/`db diff`'s delegated-call pattern). In -`--output-format json|stream-json`, the child's stdout is captured and -discarded instead of inherited (`LegacyGoProxy.execCapture`) — the raw text -never reaches the terminal, and this command emits the `Output` envelope -itself once the child exits successfully. +| Command | When | Purpose | +| ---------------------------------------------------------------------------- | -------------------------------------------- | ----------------------------------------------------------------------------------------------------------------- | +| `docker info` | `--use-docker` (default), unless `--use-api` | check whether Docker is running before choosing the Docker-unbundle downloader | +| `docker network inspect` / `network create` / `volume create` | Docker-unbundle path, when Docker is running | ensure the shared per-project network/named volume exist (same primitives as `functions deploy`'s Docker bundler) | +| `docker run --rm ... unbundle --eszip ... --output ...` | Docker-unbundle path, when Docker is running | extract the downloaded eszip into `supabase/functions//...` | +| `supabase-go functions download ... --legacy-bundle` | `--legacy-bundle` only | preserve the hidden, deprecated pre-1.120.0 bundling fallback (native TS port tracked separately, CLI-1963) | + +The `--legacy-bundle` delegated call runs with `SUPABASE_TELEMETRY_DISABLED=1` +so the Go child's own `cli_command_executed` doesn't double-count on top of +this command's own telemetry (mirrors `db pull`/`db diff`'s delegated-call +pattern). In `--output-format json|stream-json`, the child's stdout is +captured and discarded instead of inherited (`LegacyGoProxy.execCapture`) — +the raw text never reaches the terminal, and this command emits the `Output` +envelope itself once the child exits successfully. The Docker-unbundle path's +own container stdout is routed the same way: to the real stdout in text mode, +to stderr in machine-output modes (CLI-1546). ## Environment Variables @@ -55,13 +63,14 @@ itself once the child exits successfully. ## Exit Codes -| Code | Condition | -| ---- | -------------------------------------- | -| `0` | success | -| `1` | API error (non-2xx response) | -| `1` | authentication error (no token found) | -| `1` | network / connection failure | -| `1` | invalid function slug or flag conflict | +| Code | Condition | +| ---- | ---------------------------------------------------------------------- | +| `0` | success | +| `1` | API error (non-2xx response) | +| `1` | authentication error (no token found) | +| `1` | network / connection failure | +| `1` | invalid function slug or flag conflict | +| `1` | Docker-unbundle container exited non-zero (suggests `--legacy-bundle`) | ## Telemetry Events Fired @@ -73,25 +82,50 @@ itself once the child exits successfully. ### `--output-format text` (Go CLI compatible) -Prints progress and success messages as functions are downloaded. +Prints progress and success messages as functions are downloaded. The Docker-unbundle path prints +`Downloading function: ` (lowercase "function", unlike the `--use-api` path's "Downloading +Function:") and does **not** print a final "Downloaded Function ... from project ..." line — that +line only appears on the `--use-api` and `--legacy-bundle` paths (Go parity, `download.go`). ### `--output-format json` Prints a structured success result with the downloaded function slugs and project ref. On the -Docker/legacy-bundle proxy path, the Go child's stdout is captured/discarded (never inherited) so -it can't corrupt the envelope; the slug list is resolved independently for the payload. +`--legacy-bundle` proxy path, the Go child's stdout is captured/discarded (never inherited) so it +can't corrupt the envelope; the slug list is resolved independently for the payload. On the +Docker-unbundle path, the `unbundle` container's own stdout is routed to stderr instead of stdout +for the same reason. ### `--output-format stream-json` -Same envelope as `json` above (including on the proxy path). +Same envelope as `json` above (including on the proxy and Docker-unbundle paths). ## Notes - If no function name is provided, downloads all functions. - Requires a linked project (`--project-ref` or linked project config). -- Native downloads reject path traversal and symlink escapes before writing source files. -- `--use-docker` and `--legacy-bundle` are hidden flags forwarded to the Go binary for backward compatibility; they are mutually exclusive with `--use-api`. -- `--use-docker` defaults to `true` (Go parity), so a bare `supabase functions download` proxies to the Go binary's Docker-based unbundler unless `--use-api` resolves to `true`, which forces the native server-side download path instead (`apps/cli-go/cmd/functions.go:51-53`: `if useApi { useDocker = false }` reads the resolved flag value, not presence — `--use-api=false` still proxies). -- If Docker is not running, the Go binary itself prints `WARNING: Docker is not running` to stderr and falls back to its own server-side unbundler — the command still exits `0` without Docker installed or running. -- The mutual-exclusivity check only counts flags the user explicitly passed on the command line, not `--use-docker`'s default value — so `--use-api` alone never trips the "mutually exclusive" error. The Go proxy call itself also only ever forwards one of `--use-docker`/`--legacy-bundle`, never both, even though `--use-docker` defaults to `true`. -- Refreshes the linked-project telemetry cache and flushes telemetry state after resolving a project ref. +- The `--use-api` path rejects path traversal and symlink escapes before writing source files + (`resolveDownloadDestination`/`ensureContainedPath`) — the Docker-unbundle path has no equivalent + check of its own; it delegates the actual file writes to the `unbundle` subcommand running inside + the edge-runtime container, through the `supabase/functions` bind mount, matching Go's own + `extractOne` (which has no path-containment check either — this is a pre-existing, not + CLI-1963-introduced, gap shared with the Go CLI). Slugs sourced from the Management API's function + list (downloading-all) are validated against the same pattern as user-supplied slugs, on both + paths, before any per-slug download runs (CLI-1891 parity). +- `--legacy-bundle` is a hidden flag forwarded to the Go binary for backward compatibility — it + requires installing a real Deno binary on the host (`InstallOrUpgradeDeno`) and is a pre-1.120.0 + compatibility fallback; native TS port tracked separately (CLI-1963). `--use-docker` is a hidden + flag but now runs natively. +- `--use-docker`, `--use-api`, and `--legacy-bundle` are mutually exclusive. +- `--use-docker` defaults to `true` (Go parity), so a bare `supabase functions download` runs the + native Docker-unbundle downloader unless `--use-api` resolves to `true`, which forces the native + server-side download path instead (`apps/cli-go/cmd/functions.go:51-53`: `if useApi { useDocker = +false }` reads the resolved flag value, not presence — `--use-api=false` still runs Docker-unbundle). +- If Docker is not running, this command itself prints `WARNING: Docker is not running` to stderr + and falls back to the native server-side unbundler — the command still exits `0` without Docker + installed or running. +- The mutual-exclusivity check only counts flags the user explicitly passed on the command line, + not `--use-docker`'s default value — so `--use-api` alone never trips the "mutually exclusive" + error. The `--legacy-bundle` Go proxy call itself only ever forwards `--legacy-bundle`, never + `--use-docker` alongside it, even though `--use-docker` defaults to `true`. +- Refreshes the linked-project telemetry cache and flushes telemetry state after resolving a + project ref. diff --git a/apps/cli/src/legacy/commands/functions/download/download.handler.ts b/apps/cli/src/legacy/commands/functions/download/download.handler.ts index 4b666da7f6..53886acf72 100644 --- a/apps/cli/src/legacy/commands/functions/download/download.handler.ts +++ b/apps/cli/src/legacy/commands/functions/download/download.handler.ts @@ -1,8 +1,10 @@ +import { join } from "node:path"; import { Effect, Option, Stdio } from "effect"; import { downloadFunctions, - makeGoProxyDownloadArgs, + makeGoProxyLegacyBundleArgs, } from "../../../../shared/functions/download.ts"; +import { resolveEdgeRuntimeVersionPin } from "../../../../shared/functions/functions.shared.ts"; import { LegacyGoProxy } from "../../../../shared/legacy/go-proxy.service.ts"; import { LegacyPlatformApi } from "../../../auth/legacy-platform-api.service.ts"; import { LegacyCliConfig } from "../../../config/legacy-cli-config.service.ts"; @@ -22,12 +24,17 @@ export const legacyFunctionsDownload = Effect.fn("legacy.functions.download")(fu const proxy = yield* LegacyGoProxy; const stdio = yield* Stdio.Stdio; const rawArgs = yield* stdio.args; + const edgeRuntimeVersion = yield* resolveEdgeRuntimeVersionPin( + join(cliConfig.workdir, "supabase"), + ); let resolvedProjectRef = Option.none(); yield* downloadFunctions(flags, { api, projectRoot: cliConfig.workdir, rawArgs, + goViperCompat: true, + edgeRuntimeVersion, resolveProjectRef: (projectRef) => resolver.resolve(projectRef).pipe( Effect.tap((ref) => @@ -47,7 +54,7 @@ export const legacyFunctionsDownload = Effect.fn("legacy.functions.download")(fu // pattern for the CLI-1546 "stdout is payload-only in machine mode" // invariant — `downloadFunctions` emits the `Output` envelope itself. proxyDownload: (proxyFlags, projectRef, captureOutput) => { - const args = makeGoProxyDownloadArgs(proxyFlags, projectRef); + const args = makeGoProxyLegacyBundleArgs(proxyFlags.functionName, projectRef); const env = { SUPABASE_TELEMETRY_DISABLED: "1" }; return captureOutput ? Effect.asVoid(proxy.execCapture(args, { env, stdin: "ignore" })) diff --git a/apps/cli/src/legacy/commands/functions/download/download.integration.test.ts b/apps/cli/src/legacy/commands/functions/download/download.integration.test.ts index af37969828..b58604ab77 100644 --- a/apps/cli/src/legacy/commands/functions/download/download.integration.test.ts +++ b/apps/cli/src/legacy/commands/functions/download/download.integration.test.ts @@ -1,7 +1,10 @@ import { describe, expect, it } from "@effect/vitest"; -import { readFile } from "node:fs/promises"; -import { join } from "node:path"; -import { Effect, Exit, Layer, Option, Stdio } from "effect"; +import { DEFAULT_VERSIONS } from "@supabase/stack/effect"; +import { existsSync } from "node:fs"; +import { mkdir, readFile, writeFile } from "node:fs/promises"; +import { join, resolve } from "node:path"; +import { Deferred, Effect, Exit, Layer, Option, PlatformError, Sink, Stdio, Stream } from "effect"; +import { ChildProcessSpawner } from "effect/unstable/process"; import * as HttpClientResponse from "effect/unstable/http/HttpClientResponse"; import { commandRuntimeLayer } from "../../../../shared/runtime/command-runtime.layer.ts"; @@ -17,12 +20,107 @@ import { useLegacyTempWorkdir, } from "../../../../../tests/helpers/legacy-mocks.ts"; import { mockOutput } from "../../../../../tests/helpers/mocks.ts"; +import { mockChildProcessSpawner } from "../../../../../../../packages/process-compose/tests/helpers/mocks.ts"; import { LegacyGoProxy } from "../../../../shared/legacy/go-proxy.service.ts"; +import { legacyContainerRuntimeNotFoundMessage } from "../../../shared/legacy-container-cli.ts"; import { ConflictingFunctionDownloadFlagsError } from "../../../../shared/functions/download.errors.ts"; import { legacyFunctionsDownloadHandler } from "./download.command.ts"; import type { LegacyFunctionsDownloadFlags } from "./download.command.ts"; import { legacyFunctionsDownload } from "./download.handler.ts"; +const PROJECT_ID = "abcdefghijklmnopqrst"; + +/** + * Mutates the shared spawner options object from inside `onSpawn`, scoped to + * the `docker run ... unbundle` invocation specifically — every earlier + * Docker call (`info`, `network inspect`, `volume create`) in the same test + * already resolved by the time this fires, since `download.ts` awaits each + * child process sequentially, so this only ever affects the unbundle step's + * own exit code/stdio. + */ +function mockDockerUnbundle( + opts: { + readonly runExitCode?: number; + readonly runStdout?: ReadonlyArray; + readonly runStderr?: ReadonlyArray; + } = {}, +) { + const spawnerOpts: { + exitCode?: number; + stdout?: string[]; + stderr?: string[]; + onSpawn?: (record: { command: string; args: ReadonlyArray }) => void; + } = { exitCode: 0 }; + spawnerOpts.onSpawn = (record) => { + if (record.command === "docker" && record.args[0] === "run") { + spawnerOpts.exitCode = opts.runExitCode ?? 0; + spawnerOpts.stdout = opts.runStdout === undefined ? [] : [...opts.runStdout]; + spawnerOpts.stderr = opts.runStderr === undefined ? [] : [...opts.runStderr]; + } + }; + return mockChildProcessSpawner(spawnerOpts); +} + +/** + * A real ENOENT-style spawn failure for the `docker run ... unbundle` step + * specifically — distinct from `mockDockerUnbundle`'s non-zero exit code, + * which models the container starting but the `unbundle` binary itself + * failing. This models `child_process.spawn` (or the container runtime + * binary) never starting at all, which `runChildProcess` surfaces as an + * `unknown` cause rather than an `{ exitCode, stdout, stderr }` result. + * Mirrors `legacy-container-cli.unit.test.ts`'s `mockSpawner({ bothMissing: + * true })`: failing both the `docker` and `podman` fallback attempts for the + * `run` step is what makes `spawnContainerCli` surface + * `legacyContainerRuntimeNotFoundMessage` instead of retrying indefinitely. + * Every other Docker call (`info`, `network inspect`, `volume create`) + * succeeds with exit code 0, so only the unbundle step itself fails. + */ +function mockDockerRunSpawnFailure() { + const spawned: Array<{ command: string; args: ReadonlyArray }> = []; + const spawner = ChildProcessSpawner.make((command) => + Effect.gen(function* () { + const cmd = command._tag === "StandardCommand" ? command.command : ""; + const args = command._tag === "StandardCommand" ? command.args : []; + spawned.push({ command: cmd, args }); + + if (args[0] === "run") { + return yield* Effect.fail( + PlatformError.systemError({ + _tag: "NotFound", + module: "ChildProcess", + method: "spawn", + description: `${cmd} not found`, + }), + ); + } + + const exitDeferred = yield* Deferred.make(); + yield* Deferred.succeed(exitDeferred, ChildProcessSpawner.ExitCode(0)); + + return ChildProcessSpawner.makeHandle({ + pid: ChildProcessSpawner.ProcessId(1000 + spawned.length), + stdout: Stream.empty, + stderr: Stream.empty, + all: Stream.empty, + exitCode: Deferred.await(exitDeferred), + isRunning: Effect.succeed(false), + stdin: Sink.drain, + kill: () => Effect.void, + unref: Effect.succeed(Effect.void), + getInputFd: () => Sink.drain, + getOutputFd: () => Stream.empty, + }); + }), + ); + + return { + get spawned() { + return spawned; + }, + layer: Layer.succeed(ChildProcessSpawner.ChildProcessSpawner, spawner), + }; +} + const tempRoot = useLegacyTempWorkdir("supabase-functions-download-legacy-"); // `withLegacyCommandInstrumentation` threads `flags`/`command`/etc. through @@ -157,49 +255,70 @@ describe("legacy functions download", () => { }).pipe(Effect.provide(layer)); }); - it.live("proxies to Docker by default (Go parity), with no flags passed", () => { - const out = mockOutput({ format: "text" }); - const api = mockLegacyPlatformApi(); - const proxy = mockProxy(); - const layer = Layer.mergeAll( - buildLegacyTestRuntime({ - out, - api, - cliConfig: mockLegacyCliConfig({ workdir: tempRoot.current }), - }), - proxy.layer, - Stdio.layerTest({ - args: Effect.succeed([ - "functions", - "download", - "hello-world", - "--project-ref", - "abcdefghijklmnopqrst", - ]), - }), - ); + it.live( + "runs the native Docker unbundle path by default (Go parity), with no flags passed", + () => { + const out = mockOutput({ format: "text" }); + const api = mockLegacyPlatformApi(); + const proxy = mockProxy(); + // Non-empty stdout/stderr on the `docker run` step exercises both the + // text-mode stdout routing branch and the always-to-stderr container + // stderr branch in `downloadWithDockerUnbundle`. + const child = mockDockerUnbundle({ + runStdout: ["unbundle: wrote index.ts"], + runStderr: ["unbundle: warning about deno.json"], + }); + const layer = Layer.mergeAll( + buildLegacyTestRuntime({ + out, + api, + cliConfig: mockLegacyCliConfig({ workdir: tempRoot.current }), + }), + proxy.layer, + child.layer, + Stdio.layerTest({ + args: Effect.succeed([ + "functions", + "download", + "hello-world", + "--project-ref", + PROJECT_ID, + ]), + }), + ); - return Effect.gen(function* () { - // `useDocker: true` mirrors what the CLI parser now resolves to by - // default (CLI-1862) — no `--use-docker` flag appears in argv above. - yield* legacyFunctionsDownload({ ...baseFlags, useDocker: true }); + return Effect.gen(function* () { + // `useDocker: true` mirrors what the CLI parser now resolves to by + // default (CLI-1862) — no `--use-docker` flag appears in argv above. + // CLI-1963: this now runs the native Docker-unbundle path instead of + // delegating to the Go proxy. + yield* legacyFunctionsDownload({ ...baseFlags, useDocker: true }); - expect(api.requests).toEqual([]); - expect(proxy.calls).toEqual([ - [ - "functions", - "download", - "hello-world", - "--project-ref", - "abcdefghijklmnopqrst", - "--use-docker", - ], - ]); - // The delegated Go binary must not also fire its own - // `cli_command_executed` on top of this command's own instrumentation. - expect(proxy.envs).toEqual([{ SUPABASE_TELEMETRY_DISABLED: "1" }]); - }).pipe(Effect.provide(layer)); - }); + expect(proxy.calls).toEqual([]); + expect(proxy.captureCalls).toEqual([]); + expect(api.requests.some((request) => request.url.endsWith("/hello-world/body"))).toBe( + true, + ); + expect( + child.spawned.some( + (spawned) => spawned.command === "docker" && spawned.args[0] === "run", + ), + ).toBe(true); + expect(out.stderrText).toContain("Downloading function: hello-world\n"); + expect(out.stdoutText).toContain("unbundle: wrote index.ts\n"); + expect(out.stderrText).toContain("unbundle: warning about deno.json\n"); + // Go parity finding (CLI-1963 audit): unlike the server-side and + // `--legacy-bundle` paths, `downloadWithDockerUnbundle` never prints + // a "Downloaded Function ... from project ..." success line — + // guarded here against a future accidental regression. + expect(out.stderrText).not.toContain("Downloaded Function"); + // No `--debug` — the temp eszip file is removed after the run. + expect( + existsSync(join(tempRoot.current, "supabase", ".temp", "output_hello-world.eszip")), + ).toBe(false); + }).pipe(Effect.provide(layer)); + }, + ); it.live( "does not treat the --use-docker default as conflicting with an explicit --use-api", @@ -251,10 +370,124 @@ describe("legacy functions download", () => { }, ); - it.live("still proxies to Docker when --use-api=false is passed explicitly", () => { - const out = mockOutput({ format: "text" }); - const api = mockLegacyPlatformApi(); + it.live( + "still runs the native Docker unbundle path when --use-api=false is passed explicitly", + () => { + const out = mockOutput({ format: "text" }); + const api = mockLegacyPlatformApi(); + const proxy = mockProxy(); + const child = mockChildProcessSpawner({ exitCode: 0 }); + const layer = Layer.mergeAll( + buildLegacyTestRuntime({ + out, + api, + cliConfig: mockLegacyCliConfig({ workdir: tempRoot.current }), + }), + proxy.layer, + child.layer, + Stdio.layerTest({ + args: Effect.succeed([ + "functions", + "download", + "hello-world", + "--use-api=false", + "--project-ref", + PROJECT_ID, + ]), + }), + ); + + return Effect.gen(function* () { + // Go's override is value-based (`if useApi { useDocker = false }`, + // apps/cli-go/cmd/functions.go:51-53), not presence-based. An + // explicit `--use-api=false` must not be treated like `--use-api` — + // it should leave the `--use-docker` default (true) in effect and + // still run the native Docker path (CLI-1963). + yield* legacyFunctionsDownload({ ...baseFlags, useApi: false, useDocker: true }); + + expect(proxy.calls).toEqual([]); + expect(proxy.captureCalls).toEqual([]); + expect( + child.spawned.some( + (spawned) => spawned.command === "docker" && spawned.args[0] === "run", + ), + ).toBe(true); + }).pipe(Effect.provide(layer)); + }, + ); + + it.live( + "emits a JSON success envelope when running the native Docker path in machine-output mode", + () => { + const out = mockOutput({ format: "json" }); + const api = mockLegacyPlatformApi(); + const proxy = mockProxy(); + // Non-empty container stdout exercises the machine-mode branch that + // routes it to stderr instead of stdout (CLI-1546: stdout stays + // payload-only in json/stream-json modes). + const child = mockDockerUnbundle({ runStdout: ["unbundle: wrote index.ts"] }); + const layer = Layer.mergeAll( + buildLegacyTestRuntime({ + out, + api, + cliConfig: mockLegacyCliConfig({ workdir: tempRoot.current }), + }), + proxy.layer, + child.layer, + Stdio.layerTest({ + args: Effect.succeed([ + "functions", + "download", + "hello-world", + "--project-ref", + PROJECT_ID, + "--output-format", + "json", + ]), + }), + ); + + return Effect.gen(function* () { + // CLI-1963: `--use-docker` now runs the native Docker-unbundle path; + // this asserts the JSON envelope this command emits itself still + // shows up correctly, with no delegated Go child's stdout to worry + // about capturing. + yield* legacyFunctionsDownload({ ...baseFlags, useDocker: true }); + + expect(proxy.calls).toEqual([]); + expect(proxy.captureCalls).toEqual([]); + expect( + child.spawned.some( + (spawned) => spawned.command === "docker" && spawned.args[0] === "run", + ), + ).toBe(true); + expect(out.stdoutText).toBe(""); + expect(out.stderrText).toContain("unbundle: wrote index.ts\n"); + expect(out.messages).toContainEqual( + expect.objectContaining({ + type: "success", + data: { function_slugs: ["hello-world"], project_ref: PROJECT_ID }, + }), + ); + }).pipe(Effect.provide(layer)); + }, + ); + + it.live("lists remote functions and downloads each natively via Docker in machine mode", () => { + const out = mockOutput({ format: "json" }); + const api = mockLegacyPlatformApi({ + handler: (request) => + request.url.endsWith("/functions") + ? Effect.succeed( + legacyJsonResponse(request, 200, [ + { slug: "hello-world" }, + { slug: "goodbye-world" }, + ]), + ) + : Effect.succeed(legacyJsonResponse(request, 200, {})), + }); const proxy = mockProxy(); + const child = mockChildProcessSpawner({ exitCode: 0 }); const layer = Layer.mergeAll( buildLegacyTestRuntime({ out, @@ -262,44 +495,128 @@ describe("legacy functions download", () => { cliConfig: mockLegacyCliConfig({ workdir: tempRoot.current }), }), proxy.layer, + child.layer, Stdio.layerTest({ args: Effect.succeed([ "functions", "download", - "hello-world", - "--use-api=false", "--project-ref", - "abcdefghijklmnopqrst", + PROJECT_ID, + "--output-format", + "json", ]), }), ); return Effect.gen(function* () { - // Go's override is value-based (`if useApi { useDocker = false }`, - // apps/cli-go/cmd/functions.go:51-53), not presence-based. An explicit - // `--use-api=false` must not be treated like `--use-api` — it should - // leave the `--use-docker` default (true) in effect and still proxy. - yield* legacyFunctionsDownload({ ...baseFlags, useApi: false, useDocker: true }); + yield* legacyFunctionsDownload({ + ...baseFlags, + functionName: Option.none(), + useDocker: true, + }); - expect(api.requests).toEqual([]); - expect(proxy.calls).toEqual([ - [ + expect(proxy.calls).toEqual([]); + expect(proxy.captureCalls).toEqual([]); + expect( + child.spawned.filter( + (spawned) => spawned.command === "docker" && spawned.args[0] === "run", + ), + ).toHaveLength(2); + expect(out.messages).toContainEqual( + expect.objectContaining({ + type: "success", + data: { + function_slugs: ["hello-world", "goodbye-world"], + project_ref: PROJECT_ID, + }, + }), + ); + }).pipe(Effect.provide(layer)); + }); + + it.live("runs docker with the expected binds, network, and unbundle command", () => { + const out = mockOutput({ format: "text" }); + const api = mockLegacyPlatformApi(); + const proxy = mockProxy(); + const child = mockChildProcessSpawner({ exitCode: 0 }); + const layer = Layer.mergeAll( + buildLegacyTestRuntime({ + out, + api, + cliConfig: mockLegacyCliConfig({ workdir: tempRoot.current }), + }), + proxy.layer, + child.layer, + Stdio.layerTest({ + args: Effect.succeed([ "functions", "download", "hello-world", - "--project-ref", - "abcdefghijklmnopqrst", "--use-docker", + "--project-ref", + PROJECT_ID, + ]), + }), + ); + + return Effect.gen(function* () { + yield* legacyFunctionsDownload({ ...baseFlags, useDocker: true }); + + // Go: `extractOne` (`download.go:260-266`) — bind order and network + // reuse the same primitives `deploy.ts`'s own Docker-bundling path + // already uses. + expect(child.spawned.find((spawned) => spawned.args[0] === "network")).toEqual({ + command: "docker", + args: ["network", "inspect", `supabase_network_${PROJECT_ID}`], + }); + expect(child.spawned.find((spawned) => spawned.args[0] === "volume")).toEqual({ + command: "docker", + args: [ + "volume", + "create", + "--label", + `com.supabase.cli.project=${PROJECT_ID}`, + "--label", + `com.docker.compose.project=${PROJECT_ID}`, + `supabase_edge_runtime_${PROJECT_ID}`, ], + }); + + const runCommand = child.spawned.find((spawned) => spawned.args[0] === "run"); + const hostEszipPath = resolve( + tempRoot.current, + "supabase", + ".temp", + "output_hello-world.eszip", + ); + const functionsDir = resolve(tempRoot.current, "supabase", "functions"); + expect(runCommand?.args).toContain( + `supabase_edge_runtime_${PROJECT_ID}:/root/.cache/deno:rw`, + ); + expect(runCommand?.args).toContain( + `${hostEszipPath}:/root/eszips/output_hello-world.eszip:ro`, + ); + expect(runCommand?.args).toContain(`${functionsDir}:/home/deno:rw`); + expect(runCommand?.args).toContain("--network"); + expect(runCommand?.args).toContain(`supabase_network_${PROJECT_ID}`); + // The unbundle tail is always the LAST 6 args regardless of whether + // `--add-host` (Linux-only) was inserted before it. + expect(runCommand?.args.slice(-6)).toEqual([ + `public.ecr.aws/supabase/edge-runtime:v${DEFAULT_VERSIONS["edge-runtime"]}`, + "unbundle", + "--eszip", + "/root/eszips/output_hello-world.eszip", + "--output", + "/home/deno/hello-world", ]); - expect(proxy.envs).toEqual([{ SUPABASE_TELEMETRY_DISABLED: "1" }]); }).pipe(Effect.provide(layer)); }); - it.live("emits a JSON success envelope when proxying to Docker in machine-output mode", () => { - const out = mockOutput({ format: "json" }); + it.live("uses an explicit --network-id override instead of the derived network name", () => { + const out = mockOutput({ format: "text" }); const api = mockLegacyPlatformApi(); const proxy = mockProxy(); + const child = mockChildProcessSpawner({ exitCode: 0 }); const layer = Layer.mergeAll( buildLegacyTestRuntime({ out, @@ -307,63 +624,282 @@ describe("legacy functions download", () => { cliConfig: mockLegacyCliConfig({ workdir: tempRoot.current }), }), proxy.layer, + child.layer, Stdio.layerTest({ args: Effect.succeed([ "functions", "download", "hello-world", + "--use-docker", "--project-ref", - "abcdefghijklmnopqrst", - "--output-format", - "json", + PROJECT_ID, + "--network-id", + "custom-network", ]), }), ); return Effect.gen(function* () { - // CLI-1546: stdout is payload-only in machine mode, so the Go child's - // raw output must be captured/discarded (not inherited) and this - // command must emit the `Output` envelope itself, matching the native - // path's shape. + // `--network-id` is a persistent root flag (`cmd/root.go:328`), not + // registered on `functions download` itself — `explicitStringFlag` + // scans the whole argv unscoped. yield* legacyFunctionsDownload({ ...baseFlags, useDocker: true }); - expect(proxy.calls).toEqual([]); - expect(proxy.captureCalls).toEqual([ - [ + expect(child.spawned.find((spawned) => spawned.args[0] === "network")).toEqual({ + command: "docker", + args: ["network", "inspect", "custom-network"], + }); + const runCommand = child.spawned.find((spawned) => spawned.args[0] === "run"); + expect(runCommand?.args).toContain("custom-network"); + expect(runCommand?.args).not.toContain(`supabase_network_${PROJECT_ID}`); + }).pipe(Effect.provide(layer)); + }); + + it.live("keeps the temporary eszip file when --debug is passed", () => { + const out = mockOutput({ format: "text" }); + const api = mockLegacyPlatformApi(); + const proxy = mockProxy(); + const child = mockChildProcessSpawner({ exitCode: 0 }); + const layer = Layer.mergeAll( + buildLegacyTestRuntime({ + out, + api, + cliConfig: mockLegacyCliConfig({ workdir: tempRoot.current }), + }), + proxy.layer, + child.layer, + Stdio.layerTest({ + args: Effect.succeed([ "functions", "download", "hello-world", - "--project-ref", - "abcdefghijklmnopqrst", "--use-docker", - ], - ]); - expect(proxy.captureEnvs).toEqual([{ SUPABASE_TELEMETRY_DISABLED: "1" }]); - expect(out.messages).toContainEqual( - expect.objectContaining({ - type: "success", - data: { function_slugs: ["hello-world"], project_ref: "abcdefghijklmnopqrst" }, + "--project-ref", + PROJECT_ID, + "--debug", + ]), + }), + ); + + return Effect.gen(function* () { + yield* legacyFunctionsDownload({ ...baseFlags, useDocker: true }); + + expect( + existsSync(join(tempRoot.current, "supabase", ".temp", "output_hello-world.eszip")), + ).toBe(true); + }).pipe(Effect.provide(layer)); + }); + + describe("docker unbundle container failures", () => { + it.live("fails with the legacy-bundle suggestion when the container exits non-zero", () => { + const out = mockOutput({ format: "text" }); + const api = mockLegacyPlatformApi(); + const proxy = mockProxy(); + const child = mockDockerUnbundle({ runExitCode: 1, runStderr: ["boom"] }); + const layer = Layer.mergeAll( + buildLegacyTestRuntime({ + out, + api, + cliConfig: mockLegacyCliConfig({ workdir: tempRoot.current }), }), + proxy.layer, + child.layer, + Stdio.layerTest({ + args: Effect.succeed([ + "functions", + "download", + "hello-world", + "--use-docker", + "--project-ref", + PROJECT_ID, + ]), + }), + ); + + return Effect.gen(function* () { + const error = yield* legacyFunctionsDownload({ ...baseFlags, useDocker: true }).pipe( + Effect.flip, + ); + + expect(error).toBeInstanceOf(Error); + expect((error as Error).message).toBe("error running container: exit 1"); + expect((error as Error & { suggestion?: string }).suggestion).toBe( + "\nIf your function is deployed using CLI < 1.120.0, trying running supabase functions download --legacy-bundle hello-world instead.", + ); + }).pipe(Effect.provide(layer)); + }); + + it.live( + "prepends the deno v2 suggestion when deno_version is 1 and the container reports an invalid eszip", + () => { + const out = mockOutput({ format: "text" }); + const api = mockLegacyPlatformApi(); + const proxy = mockProxy(); + const child = mockDockerUnbundle({ + runExitCode: 1, + // Go's scanner requires a full-line, case-insensitive match + // (`strings.EqualFold(line, "invalid eszip v2")`, `download.go:295`) + // — a line merely containing the phrase as a substring (e.g. + // "error: invalid eszip v2 header") does not fire the suggestion. + runStderr: ["invalid eszip v2"], + }); + const layer = Layer.mergeAll( + buildLegacyTestRuntime({ + out, + api, + cliConfig: mockLegacyCliConfig({ workdir: tempRoot.current }), + }), + proxy.layer, + child.layer, + Stdio.layerTest({ + args: Effect.succeed([ + "functions", + "download", + "hello-world", + "--use-docker", + "--project-ref", + PROJECT_ID, + ]), + }), + ); + + return Effect.gen(function* () { + yield* Effect.tryPromise(() => + mkdir(join(tempRoot.current, "supabase"), { recursive: true }), + ); + yield* Effect.tryPromise(() => + writeFile( + join(tempRoot.current, "supabase", "config.toml"), + ["[edge_runtime]", "deno_version = 1", ""].join("\n"), + ), + ); + + const error = yield* legacyFunctionsDownload({ ...baseFlags, useDocker: true }).pipe( + Effect.flip, + ); + + expect(error).toBeInstanceOf(Error); + expect((error as Error).message).toBe("error running container: exit 1"); + expect((error as Error & { suggestion?: string }).suggestion).toBe( + "Please use deno v2 in supabase/config.toml to download this Function:\n\n[edge_runtime]\ndeno_version = 2\n" + + "\nIf your function is deployed using CLI < 1.120.0, trying running supabase functions download --legacy-bundle hello-world instead.", + ); + }).pipe(Effect.provide(layer)); + }, + ); + + it.live( + "does not prepend the deno v2 suggestion when deno_version is 1 but the container's error is unrelated", + () => { + const out = mockOutput({ format: "text" }); + const api = mockLegacyPlatformApi(); + const proxy = mockProxy(); + const child = mockDockerUnbundle({ runExitCode: 1, runStderr: ["permission denied"] }); + const layer = Layer.mergeAll( + buildLegacyTestRuntime({ + out, + api, + cliConfig: mockLegacyCliConfig({ workdir: tempRoot.current }), + }), + proxy.layer, + child.layer, + Stdio.layerTest({ + args: Effect.succeed([ + "functions", + "download", + "hello-world", + "--use-docker", + "--project-ref", + PROJECT_ID, + ]), + }), + ); + + return Effect.gen(function* () { + yield* Effect.tryPromise(() => + mkdir(join(tempRoot.current, "supabase"), { recursive: true }), + ); + yield* Effect.tryPromise(() => + writeFile( + join(tempRoot.current, "supabase", "config.toml"), + ["[edge_runtime]", "deno_version = 1", ""].join("\n"), + ), + ); + + const error = yield* legacyFunctionsDownload({ ...baseFlags, useDocker: true }).pipe( + Effect.flip, + ); + + expect((error as Error & { suggestion?: string }).suggestion).toBe( + "\nIf your function is deployed using CLI < 1.120.0, trying running supabase functions download --legacy-bundle hello-world instead.", + ); + }).pipe(Effect.provide(layer)); + }, + ); + }); + + it.live("fails when ensureDockerNetwork can't create a missing network", () => { + const out = mockOutput({ format: "text" }); + const api = mockLegacyPlatformApi(); + const proxy = mockProxy(); + const spawnerOpts: { + exitCode?: number; + stderr?: string[]; + onSpawn?: (record: { command: string; args: ReadonlyArray }) => void; + } = { exitCode: 0 }; + spawnerOpts.onSpawn = (record) => { + spawnerOpts.exitCode = record.command === "docker" && record.args[0] === "network" ? 1 : 0; + spawnerOpts.stderr = ["permission denied"]; + }; + const child = mockChildProcessSpawner(spawnerOpts); + const layer = Layer.mergeAll( + buildLegacyTestRuntime({ + out, + api, + cliConfig: mockLegacyCliConfig({ workdir: tempRoot.current }), + }), + proxy.layer, + child.layer, + Stdio.layerTest({ + args: Effect.succeed([ + "functions", + "download", + "hello-world", + "--use-docker", + "--project-ref", + PROJECT_ID, + ]), + }), + ); + + return Effect.gen(function* () { + const error = yield* legacyFunctionsDownload({ ...baseFlags, useDocker: true }).pipe( + Effect.flip, ); + + expect(error).toBeInstanceOf(Error); + expect((error as Error).message).toBe( + `failed to create docker network: supabase_network_${PROJECT_ID}`, + ); + expect(child.spawned.some((spawned) => spawned.args[0] === "volume")).toBe(false); + expect(child.spawned.some((spawned) => spawned.args[0] === "run")).toBe(false); + // Go parity fix (CLI-1963 review): `Effect.ensuring` wraps the whole + // Docker-extraction sequence, so the temp eszip written just before it + // is still cleaned up even though the failure happened before Docker + // ever ran — not only after a successful `runChildProcess` call. + expect( + existsSync(join(tempRoot.current, "supabase", ".temp", "output_hello-world.eszip")), + ).toBe(false); }).pipe(Effect.provide(layer)); }); it.live( - "lists remote functions before delegating when no function name is given in machine mode", + "fails with the docker-step prefix when the unbundle container itself cannot be spawned", () => { - const out = mockOutput({ format: "json" }); - const api = mockLegacyPlatformApi({ - handler: (request) => - request.url.endsWith("/functions") - ? Effect.succeed( - legacyJsonResponse(request, 200, [ - { slug: "hello-world" }, - { slug: "goodbye-world" }, - ]), - ) - : Effect.succeed(legacyJsonResponse(request, 200, {})), - }); + const out = mockOutput({ format: "text" }); + const api = mockLegacyPlatformApi(); const proxy = mockProxy(); + const child = mockDockerRunSpawnFailure(); const layer = Layer.mergeAll( buildLegacyTestRuntime({ out, @@ -371,38 +907,40 @@ describe("legacy functions download", () => { cliConfig: mockLegacyCliConfig({ workdir: tempRoot.current }), }), proxy.layer, + child.layer, Stdio.layerTest({ args: Effect.succeed([ "functions", "download", + "hello-world", + "--use-docker", "--project-ref", - "abcdefghijklmnopqrst", - "--output-format", - "json", + PROJECT_ID, ]), }), ); return Effect.gen(function* () { - yield* legacyFunctionsDownload({ - ...baseFlags, - functionName: Option.none(), - useDocker: true, - }); + const error = yield* legacyFunctionsDownload({ ...baseFlags, useDocker: true }).pipe( + Effect.flip, + ); - expect(proxy.calls).toEqual([]); - expect(proxy.captureCalls).toEqual([ - ["functions", "download", "--project-ref", "abcdefghijklmnopqrst", "--use-docker"], - ]); - expect(out.messages).toContainEqual( - expect.objectContaining({ - type: "success", - data: { - function_slugs: ["hello-world", "goodbye-world"], - project_ref: "abcdefghijklmnopqrst", - }, - }), + // Distinct from `ensureDockerNetwork`/`ensureDockerNamedVolume` + // failures (asserted above), which already self-describe and must + // NOT gain this prefix — a bare spawn/runtime-not-found failure from + // `runChildProcess` itself carries no context of its own about which + // command was running, so `withDockerStepFailure` adds one. + expect(error).toBeInstanceOf(Error); + expect((error as Error).message).toBe( + `failed to run the edge-runtime unbundle container: ${legacyContainerRuntimeNotFoundMessage}`, ); + expect((error as Error & { suggestion?: string }).suggestion).toBe( + "\nIf your function is deployed using CLI < 1.120.0, trying running supabase functions download --legacy-bundle hello-world instead.", + ); + expect(child.spawned.some((spawned) => spawned.args[0] === "run")).toBe(true); + expect( + existsSync(join(tempRoot.current, "supabase", ".temp", "output_hello-world.eszip")), + ).toBe(false); }).pipe(Effect.provide(layer)); }, ); @@ -418,6 +956,11 @@ describe("legacy functions download", () => { : Effect.succeed(legacyJsonResponse(request, 200, {})), }); const proxy = mockProxy(); + // Deterministic stand-in for `emptyEnv()`'s real `ChildProcessSpawner` + // (via `BunServices`, pulled in by `buildLegacyTestRuntime`) — `useDocker: + // true` still probes `docker info` even though this project has no + // functions to download, so this must not spawn a real `docker` process. + const child = mockChildProcessSpawner({ exitCode: 0 }); const layer = Layer.mergeAll( buildLegacyTestRuntime({ out, @@ -425,6 +968,7 @@ describe("legacy functions download", () => { cliConfig: mockLegacyCliConfig({ workdir: tempRoot.current }), }), proxy.layer, + child.layer, Stdio.layerTest({ args: Effect.succeed([ "functions", @@ -470,6 +1014,7 @@ describe("legacy functions download", () => { : Effect.succeed(legacyJsonResponse(request, 200, {})), }); const proxy = mockProxy(); + const child = mockChildProcessSpawner({ exitCode: 0 }); const layer = Layer.mergeAll( buildLegacyTestRuntime({ out, @@ -477,6 +1022,7 @@ describe("legacy functions download", () => { cliConfig: mockLegacyCliConfig({ workdir: tempRoot.current }), }), proxy.layer, + child.layer, Stdio.layerTest({ args: Effect.succeed([ "functions", diff --git a/apps/cli/src/legacy/commands/functions/serve/serve.integration.test.ts b/apps/cli/src/legacy/commands/functions/serve/serve.integration.test.ts index 8391ab53d7..a0c9dcd02e 100644 --- a/apps/cli/src/legacy/commands/functions/serve/serve.integration.test.ts +++ b/apps/cli/src/legacy/commands/functions/serve/serve.integration.test.ts @@ -14,7 +14,7 @@ import { mockLegacyTelemetryStateTracked, useLegacyTempWorkdir, } from "../../../../../tests/helpers/legacy-mocks.ts"; -import { toDockerPath } from "../../../../shared/functions/deploy.ts"; +import { toDockerPath } from "../../../../shared/functions/functions-docker.ts"; import { mockOutput, mockProcessControl, @@ -72,10 +72,10 @@ const deployMockState = vi.hoisted(() => ({ }, })); -vi.mock("../../../../shared/functions/deploy.ts", async () => { - const actual = await vi.importActual( - "../../../../shared/functions/deploy.ts", - ); +vi.mock("../../../../shared/functions/functions-docker.ts", async () => { + const actual = await vi.importActual< + typeof import("../../../../shared/functions/functions-docker.ts") + >("../../../../shared/functions/functions-docker.ts"); const { Effect } = await import("effect"); return { diff --git a/apps/cli/src/legacy/commands/start/lib/container-lifecycle.ts b/apps/cli/src/legacy/commands/start/lib/container-lifecycle.ts index 96d39ce92b..c5cb7ee66d 100644 --- a/apps/cli/src/legacy/commands/start/lib/container-lifecycle.ts +++ b/apps/cli/src/legacy/commands/start/lib/container-lifecycle.ts @@ -30,7 +30,7 @@ import { LEGACY_CLI_PROJECT_LABEL, LEGACY_CLI_WORKDIR_LABEL, } from "../../../shared/legacy-docker-ids.ts"; -import { isUserDefinedDockerNetwork } from "../../../../shared/functions/deploy.ts"; +import { isUserDefinedDockerNetwork } from "../../../../shared/functions/functions-docker.ts"; import { legacyBuildStartContainerCreateArgs, legacyApplyBitbucketStartContainerFilter, @@ -54,9 +54,10 @@ type Spawner = ChildProcessSpawner["Service"]; * otherwise silently stop recognizing the local stack's containers. * * A same-value private constant already exists at - * `shared/functions/deploy.ts` (`dockerComposeProjectLabel`, for the unrelated - * `functions deploy` Docker Desktop extension gateway) but is neither exported - * nor in the same Docker-usage domain as `start` — not hoisted from there. + * `shared/functions/functions-docker.ts` (`dockerComposeProjectLabel`, for the + * unrelated `functions deploy`/`functions serve` Docker Desktop extension + * gateway) but is neither exported nor in the same Docker-usage domain as + * `start` — not hoisted from there. */ export const LEGACY_COMPOSE_PROJECT_LABEL = "com.docker.compose.project"; @@ -229,8 +230,9 @@ function legacyPortConflictSuggestion(hostPort: string, serviceLabel: string): s * created (`docker network create host` errors with "operation is not * permitted on predefined host network"), so this returns immediately without * spawning `docker network create` at all for those names, reusing the same - * `isUserDefinedDockerNetwork` check `shared/functions/deploy.ts` already - * applies for the unrelated `functions deploy` extension-gateway network. + * `isUserDefinedDockerNetwork` check `shared/functions/functions-docker.ts` + * already applies for the unrelated `functions deploy`/`functions serve` + * extension-gateway network. */ export function legacyEnsureStartNetwork( spawner: Spawner, diff --git a/apps/cli/src/next/commands/functions/deploy/deploy.handler.ts b/apps/cli/src/next/commands/functions/deploy/deploy.handler.ts index acd17b7a1b..8d31bfaaf6 100644 --- a/apps/cli/src/next/commands/functions/deploy/deploy.handler.ts +++ b/apps/cli/src/next/commands/functions/deploy/deploy.handler.ts @@ -1,12 +1,10 @@ -import { DEFAULT_VERSIONS } from "@supabase/stack/effect"; -import { readFile } from "node:fs/promises"; -import { join } from "node:path"; import { Effect, Stdio } from "effect"; import { CliConfig } from "../../../config/cli-config.service.ts"; import { PlatformApi } from "../../../auth/platform-api.service.ts"; import { ProjectHome } from "../../../config/project-home.service.ts"; import { RuntimeInfo } from "../../../../shared/runtime/runtime-info.service.ts"; import { deployFunctions } from "../../../../shared/functions/deploy.ts"; +import { resolveEdgeRuntimeVersionPin } from "../../../../shared/functions/functions.shared.ts"; import { resolveProjectRef } from "../functions.shared.ts"; import type { FunctionsDeployFlags } from "./deploy.command.ts"; @@ -19,13 +17,7 @@ export const functionsDeploy = Effect.fn("functions.deploy")(function* ( const runtimeInfo = yield* RuntimeInfo; const stdio = yield* Stdio.Stdio; const rawArgs = yield* stdio.args; - const edgeRuntimeVersion = yield* Effect.tryPromise(() => - readFile(join(projectHome.supabaseDir, ".temp", "edge-runtime-version"), "utf8"), - ).pipe( - Effect.map((version) => version.trim()), - Effect.catch(() => Effect.succeed("")), - Effect.map((version) => version || DEFAULT_VERSIONS["edge-runtime"]), - ); + const edgeRuntimeVersion = yield* resolveEdgeRuntimeVersionPin(projectHome.supabaseDir); yield* deployFunctions(flags, { api, diff --git a/apps/cli/src/next/commands/functions/download/download.command.ts b/apps/cli/src/next/commands/functions/download/download.command.ts index 4432db7c0b..aecd8adc61 100644 --- a/apps/cli/src/next/commands/functions/download/download.command.ts +++ b/apps/cli/src/next/commands/functions/download/download.command.ts @@ -25,6 +25,7 @@ const config = { ), useDocker: Flag.boolean("use-docker").pipe( Flag.withDescription("Use Docker to unbundle functions client-side."), + Flag.withDefault(true), Flag.withHidden, ), legacyBundle: Flag.boolean("legacy-bundle").pipe( diff --git a/apps/cli/src/next/commands/functions/download/download.handler.ts b/apps/cli/src/next/commands/functions/download/download.handler.ts index 15c6238e84..bb63eaf913 100644 --- a/apps/cli/src/next/commands/functions/download/download.handler.ts +++ b/apps/cli/src/next/commands/functions/download/download.handler.ts @@ -3,8 +3,9 @@ import { PlatformApi } from "../../../auth/platform-api.service.ts"; import { ProjectHome } from "../../../config/project-home.service.ts"; import { downloadFunctions, - makeGoProxyDownloadArgs, + makeGoProxyLegacyBundleArgs, } from "../../../../shared/functions/download.ts"; +import { resolveEdgeRuntimeVersionPin } from "../../../../shared/functions/functions.shared.ts"; import { LegacyGoProxy } from "../../../../shared/legacy/go-proxy.service.ts"; import { resolveProjectRef } from "../functions.shared.ts"; import type { FunctionsDownloadFlags } from "./download.command.ts"; @@ -15,17 +16,20 @@ export const functionsDownload = Effect.fnUntraced(function* (flags: FunctionsDo const proxy = yield* LegacyGoProxy; const stdio = yield* Stdio.Stdio; const rawArgs = yield* stdio.args; + const edgeRuntimeVersion = yield* resolveEdgeRuntimeVersionPin(projectHome.supabaseDir); yield* downloadFunctions(flags, { api, projectRoot: projectHome.projectRoot, rawArgs, + goViperCompat: false, + edgeRuntimeVersion, resolveProjectRef, // In machine-output mode the child's stdout is captured and discarded // instead of inherited (CLI-1546: stdout is payload-only in machine // mode) — `downloadFunctions` emits the `Output` envelope itself. proxyDownload: (proxyFlags, projectRef, captureOutput) => { - const args = makeGoProxyDownloadArgs(proxyFlags, projectRef); + const args = makeGoProxyLegacyBundleArgs(proxyFlags.functionName, projectRef); const cwd = projectHome.projectRoot; return captureOutput ? Effect.asVoid(proxy.execCapture(args, { cwd, stdin: "ignore" })) diff --git a/apps/cli/src/next/commands/functions/download/download.integration.test.ts b/apps/cli/src/next/commands/functions/download/download.integration.test.ts index 9c1fa1a75d..72f23a0413 100644 --- a/apps/cli/src/next/commands/functions/download/download.integration.test.ts +++ b/apps/cli/src/next/commands/functions/download/download.integration.test.ts @@ -23,6 +23,7 @@ import { mockProjectLinkState, mockRuntimeInfo, } from "../../../../../tests/helpers/mocks.ts"; +import { mockChildProcessSpawner } from "../../../../../../../packages/process-compose/tests/helpers/mocks.ts"; import type { FunctionsDownloadFlags } from "./download.command.ts"; import { ConflictingFunctionDownloadFlagsError, @@ -30,6 +31,7 @@ import { InvalidFunctionSlugError, UnsafeFunctionDownloadPathError, } from "../../../../shared/functions/download.errors.ts"; +import { invalidFunctionSlugDetail } from "../../../../shared/functions/functions.shared.ts"; import { functionsDownload } from "./download.handler.ts"; const PROJECT_REF = "abcdefghijklmnopqrst"; @@ -74,6 +76,7 @@ function textResponse( status: number, body: ResponseBody = "", contentType = "text/plain", + extraHeaders: Readonly> = {}, ): HttpClientResponse.HttpClientResponse { return HttpClientResponse.fromWeb( request, @@ -81,6 +84,7 @@ function textResponse( status, headers: { "content-type": contentType, + ...extraHeaders, }, }), ); @@ -182,7 +186,15 @@ function mockDownloadApi(opts: { functionStatusBySlug?: Readonly>; functionBodyBySlug?: Readonly>; bodyBySlug?: Readonly< - Record + Record< + string, + { + status?: number; + body: ResponseBody; + contentType: string; + headers?: Readonly>; + } + > >; bodyErrorBySlug?: Readonly>; }) { @@ -233,6 +245,7 @@ function mockDownloadApi(opts: { response?.status ?? 200, response?.body ?? "", response?.contentType ?? "multipart/form-data; boundary=missing", + response?.headers ?? {}, ), ); } @@ -277,6 +290,7 @@ function setup( linked?: boolean; projectRoot?: string; rawArgs?: ReadonlyArray; + childLayer?: ReturnType["layer"]; } = {}, ) { const out = mockOutput({ format: opts.format ?? "text", interactive: false }); @@ -293,6 +307,10 @@ function setup( Stdio.layerTest({ args: Effect.succeed(opts.rawArgs ?? ["functions", "download"]), }), + // Overrides `emptyEnv()`'s real `ChildProcessSpawner` (via `BunServices`) + // so `--use-docker`'s now-default-true native path never spawns a real + // `docker` process — CLI-1963. + opts.childLayer ?? mockChildProcessSpawner({ exitCode: 0 }).layer, ); return { out, api, layer, proxy }; @@ -709,43 +727,40 @@ describe("functions download", () => { ); }); - it.live("downloads remote slugs from download-all without local slug validation", () => { + it.live("rejects a malicious remote slug from download-all before any per-slug work", () => { const tempDir = makeTempDir(); - const multipart = multipartBody([ - { - headers: { - "Content-Disposition": 'form-data; name="metadata"', - "Content-Type": "application/json", - }, - body: JSON.stringify({ deno2_entrypoint_path: "source/index.ts" }), - }, - { - headers: { - "Content-Disposition": 'form-data; name="file"; filename="source/index.ts"', - }, - body: "console.log('remote')", - }, - ]); + // Mirrors Go's own `TestDownloadAllRejectsMaliciousSlug` regression test + // (`apps/cli-go/internal/functions/download/download_test.go`) — a + // path-traversal-shaped slug returned by the (untrusted) list endpoint. + const maliciousSlug = "../../../../../poc-escaped-outside-project"; return Effect.gen(function* () { yield* Effect.tryPromise(() => writeProjectConfig(tempDir)); - const { layer } = setup(tempDir, { - list: [makeFunction({ slug: "1remote" })], - bodyBySlug: { - "1remote": multipart, - }, + const { api, layer } = setup(tempDir, { + list: [makeFunction({ slug: maliciousSlug })], }); - yield* functionsDownload({ + // CLI-1891 (Go parity): every slug sourced from the Management API's + // function list must be validated before any per-slug network or + // filesystem work — not just user-supplied CLI arguments. + const error = yield* functionsDownload({ ...BASE_FLAGS, functionName: Option.none(), - }).pipe(Effect.provide(layer)); + }).pipe(Effect.provide(layer), Effect.flip); - expect( - yield* Effect.tryPromise(() => - readFile(join(tempDir, "supabase", "functions", "1remote", "index.ts"), "utf8"), - ), - ).toBe("console.log('remote')"); + expect(error).toBeInstanceOf(Error); + expect((error as Error).message).toBe( + `failed to download function ${maliciousSlug}: ${invalidFunctionSlugDetail}`, + ); + expect((error as Error & { suggestion?: string }).suggestion).toBe( + `The Supabase API returned an unexpected function slug (${maliciousSlug}). Retry the command, and if this keeps happening, verify your network connection is not being intercepted before contacting Supabase support.`, + ); + // Only the list call happened — no GET to the malicious slug's own + // body/metadata endpoints, and nothing was written to disk. + expect(api.requests).toEqual([ + `https://api.supabase.com/v1/projects/${PROJECT_REF}/functions`, + ]); + expect(existsSync(join(tempDir, "supabase", "functions"))).toBe(false); }).pipe( Effect.ensuring(Effect.tryPromise(() => rm(tempDir, { recursive: true, force: true }))), ); @@ -851,56 +866,125 @@ describe("functions download", () => { ); }); - it.live("delegates --use-docker with the linked project ref to the Go proxy", () => { + it.live( + "runs the native Docker unbundle path for --use-docker with the linked project ref", + () => { + const tempDir = makeTempDir(); + const child = mockChildProcessSpawner({ exitCode: 0 }); + + return Effect.gen(function* () { + yield* Effect.tryPromise(() => writeProjectConfig(tempDir)); + const { out, layer, proxy } = setup(tempDir, { + bodyBySlug: { + "hello-world": { body: "fake-eszip-bytes", contentType: "application/octet-stream" }, + }, + rawArgs: ["functions", "download", "hello-world", "--use-docker"], + childLayer: child.layer, + }); + + // CLI-1963: `--use-docker` now runs the native Docker-unbundle path + // instead of delegating to the Go proxy. + yield* functionsDownload({ + ...BASE_FLAGS, + useDocker: true, + }).pipe(Effect.provide(layer)); + + expect(proxy.calls).toEqual([]); + expect(proxy.captureCalls).toEqual([]); + const runCommand = child.spawned.find( + (spawned) => spawned.command === "docker" && spawned.args[0] === "run", + ); + expect(runCommand?.args).toContain("unbundle"); + expect(out.stderrText).toContain("Downloading function: hello-world\n"); + // No `--debug` — the temp eszip file is removed after the run. + expect(existsSync(join(tempDir, "supabase", ".temp", "output_hello-world.eszip"))).toBe( + false, + ); + }).pipe( + Effect.ensuring(Effect.tryPromise(() => rm(tempDir, { recursive: true, force: true }))), + ); + }, + ); + + it.live("runs the native Docker path and emits a JSON envelope in machine mode", () => { const tempDir = makeTempDir(); + const child = mockChildProcessSpawner({ exitCode: 0 }); return Effect.gen(function* () { yield* Effect.tryPromise(() => writeProjectConfig(tempDir)); - const { layer, proxy } = setup(tempDir, { + const { out, layer, proxy } = setup(tempDir, { + format: "json", + bodyBySlug: { + "hello-world": { body: "fake-eszip-bytes", contentType: "application/octet-stream" }, + }, rawArgs: ["functions", "download", "hello-world", "--use-docker"], + childLayer: child.layer, }); + // CLI-1963: `--use-docker` now runs the native Docker-unbundle path; + // this asserts the JSON envelope this command emits itself still + // shows up correctly once the native path is exercised in machine mode. yield* functionsDownload({ ...BASE_FLAGS, useDocker: true, }).pipe(Effect.provide(layer)); - expect(proxy.calls).toEqual([ - ["functions", "download", "hello-world", "--project-ref", PROJECT_REF, "--use-docker"], - ]); + expect(proxy.calls).toEqual([]); + expect(proxy.captureCalls).toEqual([]); + expect( + child.spawned.some((spawned) => spawned.command === "docker" && spawned.args[0] === "run"), + ).toBe(true); + expect(out.messages).toContainEqual( + expect.objectContaining({ + type: "success", + message: "Downloaded Edge Function source.", + data: { + function_slugs: ["hello-world"], + project_ref: PROJECT_REF, + }, + }), + ); }).pipe( Effect.ensuring(Effect.tryPromise(() => rm(tempDir, { recursive: true, force: true }))), ); }); - it.live("captures the Go proxy's output and emits a JSON envelope in machine mode", () => { + it.live("lists remote functions and downloads each natively via Docker in machine mode", () => { const tempDir = makeTempDir(); + const child = mockChildProcessSpawner({ exitCode: 0 }); return Effect.gen(function* () { yield* Effect.tryPromise(() => writeProjectConfig(tempDir)); const { out, layer, proxy } = setup(tempDir, { format: "json", - rawArgs: ["functions", "download", "hello-world", "--use-docker"], + list: [makeFunction({ slug: "hello-world" }), makeFunction({ slug: "goodbye-world" })], + bodyBySlug: { + "hello-world": { body: "fake-eszip-bytes", contentType: "application/octet-stream" }, + "goodbye-world": { body: "fake-eszip-bytes", contentType: "application/octet-stream" }, + }, + rawArgs: ["functions", "download", "--use-docker"], + childLayer: child.layer, }); - // CLI-1546: stdout is payload-only in machine mode, so the delegated - // Go child's raw output must be captured/discarded (not inherited), - // and this command must emit the `Output` envelope itself. yield* functionsDownload({ ...BASE_FLAGS, + functionName: Option.none(), useDocker: true, }).pipe(Effect.provide(layer)); expect(proxy.calls).toEqual([]); - expect(proxy.captureCalls).toEqual([ - ["functions", "download", "hello-world", "--project-ref", PROJECT_REF, "--use-docker"], - ]); + expect(proxy.captureCalls).toEqual([]); + expect( + child.spawned.filter( + (spawned) => spawned.command === "docker" && spawned.args[0] === "run", + ), + ).toHaveLength(2); expect(out.messages).toContainEqual( expect.objectContaining({ type: "success", message: "Downloaded Edge Function source.", data: { - function_slugs: ["hello-world"], + function_slugs: ["hello-world", "goodbye-world"], project_ref: PROJECT_REF, }, }), @@ -911,38 +995,130 @@ describe("functions download", () => { }); it.live( - "lists remote functions before delegating when no function name is given in machine mode", + "defaults --use-docker to true so a bare invocation still runs the native Docker path", () => { const tempDir = makeTempDir(); + const child = mockChildProcessSpawner({ exitCode: 0 }); return Effect.gen(function* () { yield* Effect.tryPromise(() => writeProjectConfig(tempDir)); - const { out, layer, proxy } = setup(tempDir, { - format: "json", - list: [makeFunction({ slug: "hello-world" }), makeFunction({ slug: "goodbye-world" })], - rawArgs: ["functions", "download", "--use-docker"], + const { layer, proxy } = setup(tempDir, { + bodyBySlug: { + "hello-world": { body: "fake-eszip-bytes", contentType: "application/octet-stream" }, + }, + // No `--use-docker` at all — mirrors a bare `supabase functions + // download hello-world` invocation relying on the flag's default. + rawArgs: ["functions", "download", "hello-world"], + childLayer: child.layer, }); + // `useDocker: true` is what `download.command.ts`'s + // `Flag.withDefault(true)` resolves to when the flag is omitted + // (CLI-1963 parity fix — `next` was previously missing this default, + // unlike the legacy shell's equivalent command). yield* functionsDownload({ ...BASE_FLAGS, - functionName: Option.none(), useDocker: true, }).pipe(Effect.provide(layer)); expect(proxy.calls).toEqual([]); - expect(proxy.captureCalls).toEqual([ - ["functions", "download", "--project-ref", PROJECT_REF, "--use-docker"], - ]); - expect(out.messages).toContainEqual( - expect.objectContaining({ - type: "success", - message: "Downloaded Edge Function source.", - data: { - function_slugs: ["hello-world", "goodbye-world"], - project_ref: PROJECT_REF, + expect( + child.spawned.some( + (spawned) => spawned.command === "docker" && spawned.args[0] === "run", + ), + ).toBe(true); + }).pipe( + Effect.ensuring(Effect.tryPromise(() => rm(tempDir, { recursive: true, force: true }))), + ); + }, + ); + + it.live( + "falls back to the native server-side path with a warning when Docker is not running", + () => { + const tempDir = makeTempDir(); + const child = mockChildProcessSpawner({ exitCode: 1 }); + const multipart = multipartBody([ + { + headers: { + "Content-Disposition": 'form-data; name="metadata"', + "Content-Type": "application/json", + }, + body: JSON.stringify({ deno2_entrypoint_path: "source/index.ts" }), + }, + { + headers: { + "Content-Disposition": 'form-data; name="file"; filename="source/index.ts"', + }, + body: "console.log('fallback')", + }, + ]); + + return Effect.gen(function* () { + yield* Effect.tryPromise(() => writeProjectConfig(tempDir)); + const { out, layer } = setup(tempDir, { + bodyBySlug: { "hello-world": multipart }, + rawArgs: ["functions", "download", "hello-world", "--use-docker"], + childLayer: child.layer, + }); + + yield* functionsDownload({ + ...BASE_FLAGS, + useDocker: true, + }).pipe(Effect.provide(layer)); + + expect(child.spawned).toEqual([{ command: "docker", args: ["info"] }]); + expect(out.stderrText).toContain("WARNING: Docker is not running\n"); + expect( + yield* Effect.tryPromise(() => + readFile(join(tempDir, "supabase", "functions", "hello-world", "index.ts"), "utf8"), + ), + ).toBe("console.log('fallback')"); + }).pipe( + Effect.ensuring(Effect.tryPromise(() => rm(tempDir, { recursive: true, force: true }))), + ); + }, + ); + + it.live( + "writes the eszip response body to disk exactly as received, regardless of Content-Encoding", + () => { + const tempDir = makeTempDir(); + const child = mockChildProcessSpawner({ exitCode: 0 }); + // Arbitrary binary bytes, not valid brotli — this mocked `Response` (a + // hand-built `new Response(body, {...})`, unlike a real `fetch()`) + // never applies transport-level content-decoding, so a + // `Content-Encoding: br` header here must have zero effect on what + // `downloadEszipBody` does with it. If production code ever tried to + // brotli-decompress this body again, decompression itself would throw + // on these bytes, failing this test. + const rawEszipBytes = new Uint8Array([0, 1, 2, 253, 254, 255, 10, 13, 0, 128, 200]); + + return Effect.gen(function* () { + yield* Effect.tryPromise(() => writeProjectConfig(tempDir)); + const { layer } = setup(tempDir, { + bodyBySlug: { + "hello-world": { + body: new Blob([rawEszipBytes]), + contentType: "application/octet-stream", + headers: { "content-encoding": "br" }, }, - }), + }, + // `--debug` keeps the temp eszip file on disk after a successful + // run so this test can inspect the exact bytes that were written. + rawArgs: ["functions", "download", "hello-world", "--use-docker", "--debug"], + childLayer: child.layer, + }); + + yield* functionsDownload({ + ...BASE_FLAGS, + useDocker: true, + }).pipe(Effect.provide(layer)); + + const written = yield* Effect.tryPromise(() => + readFile(join(tempDir, "supabase", ".temp", "output_hello-world.eszip")), ); + expect(new Uint8Array(written)).toEqual(rawEszipBytes); }).pipe( Effect.ensuring(Effect.tryPromise(() => rm(tempDir, { recursive: true, force: true }))), ); @@ -1346,6 +1522,65 @@ describe("functions download", () => { ); }); + it.live("maps eszip body transport errors with Go-style wording (Docker path)", () => { + const tempDir = makeTempDir(); + const child = mockChildProcessSpawner({ exitCode: 0 }); + + return Effect.gen(function* () { + yield* Effect.tryPromise(() => writeProjectConfig(tempDir)); + const { layer } = setup(tempDir, { + bodyErrorBySlug: { + "hello-world": new Error("network error"), + }, + rawArgs: ["functions", "download", "hello-world", "--use-docker"], + childLayer: child.layer, + }); + + // `downloadEszipBody` (the Docker path's own GET) uses a distinct + // error prefix ("failed to get function body") from the server-side + // `downloadBody`'s ("failed to download function") — Go parity. + const error = yield* functionsDownload({ + ...BASE_FLAGS, + useDocker: true, + }).pipe(Effect.provide(layer), Effect.flip); + + expect(error).toBeInstanceOf(Error); + expect(error.message).toBe("failed to get function body: network error"); + }).pipe( + Effect.ensuring(Effect.tryPromise(() => rm(tempDir, { recursive: true, force: true }))), + ); + }); + + it.live("maps unexpected eszip body statuses with Go-style wording (Docker path)", () => { + const tempDir = makeTempDir(); + const child = mockChildProcessSpawner({ exitCode: 0 }); + + return Effect.gen(function* () { + yield* Effect.tryPromise(() => writeProjectConfig(tempDir)); + const { layer } = setup(tempDir, { + bodyBySlug: { + "hello-world": { + status: 503, + body: "unavailable", + contentType: "text/plain", + }, + }, + rawArgs: ["functions", "download", "hello-world", "--use-docker"], + childLayer: child.layer, + }); + + const error = yield* functionsDownload({ + ...BASE_FLAGS, + useDocker: true, + }).pipe(Effect.provide(layer), Effect.flip); + + expect(error).toBeInstanceOf(Error); + expect(error.message).toBe("Error status 503: unavailable"); + }).pipe( + Effect.ensuring(Effect.tryPromise(() => rm(tempDir, { recursive: true, force: true }))), + ); + }); + it.live("maps metadata fallback transport errors with Go-style wording", () => { const tempDir = makeTempDir(); const multipart = multipartBody([ diff --git a/apps/cli/src/shared/cli/cobra-flag-groups.ts b/apps/cli/src/shared/cli/cobra-flag-groups.ts index 8b2d6c0727..2ec3d23d3d 100644 --- a/apps/cli/src/shared/cli/cobra-flag-groups.ts +++ b/apps/cli/src/shared/cli/cobra-flag-groups.ts @@ -28,6 +28,31 @@ export function hasExplicitLongFlag( return false; } +/** + * Raw value of `--`/`--=value` anywhere in argv + * (unscoped — no command-path anchoring), or `undefined` if absent. + */ +export function explicitStringFlag(rawArgs: ReadonlyArray, flagName: string) { + for (let index = 0; index < rawArgs.length; index += 1) { + const token = rawArgs[index]; + if (token === `--${flagName}`) { + return rawArgs[index + 1]; + } + if (token?.startsWith(`--${flagName}=`)) { + return token.slice(flagName.length + 3); + } + } + return undefined; +} + +/** + * Whether `--` (or `--=`) appears anywhere in argv, + * unscoped. + */ +export function hasGlobalLongFlag(rawArgs: ReadonlyArray, flagName: string) { + return rawArgs.some((token) => token === `--${flagName}` || token.startsWith(`--${flagName}=`)); +} + /** * Value-taking long flags registered persistently on the Go root command * (`apps/cli-go/cmd/root.go:324-333`: `--workdir`, `--network-id`, diff --git a/apps/cli/src/shared/cli/hidden-flag.unit.test.ts b/apps/cli/src/shared/cli/hidden-flag.unit.test.ts index 7c9a9bbe07..1cab6b610b 100644 --- a/apps/cli/src/shared/cli/hidden-flag.unit.test.ts +++ b/apps/cli/src/shared/cli/hidden-flag.unit.test.ts @@ -137,13 +137,30 @@ describe("native hidden flags", () => { "--backup=false", ]).pipe(Effect.exit); expect(JSON.stringify(stopExit)).not.toContain("UnrecognizedFlag"); - yield* Command.runWith(legacyTestRoot, { version: "0.0.0-test" })([ + // `functions download --use-docker` now runs the native Docker-unbundle + // path (CLI-1963) instead of forwarding to `LegacyGoProxy` — it can fail + // for Docker-related reasons in this proxy-only test layer, same as + // `start`/`stop` above, so this only proves the hidden flag still parses. + // `--legacy-bundle` is the one remaining case that still forwards to the + // proxy, asserted below. + const downloadUseDockerExit = yield* Command.runWith(legacyTestRoot, { + version: "0.0.0-test", + })([ "functions", "download", "hello", "--project-ref", "abcdefghijklmnopqrst", "--use-docker", + ]).pipe(Effect.exit); + expect(JSON.stringify(downloadUseDockerExit)).not.toContain("UnrecognizedFlag"); + yield* Command.runWith(legacyTestRoot, { version: "0.0.0-test" })([ + "functions", + "download", + "hello", + "--project-ref", + "abcdefghijklmnopqrst", + "--legacy-bundle", ]); const useDockerExit = yield* Command.runWith(legacyTestRoot, { version: "0.0.0-test", @@ -171,7 +188,14 @@ describe("native hidden flags", () => { ); expect(proxy.calls).toEqual([ - ["functions", "download", "hello", "--project-ref", "abcdefghijklmnopqrst", "--use-docker"], + [ + "functions", + "download", + "hello", + "--project-ref", + "abcdefghijklmnopqrst", + "--legacy-bundle", + ], ]); }); diff --git a/apps/cli/src/shared/functions/deploy.ts b/apps/cli/src/shared/functions/deploy.ts index 1043a3a149..7d59849035 100644 --- a/apps/cli/src/shared/functions/deploy.ts +++ b/apps/cli/src/shared/functions/deploy.ts @@ -8,19 +8,19 @@ import { loadProjectConfig, type ResolvedFunctionConfig as ManifestFunctionConfig, } from "@supabase/config"; -import { Duration, Effect, Option, Schema, Stream } from "effect"; -import { ChildProcessSpawner } from "effect/unstable/process"; +import { Duration, Effect, Option, Schema } from "effect"; import * as HttpClientError from "effect/unstable/http/HttpClientError"; import { legacyPromptYesNo } from "../legacy/legacy-prompt-yes-no.ts"; import { CONTEXT_CANCELED_MESSAGE } from "../output/errors.ts"; import { Output } from "../output/output.service.ts"; -import { spawnContainerCli } from "../../legacy/shared/legacy-container-cli.ts"; import { legacyBold } from "../../legacy/shared/legacy-colors.ts"; import { legacyGetRegistryImageUrl } from "../../legacy/shared/legacy-docker-registry.ts"; import { findGitRootPath } from "../git/git-root.ts"; import { cobraMutuallyExclusiveErrorMessage, + explicitStringFlag, hasExplicitLongFlag, + hasGlobalLongFlag, } from "../cli/cobra-flag-groups.ts"; import { FUNCTIONS_BUNDLER_MUTEX_GROUP, @@ -33,14 +33,21 @@ import { InvalidFunctionDeploySlugError, NoFunctionsToDeployError, } from "./deploy.errors.ts"; +import { + ensureDockerNamedVolume, + ensureDockerNetwork, + isDockerRunning, + localDockerId, + resolveEdgeRuntimeVersion, + runChildProcess, + toDockerPath, + toSlash, +} from "./functions-docker.ts"; const COMPRESSED_ESZIP_MAGIC = "EZBR"; -const DENO1_EDGE_RUNTIME_VERSION = "1.68.4"; const DEPLOY_RATE_LIMIT_MAX_RETRIES = 8; const SUPABASE_FUNCTIONS_DIR = "supabase/functions"; const IMPORT_MAP_GUIDE_URL = "https://supabase.com/docs/guides/functions/import-maps"; -const INVALID_PROJECT_ID = /[^a-zA-Z0-9_.-]+/g; -const MAX_PROJECT_ID_LENGTH = 40; const WINDOWS_ABSOLUTE_PATH = /^[A-Za-z]:\//; const importPathPattern = /(?:import|export)\s+(?:type\s+)?(?:{[^{}]+}|.*?)\s*(?:from)?\s*['"](.*?)['"]|import\(\s*['"](.*?)['"]\)/gi; @@ -213,6 +220,18 @@ function validateDeploySlug(slug: string): Effect.Effect` was passed + * explicitly after `commandPath`, matching cobra's `Changed()`; + * `Option.none()` otherwise. Used only by `deployFunctions`'s + * `--no-verify-jwt` override below — kept private per this file's own + * "used by one command only -> keep it in the command's own directory" rule. + */ function explicitBooleanFlag( rawArgs: ReadonlyArray, commandPath: ReadonlyArray, @@ -222,45 +241,6 @@ function explicitBooleanFlag( return hasExplicitLongFlag(rawArgs, commandPath, flagName) ? Option.some(value) : Option.none(); } -function explicitStringFlag(rawArgs: ReadonlyArray, flagName: string) { - for (let index = 0; index < rawArgs.length; index += 1) { - const token = rawArgs[index]; - if (token === `--${flagName}`) { - return rawArgs[index + 1]; - } - if (token?.startsWith(`--${flagName}=`)) { - return token.slice(flagName.length + 3); - } - } - return undefined; -} - -function hasGlobalLongFlag(rawArgs: ReadonlyArray, flagName: string) { - return rawArgs.some((token) => token === `--${flagName}` || token.startsWith(`--${flagName}=`)); -} - -function isDenoConfigFile(pathname: string) { - const name = basename(pathname).toLowerCase(); - return name === "deno.json" || name === "deno.jsonc"; -} - -function toSlash(pathname: string) { - return pathname.replaceAll("\\", "/"); -} - -export function normalizeProjectId(source: string) { - const sanitized = source.replaceAll(INVALID_PROJECT_ID, "_").replace(/^[_.-]+/, ""); - return sanitized.length > MAX_PROJECT_ID_LENGTH - ? sanitized.slice(0, MAX_PROJECT_ID_LENGTH) - : sanitized; -} - -export function localDockerId(name: string, projectId: string) { - return `supabase_${name}_${normalizeProjectId(projectId)}`; -} - -const dockerCliProjectLabel = "com.supabase.cli.project"; -const dockerComposeProjectLabel = "com.docker.compose.project"; /** * Must stay in sync with `LEGACY_CLI_WORKDIR_LABEL` * (`legacy/shared/legacy-docker-ids.ts:95`) — same string literal, kept as a @@ -280,18 +260,6 @@ export const dockerWorkdirLabel = "com.supabase.cli.workdir"; */ const dockerNpmEnvNames = ["NPM_CONFIG_REGISTRY"] as const; -export function dockerProjectLabels(projectId: string) { - return { - [dockerCliProjectLabel]: projectId, - [dockerComposeProjectLabel]: projectId, - }; -} - -export function toDockerPath(hostPath: string) { - const normalized = toSlash(resolve(hostPath)); - return normalized.replace(/^[A-Za-z]:/, ""); -} - function toBundledFileUrl(hostPath: string) { const url = new URL("file:///"); url.pathname = toDockerPath(hostPath).replaceAll("%", "%25"); @@ -1107,15 +1075,6 @@ function createBundledMetadata( }; } -function collectByteStream(stream: Stream.Stream) { - const decoder = new TextDecoder(); - return Stream.runFold( - stream, - () => "", - (text, chunk) => text + decoder.decode(chunk, { stream: true }), - ).pipe(Effect.map((text) => text + decoder.decode())); -} - function sanitizeDockerBinds( binds: ReadonlyArray, functionsDir: string, @@ -1261,84 +1220,6 @@ function shouldUseDenoJsonDiscovery(entrypoint: string, importMap: string) { return isDenoConfigFile(importMap) && dirname(importMap) === dirname(entrypoint); } -export function isUserDefinedDockerNetwork(networkMode: string) { - return ( - networkMode.length > 0 && - networkMode !== "default" && - networkMode !== "bridge" && - networkMode !== "host" && - networkMode !== "none" - ); -} - -export const ensureDockerNetwork = Effect.fnUntraced(function* ( - networkMode: string, - projectId: string, -) { - if (!isUserDefinedDockerNetwork(networkMode)) { - return; - } - - const inspect = yield* runChildProcess("docker", ["network", "inspect", networkMode], { - stdout: "ignore", - stderr: "ignore", - }).pipe(Effect.catch(() => Effect.succeed({ exitCode: 1, stdout: "", stderr: "" }))); - if (inspect.exitCode === 0) { - return; - } - - const labels = dockerProjectLabels(projectId); - const create = yield* runChildProcess( - "docker", - [ - "network", - "create", - "--label", - `${dockerCliProjectLabel}=${labels[dockerCliProjectLabel]}`, - "--label", - `${dockerComposeProjectLabel}=${labels[dockerComposeProjectLabel]}`, - networkMode, - ], - { - stdout: "ignore", - stderr: "pipe", - }, - ); - if (create.exitCode !== 0 && !create.stderr.includes("already exists")) { - return yield* Effect.fail(new Error(`failed to create docker network: ${networkMode}`)); - } -}); - -export const ensureDockerNamedVolume = Effect.fnUntraced(function* ( - volumeName: string, - projectId: string, -) { - if (process.env["BITBUCKET_CLONE_DIR"] !== undefined) { - return; - } - - const labels = dockerProjectLabels(projectId); - const create = yield* runChildProcess( - "docker", - [ - "volume", - "create", - "--label", - `${dockerCliProjectLabel}=${labels[dockerCliProjectLabel]}`, - "--label", - `${dockerComposeProjectLabel}=${labels[dockerComposeProjectLabel]}`, - volumeName, - ], - { - stdout: "ignore", - stderr: "pipe", - }, - ); - if (create.exitCode !== 0 && !create.stderr.includes("already exists")) { - return yield* Effect.fail(new Error(`failed to create docker volume: ${volumeName}`)); - } -}); - async function shouldUsePackageJsonDiscovery(entrypoint: string, importMap: string) { if (importMap.length > 0) { return false; @@ -1351,48 +1232,6 @@ async function shouldUsePackageJsonDiscovery(entrypoint: string, importMap: stri } } -// Runs a container CLI command and collects its output. Every caller runs -// `docker`, so the spawn goes through `spawnContainerCli` to fall back to -// `podman` on Docker-less hosts. `command` is retained for the extendEnv -// default and the `functions serve` dependency-injection seam. -export const runChildProcess = Effect.fnUntraced(function* ( - command: string, - args: ReadonlyArray, - opts: { - readonly stdout?: "pipe" | "ignore"; - readonly stderr?: "pipe" | "ignore"; - readonly env?: Readonly>; - readonly extendEnv?: boolean; - } = {}, -) { - const spawner = yield* ChildProcessSpawner.ChildProcessSpawner; - const child = yield* spawnContainerCli(spawner, [...args], { - stdin: "ignore", - stdout: opts.stdout ?? "pipe", - stderr: opts.stderr ?? "pipe", - env: opts.env, - extendEnv: opts.extendEnv ?? command === "docker", - }); - - const [stdout, stderr, exitCode] = yield* Effect.all( - [ - opts.stdout === "ignore" ? Effect.succeed("") : collectByteStream(child.stdout), - opts.stderr === "ignore" ? Effect.succeed("") : collectByteStream(child.stderr), - child.exitCode.pipe(Effect.map(Number)), - ], - { concurrency: "unbounded" }, - ); - return { exitCode, stdout, stderr }; -}); - -const isDockerRunning = Effect.fnUntraced(function* () { - const result = yield* runChildProcess("docker", ["info"], { - stdout: "ignore", - stderr: "ignore", - }).pipe(Effect.catch(() => Effect.succeed({ exitCode: 1, stdout: "", stderr: "" }))); - return result.exitCode === 0; -}); - const bundleFunctionWithDocker = Effect.fnUntraced(function* ( projectId: string, edgeRuntimeVersion: string, @@ -2161,21 +2000,6 @@ const deployViaDocker = Effect.fnUntraced(function* ( } }); -export function resolveEdgeRuntimeVersion( - denoVersion: number | undefined, - defaultVersion: string, -): Effect.Effect { - if (denoVersion === undefined || denoVersion === 2) { - return Effect.succeed(defaultVersion); - } - if (denoVersion === 1) { - return Effect.succeed(DENO1_EDGE_RUNTIME_VERSION); - } - return Effect.fail( - new Error(`Failed reading config: Invalid edge_runtime.deno_version: ${denoVersion}.`), - ); -} - const pruneFunctions = Effect.fnUntraced(function* ( projectRef: string, configs: ReadonlyArray, diff --git a/apps/cli/src/shared/functions/download.ts b/apps/cli/src/shared/functions/download.ts index 7b9882f6dd..bff0926d2c 100644 --- a/apps/cli/src/shared/functions/download.ts +++ b/apps/cli/src/shared/functions/download.ts @@ -1,6 +1,7 @@ import { operationDefinitions, type ApiClient } from "@supabase/api/effect"; +import { loadProjectConfig } from "@supabase/config"; import { randomUUID } from "node:crypto"; -import { open, rename, rm } from "node:fs/promises"; +import { mkdir, open, rename, rm, writeFile } from "node:fs/promises"; import { dirname, isAbsolute, join, posix, relative, resolve, sep } from "node:path"; import { fileURLToPath } from "node:url"; import { Effect, FileSystem, Option } from "effect"; @@ -9,8 +10,20 @@ import type * as HttpClientResponse from "effect/unstable/http/HttpClientRespons import { Output } from "../output/output.service.ts"; import { cobraMutuallyExclusiveErrorMessage, + explicitStringFlag, hasExplicitLongFlag, + hasGlobalLongFlag, } from "../cli/cobra-flag-groups.ts"; +import { legacyDescribeContainerCliFailure } from "../../legacy/shared/legacy-container-cli.ts"; +import { legacyGetRegistryImageUrl } from "../../legacy/shared/legacy-docker-registry.ts"; +import { + ensureDockerNamedVolume, + ensureDockerNetwork, + isDockerRunning, + localDockerId, + resolveEdgeRuntimeVersion, + runChildProcess, +} from "./functions-docker.ts"; import { FUNCTIONS_BUNDLER_MUTEX_GROUP, invalidFunctionSlugDetail, @@ -25,6 +38,11 @@ import { } from "./download.errors.ts"; const legacyEntrypointPath = "file:///src/index.ts"; +// Go: `utils.DockerDenoDir`/`utils.DockerEszipDir` (`internal/utils/deno.go:34-35`) +// — fixed container-side paths for the docker-unbundle path, unrelated to +// deploy's `toDockerPath` host-mirroring scheme. +const DOCKER_DENO_DIR = "/home/deno"; +const DOCKER_ESZIP_DIR = "/root/eszips"; export interface DownloadFunctionsOptions { readonly functionName: Option.Option; @@ -34,15 +52,46 @@ export interface DownloadFunctionsOptions { readonly legacyBundle: boolean; } +interface DownloadRuntimeDependencies { + readonly api: ApiClient; + readonly projectRoot: string; +} + +/** Adds what the Docker-unbundle path needs beyond the server-side path. */ +interface DownloadDockerRuntimeDependencies extends DownloadRuntimeDependencies { + readonly rawArgs: ReadonlyArray; +} + +/** + * What {@link resolveEdgeRuntimeImage} needs to resolve the Docker + * edge-runtime image tag — split out so it's declared once instead of + * duplicated across `DownloadFunctionsDependencies`'s fields. + */ +interface EdgeRuntimeImageDependencies { + readonly projectRoot: string; + /** + * `true` in the legacy shell, `false` in `next` — forwarded verbatim to + * `loadProjectConfig`'s `goViperCompat` option (matches every other + * `functions`-family command, e.g. `deploy.ts`'s own `DeployFunctionsDependencies`). + */ + readonly goViperCompat: boolean; + /** + * Fallback edge-runtime image tag used when the project config doesn't pin + * `edge_runtime.deno_version` to `1` (which forces the older + * `DENO1_EDGE_RUNTIME_VERSION`) — mirrors `deploy.ts`'s own + * `edgeRuntimeVersion` dependency, read via + * `resolveEdgeRuntimeVersionPin` by the shell-specific handler. + */ + readonly edgeRuntimeVersion: string; +} + export interface DownloadFunctionsDependencies< ResolveError, ResolveRequirements, ProxyError, ProxyRequirements, -> { - readonly api: ApiClient; - readonly projectRoot: string; - readonly rawArgs: ReadonlyArray; +> + extends DownloadDockerRuntimeDependencies, EdgeRuntimeImageDependencies { readonly resolveProjectRef: ( projectRef: Option.Option, ) => Effect.Effect; @@ -51,7 +100,8 @@ export interface DownloadFunctionsDependencies< * child's raw stdout must not reach the terminal (it would corrupt the * JSON/NDJSON envelope, CLI-1546's "stdout is payload-only in machine * mode" invariant), so the dependency must capture/discard it (e.g. via - * `LegacyGoProxy.execCapture`) instead of inheriting stdio. + * `LegacyGoProxy.execCapture`) instead of inheriting stdio. Only invoked + * for `--legacy-bundle` today — `--use-docker` now runs natively (CLI-1963). */ readonly proxyDownload: ( flags: DownloadFunctionsOptions, @@ -60,29 +110,18 @@ export interface DownloadFunctionsDependencies< ) => Effect.Effect; } -interface DownloadRuntimeDependencies { - readonly api: ApiClient; - readonly projectRoot: string; -} - -export function makeGoProxyDownloadArgs( - flags: DownloadFunctionsOptions, +// `--legacy-bundle` is the only case `downloadFunctions()` still delegates to +// the Go binary for (CLI-1963) — `functionName` is the one remaining piece of +// user input the delegating branch needs to forward. +export function makeGoProxyLegacyBundleArgs( + functionName: Option.Option, projectRef: string, ): ReadonlyArray { const args: string[] = ["functions", "download"]; - if (Option.isSome(flags.functionName)) { - args.push(flags.functionName.value); - } - args.push("--project-ref", projectRef); - // At most one of these may reach the Go binary — it re-parses this argv - // fresh and enforces the same mutual exclusivity itself. `legacyBundle` - // takes priority since `useDocker` now defaults to `true` (CLI-1862) and - // would otherwise ride along on every `--legacy-bundle` invocation. - if (flags.legacyBundle) { - args.push("--legacy-bundle"); - } else if (flags.useDocker) { - args.push("--use-docker"); + if (Option.isSome(functionName)) { + args.push(functionName.value); } + args.push("--project-ref", projectRef, "--legacy-bundle"); return args; } @@ -127,6 +166,31 @@ function validateSlug(slug: string): Effect.Effect` argument), which fails with a plain `InvalidFunctionSlugError` and no + * "failed to download function" prefix or suggestion. + */ +function validateRemoteSlug(slug: string): Effect.Effect { + if (validateFunctionSlugMessage(slug) === undefined) { + return Effect.void; + } + + return Effect.fail( + Object.assign(new Error(`failed to download function ${slug}: ${invalidFunctionSlugDetail}`), { + suggestion: `The Supabase API returned an unexpected function slug (${slug}). Retry the command, and if this keeps happening, verify your network connection is not being intercepted before contacting Supabase support.`, + }), + ); +} + const downloadCommandPath = ["functions", "download"] as const; function validateDownloadFlags( @@ -678,6 +742,273 @@ const downloadBody = Effect.fnUntraced(function* ( return yield* Effect.fail(new Error(`Error status ${response.status}: ${body}`)); }); +// Go: `downloadOne` (`apps/cli-go/internal/functions/download/download.go:218-245`) +// — no `Accept` override (contrast `downloadBody` above, which requests +// `multipart/form-data` for the server-side path). Go explicitly decodes a +// brotli `Content-Encoding` itself because Go's `http.Transport` only +// auto-decodes `gzip`; this TS CLI's transport (`effect/unstable/http`'s +// `FetchHttpClient`, backed by the platform `fetch`) already transparently +// decodes `br` per the Fetch spec — while still reporting +// `Content-Encoding: br` on the exposed `Response.headers` (confirmed +// empirically: a `fetch()` against a real `Content-Encoding: br` response +// returns already-decompressed bytes from `arrayBuffer()`). Re-running +// `brotliDecompressSync` here would therefore throw on already-decoded +// bytes, so this reads the body as-is and does not re-implement Go's manual +// decode step. Error prefix ("failed to get function body") is deliberately +// distinct from `downloadBody`'s ("failed to download function") — the two +// Go call sites use different wording. +const downloadEszipBody = Effect.fnUntraced(function* ( + api: ApiClient, + projectRef: string, + slug: string, +) { + const response = yield* api + .executeRaw(operationDefinitions.v1GetAFunctionBody, { + ref: projectRef, + function_slug: slug, + }) + .pipe(Effect.mapError((error) => mapTransportError("failed to get function body", error))); + + if (response.status !== 200) { + const body = yield* response.text.pipe(Effect.orElseSucceed(() => "")); + return yield* Effect.fail(new Error(`Error status ${response.status}: ${body}`)); + } + + return new Uint8Array( + yield* response.arrayBuffer.pipe( + Effect.mapError( + (cause) => + new Error( + `failed to download file: ${cause instanceof Error ? cause.message : String(cause)}`, + ), + ), + ), + ); +}); + +function suggestLegacyBundle(slug: string): string { + // Go: `suggestLegacyBundle` (`download.go:314-316`) — verbatim, including + // the source's own "trying running" wording and its leading newline. + return `\nIf your function is deployed using CLI < 1.120.0, trying running supabase functions download --legacy-bundle ${slug} instead.`; +} + +function suggestDenoV2(): string { + // Go: `suggestDenoV2` (`download.go:306-312`), verbatim including its + // trailing newline. + return "Please use deno v2 in supabase/config.toml to download this Function:\n\n[edge_runtime]\ndeno_version = 2\n"; +} + +/** + * Attaches Go's `suggestLegacyBundle` hint to any Docker-extraction failure — + * matches `downloadWithDockerUnbundle`'s `CmdSuggestion += + * suggestLegacyBundle(slug)` (`download.go:211-214`), which runs whenever + * `extractOne` fails for *any* reason (network/volume creation, container + * create/start, log streaming, container inspect), not just a non-zero exit + * code. `ensureDockerNetwork`/`ensureDockerNamedVolume` already prefix their + * own "failed to create docker network/volume: ..." context on the failures + * they raise themselves (`functions-docker.ts`), so this only normalizes + * (never re-prefixes) whatever `legacyDescribeContainerCliFailure` reports. + */ +function withLegacyBundleSuggestion(slug: string) { + return (cause: unknown): Error => + Object.assign(new Error(legacyDescribeContainerCliFailure(cause)), { + suggestion: suggestLegacyBundle(slug), + }); +} + +/** + * Same as {@link withLegacyBundleSuggestion}, plus a `step` prefix — for + * `runChildProcess` itself, whose own failure (a spawn error, or the + * `PlatformError` `functions-docker.ts`'s hoisted `collectByteStream` erases + * to `unknown`) carries no context of its own about which command was + * running, unlike `ensureDockerNetwork`/`ensureDockerNamedVolume`'s + * self-describing errors. + */ +function withDockerStepFailure(step: string, slug: string) { + return (cause: unknown): Error => + Object.assign(new Error(`${step}: ${legacyDescribeContainerCliFailure(cause)}`), { + suggestion: suggestLegacyBundle(slug), + }); +} + +// Go: `Config.EdgeRuntime.Image` (`extractOne`, `download.go:271`) resolves +// from `edge_runtime.deno_version` — `1` pins the older +// `DENO1_EDGE_RUNTIME_VERSION`, anything else (including unset) uses the +// project's configured/default tag (`resolveEdgeRuntimeVersion`, shared with +// `deploy.ts`). `project_id` mirrors `deploy.ts`'s own +// `deployConfig?.project_id ?? projectRef` fallback for Docker network/volume +// naming (`GetId`, `internal/utils/config.go:57-58`). Resolved once per +// invocation by the caller (`downloadFunctions`), not once per slug — Go's +// `Config` is likewise loaded once, before any per-function work. +const resolveEdgeRuntimeImage = Effect.fnUntraced(function* ( + dependencies: EdgeRuntimeImageDependencies, + projectRef: string, +) { + const loadedConfig = yield* loadProjectConfig(dependencies.projectRoot, { + projectRef, + goViperCompat: dependencies.goViperCompat, + }); + const denoVersion = loadedConfig?.config?.edge_runtime.deno_version; + const projectId = loadedConfig?.config?.project_id ?? projectRef; + const edgeRuntimeVersion = yield* resolveEdgeRuntimeVersion( + denoVersion, + dependencies.edgeRuntimeVersion, + ); + return { + projectId, + denoVersion, + image: legacyGetRegistryImageUrl(`supabase/edge-runtime:v${edgeRuntimeVersion}`), + }; +}); + +interface EdgeRuntimeImage { + readonly projectId: string; + readonly denoVersion: number | undefined; + readonly image: string; +} + +// Go: `downloadWithDockerUnbundle`/`extractOne` +// (`download.go:198-282`) — downloads the function body as an eszip, writes +// it to a temp file, then runs the edge-runtime image's `unbundle` +// subcommand against it, mounting the *shared* `supabase/functions` +// directory (not the slug's own subdirectory — `download_test.go:267-271` +// asserts this explicitly). +const downloadWithDockerUnbundle = Effect.fnUntraced(function* ( + dependencies: DownloadDockerRuntimeDependencies, + edgeRuntimeImage: EdgeRuntimeImage, + projectRef: string, + slug: string, +) { + const output = yield* Output; + + // Go: `downloadOne` (`download.go:219`) — lowercase "function", distinct + // from the server-side path's "Downloading Function:" (capital F, + // `downloadWithServerSideUnbundle`, `download.go:329`). + yield* output.raw(`Downloading function: ${slug}\n`, "stderr"); + + const eszip = yield* downloadEszipBody(dependencies.api, projectRef, slug); + + const tempDir = join(dependencies.projectRoot, "supabase", ".temp"); + yield* Effect.tryPromise({ + try: () => mkdir(tempDir, { recursive: true }), + catch: (cause) => + new Error(`failed to mkdir: ${cause instanceof Error ? cause.message : String(cause)}`), + }); + const eszipFileName = `output_${slug}.eszip`; + const eszipPath = join(tempDir, eszipFileName); + yield* Effect.tryPromise({ + try: () => writeFile(eszipPath, eszip), + catch: (cause) => + new Error( + `failed to download file: ${cause instanceof Error ? cause.message : String(cause)}`, + ), + }); + + // Go: the `defer fsys.Remove(eszipPath)` cleanup is registered right after + // the write and covers the whole of `extractOne`, including the container + // run — it fires on every return path, success or failure + // (`download.go:203-209`). `Effect.ensuring` below is the equivalent: it + // wraps every step from here on so a failure resolving the network/volume, + // spawning Docker, or a non-zero container exit all still clean up the + // temp eszip, matching Go instead of only doing so on the happy path. + const debugEnabled = hasGlobalLongFlag(dependencies.rawArgs, "debug"); + const cleanupEszip = debugEnabled + ? Effect.void + : Effect.tryPromise({ + try: () => rm(eszipPath, { force: true }), + catch: (cause) => (cause instanceof Error ? cause.message : String(cause)), + }).pipe(Effect.catch((message) => output.raw(`${message}\n`, "stderr"))); + + const { projectId, denoVersion, image } = edgeRuntimeImage; + const functionsDir = resolve(dependencies.projectRoot, "supabase", "functions"); + const hostEszipPath = resolve(eszipPath); + const dockerEszipPath = posix.join(DOCKER_ESZIP_DIR, eszipFileName); + const dockerOutputPath = posix.join(DOCKER_DENO_DIR, slug); + + // Go: `viper.GetString("network-id")` else `NetId` (`docker.go:379-383`) — + // `--network-id` is a persistent root flag (`cmd/root.go:328`), not + // registered on `functions download` itself. + const networkMode = + explicitStringFlag(dependencies.rawArgs, "network-id") ?? localDockerId("network", projectId); + + const extract = Effect.gen(function* () { + yield* ensureDockerNetwork(networkMode, projectId).pipe( + Effect.mapError(withLegacyBundleSuggestion(slug)), + ); + yield* ensureDockerNamedVolume(localDockerId("edge_runtime", projectId), projectId).pipe( + Effect.mapError(withLegacyBundleSuggestion(slug)), + ); + + // Bind order matches `extractOne` (`download.go:260-266`) exactly. + const binds = [ + `${localDockerId("edge_runtime", projectId)}:/root/.cache/deno:rw`, + `${hostEszipPath}:${dockerEszipPath}:ro`, + `${functionsDir}:${DOCKER_DENO_DIR}:rw`, + ]; + const command = [ + "run", + "--rm", + ...binds.flatMap((bind) => ["-v", bind]), + "--network", + networkMode, + ]; + if (process.platform === "linux") { + command.push("--add-host", "host.docker.internal:host-gateway"); + } + command.push(image, "unbundle", "--eszip", dockerEszipPath, "--output", dockerOutputPath); + + const result = yield* runChildProcess("docker", command, { + stdout: "pipe", + stderr: "pipe", + }).pipe( + Effect.mapError( + withDockerStepFailure("failed to run the edge-runtime unbundle container", slug), + ), + ); + + // Go pipes the container's stdout straight to `os.Stdout` (`download.go:279`); + // machine-output modes must keep stdout payload-only (CLI-1546), so this + // mirrors `deploy.ts`'s own `bundleFunctionWithDocker` routing. + if (result.stdout.length > 0) { + yield* output.raw(result.stdout, output.format === "text" ? "stdout" : "stderr"); + } + if (result.stderr.length > 0) { + yield* output.raw(result.stderr, "stderr"); + } + + if (result.exitCode !== 0) { + // Go's `getErrorLogger` (deno-v1 only) sets `CmdSuggestion = + // suggestDenoV2()` (assignment) as soon as a full stderr line reads + // "invalid eszip v2" (case-insensitive), then `downloadWithDockerUnbundle` + // appends `suggestLegacyBundle` (`+=`) once extraction has failed + // (`download.go:213,284-304`). Go's own implementation races these two + // goroutines (the pipe writer is never closed) — this resolves that + // race deterministically to the common (non-race) ordering instead of + // reproducing the nondeterminism. The line match is exact (not a + // substring) to match Go's `strings.EqualFold(line, "invalid eszip v2")`. + const invalidEszipV2 = + denoVersion === 1 && + result.stderr + .split(/\r?\n/) + .some((line) => line.trim().toLowerCase() === "invalid eszip v2"); + const suggestion = (invalidEszipV2 ? suggestDenoV2() : "") + suggestLegacyBundle(slug); + return yield* Effect.fail( + Object.assign(new Error(`error running container: exit ${result.exitCode}`), { + suggestion, + }), + ); + } + + // Go: `downloadWithDockerUnbundle` has no final "Downloaded Function ..." + // print, unlike `RunLegacy`/`downloadWithServerSideUnbundle` — its only + // stdout/stderr text is "Downloading function: ..." above plus whatever + // the `unbundle` container itself wrote. + return slug; + }); + + return yield* extract.pipe(Effect.ensuring(cleanupEszip)); +}); + const downloadSingle = Effect.fnUntraced(function* ( dependencies: DownloadRuntimeDependencies, projectRef: string, @@ -764,11 +1095,17 @@ export function downloadFunctions MAX_PROJECT_ID_LENGTH + ? sanitized.slice(0, MAX_PROJECT_ID_LENGTH) + : sanitized; +} + +export function localDockerId(name: string, projectId: string) { + return `supabase_${name}_${normalizeProjectId(projectId)}`; +} + +const dockerCliProjectLabel = "com.supabase.cli.project"; +const dockerComposeProjectLabel = "com.docker.compose.project"; + +export function dockerProjectLabels(projectId: string) { + return { + [dockerCliProjectLabel]: projectId, + [dockerComposeProjectLabel]: projectId, + }; +} + +export function toDockerPath(hostPath: string) { + const normalized = toSlash(resolve(hostPath)); + return normalized.replace(/^[A-Za-z]:/, ""); +} + +function collectByteStream(stream: Stream.Stream) { + const decoder = new TextDecoder(); + return Stream.runFold( + stream, + () => "", + (text, chunk) => text + decoder.decode(chunk, { stream: true }), + ).pipe(Effect.map((text) => text + decoder.decode())); +} + +// Runs a container CLI command and collects its output. Every caller runs +// `docker`, so the spawn goes through `spawnContainerCli` to fall back to +// `podman` on Docker-less hosts. `command` is retained for the extendEnv +// default and the `functions serve` dependency-injection seam. +export const runChildProcess = Effect.fnUntraced(function* ( + command: string, + args: ReadonlyArray, + opts: { + readonly stdout?: "pipe" | "ignore"; + readonly stderr?: "pipe" | "ignore"; + readonly env?: Readonly>; + readonly extendEnv?: boolean; + } = {}, +) { + const spawner = yield* ChildProcessSpawner.ChildProcessSpawner; + const child = yield* spawnContainerCli(spawner, [...args], { + stdin: "ignore", + stdout: opts.stdout ?? "pipe", + stderr: opts.stderr ?? "pipe", + env: opts.env, + extendEnv: opts.extendEnv ?? command === "docker", + }); + + const [stdout, stderr, exitCode] = yield* Effect.all( + [ + opts.stdout === "ignore" ? Effect.succeed("") : collectByteStream(child.stdout), + opts.stderr === "ignore" ? Effect.succeed("") : collectByteStream(child.stderr), + child.exitCode.pipe(Effect.map(Number)), + ], + { concurrency: "unbounded" }, + ); + return { exitCode, stdout, stderr }; +}); + +export function isUserDefinedDockerNetwork(networkMode: string) { + return ( + networkMode.length > 0 && + networkMode !== "default" && + networkMode !== "bridge" && + networkMode !== "host" && + networkMode !== "none" + ); +} + +export const ensureDockerNetwork = Effect.fnUntraced(function* ( + networkMode: string, + projectId: string, +) { + if (!isUserDefinedDockerNetwork(networkMode)) { + return; + } + + const inspect = yield* runChildProcess("docker", ["network", "inspect", networkMode], { + stdout: "ignore", + stderr: "ignore", + }).pipe(Effect.catch(() => Effect.succeed({ exitCode: 1, stdout: "", stderr: "" }))); + if (inspect.exitCode === 0) { + return; + } + + const labels = dockerProjectLabels(projectId); + const create = yield* runChildProcess( + "docker", + [ + "network", + "create", + "--label", + `${dockerCliProjectLabel}=${labels[dockerCliProjectLabel]}`, + "--label", + `${dockerComposeProjectLabel}=${labels[dockerComposeProjectLabel]}`, + networkMode, + ], + { + stdout: "ignore", + stderr: "pipe", + }, + ); + if (create.exitCode !== 0 && !create.stderr.includes("already exists")) { + return yield* Effect.fail(new Error(`failed to create docker network: ${networkMode}`)); + } +}); + +export const ensureDockerNamedVolume = Effect.fnUntraced(function* ( + volumeName: string, + projectId: string, +) { + if (process.env["BITBUCKET_CLONE_DIR"] !== undefined) { + return; + } + + const labels = dockerProjectLabels(projectId); + const create = yield* runChildProcess( + "docker", + [ + "volume", + "create", + "--label", + `${dockerCliProjectLabel}=${labels[dockerCliProjectLabel]}`, + "--label", + `${dockerComposeProjectLabel}=${labels[dockerComposeProjectLabel]}`, + volumeName, + ], + { + stdout: "ignore", + stderr: "pipe", + }, + ); + if (create.exitCode !== 0 && !create.stderr.includes("already exists")) { + return yield* Effect.fail(new Error(`failed to create docker volume: ${volumeName}`)); + } +}); + +export const isDockerRunning = Effect.fnUntraced(function* () { + const result = yield* runChildProcess("docker", ["info"], { + stdout: "ignore", + stderr: "ignore", + }).pipe(Effect.catch(() => Effect.succeed({ exitCode: 1, stdout: "", stderr: "" }))); + return result.exitCode === 0; +}); + +export function resolveEdgeRuntimeVersion( + denoVersion: number | undefined, + defaultVersion: string, +): Effect.Effect { + if (denoVersion === undefined || denoVersion === 2) { + return Effect.succeed(defaultVersion); + } + if (denoVersion === 1) { + return Effect.succeed(DENO1_EDGE_RUNTIME_VERSION); + } + return Effect.fail( + new Error(`Failed reading config: Invalid edge_runtime.deno_version: ${denoVersion}.`), + ); +} diff --git a/apps/cli/src/shared/functions/functions.shared.ts b/apps/cli/src/shared/functions/functions.shared.ts index 8c961e867d..731785e7d6 100644 --- a/apps/cli/src/shared/functions/functions.shared.ts +++ b/apps/cli/src/shared/functions/functions.shared.ts @@ -1,3 +1,8 @@ +import { DEFAULT_VERSIONS } from "@supabase/stack/effect"; +import { readFile } from "node:fs/promises"; +import { join } from "node:path"; +import { Effect } from "effect"; + const functionSlugPattern = /^[A-Za-z][A-Za-z0-9_-]*$/; export const invalidFunctionSlugDetail = @@ -16,3 +21,20 @@ export const FUNCTIONS_PROJECT_REF_SAFE_FLAGS = ["project-ref"] as const; // `MarkFlagsMutuallyExclusive("use-api", "use-docker", "legacy-bundle")` // (`cmd/functions.go:158,182`). export const FUNCTIONS_BUNDLER_MUTEX_GROUP = ["use-api", "use-docker", "legacy-bundle"] as const; + +/** + * Go: `Config.EdgeRuntime.Image` reflects `supabase/.temp/edge-runtime-version` + * when present (`pkg/config/config.go:847-849`) — shared by every `functions` + * command that resolves a Docker edge-runtime image (`deploy`, `download`) in + * both shells, so this is the single home for the file-read rather than four + * copies of the same `readFile` -> `trim` -> fallback pipeline. + */ +export const resolveEdgeRuntimeVersionPin = Effect.fnUntraced(function* (supabaseDir: string) { + return yield* Effect.tryPromise(() => + readFile(join(supabaseDir, ".temp", "edge-runtime-version"), "utf8"), + ).pipe( + Effect.map((version) => version.trim()), + Effect.catch(() => Effect.succeed("")), + Effect.map((version) => version || DEFAULT_VERSIONS["edge-runtime"]), + ); +}); diff --git a/apps/cli/src/shared/functions/serve.ts b/apps/cli/src/shared/functions/serve.ts index a4aab1f19f..2970651a19 100644 --- a/apps/cli/src/shared/functions/serve.ts +++ b/apps/cli/src/shared/functions/serve.ts @@ -51,19 +51,21 @@ import { discoverFunctionSlugs, dockerBindContainerPath, dockerBindHostPath, - dockerProjectLabels, dockerWorkdirLabel, + rawFunctionConfigRecord, + resolveFunctionConfigs, + type ResolvedDeployFunctionConfig, +} from "./deploy.ts"; +import { + dockerProjectLabels, ensureDockerNamedVolume, ensureDockerNetwork, localDockerId, normalizeProjectId, - rawFunctionConfigRecord, resolveEdgeRuntimeVersion, - resolveFunctionConfigs, runChildProcess, toDockerPath, - type ResolvedDeployFunctionConfig, -} from "./deploy.ts"; +} from "./functions-docker.ts"; const decodeProjectConfig = Schema.decodeUnknownSync(ProjectConfigSchema); const defaultProjectConfig = decodeProjectConfig({}); diff --git a/apps/cli/src/shared/output/normalize-error.ts b/apps/cli/src/shared/output/normalize-error.ts index 7d0976dd05..c40f72a34e 100644 --- a/apps/cli/src/shared/output/normalize-error.ts +++ b/apps/cli/src/shared/output/normalize-error.ts @@ -194,12 +194,18 @@ export function normalizeCliError( const code = readString(error, "_tag") ?? "UnknownError"; const message = readString(error, "message") ?? readString(error, "detail") ?? code; const detail = readString(error, "detail"); - const suggestion = readString(error, "suggestion"); + // Raw read: some producers' suggestion text is meaningful leading/trailing + // whitespace, not incidental — e.g. `suggestLegacyBundle`'s Go-parity + // string (`shared/functions/download.ts`) starts with `\n` to reproduce + // Go's blank separator line before the hint (`cmd/root.go:301-302`, + // `Fprintln(os.Stderr, CmdSuggestion)`). `readString` would trim exactly + // that away. + const suggestion = readRawString(error, "suggestion"); return { code, message, ...(detail && detail !== message ? { detail } : {}), - ...(suggestion ? { suggestion } : {}), + ...(suggestion !== undefined && suggestion.length > 0 ? { suggestion } : {}), }; } From 265a39ae79a9adf4bb2ccfba22319ec94770877a Mon Sep 17 00:00:00 2001 From: Colum Ferry Date: Wed, 5 Aug 2026 14:56:30 +0100 Subject: [PATCH 02/19] fix(functions): treat empty --network-id as the generated network (review: CLI-1963) Go's DockerStart only overrides the Docker network when len(viper.GetString("network-id")) > 0 (internal/utils/docker.go:379-382). The native functions download/deploy Docker paths used explicitStringFlag(...) ?? localDockerId(...), which returns "" (not undefined) for --network-id=, so an explicit empty override was invoked verbatim instead of falling back to the generated network. Adds explicitNonEmptyStringFlag (cobra-flag-groups.ts), which folds in Go's len(value) > 0 gate, and switches both download.ts and deploy.ts's docker network resolution to it. --- .../download/download.integration.test.ts | 50 ++++++++++++++++++- apps/cli/src/shared/cli/cobra-flag-groups.ts | 22 +++++++- apps/cli/src/shared/functions/deploy.ts | 10 +++- apps/cli/src/shared/functions/download.ts | 11 ++-- 4 files changed, 84 insertions(+), 9 deletions(-) diff --git a/apps/cli/src/legacy/commands/functions/download/download.integration.test.ts b/apps/cli/src/legacy/commands/functions/download/download.integration.test.ts index b58604ab77..d82731f0eb 100644 --- a/apps/cli/src/legacy/commands/functions/download/download.integration.test.ts +++ b/apps/cli/src/legacy/commands/functions/download/download.integration.test.ts @@ -641,8 +641,8 @@ describe("legacy functions download", () => { return Effect.gen(function* () { // `--network-id` is a persistent root flag (`cmd/root.go:328`), not - // registered on `functions download` itself — `explicitStringFlag` - // scans the whole argv unscoped. + // registered on `functions download` itself — + // `explicitNonEmptyStringFlag` scans the whole argv unscoped. yield* legacyFunctionsDownload({ ...baseFlags, useDocker: true }); expect(child.spawned.find((spawned) => spawned.args[0] === "network")).toEqual({ @@ -655,6 +655,52 @@ describe("legacy functions download", () => { }).pipe(Effect.provide(layer)); }); + it.live( + "falls back to the generated network name when --network-id is passed with an empty value", + () => { + // Go only overrides the network when `len(viper.GetString("network-id")) > 0` + // (`internal/utils/docker.go:379-382`) — an explicit-but-empty + // `--network-id=` must fall through to the generated network name just + // like an omitted flag (review round on CLI-1963's `functions download` + // port). + const out = mockOutput({ format: "text" }); + const api = mockLegacyPlatformApi(); + const proxy = mockProxy(); + const child = mockChildProcessSpawner({ exitCode: 0 }); + const layer = Layer.mergeAll( + buildLegacyTestRuntime({ + out, + api, + cliConfig: mockLegacyCliConfig({ workdir: tempRoot.current }), + }), + proxy.layer, + child.layer, + Stdio.layerTest({ + args: Effect.succeed([ + "functions", + "download", + "hello-world", + "--use-docker", + "--project-ref", + PROJECT_ID, + "--network-id=", + ]), + }), + ); + + return Effect.gen(function* () { + yield* legacyFunctionsDownload({ ...baseFlags, useDocker: true }); + + expect(child.spawned.find((spawned) => spawned.args[0] === "network")).toEqual({ + command: "docker", + args: ["network", "inspect", `supabase_network_${PROJECT_ID}`], + }); + const runCommand = child.spawned.find((spawned) => spawned.args[0] === "run"); + expect(runCommand?.args).toContain(`supabase_network_${PROJECT_ID}`); + }).pipe(Effect.provide(layer)); + }, + ); + it.live("keeps the temporary eszip file when --debug is passed", () => { const out = mockOutput({ format: "text" }); const api = mockLegacyPlatformApi(); diff --git a/apps/cli/src/shared/cli/cobra-flag-groups.ts b/apps/cli/src/shared/cli/cobra-flag-groups.ts index 2ec3d23d3d..c07d1b4592 100644 --- a/apps/cli/src/shared/cli/cobra-flag-groups.ts +++ b/apps/cli/src/shared/cli/cobra-flag-groups.ts @@ -30,9 +30,12 @@ export function hasExplicitLongFlag( /** * Raw value of `--`/`--=value` anywhere in argv - * (unscoped — no command-path anchoring), or `undefined` if absent. + * (unscoped — no command-path anchoring), or `undefined` if absent. Not + * exported — every current call site needs Go's `len(value) > 0` gate too + * (see {@link explicitNonEmptyStringFlag}); re-export this directly if a + * future caller genuinely needs presence-only semantics. */ -export function explicitStringFlag(rawArgs: ReadonlyArray, flagName: string) { +function explicitStringFlag(rawArgs: ReadonlyArray, flagName: string) { for (let index = 0; index < rawArgs.length; index += 1) { const token = rawArgs[index]; if (token === `--${flagName}`) { @@ -45,6 +48,21 @@ export function explicitStringFlag(rawArgs: ReadonlyArray, flagName: str return undefined; } +/** + * Same as {@link explicitStringFlag}, but treats an explicit empty value + * (`--=`) as unset — matching Go call sites that gate on + * `len(viper.GetString(flagName)) > 0` rather than mere presence (e.g. + * `--network-id`, `apps/cli-go/internal/utils/docker.go:379-382`). pflag + * still marks the flag `Changed` for `--network-id=`, but Go's own + * `if networkId := viper.GetString("network-id"); len(networkId) > 0` + * falls through to the generated network name for that value just like an + * omitted flag would (review round on CLI-1963's `functions download` port). + */ +export function explicitNonEmptyStringFlag(rawArgs: ReadonlyArray, flagName: string) { + const value = explicitStringFlag(rawArgs, flagName); + return value !== undefined && value.length > 0 ? value : undefined; +} + /** * Whether `--` (or `--=`) appears anywhere in argv, * unscoped. diff --git a/apps/cli/src/shared/functions/deploy.ts b/apps/cli/src/shared/functions/deploy.ts index 7d59849035..3295813801 100644 --- a/apps/cli/src/shared/functions/deploy.ts +++ b/apps/cli/src/shared/functions/deploy.ts @@ -18,7 +18,7 @@ import { legacyGetRegistryImageUrl } from "../../legacy/shared/legacy-docker-reg import { findGitRootPath } from "../git/git-root.ts"; import { cobraMutuallyExclusiveErrorMessage, - explicitStringFlag, + explicitNonEmptyStringFlag, hasExplicitLongFlag, hasGlobalLongFlag, } from "../cli/cobra-flag-groups.ts"; @@ -2201,7 +2201,13 @@ export function deployFunctions( join(dependencies.projectRoot, SUPABASE_FUNCTIONS_DIR), configs, dependencies.api, - explicitStringFlag(dependencies.rawArgs, "network-id"), + // Go only treats `--network-id` as an override when + // `len(viper.GetString("network-id")) > 0` + // (`internal/utils/docker.go:379-382`) — an explicit-but-empty + // `--network-id=` must fall through to the generated network + // name (`dockerNetworkId?: string` → `undefined`) just like an + // omitted flag. + explicitNonEmptyStringFlag(dependencies.rawArgs, "network-id"), debugEnabled, styleEmphasis, ); diff --git a/apps/cli/src/shared/functions/download.ts b/apps/cli/src/shared/functions/download.ts index bff0926d2c..be624872d5 100644 --- a/apps/cli/src/shared/functions/download.ts +++ b/apps/cli/src/shared/functions/download.ts @@ -10,7 +10,7 @@ import type * as HttpClientResponse from "effect/unstable/http/HttpClientRespons import { Output } from "../output/output.service.ts"; import { cobraMutuallyExclusiveErrorMessage, - explicitStringFlag, + explicitNonEmptyStringFlag, hasExplicitLongFlag, hasGlobalLongFlag, } from "../cli/cobra-flag-groups.ts"; @@ -927,9 +927,14 @@ const downloadWithDockerUnbundle = Effect.fnUntraced(function* ( // Go: `viper.GetString("network-id")` else `NetId` (`docker.go:379-383`) — // `--network-id` is a persistent root flag (`cmd/root.go:328`), not - // registered on `functions download` itself. + // registered on `functions download` itself. Go only treats the override as + // set when `len(networkId) > 0`, so an explicit-but-empty `--network-id=` + // must fall through to the generated network name too, not just an omitted + // flag — `explicitNonEmptyStringFlag` (unlike the unexported + // `explicitStringFlag`) treats that case as unset for exactly this reason. const networkMode = - explicitStringFlag(dependencies.rawArgs, "network-id") ?? localDockerId("network", projectId); + explicitNonEmptyStringFlag(dependencies.rawArgs, "network-id") ?? + localDockerId("network", projectId); const extract = Effect.gen(function* () { yield* ensureDockerNetwork(networkMode, projectId).pipe( From 810323314797d6c8e1ee57933358c9cdf9b92b0c Mon Sep 17 00:00:00 2001 From: Colum Ferry Date: Wed, 5 Aug 2026 14:59:21 +0100 Subject: [PATCH 03/19] fix(functions): preserve v-prefixed edge-runtime-version pins (review: CLI-1963) Go's replaceImageTag (pkg/config/utils.go:81-84) appends the raw content of supabase/.temp/edge-runtime-version verbatim after the image's `:`, so a pin can legitimately already carry its own `v` prefix (both forms are exercised elsewhere in this codebase, e.g. legacy-edge-runtime-image.unit.test.ts's "v9.9.9" fixture vs. deploy.integration.test.ts's bare "9.9.9"). The native download Docker path always prepended `v` to the resolved version, so a v-prefixed pin produced `supabase/edge-runtime:vv9.9.9`, which Docker fails to pull. Hoists serve.ts's existing edgeRuntimeImageTag helper (which already handled this correctly) into the shared functions-docker.ts, and applies it in download.ts and deploy.ts, which had the same unprefixed-vs-prefixed bug in their own inline `v${version}` construction. --- .../download/download.integration.test.ts | 46 +++++++++++++++++++ apps/cli/src/shared/functions/deploy.ts | 7 ++- apps/cli/src/shared/functions/download.ts | 8 +++- .../src/shared/functions/functions-docker.ts | 18 ++++++++ apps/cli/src/shared/functions/serve.ts | 5 +- 5 files changed, 78 insertions(+), 6 deletions(-) diff --git a/apps/cli/src/legacy/commands/functions/download/download.integration.test.ts b/apps/cli/src/legacy/commands/functions/download/download.integration.test.ts index d82731f0eb..71278992f8 100644 --- a/apps/cli/src/legacy/commands/functions/download/download.integration.test.ts +++ b/apps/cli/src/legacy/commands/functions/download/download.integration.test.ts @@ -701,6 +701,52 @@ describe("legacy functions download", () => { }, ); + it.live("does not double-prefix an already v-prefixed edge-runtime-version pin", () => { + // Go's `replaceImageTag` (`pkg/config/utils.go:81-84`) appends the pin + // file's raw content verbatim after the image's `:`, so a pin already + // carrying its own `v` prefix (a legitimate form — see + // `legacy-edge-runtime-image.unit.test.ts`'s own `"v9.9.9"` fixture) must + // not be prepended with a second `v` (review round on CLI-1963's + // `functions download` port). + const out = mockOutput({ format: "text" }); + const api = mockLegacyPlatformApi(); + const proxy = mockProxy(); + const child = mockChildProcessSpawner({ exitCode: 0 }); + const layer = Layer.mergeAll( + buildLegacyTestRuntime({ + out, + api, + cliConfig: mockLegacyCliConfig({ workdir: tempRoot.current }), + }), + proxy.layer, + child.layer, + Stdio.layerTest({ + args: Effect.succeed([ + "functions", + "download", + "hello-world", + "--use-docker", + "--project-ref", + PROJECT_ID, + ]), + }), + ); + + return Effect.gen(function* () { + yield* Effect.tryPromise(() => + mkdir(join(tempRoot.current, "supabase", ".temp"), { recursive: true }), + ); + yield* Effect.tryPromise(() => + writeFile(join(tempRoot.current, "supabase", ".temp", "edge-runtime-version"), "v9.9.9\n"), + ); + + yield* legacyFunctionsDownload({ ...baseFlags, useDocker: true }); + + const runCommand = child.spawned.find((spawned) => spawned.args[0] === "run"); + expect(runCommand?.args.slice(-6)[0]).toBe("public.ecr.aws/supabase/edge-runtime:v9.9.9"); + }).pipe(Effect.provide(layer)); + }); + it.live("keeps the temporary eszip file when --debug is passed", () => { const out = mockOutput({ format: "text" }); const api = mockLegacyPlatformApi(); diff --git a/apps/cli/src/shared/functions/deploy.ts b/apps/cli/src/shared/functions/deploy.ts index 3295813801..9ad0320e8e 100644 --- a/apps/cli/src/shared/functions/deploy.ts +++ b/apps/cli/src/shared/functions/deploy.ts @@ -34,6 +34,7 @@ import { NoFunctionsToDeployError, } from "./deploy.errors.ts"; import { + edgeRuntimeImageTag, ensureDockerNamedVolume, ensureDockerNetwork, isDockerRunning, @@ -1279,7 +1280,11 @@ const bundleFunctionWithDocker = Effect.fnUntraced(function* ( } command.push( - legacyGetRegistryImageUrl(`supabase/edge-runtime:v${edgeRuntimeVersion}`), + // `edgeRuntimeImageTag`, not a bare `v${edgeRuntimeVersion}` prepend — + // `edgeRuntimeVersion` can come from a `.temp/edge-runtime-version` pin + // that's already `v`-prefixed (see the helper's doc in + // `functions-docker.ts`); blindly prepending `v` double-prefixes it. + legacyGetRegistryImageUrl(`supabase/edge-runtime:${edgeRuntimeImageTag(edgeRuntimeVersion)}`), "bundle", "--entrypoint", toDockerPath(config.entrypoint), diff --git a/apps/cli/src/shared/functions/download.ts b/apps/cli/src/shared/functions/download.ts index be624872d5..d767497a3b 100644 --- a/apps/cli/src/shared/functions/download.ts +++ b/apps/cli/src/shared/functions/download.ts @@ -17,6 +17,7 @@ import { import { legacyDescribeContainerCliFailure } from "../../legacy/shared/legacy-container-cli.ts"; import { legacyGetRegistryImageUrl } from "../../legacy/shared/legacy-docker-registry.ts"; import { + edgeRuntimeImageTag, ensureDockerNamedVolume, ensureDockerNetwork, isDockerRunning, @@ -857,7 +858,12 @@ const resolveEdgeRuntimeImage = Effect.fnUntraced(function* ( return { projectId, denoVersion, - image: legacyGetRegistryImageUrl(`supabase/edge-runtime:v${edgeRuntimeVersion}`), + // `edgeRuntimeImageTag` (not a bare `v${edgeRuntimeVersion}` prepend) — + // `dependencies.edgeRuntimeVersion` comes from a `.temp/edge-runtime-version` + // pin that may already carry its own `v` prefix (see the helper's doc). + image: legacyGetRegistryImageUrl( + `supabase/edge-runtime:${edgeRuntimeImageTag(edgeRuntimeVersion)}`, + ), }; }); diff --git a/apps/cli/src/shared/functions/functions-docker.ts b/apps/cli/src/shared/functions/functions-docker.ts index 19712a790f..8862964ac0 100644 --- a/apps/cli/src/shared/functions/functions-docker.ts +++ b/apps/cli/src/shared/functions/functions-docker.ts @@ -186,3 +186,21 @@ export function resolveEdgeRuntimeVersion( new Error(`Failed reading config: Invalid edge_runtime.deno_version: ${denoVersion}.`), ); } + +/** + * Formats a resolved edge-runtime version as a Docker tag, tolerating a + * pin that's already `v`-prefixed. `resolveEdgeRuntimeVersion`'s own + * defaults are bare (`"1.74.2"`, `DENO1_EDGE_RUNTIME_VERSION`), but a value + * sourced from `supabase/.temp/edge-runtime-version` can legitimately be + * either form — Go's `replaceImageTag` (`pkg/config/utils.go:81-84`) appends + * the pin file's raw content verbatim after the image's `:`, and both forms + * are exercised elsewhere in this codebase (`legacy-edge-runtime-image.ts`'s + * own `replaceImageTag` port, and its and `services.integration.test.ts`'s + * `"v9.9.9"` fixtures alongside `deploy.integration.test.ts`'s bare + * `"9.9.9"`). Blindly prepending `v` — as every caller below did before this + * helper existed — double-prefixes an already-`v`-prefixed pin + * (`supabase/edge-runtime:vv9.9.9`), which docker then simply fails to pull. + */ +export function edgeRuntimeImageTag(version: string): string { + return version.startsWith("v") ? version : `v${version}`; +} diff --git a/apps/cli/src/shared/functions/serve.ts b/apps/cli/src/shared/functions/serve.ts index 2970651a19..725e27c70f 100644 --- a/apps/cli/src/shared/functions/serve.ts +++ b/apps/cli/src/shared/functions/serve.ts @@ -58,6 +58,7 @@ import { } from "./deploy.ts"; import { dockerProjectLabels, + edgeRuntimeImageTag, ensureDockerNamedVolume, ensureDockerNetwork, localDockerId, @@ -1382,10 +1383,6 @@ async function writeServeMainTemplateFile(template: string, dir: string) { return { bind: `${pathname}:${serveMainContainerPath}:ro,Z` } as const; } -function edgeRuntimeImageTag(version: string) { - return version.startsWith("v") ? version : `v${version}`; -} - const resolveServeFunctionConfigs = Effect.fnUntraced(function* ( projectRoot: string, supabaseDir: string, From 5583c93b83cf4b12081cbc977aa784b44e1942dc Mon Sep 17 00:00:00 2001 From: Colum Ferry Date: Wed, 5 Aug 2026 15:00:44 +0100 Subject: [PATCH 04/19] fix(functions): request the raw eszip body instead of a negotiated JSON response (review: CLI-1963) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit v1GetAFunctionBody's generated contract marks its response kind: "json", so executeRaw() defaults to Accept: application/json for it (buildRequest's unconditional acceptJson for json-kind operations). Go's own downloadOne (the Docker-unbundle path this mirrors) sends no Accept header at all, unlike the server-side path's explicit multipart/form-data override, so the default JSON negotiation here could receive a negotiated JSON response instead of the raw eszip bytes and fail downstream in edge-runtime unbundle. Overrides the request's Accept header to */* (no preference) — the closest equivalent this API surface has to Go sending no header. --- .../download/download.integration.test.ts | 39 +++++++++++++++ apps/cli/src/shared/functions/download.ts | 47 ++++++++++++------- 2 files changed, 68 insertions(+), 18 deletions(-) diff --git a/apps/cli/src/legacy/commands/functions/download/download.integration.test.ts b/apps/cli/src/legacy/commands/functions/download/download.integration.test.ts index 71278992f8..5e7542f130 100644 --- a/apps/cli/src/legacy/commands/functions/download/download.integration.test.ts +++ b/apps/cli/src/legacy/commands/functions/download/download.integration.test.ts @@ -612,6 +612,45 @@ describe("legacy functions download", () => { }).pipe(Effect.provide(layer)); }); + it.live("requests the raw eszip body instead of a negotiated JSON response", () => { + // `v1GetAFunctionBody`'s generated contract marks its response + // `kind: "json"`, so `executeRaw` would otherwise default to + // `Accept: application/json` (`buildRequest`'s unconditional `acceptJson` + // for json-kind operations) and risk a negotiated JSON response instead + // of the raw eszip body Go's un-overridden request receives (review round + // on CLI-1963's `functions download` port). + const out = mockOutput({ format: "text" }); + const api = mockLegacyPlatformApi(); + const proxy = mockProxy(); + const child = mockChildProcessSpawner({ exitCode: 0 }); + const layer = Layer.mergeAll( + buildLegacyTestRuntime({ + out, + api, + cliConfig: mockLegacyCliConfig({ workdir: tempRoot.current }), + }), + proxy.layer, + child.layer, + Stdio.layerTest({ + args: Effect.succeed([ + "functions", + "download", + "hello-world", + "--use-docker", + "--project-ref", + PROJECT_ID, + ]), + }), + ); + + return Effect.gen(function* () { + yield* legacyFunctionsDownload({ ...baseFlags, useDocker: true }); + + const bodyRequest = api.requests.find((request) => request.url.endsWith("/hello-world/body")); + expect(bodyRequest?.headers["accept"]).toBe("*/*"); + }).pipe(Effect.provide(layer)); + }); + it.live("uses an explicit --network-id override instead of the derived network name", () => { const out = mockOutput({ format: "text" }); const api = mockLegacyPlatformApi(); diff --git a/apps/cli/src/shared/functions/download.ts b/apps/cli/src/shared/functions/download.ts index d767497a3b..05179145bd 100644 --- a/apps/cli/src/shared/functions/download.ts +++ b/apps/cli/src/shared/functions/download.ts @@ -744,30 +744,41 @@ const downloadBody = Effect.fnUntraced(function* ( }); // Go: `downloadOne` (`apps/cli-go/internal/functions/download/download.go:218-245`) -// — no `Accept` override (contrast `downloadBody` above, which requests -// `multipart/form-data` for the server-side path). Go explicitly decodes a -// brotli `Content-Encoding` itself because Go's `http.Transport` only -// auto-decodes `gzip`; this TS CLI's transport (`effect/unstable/http`'s -// `FetchHttpClient`, backed by the platform `fetch`) already transparently -// decodes `br` per the Fetch spec — while still reporting -// `Content-Encoding: br` on the exposed `Response.headers` (confirmed -// empirically: a `fetch()` against a real `Content-Encoding: br` response -// returns already-decompressed bytes from `arrayBuffer()`). Re-running -// `brotliDecompressSync` here would therefore throw on already-decoded -// bytes, so this reads the body as-is and does not re-implement Go's manual -// decode step. Error prefix ("failed to get function body") is deliberately -// distinct from `downloadBody`'s ("failed to download function") — the two -// Go call sites use different wording. +// sends this request with no `Accept` header set at all (contrast +// `downloadBody` above, which requests `multipart/form-data` for the +// server-side path). This operation's generated contract marks its response +// `kind: "json"` (`packages/api/src/generated/contracts.ts`), so +// `executeRaw` would otherwise default to `Accept: application/json` here +// (`buildRequest`'s unconditional `acceptJson` for json-kind operations, +// `packages/api/src/internal/client.ts`) and risk a negotiated JSON response +// instead of the raw eszip body — overriding to `*/*` (no preference) is the +// closest equivalent this API surface has to Go sending no header at all. +// Go explicitly decodes a brotli `Content-Encoding` itself because Go's +// `http.Transport` only auto-decodes `gzip`; this TS CLI's transport +// (`effect/unstable/http`'s `FetchHttpClient`, backed by the platform +// `fetch`) already transparently decodes `br` per the Fetch spec — while +// still reporting `Content-Encoding: br` on the exposed `Response.headers` +// (confirmed empirically: a `fetch()` against a real `Content-Encoding: br` +// response returns already-decompressed bytes from `arrayBuffer()`). +// Re-running `brotliDecompressSync` here would therefore throw on +// already-decoded bytes, so this reads the body as-is and does not +// re-implement Go's manual decode step. Error prefix ("failed to get +// function body") is deliberately distinct from `downloadBody`'s ("failed to +// download function") — the two Go call sites use different wording. const downloadEszipBody = Effect.fnUntraced(function* ( api: ApiClient, projectRef: string, slug: string, ) { const response = yield* api - .executeRaw(operationDefinitions.v1GetAFunctionBody, { - ref: projectRef, - function_slug: slug, - }) + .executeRaw( + operationDefinitions.v1GetAFunctionBody, + { + ref: projectRef, + function_slug: slug, + }, + { Accept: "*/*" }, + ) .pipe(Effect.mapError((error) => mapTransportError("failed to get function body", error))); if (response.status !== 200) { From bc765ee4234916edf7cec9598b61ebcf415f97ab Mon Sep 17 00:00:00 2001 From: Colum Ferry Date: Wed, 5 Aug 2026 15:02:16 +0100 Subject: [PATCH 05/19] fix(functions): validate project config before falling back from Docker (review: CLI-1963) Go's Run calls flags.LoadConfig(fsys) unconditionally at the very top, before checking useDocker or whether Docker itself is running (download.go:135-138). The native download path only resolved/validated the project config (via resolveEdgeRuntimeImage) inside the isDockerRunning() branch, so a default `functions download` with an invalid edge_runtime.deno_version proceeded straight to the API/filesystem side-effecting server-side path whenever Docker was down or --use-api was passed, instead of failing up front like Go. Resolves resolveEdgeRuntimeImage unconditionally before branching on --use-api/--use-docker/Docker's running state. --- .../download/download.integration.test.ts | 57 +++++++++++++++++++ apps/cli/src/shared/functions/download.ts | 21 +++++-- 2 files changed, 72 insertions(+), 6 deletions(-) diff --git a/apps/cli/src/legacy/commands/functions/download/download.integration.test.ts b/apps/cli/src/legacy/commands/functions/download/download.integration.test.ts index 5e7542f130..23ded13dae 100644 --- a/apps/cli/src/legacy/commands/functions/download/download.integration.test.ts +++ b/apps/cli/src/legacy/commands/functions/download/download.integration.test.ts @@ -821,6 +821,63 @@ describe("legacy functions download", () => { }).pipe(Effect.provide(layer)); }); + it.live( + "fails on an invalid project config before falling back when Docker is not running", + () => { + // Go's `Run` calls `flags.LoadConfig(fsys)` unconditionally at the very + // top, before checking whether Docker is running (`download.go:135-138`) + // — an invalid `supabase/config.toml` must fail up front instead of + // silently falling through to the server-side path's API/filesystem + // side effects (review round on CLI-1963's `functions download` port). + const out = mockOutput({ format: "text" }); + const api = mockLegacyPlatformApi(); + const proxy = mockProxy(); + // Every docker command (including the `docker info` probe) fails, + // modeling Docker not running. + const child = mockChildProcessSpawner({ exitCode: 1 }); + const layer = Layer.mergeAll( + buildLegacyTestRuntime({ + out, + api, + cliConfig: mockLegacyCliConfig({ workdir: tempRoot.current }), + }), + proxy.layer, + child.layer, + Stdio.layerTest({ + args: Effect.succeed([ + "functions", + "download", + "hello-world", + "--project-ref", + PROJECT_ID, + ]), + }), + ); + + return Effect.gen(function* () { + yield* Effect.tryPromise(() => + mkdir(join(tempRoot.current, "supabase"), { recursive: true }), + ); + yield* Effect.tryPromise(() => + writeFile( + join(tempRoot.current, "supabase", "config.toml"), + ["[edge_runtime]", "deno_version = 3", ""].join("\n"), + ), + ); + + const error = yield* legacyFunctionsDownload({ ...baseFlags, useDocker: true }).pipe( + Effect.flip, + ); + + expect(error).toBeInstanceOf(Error); + expect((error as Error).message).toBe( + "Failed reading config: Invalid edge_runtime.deno_version: 3.", + ); + expect(api.requests).toEqual([]); + }).pipe(Effect.provide(layer)); + }, + ); + describe("docker unbundle container failures", () => { it.live("fails with the legacy-bundle suggestion when the container exits non-zero", () => { const out = mockOutput({ format: "text" }); diff --git a/apps/cli/src/shared/functions/download.ts b/apps/cli/src/shared/functions/download.ts index 05179145bd..1ea6e10a08 100644 --- a/apps/cli/src/shared/functions/download.ts +++ b/apps/cli/src/shared/functions/download.ts @@ -1174,6 +1174,19 @@ export function downloadFunctions Date: Wed, 5 Aug 2026 15:03:43 +0100 Subject: [PATCH 06/19] fix(functions): skip the named Deno cache volume bind on Bitbucket (review: CLI-1963) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Go's DockerStart drops the named-volume bind entirely on Bitbucket (internal/utils/docker.go:400-405) rather than just skipping its explicit creation — `docker run -v :...` would otherwise still implicitly create the named volume, which Bitbucket's restricted Docker environment doesn't allow. The native Docker-unbundle path's ensureDockerNamedVolume already skipped the explicit `docker volume create` under BITBUCKET_CLONE_DIR, but the manually-built `docker run -v ...` bind list still unconditionally included the named-volume bind, so the container run itself could still fail in Bitbucket's restricted environment. Applies the same BITBUCKET_CLONE_DIR carve-out deploy.ts's buildDockerBinds already uses. --- .../download/download.integration.test.ts | 66 +++++++++++++++++++ apps/cli/src/shared/functions/download.ts | 12 +++- 2 files changed, 76 insertions(+), 2 deletions(-) diff --git a/apps/cli/src/legacy/commands/functions/download/download.integration.test.ts b/apps/cli/src/legacy/commands/functions/download/download.integration.test.ts index 23ded13dae..e73d81ab06 100644 --- a/apps/cli/src/legacy/commands/functions/download/download.integration.test.ts +++ b/apps/cli/src/legacy/commands/functions/download/download.integration.test.ts @@ -612,6 +612,72 @@ describe("legacy functions download", () => { }).pipe(Effect.provide(layer)); }); + it.live("omits the named Deno cache volume bind on Bitbucket", () => { + // Go's `DockerStart` drops the named-volume bind entirely on Bitbucket + // (`internal/utils/docker.go:400-405`) rather than just skipping its + // explicit creation — `docker run -v :...` would otherwise still + // implicitly create the named volume, which Bitbucket's restricted Docker + // environment doesn't allow (review round on CLI-1963's `functions + // download` port; `deploy.ts`'s `buildDockerBinds` already applies this + // same carve-out). + const out = mockOutput({ format: "text" }); + const api = mockLegacyPlatformApi(); + const proxy = mockProxy(); + const child = mockChildProcessSpawner({ exitCode: 0 }); + const layer = Layer.mergeAll( + buildLegacyTestRuntime({ + out, + api, + cliConfig: mockLegacyCliConfig({ workdir: tempRoot.current }), + }), + proxy.layer, + child.layer, + Stdio.layerTest({ + args: Effect.succeed([ + "functions", + "download", + "hello-world", + "--use-docker", + "--project-ref", + PROJECT_ID, + ]), + }), + ); + + const previousBitbucketCloneDir = process.env["BITBUCKET_CLONE_DIR"]; + process.env["BITBUCKET_CLONE_DIR"] = "/opt/atlassian/pipelines/agent/build"; + + return Effect.gen(function* () { + yield* legacyFunctionsDownload({ ...baseFlags, useDocker: true }); + + const runCommand = child.spawned.find((spawned) => spawned.args[0] === "run"); + expect(runCommand?.args).not.toContain( + `supabase_edge_runtime_${PROJECT_ID}:/root/.cache/deno:rw`, + ); + const hostEszipPath = resolve( + tempRoot.current, + "supabase", + ".temp", + "output_hello-world.eszip", + ); + expect(runCommand?.args).toContain( + `${hostEszipPath}:/root/eszips/output_hello-world.eszip:ro`, + ); + }) + .pipe(Effect.provide(layer)) + .pipe( + Effect.ensuring( + Effect.sync(() => { + if (previousBitbucketCloneDir === undefined) { + delete process.env["BITBUCKET_CLONE_DIR"]; + } else { + process.env["BITBUCKET_CLONE_DIR"] = previousBitbucketCloneDir; + } + }), + ), + ); + }); + it.live("requests the raw eszip body instead of a negotiated JSON response", () => { // `v1GetAFunctionBody`'s generated contract marks its response // `kind: "json"`, so `executeRaw` would otherwise default to diff --git a/apps/cli/src/shared/functions/download.ts b/apps/cli/src/shared/functions/download.ts index 1ea6e10a08..da35f8f3e9 100644 --- a/apps/cli/src/shared/functions/download.ts +++ b/apps/cli/src/shared/functions/download.ts @@ -961,9 +961,17 @@ const downloadWithDockerUnbundle = Effect.fnUntraced(function* ( Effect.mapError(withLegacyBundleSuggestion(slug)), ); - // Bind order matches `extractOne` (`download.go:260-266`) exactly. + // Bind order matches `extractOne` (`download.go:260-266`) exactly. Go's + // `DockerStart` drops the named-volume bind entirely on Bitbucket + // (`internal/utils/docker.go:400-405`) rather than just skipping its + // explicit creation — `docker run -v :...` would otherwise still + // implicitly create the named volume, which Bitbucket's restricted Docker + // environment doesn't allow, same carve-out as `deploy.ts`'s + // `buildDockerBinds`. const binds = [ - `${localDockerId("edge_runtime", projectId)}:/root/.cache/deno:rw`, + ...(process.env["BITBUCKET_CLONE_DIR"] === undefined + ? [`${localDockerId("edge_runtime", projectId)}:/root/.cache/deno:rw`] + : []), `${hostEszipPath}:${dockerEszipPath}:ro`, `${functionsDir}:${DOCKER_DENO_DIR}:rw`, ]; From 51524c620657e7b715b0204b9d74e27d0f213ef5 Mon Sep 17 00:00:00 2001 From: Colum Ferry Date: Wed, 5 Aug 2026 15:05:49 +0100 Subject: [PATCH 07/19] fix(functions): honor pflag boolean value for --debug in eszip cleanup (review: CLI-1963) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Go gates the Docker-unbundle path's temp-eszip cleanup on viper.GetBool("DEBUG") (download.go:203), so an explicit --debug=false resolves to false (cleanup runs). The native path used hasGlobalLongFlag(rawArgs, "debug"), a presence-only check, so --debug=false was treated the same as --debug and skipped cleanup — the opposite of Go. Adds explicitBooleanLongFlag (cobra-flag-groups.ts), which reads the last explicit occurrence's pflag-parsed boolean value instead of mere presence, and switches this call site to it. SUPABASE_DEBUG env-var fallback remains a separate, pre-existing gap shared by every other hasGlobalLongFlag(rawArgs, "debug") site (e.g. deploy.ts) and the legacy debug logger, left open rather than fixed piecemeal here. --- .../download/download.integration.test.ts | 42 +++++++++++++++++++ apps/cli/src/shared/cli/cobra-flag-groups.ts | 36 ++++++++++++++++ apps/cli/src/shared/functions/download.ts | 14 ++++++- 3 files changed, 90 insertions(+), 2 deletions(-) diff --git a/apps/cli/src/legacy/commands/functions/download/download.integration.test.ts b/apps/cli/src/legacy/commands/functions/download/download.integration.test.ts index e73d81ab06..6bbdbd3f6d 100644 --- a/apps/cli/src/legacy/commands/functions/download/download.integration.test.ts +++ b/apps/cli/src/legacy/commands/functions/download/download.integration.test.ts @@ -887,6 +887,48 @@ describe("legacy functions download", () => { }).pipe(Effect.provide(layer)); }); + it.live( + "removes the temporary eszip file when --debug=false overrides the flag's own presence", + () => { + // Go gates this on `viper.GetBool("DEBUG")`, so an explicit + // `--debug=false` resolves to `false` (cleanup runs) — a presence-only + // check would get this backwards and treat `--debug=false` like + // `--debug` (review round on CLI-1963's `functions download` port). + const out = mockOutput({ format: "text" }); + const api = mockLegacyPlatformApi(); + const proxy = mockProxy(); + const child = mockChildProcessSpawner({ exitCode: 0 }); + const layer = Layer.mergeAll( + buildLegacyTestRuntime({ + out, + api, + cliConfig: mockLegacyCliConfig({ workdir: tempRoot.current }), + }), + proxy.layer, + child.layer, + Stdio.layerTest({ + args: Effect.succeed([ + "functions", + "download", + "hello-world", + "--use-docker", + "--project-ref", + PROJECT_ID, + "--debug=false", + ]), + }), + ); + + return Effect.gen(function* () { + yield* legacyFunctionsDownload({ ...baseFlags, useDocker: true }); + + expect( + existsSync(join(tempRoot.current, "supabase", ".temp", "output_hello-world.eszip")), + ).toBe(false); + }).pipe(Effect.provide(layer)); + }, + ); + it.live( "fails on an invalid project config before falling back when Docker is not running", () => { diff --git a/apps/cli/src/shared/cli/cobra-flag-groups.ts b/apps/cli/src/shared/cli/cobra-flag-groups.ts index c07d1b4592..46975b2fa6 100644 --- a/apps/cli/src/shared/cli/cobra-flag-groups.ts +++ b/apps/cli/src/shared/cli/cobra-flag-groups.ts @@ -71,6 +71,42 @@ export function hasGlobalLongFlag(rawArgs: ReadonlyArray, flagName: stri return rawArgs.some((token) => token === `--${flagName}` || token.startsWith(`--${flagName}=`)); } +const PFLAG_BOOLEAN_FALSE_VALUES: ReadonlySet = new Set([ + "0", + "f", + "F", + "false", + "FALSE", + "False", +]); + +/** + * Last explicit `--`/`--=` boolean occurrence in + * argv, or `undefined` when the flag never appears — matching pflag/viper's + * shared-variable last-`Set()`-wins semantics (mirrors + * `legacyExperimentalFlagFromArgs`, `shared/legacy/global-flags.ts`). A bare + * `--` records pflag's bool `NoOptDefVal` (`true`); an inline value + * is parsed through pflag's `strconv.ParseBool` false set — anything else + * (including garbage) is truthy, same as `cast.ToBool`'s permissive default. + * Unlike {@link hasGlobalLongFlag}, this distinguishes `--=false` + * from presence alone, which matters for Go call sites gated on + * `viper.GetBool` rather than "was the flag passed at all". + */ +export function explicitBooleanLongFlag( + rawArgs: ReadonlyArray, + flagName: string, +): boolean | undefined { + let result: boolean | undefined; + for (const token of rawArgs) { + if (token === `--${flagName}`) { + result = true; + } else if (token.startsWith(`--${flagName}=`)) { + result = !PFLAG_BOOLEAN_FALSE_VALUES.has(token.slice(flagName.length + 3)); + } + } + return result; +} + /** * Value-taking long flags registered persistently on the Go root command * (`apps/cli-go/cmd/root.go:324-333`: `--workdir`, `--network-id`, diff --git a/apps/cli/src/shared/functions/download.ts b/apps/cli/src/shared/functions/download.ts index da35f8f3e9..d4b18e84f5 100644 --- a/apps/cli/src/shared/functions/download.ts +++ b/apps/cli/src/shared/functions/download.ts @@ -10,9 +10,9 @@ import type * as HttpClientResponse from "effect/unstable/http/HttpClientRespons import { Output } from "../output/output.service.ts"; import { cobraMutuallyExclusiveErrorMessage, + explicitBooleanLongFlag, explicitNonEmptyStringFlag, hasExplicitLongFlag, - hasGlobalLongFlag, } from "../cli/cobra-flag-groups.ts"; import { legacyDescribeContainerCliFailure } from "../../legacy/shared/legacy-container-cli.ts"; import { legacyGetRegistryImageUrl } from "../../legacy/shared/legacy-docker-registry.ts"; @@ -928,7 +928,17 @@ const downloadWithDockerUnbundle = Effect.fnUntraced(function* ( // wraps every step from here on so a failure resolving the network/volume, // spawning Docker, or a non-zero container exit all still clean up the // temp eszip, matching Go instead of only doing so on the happy path. - const debugEnabled = hasGlobalLongFlag(dependencies.rawArgs, "debug"); + // + // Go gates this on `viper.GetBool("DEBUG")` (`download.go:203`), which + // resolves an explicit `--debug=false` to `false` (cleanup runs) — a plain + // presence check would get that backwards, so this reads the last explicit + // occurrence's boolean value instead (`explicitBooleanLongFlag`), falling + // back to `false` (cleanup runs) when `--debug` never appears. `SUPABASE_DEBUG` + // env-var fallback is a separate, pre-existing gap shared with every other + // `hasGlobalLongFlag(rawArgs, "debug")` call site in this file family + // (e.g. `deploy.ts`) and the legacy debug logger itself, none of which + // currently honor it either — left open rather than fixed piecemeal here. + const debugEnabled = explicitBooleanLongFlag(dependencies.rawArgs, "debug") ?? false; const cleanupEszip = debugEnabled ? Effect.void : Effect.tryPromise({ From b88cf6a1302b55f90b8a24e7edc1872b369b0d9e Mon Sep 17 00:00:00 2001 From: Colum Ferry Date: Wed, 5 Aug 2026 16:57:00 +0100 Subject: [PATCH 08/19] fix(functions): treat container: as a non-user-defined docker network mode (review: CLI-1963) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Go's container.NetworkMode.IsUserDefined() explicitly excludes IsContainer() (docker/api/types/container/hostconfig_unix.go:23-25), so DockerNetworkCreateIfNotExists never inspects or creates a network for --network-id container: — the mode attaches to another container's stack and is passed straight through to `docker run --network`. The shared isUserDefinedDockerNetwork predicate (used by deploy.ts, serve.ts, download.ts, and start's container lifecycle) didn't exclude this case, so the Docker download path's preflight would have run `docker network inspect`/`create container:redis` before `docker run`. Fixed once in the shared predicate so every consumer gets the same fix. --- .../download/download.integration.test.ts | 43 +++++++++++++++++++ .../lib/container-lifecycle.unit.test.ts | 16 +++++++ .../src/shared/functions/functions-docker.ts | 18 +++++++- 3 files changed, 76 insertions(+), 1 deletion(-) diff --git a/apps/cli/src/legacy/commands/functions/download/download.integration.test.ts b/apps/cli/src/legacy/commands/functions/download/download.integration.test.ts index 6bbdbd3f6d..861784cf37 100644 --- a/apps/cli/src/legacy/commands/functions/download/download.integration.test.ts +++ b/apps/cli/src/legacy/commands/functions/download/download.integration.test.ts @@ -806,6 +806,49 @@ describe("legacy functions download", () => { }, ); + it.live("skips network creation for a container: network mode", () => { + // Go's `container.NetworkMode.IsUserDefined()` + // (`docker/api/types/container/hostconfig_unix.go:23-25`) explicitly + // excludes `IsContainer()` — `--network-id container:redis` attaches to + // another container's network stack, so `DockerNetworkCreateIfNotExists` + // never inspects or creates a network for it, and the mode is passed + // straight through to `docker run --network` (review round on + // CLI-1963's `functions download` port). + const out = mockOutput({ format: "text" }); + const api = mockLegacyPlatformApi(); + const proxy = mockProxy(); + const child = mockChildProcessSpawner({ exitCode: 0 }); + const layer = Layer.mergeAll( + buildLegacyTestRuntime({ + out, + api, + cliConfig: mockLegacyCliConfig({ workdir: tempRoot.current }), + }), + proxy.layer, + child.layer, + Stdio.layerTest({ + args: Effect.succeed([ + "functions", + "download", + "hello-world", + "--use-docker", + "--project-ref", + PROJECT_ID, + "--network-id", + "container:redis", + ]), + }), + ); + + return Effect.gen(function* () { + yield* legacyFunctionsDownload({ ...baseFlags, useDocker: true }); + + expect(child.spawned.find((spawned) => spawned.args[0] === "network")).toBeUndefined(); + const runCommand = child.spawned.find((spawned) => spawned.args[0] === "run"); + expect(runCommand?.args).toContain("container:redis"); + }).pipe(Effect.provide(layer)); + }); + it.live("does not double-prefix an already v-prefixed edge-runtime-version pin", () => { // Go's `replaceImageTag` (`pkg/config/utils.go:81-84`) appends the pin // file's raw content verbatim after the image's `:`, so a pin already diff --git a/apps/cli/src/legacy/commands/start/lib/container-lifecycle.unit.test.ts b/apps/cli/src/legacy/commands/start/lib/container-lifecycle.unit.test.ts index cd77a460b4..66cb6084ed 100644 --- a/apps/cli/src/legacy/commands/start/lib/container-lifecycle.unit.test.ts +++ b/apps/cli/src/legacy/commands/start/lib/container-lifecycle.unit.test.ts @@ -747,6 +747,22 @@ describe("legacyEnsureStartNetwork", () => { ); }, ); + + it.live("skips docker network create for a container: network mode", () => { + // Go's `container.NetworkMode.IsUserDefined()` + // (`docker/api/types/container/hostconfig_unix.go:23-25`) explicitly + // excludes `IsContainer()` — `--network-id container:redis` attaches to + // another container's network stack, not a name `docker network create` + // could ever act on (review round on CLI-1963's `functions download` + // port, which surfaced the same gap in the shared + // `isUserDefinedDockerNetwork` predicate this helper reuses). + const mock = mockSpawner(() => ({ exitCode: 1, stderr: "some failure" })); + return legacyEnsureStartNetwork(mock.spawner, "container:redis", {}).pipe( + Effect.map(() => { + expect(mock.spawned).toEqual([]); + }), + ); + }); }); describe("legacyEnsureStartVolume", () => { diff --git a/apps/cli/src/shared/functions/functions-docker.ts b/apps/cli/src/shared/functions/functions-docker.ts index 8862964ac0..485eca85c3 100644 --- a/apps/cli/src/shared/functions/functions-docker.ts +++ b/apps/cli/src/shared/functions/functions-docker.ts @@ -86,13 +86,29 @@ export const runChildProcess = Effect.fnUntraced(function* ( return { exitCode, stdout, stderr }; }); +// Go: `container.NetworkMode.IsContainer()` (`docker/api/types/container/hostconfig.go:152-155`, +// via the unexported `containerID` helper, same file:493-499) — `--network container:` +// (Docker's syntax for attaching to another container's network stack) is recognized by a bare +// `"container:"` prefix before the first `:`, regardless of what (if anything) follows it. +function isContainerDockerNetworkMode(networkMode: string) { + const separatorIndex = networkMode.indexOf(":"); + return separatorIndex !== -1 && networkMode.slice(0, separatorIndex) === "container"; +} + +// Go: `container.NetworkMode.IsUserDefined()` (`docker/api/types/container/hostconfig_unix.go:23-25`) +// — `!IsDefault() && !IsBridge() && !IsHost() && !IsNone() && !IsContainer()`. Omitting the +// `IsContainer()` exclusion would make `DockerNetworkCreateIfNotExists` +// (`internal/utils/docker.go:63`) run `docker network inspect`/`create` against a +// `container:` mode, which isn't a network name at all — Go passes that mode straight +// through to the container's `NetworkMode` without ever touching the network subsystem. export function isUserDefinedDockerNetwork(networkMode: string) { return ( networkMode.length > 0 && networkMode !== "default" && networkMode !== "bridge" && networkMode !== "host" && - networkMode !== "none" + networkMode !== "none" && + !isContainerDockerNetworkMode(networkMode) ); } From f0d3361e323180e054753196a59c0270b43067cf Mon Sep 17 00:00:00 2001 From: Colum Ferry Date: Wed, 5 Aug 2026 16:58:56 +0100 Subject: [PATCH 09/19] fix(functions): resolve legacy Docker download config from the exact workdir, toml-only (review: CLI-1963) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Go's flags.LoadConfig only ever resolves supabase/config.toml from the already-resolved workdir, with no ancestor climb and no concept of a JSON project config (pkg/config/utils.go:43-48). resolveEdgeRuntimeImage's loadProjectConfig call omitted search: false/tomlOnly: true, so the legacy shell's Docker download path could pick up an unrelated ancestor project's config.toml, or prefer a stray supabase/config.json over config.toml — both diverging from Go. Gated on goViperCompat so the next shell keeps the package's existing (non-Go-parity) defaults, matching legacy-local-project-context.ts and start.handler.ts's established pattern for the same options. Also documents (not fixed here) a separate, pre-existing gap the same review round surfaced: resolveEdgeRuntimeImage resolves a single registry URL with no ECR/GHCR/Docker Hub retry, unlike Go's DockerResolveImageIfNotCached — shared with deploy.ts/serve.ts's own already-shipped native Docker paths, so it's a cross-cutting follow-up rather than a download-only fix. --- .../download/download.integration.test.ts | 118 ++++++++++++++++++ apps/cli/src/shared/functions/download.ts | 26 ++++ 2 files changed, 144 insertions(+) diff --git a/apps/cli/src/legacy/commands/functions/download/download.integration.test.ts b/apps/cli/src/legacy/commands/functions/download/download.integration.test.ts index 861784cf37..e161425c04 100644 --- a/apps/cli/src/legacy/commands/functions/download/download.integration.test.ts +++ b/apps/cli/src/legacy/commands/functions/download/download.integration.test.ts @@ -806,6 +806,124 @@ describe("legacy functions download", () => { }, ); + it.live( + "does not climb to an ancestor project's config.toml for the Docker download path", + () => { + // Go's `flags.LoadConfig` only ever resolves `supabase/config.toml` from + // the already-resolved workdir, with no ancestor climb + // (`NewPathBuilder`, `pkg/config/utils.go:43-48`) — mirrored here by + // `resolveEdgeRuntimeImage`'s `search: false` (review round on + // CLI-1963's `functions download` port). A nested workdir with no + // `supabase/config.toml` of its own must fall back to `--project-ref` + // for network/volume naming, not an ancestor project's configured + // `project_id`, even though `cliConfig.workdir` sits right inside one. + const out = mockOutput({ format: "text" }); + const api = mockLegacyPlatformApi(); + const proxy = mockProxy(); + const child = mockChildProcessSpawner({ exitCode: 0 }); + const nestedWorkdir = join(tempRoot.current, "nested"); + const layer = Layer.mergeAll( + buildLegacyTestRuntime({ + out, + api, + cliConfig: mockLegacyCliConfig({ workdir: nestedWorkdir }), + }), + proxy.layer, + child.layer, + Stdio.layerTest({ + args: Effect.succeed([ + "functions", + "download", + "hello-world", + "--use-docker", + "--project-ref", + PROJECT_ID, + ]), + }), + ); + + return Effect.gen(function* () { + yield* Effect.tryPromise(() => mkdir(nestedWorkdir, { recursive: true })); + yield* Effect.tryPromise(() => + mkdir(join(tempRoot.current, "supabase"), { recursive: true }), + ); + yield* Effect.tryPromise(() => + writeFile( + join(tempRoot.current, "supabase", "config.toml"), + ['project_id = "ancestor-project"', ""].join("\n"), + ), + ); + + yield* legacyFunctionsDownload({ ...baseFlags, useDocker: true }); + + expect(child.spawned.find((spawned) => spawned.args[0] === "network")).toEqual({ + command: "docker", + args: ["network", "inspect", `supabase_network_${PROJECT_ID}`], + }); + const runCommand = child.spawned.find((spawned) => spawned.args[0] === "run"); + expect(runCommand?.args).toContain(`supabase_network_${PROJECT_ID}`); + expect(runCommand?.args).not.toContain("supabase_network_ancestor-project"); + }).pipe(Effect.provide(layer)); + }, + ); + + it.live("prefers config.toml over a stray config.json for the Docker download path", () => { + // Go's `NewPathBuilder`/`Config.Load` (`pkg/config/utils.go:43-48`) has + // no concept of a JSON project config file — it always resolves + // `supabase/config.toml`, mirrored here by `resolveEdgeRuntimeImage`'s + // `tomlOnly: true` (review round on CLI-1963's `functions download` + // port). A workdir with both files must resolve `project_id` from + // `config.toml`, not prefer the JSON file as the package loader + // otherwise would. + const out = mockOutput({ format: "text" }); + const api = mockLegacyPlatformApi(); + const proxy = mockProxy(); + const child = mockChildProcessSpawner({ exitCode: 0 }); + const layer = Layer.mergeAll( + buildLegacyTestRuntime({ + out, + api, + cliConfig: mockLegacyCliConfig({ workdir: tempRoot.current }), + }), + proxy.layer, + child.layer, + Stdio.layerTest({ + args: Effect.succeed([ + "functions", + "download", + "hello-world", + "--use-docker", + "--project-ref", + PROJECT_ID, + ]), + }), + ); + + return Effect.gen(function* () { + yield* Effect.tryPromise(() => + mkdir(join(tempRoot.current, "supabase"), { recursive: true }), + ); + yield* Effect.tryPromise(() => + writeFile( + join(tempRoot.current, "supabase", "config.toml"), + ['project_id = "toml-project"', ""].join("\n"), + ), + ); + yield* Effect.tryPromise(() => + writeFile( + join(tempRoot.current, "supabase", "config.json"), + JSON.stringify({ project_id: "json-project" }), + ), + ); + + yield* legacyFunctionsDownload({ ...baseFlags, useDocker: true }); + + const runCommand = child.spawned.find((spawned) => spawned.args[0] === "run"); + expect(runCommand?.args).toContain("supabase_network_toml-project"); + expect(runCommand?.args).not.toContain("supabase_network_json-project"); + }).pipe(Effect.provide(layer)); + }); + it.live("skips network creation for a container: network mode", () => { // Go's `container.NetworkMode.IsUserDefined()` // (`docker/api/types/container/hostconfig_unix.go:23-25`) explicitly diff --git a/apps/cli/src/shared/functions/download.ts b/apps/cli/src/shared/functions/download.ts index d4b18e84f5..3b961f441f 100644 --- a/apps/cli/src/shared/functions/download.ts +++ b/apps/cli/src/shared/functions/download.ts @@ -859,6 +859,17 @@ const resolveEdgeRuntimeImage = Effect.fnUntraced(function* ( const loadedConfig = yield* loadProjectConfig(dependencies.projectRoot, { projectRef, goViperCompat: dependencies.goViperCompat, + // `search: false`/`tomlOnly: true` only under `goViperCompat` (the legacy caller, whose + // `dependencies.projectRoot` is `cliConfig.workdir` — already Go's fully-resolved chdir + // target, same reasoning as `legacy-local-project-context.ts`/`start.handler.ts`). Go's + // `flags.LoadConfig` (`pkg/config/utils.go:43-48`) only ever resolves `supabase/config.toml` + // from that exact workdir, with no ancestor climb and no concept of a JSON project config — + // leaving these unset here would let an unrelated ancestor project's config win, or a stray + // `supabase/config.json` be preferred over `config.toml`, for the legacy shell's Docker + // download path specifically. The `next` shell keeps the package defaults (ancestor search, + // JSON preferred), matching its other non-Go-parity `loadProjectConfig` callers. + search: dependencies.goViperCompat ? false : undefined, + tomlOnly: dependencies.goViperCompat, }); const denoVersion = loadedConfig?.config?.edge_runtime.deno_version; const projectId = loadedConfig?.config?.project_id ?? projectRef; @@ -872,6 +883,21 @@ const resolveEdgeRuntimeImage = Effect.fnUntraced(function* ( // `edgeRuntimeImageTag` (not a bare `v${edgeRuntimeVersion}` prepend) — // `dependencies.edgeRuntimeVersion` comes from a `.temp/edge-runtime-version` // pin that may already carry its own `v` prefix (see the helper's doc). + // + // Single `legacyGetRegistryImageUrl` value, not the ECR→GHCR→Docker-Hub + // retry `legacyGetRegistryImageUrlCandidates` gives `start` (review round + // on CLI-1963's `functions download` port). Go's `DockerStart` resolves + // `config.Image` through `DockerResolveImageIfNotCached` + // (`internal/utils/docker.go:326-348,363-365`), which tries every + // registry candidate — including for this exact edge-runtime unbundle + // container — so an ECR outage/throttle that the previous Go-delegated + // default path would have survived can now fail this native path outright. + // Pre-existing, not introduced by this PR: `deploy.ts`'s and `serve.ts`'s + // own already-shipped native Docker paths resolve this identically + // (single-URL, no retry) — `legacyGetRegistryImageUrlCandidates` has only + // ever been wired up for `start` (see its own doc comment). Extending the + // retry to all three `functions` Docker paths is a shared, cross-cutting + // follow-up, not something to fix piecemeal for `download` alone. image: legacyGetRegistryImageUrl( `supabase/edge-runtime:${edgeRuntimeImageTag(edgeRuntimeVersion)}`, ), From d4530b1fcb9ffc14ef69773fd8ff925e78b47ee5 Mon Sep 17 00:00:00 2001 From: Colum Ferry Date: Wed, 5 Aug 2026 18:41:11 +0100 Subject: [PATCH 10/19] docs(functions): document deno-version-default and network-id env-var gaps (review: CLI-1963) Codex flagged that resolveEdgeRuntimeImage falls back to the v2 default when config.toml is absent (ignoring SUPABASE_EDGE_RUNTIME_DENO_VERSION), and that networkMode resolution never checks SUPABASE_NETWORK_ID the way Go's viper AutomaticEnv does for the --network-id persistent flag. Both are confirmed real gaps, but pre-existing and cross-cutting rather than introduced here: deploy.ts has the identical deno_version fallback today (config.toml present or not, since @supabase/config has no generic env-var struct binding at all), and start.handler.ts/deploy.ts/serve.ts's own network-id resolution don't check SUPABASE_NETWORK_ID either. Fixing either belongs in one shared place, not duplicated per Docker-path call site in download.ts alone -- left open, matching this PR's existing precedent for the registry-fallback gap. Documented inline and in the PR description's "Judgement calls left open" section instead of silently resolving the review threads. --- apps/cli/src/shared/functions/download.ts | 33 +++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/apps/cli/src/shared/functions/download.ts b/apps/cli/src/shared/functions/download.ts index 3b961f441f..c0fef4e858 100644 --- a/apps/cli/src/shared/functions/download.ts +++ b/apps/cli/src/shared/functions/download.ts @@ -871,6 +871,26 @@ const resolveEdgeRuntimeImage = Effect.fnUntraced(function* ( search: dependencies.goViperCompat ? false : undefined, tomlOnly: dependencies.goViperCompat, }); + // A project with no `supabase/config.toml`/`config.json` makes + // `loadProjectConfig` return `null` outright, so `denoVersion` below falls + // through to `undefined` and this always resolves the v2 default. Go's + // `flags.LoadConfig` never short-circuits like that: `Config.Load` → + // `loadFromFile` (`pkg/config/config.go:579-611`) merges the template + // defaults and enables `viper.AutomaticEnv()` with `SetEnvPrefix("SUPABASE")` + // *before* attempting to read the file — `mergeFileConfig` (`config.go:701-716`) + // simply no-ops on `os.ErrNotExist` — so `SUPABASE_EDGE_RUNTIME_DENO_VERSION=1` + // (or the same key in `supabase/.env`, via `loadNestedEnv`) still pins the + // deno-v1 image even with no config.toml on disk. Pre-existing, not + // introduced by this PR: `@supabase/config`'s `loadProjectConfig` has no + // equivalent of Go's generic `ExperimentalBindStruct`+`AutomaticEnv` field + // binding at all (it only expands literal `env(...)` references already + // written inside the TOML), so `deploy.ts`'s identical + // `resolveEdgeRuntimeVersion(deployConfig?.edge_runtime.deno_version, ...)` + // call has the same gap whether or not config.toml exists. A fix belongs in + // the shared config-loading layer every native caller goes through (`gen + // types`, `next start`, `functions dev/serve/deploy`, …), not duplicated + // per call site here — left open (review round on CLI-1963's `functions + // download` port). const denoVersion = loadedConfig?.config?.edge_runtime.deno_version; const projectId = loadedConfig?.config?.project_id ?? projectRef; const edgeRuntimeVersion = yield* resolveEdgeRuntimeVersion( @@ -985,6 +1005,19 @@ const downloadWithDockerUnbundle = Effect.fnUntraced(function* ( // must fall through to the generated network name too, not just an omitted // flag — `explicitNonEmptyStringFlag` (unlike the unexported // `explicitStringFlag`) treats that case as unset for exactly this reason. + // + // Go's root `init()` also binds every persistent flag (including + // `network-id`) through `viper.BindPFlags` after enabling + // `viper.AutomaticEnv()` with `SetEnvPrefix("SUPABASE")` and a `-`→`_` + // replacer (`cmd/root.go:316-334`), so `SUPABASE_NETWORK_ID` overrides the + // flag's empty default whenever `--network-id` itself is never passed — + // this raw-argv-only lookup has no equivalent env-var fallback. Pre-existing, + // not introduced by this PR: `start.handler.ts`'s `LegacyNetworkIdFlag` and + // `deploy.ts`'s/`serve.ts`'s own network-id resolution don't check + // `SUPABASE_NETWORK_ID` either — no native command does today. A fix + // belongs in one shared place for the global `--network-id` resolution, + // not duplicated per Docker-path call site here — left open (review round + // on CLI-1963's `functions download` port). const networkMode = explicitNonEmptyStringFlag(dependencies.rawArgs, "network-id") ?? localDockerId("network", projectId); From 695afaaf6edd498e9cc7c4bc49a5015927eb4d19 Mon Sep 17 00:00:00 2001 From: Colum Ferry Date: Wed, 5 Aug 2026 19:44:32 +0100 Subject: [PATCH 11/19] fix(functions): honor the final occurrence of a repeated --network-id flag (review: CLI-1963) pflag/viper string flags are shared-variable, last-Set()-wins (confirmed empirically with a scratch pflag.FlagSet.Parse probe: --network-id old --network-id ci-net resolves to ci-net; a trailing --network-id= clears an earlier non-empty value). explicitStringFlag returned on the first argv match instead of scanning for the last, unlike this file's own explicitBooleanLongFlag and the legacy shell's legacyPflagStringValue, which already implement last-wins. Fixed to keep scanning, plus regression tests covering the repeated-override and repeated-then-cleared cases. --- .../download/download.integration.test.ts | 141 ++++++++++++++++++ apps/cli/src/shared/cli/cobra-flag-groups.ts | 23 +-- 2 files changed, 155 insertions(+), 9 deletions(-) diff --git a/apps/cli/src/legacy/commands/functions/download/download.integration.test.ts b/apps/cli/src/legacy/commands/functions/download/download.integration.test.ts index e161425c04..a5c10c6f7a 100644 --- a/apps/cli/src/legacy/commands/functions/download/download.integration.test.ts +++ b/apps/cli/src/legacy/commands/functions/download/download.integration.test.ts @@ -806,6 +806,94 @@ describe("legacy functions download", () => { }, ); + it.live("honors the final occurrence of a repeated --network-id flag", () => { + // pflag/viper string flags are shared-variable, last-`Set()`-wins + // (confirmed empirically: `pflag.FlagSet.Parse` on + // `--network-id old --network-id custom-network` resolves to + // `custom-network`) — `explicitStringFlag` must keep scanning past the + // first match instead of returning early (review round on CLI-1963's + // `functions download` port). + const out = mockOutput({ format: "text" }); + const api = mockLegacyPlatformApi(); + const proxy = mockProxy(); + const child = mockChildProcessSpawner({ exitCode: 0 }); + const layer = Layer.mergeAll( + buildLegacyTestRuntime({ + out, + api, + cliConfig: mockLegacyCliConfig({ workdir: tempRoot.current }), + }), + proxy.layer, + child.layer, + Stdio.layerTest({ + args: Effect.succeed([ + "functions", + "download", + "hello-world", + "--use-docker", + "--project-ref", + PROJECT_ID, + "--network-id", + "old-network", + "--network-id", + "custom-network", + ]), + }), + ); + + return Effect.gen(function* () { + yield* legacyFunctionsDownload({ ...baseFlags, useDocker: true }); + + const runCommand = child.spawned.find((spawned) => spawned.args[0] === "run"); + expect(runCommand?.args).toContain("custom-network"); + expect(runCommand?.args).not.toContain("old-network"); + }).pipe(Effect.provide(layer)); + }); + + it.live( + "falls back to the generated network name when the final --network-id occurrence is empty", + () => { + // Same last-wins rule as above, applied to Go's `len(networkId) > 0` + // gate: a non-empty default followed by an explicit-but-empty override + // must fall through to the generated network name, not the earlier + // non-empty value. + const out = mockOutput({ format: "text" }); + const api = mockLegacyPlatformApi(); + const proxy = mockProxy(); + const child = mockChildProcessSpawner({ exitCode: 0 }); + const layer = Layer.mergeAll( + buildLegacyTestRuntime({ + out, + api, + cliConfig: mockLegacyCliConfig({ workdir: tempRoot.current }), + }), + proxy.layer, + child.layer, + Stdio.layerTest({ + args: Effect.succeed([ + "functions", + "download", + "hello-world", + "--use-docker", + "--project-ref", + PROJECT_ID, + "--network-id", + "custom-network", + "--network-id=", + ]), + }), + ); + + return Effect.gen(function* () { + yield* legacyFunctionsDownload({ ...baseFlags, useDocker: true }); + + const runCommand = child.spawned.find((spawned) => spawned.args[0] === "run"); + expect(runCommand?.args).toContain(`supabase_network_${PROJECT_ID}`); + expect(runCommand?.args).not.toContain("custom-network"); + }).pipe(Effect.provide(layer)); + }, + ); + it.live( "does not climb to an ancestor project's config.toml for the Docker download path", () => { @@ -1509,6 +1597,59 @@ describe("legacy functions download", () => { }).pipe(Effect.provide(layer)); }); + it.live("fails loudly instead of silently dropping a malformed function-list entry", () => { + // Go: `FunctionResponse.Slug` (`pkg/api/types.gen.go:6465`) is a + // required, non-pointer `string` — a list entry with no "slug" key + // decodes to the zero value "" and then fails `ValidateFunctionSlug` + // in `downloadAll` (`download.go:182-188`), rather than being dropped + // from the list. A malicious/compromised API response returning + // `[{}]` must surface an error here too, never "No functions found." + // nor a silent partial download (review round on CLI-1963's + // `functions download` port). + const out = mockOutput({ format: "json" }); + const api = mockLegacyPlatformApi({ + handler: (request) => + request.url.endsWith("/functions") + ? Effect.succeed(legacyJsonResponse(request, 200, [{}])) + : Effect.succeed(legacyJsonResponse(request, 200, {})), + }); + const proxy = mockProxy(); + const child = mockChildProcessSpawner({ exitCode: 0 }); + const layer = Layer.mergeAll( + buildLegacyTestRuntime({ + out, + api, + cliConfig: mockLegacyCliConfig({ workdir: tempRoot.current }), + }), + proxy.layer, + child.layer, + Stdio.layerTest({ + args: Effect.succeed([ + "functions", + "download", + "--project-ref", + "abcdefghijklmnopqrst", + "--output-format", + "json", + ]), + }), + ); + + return Effect.gen(function* () { + const exit = yield* legacyFunctionsDownload({ + ...baseFlags, + functionName: Option.none(), + useDocker: true, + }).pipe(Effect.exit); + + expect(Exit.isFailure(exit)).toBe(true); + expect(proxy.calls).toEqual([]); + expect(out.messages).not.toContainEqual( + expect.objectContaining({ type: "success", message: "No functions found." }), + ); + }).pipe(Effect.provide(layer)); + }); + it.live("forwards only --legacy-bundle to the Go proxy, not the --use-docker default too", () => { const out = mockOutput({ format: "text" }); const api = mockLegacyPlatformApi(); diff --git a/apps/cli/src/shared/cli/cobra-flag-groups.ts b/apps/cli/src/shared/cli/cobra-flag-groups.ts index 46975b2fa6..88d80c350c 100644 --- a/apps/cli/src/shared/cli/cobra-flag-groups.ts +++ b/apps/cli/src/shared/cli/cobra-flag-groups.ts @@ -30,22 +30,27 @@ export function hasExplicitLongFlag( /** * Raw value of `--`/`--=value` anywhere in argv - * (unscoped — no command-path anchoring), or `undefined` if absent. Not - * exported — every current call site needs Go's `len(value) > 0` gate too - * (see {@link explicitNonEmptyStringFlag}); re-export this directly if a - * future caller genuinely needs presence-only semantics. + * (unscoped — no command-path anchoring), or `undefined` if absent. + * pflag string flags are shared-variable, last-`Set()`-wins (same rule + * {@link explicitBooleanLongFlag} and `legacyPflagStringValue` already + * follow) — a repeated `-- old -- new` must resolve to + * `new`, so this keeps scanning after a match instead of returning early + * (review round on CLI-1963's `functions download` port). Not exported — + * every current call site needs Go's `len(value) > 0` gate too (see + * {@link explicitNonEmptyStringFlag}); re-export this directly if a future + * caller genuinely needs presence-only semantics. */ function explicitStringFlag(rawArgs: ReadonlyArray, flagName: string) { + let result: string | undefined; for (let index = 0; index < rawArgs.length; index += 1) { const token = rawArgs[index]; if (token === `--${flagName}`) { - return rawArgs[index + 1]; - } - if (token?.startsWith(`--${flagName}=`)) { - return token.slice(flagName.length + 3); + result = rawArgs[index + 1]; + } else if (token?.startsWith(`--${flagName}=`)) { + result = token.slice(flagName.length + 3); } } - return undefined; + return result; } /** From d556716a6ea8c730198537ad720035c45af9d0d6 Mon Sep 17 00:00:00 2001 From: Colum Ferry Date: Wed, 5 Aug 2026 19:45:25 +0100 Subject: [PATCH 12/19] fix(functions): surface malformed function-list entries instead of dropping them (review: CLI-1963) Go's FunctionResponse.Slug (apps/cli-go/pkg/api/types.gen.go:6465) is a required, non-pointer string: a list entry with a missing or null "slug" decodes to the zero value "" rather than erroring, and that empty slug then fails ValidateFunctionSlug loudly in downloadAll (download.go:182-188) instead of vanishing from the list. listRemoteFunctionSlugs's flatMap filtered such entries out entirely, defeating part of the CLI-1891 validation this PR added for exactly this "compromised/malformed API response" threat model. Preserve the entry (coerced to "") so the existing validateRemoteSlug/validateSlug check catches it, matching Go instead of reporting "No functions found." or a silent partial download. --- apps/cli/src/shared/functions/download.ts | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/apps/cli/src/shared/functions/download.ts b/apps/cli/src/shared/functions/download.ts index c0fef4e858..a860cc56da 100644 --- a/apps/cli/src/shared/functions/download.ts +++ b/apps/cli/src/shared/functions/download.ts @@ -664,9 +664,21 @@ const listRemoteFunctionSlugs = Effect.fnUntraced(function* (api: ApiClient, pro if (!Array.isArray(parsed)) { throw new Error("expected functions list response to be an array"); } - return parsed.flatMap((value) => { + // Go: `FunctionResponse.Slug` (`apps/cli-go/pkg/api/types.gen.go:6465`) + // is a required, non-pointer `string` — a list entry with a missing or + // `null` "slug" decodes to the zero value `""` rather than erroring + // (`encoding/json`'s documented null-into-non-pointer no-op), and that + // empty slug then fails loudly downstream (`validateRemoteSlug`, + // matching Go's own per-item `ValidateFunctionSlug` in `downloadAll`, + // `download.go:182-188`) instead of silently vanishing from the list. + // Coercing here (rather than filtering the entry out, as before) + // preserves that "always surface an unexpected API response, never + // silently download fewer functions than requested" invariant — the + // exact CLI-1891 threat model `validateRemoteSlug` exists for (review + // round on CLI-1963's `functions download` port). + return parsed.map((value) => { const slug = getObjectProperty(value, "slug"); - return typeof slug === "string" ? [slug] : []; + return typeof slug === "string" ? slug : ""; }); }, catch: (cause) => From 78342387681cc5908e4bcd7f3f6bc223c9ef5244 Mon Sep 17 00:00:00 2001 From: Colum Ferry Date: Wed, 5 Aug 2026 19:45:44 +0100 Subject: [PATCH 13/19] docs(functions): document project_id-validation gap for Docker download configs (review: CLI-1963) Go's Config.Validate (pkg/config/config.go:990-991) rejects a config.toml with project_id = "" up front, inside flags.LoadConfig, before any Docker/API work. resolveEdgeRuntimeImage's `?? projectRef` fallback only substitutes on null/undefined, so an explicit empty project_id sails through instead. Pre-existing and cross-cutting, not specific to this PR: deploy.ts's identical deployConfig?.project_id ?? projectRef fallback (deploy.ts:2201) has the same gap, and no native functions Docker path (deploy/serve/download) routes its config through Config.Validate parity checks at all -- that port has one home today (legacy-config-validate.ts's legacyValidateResolvedConfig), wired up only for the db/migration loader and status/stop resolver. Left open, same treatment as the registry-fallback/config-defaults/network-id-env gaps already documented above -- belongs in the shared config-loading layer every native caller goes through, not duplicated per call site. --- apps/cli/src/shared/functions/download.ts | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/apps/cli/src/shared/functions/download.ts b/apps/cli/src/shared/functions/download.ts index a860cc56da..049845a534 100644 --- a/apps/cli/src/shared/functions/download.ts +++ b/apps/cli/src/shared/functions/download.ts @@ -904,6 +904,23 @@ const resolveEdgeRuntimeImage = Effect.fnUntraced(function* ( // per call site here — left open (review round on CLI-1963's `functions // download` port). const denoVersion = loadedConfig?.config?.edge_runtime.deno_version; + // `?? projectRef` only substitutes on `null`/`undefined`, so a config.toml + // with an explicit `project_id = ""` still resolves to the empty string + // here (`supabase_network_`/`supabase_edge_runtime_`) instead of failing + // up front. Go's `Config.Validate` rejects that same config with "Missing + // required field in config: project_id" (`pkg/config/config.go:990-991`) + // before `flags.LoadConfig` ever returns to `Run` — before any Docker/API + // work. Pre-existing and cross-cutting, not introduced by this PR: + // `deploy.ts`'s identical `deployConfig?.project_id ?? projectRef` + // fallback (`deploy.ts:2201`) has the same gap, and no native `functions` + // Docker path (`deploy`, `serve`, `download`) routes a loaded config + // through `Config.Validate` parity checks at all — that port has one home + // today, `legacy-config-validate.ts`'s `legacyValidateResolvedConfig`, + // wired up only for the db/migration loader and the status/stop resolver. + // Wiring `Config.Validate` into every native config-consuming command + // belongs in the shared config-loading layer, not duplicated per + // Docker-path call site here — left open, same as the config-defaults gap + // above (review round on CLI-1963's `functions download` port). const projectId = loadedConfig?.config?.project_id ?? projectRef; const edgeRuntimeVersion = yield* resolveEdgeRuntimeVersion( denoVersion, From 8b7dad19bb0af76f7d03103ec3ff84f6d36522c6 Mon Sep 17 00:00:00 2001 From: Colum Ferry Date: Wed, 5 Aug 2026 20:38:18 +0100 Subject: [PATCH 14/19] docs(functions): fix SIDE_EFFECTS.md config-read scope for download (review: CLI-1963) resolveEdgeRuntimeImage() (and its config.toml/config.json read) runs unconditionally after resolving the project ref, before the --use-api check -- matching Go's flags.LoadConfig running unconditionally at the top of Run. The doc previously claimed --use-api reads no project config at all, which is now stale. Also documents BITBUCKET_CLONE_DIR: the new Docker-unbundle path skips creating the named Deno-cache volume and its bind mount when set, mirroring deploy.ts's existing carve-out; the Environment Variables table omitted it entirely. --- .../functions/download/SIDE_EFFECTS.md | 35 ++++++++++--------- 1 file changed, 18 insertions(+), 17 deletions(-) diff --git a/apps/cli/src/legacy/commands/functions/download/SIDE_EFFECTS.md b/apps/cli/src/legacy/commands/functions/download/SIDE_EFFECTS.md index a32ee01293..4fe5177c4b 100644 --- a/apps/cli/src/legacy/commands/functions/download/SIDE_EFFECTS.md +++ b/apps/cli/src/legacy/commands/functions/download/SIDE_EFFECTS.md @@ -2,15 +2,15 @@ ## Files Read -| Path | Format | When | -| --------------------------------------------------- | ---------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| `/access-token` | plain text | when `SUPABASE_ACCESS_TOKEN` unset and keyring unavailable | -| `/profile` | plain text | when `--profile` and `SUPABASE_PROFILE` are both unset | -| `.yaml` | YAML | when `SUPABASE_PROFILE` or `--profile` points to a file | -| `/supabase/.temp/project-ref` | plain text | when `--project-ref` and `SUPABASE_PROJECT_ID` are both unset | -| `/supabase/.temp/edge-runtime-version` | plain text | Docker-unbundle path: overrides the default edge-runtime image tag when present | -| `/supabase/config.toml` (or `config.json`) | TOML/JSON | Docker-unbundle path: resolves `edge_runtime.deno_version` and `project_id` (`loadProjectConfig`) — a new file-read surface versus the `--use-api` path, which reads no project config | -| `/telemetry.json` | JSON | when present, before post-run telemetry state is refreshed | +| Path | Format | When | +| --------------------------------------------------- | ---------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `/access-token` | plain text | when `SUPABASE_ACCESS_TOKEN` unset and keyring unavailable | +| `/profile` | plain text | when `--profile` and `SUPABASE_PROFILE` are both unset | +| `.yaml` | YAML | when `SUPABASE_PROFILE` or `--profile` points to a file | +| `/supabase/.temp/project-ref` | plain text | when `--project-ref` and `SUPABASE_PROJECT_ID` are both unset | +| `/supabase/.temp/edge-runtime-version` | plain text | Docker-unbundle path: overrides the default edge-runtime image tag when present | +| `/supabase/config.toml` (or `config.json`) | TOML/JSON | Read unconditionally after resolving the project ref, before checking `--use-api`/`--use-docker` or whether Docker is running — resolves `edge_runtime.deno_version` and `project_id` (`loadProjectConfig`) for the Docker-unbundle path. Matches Go's `flags.LoadConfig` running unconditionally at the top of `Run` (`download.go:131-138`): a malformed config now fails here even on the `--use-api` invocation. | +| `/telemetry.json` | JSON | when present, before post-run telemetry state is refreshed | ## Files Written @@ -52,14 +52,15 @@ to stderr in machine-output modes (CLI-1546). ## Environment Variables -| Variable | Purpose | Required? | -| ----------------------- | --------------------------------------------------------------- | ------------------------------------------------------------------------------------- | -| `SUPABASE_ACCESS_TOKEN` | auth token (bypasses credential file/keyring lookup) | no (falls back to keyring → `/access-token`) | -| `SUPABASE_HOME` | overrides where `telemetry.json` and `profile` are read/written | no (defaults to `~/.supabase`) | -| `SUPABASE_NO_KEYRING` | disables the OS keyring, forcing the access-token file fallback | no | -| `SUPABASE_PROFILE` | select a built-in profile or YAML profile file with `api_url:` | no (falls back to `~/.supabase/profile` -> `supabase`) | -| `SUPABASE_PROJECT_ID` | provides the project ref when `--project-ref` is unset | no (falls back to `/supabase/.temp/project-ref`) | -| `SUPABASE_WORKDIR` | sets `` for local Supabase temp files | no (falls back to `--workdir` -> nearest ancestor with `supabase/config.toml` -> cwd) | +| Variable | Purpose | Required? | +| ----------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | ------------------------------------------------------------------------------------- | +| `SUPABASE_ACCESS_TOKEN` | auth token (bypasses credential file/keyring lookup) | no (falls back to keyring → `/access-token`) | +| `SUPABASE_HOME` | overrides where `telemetry.json` and `profile` are read/written | no (defaults to `~/.supabase`) | +| `SUPABASE_NO_KEYRING` | disables the OS keyring, forcing the access-token file fallback | no | +| `SUPABASE_PROFILE` | select a built-in profile or YAML profile file with `api_url:` | no (falls back to `~/.supabase/profile` -> `supabase`) | +| `SUPABASE_PROJECT_ID` | provides the project ref when `--project-ref` is unset | no (falls back to `/supabase/.temp/project-ref`) | +| `SUPABASE_WORKDIR` | sets `` for local Supabase temp files | no (falls back to `--workdir` -> nearest ancestor with `supabase/config.toml` -> cwd) | +| `BITBUCKET_CLONE_DIR` | Docker-unbundle path: when set, skips creating the named Deno-cache volume and omits its bind mount from the `docker run` command (Bitbucket's restricted Docker environment rejects both) | no | ## Exit Codes From b856a5318c7201b90726b9d1368d4b9ffca9fea9 Mon Sep 17 00:00:00 2001 From: Colum Ferry Date: Wed, 5 Aug 2026 20:38:28 +0100 Subject: [PATCH 15/19] fix(functions): style the Docker-unbundle progress slug in legacy shell (review: CLI-1963) Go's downloadOne bolds the slug on the "Downloading function:" progress line (utils.Bold, download.go:219); the new native Docker-unbundle path wrote the plain slug with no styling. Adds an optional styleEmphasis hook to DownloadDockerRuntimeDependencies (defaulting to identity, mirroring deploy.ts's DeployFunctionsDependencies.styleEmphasis) and wires the legacy handler to inject legacyBold, keeping next isolated from legacy/-specific rendering. downloadSingle's server-side path has the identical unstyled-slug gap, but it predates this PR (#5527) rather than being introduced here, so it's left as-is. --- .../functions/download/download.handler.ts | 4 ++++ apps/cli/src/shared/functions/download.ts | 16 ++++++++++++++-- 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/apps/cli/src/legacy/commands/functions/download/download.handler.ts b/apps/cli/src/legacy/commands/functions/download/download.handler.ts index 53886acf72..2b89b6aa75 100644 --- a/apps/cli/src/legacy/commands/functions/download/download.handler.ts +++ b/apps/cli/src/legacy/commands/functions/download/download.handler.ts @@ -6,6 +6,7 @@ import { } from "../../../../shared/functions/download.ts"; import { resolveEdgeRuntimeVersionPin } from "../../../../shared/functions/functions.shared.ts"; import { LegacyGoProxy } from "../../../../shared/legacy/go-proxy.service.ts"; +import { legacyBold } from "../../../shared/legacy-colors.ts"; import { LegacyPlatformApi } from "../../../auth/legacy-platform-api.service.ts"; import { LegacyCliConfig } from "../../../config/legacy-cli-config.service.ts"; import { LegacyProjectRefResolver } from "../../../config/legacy-project-ref.service.ts"; @@ -35,6 +36,9 @@ export const legacyFunctionsDownload = Effect.fn("legacy.functions.download")(fu rawArgs, goViperCompat: true, edgeRuntimeVersion, + // Go: `utils.Bold` on the `Downloading function:` slug (`downloadOne`, + // `download.go:219`, stderr) — matches `legacyBold`'s default TTY gate. + styleEmphasis: (text) => legacyBold(text), resolveProjectRef: (projectRef) => resolver.resolve(projectRef).pipe( Effect.tap((ref) => diff --git a/apps/cli/src/shared/functions/download.ts b/apps/cli/src/shared/functions/download.ts index 049845a534..b0cfb10531 100644 --- a/apps/cli/src/shared/functions/download.ts +++ b/apps/cli/src/shared/functions/download.ts @@ -61,6 +61,14 @@ interface DownloadRuntimeDependencies { /** Adds what the Docker-unbundle path needs beyond the server-side path. */ interface DownloadDockerRuntimeDependencies extends DownloadRuntimeDependencies { readonly rawArgs: ReadonlyArray; + /** + * Optional shell-specific styling hook for the `Downloading function:` + * progress line — mirrors `deploy.ts`'s `DeployFunctionsDependencies.styleEmphasis`. + * Defaults to identity (plain text); the legacy shell injects Go's bold + * styling here so the next shell stays isolated from `legacy/`-specific + * rendering. Go: `utils.Bold(slug)` (`downloadOne`, `download.go:219`). + */ + readonly styleEmphasis?: (text: string) => string; } /** @@ -972,11 +980,15 @@ const downloadWithDockerUnbundle = Effect.fnUntraced(function* ( slug: string, ) { const output = yield* Output; + const styleEmphasis = dependencies.styleEmphasis ?? ((text: string) => text); // Go: `downloadOne` (`download.go:219`) — lowercase "function", distinct // from the server-side path's "Downloading Function:" (capital F, - // `downloadWithServerSideUnbundle`, `download.go:329`). - yield* output.raw(`Downloading function: ${slug}\n`, "stderr"); + // `downloadWithServerSideUnbundle`, `download.go:329`). Both Go call sites + // bold the slug (`utils.Bold`); this path is new in CLI-1963, so it picks + // up the styling hook now. `downloadSingle`'s server-side path below has + // the identical gap, but predates this PR (#5527) — left as-is here. + yield* output.raw(`Downloading function: ${styleEmphasis(slug)}\n`, "stderr"); const eszip = yield* downloadEszipBody(dependencies.api, projectRef, slug); From 118787458438fe22b9eb1d1458d0c9cd08129cf7 Mon Sep 17 00:00:00 2001 From: Colum Ferry Date: Wed, 5 Aug 2026 21:42:56 +0100 Subject: [PATCH 16/19] fix(functions): fail the whole function list on a typed non-string slug (review: CLI-1963) Go's generated client unmarshals the entire []FunctionResponse array in one json.Unmarshal call (apps/cli-go/pkg/api/client.gen.go:22186-22208) -- a type mismatch on any single element's slug (a required string field) fails that call outright, and ParseV1ListAllFunctionsResponse returns before ever assigning response.JSON200, so downloadAll fails with "failed to list functions: ..." before downloading anything. listRemoteFunctionSlugs instead coerced a present-but-non-string slug (e.g. 123) to "", so an earlier well-formed entry in the same list would already be downloaded before the later entry's validation error surfaced. Throw immediately on a present, non-string slug (still zero-valuing missing/null, matching Go's null-into-non-pointer no-op) to preserve Go's fail-before-any-download ordering. Confirmed empirically with a scratch json.Unmarshal probe. --- .../download/download.integration.test.ts | 43 +++++++++++++++++++ apps/cli/src/shared/functions/download.ts | 23 +++++++++- 2 files changed, 65 insertions(+), 1 deletion(-) diff --git a/apps/cli/src/next/commands/functions/download/download.integration.test.ts b/apps/cli/src/next/commands/functions/download/download.integration.test.ts index 72f23a0413..cfe88deb0d 100644 --- a/apps/cli/src/next/commands/functions/download/download.integration.test.ts +++ b/apps/cli/src/next/commands/functions/download/download.integration.test.ts @@ -766,6 +766,49 @@ describe("functions download", () => { ); }); + it.live( + "fails the whole list before downloading anything when a slug is typed as a non-string", + () => { + const tempDir = makeTempDir(); + + return Effect.gen(function* () { + yield* Effect.tryPromise(() => writeProjectConfig(tempDir)); + // Go's generated client unmarshals the whole `[]FunctionResponse` + // array in one `json.Unmarshal` call + // (`apps/cli-go/pkg/api/client.gen.go:22186-22208`); a type mismatch + // on any single element's `slug` (a required `string` field) fails + // that call outright, so `downloadAll` fails with "failed to list + // functions: ..." before downloading anything — including the + // earlier, well-formed "ok" entry. Confirmed empirically: + // `json.Unmarshal([]byte(`[{"slug":"ok"},{"slug":123}]`), &dest)` + // returns a `*json.UnmarshalTypeError`, and the generated parser + // returns before ever assigning `response.JSON200`. + const { api, layer } = setup(tempDir, { + listBody: [{ slug: "ok" }, { slug: 123 }], + }); + + const error = yield* functionsDownload({ + ...BASE_FLAGS, + functionName: Option.none(), + }).pipe(Effect.provide(layer), Effect.flip); + + expect(error).toBeInstanceOf(InvalidFunctionDownloadResponseError); + expect((error as Error).message).toBe( + "failed to read functions list: expected function slug to be a string, got number", + ); + // Only the list call happened — "ok" was never downloaded, matching + // Go's atomic list-decode failure instead of downloading it before + // hitting the later entry's error. + expect(api.requests).toEqual([ + `https://api.supabase.com/v1/projects/${PROJECT_REF}/functions`, + ]); + expect(existsSync(join(tempDir, "supabase", "functions"))).toBe(false); + }).pipe( + Effect.ensuring(Effect.tryPromise(() => rm(tempDir, { recursive: true, force: true }))), + ); + }, + ); + it.live("prints the download-all success line when the project has one function", () => { const tempDir = makeTempDir(); const multipart = multipartBody([ diff --git a/apps/cli/src/shared/functions/download.ts b/apps/cli/src/shared/functions/download.ts index b0cfb10531..c8490ad885 100644 --- a/apps/cli/src/shared/functions/download.ts +++ b/apps/cli/src/shared/functions/download.ts @@ -684,9 +684,30 @@ const listRemoteFunctionSlugs = Effect.fnUntraced(function* (api: ApiClient, pro // silently download fewer functions than requested" invariant — the // exact CLI-1891 threat model `validateRemoteSlug` exists for (review // round on CLI-1963's `functions download` port). + // + // A "slug" present but typed as something other than string/null is a + // different case: Go's generated client decodes the *entire* array in + // one `json.Unmarshal` call (`ParseV1ListAllFunctionsResponse`, + // `apps/cli-go/pkg/api/client.gen.go:22186-22208`), and a type mismatch + // on any single element fails that whole call — confirmed empirically + // (`json.Unmarshal([]byte(`+"`"+`[{"slug":"ok"},{"slug":123}]`+"`"+`), &dest)` + // returns a `*json.UnmarshalTypeError`; `dest` is partially populated in + // memory, but `ParseV1ListAllFunctionsResponse` returns before ever + // assigning `response.JSON200`, discarding it), so `V1ListAllFunctionsWithResponse` + // returns an error and `downloadAll` fails with "failed to list + // functions: ..." before downloading anything — not after downloading + // the earlier, well-formed entries. Throwing here (rather than + // coercing to `""` like the missing/null case above) preserves that + // same fail-before-any-download ordering. return parsed.map((value) => { const slug = getObjectProperty(value, "slug"); - return typeof slug === "string" ? slug : ""; + if (slug === null || slug === undefined) { + return ""; + } + if (typeof slug !== "string") { + throw new Error(`expected function slug to be a string, got ${typeof slug}`); + } + return slug; }); }, catch: (cause) => From 29f2f28f9bc8a4a3935997a93b8d3db0dbc6c3a9 Mon Sep 17 00:00:00 2001 From: Colum Ferry Date: Wed, 5 Aug 2026 21:43:49 +0100 Subject: [PATCH 17/19] docs(functions): document project-dotenv registry gap as cross-cutting (review: CLI-1963) resolveEdgeRuntimeImage calls legacyGetRegistryImageUrl with no projectEnvValues, so a SUPABASE_INTERNAL_IMAGE_REGISTRY set only in supabase/.env (not the ambient shell) is invisible here, unlike Go's flags.LoadConfig -> loadNestedEnv, which os.Setenvs every project dotenv key into the process env before GetRegistry() ever reads it. Confirmed real, but pre-existing and cross-cutting, not specific to this PR: deploy.ts and serve.ts call the same helper the same way -- the only caller that resolves and threads project dotenv today is start, via legacyLoadLocalProjectContext. Belongs in the shared config-loading layer every native functions Docker path goes through, not duplicated per call site -- left open, same treatment already applied to the registry-fallback/config-defaults/network-id-env/ Config.Validate gaps in this same function. --- apps/cli/src/shared/functions/download.ts | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/apps/cli/src/shared/functions/download.ts b/apps/cli/src/shared/functions/download.ts index c8490ad885..20afd845f9 100644 --- a/apps/cli/src/shared/functions/download.ts +++ b/apps/cli/src/shared/functions/download.ts @@ -976,6 +976,28 @@ const resolveEdgeRuntimeImage = Effect.fnUntraced(function* ( // ever been wired up for `start` (see its own doc comment). Extending the // retry to all three `functions` Docker paths is a shared, cross-cutting // follow-up, not something to fix piecemeal for `download` alone. + // + // No `projectEnvValues` argument either (review round on CLI-1963's + // `functions download` port): Go's `flags.LoadConfig` → `loadNestedEnv` + // (`pkg/config/config.go:1220-1258`) calls `godotenv.Load` on every + // project dotenv file, which `os.Setenv`s each key into the process env + // — ambient-wins, but a `SUPABASE_INTERNAL_IMAGE_REGISTRY` set only in + // `supabase/.env` (not the ambient shell) is visible to `GetRegistry()`'s + // later `viper.GetString("INTERNAL_IMAGE_REGISTRY")` read + // (`internal/utils/docker.go:221-227`) regardless. `loadProjectConfig` + // above only uses its own dotenv read internally, for `env(...)` + // interpolation — it doesn't return the values, so this call falls back + // to `legacyGetRegistryOverride`'s ambient-only `process.env` read and + // misses a project-local registry mirror configured only via dotenv. + // Pre-existing and cross-cutting, not introduced by this PR: `deploy.ts` + // (`deploy.ts:1287`) and `serve.ts` (`serve.ts:1756`) call the same + // `legacyGetRegistryImageUrl` with no `projectEnvValues` either — the + // only caller that resolves and threads it today is `start`, via + // `legacyLoadLocalProjectContext`/`legacyGetRegistryImageUrlCandidates`. + // Loading project dotenv for every native `functions` Docker path belongs + // in the shared config-loading layer, not duplicated per call site here + // — left open, same treatment as the config-defaults/network-id-env/ + // Config.Validate gaps above. image: legacyGetRegistryImageUrl( `supabase/edge-runtime:${edgeRuntimeImageTag(edgeRuntimeVersion)}`, ), From 42b5c371f645760b44f2b2d00359e5930ce9f26b Mon Sep 17 00:00:00 2001 From: Colum Ferry Date: Wed, 5 Aug 2026 22:47:36 +0100 Subject: [PATCH 18/19] docs(functions): fix legacy download SIDE_EFFECTS.md file-read matrix (review: CLI-1963) - edge-runtime-version pin is read unconditionally by resolveEdgeRuntimeVersionPin() before the --use-api/Docker choice, not only on the Docker-unbundle path. - goViperCompat's tomlOnly:true means config.json is never a legacy read path; drop the "(or config.json)" implication from config.toml's row. - list the SUPABASE_INTERNAL_IMAGE_REGISTRY env var, read unconditionally while resolving the edge-runtime image (even on --use-api invocations). --- .../functions/download/SIDE_EFFECTS.md | 37 ++++++++++--------- 1 file changed, 19 insertions(+), 18 deletions(-) diff --git a/apps/cli/src/legacy/commands/functions/download/SIDE_EFFECTS.md b/apps/cli/src/legacy/commands/functions/download/SIDE_EFFECTS.md index 4fe5177c4b..11f217cb53 100644 --- a/apps/cli/src/legacy/commands/functions/download/SIDE_EFFECTS.md +++ b/apps/cli/src/legacy/commands/functions/download/SIDE_EFFECTS.md @@ -2,15 +2,15 @@ ## Files Read -| Path | Format | When | -| --------------------------------------------------- | ---------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| `/access-token` | plain text | when `SUPABASE_ACCESS_TOKEN` unset and keyring unavailable | -| `/profile` | plain text | when `--profile` and `SUPABASE_PROFILE` are both unset | -| `.yaml` | YAML | when `SUPABASE_PROFILE` or `--profile` points to a file | -| `/supabase/.temp/project-ref` | plain text | when `--project-ref` and `SUPABASE_PROJECT_ID` are both unset | -| `/supabase/.temp/edge-runtime-version` | plain text | Docker-unbundle path: overrides the default edge-runtime image tag when present | -| `/supabase/config.toml` (or `config.json`) | TOML/JSON | Read unconditionally after resolving the project ref, before checking `--use-api`/`--use-docker` or whether Docker is running — resolves `edge_runtime.deno_version` and `project_id` (`loadProjectConfig`) for the Docker-unbundle path. Matches Go's `flags.LoadConfig` running unconditionally at the top of `Run` (`download.go:131-138`): a malformed config now fails here even on the `--use-api` invocation. | -| `/telemetry.json` | JSON | when present, before post-run telemetry state is refreshed | +| Path | Format | When | +| ----------------------------------------------- | ---------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `/access-token` | plain text | when `SUPABASE_ACCESS_TOKEN` unset and keyring unavailable | +| `/profile` | plain text | when `--profile` and `SUPABASE_PROFILE` are both unset | +| `.yaml` | YAML | when `SUPABASE_PROFILE` or `--profile` points to a file | +| `/supabase/.temp/project-ref` | plain text | when `--project-ref` and `SUPABASE_PROJECT_ID` are both unset | +| `/supabase/.temp/edge-runtime-version` | plain text | Read unconditionally by `resolveEdgeRuntimeVersionPin()` in the handler, before the shared downloader chooses `--use-api` vs Docker — only affects the resolved edge-runtime image tag on the Docker-unbundle path | +| `/supabase/config.toml` | TOML | Read unconditionally after resolving the project ref, before checking `--use-api`/`--use-docker` or whether Docker is running — resolves `edge_runtime.deno_version` and `project_id` (`loadProjectConfig`) for the Docker-unbundle path. `goViperCompat`'s `tomlOnly: true` means `config.json` is never read here, unlike other `loadProjectConfig` callers. Matches Go's `flags.LoadConfig` running unconditionally at the top of `Run` (`download.go:131-138`): a malformed config now fails here even on the `--use-api` invocation. | +| `/telemetry.json` | JSON | when present, before post-run telemetry state is refreshed | ## Files Written @@ -52,15 +52,16 @@ to stderr in machine-output modes (CLI-1546). ## Environment Variables -| Variable | Purpose | Required? | -| ----------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | ------------------------------------------------------------------------------------- | -| `SUPABASE_ACCESS_TOKEN` | auth token (bypasses credential file/keyring lookup) | no (falls back to keyring → `/access-token`) | -| `SUPABASE_HOME` | overrides where `telemetry.json` and `profile` are read/written | no (defaults to `~/.supabase`) | -| `SUPABASE_NO_KEYRING` | disables the OS keyring, forcing the access-token file fallback | no | -| `SUPABASE_PROFILE` | select a built-in profile or YAML profile file with `api_url:` | no (falls back to `~/.supabase/profile` -> `supabase`) | -| `SUPABASE_PROJECT_ID` | provides the project ref when `--project-ref` is unset | no (falls back to `/supabase/.temp/project-ref`) | -| `SUPABASE_WORKDIR` | sets `` for local Supabase temp files | no (falls back to `--workdir` -> nearest ancestor with `supabase/config.toml` -> cwd) | -| `BITBUCKET_CLONE_DIR` | Docker-unbundle path: when set, skips creating the named Deno-cache volume and omits its bind mount from the `docker run` command (Bitbucket's restricted Docker environment rejects both) | no | +| Variable | Purpose | Required? | +| ---------------------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------- | +| `SUPABASE_ACCESS_TOKEN` | auth token (bypasses credential file/keyring lookup) | no (falls back to keyring → `/access-token`) | +| `SUPABASE_HOME` | overrides where `telemetry.json` and `profile` are read/written | no (defaults to `~/.supabase`) | +| `SUPABASE_NO_KEYRING` | disables the OS keyring, forcing the access-token file fallback | no | +| `SUPABASE_PROFILE` | select a built-in profile or YAML profile file with `api_url:` | no (falls back to `~/.supabase/profile` -> `supabase`) | +| `SUPABASE_PROJECT_ID` | provides the project ref when `--project-ref` is unset | no (falls back to `/supabase/.temp/project-ref`) | +| `SUPABASE_WORKDIR` | sets `` for local Supabase temp files | no (falls back to `--workdir` -> nearest ancestor with `supabase/config.toml` -> cwd) | +| `BITBUCKET_CLONE_DIR` | Docker-unbundle path: when set, skips creating the named Deno-cache volume and omits its bind mount from the `docker run` command (Bitbucket's restricted Docker environment rejects both) | no | +| `SUPABASE_INTERNAL_IMAGE_REGISTRY` | selects the registry the edge-runtime unbundle image is pulled from (`legacyGetRegistryImageUrl`); read unconditionally while resolving the image, before the `--use-api`/Docker choice is finalized — also consumed on the `--use-api` invocation even though it never pulls an image | no (defaults to `public.ecr.aws`) | ## Exit Codes From 39b98d0ebbcf75ceb24d2f24d1440edf1c33f000 Mon Sep 17 00:00:00 2001 From: Colum Ferry Date: Wed, 5 Aug 2026 22:47:47 +0100 Subject: [PATCH 19/19] fix(functions): style the --legacy-bundle suggestion aqua in legacy shell (review: CLI-1963) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Go wraps the suggested `--legacy-bundle` command in utils.Aqua (suggestLegacyBundle, download.go:315); the Docker-unbundle port hard-coded plain text even though this same file already threads a styleEmphasis hook for the sibling "Downloading function:" line. Add a matching styleAqua dependency, injected as legacyAqua from the legacy handler (next stays plain, same isolation rationale as styleEmphasis). Also documents three confirmed-but-left-open cross-cutting gaps found in the same review round (buffered instead of streamed unbundle container output, missing container labels, unstyled Docker-down warning) — each already present unmodified in deploy.ts's Docker bundler, so fixing them only here would create asymmetry between the two commands. See the PR description's "Judgement calls left open" section. --- .../functions/download/download.handler.ts | 6 +- apps/cli/src/shared/functions/download.ts | 80 ++++++++++++++++--- 2 files changed, 74 insertions(+), 12 deletions(-) diff --git a/apps/cli/src/legacy/commands/functions/download/download.handler.ts b/apps/cli/src/legacy/commands/functions/download/download.handler.ts index 2b89b6aa75..f53170995b 100644 --- a/apps/cli/src/legacy/commands/functions/download/download.handler.ts +++ b/apps/cli/src/legacy/commands/functions/download/download.handler.ts @@ -6,7 +6,7 @@ import { } from "../../../../shared/functions/download.ts"; import { resolveEdgeRuntimeVersionPin } from "../../../../shared/functions/functions.shared.ts"; import { LegacyGoProxy } from "../../../../shared/legacy/go-proxy.service.ts"; -import { legacyBold } from "../../../shared/legacy-colors.ts"; +import { legacyAqua, legacyBold } from "../../../shared/legacy-colors.ts"; import { LegacyPlatformApi } from "../../../auth/legacy-platform-api.service.ts"; import { LegacyCliConfig } from "../../../config/legacy-cli-config.service.ts"; import { LegacyProjectRefResolver } from "../../../config/legacy-project-ref.service.ts"; @@ -39,6 +39,10 @@ export const legacyFunctionsDownload = Effect.fn("legacy.functions.download")(fu // Go: `utils.Bold` on the `Downloading function:` slug (`downloadOne`, // `download.go:219`, stderr) — matches `legacyBold`'s default TTY gate. styleEmphasis: (text) => legacyBold(text), + // Go: `utils.Aqua` on the suggested `--legacy-bundle` command + // (`suggestLegacyBundle`, `download.go:315`, stderr) — matches + // `legacyAqua`'s default TTY gate. + styleAqua: (text) => legacyAqua(text), resolveProjectRef: (projectRef) => resolver.resolve(projectRef).pipe( Effect.tap((ref) => diff --git a/apps/cli/src/shared/functions/download.ts b/apps/cli/src/shared/functions/download.ts index 20afd845f9..2d849fd4bd 100644 --- a/apps/cli/src/shared/functions/download.ts +++ b/apps/cli/src/shared/functions/download.ts @@ -69,6 +69,14 @@ interface DownloadDockerRuntimeDependencies extends DownloadRuntimeDependencies * rendering. Go: `utils.Bold(slug)` (`downloadOne`, `download.go:219`). */ readonly styleEmphasis?: (text: string) => string; + /** + * Optional shell-specific styling hook for the `--legacy-bundle` command + * suggested inside {@link suggestLegacyBundle} — same isolation rationale + * as {@link styleEmphasis}, just a different Go colour. Go: + * `utils.Aqua("supabase functions download --legacy-bundle "+slug)` + * (`suggestLegacyBundle`, `download.go:315`). + */ + readonly styleAqua?: (text: string) => string; } /** @@ -839,10 +847,15 @@ const downloadEszipBody = Effect.fnUntraced(function* ( ); }); -function suggestLegacyBundle(slug: string): string { +function suggestLegacyBundle( + slug: string, + styleAqua: (text: string) => string = (text) => text, +): string { // Go: `suggestLegacyBundle` (`download.go:314-316`) — verbatim, including - // the source's own "trying running" wording and its leading newline. - return `\nIf your function is deployed using CLI < 1.120.0, trying running supabase functions download --legacy-bundle ${slug} instead.`; + // the source's own "trying running" wording and its leading newline. Go + // wraps only the suggested command itself in `utils.Aqua`, not the whole + // sentence — `styleAqua` mirrors that scope exactly. + return `\nIf your function is deployed using CLI < 1.120.0, trying running ${styleAqua(`supabase functions download --legacy-bundle ${slug}`)} instead.`; } function suggestDenoV2(): string { @@ -862,10 +875,10 @@ function suggestDenoV2(): string { * they raise themselves (`functions-docker.ts`), so this only normalizes * (never re-prefixes) whatever `legacyDescribeContainerCliFailure` reports. */ -function withLegacyBundleSuggestion(slug: string) { +function withLegacyBundleSuggestion(slug: string, styleAqua?: (text: string) => string) { return (cause: unknown): Error => Object.assign(new Error(legacyDescribeContainerCliFailure(cause)), { - suggestion: suggestLegacyBundle(slug), + suggestion: suggestLegacyBundle(slug, styleAqua), }); } @@ -877,10 +890,10 @@ function withLegacyBundleSuggestion(slug: string) { * running, unlike `ensureDockerNetwork`/`ensureDockerNamedVolume`'s * self-describing errors. */ -function withDockerStepFailure(step: string, slug: string) { +function withDockerStepFailure(step: string, slug: string, styleAqua?: (text: string) => string) { return (cause: unknown): Error => Object.assign(new Error(`${step}: ${legacyDescribeContainerCliFailure(cause)}`), { - suggestion: suggestLegacyBundle(slug), + suggestion: suggestLegacyBundle(slug, styleAqua), }); } @@ -1024,6 +1037,7 @@ const downloadWithDockerUnbundle = Effect.fnUntraced(function* ( ) { const output = yield* Output; const styleEmphasis = dependencies.styleEmphasis ?? ((text: string) => text); + const styleAqua = dependencies.styleAqua ?? ((text: string) => text); // Go: `downloadOne` (`download.go:219`) — lowercase "function", distinct // from the server-side path's "Downloading Function:" (capital F, @@ -1108,10 +1122,10 @@ const downloadWithDockerUnbundle = Effect.fnUntraced(function* ( const extract = Effect.gen(function* () { yield* ensureDockerNetwork(networkMode, projectId).pipe( - Effect.mapError(withLegacyBundleSuggestion(slug)), + Effect.mapError(withLegacyBundleSuggestion(slug, styleAqua)), ); yield* ensureDockerNamedVolume(localDockerId("edge_runtime", projectId), projectId).pipe( - Effect.mapError(withLegacyBundleSuggestion(slug)), + Effect.mapError(withLegacyBundleSuggestion(slug, styleAqua)), ); // Bind order matches `extractOne` (`download.go:260-266`) exactly. Go's @@ -1128,6 +1142,22 @@ const downloadWithDockerUnbundle = Effect.fnUntraced(function* ( `${hostEszipPath}:${dockerEszipPath}:ro`, `${functionsDir}:${DOCKER_DENO_DIR}:rw`, ]; + // No `com.supabase.cli.project`/`com.docker.compose.project` labels on + // this container itself — Go's `DockerStart` (`internal/utils/docker.go:372-376`) + // sets both unconditionally on `config.Labels` for every container it + // starts via the Engine API, including this exact unbundle container + // (`DockerRunOnceWithConfig` → `DockerStart`, `download.go:268`), so + // label-based cleanup/inspection can't associate an orphaned one-shot + // container with the project if the CLI is interrupted mid-run. Pre-existing + // and cross-cutting, not introduced by this PR: `deploy.ts`'s own + // `bundleFunctionWithDocker` builds an equally raw `docker run` command + // (its own `command.push(image, "bundle", ...)`) with the identical gap — + // only `ensureDockerNetwork`/`ensureDockerNamedVolume` (`functions-docker.ts`) + // thread `dockerProjectLabels` today, for the network/volume they create, + // not for the one-shot containers either Docker path runs. Adding `--label` + // to every `functions` Docker `run` invocation belongs in a shared + // container-build helper both call sites use, not duplicated per call site + // here — left open (review round on CLI-1963's `functions download` port). const command = [ "run", "--rm", @@ -1140,12 +1170,26 @@ const downloadWithDockerUnbundle = Effect.fnUntraced(function* ( } command.push(image, "unbundle", "--eszip", dockerEszipPath, "--output", dockerOutputPath); + // Go pipes the container's stdout/stderr straight to `os.Stdout`/`getErrorLogger()` + // while the container runs (`DockerRunOnceWithConfig`, copied live via the + // log stream) — this awaits `runChildProcess`, which buffers the whole + // run via `collectByteStream`'s `Stream.runFold` and only writes below + // once the process exits, so live progress/error output is hidden and + // stdout/stderr ordering can't be preserved relative to each other while + // the container is still running. Pre-existing and cross-cutting, not + // introduced by this PR: `deploy.ts`'s `bundleFunctionWithDocker` (added + // in #5561, before `functions-docker.ts` existed as its own file) calls + // the exact same `runChildProcess` helper the exact same way for its own + // bundler container. A real fix needs a streaming variant of + // `runChildProcess` used by both `functions` Docker paths, not a + // one-off change here — left open (review round on CLI-1963's `functions + // download` port). const result = yield* runChildProcess("docker", command, { stdout: "pipe", stderr: "pipe", }).pipe( Effect.mapError( - withDockerStepFailure("failed to run the edge-runtime unbundle container", slug), + withDockerStepFailure("failed to run the edge-runtime unbundle container", slug, styleAqua), ), ); @@ -1174,7 +1218,8 @@ const downloadWithDockerUnbundle = Effect.fnUntraced(function* ( result.stderr .split(/\r?\n/) .some((line) => line.trim().toLowerCase() === "invalid eszip v2"); - const suggestion = (invalidEszipV2 ? suggestDenoV2() : "") + suggestLegacyBundle(slug); + const suggestion = + (invalidEszipV2 ? suggestDenoV2() : "") + suggestLegacyBundle(slug, styleAqua); return yield* Effect.fail( Object.assign(new Error(`error running container: exit ${result.exitCode}`), { suggestion, @@ -1361,6 +1406,19 @@ export function downloadFunctions