From 325fcd968a7c788cc60f92f4d2e17ebe8c42f077 Mon Sep 17 00:00:00 2001 From: dada-sudo Date: Wed, 11 Nov 2026 16:03:04 +0100 Subject: [PATCH] fix(cli): keep a service's Dockerfile inside its folder, and bound the SSE frame buffer `link` checks the path `--dockerfile` gives, but `deploy` reads the path back from `.antelope/cloud.json` and packed whatever it named. That file is meant to be committed, so a change to it could make a deploy pack a file from outside the service folder, through `..` or a symlinked folder, into the upload. `createSourceArchive` now resolves the path and refuses one that leaves the folder with `service_path_outside_project`. `parseSse` appended to its buffer until a frame ended, without a limit. It now stops with `invalid_api_response` once one frame passes 1 MiB. The CLI and any server that relays a stream through the SDK share this parser. The release workflow's `validate` job only reads the repository, so it no longer inherits the workflow's write and OIDC permissions. Co-Authored-By: Claude --- .github/workflows/release-npm.yml | 4 +++ cli/src/archive.ts | 39 +++++++++++++++++++---- cli/test/contracts.test.ts | 52 ++++++++++++++++++++++++++++++- sdk/src/sse.ts | 17 ++++++++++ 4 files changed, 105 insertions(+), 7 deletions(-) diff --git a/.github/workflows/release-npm.yml b/.github/workflows/release-npm.yml index 136c78c..ca4885f 100644 --- a/.github/workflows/release-npm.yml +++ b/.github/workflows/release-npm.yml @@ -27,6 +27,10 @@ jobs: validate: name: Validate package release metadata runs-on: ubuntu-latest + # Reads the repository only: the write and OIDC grants of the workflow + # belong to the publishing job. + permissions: + contents: read steps: - uses: actions/checkout@v4 diff --git a/cli/src/archive.ts b/cli/src/archive.ts index 3a79407..d7a341a 100644 --- a/cli/src/archive.ts +++ b/cli/src/archive.ts @@ -1,8 +1,9 @@ -import { lstat, readdir, readFile } from "node:fs/promises"; -import { join, relative, sep } from "node:path"; +import { lstat, readdir, readFile, realpath } from "node:fs/promises"; +import { isAbsolute, join, relative, sep } from "node:path"; import { buffer } from "node:stream/consumers"; import ignore from "ignore"; import { pack } from "tar-stream"; +import { CliError } from "./errors.js"; const DEFAULT_IGNORES = [".git", "node_modules", ".antelope"]; const DEFAULT_IGNORE_SET = new Set(DEFAULT_IGNORES); @@ -49,6 +50,29 @@ function tarName(path: string): string { const DOCKERFILE = "Dockerfile"; +/** + * The Dockerfile a service names, once it is known to be a file of the + * service folder. `link` checks the path it is given, but `deploy` reads it + * back from `.antelope/cloud.json`, a committed file anyone who can change + * the repository can edit: without this check a path such as + * `../../.ssh/id_ed25519`, or one through a symlinked folder, would pack a + * file of the deployer's machine into the upload. + * + * @throws CliError `service_path_outside_project` when the path leaves the folder. + */ +async function containedDockerfile( + root: string, + dockerfile: string, +): Promise { + const path = join(root, dockerfile); + const [folder, target] = await Promise.all([realpath(root), realpath(path)]); + const inside = relative(folder, target); + if (inside === "" || inside.startsWith("..") || isAbsolute(inside)) { + throw new CliError("service_path_outside_project", dockerfile); + } + return path; +} + /** * The service folder as a tar archive. With `dockerfile`, that file is packed * as the archive's `Dockerfile`, in place of the folder's own. @@ -57,6 +81,9 @@ export async function createSourceArchive( root: string, dockerfile?: string, ): Promise { + const suppliedDockerfile = dockerfile + ? await containedDockerfile(root, dockerfile) + : undefined; const filter = ignore().add(await ignoreRules(root)); const listed = await filesUnder(root, filter); const files = dockerfile @@ -65,10 +92,10 @@ export async function createSourceArchive( const archive = pack(); const complete = buffer(archive); for (const file of files) { - const path = join( - root, - dockerfile && file === DOCKERFILE ? dockerfile : file, - ); + const path = + suppliedDockerfile && file === DOCKERFILE + ? suppliedDockerfile + : join(root, file); const stats = await lstat(path); if (!stats.isFile()) continue; archive.entry( diff --git a/cli/test/contracts.test.ts b/cli/test/contracts.test.ts index e7a5c13..b820c6c 100644 --- a/cli/test/contracts.test.ts +++ b/cli/test/contracts.test.ts @@ -1,5 +1,12 @@ import assert from "node:assert/strict"; -import { mkdir, mkdtemp, readFile, utimes, writeFile } from "node:fs/promises"; +import { + mkdir, + mkdtemp, + readFile, + symlink, + utimes, + writeFile, +} from "node:fs/promises"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { describe, test } from "node:test"; @@ -113,6 +120,49 @@ describe("contracts", () => { assert.deepEqual(await createSourceArchive(root), archive); }); + test("refuses a Dockerfile read from the project file that leaves the service folder", async () => { + const parent = await mkdtemp(join(tmpdir(), "cloud-dockerfile-escape-")); + const root = join(parent, "service"); + await mkdir(root); + await mkdir(join(parent, "outside")); + await writeFile(join(root, "index.js"), ""); + await writeFile(join(parent, "outside", "secret"), "not for upload"); + await assert.rejects( + createSourceArchive(root, "../outside/secret"), + /service_path_outside_project/, + ); + const linked = await symlink( + join(parent, "outside"), + join(root, "linked"), + "junction", + ).then( + () => true, + () => false, + ); + if (linked) { + await assert.rejects( + createSourceArchive(root, "linked/secret"), + /service_path_outside_project/, + ); + } + }); + + test("ends a stream whose frame never ends, before it fills memory", async () => { + const chunk = "x".repeat(65_536); + let sent = 0; + const endless = new ReadableStream({ + pull(controller) { + sent += 1; + controller.enqueue(new TextEncoder().encode(chunk)); + }, + }); + const read = async (): Promise => { + for await (const event of parseSse(new Response(endless))) void event; + }; + await assert.rejects(read(), /invalid_api_response/); + assert.ok(sent <= 32, `read ${sent} chunks before stopping`); + }); + test("packs a service's own Dockerfile as the archive's Dockerfile", async () => { const root = await mkdtemp(join(tmpdir(), "cloud-dockerfile-")); await writeFile(join(root, "Dockerfile"), "FROM backend\n"); diff --git a/sdk/src/sse.ts b/sdk/src/sse.ts index e247eb4..4762bcc 100644 --- a/sdk/src/sse.ts +++ b/sdk/src/sse.ts @@ -1,3 +1,5 @@ +import { InvalidApiResponseError } from "./api-error.js"; + export interface SseEvent { event: string; data: unknown; @@ -26,6 +28,17 @@ function fieldValue(value: string): string { return value.startsWith(" ") ? value.slice(1) : value; } +/** + * The most text one frame may hold before its end. A stream that never ends + * a frame would otherwise grow the buffer for as long as it is read. + */ +const MAX_FRAME_LENGTH = 1_048_576; + +/** + * Reads the events of a server-sent stream. + * + * @throws InvalidApiResponseError when one frame passes 1 MiB without ending. + */ export async function* parseSse(response: Response): AsyncGenerator { if (!response.body) return; const reader = response.body.pipeThrough(new TextDecoderStream()).getReader(); @@ -35,6 +48,10 @@ export async function* parseSse(response: Response): AsyncGenerator { buffer += value ?? ""; const frames = buffer.split(/\r?\n\r?\n/); buffer = frames.pop() ?? ""; + if (buffer.length > MAX_FRAME_LENGTH) { + await reader.cancel(); + throw new InvalidApiResponseError(); + } for (const frame of frames) { const event = eventOf(frame.split(/\r?\n/)); if (event) yield event;