From 36559760f7289260af5307cc5b12991789da728c Mon Sep 17 00:00:00 2001 From: mvm Date: Fri, 28 Aug 2026 10:37:41 -0500 Subject: [PATCH 1/8] fix: harden middlecache downloads and consume OpenAPI archive The 24MB openapi.json was fetched over HTTP during the build. With default Accept-Encoding, middlecache served on-the-fly brotli, which has no integrity checksum; a corrupted/truncated transfer silently decompressed to garbage, intermittently failing the build with "Bad control character in string literal in JSON". - downloadToDotTempIfNotPresent now requests identity encoding (no brotli), writes atomically via a temp file, checks response.ok and Content-Length, and retries up to 3 times. An optional validate callback makes the whole download+validate unit retryable. - getSchema now prefers the gzip-compressed openapi.tar.gz from middlecache (extracted with tar, validated by gzip CRC32 + JSON.parse before use), falling back to the raw openapi.json. - Adds unit tests covering retry, validation, size mismatch, and existing-file handling. --- src/util/api.ts | 71 ++++++++++--- src/util/custom-loaders.node.test.ts | 152 +++++++++++++++++++++++++++ src/util/custom-loaders.ts | 98 +++++++++++++++-- 3 files changed, 300 insertions(+), 21 deletions(-) create mode 100644 src/util/custom-loaders.node.test.ts diff --git a/src/util/api.ts b/src/util/api.ts index 98b458c133a..c9869ff824b 100644 --- a/src/util/api.ts +++ b/src/util/api.ts @@ -1,33 +1,78 @@ /** * OpenAPI schema loader for the APIRequest component. * - * Fetches the Cloudflare API OpenAPI document from middlecache and dereferences - * all `$ref`s. The file is cached to `.tmp/middlecache/` (gitignored) via - * `downloadToDotTempIfNotPresent`, so the fetch only happens once per clean - * checkout. Dereferenced result is memoized at module scope so the deref runs - * once per build, not per component instance. + * Downloads the Cloudflare API OpenAPI document from middlecache, preferring + * the gzip-compressed tar (openapi.tar.gz) and falling back to the raw + * openapi.json. Files are extracted/cached under `.tmp/middlecache/` + * (gitignored) via `downloadToDotTempIfNotPresent`, so the fetch only happens + * once per clean checkout. Downloads are validated (extract + parse) so a + * corrupt transfer is retried rather than silently crashing the build. + * Dereferenced result is memoized at module scope so the deref runs once per + * build, not per component instance. */ import SwaggerParser from "@apidevtools/swagger-parser"; import type { OpenAPI } from "openapi-types"; -import { downloadToDotTempIfNotPresent } from "./custom-loaders"; +import { + downloadToDotTempIfNotPresent, + extractTarGz, + getDotTmpPath, +} from "./custom-loaders"; import { readFile } from "node:fs/promises"; -import { fileURLToPath } from "node:url"; import { join } from "node:path"; const MIDDLECACHE_BASE_URL = "https://middlecache.ced.cloudflare.com/"; +const API_SCHEMAS_ARCHIVE_PATH = "v1/cloudflare-api-schemas/openapi.tar.gz"; const API_SCHEMAS_PATH = "v1/cloudflare-api-schemas/openapi.json"; let schema: OpenAPI.Document | undefined; export const getSchema = async () => { if (!schema) { - await downloadToDotTempIfNotPresent( - `${MIDDLECACHE_BASE_URL}${API_SCHEMAS_PATH}`, - `middlecache/${API_SCHEMAS_PATH}`, + const dotTmpPath = getDotTmpPath(); + const extractDir = join( + dotTmpPath, + "middlecache", + "v1", + "cloudflare-api-schemas", ); - const dotTmpPath = fileURLToPath(new URL("../../.tmp", import.meta.url)); - const filePath = join(dotTmpPath, "middlecache", API_SCHEMAS_PATH); - const raw = await readFile(filePath, "utf8"); + + try { + await downloadToDotTempIfNotPresent( + `${MIDDLECACHE_BASE_URL}${API_SCHEMAS_ARCHIVE_PATH}`, + `middlecache/${API_SCHEMAS_ARCHIVE_PATH}`, + { + validate: async (archivePath) => { + // Extract + parse as the integrity check: a corrupt or truncated + // download fails here (gzip CRC32 + JSON) and is re-downloaded + // instead of producing garbage that crashes JSON.parse later. + await extractTarGz(archivePath, extractDir); + const raw = await readFile( + join(extractDir, "openapi.json"), + "utf8", + ); + void JSON.parse(raw); + }, + }, + ); + } catch (err) { + // Fall back to the raw openapi.json if the archive is not available + // yet (e.g. the middlecache pipeline has not shipped it). + console.warn( + `Failed to fetch OpenAPI archive, falling back to raw openapi.json: ${(err as Error).message}`, + ); + await downloadToDotTempIfNotPresent( + `${MIDDLECACHE_BASE_URL}${API_SCHEMAS_PATH}`, + `middlecache/${API_SCHEMAS_PATH}`, + { + validate: async (filePath) => { + const raw = await readFile(filePath, "utf8"); + void JSON.parse(raw); + }, + }, + ); + } + + const raw = await readFile(join(extractDir, "openapi.json"), "utf8"); schema = await SwaggerParser.dereference(JSON.parse(raw)); } diff --git a/src/util/custom-loaders.node.test.ts b/src/util/custom-loaders.node.test.ts new file mode 100644 index 00000000000..cb1fbc68d5b --- /dev/null +++ b/src/util/custom-loaders.node.test.ts @@ -0,0 +1,152 @@ +import { afterEach, beforeEach, describe, expect, test, vi } from "vitest"; +import fs from "fs"; +import { join } from "node:path"; + +import { downloadToDotTempIfNotPresent, getDotTmpPath } from "./custom-loaders"; + +const dotTmpPath = getDotTmpPath(); +const TEST_DIR = join(dotTmpPath, "middlecache", "__custom-loaders-test__"); +const TEST_DEST = "middlecache/__custom-loaders-test__/file.txt"; + +const okResponse = (body: string) => new Response(body); +const errResponse = (status: number) => new Response("error", { status }); + +describe("downloadToDotTempIfNotPresent", () => { + beforeEach(() => { + fs.rmSync(TEST_DIR, { recursive: true, force: true }); + }); + + afterEach(() => { + fs.rmSync(TEST_DIR, { recursive: true, force: true }); + vi.unstubAllGlobals(); + }); + + test("downloads and writes the file when fetch succeeds", async () => { + vi.stubGlobal( + "fetch", + vi.fn().mockResolvedValue(okResponse("hello world")), + ); + + await downloadToDotTempIfNotPresent( + "https://example.com/file.txt", + TEST_DEST, + ); + + expect(fs.readFileSync(join(TEST_DIR, "file.txt"), "utf8")).toBe( + "hello world", + ); + }); + + test("retries on HTTP error then succeeds", async () => { + const fetchMock = vi + .fn() + .mockResolvedValueOnce(errResponse(500)) + .mockResolvedValueOnce(errResponse(500)) + .mockResolvedValueOnce(okResponse("ok")); + vi.stubGlobal("fetch", fetchMock); + + await downloadToDotTempIfNotPresent( + "https://example.com/file.txt", + TEST_DEST, + ); + + expect(fetchMock).toHaveBeenCalledTimes(3); + expect(fs.readFileSync(join(TEST_DIR, "file.txt"), "utf8")).toBe("ok"); + }); + + test("throws after exhausting retries", async () => { + const fetchMock = vi.fn().mockResolvedValue(errResponse(500)); + vi.stubGlobal("fetch", fetchMock); + + await expect( + downloadToDotTempIfNotPresent("https://example.com/file.txt", TEST_DEST), + ).rejects.toThrow(/HTTP 500/); + expect(fetchMock).toHaveBeenCalledTimes(3); + }); + + test("re-downloads when the validate callback rejects", async () => { + const fetchMock = vi + .fn() + .mockResolvedValueOnce(okResponse("corrupt")) + .mockResolvedValueOnce(okResponse("valid")); + vi.stubGlobal("fetch", fetchMock); + const validate = vi + .fn() + .mockRejectedValueOnce(new Error("corrupt file")) + .mockResolvedValueOnce(undefined); + + await downloadToDotTempIfNotPresent( + "https://example.com/file.txt", + TEST_DEST, + { + validate, + }, + ); + + expect(fetchMock).toHaveBeenCalledTimes(2); + expect(fs.readFileSync(join(TEST_DIR, "file.txt"), "utf8")).toBe("valid"); + }); + + test("retries when downloaded size mismatches content-length", async () => { + const lengthMismatch = () => { + // Real body ("abc", 3 bytes) but a fake content-length header (10). + const res = new Response("abc"); + return { + ok: true, + status: 200, + statusText: "OK", + body: res.body, + headers: { get: () => "10" }, + }; + }; + const fetchMock = vi + .fn() + .mockResolvedValueOnce(lengthMismatch()) + .mockResolvedValueOnce(okResponse("ok")); + vi.stubGlobal("fetch", fetchMock); + + await downloadToDotTempIfNotPresent( + "https://example.com/file.txt", + TEST_DEST, + ); + + expect(fetchMock).toHaveBeenCalledTimes(2); + expect(fs.readFileSync(join(TEST_DIR, "file.txt"), "utf8")).toBe("ok"); + }); + + test("skips download when the file already exists and validates", async () => { + fs.mkdirSync(TEST_DIR, { recursive: true }); + fs.writeFileSync(join(TEST_DIR, "file.txt"), "existing"); + const fetchMock = vi.fn(); + vi.stubGlobal("fetch", fetchMock); + + await downloadToDotTempIfNotPresent( + "https://example.com/file.txt", + TEST_DEST, + ); + + expect(fetchMock).not.toHaveBeenCalled(); + }); + + test("re-downloads when an existing file fails validation", async () => { + fs.mkdirSync(TEST_DIR, { recursive: true }); + fs.writeFileSync(join(TEST_DIR, "file.txt"), "stale"); + const fetchMock = vi.fn().mockResolvedValue(okResponse("fresh")); + vi.stubGlobal("fetch", fetchMock); + const validate = vi + .fn() + .mockRejectedValueOnce(new Error("bad file")) + .mockResolvedValueOnce(undefined); + + await downloadToDotTempIfNotPresent( + "https://example.com/file.txt", + TEST_DEST, + { + validate, + }, + ); + + expect(fetchMock).toHaveBeenCalledTimes(1); + expect(fs.readFileSync(join(TEST_DIR, "file.txt"), "utf8")).toBe("fresh"); + }); +}); diff --git a/src/util/custom-loaders.ts b/src/util/custom-loaders.ts index 7cc3c068e0d..9e03667bb62 100644 --- a/src/util/custom-loaders.ts +++ b/src/util/custom-loaders.ts @@ -7,22 +7,42 @@ import { fileURLToPath } from "node:url"; import { Readable } from "node:stream"; import type { ReadableStream as WebReadableStream } from "node:stream/web"; import { writeFile } from "node:fs/promises"; +import { spawn } from "node:child_process"; import fs from "fs"; import { dirname, join } from "path"; import * as z from "zod"; +const MAX_DOWNLOAD_ATTEMPTS = 3; + +/** + * Resolve the repo-root `.tmp/` directory used for downloaded artifacts. + * Prefers the file-relative path (how Astro/tsx resolve it) and falls back to + * the current working directory (needed under Vitest, where `import.meta.url` + * is not a `file://` URL). + */ +export const getDotTmpPath = () => { + try { + return fileURLToPath(new URL("../../.tmp", import.meta.url)); + } catch { + return join(process.cwd(), ".tmp"); + } +}; + /** * downloadToDotTempIfNotPresent is a convenience function for handling downloads to a .tmp directory * within the source repo * * @param url - source URL * @param dotTmpDestination - path relative to .tmp/ as destination for downloaded file + * @param options - { validate: optional async check run against the downloaded file; a rejected + * promise discards the file and triggers a re-download } */ export async function downloadToDotTempIfNotPresent( url: string, dotTmpDestination: string, + options: { validate?: (filePath: string) => Promise } = {}, ) { const source = z.url().parse(url); const relativeDestination = z @@ -38,28 +58,90 @@ export async function downloadToDotTempIfNotPresent( const destinationParts = relativeDestination.split("/"); const universalRelativeDestination = join(...destinationParts); - const dotTmpPath = fileURLToPath(new URL("../../.tmp", import.meta.url)); + const dotTmpPath = getDotTmpPath(); const destination = join(dotTmpPath, universalRelativeDestination); - if (!fs.existsSync(destination)) { - fs.mkdirSync(dirname(destination), { recursive: true }); - - const response = await fetch(source); + for (let attempt = 1; attempt <= MAX_DOWNLOAD_ATTEMPTS; attempt++) { try { + if (fs.existsSync(destination)) { + await options.validate?.(destination); + return; + } + + fs.mkdirSync(dirname(destination), { recursive: true }); + + // Write to a temp file first so a partial/failed download never + // leaves a file that looks "present". + const tmpDestination = `${destination}.tmp`; + fs.rmSync(tmpDestination, { force: true }); + + // Request the identity encoding so middlecache serves the bytes + // as-is rather than on-the-fly brotli, which has no integrity check + // and can silently decompress a truncated transfer into garbage. + const response = await fetch(source, { + headers: { "Accept-Encoding": "identity" }, + }); + + if (!response.ok) { + throw new Error( + `Failed to download ${url}: HTTP ${response.status} ${response.statusText}`, + ); + } + // Stream file to destination to avoid storing in memory await writeFile( - destination, + tmpDestination, Readable.fromWeb(response.body! as WebReadableStream), ); + + const expectedLength = Number(response.headers.get("content-length")); + if (Number.isFinite(expectedLength) && expectedLength > 0) { + const actualLength = fs.statSync(tmpDestination).size; + if (actualLength !== expectedLength) { + throw new Error( + `Downloaded file size mismatch for ${url}: expected ${expectedLength} bytes, got ${actualLength}`, + ); + } + } + + fs.renameSync(tmpDestination, destination); + await options.validate?.(destination); + return; } catch (err) { - // Clean up partial download if stream fails fs.rmSync(destination, { force: true }); - throw err; + fs.rmSync(`${destination}.tmp`, { force: true }); + if (attempt === MAX_DOWNLOAD_ATTEMPTS) { + throw err; + } + console.warn( + `Retrying download of ${url} (attempt ${attempt}/${MAX_DOWNLOAD_ATTEMPTS}): ${(err as Error).message}`, + ); } } } +/** + * Extract a gzip-compressed tar archive into destinationDir. + */ +export async function extractTarGz( + tarballPath: string, + destinationDir: string, +): Promise { + fs.mkdirSync(destinationDir, { recursive: true }); + const tar = spawn("tar", ["-xzf", tarballPath, "-C", destinationDir], { + stdio: "ignore", + }); + const exitCode = await new Promise((resolve) => + tar.on("close", resolve), + ); + if (exitCode !== 0) { + throw new Error( + `tar extraction failed for ${tarballPath} (exit code ${exitCode})`, + ); + } +} + /** * middlecache loader expects a middlecache path * From c4e3921aa32dbe113b30ce43b679d666eb86219b Mon Sep 17 00:00:00 2001 From: mvm Date: Fri, 28 Aug 2026 10:49:34 -0500 Subject: [PATCH 2/8] refactor: reuse extractTarGz helper for skills archive bin/fetch-skills.ts had its own inline tar spawn; extractTarGz (shared with the OpenAPI archive path) now supports stripComponents and fetch-skills uses it instead. Adds extractTarGz unit tests. --- bin/fetch-skills.ts | 22 +++------ src/util/custom-loaders.node.test.ts | 73 +++++++++++++++++++++++++++- src/util/custom-loaders.ts | 16 ++++-- 3 files changed, 93 insertions(+), 18 deletions(-) diff --git a/bin/fetch-skills.ts b/bin/fetch-skills.ts index db2109bc7b8..ba0d648c6d3 100644 --- a/bin/fetch-skills.ts +++ b/bin/fetch-skills.ts @@ -1,10 +1,12 @@ #!/usr/bin/env tsx -import { spawn } from "child_process"; import fs from "fs"; import { join } from "path"; -import { downloadToDotTempIfNotPresent } from "../src/util/custom-loaders"; +import { + downloadToDotTempIfNotPresent, + extractTarGz, +} from "../src/util/custom-loaders"; const MIDDLECACHE_BASE_URL = "https://middlecache.ced.cloudflare.com/"; const SKILLS_MIDDLECACHE_PATH = "v1/cloudflare-skills/skills.tar.gz"; @@ -66,18 +68,10 @@ fs.mkdirSync(SKILLS_DIR, { recursive: true }); // Extract the tarball from .tmp/ into ./skills/. // The archive contains skills//... so we strip the leading "skills/" // component and extract into SKILLS_DIR. -const tar = spawn( - "tar", - ["--strip-components=1", "-xz", "-C", SKILLS_DIR, "-f", tarballPath], - { stdio: "inherit" }, -); - -const exitCode = await new Promise((resolve) => - tar.on("close", resolve), -); - -if (exitCode !== 0) { - fail(`tar exited with code ${exitCode}`); +try { + await extractTarGz(tarballPath, SKILLS_DIR, { stripComponents: 1 }); +} catch (err) { + fail(`tar extraction failed: ${(err as Error).message}`); } const cloudflareSkills = fs diff --git a/src/util/custom-loaders.node.test.ts b/src/util/custom-loaders.node.test.ts index cb1fbc68d5b..91ebfe2ccab 100644 --- a/src/util/custom-loaders.node.test.ts +++ b/src/util/custom-loaders.node.test.ts @@ -1,8 +1,13 @@ import { afterEach, beforeEach, describe, expect, test, vi } from "vitest"; import fs from "fs"; +import { spawnSync } from "node:child_process"; import { join } from "node:path"; -import { downloadToDotTempIfNotPresent, getDotTmpPath } from "./custom-loaders"; +import { + downloadToDotTempIfNotPresent, + extractTarGz, + getDotTmpPath, +} from "./custom-loaders"; const dotTmpPath = getDotTmpPath(); const TEST_DIR = join(dotTmpPath, "middlecache", "__custom-loaders-test__"); @@ -150,3 +155,69 @@ describe("downloadToDotTempIfNotPresent", () => { expect(fs.readFileSync(join(TEST_DIR, "file.txt"), "utf8")).toBe("fresh"); }); }); + +describe("extractTarGz", () => { + const FIXTURE = join(dotTmpPath, "middlecache", "__extract-test__"); + + const buildFixture = () => { + fs.mkdirSync(join(FIXTURE, "src", "skills", "skill-name"), { + recursive: true, + }); + fs.writeFileSync( + join(FIXTURE, "src", "skills", "skill-name", "file.txt"), + "content", + ); + spawnSync( + "tar", + [ + "-czf", + join(FIXTURE, "fixture.tar.gz"), + "-C", + join(FIXTURE, "src"), + "skills", + ], + { stdio: "ignore" }, + ); + }; + + beforeEach(() => { + fs.rmSync(FIXTURE, { recursive: true, force: true }); + }); + + afterEach(() => { + fs.rmSync(FIXTURE, { recursive: true, force: true }); + }); + + test("extracts without stripping components", async () => { + buildFixture(); + const dest = join(FIXTURE, "out"); + + await extractTarGz(join(FIXTURE, "fixture.tar.gz"), dest); + + expect( + fs.readFileSync(join(dest, "skills", "skill-name", "file.txt"), "utf8"), + ).toBe("content"); + }); + + test("strips leading components when stripComponents is set", async () => { + buildFixture(); + const dest = join(FIXTURE, "out"); + + await extractTarGz(join(FIXTURE, "fixture.tar.gz"), dest, { + stripComponents: 1, + }); + + expect(fs.readFileSync(join(dest, "skill-name", "file.txt"), "utf8")).toBe( + "content", + ); + }); + + test("throws on a corrupt archive", async () => { + fs.mkdirSync(FIXTURE, { recursive: true }); + fs.writeFileSync(join(FIXTURE, "bad.tar.gz"), "not a gzip archive"); + + await expect( + extractTarGz(join(FIXTURE, "bad.tar.gz"), join(FIXTURE, "out")), + ).rejects.toThrow(/tar extraction failed/); + }); +}); diff --git a/src/util/custom-loaders.ts b/src/util/custom-loaders.ts index 9e03667bb62..fd28249dfc5 100644 --- a/src/util/custom-loaders.ts +++ b/src/util/custom-loaders.ts @@ -123,15 +123,25 @@ export async function downloadToDotTempIfNotPresent( /** * Extract a gzip-compressed tar archive into destinationDir. + * + * @param options.stripComponents - strip the given number of leading path + * components from each entry before extracting (matches the skills archive, + * which contains a top-level `skills/` directory). */ export async function extractTarGz( tarballPath: string, destinationDir: string, + options: { stripComponents?: number } = {}, ): Promise { fs.mkdirSync(destinationDir, { recursive: true }); - const tar = spawn("tar", ["-xzf", tarballPath, "-C", destinationDir], { - stdio: "ignore", - }); + + const args = ["-xz", "-C", destinationDir]; + if (options.stripComponents && options.stripComponents > 0) { + args.push(`--strip-components=${options.stripComponents}`); + } + args.push("-f", tarballPath); + + const tar = spawn("tar", args, { stdio: "ignore" }); const exitCode = await new Promise((resolve) => tar.on("close", resolve), ); From 49e689145c08fd425a58dabf692fb837c28f2135 Mon Sep 17 00:00:00 2001 From: mvm Date: Fri, 28 Aug 2026 11:41:08 -0500 Subject: [PATCH 3/8] fix: fetch OpenAPI schema in prebuild instead of during prerender MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The build failed intermittently with ENOENT on openapi.json because getSchema downloaded the schema lazily during prerender. Pages render in parallel, so concurrent downloads raced on the shared .tmp file — a retry's cleanup could delete a sibling call's freshly-written file. Mirror the skills flow: a new bin/fetch-openapi.ts (run from the prebuild/predev hooks) downloads the gzip-compressed openapi.tar.gz from middlecache and extracts openapi.json to .tmp before the build starts. getSchema now just reads the local file. - bin/fetch-openapi.ts: download archive (fall back to raw openapi.json), extract + parse as the integrity check; --soft for predev - package.json: prebuild/predev run fetch-openapi after fetch-skills - api.ts: getSchema reads .tmp/.../openapi.json, single-flighted - custom-loaders.ts: getDotTmpPath now resolves the repo root by walking up to package.json (tsx prebuild and the bundled prerender resolve import.meta.url to different depths); downloadToDotTempIfNotPresent dedups concurrent same-destination downloads - tests: concurrent-download dedup + retry-after-failure coverage Verified: full `pnpm run build` succeeds (8932 pages), check/lint/tests pass. --- bin/fetch-openapi.ts | 76 +++++++++++++++++++++ package.json | 4 +- src/util/api.ts | 98 +++++++++------------------- src/util/custom-loaders.node.test.ts | 36 ++++++++++ src/util/custom-loaders.ts | 66 ++++++++++++++++--- 5 files changed, 202 insertions(+), 78 deletions(-) create mode 100644 bin/fetch-openapi.ts diff --git a/bin/fetch-openapi.ts b/bin/fetch-openapi.ts new file mode 100644 index 00000000000..97f4fc6dc09 --- /dev/null +++ b/bin/fetch-openapi.ts @@ -0,0 +1,76 @@ +#!/usr/bin/env tsx + +import { readFile } from "node:fs/promises"; +import { join } from "node:path"; + +import { + downloadToDotTempIfNotPresent, + extractTarGz, + getDotTmpPath, +} from "../src/util/custom-loaders"; + +const MIDDLECACHE_BASE_URL = "https://middlecache.ced.cloudflare.com/"; +const OPENAPI_ARCHIVE_PATH = "v1/cloudflare-api-schemas/openapi.tar.gz"; +const OPENAPI_JSON_PATH = "v1/cloudflare-api-schemas/openapi.json"; + +// --soft: warn and continue on failure instead of exiting non-zero. +// Used by the predev hook so a network failure doesn't block local development. +const soft = process.argv.includes("--soft"); + +const fail = (message: string): never => { + if (soft) { + console.warn( + `Warning: ${message} — API endpoint pages will not work without the schema`, + ); + process.exit(0); + } + console.error(`Error: ${message}`); + process.exit(1); +}; + +const extractDir = join( + getDotTmpPath(), + "middlecache", + "v1", + "cloudflare-api-schemas", +); + +console.log("Fetching Cloudflare API OpenAPI schema from middlecache"); + +try { + try { + // Prefer the gzip-compressed archive (integrity-checkable via gzip + // CRC32), falling back to the raw openapi.json. + await downloadToDotTempIfNotPresent( + `${MIDDLECACHE_BASE_URL}${OPENAPI_ARCHIVE_PATH}`, + `middlecache/${OPENAPI_ARCHIVE_PATH}`, + { + validate: async (archivePath) => { + // Extract + parse as the integrity check: a corrupt or truncated + // download fails here (gzip CRC32 + JSON) and is re-downloaded. + await extractTarGz(archivePath, extractDir); + const raw = await readFile(join(extractDir, "openapi.json"), "utf8"); + void JSON.parse(raw); + }, + }, + ); + } catch (err) { + console.warn( + `Failed to fetch OpenAPI archive, falling back to raw openapi.json: ${(err as Error).message}`, + ); + await downloadToDotTempIfNotPresent( + `${MIDDLECACHE_BASE_URL}${OPENAPI_JSON_PATH}`, + `middlecache/${OPENAPI_JSON_PATH}`, + { + validate: async (filePath) => { + const raw = await readFile(filePath, "utf8"); + void JSON.parse(raw); + }, + }, + ); + } +} catch (err) { + fail(`fetch failed: ${err}`); +} + +console.log("OpenAPI schema ready"); diff --git a/package.json b/package.json index aff278e00d9..b214ebf1f1f 100644 --- a/package.json +++ b/package.json @@ -5,14 +5,14 @@ "scripts": { "preinstall": "npx --yes only-allow pnpm", "astro": "astro", - "prebuild": "tsx bin/fetch-skills.ts", + "prebuild": "tsx bin/fetch-skills.ts && tsx bin/fetch-openapi.ts", "build": "astro build", "build:incremental": "INCREMENTAL_BUILD=true astro build", "typegen:worker": "wrangler types ./worker/worker-configuration.d.ts", "check": "pnpm run check:astro && pnpm run check:worker", "check:astro": "astro check --minimumFailingSeverity=hint", "check:worker": "tsc --noEmit -p ./worker/tsconfig.json", - "predev": "tsx bin/fetch-skills.ts --soft", + "predev": "tsx bin/fetch-skills.ts --soft && tsx bin/fetch-openapi.ts --soft", "dev": "astro dev", "format": "pnpm run format:core:fix && pnpm run format:data:fix && pnpm run format:content:fix", "format:check": "pnpm run format:core:check && pnpm run format:data:check && pnpm run format:content:check", diff --git a/src/util/api.ts b/src/util/api.ts index c9869ff824b..a03aa3efab8 100644 --- a/src/util/api.ts +++ b/src/util/api.ts @@ -1,81 +1,47 @@ /** * OpenAPI schema loader for the APIRequest component. * - * Downloads the Cloudflare API OpenAPI document from middlecache, preferring - * the gzip-compressed tar (openapi.tar.gz) and falling back to the raw - * openapi.json. Files are extracted/cached under `.tmp/middlecache/` - * (gitignored) via `downloadToDotTempIfNotPresent`, so the fetch only happens - * once per clean checkout. Downloads are validated (extract + parse) so a - * corrupt transfer is retried rather than silently crashing the build. - * Dereferenced result is memoized at module scope so the deref runs once per - * build, not per component instance. + * The schema is fetched and extracted by `bin/fetch-openapi.ts`, which runs + * from the `prebuild`/`predev` hooks (see package.json), so the prerender only + * reads the local copy at `.tmp/middlecache/v1/cloudflare-api-schemas/`. + * Dereferenced result is memoized so the deref runs once per build, not per + * component instance. */ import SwaggerParser from "@apidevtools/swagger-parser"; import type { OpenAPI } from "openapi-types"; -import { - downloadToDotTempIfNotPresent, - extractTarGz, - getDotTmpPath, -} from "./custom-loaders"; +import { getDotTmpPath } from "./custom-loaders"; import { readFile } from "node:fs/promises"; import { join } from "node:path"; -const MIDDLECACHE_BASE_URL = "https://middlecache.ced.cloudflare.com/"; -const API_SCHEMAS_ARCHIVE_PATH = "v1/cloudflare-api-schemas/openapi.tar.gz"; -const API_SCHEMAS_PATH = "v1/cloudflare-api-schemas/openapi.json"; +const OPENAPI_JSON_PATH = join( + getDotTmpPath(), + "middlecache", + "v1", + "cloudflare-api-schemas", + "openapi.json", +); -let schema: OpenAPI.Document | undefined; +let schemaPromise: Promise | undefined; -export const getSchema = async () => { - if (!schema) { - const dotTmpPath = getDotTmpPath(); - const extractDir = join( - dotTmpPath, - "middlecache", - "v1", - "cloudflare-api-schemas", +const loadSchema = async (): Promise => { + let raw: string; + try { + raw = await readFile(OPENAPI_JSON_PATH, "utf8"); + } catch (err) { + throw new Error( + `OpenAPI schema not found at ${OPENAPI_JSON_PATH} — run \`pnpm run build\` (or \`pnpm prebuild\`) first. ${(err as Error).message}`, ); - - try { - await downloadToDotTempIfNotPresent( - `${MIDDLECACHE_BASE_URL}${API_SCHEMAS_ARCHIVE_PATH}`, - `middlecache/${API_SCHEMAS_ARCHIVE_PATH}`, - { - validate: async (archivePath) => { - // Extract + parse as the integrity check: a corrupt or truncated - // download fails here (gzip CRC32 + JSON) and is re-downloaded - // instead of producing garbage that crashes JSON.parse later. - await extractTarGz(archivePath, extractDir); - const raw = await readFile( - join(extractDir, "openapi.json"), - "utf8", - ); - void JSON.parse(raw); - }, - }, - ); - } catch (err) { - // Fall back to the raw openapi.json if the archive is not available - // yet (e.g. the middlecache pipeline has not shipped it). - console.warn( - `Failed to fetch OpenAPI archive, falling back to raw openapi.json: ${(err as Error).message}`, - ); - await downloadToDotTempIfNotPresent( - `${MIDDLECACHE_BASE_URL}${API_SCHEMAS_PATH}`, - `middlecache/${API_SCHEMAS_PATH}`, - { - validate: async (filePath) => { - const raw = await readFile(filePath, "utf8"); - void JSON.parse(raw); - }, - }, - ); - } - - const raw = await readFile(join(extractDir, "openapi.json"), "utf8"); - - schema = await SwaggerParser.dereference(JSON.parse(raw)); } + return await SwaggerParser.dereference(JSON.parse(raw)); +}; - return schema; +/** + * Load (and cache) the Cloudflare API OpenAPI document. Prerender renders + * pages in parallel, so this is single-flighted to avoid duplicate derefs. + */ +export const getSchema = (): Promise => { + if (!schemaPromise) { + schemaPromise = loadSchema(); + } + return schemaPromise; }; diff --git a/src/util/custom-loaders.node.test.ts b/src/util/custom-loaders.node.test.ts index 91ebfe2ccab..53cdedda9d5 100644 --- a/src/util/custom-loaders.node.test.ts +++ b/src/util/custom-loaders.node.test.ts @@ -154,6 +154,42 @@ describe("downloadToDotTempIfNotPresent", () => { expect(fetchMock).toHaveBeenCalledTimes(1); expect(fs.readFileSync(join(TEST_DIR, "file.txt"), "utf8")).toBe("fresh"); }); + + test("concurrent calls to the same destination share a single download", async () => { + const fetchMock = vi.fn().mockResolvedValue(okResponse("shared")); + vi.stubGlobal("fetch", fetchMock); + + await Promise.all([ + downloadToDotTempIfNotPresent("https://example.com/file.txt", TEST_DEST), + downloadToDotTempIfNotPresent("https://example.com/file.txt", TEST_DEST), + downloadToDotTempIfNotPresent("https://example.com/file.txt", TEST_DEST), + ]); + + expect(fetchMock).toHaveBeenCalledTimes(1); + expect(fs.readFileSync(join(TEST_DIR, "file.txt"), "utf8")).toBe("shared"); + }); + + test("a failed in-flight download can be retried by a later call", async () => { + const fetchMock = vi + .fn() + .mockResolvedValueOnce(errResponse(500)) + .mockResolvedValueOnce(errResponse(500)) + .mockResolvedValueOnce(errResponse(500)) + .mockResolvedValueOnce(okResponse("ok")); + vi.stubGlobal("fetch", fetchMock); + + await expect( + downloadToDotTempIfNotPresent("https://example.com/file.txt", TEST_DEST), + ).rejects.toThrow(/HTTP 500/); + + await downloadToDotTempIfNotPresent( + "https://example.com/file.txt", + TEST_DEST, + ); + + expect(fetchMock).toHaveBeenCalledTimes(4); + expect(fs.readFileSync(join(TEST_DIR, "file.txt"), "utf8")).toBe("ok"); + }); }); describe("extractTarGz", () => { diff --git a/src/util/custom-loaders.ts b/src/util/custom-loaders.ts index fd28249dfc5..e42dab0b939 100644 --- a/src/util/custom-loaders.ts +++ b/src/util/custom-loaders.ts @@ -15,17 +15,45 @@ import * as z from "zod"; const MAX_DOWNLOAD_ATTEMPTS = 3; +// Serialize concurrent downloads of the same destination. Prerender renders +// pages in parallel, so multiple callers can hit the same middlecache file at +// once; without this they race on the temp-file write/rename and a retry's +// cleanup can delete a sibling call's freshly-written output. +const inFlightDownloads = new Map>(); + /** * Resolve the repo-root `.tmp/` directory used for downloaded artifacts. - * Prefers the file-relative path (how Astro/tsx resolve it) and falls back to - * the current working directory (needed under Vitest, where `import.meta.url` - * is not a `file://` URL). + * + * The tsx prebuild scripts and the bundled prerender resolve `import.meta.url` + * to different locations (source files vs `dist/.prerender/chunks/`), so the + * repo root is found by walking up from the module location until a + * `package.json` is found. Falls back to the current working directory for + * runtimes where `import.meta.url` is not a `file://` URL (e.g. Vitest). */ export const getDotTmpPath = () => { try { - return fileURLToPath(new URL("../../.tmp", import.meta.url)); + const moduleDir = dirname(fileURLToPath(import.meta.url)); + const root = findRepoRoot(moduleDir); + if (root) { + return join(root, ".tmp"); + } } catch { - return join(process.cwd(), ".tmp"); + // not a file:// URL (e.g. under Vitest) + } + return join(process.cwd(), ".tmp"); +}; + +const findRepoRoot = (startDir: string): string | undefined => { + let dir = startDir; + for (;;) { + if (fs.existsSync(join(dir, "package.json"))) { + return dir; + } + const parent = dirname(dir); + if (parent === dir) { + return undefined; + } + dir = parent; } }; @@ -58,14 +86,32 @@ export async function downloadToDotTempIfNotPresent( const destinationParts = relativeDestination.split("/"); const universalRelativeDestination = join(...destinationParts); - const dotTmpPath = getDotTmpPath(); + const destination = join(getDotTmpPath(), universalRelativeDestination); - const destination = join(dotTmpPath, universalRelativeDestination); + const inFlight = inFlightDownloads.get(destination); + if (inFlight) { + return inFlight; + } + + const promise = downloadWithRetry(source, url, destination, options.validate); + inFlightDownloads.set(destination, promise); + try { + await promise; + } finally { + inFlightDownloads.delete(destination); + } +} +const downloadWithRetry = async ( + source: string, + url: string, + destination: string, + validate: ((filePath: string) => Promise) | undefined, +) => { for (let attempt = 1; attempt <= MAX_DOWNLOAD_ATTEMPTS; attempt++) { try { if (fs.existsSync(destination)) { - await options.validate?.(destination); + await validate?.(destination); return; } @@ -106,7 +152,7 @@ export async function downloadToDotTempIfNotPresent( } fs.renameSync(tmpDestination, destination); - await options.validate?.(destination); + await validate?.(destination); return; } catch (err) { fs.rmSync(destination, { force: true }); @@ -119,7 +165,7 @@ export async function downloadToDotTempIfNotPresent( ); } } -} +}; /** * Extract a gzip-compressed tar archive into destinationDir. From ff285270de19c93ffa5b20ac051ec51a53b5e420 Mon Sep 17 00:00:00 2001 From: mvm Date: Fri, 28 Aug 2026 11:45:05 -0500 Subject: [PATCH 4/8] fix: skip fetch-openapi when schema already exists Match bin/fetch-skills.ts behavior: print a skip message and exit when the extracted openapi.json is already present, with --force to re-fetch. --- bin/fetch-openapi.ts | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/bin/fetch-openapi.ts b/bin/fetch-openapi.ts index 97f4fc6dc09..77e06b877fe 100644 --- a/bin/fetch-openapi.ts +++ b/bin/fetch-openapi.ts @@ -1,5 +1,6 @@ #!/usr/bin/env tsx +import fs from "fs"; import { readFile } from "node:fs/promises"; import { join } from "node:path"; @@ -15,7 +16,9 @@ const OPENAPI_JSON_PATH = "v1/cloudflare-api-schemas/openapi.json"; // --soft: warn and continue on failure instead of exiting non-zero. // Used by the predev hook so a network failure doesn't block local development. +// --force: re-fetch even if the schema already exists. const soft = process.argv.includes("--soft"); +const force = process.argv.includes("--force"); const fail = (message: string): never => { if (soft) { @@ -34,6 +37,14 @@ const extractDir = join( "v1", "cloudflare-api-schemas", ); +const openapiFile = join(extractDir, "openapi.json"); + +if (fs.existsSync(openapiFile) && !force) { + console.log( + "OpenAPI schema already exists, skipping fetch. (run `pnpm tsx bin/fetch-openapi.ts --force` to re-fetch)", + ); + process.exit(0); +} console.log("Fetching Cloudflare API OpenAPI schema from middlecache"); From a797a5865dbdf64e2f93333c4d6b46d68afde51a Mon Sep 17 00:00:00 2001 From: mvm Date: Fri, 28 Aug 2026 12:19:29 -0500 Subject: [PATCH 5/8] fix: run OpenAPI schema fetch in incremental builds Workers Builds invokes `pnpm run build:incremental`, which skipped the `prebuild` hook, so `bin/fetch-openapi.ts` never ran and prerendering failed with ENOENT on the schema file. Add a `prebuild:incremental` hook mirroring `prebuild` (fetch-skills + fetch-openapi) and make `getSchema` read-only so a build invoked without the pre-step fails loudly instead of silently downloading mid-render. Also factor the middlecache fetch/extract logic into `src/util/openapi-schema.ts` shared by `bin/fetch-openapi.ts` and `getSchema`. --- bin/fetch-openapi.ts | 53 +++--------------------------- package.json | 3 +- src/util/api.ts | 30 ++++++++--------- src/util/openapi-schema.ts | 67 ++++++++++++++++++++++++++++++++++++++ 4 files changed, 87 insertions(+), 66 deletions(-) create mode 100644 src/util/openapi-schema.ts diff --git a/bin/fetch-openapi.ts b/bin/fetch-openapi.ts index 77e06b877fe..485d7072a4d 100644 --- a/bin/fetch-openapi.ts +++ b/bin/fetch-openapi.ts @@ -1,18 +1,11 @@ #!/usr/bin/env tsx import fs from "fs"; -import { readFile } from "node:fs/promises"; -import { join } from "node:path"; import { - downloadToDotTempIfNotPresent, - extractTarGz, - getDotTmpPath, -} from "../src/util/custom-loaders"; - -const MIDDLECACHE_BASE_URL = "https://middlecache.ced.cloudflare.com/"; -const OPENAPI_ARCHIVE_PATH = "v1/cloudflare-api-schemas/openapi.tar.gz"; -const OPENAPI_JSON_PATH = "v1/cloudflare-api-schemas/openapi.json"; + fetchOpenApiSchema, + getOpenApiJsonPath, +} from "../src/util/openapi-schema"; // --soft: warn and continue on failure instead of exiting non-zero. // Used by the predev hook so a network failure doesn't block local development. @@ -31,13 +24,7 @@ const fail = (message: string): never => { process.exit(1); }; -const extractDir = join( - getDotTmpPath(), - "middlecache", - "v1", - "cloudflare-api-schemas", -); -const openapiFile = join(extractDir, "openapi.json"); +const openapiFile = getOpenApiJsonPath(); if (fs.existsSync(openapiFile) && !force) { console.log( @@ -49,37 +36,7 @@ if (fs.existsSync(openapiFile) && !force) { console.log("Fetching Cloudflare API OpenAPI schema from middlecache"); try { - try { - // Prefer the gzip-compressed archive (integrity-checkable via gzip - // CRC32), falling back to the raw openapi.json. - await downloadToDotTempIfNotPresent( - `${MIDDLECACHE_BASE_URL}${OPENAPI_ARCHIVE_PATH}`, - `middlecache/${OPENAPI_ARCHIVE_PATH}`, - { - validate: async (archivePath) => { - // Extract + parse as the integrity check: a corrupt or truncated - // download fails here (gzip CRC32 + JSON) and is re-downloaded. - await extractTarGz(archivePath, extractDir); - const raw = await readFile(join(extractDir, "openapi.json"), "utf8"); - void JSON.parse(raw); - }, - }, - ); - } catch (err) { - console.warn( - `Failed to fetch OpenAPI archive, falling back to raw openapi.json: ${(err as Error).message}`, - ); - await downloadToDotTempIfNotPresent( - `${MIDDLECACHE_BASE_URL}${OPENAPI_JSON_PATH}`, - `middlecache/${OPENAPI_JSON_PATH}`, - { - validate: async (filePath) => { - const raw = await readFile(filePath, "utf8"); - void JSON.parse(raw); - }, - }, - ); - } + await fetchOpenApiSchema(); } catch (err) { fail(`fetch failed: ${err}`); } diff --git a/package.json b/package.json index b214ebf1f1f..c20ccd71def 100644 --- a/package.json +++ b/package.json @@ -38,7 +38,8 @@ "flue:reset:local": "rm -rf .flue/.wrangler/state .flue/.wrangler/tmp .flue/dist/cloudflare_docs_flue/.wrangler/state && echo 'Cleared local flue dev state (Durable Objects + R2). Stop the dev server before running this.'", "flue:evals": "tsx .flue/bin/run-evals.ts", "lint": "eslint", - "prepare": "husky" + "prepare": "husky", + "prebuild:incremental": "tsx bin/fetch-skills.ts && tsx bin/fetch-openapi.ts" }, "devDependencies": { "@actions/core": "3.0.1", diff --git a/src/util/api.ts b/src/util/api.ts index a03aa3efab8..3752547a5b7 100644 --- a/src/util/api.ts +++ b/src/util/api.ts @@ -1,37 +1,33 @@ /** * OpenAPI schema loader for the APIRequest component. * - * The schema is fetched and extracted by `bin/fetch-openapi.ts`, which runs - * from the `prebuild`/`predev` hooks (see package.json), so the prerender only - * reads the local copy at `.tmp/middlecache/v1/cloudflare-api-schemas/`. - * Dereferenced result is memoized so the deref runs once per build, not per + * The schema is fetched by `bin/fetch-openapi.ts` from the `prebuild` and + * `prebuild:incremental` hooks (see package.json). `getSchema` reads the local + * copy and fails loudly if it is missing, so a build invoked without the + * pre-step is caught early instead of silently downloading mid-render. The + * dereferenced result is memoized so the deref runs once per build, not per * component instance. */ import SwaggerParser from "@apidevtools/swagger-parser"; import type { OpenAPI } from "openapi-types"; -import { getDotTmpPath } from "./custom-loaders"; import { readFile } from "node:fs/promises"; -import { join } from "node:path"; - -const OPENAPI_JSON_PATH = join( - getDotTmpPath(), - "middlecache", - "v1", - "cloudflare-api-schemas", - "openapi.json", -); +import { getOpenApiJsonPath } from "./openapi-schema"; let schemaPromise: Promise | undefined; const loadSchema = async (): Promise => { + const openapiFile = getOpenApiJsonPath(); + let raw: string; try { - raw = await readFile(OPENAPI_JSON_PATH, "utf8"); - } catch (err) { + raw = await readFile(openapiFile, "utf8"); + } catch (cause) { throw new Error( - `OpenAPI schema not found at ${OPENAPI_JSON_PATH} — run \`pnpm run build\` (or \`pnpm prebuild\`) first. ${(err as Error).message}`, + `OpenAPI schema not found at ${openapiFile}. Run \`pnpm run build\` (or \`pnpm run build:incremental\`) so the prebuild hook fetches it first.`, + { cause }, ); } + return await SwaggerParser.dereference(JSON.parse(raw)); }; diff --git a/src/util/openapi-schema.ts b/src/util/openapi-schema.ts new file mode 100644 index 00000000000..1433ee1255c --- /dev/null +++ b/src/util/openapi-schema.ts @@ -0,0 +1,67 @@ +/** + * Download and extract the Cloudflare API OpenAPI schema from middlecache. + * + * Used by `bin/fetch-openapi.ts`, which runs from the `prebuild`, + * `prebuild:incremental`, and `predev` hooks (see package.json). + */ +import { readFile } from "node:fs/promises"; +import { join } from "node:path"; + +import { + downloadToDotTempIfNotPresent, + extractTarGz, + getDotTmpPath, +} from "./custom-loaders"; + +const MIDDLECACHE_BASE_URL = "https://middlecache.ced.cloudflare.com/"; +const OPENAPI_ARCHIVE_PATH = "v1/cloudflare-api-schemas/openapi.tar.gz"; +const OPENAPI_JSON_PATH = "v1/cloudflare-api-schemas/openapi.json"; + +export const getOpenApiExtractDir = () => + join(getDotTmpPath(), "middlecache", "v1", "cloudflare-api-schemas"); + +export const getOpenApiJsonPath = () => + join(getOpenApiExtractDir(), "openapi.json"); + +/** + * Download the schema into `.tmp` and return the path to `openapi.json`. + * + * Prefers the gzip-compressed archive (integrity-checkable via gzip CRC32), + * falling back to the raw openapi.json. Downloads are validated (extract + + * parse) and retried by `downloadToDotTempIfNotPresent`. + */ +export const fetchOpenApiSchema = async (): Promise => { + const extractDir = getOpenApiExtractDir(); + + try { + await downloadToDotTempIfNotPresent( + `${MIDDLECACHE_BASE_URL}${OPENAPI_ARCHIVE_PATH}`, + `middlecache/${OPENAPI_ARCHIVE_PATH}`, + { + validate: async (archivePath) => { + // Extract + parse as the integrity check: a corrupt or truncated + // download fails here (gzip CRC32 + JSON) and is re-downloaded. + await extractTarGz(archivePath, extractDir); + const raw = await readFile(join(extractDir, "openapi.json"), "utf8"); + void JSON.parse(raw); + }, + }, + ); + } catch (err) { + console.warn( + `Failed to fetch OpenAPI archive, falling back to raw openapi.json: ${(err as Error).message}`, + ); + await downloadToDotTempIfNotPresent( + `${MIDDLECACHE_BASE_URL}${OPENAPI_JSON_PATH}`, + `middlecache/${OPENAPI_JSON_PATH}`, + { + validate: async (filePath) => { + const raw = await readFile(filePath, "utf8"); + void JSON.parse(raw); + }, + }, + ); + } + + return getOpenApiJsonPath(); +}; From eb52c7f1ed93b73735a52b34495be21a531b6afd Mon Sep 17 00:00:00 2001 From: mvm Date: Fri, 28 Aug 2026 12:59:40 -0500 Subject: [PATCH 6/8] fix: address PR review findings on middlecache download hardening - custom-loaders: reject tar members with `..` segments or absolute paths (zip-slip); reject on spawn `error` instead of hanging; capture tar stderr in extraction errors - openapi-schema: extract to a staging dir and atomically promote on success, so a failed extract/parse leaves no stale files to mask a fresh download failure - tests: check spawnSync status when building fixtures; add regression tests for unsafe tar member paths - package.json: share the fetch-skills + fetch-openapi command via a single fetch:assets script used by prebuild and prebuild:incremental --- package.json | 5 ++- src/util/custom-loaders.node.test.ts | 61 +++++++++++++++++++++++++++- src/util/custom-loaders.ts | 45 +++++++++++++++++--- src/util/openapi-schema.ts | 31 +++++++++++--- 4 files changed, 126 insertions(+), 16 deletions(-) diff --git a/package.json b/package.json index c20ccd71def..14f79723422 100644 --- a/package.json +++ b/package.json @@ -5,7 +5,7 @@ "scripts": { "preinstall": "npx --yes only-allow pnpm", "astro": "astro", - "prebuild": "tsx bin/fetch-skills.ts && tsx bin/fetch-openapi.ts", + "prebuild": "pnpm run fetch:assets", "build": "astro build", "build:incremental": "INCREMENTAL_BUILD=true astro build", "typegen:worker": "wrangler types ./worker/worker-configuration.d.ts", @@ -39,7 +39,8 @@ "flue:evals": "tsx .flue/bin/run-evals.ts", "lint": "eslint", "prepare": "husky", - "prebuild:incremental": "tsx bin/fetch-skills.ts && tsx bin/fetch-openapi.ts" + "prebuild:incremental": "pnpm run fetch:assets", + "fetch:assets": "tsx bin/fetch-skills.ts && tsx bin/fetch-openapi.ts" }, "devDependencies": { "@actions/core": "3.0.1", diff --git a/src/util/custom-loaders.node.test.ts b/src/util/custom-loaders.node.test.ts index 53cdedda9d5..ddec7733248 100644 --- a/src/util/custom-loaders.node.test.ts +++ b/src/util/custom-loaders.node.test.ts @@ -2,6 +2,7 @@ import { afterEach, beforeEach, describe, expect, test, vi } from "vitest"; import fs from "fs"; import { spawnSync } from "node:child_process"; import { join } from "node:path"; +import zlib from "node:zlib"; import { downloadToDotTempIfNotPresent, @@ -16,6 +17,37 @@ const TEST_DEST = "middlecache/__custom-loaders-test__/file.txt"; const okResponse = (body: string) => new Response(body); const errResponse = (status: number) => new Response("error", { status }); +// Minimal ustar header so a test can craft an archive with an arbitrary +// (potentially unsafe) member path, which the system `tar` refuses to create. +const ustarHeader = (name: string, size: number): Buffer => { + const header = Buffer.alloc(512); + header.write(name, 0, 100, "utf8"); + header.write("0000644\0", 100, 8, "ascii"); + header.write("0000000\0", 108, 8, "ascii"); + header.write("0000000\0", 116, 8, "ascii"); + header.write(size.toString(8).padStart(11, "0") + "\0", 124, 12, "ascii"); + header.write("00000000000\0", 136, 12, "ascii"); + header.fill(0x20, 148, 156); + header.write("0", 156, 1, "ascii"); + const checksum = header.reduce((sum, byte) => sum + byte, 0); + header.write(checksum.toString(8).padStart(6, "0"), 148, 6, "ascii"); + header[154] = 0; + header[155] = 0x20; + return header; +}; + +const buildUnsafeArchive = (tarballPath: string, memberPath: string) => { + const body = Buffer.from("evil"); + const block = Buffer.alloc(512); + body.copy(block); + const raw = Buffer.concat([ + ustarHeader(memberPath, body.length), + block, + Buffer.alloc(1024), + ]); + fs.writeFileSync(tarballPath, zlib.gzipSync(raw)); +}; + describe("downloadToDotTempIfNotPresent", () => { beforeEach(() => { fs.rmSync(TEST_DIR, { recursive: true, force: true }); @@ -203,7 +235,7 @@ describe("extractTarGz", () => { join(FIXTURE, "src", "skills", "skill-name", "file.txt"), "content", ); - spawnSync( + const result = spawnSync( "tar", [ "-czf", @@ -212,8 +244,15 @@ describe("extractTarGz", () => { join(FIXTURE, "src"), "skills", ], - { stdio: "ignore" }, + { stdio: ["ignore", "ignore", "pipe"] }, ); + if (result.status !== 0 || result.error) { + throw new Error( + `Failed to build test fixture archive: ${ + result.error?.message ?? "" + } ${result.stderr?.toString() ?? ""}`.trim(), + ); + } }; beforeEach(() => { @@ -256,4 +295,22 @@ describe("extractTarGz", () => { extractTarGz(join(FIXTURE, "bad.tar.gz"), join(FIXTURE, "out")), ).rejects.toThrow(/tar extraction failed/); }); + + test("rejects archive members that escape the destination directory", async () => { + fs.mkdirSync(FIXTURE, { recursive: true }); + buildUnsafeArchive(join(FIXTURE, "escape.tar.gz"), "../evil.txt"); + + await expect( + extractTarGz(join(FIXTURE, "escape.tar.gz"), join(FIXTURE, "out")), + ).rejects.toThrow(/unsafe member path/); + }); + + test("rejects archive members with absolute paths", async () => { + fs.mkdirSync(FIXTURE, { recursive: true }); + buildUnsafeArchive(join(FIXTURE, "escape.tar.gz"), "/etc/evil.txt"); + + await expect( + extractTarGz(join(FIXTURE, "escape.tar.gz"), join(FIXTURE, "out")), + ).rejects.toThrow(/unsafe member path/); + }); }); diff --git a/src/util/custom-loaders.ts b/src/util/custom-loaders.ts index e42dab0b939..ba7699a839b 100644 --- a/src/util/custom-loaders.ts +++ b/src/util/custom-loaders.ts @@ -7,7 +7,7 @@ import { fileURLToPath } from "node:url"; import { Readable } from "node:stream"; import type { ReadableStream as WebReadableStream } from "node:stream/web"; import { writeFile } from "node:fs/promises"; -import { spawn } from "node:child_process"; +import { spawn, spawnSync } from "node:child_process"; import fs from "fs"; import { dirname, join } from "path"; @@ -170,6 +170,11 @@ const downloadWithRetry = async ( /** * Extract a gzip-compressed tar archive into destinationDir. * + * Member paths are validated before extraction: entries containing `..` + * segments or leading `/` are rejected (zip-slip) so a network-downloaded + * archive cannot write outside destinationDir. Extraction failures reject + * with captured stderr instead of hanging. + * * @param options.stripComponents - strip the given number of leading path * components from each entry before extracting (matches the skills archive, * which contains a top-level `skills/` directory). @@ -179,6 +184,27 @@ export async function extractTarGz( destinationDir: string, options: { stripComponents?: number } = {}, ): Promise { + // Refuse archives whose members could escape destinationDir. + const list = spawnSync("tar", ["-tzf", tarballPath], { encoding: "utf8" }); + if (list.status !== 0 || list.error) { + throw new Error( + `tar extraction failed for ${tarballPath}: not a valid archive${ + list.stderr ? `: ${list.stderr.trim()}` : "" + }`, + ); + } + for (const member of list.stdout.split("\n")) { + const name = member.trimEnd(); + if (!name) { + continue; + } + if (name.startsWith("/") || name.split("/").includes("..")) { + throw new Error( + `tar extraction failed for ${tarballPath}: refusing unsafe member path "${name}"`, + ); + } + } + fs.mkdirSync(destinationDir, { recursive: true }); const args = ["-xz", "-C", destinationDir]; @@ -187,13 +213,20 @@ export async function extractTarGz( } args.push("-f", tarballPath); - const tar = spawn("tar", args, { stdio: "ignore" }); - const exitCode = await new Promise((resolve) => - tar.on("close", resolve), - ); + const tar = spawn("tar", args, { stdio: ["ignore", "ignore", "pipe"] }); + const stderr: Buffer[] = []; + tar.stderr?.on("data", (chunk: Buffer) => stderr.push(chunk)); + const exitCode = await new Promise((resolve, reject) => { + tar.on("error", reject); + tar.on("close", resolve); + }); if (exitCode !== 0) { throw new Error( - `tar extraction failed for ${tarballPath} (exit code ${exitCode})`, + `tar extraction failed for ${tarballPath} (exit code ${exitCode})${ + stderr.length + ? `: ${Buffer.concat(stderr).toString("utf8").trim()}` + : "" + }`, ); } } diff --git a/src/util/openapi-schema.ts b/src/util/openapi-schema.ts index 1433ee1255c..0ffd5ab5fa9 100644 --- a/src/util/openapi-schema.ts +++ b/src/util/openapi-schema.ts @@ -4,7 +4,7 @@ * Used by `bin/fetch-openapi.ts`, which runs from the `prebuild`, * `prebuild:incremental`, and `predev` hooks (see package.json). */ -import { readFile } from "node:fs/promises"; +import { mkdir, readFile, rename, rm } from "node:fs/promises"; import { join } from "node:path"; import { @@ -39,11 +39,30 @@ export const fetchOpenApiSchema = async (): Promise => { `middlecache/${OPENAPI_ARCHIVE_PATH}`, { validate: async (archivePath) => { - // Extract + parse as the integrity check: a corrupt or truncated - // download fails here (gzip CRC32 + JSON) and is re-downloaded. - await extractTarGz(archivePath, extractDir); - const raw = await readFile(join(extractDir, "openapi.json"), "utf8"); - void JSON.parse(raw); + // Extract into a staging directory, then promote it on success + // so a failed extract or parse never leaves stale or partial + // files in the destination that could mask a fresh failure. + const stagingDir = join( + getDotTmpPath(), + "middlecache", + "v1", + ".openapi-staging", + ); + await rm(stagingDir, { recursive: true, force: true }); + try { + await mkdir(stagingDir, { recursive: true }); + await extractTarGz(archivePath, stagingDir); + const raw = await readFile( + join(stagingDir, "openapi.json"), + "utf8", + ); + void JSON.parse(raw); + await rm(extractDir, { recursive: true, force: true }); + await rename(stagingDir, extractDir); + } catch (err) { + await rm(stagingDir, { recursive: true, force: true }); + throw err; + } }, }, ); From b1de7eb776820ca7d8fdec6e9141fcae6b29e460 Mon Sep 17 00:00:00 2001 From: mvm Date: Fri, 28 Aug 2026 13:12:00 -0500 Subject: [PATCH 7/8] fix: reject fetch responses without a body instead of asserting Replaces the `response.body!` non-null assertion in downloadWithRetry with an explicit check, so a bodyless response (e.g. 204/HEAD) fails with a clear error instead of an unhelpful TypeError from Readable.fromWeb(null). Adds a regression test. --- src/util/custom-loaders.node.test.ts | 11 +++++++++++ src/util/custom-loaders.ts | 6 +++++- 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/src/util/custom-loaders.node.test.ts b/src/util/custom-loaders.node.test.ts index ddec7733248..25d8650708c 100644 --- a/src/util/custom-loaders.node.test.ts +++ b/src/util/custom-loaders.node.test.ts @@ -101,6 +101,17 @@ describe("downloadToDotTempIfNotPresent", () => { expect(fetchMock).toHaveBeenCalledTimes(3); }); + test("rejects when the response has no body", async () => { + vi.stubGlobal( + "fetch", + vi.fn().mockResolvedValue(new Response(null, { status: 200 })), + ); + + await expect( + downloadToDotTempIfNotPresent("https://example.com/file.txt", TEST_DEST), + ).rejects.toThrow(/Missing response body/); + }); + test("re-downloads when the validate callback rejects", async () => { const fetchMock = vi .fn() diff --git a/src/util/custom-loaders.ts b/src/util/custom-loaders.ts index ba7699a839b..8562c89eb3d 100644 --- a/src/util/custom-loaders.ts +++ b/src/util/custom-loaders.ts @@ -135,10 +135,14 @@ const downloadWithRetry = async ( ); } + if (!response.body) { + throw new Error(`Missing response body for ${url}`); + } + // Stream file to destination to avoid storing in memory await writeFile( tmpDestination, - Readable.fromWeb(response.body! as WebReadableStream), + Readable.fromWeb(response.body as WebReadableStream), ); const expectedLength = Number(response.headers.get("content-length")); From fa057e715d6365c00922c34604b65399a79ee391 Mon Sep 17 00:00:00 2001 From: mvm Date: Fri, 28 Aug 2026 13:47:38 -0500 Subject: [PATCH 8/8] test: trigger a build with no source changes to measure warm-cache build time