feat(cli): port functions delete & download #5527
Conversation
![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: 7e342884e8
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
7ttp
force-pushed
the
feat/functions-management
branch
from
e96342f to
439d1da
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 439d1da30e
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| yield* downloadFunctions(flags, { | ||
| api, | ||
| projectRoot: cliConfig.workdir, |
There was a problem hiding this comment.
For a default stable-shell functions download invocation, this now calls the native multipart path with flags.useDocker === false; the previous wrapper forwarded no flag to the Go binary, whose command definition defaults --use-docker to true and only flips it off when --use-api is set (apps/cli-go/cmd/functions.go lines 51-54 and 179-181). In environments where Docker is running, users who do not pass any hidden flag now get server-side unbundling instead of the old Docker unbundle path, which is a behavior change for the legacy command.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
seems intentional for the ts port :D
default should be api/docker-less now, with --use-docker / --legacy-bundle kept only as hidden compatibility escape hatches
thoughts? @Coly010
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a261cde204
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e19a43151a
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
![depthfirst-app[bot]](https://avatars.githubusercontent.com/in/1021002?s=60&v=4){kind=small}
| ); | ||
| const parent = dirname(destination); | ||
| yield* makeContainedDirectory(realFunctionsRoot, parent, file.path); | ||
| const existingLink = yield* fs.readLink(destination).pipe(Effect.option); |
There was a problem hiding this comment.
🟡 Severity: MEDIUM
TOCTOU race between fs.readLink(destination) and fs.writeFile(destination, file.body). An attacker with local filesystem access could plant a symlink at destination in this window, causing server-controlled file content to be written to an arbitrary path outside functionsRoot. The non-atomic check-then-write pattern cannot prevent a concurrent symlink() call.
Helpful? Add 👍 / 👎
💡 Fix Suggestion
Suggestion: Replace the non-atomic check-then-write pattern with an atomic write that prevents symlink following using O_NOFOLLOW. Since Effect's FileSystem abstraction does not expose low-level open flags, use node:fs/promises directly via Effect.tryPromise to open the file with constants.O_WRONLY | constants.O_CREAT | constants.O_TRUNC | constants.O_NOFOLLOW flags. This causes the OS to fail atomically if destination is a symlink at the moment of opening, eliminating the TOCTOU window entirely. The pre-write readLink check (lines 669-676) should be removed and replaced with this atomic open-and-write, mapping the resulting ELOOP/ENOTDIR errno to an UnsafeFunctionDownloadPathError. As a complementary defense-in-depth layer (consistent with the existing makeContainedDirectory pattern), also add a post-write fs.realPath(destination) + ensureContainedPath(realFunctionsRoot, realDestination, file.path) check after the write succeeds to catch any edge cases on platforms where O_NOFOLLOW is unavailable. Example replacement for lines 669-677:
import { open, constants } from 'node:fs/promises';
// ...
// Replace the readLink check + writeFile with an atomic O_NOFOLLOW open:
const fd = yield* Effect.tryPromise({
try: () => open(destination, constants.O_WRONLY | constants.O_CREAT | constants.O_TRUNC | constants.O_NOFOLLOW),
catch: (e) => new UnsafeFunctionDownloadPathError({
message: `refusing to write to symlink while extracting Function file: ${file.path}: ${e}`,
}),
});
yield* Effect.tryPromise({
try: () => fd.writeFile(file.body).finally(() => fd.close()),
catch: (e) => new UnsafeFunctionDownloadPathError({ message: `failed to write file: ${file.path}: ${e}` }),
});
const realDestination = yield* fs.realPath(destination);
yield* ensureContainedPath(realFunctionsRoot, realDestination, file.path);1 participant
TL;DR
ports
supabase functions deleteandsupabase functions downloadto native ts.What’s introduced
adds native delete and download flows using the management api
while preserving compatibility and existing output behavior
& integration coverage around it!
ref:
will introduce
supabase functions deployas a followup :)