From c85bfcda3005ed9154cc54629e3919451a42c22c Mon Sep 17 00:00:00 2001 From: Colum Ferry Date: Thu, 30 Jul 2026 14:11:03 +0100 Subject: [PATCH 1/2] fix(cli): match Go bundler env and deploy path anchoring (CLI-1985) Two functions deploy parity fixes against the pinned Go CLI: - Stop forwarding NPM_AUTH_TOKEN into the eszip Docker bundler container; Go forwards only NPM_CONFIG_REGISTRY (bundle.go:68-70). This reverts the TS-only forwarding from #5645 (the Go-side #4933 was closed unmerged). BREAKING for private-registry users whose .npmrc expands NPM_AUTH_TOKEN during --use-docker deploys: inline the token in .npmrc or deploy via --use-api instead. - Anchor API-deploy uploaded file names and the server-recorded entrypoint_path/import_map_path/static_patterns at the workdir, matching Go's toRelPath (relative to os.Getwd(), forward slashes). The git-root import-walk boundary from #5755 is kept, so monorepo imports outside the workdir still deploy, now with Go-style ../-relative names. --- .../commands/functions/deploy/SIDE_EFFECTS.md | 21 ++-- .../deploy/deploy.integration.test.ts | 104 ++++++++++++++++++ .../deploy/deploy.integration.test.ts | 28 ++--- apps/cli/src/shared/functions/deploy.ts | 54 +++++++-- 4 files changed, 176 insertions(+), 31 deletions(-) diff --git a/apps/cli/src/legacy/commands/functions/deploy/SIDE_EFFECTS.md b/apps/cli/src/legacy/commands/functions/deploy/SIDE_EFFECTS.md index caa742cc89..88402efeed 100644 --- a/apps/cli/src/legacy/commands/functions/deploy/SIDE_EFFECTS.md +++ b/apps/cli/src/legacy/commands/functions/deploy/SIDE_EFFECTS.md @@ -43,13 +43,13 @@ Docker bundling may pull or run the configured edge-runtime image and uses the ## Environment Variables -| Variable | Purpose | Required? | -| ---------------------------------- | ---------------------------------------------------- | ------------------------------------------------------- | -| `SUPABASE_ACCESS_TOKEN` | auth token (bypasses credential file/keyring lookup) | no (falls back to keyring → `~/.supabase/access-token`) | -| `SUPABASE_PROJECT_ID` | optional project ref fallback | no | -| `SUPABASE_INTERNAL_IMAGE_REGISTRY` | selects the Functions bundler image registry | no | -| `NPM_CONFIG_REGISTRY` | forwarded into Docker bundling when set | no | -| `DEBUG` | enables verbose Docker bundle output when `true` | no | +| Variable | Purpose | Required? | +| ---------------------------------- | --------------------------------------------------------------------------------------------------------------- | ------------------------------------------------------- | +| `SUPABASE_ACCESS_TOKEN` | auth token (bypasses credential file/keyring lookup) | no (falls back to keyring → `~/.supabase/access-token`) | +| `SUPABASE_PROJECT_ID` | optional project ref fallback | no | +| `SUPABASE_INTERNAL_IMAGE_REGISTRY` | selects the Functions bundler image registry | no | +| `NPM_CONFIG_REGISTRY` | forwarded into Docker bundling when set (the only npm variable forwarded, matching Go; `NPM_AUTH_TOKEN` is not) | no | +| `DEBUG` | enables verbose Docker bundle output when `true` | no | ## Exit Codes @@ -81,6 +81,13 @@ Legacy `--output` / `-o` does not change deploy output, matching the Go command. ## Notes - If no function name is provided, deploys all functions found in `supabase/functions/`. +- API-based deploys anchor uploaded file names and the recorded `entrypoint_path` / + `import_map_path` / `static_patterns` at the workdir, matching Go's `toRelPath` + (relative to `os.Getwd()`, forward slashes). Imports outside the workdir but inside + the nearest git root still upload, with `../`-relative names. The git-root + containment boundary is a TS-only safeguard with no Go equivalent — Go uploads any + reachable import unbounded; #5755 widened the TS boundary from the workdir to the + git root. - Requires a linked project unless `--project-ref` is provided. - Uses API/server-side bundling by default; `--use-docker` and `--legacy-bundle` select local bundling. - `--use-api`, `--use-docker`, and `--legacy-bundle` are mutually exclusive deploy modes. diff --git a/apps/cli/src/legacy/commands/functions/deploy/deploy.integration.test.ts b/apps/cli/src/legacy/commands/functions/deploy/deploy.integration.test.ts index 9f735d97c5..f5c42850e6 100644 --- a/apps/cli/src/legacy/commands/functions/deploy/deploy.integration.test.ts +++ b/apps/cli/src/legacy/commands/functions/deploy/deploy.integration.test.ts @@ -325,6 +325,110 @@ describe("legacy functions deploy", () => { ); }); + it.live("anchors API upload paths at the workdir when the git root is an ancestor", () => { + // Go parity (CLI-1985): Go's `toRelPath` (`pkg/function/deploy.go:94-103`) + // anchors uploaded file names and the server-recorded `entrypoint_path` / + // `import_map_path` at `os.Getwd()` — the workdir — never at the git root. + // Monorepo imports outside the workdir (allowed since #5755) upload with + // Go-style `../`-relative names. + const repoRoot = tempRoot.current; + const workdir = join(repoRoot, "app"); + const multiparts: Array<{ metadata?: string; fileNames: ReadonlyArray }> = []; + const out = mockOutput({ format: "text" }); + const api = mockLegacyPlatformApi({ + handler: (request) => { + if (request.body._tag === "FormData") { + const metadata = request.body.formData.get("metadata"); + multiparts.push({ + metadata: typeof metadata === "string" ? metadata : undefined, + fileNames: request.body.formData + .getAll("file") + .flatMap((part) => (part instanceof File ? [part.name] : [])), + }); + } + if (request.method === "GET") { + return Effect.succeed(legacyJsonResponse(request, 200, [])); + } + return Effect.succeed( + legacyJsonResponse(request, 201, { + id: "function-id", + slug: "hello-world", + name: "hello-world", + status: "ACTIVE", + version: 2, + created_at: 1_687_423_025_152, + updated_at: 1_687_423_025_152, + verify_jwt: true, + import_map: true, + entrypoint_path: "supabase/functions/hello-world/index.ts", + import_map_path: "supabase/functions/hello-world/deno.json", + }), + ); + }, + }); + const layer = Layer.mergeAll( + buildLegacyTestRuntime({ + out, + api, + cliConfig: mockLegacyCliConfig({ workdir }), + runtimeInfo: mockRuntimeInfo({ cwd: workdir }), + }), + Layer.succeed(LegacyYesFlag, false), + Stdio.layerTest({ + args: Effect.succeed(["functions", "deploy", "hello-world", "--use-api"]), + }), + ); + + return Effect.gen(function* () { + yield* Effect.tryPromise(() => mkdir(join(repoRoot, ".git"), { recursive: true })); + yield* Effect.tryPromise(() => writeProjectConfig(workdir)); + yield* Effect.tryPromise(() => + writeLocalFunction( + workdir, + "hello-world", + 'import { shared } from "@repo/shared"\nDeno.serve(() => new Response(shared))\n', + ), + ); + yield* Effect.tryPromise(() => + mkdir(join(repoRoot, "packages", "shared", "src"), { recursive: true }), + ); + yield* Effect.tryPromise(() => + writeFile( + join(repoRoot, "packages", "shared", "src", "index.ts"), + 'export const shared = "ok"\n', + ), + ); + yield* Effect.tryPromise(() => + writeFile( + join(workdir, "supabase", "functions", "hello-world", "deno.json"), + JSON.stringify({ + imports: { "@repo/shared": "../../../../packages/shared/src/index.ts" }, + }), + ), + ); + + yield* legacyFunctionsDeploy(baseFlags); + + expect(multiparts[0]?.metadata).toContain( + '"entrypoint_path":"supabase/functions/hello-world/index.ts"', + ); + expect(multiparts[0]?.metadata).toContain( + '"import_map_path":"supabase/functions/hello-world/deno.json"', + ); + expect(multiparts[0]?.fileNames).toContain("supabase/functions/hello-world/index.ts"); + expect(multiparts[0]?.fileNames).toContain("../packages/shared/src/index.ts"); + expect(out.stderrText).toContain( + "Uploading asset (hello-world): ../packages/shared/src/index.ts\n", + ); + expect(out.stderrText).not.toContain("WARN: Skipping import path outside source root:"); + }).pipe( + Effect.provide(layer), + Effect.ensuring( + Effect.tryPromise(() => rm(tempRoot.current, { recursive: true, force: true })), + ), + ); + }); + it.live("deploys config-declared custom entrypoints when deploying all functions", () => { const out = mockOutput({ format: "text" }); const api = mockLegacyPlatformApi({ diff --git a/apps/cli/src/next/commands/functions/deploy/deploy.integration.test.ts b/apps/cli/src/next/commands/functions/deploy/deploy.integration.test.ts index 5709a21d5a..2f7a93c340 100644 --- a/apps/cli/src/next/commands/functions/deploy/deploy.integration.test.ts +++ b/apps/cli/src/next/commands/functions/deploy/deploy.integration.test.ts @@ -1233,16 +1233,17 @@ describe("functions deploy", () => { functionNames: ["hello-world"], }).pipe(Effect.provide(layer)); - expect(api.multiparts[0]?.fileNames).toContain("app/supabase/functions/hello-world/index.ts"); - expect(api.multiparts[0]?.fileNames).toContain( - "app/supabase/functions/hello-world/deno.json", - ); - expect(api.multiparts[0]?.fileNames).toContain("packages/shared/src/index.ts"); + // Go parity (CLI-1985): names anchored at the workdir like Go's + // `toRelPath` (relative to `os.Getwd()`), so git-root workspace imports + // outside the workdir upload with `../`-relative names. + expect(api.multiparts[0]?.fileNames).toContain("supabase/functions/hello-world/index.ts"); + expect(api.multiparts[0]?.fileNames).toContain("supabase/functions/hello-world/deno.json"); + expect(api.multiparts[0]?.fileNames).toContain("../packages/shared/src/index.ts"); expect(api.multiparts[0]?.metadata).toContain( - '"entrypoint_path":"app/supabase/functions/hello-world/index.ts"', + '"entrypoint_path":"supabase/functions/hello-world/index.ts"', ); expect(api.multiparts[0]?.metadata).toContain( - '"import_map_path":"app/supabase/functions/hello-world/deno.json"', + '"import_map_path":"supabase/functions/hello-world/deno.json"', ); expect(out.stderrText).not.toContain("WARN: Skipping import path outside source root:"); }).pipe(Effect.ensuring(cleanupTempDir(repoRoot))); @@ -1290,9 +1291,9 @@ describe("functions deploy", () => { functionNames: ["hello-world"], }).pipe(Effect.provide(layer)); - expect(api.multiparts[0]?.fileNames).toContain("packages/shared/src/index.ts"); + expect(api.multiparts[0]?.fileNames).toContain("../packages/shared/src/index.ts"); expect(api.multiparts[0]?.metadata).toContain( - '"entrypoint_path":"app/supabase/functions/hello-world/index.ts"', + '"entrypoint_path":"supabase/functions/hello-world/index.ts"', ); }).pipe(Effect.ensuring(cleanupTempDir(repoRoot))); }); @@ -1600,7 +1601,7 @@ describe("functions deploy", () => { }).pipe(Effect.ensuring(cleanupTempDir(tempDir))); }); - it.live("forwards npm auth environment to the Docker bundler", () => { + it.live("forwards only NPM_CONFIG_REGISTRY to the Docker bundler", () => { const tempDir = makeTempDir(); const previousRegistry = process.env["NPM_CONFIG_REGISTRY"]; const previousToken = process.env["NPM_AUTH_TOKEN"]; @@ -1655,9 +1656,10 @@ describe("functions deploy", () => { args[index - 1] === "-e" ? [arg] : [], ); - expect(forwardedEnv).toEqual( - expect.arrayContaining(["NPM_CONFIG_REGISTRY", "NPM_AUTH_TOKEN"]), - ); + // Go parity (`bundle.go:68-70`, CLI-1985): only NPM_CONFIG_REGISTRY is + // forwarded into the bundler container; NPM_AUTH_TOKEN is not. + expect(forwardedEnv).toContain("NPM_CONFIG_REGISTRY"); + expect(forwardedEnv).not.toContain("NPM_AUTH_TOKEN"); expect(forwardedEnv).not.toContain("NPM_AUTH_TOKEN=test-token"); }).pipe(Effect.ensuring(Effect.all([cleanupTempDir(tempDir), restoreEnv]))); }); diff --git a/apps/cli/src/shared/functions/deploy.ts b/apps/cli/src/shared/functions/deploy.ts index ecbcc44049..897e594888 100644 --- a/apps/cli/src/shared/functions/deploy.ts +++ b/apps/cli/src/shared/functions/deploy.ts @@ -261,7 +261,14 @@ const dockerComposeProjectLabel = "com.docker.compose.project"; * directory using its OWN workdir rather than the caller's cwd. */ export const dockerWorkdirLabel = "com.supabase.cli.workdir"; -const dockerNpmEnvNames = ["NPM_CONFIG_REGISTRY", "NPM_AUTH_TOKEN"] as const; +/** + * Go parity (`apps/cli-go/internal/functions/deploy/bundle.go:68-70`): the eszip + * bundler container receives only `NPM_CONFIG_REGISTRY` from the host + * environment. `NPM_AUTH_TOKEN` is deliberately NOT forwarded — the Go-side PR + * proposing it (supabase/cli#4933) was closed unmerged, and CLI-1985 ruled + * strict parity over the TS-only forwarding that #5645 had added. + */ +const dockerNpmEnvNames = ["NPM_CONFIG_REGISTRY"] as const; export function dockerProjectLabels(projectId: string) { return { @@ -890,6 +897,7 @@ async function resolveImportMapAllowedRoots(projectRoot: string, importMapPath: async function writeSourceDeployForm( sourceRoot: string, + workdir: string, config: ResolvedDeployFunctionConfig, metadata: SourceDeployMetadata, outputRaw: (text: string) => Effect.Effect, @@ -905,7 +913,10 @@ async function writeSourceDeployForm( return; } uploadedAssets.add(realPathname); - const relativePath = toApiRelativePath(sourceRoot, pathname); + // Uploaded file names are anchored at the workdir like Go's `toRelPath` + // (`apps/cli-go/pkg/function/deploy.go:94-103`, relative to `os.Getwd()`), + // NOT at `sourceRoot` — see the CLI-1985 note in `deployViaApi`. + const relativePath = toApiRelativePath(workdir, pathname); await Effect.runPromise(outputRaw(`Uploading asset (${config.slug}): ${relativePath}\n`)); form.append("file", new File([contents], relativePath)); }; @@ -952,7 +963,7 @@ async function writeSourceDeployForm( importMap, pathname, importMapAllowedRoots, - sourceRoot, + workdir, uploadImportMapTargetAsset, async (message) => { await Effect.runPromise(outputRaw(message)); @@ -1006,7 +1017,7 @@ async function writeSourceDeployForm( importMap, config.entrypoint, [realSourceRoot], - sourceRoot, + workdir, uploadAsset, async (message) => { await Effect.runPromise(outputRaw(message)); @@ -1017,8 +1028,14 @@ async function writeSourceDeployForm( return form; } +/** + * Server-recorded metadata paths are anchored at the workdir, matching Go's + * `toRelPath` (`apps/cli-go/pkg/function/deploy.go:42-57,94-103`): relative to + * `os.Getwd()` (the Go CLI chdirs to the workdir), forward slashes via + * `filepath.ToSlash` — see the CLI-1985 note in `deployViaApi`. + */ function createSourceMetadata( - sourceRoot: string, + workdir: string, config: ResolvedDeployFunctionConfig, remote?: RemoteFunction, ): SourceDeployMetadata { @@ -1026,10 +1043,10 @@ function createSourceMetadata( return { name: config.slug, ...(verifyJwt === undefined ? {} : { verify_jwt: verifyJwt }), - entrypoint_path: toApiRelativePath(sourceRoot, config.entrypoint), + entrypoint_path: toApiRelativePath(workdir, config.entrypoint), import_map_path: - config.importMap.length > 0 ? toApiRelativePath(sourceRoot, config.importMap) : "", - static_patterns: config.staticFiles.map((pathname) => toApiRelativePath(sourceRoot, pathname)), + config.importMap.length > 0 ? toApiRelativePath(workdir, config.importMap) : "", + static_patterns: config.staticFiles.map((pathname) => toApiRelativePath(workdir, pathname)), }; } @@ -1545,6 +1562,7 @@ const uploadFunctionSource = Effect.fnUntraced(function* ( api: ApiClient, projectRef: string, sourceRoot: string, + workdir: string, config: ResolvedDeployFunctionConfig, metadata: SourceDeployMetadata, bundleOnly: boolean, @@ -1552,7 +1570,7 @@ const uploadFunctionSource = Effect.fnUntraced(function* ( const output = yield* Output; const files = yield* Effect.tryPromise({ try: async () => { - const form = await writeSourceDeployForm(sourceRoot, config, metadata, (text) => + const form = await writeSourceDeployForm(sourceRoot, workdir, config, metadata, (text) => output.raw(text, "stderr"), ); return form.getAll("file").flatMap((part) => (part instanceof Blob ? [part] : [])); @@ -1940,6 +1958,18 @@ const deployViaApi = Effect.fnUntraced(function* ( jobs: number, ) { const output = yield* Output; + // CLI-1985: uploaded file names and the server-recorded metadata paths + // (`entrypoint_path`, `import_map_path`, `static_patterns`) are anchored at the + // workdir (`projectRoot`), matching the pinned Go CLI's `toRelPath`, which is + // relative to `os.Getwd()` after the CLI chdirs to the workdir + // (`apps/cli-go/pkg/function/deploy.go:94-103`, `internal/utils/misc.go:238`). + // Upstream Go never anchored deploy paths at the git root — that was a TS-only + // divergence introduced by #5755. The import-walk *boundary* (which files may + // be uploaded at all) intentionally stays at the nearest git root: the boundary + // itself is a TS-only safeguard with no Go equivalent (Go's `WalkImportPaths` + // uploads any reachable import unbounded; #5755 widened the TS boundary from + // the workdir to the git root so monorepo imports outside the workdir deploy). + // Such files upload with Go-`toRelPath`-style `../`-relative names. const sourceRoot = yield* Effect.tryPromise({ try: () => resolveFunctionsSourceRoot(projectRoot), catch: (error) => (error instanceof Error ? error : new Error(String(error))), @@ -1965,8 +1995,9 @@ const deployViaApi = Effect.fnUntraced(function* ( api, projectRef, sourceRoot, + projectRoot, config, - createSourceMetadata(sourceRoot, config, remoteBySlug.get(config.slug)), + createSourceMetadata(projectRoot, config, remoteBySlug.get(config.slug)), false, ); return; @@ -1982,8 +2013,9 @@ const deployViaApi = Effect.fnUntraced(function* ( api, projectRef, sourceRoot, + projectRoot, config, - createSourceMetadata(sourceRoot, config, remoteBySlug.get(config.slug)), + createSourceMetadata(projectRoot, config, remoteBySlug.get(config.slug)), true, ), ); From e1b18f3d479f1fca5f2a6a21bdbb9e61f7fe9850 Mon Sep 17 00:00:00 2001 From: Colum Ferry Date: Mon, 3 Aug 2026 10:53:17 +0100 Subject: [PATCH 2/2] fix(cli): reject deploy uploads whose relative name escapes the workdir (CLI-1985) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Go's writeForm/addFile opens every uploaded path through an fs.FS, which rejects any path containing a ".." element via fs.ValidPath before the read (and thus the upload) happens. A workdir≠git-root layout could make the TS deploy path anchoring produce a multipart File name like "../packages/shared/src/index.ts" that escapes the workdir and reaches the server for the first time from any CLI. Hard-fail with the same Go-parity error before any upload is attempted, and update the deploy integration tests that had been asserting the escaping upload succeeded. --- .../deploy/deploy.integration.test.ts | 35 ++-- .../deploy/deploy.integration.test.ts | 187 ++++++++++-------- apps/cli/src/shared/functions/deploy.ts | 18 ++ 3 files changed, 144 insertions(+), 96 deletions(-) diff --git a/apps/cli/src/legacy/commands/functions/deploy/deploy.integration.test.ts b/apps/cli/src/legacy/commands/functions/deploy/deploy.integration.test.ts index b2d575fa14..fdbc2dbdd5 100644 --- a/apps/cli/src/legacy/commands/functions/deploy/deploy.integration.test.ts +++ b/apps/cli/src/legacy/commands/functions/deploy/deploy.integration.test.ts @@ -1,7 +1,7 @@ import { describe, expect, it } from "@effect/vitest"; import { mkdir, rm, writeFile } from "node:fs/promises"; import { join } from "node:path"; -import { Effect, Layer, Option, Stdio } from "effect"; +import { Effect, Exit, Layer, Option, Stdio } from "effect"; import { LegacyYesFlag } from "../../../../shared/legacy/global-flags.ts"; import { @@ -325,12 +325,18 @@ describe("legacy functions deploy", () => { ); }); - it.live("anchors API upload paths at the workdir when the git root is an ancestor", () => { + it.live("rejects a bundled file whose workdir-relative name escapes with a `..` segment", () => { // Go parity (CLI-1985): Go's `toRelPath` (`pkg/function/deploy.go:94-103`) // anchors uploaded file names and the server-recorded `entrypoint_path` / // `import_map_path` at `os.Getwd()` — the workdir — never at the git root. - // Monorepo imports outside the workdir (allowed since #5755) upload with - // Go-style `../`-relative names. + // A monorepo import outside the workdir but inside the git root (allowed + // by the source-root containment check since #5755) would otherwise + // upload with a Go-style `../`-relative name. Go's `writeForm`/`addFile` + // (`pkg/function/deploy.go:251-284`) opens every uploaded path through an + // `fs.FS`, which rejects any path containing a `..` element (`fs.ValidPath`) + // before the read — and thus the upload — happens. This asserts the CLI + // hard-fails the same way instead of letting the `..`-relative name reach + // the server. const repoRoot = tempRoot.current; const workdir = join(repoRoot, "app"); const multiparts: Array<{ metadata?: string; fileNames: ReadonlyArray }> = []; @@ -407,20 +413,15 @@ describe("legacy functions deploy", () => { ), ); - yield* legacyFunctionsDeploy(baseFlags); + const exit = yield* Effect.exit(legacyFunctionsDeploy(baseFlags)); - expect(multiparts[0]?.metadata).toContain( - '"entrypoint_path":"supabase/functions/hello-world/index.ts"', - ); - expect(multiparts[0]?.metadata).toContain( - '"import_map_path":"supabase/functions/hello-world/deno.json"', - ); - expect(multiparts[0]?.fileNames).toContain("supabase/functions/hello-world/index.ts"); - expect(multiparts[0]?.fileNames).toContain("../packages/shared/src/index.ts"); - expect(out.stderrText).toContain( - "Uploading asset (hello-world): ../packages/shared/src/index.ts\n", - ); - expect(out.stderrText).not.toContain("WARN: Skipping import path outside source root:"); + expect(Exit.isFailure(exit)).toBe(true); + if (Exit.isFailure(exit)) { + expect(String(exit.cause)).toContain( + "failed to read file: open ../packages/shared/src/index.ts: invalid argument", + ); + } + expect(multiparts).toHaveLength(0); }).pipe( Effect.provide(layer), Effect.ensuring( diff --git a/apps/cli/src/next/commands/functions/deploy/deploy.integration.test.ts b/apps/cli/src/next/commands/functions/deploy/deploy.integration.test.ts index 103695cda8..ea8fe70489 100644 --- a/apps/cli/src/next/commands/functions/deploy/deploy.integration.test.ts +++ b/apps/cli/src/next/commands/functions/deploy/deploy.integration.test.ts @@ -857,7 +857,12 @@ describe("functions deploy", () => { }).pipe(Effect.ensuring(cleanupTempDir(tempDir))); }); - it.live("uploads an explicit import map outside the project root", () => { + it.live("rejects an explicit import map outside the project root", () => { + // Go parity (CLI-1985): `--import-map` outside the workdir resolves to a + // `..`-relative name via Go's `toRelPath`, same as an auto-discovered + // monorepo import — Go's `writeForm`/`addFile` rejects any such path via + // `fs.ValidPath` before the upload happens, regardless of how the escaping + // path was reached. const tempDir = makeTempDir(); const projectDir = join(tempDir, "project"); const sharedDir = join(tempDir, "shared"); @@ -880,21 +885,26 @@ describe("functions deploy", () => { ], }); - yield* functionsDeploy({ - ...BASE_FLAGS, - functionNames: ["hello-world"], - importMap: Option.some("../shared/import_map.json"), - }).pipe(Effect.provide(layer)); - - expect(api.multiparts[0]?.metadata).toContain( - '"import_map_path":"../shared/import_map.json"', + const exit = yield* Effect.exit( + functionsDeploy({ + ...BASE_FLAGS, + functionNames: ["hello-world"], + importMap: Option.some("../shared/import_map.json"), + }).pipe(Effect.provide(layer)), ); - expect(api.multiparts[0]?.fileNames).toContain("../shared/import_map.json"); + + expect(Exit.isFailure(exit)).toBe(true); + if (Exit.isFailure(exit)) { + expect(String(exit.cause)).toContain( + "failed to read file: open ../shared/import_map.json: invalid argument", + ); + } + expect(api.multiparts).toHaveLength(0); }).pipe(Effect.ensuring(cleanupTempDir(tempDir))); }); it.live( - "uploads local targets referenced by an explicit import map outside the project root", + "rejects local targets referenced by an explicit import map outside the project root", () => { const tempDir = makeTempDir(); const projectDir = join(tempDir, "project"); @@ -933,15 +943,21 @@ describe("functions deploy", () => { ], }); - yield* functionsDeploy({ - ...BASE_FLAGS, - functionNames: ["hello-world"], - importMap: Option.some("../shared/import_map.json"), - }).pipe(Effect.provide(layer)); + const exit = yield* Effect.exit( + functionsDeploy({ + ...BASE_FLAGS, + functionNames: ["hello-world"], + importMap: Option.some("../shared/import_map.json"), + }).pipe(Effect.provide(layer)), + ); - expect(api.multiparts[0]?.fileNames).toContain("../shared/import_map.json"); - expect(api.multiparts[0]?.fileNames).toContain("../shared/lib.ts"); - expect(api.multiparts[0]?.fileNames).toContain("../shared/helper.ts"); + expect(Exit.isFailure(exit)).toBe(true); + if (Exit.isFailure(exit)) { + expect(String(exit.cause)).toContain( + "failed to read file: open ../shared/import_map.json: invalid argument", + ); + } + expect(api.multiparts).toHaveLength(0); }).pipe(Effect.ensuring(cleanupTempDir(tempDir))); }, ); @@ -1315,63 +1331,71 @@ describe("functions deploy", () => { }).pipe(Effect.ensuring(Effect.all([cleanupTempDir(tempDir), cleanupTempDir(outsideDir)]))); }); - it.live("uploads git-root workspace imports through the API", () => { - const repoRoot = makeTempDir(); - const projectRoot = join(repoRoot, "app"); - const sharedPath = join(repoRoot, "packages", "shared", "src", "index.ts"); + it.live( + "rejects a git-root workspace import that escapes the workdir with a `..` segment", + () => { + // Go parity (CLI-1985): names are anchored at the workdir like Go's + // `toRelPath` (relative to `os.Getwd()`), so a git-root workspace import + // outside the workdir resolves to a `..`-relative name. Go's + // `writeForm`/`addFile` (`pkg/function/deploy.go:251-284`) opens every + // uploaded path through an `fs.FS`, which rejects any path containing a + // `..` element (`fs.ValidPath`) before the read — and thus the upload — + // happens. Assert the CLI hard-fails the same way instead of letting a + // `..`-relative name reach the server. + const repoRoot = makeTempDir(); + const projectRoot = join(repoRoot, "app"); + const sharedPath = join(repoRoot, "packages", "shared", "src", "index.ts"); - return Effect.gen(function* () { - yield* Effect.promise(() => mkdir(join(repoRoot, ".git"), { recursive: true })); - yield* Effect.promise(() => writeProjectConfig(projectRoot)); - yield* Effect.promise(() => - writeLocalFunction( - projectRoot, - "hello-world", - [ - 'import { shared } from "@repo/shared"', - "Deno.serve(() => new Response(shared))", - "", - ].join("\n"), - ), - ); - yield* Effect.promise(() => mkdir(dirname(sharedPath), { recursive: true })); - yield* Effect.promise(() => writeFile(sharedPath, 'export const shared = "ok"\n')); - yield* Effect.promise(() => - writeFile( - join(projectRoot, "supabase", "functions", "hello-world", "deno.json"), - JSON.stringify({ - imports: { "@repo/shared": "../../../../packages/shared/src/index.ts" }, - }), - ), - ); + return Effect.gen(function* () { + yield* Effect.promise(() => mkdir(join(repoRoot, ".git"), { recursive: true })); + yield* Effect.promise(() => writeProjectConfig(projectRoot)); + yield* Effect.promise(() => + writeLocalFunction( + projectRoot, + "hello-world", + [ + 'import { shared } from "@repo/shared"', + "Deno.serve(() => new Response(shared))", + "", + ].join("\n"), + ), + ); + yield* Effect.promise(() => mkdir(dirname(sharedPath), { recursive: true })); + yield* Effect.promise(() => writeFile(sharedPath, 'export const shared = "ok"\n')); + yield* Effect.promise(() => + writeFile( + join(projectRoot, "supabase", "functions", "hello-world", "deno.json"), + JSON.stringify({ + imports: { "@repo/shared": "../../../../packages/shared/src/index.ts" }, + }), + ), + ); - const { out, api, layer } = setup(projectRoot, { - projectRoot, - rawArgs: ["functions", "deploy", "hello-world"], - }); + const { out, api, layer } = setup(projectRoot, { + projectRoot, + rawArgs: ["functions", "deploy", "hello-world"], + }); - yield* functionsDeploy({ - ...BASE_FLAGS, - functionNames: ["hello-world"], - }).pipe(Effect.provide(layer)); + const exit = yield* Effect.exit( + functionsDeploy({ + ...BASE_FLAGS, + functionNames: ["hello-world"], + }).pipe(Effect.provide(layer)), + ); - // Go parity (CLI-1985): names anchored at the workdir like Go's - // `toRelPath` (relative to `os.Getwd()`), so git-root workspace imports - // outside the workdir upload with `../`-relative names. - expect(api.multiparts[0]?.fileNames).toContain("supabase/functions/hello-world/index.ts"); - expect(api.multiparts[0]?.fileNames).toContain("supabase/functions/hello-world/deno.json"); - expect(api.multiparts[0]?.fileNames).toContain("../packages/shared/src/index.ts"); - expect(api.multiparts[0]?.metadata).toContain( - '"entrypoint_path":"supabase/functions/hello-world/index.ts"', - ); - expect(api.multiparts[0]?.metadata).toContain( - '"import_map_path":"supabase/functions/hello-world/deno.json"', - ); - expect(out.stderrText).not.toContain("WARN: Skipping import path outside source root:"); - }).pipe(Effect.ensuring(cleanupTempDir(repoRoot))); - }); + expect(Exit.isFailure(exit)).toBe(true); + if (Exit.isFailure(exit)) { + expect(String(exit.cause)).toContain( + "failed to read file: open ../packages/shared/src/index.ts: invalid argument", + ); + } + expect(api.multiparts).toHaveLength(0); + expect(out.stderrText).not.toContain("WARN: Skipping import path outside source root:"); + }).pipe(Effect.ensuring(cleanupTempDir(repoRoot))); + }, + ); - it.live("treats a .git file as the repo root marker for API uploads", () => { + it.live("rejects an escaping import even when a `.git` file marks the repo root", () => { const repoRoot = makeTempDir(); const projectRoot = join(repoRoot, "app"); const sharedPath = join(repoRoot, "packages", "shared", "src", "index.ts"); @@ -1408,15 +1432,20 @@ describe("functions deploy", () => { rawArgs: ["functions", "deploy", "hello-world"], }); - yield* functionsDeploy({ - ...BASE_FLAGS, - functionNames: ["hello-world"], - }).pipe(Effect.provide(layer)); - - expect(api.multiparts[0]?.fileNames).toContain("../packages/shared/src/index.ts"); - expect(api.multiparts[0]?.metadata).toContain( - '"entrypoint_path":"supabase/functions/hello-world/index.ts"', + const exit = yield* Effect.exit( + functionsDeploy({ + ...BASE_FLAGS, + functionNames: ["hello-world"], + }).pipe(Effect.provide(layer)), ); + + expect(Exit.isFailure(exit)).toBe(true); + if (Exit.isFailure(exit)) { + expect(String(exit.cause)).toContain( + "failed to read file: open ../packages/shared/src/index.ts: invalid argument", + ); + } + expect(api.multiparts).toHaveLength(0); }).pipe(Effect.ensuring(cleanupTempDir(repoRoot))); }); diff --git a/apps/cli/src/shared/functions/deploy.ts b/apps/cli/src/shared/functions/deploy.ts index d3d3bacab2..a04077e212 100644 --- a/apps/cli/src/shared/functions/deploy.ts +++ b/apps/cli/src/shared/functions/deploy.ts @@ -319,6 +319,21 @@ function isContainedInAnyPath(roots: ReadonlyArray, candidate: string) { return roots.some((root) => isContainedPath(root, candidate)); } +/** + * Go parity (`apps/cli-go/pkg/function/deploy.go:251-284`, via + * `afero.IOFS.Open` → `fs.ValidPath`): `writeForm`'s `addFile` opens every + * uploaded path through an `fs.FS`, which rejects any path containing a `..` + * element before the read (and thus the upload) happens. A workdir≠git-root + * layout can otherwise produce a multipart `File` name like + * `../packages/shared/src/index.ts` that escapes the anchor dir — reject it + * the same way Go does, before any upload is attempted. + */ +function hasParentPathSegment(relativePath: string) { + return toSlash(relativePath) + .split("/") + .some((segment) => segment === ".."); +} + async function realpathIfExists(pathname: string) { try { return await realpath(resolve(pathname)); @@ -917,6 +932,9 @@ async function writeSourceDeployForm( // (`apps/cli-go/pkg/function/deploy.go:94-103`, relative to `os.Getwd()`), // NOT at `sourceRoot` — see the CLI-1985 note in `deployViaApi`. const relativePath = toApiRelativePath(workdir, pathname); + if (hasParentPathSegment(relativePath)) { + throw new Error(`failed to read file: open ${relativePath}: invalid argument`); + } await Effect.runPromise(outputRaw(`Uploading asset (${config.slug}): ${relativePath}\n`)); form.append("file", new File([contents], relativePath)); };