fix(cli): match Go bundler env and deploy path anchoring (CLI-1985) - #6005
Merged
Coly010 merged 3 commits intoAug 3, 2026
Conversation
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.
Contributor
Author
|
@codex review |
|
Codex Review: Didn't find any major issues. Breezy! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
…5-functions-deploy-rulings-npm_auth_token-forwarding-and-git
Coly010
marked this pull request as ready for review
July 31, 2026 09:55
Contributor
Supabase CLI previewnpx --yes https://pkg.pr.new/supabase/cli/supabase@e1b18f3d479f1fca5f2a6a21bdbb9e61f7fe9850Preview package for commit |
kanadgupta
approved these changes
Jul 31, 2026
kanadgupta
left a comment
kanadgupta
left a comment
Member
There was a problem hiding this comment.
Claude discovered a potential issue where the server might reject the file name and suggests trying this yourself — I think this would be better suited for the e2e tests I'm hoping to work on soon. Will defer to you on the best course of action here — LGTM otherwise!
…ir (CLI-1985) 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.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e1b18f3d47
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Coly010
deleted the
columferry/cli-1985-functions-deploy-rulings-npm_auth_token-forwarding-and-git
branch
August 3, 2026 10:27
pull Bot
pushed a commit
to oogalieboogalie/cli
that referenced
this pull request
Aug 5, 2026
…audit (CLI-1967) (supabase#6074) ## What Doc/comment-accuracy sweep from `apps/cli/docs/go-parity-audit-2026-07-24.md` §6 (untracked locally, not part of this repo). No runtime behavior changes — every item below was independently re-verified against current Go (`apps/cli-go/`) and TS source before fixing, since the audit is 12 days old and several items had drifted further in that time (in both directions). ## Fixed - **`docs/go-cli-porting-status.md`** — `functions list` legacy-shell status `wrapped` → `ported` (verified: zero `LegacyGoProxy` refs). `functions download` status corrected the other way, `ported` → `wrapped` (its handler still delegates wholesale via `LegacyGoProxy` for the default `--use-docker` path; only `--use-api` is native — noted as a hybrid in the row). Added a `ported` definition to the legacy-status legend (it only defined `wrapped`/`missing`, despite `ported` being ~90% of rows) and retitled that section from "Legacy Shell Wrapping Status" to "Legacy Shell Command Status" to match. Fixed the adjacent `functions delete/deploy/list/new/serve` legacy-shell notes, which all still said "Wrapped in legacy shell" despite being natively ported. - **`legacy-pgdelta.seam.service.ts`** — fixed two stale doc comments (`execInherit`, `ensureLocalDatabaseStarted`) that referenced things as "not yet ported" when they now are (`db reset`, `start`/`db start`). Did **not** touch `exportCatalog`'s doc comment / `LegacyCatalogMode`, which the audit also flagged — that exact hunk is already being rewritten more completely by the in-flight CLI-1959 PR (supabase#6061, open); fixing it here would guarantee a conflict with a strictly better version. `legacy-db-bootstrap.seam.service.ts` (the audit's other named file) was checked and found already accurate — CLI-1954/1955 (native `db start`/`db reset --local`) are still unmerged, so its "not yet ported" claim is currently true. - **`network-restrictions/{get,update}/SIDE_EFFECTS.md`** — the `-o {json,yaml,toml,env}` sections previously implied Go itself produces distinct byte-identical output per format. Verified against Go source: `restrictions/get`/`update` never read `OutputFormat` at all — they always print the same 3-line `fmt.Printf` template regardless of `-o`. Corrected both docs to state this plainly, documented that TS's format-specific output here is a deliberate TS-only enhancement with no real Go behavior to match (including no Go casing convention, since TS uses the map-shaped encoders rather than CLI-1975's struct-spec ones), and trimmed the resulting repetition. - **`inspect/report/SIDE_EFFECTS.md`** — added the empty/no-file divergence on `COPY` failure: Go's `copyToCSV` opens the output file with `O_TRUNC` before running the query, so a failing query still leaves a file (empty or partial); TS buffers in memory and only writes on success, leaving no file on a fresh run — and leaving the *previous* run's stale CSV in place on a same-day re-run (the more consequential case). Cross-referenced from `legacy-db-connection.errors.ts`'s `LegacyDbCopyError` doc comment, which already covered the message-text angle of the same divergence. - **`domains.cname.ts`** — the comment describing Go's CNAME "failed to locate" error dump wrongly implied Go embeds readable JSON. Verified against Go source and empirically (compiled the equivalent locally): Go JSON-marshals the answers to a `[]byte`, then formats that `[]byte` with `%+v`, which Go's `fmt` renders as an uncapped decimal byte-value array, not the JSON text — a `%+v`-on-`[]byte` footgun, not an intended format. Fixed both the function's JSDoc and the inline comment (they'd contradicted each other after an earlier pass), and cross-referenced the divergence from `domains/SIDE_EFFECTS.md`. - **`branches/orgs/projects/secrets` SIDE_EFFECTS `-o toml`/`-o yaml` claims**, **`functions deploy`'s `NPM_AUTH_TOKEN` env table entry**, and **`update-root-key.handler.ts`'s color comment** — all already fixed by CLI-1975 (supabase#6002), CLI-1985 (supabase#6005), and CLI-1990 (supabase#5978) respectively, which merged after the audit ran. Verified current state matches; no changes needed. - **`start/SIDE_EFFECTS.md`'s `--ignore-health-check` ruling** — already fully handled by CLI-1987 (supabase#6007, merged), whose own description explicitly says CLI-1967 should not re-document it. Left untouched. ## Left as noted, not fixed (out of scope for a docs-only pass) - The `network-restrictions get`/`update` TS-only `-o` support is a real, pre-existing behavioral divergence from Go (Go has no such behavior at all for these two commands) — documented accurately here, but whether it should be *removed* to enforce strict parity is a ruling this PR doesn't make. - `docs/go-cli-porting-status.md`'s "Functions" section (next/-shell table) has a larger, pre-existing inaccuracy discovered while fixing the adjacent legacy-shell notes: it claims there's "still no dedicated `functions` CLI surface" in `next/`, but `next/commands/functions/` already exists (list/delete/deploy/download/new/dev, registered in `next/cli/root.ts`). Added command-path links and flagged the section as needing its own flag-by-flag parity audit rather than silently reclassifying rows without one. Fixes CLI-1967
avallete
pushed a commit
that referenced
this pull request
Aug 7, 2026
Verified every bullet against the actual v2.111.0..v2.112.0 tree diff (the range came from a rebased develop rollup, so commit subjects alone were misleading) and applied the Codex review feedback: - Drop #6072 and #6080: packages/next-only, not in the stable shell - Drop #6003 and #6057: the stable CLI does not exercise the stack binary cache; #6057's "rollup" was the develop->main rollup PR, not the Rollup bundler - Add breaking-change section for NPM_AUTH_TOKEN no longer being forwarded by functions deploy (#6005, ships in this tag) - Note the one-time stop/start needed for already-stuck stacks (#6093) - Scope #5956 to functions download path traversal - Point db pull --experimental users at --declarative (#6028) - Fix #6065 and #6066 descriptions to match the shipped changes - Recount the internal tail (33 uncited commits in the compare range) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018h8krKpNet85649nqtj95b
ivasilov
added a commit
to supabase/supabase
that referenced
this pull request
Aug 19, 2026
…tudio E2E `Start supabase` flake) (#49198) <!-- ccr-slack-attribution --> _Requested by **Ivan Vasilov** · [Slack thread](https://supabase.slack.com/archives/C063LNYJJKS/p1787058646458219?thread_ts=1787058646.458219&cid=C063LNYJJKS)_ **Before:** the root `package.json` pins the Supabase CLI at `supabase: ^2.76.10`, and `pnpm-lock.yaml` resolves it to `2.76.14`. **After:** it pins `supabase: ^2.114.0`. This bumps the Supabase CLI that `pnpm run e2e:setup:cli` and `pnpm run setup:cli` shell out to, so local dev and the E2E workflows boot the local stack with a CLI from this month instead of one from ~38 minor releases ago. **How:** a one-line version change to the `supabase` devDependency in the root `package.json`. Nothing else in the repo changes — no workflow, config, or test changes. ###⚠️ This PR is incomplete: `pnpm-lock.yaml` still needs regenerating `pnpm-lock.yaml` is **not** updated in this PR, so `pnpm install --frozen-lockfile` will fail until someone runs: ```bash pnpm install --lockfile-only ``` and pushes the result to this branch. The lockfile could not be regenerated in the environment this PR was authored in: pnpm re-resolves `apps/studio`'s `"@std/path": "npm:@jsr/std__path@^1.0.8"` on every install, and `npm.jsr.io` is not reachable from there (`ERR_PNPM_FETCH_403`). Treat this PR as needing one extra commit before it can go green. ### Why `^2.114.0` and not `^2.115.0` `2.115.0` is the current `latest` on npm, but it was published only hours ago, and `pnpm-workspace.yaml` sets `minimumReleaseAge: 4320` (3 days) with `supabase` not in `minimumReleaseAgeExclude`. Pinning `2.115.0` today would fail the repo's own supply-chain check. `2.114.0` (2026-08-12) is the newest release that satisfies that policy. ## I have read the [CONTRIBUTING.md](https://github.com/supabase/supabase/blob/master/CONTRIBUTING.md) file. YES ## What kind of change does this PR introduce? Dependency bump. **Speculative** — this is an experiment, not a confirmed fix. ## What is the current behavior? The `Selfhosted Studio E2E Tests` workflow has been failing on `master` at the `Start supabase` step. Recent runs: - https://github.com/supabase/supabase/actions/runs/32092940311 - https://github.com/supabase/supabase/actions/runs/32131961447 In the Slack thread, Ivan Vasilov suggested trying a newer CLI and Alaister Young endorsed giving it a go. ## What is the new behavior? The workflow runs `supabase start` with CLI 2.114.0 instead of 2.76.14. The question this PR is trying to answer is simply **"does a newer CLI help this flake?"** It is not a diagnosis and not a claimed fix. If CI still fails at `Start supabase` on this branch, the bump can be kept or dropped on its own merits and the investigation continues elsewhere. ## Additional context **Verification status:** none locally. The bump was not exercised locally — this repo checkout has no `node_modules` (see the lockfile note above), so `pnpm typecheck`, `pnpm lint`, and `pnpm test:studio` were not run, and neither was `supabase start`. CI on this PR is the only signal. **Call-site compatibility check.** CLI 2.99/2.100 moved to a new TypeScript shell with a stricter argument parser: command-specific flags must now come *after* the subcommand. Both call sites in the root `package.json` already use that order, so no script changes are needed: ``` supabase stop --all --no-backup --workdir ./e2e/studio supabase start --exclude studio,mailpit --workdir ./e2e/studio ``` **Changelog entries between 2.76.14 and 2.114.0 that touch `supabase start` or local config.** Listed so reviewers know what changed in the range — **not** as a claim about what is failing in CI: - **2.112.0** — `supabase start` no longer hangs when analytics migrations fail; the analytics container exits and retries instead of booting against an unmigrated database ([#6093](supabase/cli#6093)). - **2.112.0** — `supabase start` reuses existing volumes instead of failing when they already exist ([#6037](supabase/cli#6037)); Kong reloads after `supabase db reset` ([#6017](supabase/cli#6017)); custom auth email templates survive `db reset` ([#6065](supabase/cli#6065)); `supabase start` works on SELinux-enforcing hosts ([#6000](supabase/cli#6000)). - **2.106.0 — behavior change worth watching.** `[api].auto_expose_new_tables` now resolves to `false` when unset, and local start/reset revokes default Data API privileges for newly created `public` tables, sequences, and functions ([#5524](supabase/cli#5524)). Neither `supabase/config.toml` nor `e2e/studio/supabase/config.toml` sets this key, so this default applies. If E2E specs create `public` objects and then read them through the Data API, they may need explicit `GRANT`s (the deprecated escape hatch is `auto_expose_new_tables = true`). - **2.106.0** — when the CLI detects a coding-agent environment, or `--agent yes` is passed, commands default to JSON output ([#5532](supabase/cli#5532)). `e2e:setup:cli` already passes `--output json` to `supabase status` explicitly, so this should be a no-op here. - **2.100.0** — stricter flag ordering, covered above. - **2.112.0** — `functions deploy` no longer forwards `NPM_AUTH_TOKEN` into Docker bundling ([#6005](supabase/cli#6005)). Not used by these workflows. - **2.107.0** — pg-delta is the default schema diff engine for `db diff` / `db pull` on new projects ([#5511](supabase/cli#5511)). - Many bundled Docker image bumps across the range (`supabase/postgres` 17.6.1.087 → later patches, `postgres-meta`, `vector` 0.28.1 → 0.53.0, Studio image), plus `fix(analytics): wait for logflare before starting vector` (2.84.3) and `fix: use correct docker.sock binding with vector` (2.84.7). Full comparison: supabase/cli@v2.76.14...v2.114.0 --- _Generated by [Claude Code](https://claude.ai/code/session_0143DrDMGnSSwuHebTPJv7ZY)_ --------- Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: Ivan Vasilov <vasilov.ivan@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two
functions deploydivergences from the pinned Go CLI (apps/cli-go), resolved per the CLI-1985 ruling (Colum, 2026-07-30: take each point's documented recommended option).Fixes CLI-1985
⚖ Parity ruling applied
Point 1 —
NPM_AUTH_TOKENis no longer forwarded into the Docker bundler (strict parity; breaking for private-registry users)Decision: remove the forwarding. The eszip bundler container now receives only
NPM_CONFIG_REGISTRYfrom the host, exactly matching Go (apps/cli-go/internal/functions/deploy/bundle.go:68-70).Evidence:
NPM_AUTH_TOKENat any point in its history — onlyNPM_CONFIG_REGISTRY(added in8e17f033).functions deployfails to read.npmrcenvironment variables for private npm registry auth (workspacedeno.json) #4927) was closed unmerged on 2026-06-22 ("The command is now ported in TypeScript so I'm closing this PR").User-visible change (flagging prominently): users whose
.npmrcexpands${NPM_AUTH_TOKEN}for private npm registries will find--use-docker/--legacy-bundledeploys failing registry auth again (the pre-#5645 and Go CLI behavior; re-opens the CI/CD-host case of #4927). Workarounds: inline the token in.npmrc, or deploy via the default--use-apipath. Per the strict-parity contract (stderr bytes included), no TS-only warning was added when the variable is set — a DX reviewer requested one and it was rejected on parity grounds; the breaking impact is documented here and in the commit message instead.Shared-code caveat (per the ruling):
dockerNpmEnvlives inapps/cli/src/shared/functions/deploy.tsand serves both shells —next/(functions deploy) andlegacy/. The removal therefore applies to the next/ shell too. The strict-parity contract only binds the legacy shell, but keeping one code path is the simplest correct design per repo policy, so next/ loses the forwarding as well — stated here explicitly.functions serveis unaffected (it has its own env handling, matching Go's serve which loadssupabase/functions/.env).Point 2 — API-deploy upload paths re-anchored at the workdir (align to the pinned oracle; behaviour change)
Directive: confirm the intended reference point first, then align or record.
Evidence found:
pkg/function/deploy.go(pre- and post-monorepo move) showstoRelPathanchored atos.Getwd()since29021998("convert all paths to relative for deploy", fix: convert all paths to relative for deploy #3403), unchanged since. The Go CLI chdirs to the workdir (internal/utils/misc.go:238), soos.Getwd()≡ the workdir.failed to read file: open ../common/index.ts) — not a port of newer upstream Go behavior. There is no newer upstream Go reference to record against.Decision (per the ruling's matrix — upstream never did this → align): uploaded multipart file names and the server-recorded
entrypoint_path/import_map_path/static_patternsare now anchored at the workdir with Go's exacttoRelPathsemantics (relative toos.Getwd(), forward slashes,../-relative when the file lies outside the workdir).Scope note: #5755's import-walk containment boundary (which files may be uploaded at all) is intentionally kept at the nearest git root. The boundary is a TS-only safeguard with no Go equivalent — Go's walker uploads any reachable import unbounded (and then hard-fails opening
..paths throughafero.NewIOFS, which is exactly bug #3467). Reverting the boundary would re-break #3467 and is outside CLI-1985's anchoring scope.User-visible change: in monorepos where the git root is an ancestor of the workdir, redeploys now record
supabase/functions/<slug>/index.ts(matching what the Go CLI records and the dashboard shows for Go deploys) instead ofapps/myapp/supabase/functions/<slug>/index.ts. Imports outside the workdir but inside the git root still deploy, uploading with Go-style../-relative names — the same name shape Go'stoRelPathemits, so nothing new is required of the server. Non-monorepo projects (git root == workdir, the common case) are byte-for-byte unchanged.What changed
apps/cli/src/shared/functions/deploy.ts—dockerNpmEnvNamestrimmed toNPM_CONFIG_REGISTRY;deployViaApinow threads the workdir as the path anchor throughuploadFunctionSource/writeSourceDeployForm/createSourceMetadatawhile the git-rootsourceRootremains the containment boundary; ENOENT warn display paths follow the workdir anchor (matching Go's workdir-relative walker paths). Docker bind construction is untouched.apps/cli/src/legacy/commands/functions/deploy/deploy.integration.test.ts— new regression test: workdir≠git-root monorepo deploy asserts workdir-anchored metadata,../-relative upload names, and the Go-parityUploading assetstderr line.apps/cli/src/next/commands/functions/deploy/deploy.integration.test.ts— the two git-root upload tests updated to the workdir anchoring; the npm env test now assertsNPM_CONFIG_REGISTRYis forwarded andNPM_AUTH_TOKENis not.apps/cli/src/legacy/commands/functions/deploy/SIDE_EFFECTS.md— env table states onlyNPM_CONFIG_REGISTRYis forwarded; new note documents the workdir anchoring and the TS-only git-root boundary.All four changed/added tests fail against the previous implementation and pass with this change.