feat(cli): add supabase workers push - #6262
Conversation
aa0ea27 to
fa9be15
Compare
fa9be15 to
959520b
Compare
Builds and deploys workers into the linked project, and brings the Management API seam with it. Registered under `deploy` as an alias, for anyone reaching for the `supabase functions` verb out of habit. Given no names it deploys every worker in the project, matching `supabase functions deploy`, whose conventions this command set otherwise mirrors. "Every worker" is the union of the directories under the workers root and the `[workers.<name>]` entries, so one with a `source` pointing outside that root is not missed, and the order is sorted rather than whatever the filesystem returned. Deploys run one at a time: each is a server-side container build, so interleaving them would both compete for the alpha's per-project capacity and shred the progress output; the first failure stops the run. The flow is mint an upload slot, PUT the `.tar.gz` build context straight at the presigned URL, deploy, then poll until `build_state` leaves `building`. The upload carries no Supabase credentials: the signature in the URL is the authorization, and the bytes never pass through the management API. Polling is a `Schedule`, and tolerates a few consecutive read failures so one blip does not throw away a deploy that is progressing. Which spec is sent depends on the runtime: a `dockerfile` worker sends a context and no `spec.runtime`, a catalog runtime sends both, and a bare `sandbox` sends the runtime alone and skips packaging, so it has no URL. A directory with no `[workers.<name>] runtime` has one guessed from marker files, reported on stderr with a nudge to pin it down. An empty source directory is refused rather than deployed as an image with nothing in it. The build context is packaged in-process rather than by shelling out to `tar`, whose BSD, GNU and absent-on-Windows variants each produce a different archive from the same tree. `tar.ts` writes USTAR directly: files, directories and symlinks, refusing a value too large for an octal header field instead of letting it spill into the next one and read back as a plausible but wrong size. Symlinks are stored as links rather than followed — anything pnpm installs is symlink-dense, so following them would inline every dependency and walk into a link pointing at an ancestor. This is the first command in this shell to call a v2 Management API route; every other one here is a Go-parity port and uses v1 only.
The Workers routes answer 404 both for a project outside the alpha's
allow-list and for a project ref that names nothing this account can see, and
the CLI read every one of them as the former. A mistyped `--project-ref` was
answered with "Workers are in private alpha. Ask in the Supabase dashboard to
have this project enrolled." — sending someone to request enrolment for a
project that does not exist, and never mentioning the ref.
The two are distinguishable in the body:
not enrolled {"error":{"code":"generic_not_found","message":"Workers are not available for this project"}}
no such project{"error":{"code":"not_found","message":"Not Found"}}
so classify on `error.code` and raise the new WorkerProjectNotFoundError for
`not_found`, pointing at the ref, `supabase link` and `supabase login`. Only
that exact code is treated as a missing project; an unrecognized body keeps
the enrolment answer, since that is what the allow-list has historically
returned and guessing the other way would send someone to check a ref that is
fine.
The existing coverage asserted against a `{message}` body the API does not
send, so it is retargeted at the real shapes.
959520b to
d38a32b
Compare
![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: 959520b26b
ℹ️ 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".
| const contents = yield* fs | ||
| .readFile(absolutePath) | ||
| .pipe(Effect.orElseSucceed(() => new Uint8Array(0))); |
There was a problem hiding this comment.
When a source file is unreadable or disappears between stat and readFile, this fallback silently archives it as a zero-byte file; the analogous directory fallback also drops an unreadable subtree. The command can therefore report a successful upload and deploy an incomplete application instead of stopping packaging with a typed filesystem error.
AGENTS.md reference: AGENTS.md:L64-L68
Useful? React with 👍 / 👎.
| const instanceCountOrUndefined = (value: unknown): number | undefined => | ||
| typeof value === "number" && Number.isInteger(value) && value >= 0 ? value : undefined; |
There was a problem hiding this comment.
For instances = -1 or instances = 1.5, this silently returns undefined and push deploys the default of 1, potentially scaling a worker contrary to the user's configuration. The claimed schema error never occurs because packages/config/src/workers.ts currently accepts an unconstrained Schema.Number; validate the integer range in the schema or fail here rather than changing the requested deployment.
Useful? React with 👍 / 👎.
| const worker = yield* getWorker(api, projectRef, name).pipe( | ||
| Effect.retry({ schedule: Schedule.recurs(2) }), | ||
| ); |
There was a problem hiding this comment.
When the Management API produces three consecutive transient read failures, Schedule.recurs(2) exhausts almost immediately and aborts the command even though the surrounding build poll has a ten-minute budget. A brief outage can therefore leave a remotely progressing deploy while the CLI reports failure; retry this polling read with a wall-clock-bounded schedule instead of an attempt count.
AGENTS.md reference: AGENTS.md:L168-L172
Useful? React with 👍 / 👎.
|
|
||
| export type LegacyWorkersPushFlags = CliCommand.Command.Config.Infer<typeof config>; | ||
|
|
||
| export const legacyWorkersPushCommand = Command.make("push", config).pipe( |
There was a problem hiding this comment.
This exposes a new Management API workflow whose correctness depends on a real presigned upload and asynchronous backend build, but a repository-wide search of apps/cli/src/**/*.live.test.ts finds no Workers live suite. Add a gated golden-path live test so the real upload/deploy contract is exercised as required for new Management API commands.
AGENTS.md reference: apps/cli/AGENTS.md:L520-L524
Useful? React with 👍 / 👎.
| const request = ( | ||
| slot.method.toUpperCase() === "POST" | ||
| ? HttpClientRequest.post(slot.url) | ||
| : HttpClientRequest.put(slot.url) | ||
| ).pipe(HttpClientRequest.bodyUint8Array(archive, "application/gzip")); |
There was a problem hiding this comment.
When --debug is enabled, this request uses the top-level legacyHttpClientLayer, whose request mapper passes the complete req.url to LegacyDebugLogger.http; the query string of this presigned URL is the upload credential, so it is printed to stderr and can be exposed in copied debug logs or CI artifacts. Preserve the required request logging while stripping the query string for this storage request.
AGENTS.md reference: apps/cli/AGENTS.md:L315-L317
Useful? React with 👍 / 👎.
| export const packageWorkerDirectory = Effect.fnUntraced(function* (dir: string) { | ||
| const entries = yield* collectEntries(dir, ""); | ||
| const archive = gzipSync(createTar(entries)); | ||
|
|
||
| return { | ||
| archive: new Uint8Array(archive), |
There was a problem hiding this comment.
For a large worker tree—especially the explicitly supported case with node_modules—this first retains every file in entries, then allocates the complete uncompressed tar, the compressed archive, and another copy for the returned Uint8Array. Peak memory is therefore several times the source size and can terminate the CLI before upload; create and gzip the tar as a stream and send that stream to the upload request.
Useful? React with 👍 / 👎.
| // `-o` asks for a machine-readable stdout, so nothing human may be written | ||
| // to it — `output.success` logs to stdout in text mode. | ||
| if (yield* legacyEmitWorkersGoOutput(payload)) { |
There was a problem hiding this comment.
With workers push -o env, all uploads, deployments, and build polling finish before this call discovers that the payload contains the workers array and raises LegacyWorkersEnvNotSupportedError. The command thus exits as a failure after changing the remote project, encouraging a retry that starts another deployment; reject this format before entering the deployment loop or provide a valid env encoding.
Useful? React with 👍 / 👎.
| get [ErrorActionabilityId](): CliErrorActionabilityDeclaration { | ||
| return actionability.apiStatus; | ||
| } |
There was a problem hiding this comment.
Because this error carries the Management API status but always returns the generic apiStatus declaration, responses such as 401 and 403 are grouped as service failures instead of authentication or permission failures, corrupting the actionability KPIs for every Workers endpoint. Return statusCodeActionability(this.status) so the classification follows the typed status field.
AGENTS.md reference: apps/cli/AGENTS.md:L414-L416
Useful? React with 👍 / 👎.
| ## Files Written | ||
|
|
||
| | Path | Format | When | | ||
| | ---- | ------ | ---- | | ||
| | — | — | — | |
There was a problem hiding this comment.
The handler always runs linkedProjectCache.cache(projectRef) and telemetryState.flush, so on a cache miss it can call GET /v1/projects/{ref} and write <workdir>/supabase/.temp/linked-project.json, while the telemetry state is written under the Supabase home; declaring that no files are written makes this compatibility checklist inaccurate and omits an API route used by the command. Record these conditional and unconditional effects in this document.
AGENTS.md reference: apps/cli/AGENTS.md:L383-L394
Useful? React with 👍 / 👎.
| const packaged = yield* packageWorkerDirectory(worker.sourceDir).pipe( | ||
| Effect.tapError(() => packaging.fail()), | ||
| ); | ||
| yield* packaging.clear(); |
There was a problem hiding this comment.
A source containing only empty subdirectories passes the top-level readDirectory check, but packaging produces fileCount === 0 and the command proceeds to upload and deploy it. For catalog runtimes this can produce an active image with no handler even though the preceding guard explicitly intends to reject “nothing to deploy”; check the packaged entry count before minting the upload slot.
Useful? React with 👍 / 👎.
![depthfirst-app[bot]](https://avatars.githubusercontent.com/in/1021002?s=60&v=4){kind=small}
| continue; | ||
| } | ||
|
|
||
| const contents = yield* fs |
There was a problem hiding this comment.
🟡 Severity: MEDIUM
This packaging path reads every regular file, including .env files, private keys, and .git contents, into the build archive; push uploads that archive to control-plane storage. A normal deployment can therefore disclose credentials and repository secrets to the remote build context and anyone able to access the staged artifact.
Helpful? Add 👍 / 👎
💡 Fix Suggestion
Suggestion: Add an exclusion list at the top of the for loop in collectEntries (around line 36) to skip sensitive files and directories before they are traversed or read. This must be done at the loop entry point (where name is first available) so that entire directory trees like .git are not recursed into, not just at the file-read step (line 77). Add a check like:
const SENSITIVE_NAMES = new Set([".git", ".svn", ".hg"]);
const SENSITIVE_FILE_PATTERNS = [/^\.env(\..+)?$/, /\.(pem|key|p12|pfx|cer|crt|jks)$/i];
for (const name of [...names].sort()) {
// Skip well-known sensitive directories and credential files
if (SENSITIVE_NAMES.has(name) || SENSITIVE_FILE_PATTERNS.some((p) => p.test(name))) {
continue;
}
// ... rest of loop
}Also update the comment at line 9 which currently reads "Nothing is excluded" to document the exclusion list. Additionally, consider supporting a .workerignore or .dockerignore file in the worker source directory to let users extend the exclusion list for project-specific secrets.
| slot: WorkerUploadSlot, | ||
| archive: Uint8Array, | ||
| ) { | ||
| const client = yield* HttpClient.HttpClient; |
There was a problem hiding this comment.
🟡 Severity: MEDIUM
The upload client is the generic legacy HTTP client, which logs complete request URLs when --debug is enabled. slot.url is a presigned storage URL whose query string authorizes a write; exposing it in stderr/CI logs lets a log reader overwrite the staged context and potentially control the deployed worker.
Helpful? Add 👍 / 👎
💡 Fix Suggestion
Suggestion: Avoid passing the debug-logging HTTP client to uploadBuildContext. There are two recommended approaches:
Option A (preferred) – provide a plain FetchHttpClient to the upload call only: In push.handler.ts, wrap the uploadBuildContext(...) call with a locally provided fresh FetchHttpClient.layer so the presigned URL never reaches the debug-logger middleware: yield* Effect.provide(uploadBuildContext(slot, archive), FetchHttpClient.layer). This is the most targeted fix because it scopes the non-logging client strictly to the presigned upload.
Option B – redact presigned URLs in the debug logger: In legacy-http-debug.layer.ts, strip the query string from the URL before logging when the URL looks like a presigned object-store URL (e.g., contains X-Amz-Signature, X-Goog-Signature, or similar parameters): logger.http(req.method, redactPresignedUrl(req.url)). This is a defence-in-depth measure but does not fix the architectural coupling.
Either way, the uploadBuildContext function's own doc-comment already states that "the bytes never pass through the Management API, so this goes out on the plain HTTP client with no Supabase credentials attached". The fix should make that plain client literal – i.e. one that does NOT inherit the debug-logging middleware – so the authorisation query parameters in slot.url are never written to stderr or CI logs.
| let contextUploadId: string; | ||
| { | ||
| const packaging = yield* output.task("Packaging worker..."); | ||
| const packaged = yield* packageWorkerDirectory(worker.sourceDir).pipe( |
There was a problem hiding this comment.
🟡 Severity: MEDIUM
Project-controlled source is resolved without confinement before this call; absolute or .. paths, and directory symlinks, can point outside the project. push then archives and uploads those unrelated files, so a repository/config selecting such a path can disclose local credentials and other sensitive data to the Workers build service.
Helpful? Add 👍 / 👎
💡 Fix Suggestion
Suggestion: The root cause is in workerSourceDir in apps/cli/src/shared/workers/worker-paths.ts (line 161-169), which blindly calls resolve(projectRoot, configuredSource) with no path confinement check for source values read from config.toml. Unlike resolveWorkerSource (which validates the --source CLI flag and uses isAtOrUnder to ensure the path stays within the project), workerSourceDir applies no such validation, allowing source = "/etc" or source = "../../.ssh" to pass through unchecked. Fix this by converting workerSourceDir into an Effect-returning function that reuses the same isAtOrUnder confinement logic as resolveWorkerSource: after resolving the raw configured source against projectRoot, reject it (with InvalidWorkerSourceError) if the resolved absolute path is not at or under projectRoot, if it equals projectRoot or supabaseDir itself, or if it falls inside a reserved subdirectory like functions/ or migrations/. Update the caller in legacyDescribeWorker (workers.shared.ts line 82) to yield the resulting Effect, making it an effectful function. This ensures that both the CLI flag path and the config-file path go through identical confinement checks before any directory is archived and uploaded.
Supabase CLI previewnpx --yes https://pkg.pr.new/supabase/cli/supabase@d38a32b1ac5aba6fa67015d1b78b22d1a48ed508Preview package for commit |
1 participant
Summary
Adds
supabase workers push(aliaseddeploy) and the machinery it needs:workers-api.ts— the typed Workers Management API client.tar.ts/worker-package.ts— packaging a worker directory into the buildcontext that gets uploaded.
worker-classify.ts— best-effort runtime detection from marker files, so adirectory with no
[workers.<name>] runtimecan still deploy. The guess isalways reported with a nudge to pin it down, never applied silently.
Stack 3 of 4, on top of
workers new(#6261).Linked issue
FUNC-753 (Linear). Supabase maintainer, exempt from the
open-for-contributionflow.Checklist