From ed6b04fe27ab2a443938ad95e33e72f5582099d6 Mon Sep 17 00:00:00 2001 From: James Date: Wed, 9 Sep 2026 03:25:07 -0700 Subject: [PATCH 1/2] fix(cli): validate captured image and font downloads --- bun.lock | 4 + packages/cli/package.json | 2 + .../cli/src/capture/assetDownloader.test.ts | 136 +++++++++++++++- packages/cli/src/capture/assetDownloader.ts | 36 +++-- .../src/capture/captureFontValidation.test.ts | 40 +++++ .../cli/src/capture/captureFontValidation.ts | 59 +++++++ .../capture/captureImageValidation.test.ts | 42 +++++ .../cli/src/capture/captureImageValidation.ts | 149 ++++++++++++++++++ .../src/capture/readBoundedResponse.test.ts | 54 +++++++ .../cli/src/capture/readBoundedResponse.ts | 28 ++++ 10 files changed, 528 insertions(+), 22 deletions(-) create mode 100644 packages/cli/src/capture/captureFontValidation.test.ts create mode 100644 packages/cli/src/capture/captureFontValidation.ts create mode 100644 packages/cli/src/capture/captureImageValidation.test.ts create mode 100644 packages/cli/src/capture/captureImageValidation.ts create mode 100644 packages/cli/src/capture/readBoundedResponse.test.ts create mode 100644 packages/cli/src/capture/readBoundedResponse.ts diff --git a/bun.lock b/bun.lock index 9b76975680..7697f43c3c 100644 --- a/bun.lock +++ b/bun.lock @@ -67,6 +67,7 @@ "adm-zip": "^0.6.0", "citty": "^0.2.1", "compare-versions": "^6.1.1", + "css-tree": "^3.2.1", "debug": "^4.4.0", "esbuild": "^0.25.12", "fontkit": "^2.0.4", @@ -91,6 +92,7 @@ "@hyperframes/producer": "workspace:*", "@hyperframes/studio": "workspace:*", "@hyperframes/studio-server": "workspace:*", + "@types/css-tree": "^3.2.0", "@types/fontkit": "^2.0.9", "@types/mime-types": "^3.0.1", "@types/node": "^25.0.10", @@ -1160,6 +1162,8 @@ "@types/chrome": ["@types/chrome@0.0.326", "", { "dependencies": { "@types/filesystem": "*", "@types/har-format": "*" } }, "sha512-WS7jKf3ZRZFHOX7dATCZwqNJgdfiSF0qBRFxaO0LhIOvTNBrfkab26bsZwp6EBpYtqp8loMHJTnD6vDTLWPKYw=="], + "@types/css-tree": ["@types/css-tree@3.2.0", "", {}, "sha512-J5KXmk6BFIepOT7280FdFyNs4c5fh0Uee0otjKEvzchrRs38Ii9qminqc4ds0L19X8Zd/rT+brR9jkBthygjWw=="], + "@types/deep-eql": ["@types/deep-eql@4.0.2", "", {}, "sha512-c9h9dVVMigMPc4bwTvC5dxqtqJZwQPePsWjPlpSOnojbor6pGqdk541lfA7AqFQr5pB1BRdq0juY9db81BwyFw=="], "@types/dom-mediacapture-transform": ["@types/dom-mediacapture-transform@0.1.11", "", { "dependencies": { "@types/dom-webcodecs": "*" } }, "sha512-Y2p+nGf1bF2XMttBnsVPHUWzRRZzqUoJAKmiP10b5umnO6DDrWI0BrGDJy1pOHoOULVmGSfFNkQrAlC5dcj6nQ=="], diff --git a/packages/cli/package.json b/packages/cli/package.json index a46445a3e5..42f90d188b 100644 --- a/packages/cli/package.json +++ b/packages/cli/package.json @@ -35,6 +35,7 @@ "adm-zip": "^0.6.0", "citty": "^0.2.1", "compare-versions": "^6.1.1", + "css-tree": "^3.2.1", "debug": "^4.4.0", "esbuild": "^0.25.12", "fontkit": "^2.0.4", @@ -59,6 +60,7 @@ "@hyperframes/producer": "workspace:*", "@hyperframes/studio": "workspace:*", "@hyperframes/studio-server": "workspace:*", + "@types/css-tree": "^3.2.0", "@types/fontkit": "^2.0.9", "@types/mime-types": "^3.0.1", "@types/node": "^25.0.10", diff --git a/packages/cli/src/capture/assetDownloader.test.ts b/packages/cli/src/capture/assetDownloader.test.ts index 857c898baa..ae6ae28a73 100644 --- a/packages/cli/src/capture/assetDownloader.test.ts +++ b/packages/cli/src/capture/assetDownloader.test.ts @@ -1,5 +1,5 @@ import { afterEach, describe, expect, it, vi } from "vitest"; -import { mkdtempSync, rmSync } from "node:fs"; +import { mkdtempSync, readFileSync, readdirSync, rmSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { @@ -289,7 +289,20 @@ describe("drop counts — why a referenced asset is not in the capture", () => { await withTempDir(async (dir) => { vi.stubGlobal( "fetch", - vi.fn(async () => new Response(new Uint8Array(2048), { status: 200 })), + vi.fn( + async () => + new Response( + new Uint8Array( + readFileSync( + new URL( + "../../../../docs/public/catalog/assets/a634cb9e7783af7e.woff2", + import.meta.url, + ), + ), + ), + { status: 200 }, + ), + ), ); const { css, drops } = await downloadAndRewriteFonts(fontCss(1), dir); expect(css).toContain("assets/fonts/font-0.woff2"); @@ -303,11 +316,21 @@ describe("drop counts — why a referenced asset is not in the capture", () => { }); it("counts an image dropped for being under the raster floor", async () => { - // 9 KB is under the 10 KB floor. Nothing lands, and the reason is now on the record. + // A valid small PNG is under the 10 KB floor; preserve the recorded drop reason. await withTempDir(async (dir) => { vi.stubGlobal( "fetch", - vi.fn(async () => new Response(new Uint8Array(9000), { status: 200 })), + vi.fn( + async () => + new Response( + new Uint8Array( + await sharp({ create: { width: 2, height: 2, channels: 4, background: "red" } }) + .png() + .toBuffer(), + ), + { status: 200 }, + ), + ), ); const { assets, drops } = await downloadAssets(tokensWithNoSvgs(), dir, [ { type: "Image", url: "https://cdn.example/hero.png", contexts: ["img[src]"] }, @@ -374,7 +397,8 @@ describe("asset fetches present the same identity as the page navigation", () => headers: { "content-type": "text/html" }, }); } - const body = new Uint8Array(4096); + const body = + ''; return new Response(body, { status: 200, headers: { "content-type": "image/svg+xml" } }); }); } @@ -404,6 +428,22 @@ describe("asset fetches present the same identity as the page navigation", () => }); }); +async function testIco(): Promise { + const png = await sharp({ create: { width: 16, height: 16, channels: 4, background: "red" } }) + .png() + .toBuffer(); + const header = Buffer.alloc(22); + header.writeUInt16LE(1, 2); + header.writeUInt16LE(1, 4); + header[6] = 16; + header[7] = 16; + header.writeUInt16LE(1, 10); + header.writeUInt16LE(32, 12); + header.writeUInt32LE(png.length, 14); + header.writeUInt32LE(22, 18); + return Buffer.concat([header, png]); +} + describe("declared icons — keep them all, headline the bare mark", () => { afterEach(() => vi.unstubAllGlobals()); @@ -451,7 +491,7 @@ describe("declared icons — keep them all, headline the bare mark", () => { "fetch", serve({ "favicon.svg": BADGE_SVG, - "favicon.ico": Buffer.from("not a decodable ico"), + "favicon.ico": await testIco(), "apple-icon.png": await solidPng(1), }), ); @@ -474,7 +514,7 @@ describe("declared icons — keep them all, headline the bare mark", () => { "fetch", serve({ "favicon.svg": BADGE_SVG, - "favicon.ico": Buffer.from("not a decodable ico"), + "favicon.ico": await testIco(), "apple-icon.png": await solidPng(1), }), ); @@ -537,3 +577,85 @@ describe("declared icons — keep them all, headline the bare mark", () => { }); }); }); + +describe("capture download security boundaries", () => { + afterEach(() => vi.unstubAllGlobals()); + + it("canonicalizes an icon ADS suffix and its promoted copy while preserving SVG bytes", async () => { + await withTempDir(async (dir) => { + const body = + ''; + vi.stubGlobal( + "fetch", + vi.fn(async () => new Response(body)), + ); + const { icons } = await downloadAssets( + tokensWithNoSvgs(), + dir, + [], + [ + { + rel: "icon", + href: "https://public.example/favicon.svg:payload", + sizes: null, + type: null, + }, + ], + ); + expect(icons.icons[0]?.file).toBe("assets/icon-icon-unsized.svg"); + expect(icons.headline?.file).toBe("assets/favicon.svg"); + expect(readFileSync(join(dir, "assets/favicon.svg"), "utf8")).toBe(body); + }); + }); + + it("does not publish arbitrary bytes as an OG image", async () => { + await withTempDir(async (dir) => { + vi.stubGlobal( + "fetch", + vi.fn(async () => new Response(new Uint8Array(6000).fill(65))), + ); + const tokens = tokensWithNoSvgs(); + tokens.ogImage = "https://public.example/og.jpg::$DATA"; + const { assets, drops } = await downloadAssets(tokens, dir); + expect(assets).toEqual([]); + expect(drops.unavailable).toBe(1); + expect(readdirSync(join(dir, "assets"))).not.toContain("og-image.jpg::$DATA"); + }); + }); + + it.each(["font.woff2:ads", "CON.woff2"])( + "writes valid font %s under a safe name and rewrites CSS", + async (name) => { + await withTempDir(async (dir) => { + const bytes = readFileSync( + new URL("../../../../docs/public/catalog/assets/a634cb9e7783af7e.woff2", import.meta.url), + ); + vi.stubGlobal( + "fetch", + vi.fn(async () => new Response(new Uint8Array(bytes))), + ); + const source = `@font-face{font-family:Demo;src:url(https://public.example/${name})}`; + const result = await downloadAndRewriteFonts(source, dir); + const names = readdirSync(join(dir, "assets/fonts")); + expect(names).toHaveLength(1); + expect(names[0]).not.toMatch(/:|^CON\./i); + expect(result.css).toContain(`assets/fonts/${names[0]}`); + expect(readFileSync(join(dir, "assets/fonts", names[0]!))).toEqual(bytes); + }); + }, + ); + + it("leaves invalid font URLs unchanged and records their rejection", async () => { + await withTempDir(async (dir) => { + vi.stubGlobal( + "fetch", + vi.fn(async () => new Response(new Uint8Array(2048))), + ); + const source = "@font-face{font-family:Demo;src:url(https://public.example/font.woff2)}"; + const result = await downloadAndRewriteFonts(source, dir); + expect(result.css).toBe(source); + expect(result.drops.unavailable).toBe(1); + expect(readdirSync(join(dir, "assets/fonts"))).toEqual([]); + }); + }); +}); diff --git a/packages/cli/src/capture/assetDownloader.ts b/packages/cli/src/capture/assetDownloader.ts index 0f827be6d2..e4905990c4 100644 --- a/packages/cli/src/capture/assetDownloader.ts +++ b/packages/cli/src/capture/assetDownloader.ts @@ -13,6 +13,9 @@ import type { CatalogedAsset } from "./assetCataloger.js"; import { CAPTURE_USER_AGENT } from "./userAgent.js"; import { rankIconCandidates, type IconCandidate } from "./faviconRanker.js"; import { classifyIcon, type IconShape } from "./iconClassifier.js"; +import { readBoundedResponse } from "./readBoundedResponse.js"; +import { captureFontExtension, captureFontFilename } from "./captureFontValidation.js"; +import { captureImageExtension } from "./captureImageValidation.js"; interface DownloadBudgetOptions { remainingMs?: () => number; @@ -190,9 +193,10 @@ async function fetchAndInspectIcon( outputDir: string, timeoutMs: number, ): Promise<{ record: IconRecord; buffer: Buffer } | null> { - const ext = extname(new URL(icon.href).pathname) || ".ico"; const buffer = await fetchBuffer(icon.href, timeoutMs); if (!buffer) return null; + const ext = await captureImageExtension(buffer); + if (!ext) return null; const file = `assets/${stem}${ext}`; writeFileSync(join(outputDir, file), buffer); @@ -405,14 +409,17 @@ export async function downloadAssets( const results = await Promise.allSettled( batch.map(async ({ url, isPoster, catalog }) => { const parsedUrl = new URL(url); - const pathExt = extname(parsedUrl.pathname); - const ext = pathExt && pathExt.length <= 5 ? pathExt : ".jpg"; const buffer = await fetchBuffer(url, Math.min(10_000, remainingMs)); if (!buffer) { drops.unavailable++; return null; } - const isSvg = ext === ".svg" || url.includes(".svg"); + const ext = await captureImageExtension(buffer); + if (!ext) { + drops.unavailable++; + return null; + } + const isSvg = ext === ".svg"; const minSize = isSvg ? 200 : 10000; if (buffer.length < minSize) { drops["size-floor"]++; @@ -464,17 +471,17 @@ export async function downloadAssets( if (tokens.ogImage && !downloadedUrls.has(normalizeUrl(tokens.ogImage))) { const remainingMs = options.remainingMs?.() ?? 10_000; try { - const ext = extname(new URL(tokens.ogImage).pathname) || ".jpg"; - const localPath = `assets/og-image${ext}`; if (remainingMs <= 0) { drops["budget-exhausted"]++; } else { const buffer = await fetchBuffer(tokens.ogImage, Math.min(10_000, remainingMs)); - if (!buffer) { + const ext = buffer && (await captureImageExtension(buffer)); + if (!buffer || !ext) { drops.unavailable++; } else if (buffer.length <= 5000) { drops["size-floor"]++; } else { + const localPath = `assets/og-image${ext}`; writeFileSync(join(outputDir, localPath), buffer); assets.push({ url: tokens.ogImage, localPath, type: "image" }); } @@ -575,13 +582,12 @@ export async function downloadAndRewriteFonts( count++; try { - const urlObj = new URL(fontUrl); - const filename = urlObj.pathname.split("/").pop() || `font-${count}.woff2`; - const localPath = join(assetsDir, filename); - const relativePath = `assets/fonts/${filename}`; - const buffer = await fetchBuffer(fontUrl, Math.min(10_000, remainingMs)); - if (buffer) { + const extension = buffer && captureFontExtension(buffer); + if (buffer && extension) { + const filename = captureFontFilename(fontUrl, extension); + const localPath = join(assetsDir, filename); + const relativePath = `assets/fonts/${filename}`; writeFileSync(localPath, buffer); rewritten = rewritten.split(fontUrl).join(relativePath); } else { @@ -697,8 +703,8 @@ async function fetchBuffer(url: string, timeoutMs = 10_000): Promise { + it("accepts the existing catalog WOFF2 font without changing its bytes", () => { + expect(captureFontExtension(font)).toBe(".woff2"); + expect(captureFontFilename("https://fonts.example/font-0.woff2", ".woff2")).toBe( + "font-0.woff2", + ); + }); + + it.each(["font.woff2:ads", "CON.woff2", "nul.extra.woff2", "LPT1.woff2", "bad%5Cname.woff2"])( + "does not publish remote Windows-special filename %s", + (name) => { + const filename = captureFontFilename(`https://fonts.example/${name}`, ".woff2"); + const path = win32.join("C:\\capture\\assets\\fonts", filename); + expect(filename).not.toMatch(/[:\\/]/); + expect(filename).not.toMatch(/^(con|nul|prn|aux|com[1-9]|lpt[1-9])(?:\.|$)/i); + expect(win32.dirname(path)).toBe("C:\\capture\\assets\\fonts"); + }, + ); + + it("uses the validated extension regardless of URL suffix", () => { + expect(captureFontFilename("https://fonts.example/site.ttf", ".woff2")).toBe("site.woff2"); + }); + + it("rejects arbitrary bytes, truncated fonts, and oversized expansion declarations", () => { + expect(captureFontExtension(Buffer.alloc(2048))).toBeNull(); + expect(captureFontExtension(font.subarray(0, 48))).toBeNull(); + const bomb = Buffer.from(font); + bomb.writeUInt32BE(0xffffffff, 16); + expect(captureFontExtension(bomb)).toBeNull(); + }); +}); diff --git a/packages/cli/src/capture/captureFontValidation.ts b/packages/cli/src/capture/captureFontValidation.ts new file mode 100644 index 0000000000..06728a17e3 --- /dev/null +++ b/packages/cli/src/capture/captureFontValidation.ts @@ -0,0 +1,59 @@ +import { createHash } from "node:crypto"; +import { create } from "fontkit"; + +const MAX_FONT_BYTES = 75 * 1024 * 1024; + +/** Identify the container from bytes and require readable font metrics before publication. */ +function fontContainerExtension(bytes: Buffer): string | null { + if (bytes.length < 12 || bytes.length > MAX_FONT_BYTES) return null; + const signature = bytes.toString("ascii", 0, 4); + let extension: string; + if (signature === "wOFF" || signature === "wOF2") { + return woffExtension(bytes, signature); + } else if (signature === "OTTO") { + extension = ".otf"; + } else if (bytes.readUInt32BE(0) === 0x00010000 || signature === "true") { + extension = ".ttf"; + } else { + return null; + } + return extension; +} + +function woffExtension(bytes: Buffer, signature: string): string | null { + const headerSize = signature === "wOFF" ? 44 : 48; + if (bytes.length < headerSize || bytes.readUInt32BE(8) !== bytes.length) return null; + const expandedSize = bytes.readUInt32BE(16); + if (expandedSize < 12 || expandedSize > MAX_FONT_BYTES) return null; + return signature === "wOFF" ? ".woff" : ".woff2"; +} + +export function captureFontExtension(bytes: Buffer): string | null { + const extension = fontContainerExtension(bytes); + if (!extension) return null; + try { + const font = create(bytes); + if (!("numGlyphs" in font) || font.numGlyphs <= 0 || font.numGlyphs > 65535) return null; + if (!Number.isFinite(font.unitsPerEm) || font.unitsPerEm <= 0) return null; + // Exercise the character map too: fontkit creates several containers lazily. + if (!Array.isArray(font.characterSet)) return null; + return extension; + } catch { + return null; + } +} + +/** Preserve ordinary names; remote URL spelling must never select a Windows device or stream. */ +export function captureFontFilename(url: string, extension: string): string { + const basename = new URL(url).pathname.split("/").pop() ?? ""; + const dot = basename.lastIndexOf("."); + const stem = dot > 0 ? basename.slice(0, dot) : basename; + if ( + /^[a-zA-Z0-9_-][a-zA-Z0-9_.-]{0,119}$/.test(stem) && + !/[. ]$/.test(stem) && + !/^(?:con|prn|aux|nul|com[1-9]|lpt[1-9])(?:\.|$)/i.test(stem) + ) { + return `${stem}${extension}`; + } + return `font-${createHash("sha256").update(url).digest("hex").slice(0, 16)}${extension}`; +} diff --git a/packages/cli/src/capture/captureImageValidation.test.ts b/packages/cli/src/capture/captureImageValidation.test.ts new file mode 100644 index 0000000000..a297f47c38 --- /dev/null +++ b/packages/cli/src/capture/captureImageValidation.test.ts @@ -0,0 +1,42 @@ +import sharp from "sharp"; +import { describe, expect, it } from "vitest"; +import { captureImageExtension } from "./captureImageValidation.js"; + +function svg(content: string): Buffer { + return Buffer.from( + `${content}`, + ); +} + +describe("captured image validation", () => { + it("recognizes actual PNG bytes without needing a trusted URL or content type", async () => { + const bytes = await sharp({ create: { width: 2, height: 2, channels: 4, background: "red" } }) + .png() + .toBuffer(); + expect(await captureImageExtension(bytes)).toBe(".png"); + expect(await captureImageExtension(bytes.subarray(0, 40))).toBeNull(); + expect(await captureImageExtension(Buffer.alloc(6000, 65))).toBeNull(); + }); + + it("preserves theme CSS, custom properties, and local gradient references", async () => { + const bytes = svg( + '', + ); + const original = Buffer.from(bytes); + expect(await captureImageExtension(bytes)).toBe(".svg"); + expect(bytes).toEqual(original); + }); + + it.each([ + "", + '', + "
active HTML
", + '', + '', + "", + "", + '', + ])("rejects active or externally loading SVG: %s", async (content) => { + expect(await captureImageExtension(svg(content))).toBeNull(); + }); +}); diff --git a/packages/cli/src/capture/captureImageValidation.ts b/packages/cli/src/capture/captureImageValidation.ts new file mode 100644 index 0000000000..70345724ce --- /dev/null +++ b/packages/cli/src/capture/captureImageValidation.ts @@ -0,0 +1,149 @@ +import sharp from "sharp"; +import { DOMParser } from "linkedom"; +import { parse, walk } from "css-tree"; + +const SVG_ELEMENTS = new Set( + "svg g defs title desc metadata path rect circle ellipse line polyline polygon text tspan textPath use symbol image clipPath mask pattern marker linearGradient radialGradient stop filter feBlend feColorMatrix feComponentTransfer feComposite feConvolveMatrix feDiffuseLighting feDisplacementMap feDistantLight feDropShadow feFlood feFuncA feFuncB feFuncG feFuncR feGaussianBlur feImage feMerge feMergeNode feMorphology feOffset fePointLight feSpecularLighting feSpotLight feTile feTurbulence style" + .toLowerCase() + .split(" "), +); + +function localImageReference(value: string): boolean { + const ref = value.trim(); + return ( + ref.startsWith("#") || /^data:image\/(?:png|jpeg|gif|webp);base64,[a-z0-9+/=\s]+$/i.test(ref) + ); +} + +function passiveCss(source: string, context: "stylesheet" | "declarationList" | "value"): boolean { + // Escapes can disguise fetch-bearing tokens; keep the accepted spelling unambiguous. + if (source.includes("\\")) return false; + let safe = true; + try { + walk(parse(source, { context, parseCustomProperty: true }), (node) => { + if (node.type === "Raw") safe = false; + if ( + node.type === "Function" && + ["image", "image-set", "-webkit-image-set", "src", "paint"].includes( + node.name.toLowerCase(), + ) + ) + safe = false; + if (node.type === "Url" && !localImageReference(node.value)) safe = false; + if ( + node.type === "Atrule" && + !["media", "supports", "keyframes"].includes(node.name.toLowerCase()) + ) + safe = false; + }); + return safe; + } catch { + return false; + } +} + +/** Accept passive SVG without reserializing it or losing theme-dependent paint. */ +function passiveSvg(bytes: Buffer): boolean { + const source = bytes.toString("utf8"); + if (/ 10000) return false; + return elements.every((element) => { + if (!SVG_ELEMENTS.has(element.localName.toLowerCase())) return false; + if (element.localName === "style" && !passiveCss(element.textContent ?? "", "stylesheet")) + return false; + return element + .getAttributeNames() + .every((attribute: string) => + passiveSvgAttribute(attribute.toLowerCase(), element.getAttribute(attribute) ?? ""), + ); + }); +} + +function passiveSvgAttribute(name: string, value: string): boolean { + if (name.startsWith("on") || name === "xml:base") return false; + if (["href", "xlink:href", "src"].includes(name)) return localImageReference(value); + if (name === "style") return passiveCss(value, "declarationList"); + return !/url\s*\(|\\/i.test(value) || passiveCss(value, "value"); +} + +async function validIco(bytes: Buffer): Promise { + if (bytes.length < 22 || bytes.readUInt32LE(0) !== 0x00010000) return false; + const count = bytes.readUInt16LE(4); + if (count === 0 || count > 256 || 6 + count * 16 > bytes.length) return false; + for (let i = 0; i < count; i++) { + const size = bytes.readUInt32LE(6 + i * 16 + 8); + const offset = bytes.readUInt32LE(6 + i * 16 + 12); + if (!validIconEntry(size, offset, 6 + count * 16, bytes.length)) return false; + if (!(await validIconImage(bytes.subarray(offset, offset + size)))) return false; + } + return true; +} + +function validIconEntry( + size: number, + offset: number, + directoryEnd: number, + length: number, +): boolean { + return size >= 40 && offset >= directoryEnd && size <= length - offset; +} + +async function validIconImage(bytes: Buffer): Promise { + const png = bytes.subarray(0, 8).equals(Buffer.from([137, 80, 78, 71, 13, 10, 26, 10])); + if (!png) return validIconBitmap(bytes); + try { + await sharp(bytes, { limitInputPixels: 65536 }).resize(1, 1).raw().toBuffer(); + return true; + } catch { + return false; + } +} + +function validIconBitmap(bytes: Buffer): boolean { + const dibSize = bytes.readUInt32LE(0); + if (![40, 108, 124].includes(dibSize) || bytes.length < dibSize) return false; + const width = bytes.readInt32LE(4); + const height = bytes.readInt32LE(8); + const bits = bytes.readUInt16LE(14); + if (!validIconDimensions(width, height)) return false; + if (bytes.readUInt16LE(12) !== 1 || ![1, 4, 8, 16, 24, 32].includes(bits)) return false; + if (bytes.readUInt32LE(16) !== 0) return false; + const pixels = Math.ceil((width * bits) / 32) * 4 * (height / 2); + const palette = bits <= 8 ? 4 * (1 << bits) : 0; + return dibSize + palette + pixels <= bytes.length; +} + +function validIconDimensions(width: number, height: number): boolean { + return width > 0 && width <= 256 && height > 0 && height <= 512 && height % 2 === 0; +} + +/** Choose a canonical image extension from the content, never a remote URL suffix. */ +export async function captureImageExtension(bytes: Buffer): Promise { + if (await validIco(bytes)) return ".ico"; + try { + // Reject active XML before passing it to an image decoder. + if (/^\s* = { + jpeg: ".jpg", + png: ".png", + webp: ".webp", + gif: ".gif", + avif: ".avif", + heif: metadata.compression === "av1" ? ".avif" : ".heic", + tiff: ".tiff", + }; + const extension = metadata.format && extensions[metadata.format]; + if (!extension) return null; + await image.resize(1, 1).raw().toBuffer(); + return extension; + } catch { + return null; + } +} diff --git a/packages/cli/src/capture/readBoundedResponse.test.ts b/packages/cli/src/capture/readBoundedResponse.test.ts new file mode 100644 index 0000000000..218e1e025e --- /dev/null +++ b/packages/cli/src/capture/readBoundedResponse.test.ts @@ -0,0 +1,54 @@ +import { describe, expect, it, vi } from "vitest"; +import { readBoundedResponse } from "./readBoundedResponse.js"; + +describe("bounded capture responses", () => { + it("preserves exact bytes at the limit across chunks", async () => { + const body = new ReadableStream({ + start(controller) { + controller.enqueue(new Uint8Array([0, 255])); + controller.enqueue(new Uint8Array([10, 13])); + controller.close(); + }, + }); + expect(await readBoundedResponse(new Response(body), 4)).toEqual(Buffer.from([0, 255, 10, 13])); + }); + + it.each([undefined, "1"])( + "cancels an oversized stream with declared length %s", + async (length) => { + const cancel = vi.fn(); + const body = new ReadableStream({ + pull(controller) { + controller.enqueue(new Uint8Array(3)); + }, + cancel, + }); + const headers = new Headers(); + if (length !== undefined) headers.set("content-length", length); + expect(await readBoundedResponse(new Response(body, { headers }), 4)).toBeNull(); + expect(cancel).toHaveBeenCalledOnce(); + expect(body.locked).toBe(false); + }, + ); + + it("cancels a declared oversized response before reading", async () => { + const cancel = vi.fn(); + const pull = vi.fn(); + const body = new ReadableStream({ pull, cancel }, { highWaterMark: 0 }); + expect( + await readBoundedResponse(new Response(body, { headers: { "content-length": "5" } }), 4), + ).toBeNull(); + expect(pull).not.toHaveBeenCalled(); + expect(cancel).toHaveBeenCalledOnce(); + }); + + it("propagates a broken response and releases the reader", async () => { + const body = new ReadableStream({ + start(controller) { + controller.error(new Error("connection lost")); + }, + }); + await expect(readBoundedResponse(new Response(body), 4)).rejects.toThrow("connection lost"); + expect(body.locked).toBe(false); + }); +}); diff --git a/packages/cli/src/capture/readBoundedResponse.ts b/packages/cli/src/capture/readBoundedResponse.ts new file mode 100644 index 0000000000..b06aa7ee2d --- /dev/null +++ b/packages/cli/src/capture/readBoundedResponse.ts @@ -0,0 +1,28 @@ +/** Read a download without trusting Content-Length or retaining an oversized response. */ +export async function readBoundedResponse( + response: Response, + maxBytes: number, +): Promise { + if (!response.body) return null; + const reader = response.body.getReader(); + let complete = false; + try { + const declared = Number(response.headers.get("content-length")); + if (declared > maxBytes) return null; + const chunks: Uint8Array[] = []; + let total = 0; + while (true) { + const { done, value } = await reader.read(); + if (done) { + complete = true; + return Buffer.concat(chunks, total); + } + total += value.byteLength; + if (total > maxBytes) return null; + chunks.push(value); + } + } finally { + if (!complete) await reader.cancel().catch(() => {}); + reader.releaseLock(); + } +} From 889c2b6d31b1ecd7bc21fbf7c92946bdcecb26a6 Mon Sep 17 00:00:00 2001 From: James Date: Wed, 9 Sep 2026 03:43:33 -0700 Subject: [PATCH 2/2] fix(cli): bound nested assets and preserve distinct fonts --- .../cli/src/capture/assetDownloader.test.ts | 48 +++++++++++++ packages/cli/src/capture/assetDownloader.ts | 48 ++++++++++--- .../src/capture/captureFontValidation.test.ts | 12 ++++ .../cli/src/capture/captureFontValidation.ts | 15 +++- .../capture/captureImageValidation.test.ts | 34 ++++++++++ .../cli/src/capture/captureImageValidation.ts | 68 +++++++++++++++---- packages/cli/src/capture/index.ts | 4 ++ .../src/capture/readBoundedResponse.test.ts | 9 +++ .../cli/src/capture/readBoundedResponse.ts | 17 ++++- 9 files changed, 229 insertions(+), 26 deletions(-) diff --git a/packages/cli/src/capture/assetDownloader.test.ts b/packages/cli/src/capture/assetDownloader.test.ts index ae6ae28a73..ffa9243a13 100644 --- a/packages/cli/src/capture/assetDownloader.test.ts +++ b/packages/cli/src/capture/assetDownloader.test.ts @@ -579,6 +579,54 @@ describe("declared icons — keep them all, headline the bare mark", () => { }); describe("capture download security boundaries", () => { + it("preserves two different fonts whose URL extensions canonicalize to the same name", async () => { + await withTempDir(async (dir) => { + const first = readFileSync( + new URL("../../../../docs/public/catalog/assets/a634cb9e7783af7e.woff2", import.meta.url), + ); + const second = readFileSync( + new URL("../../../../docs/public/catalog/assets/8963f64fa28dc4ae.woff2", import.meta.url), + ); + expect(first.equals(second)).toBe(false); + vi.stubGlobal( + "fetch", + vi.fn( + async (url: string) => + new Response(new Uint8Array(url.endsWith(".ttf") ? first : second)), + ), + ); + const css = + "@font-face{font-family:A;src:url(https://fonts.example/site.ttf)} @font-face{font-family:B;src:url(https://fonts.example/site.woff2)}"; + const result = await downloadAndRewriteFonts(css, dir); + expect(result.css).toContain("assets/fonts/site.woff2"); + expect(result.css).toContain("assets/fonts/site-2.woff2"); + expect(readFileSync(join(dir, "assets/fonts/site.woff2"))).toEqual(first); + expect(readFileSync(join(dir, "assets/fonts/site-2.woff2"))).toEqual(second); + }); + }); + + it("shares the capture byte budget between fonts and icons", async () => { + await withTempDir(async (dir) => { + const bytes = readFileSync( + new URL("../../../../docs/public/catalog/assets/a634cb9e7783af7e.woff2", import.meta.url), + ); + const fetchMock = vi.fn(async () => new Response(new Uint8Array(bytes))); + vi.stubGlobal("fetch", fetchMock); + const byteBudget = { remainingBytes: bytes.length }; + await downloadAndRewriteFonts( + "@font-face{font-family:A;src:url(https://fonts.example/site.woff2)}", + dir, + { byteBudget }, + ); + const result = await downloadAssets(tokensWithNoSvgs(), dir, [], OPENAI_ICONS, { + byteBudget, + }); + expect(result.icons.icons).toEqual([]); + expect(fetchMock).toHaveBeenCalledTimes(1); + expect(byteBudget.remainingBytes).toBe(0); + }); + }); + afterEach(() => vi.unstubAllGlobals()); it("canonicalizes an icon ADS suffix and its promoted copy while preserving SVG bytes", async () => { diff --git a/packages/cli/src/capture/assetDownloader.ts b/packages/cli/src/capture/assetDownloader.ts index e4905990c4..a9730653a9 100644 --- a/packages/cli/src/capture/assetDownloader.ts +++ b/packages/cli/src/capture/assetDownloader.ts @@ -13,11 +13,16 @@ import type { CatalogedAsset } from "./assetCataloger.js"; import { CAPTURE_USER_AGENT } from "./userAgent.js"; import { rankIconCandidates, type IconCandidate } from "./faviconRanker.js"; import { classifyIcon, type IconShape } from "./iconClassifier.js"; -import { readBoundedResponse } from "./readBoundedResponse.js"; +import { + readBoundedResponse, + createCaptureDownloadBudget, + type DownloadByteBudget, +} from "./readBoundedResponse.js"; import { captureFontExtension, captureFontFilename } from "./captureFontValidation.js"; import { captureImageExtension } from "./captureImageValidation.js"; interface DownloadBudgetOptions { + byteBudget?: DownloadByteBudget; remainingMs?: () => number; } @@ -192,8 +197,9 @@ async function fetchAndInspectIcon( stem: string, outputDir: string, timeoutMs: number, + byteBudget?: DownloadByteBudget, ): Promise<{ record: IconRecord; buffer: Buffer } | null> { - const buffer = await fetchBuffer(icon.href, timeoutMs); + const buffer = await fetchBuffer(icon.href, timeoutMs, 2 * 1024 * 1024, byteBudget); if (!buffer) return null; const ext = await captureImageExtension(buffer); if (!ext) return null; @@ -275,6 +281,7 @@ async function downloadDeclaredIcons( stem, outputDir, Math.min(10_000, remainingMs), + options.byteBudget, ); if (!got) { drops.unavailable++; @@ -306,6 +313,7 @@ export async function downloadAssets( faviconLinks?: IconCandidate[], options: DownloadBudgetOptions = {}, ): Promise<{ assets: DownloadedAsset[]; drops: AssetDropCounts; icons: IconManifest }> { + options = { ...options, byteBudget: options.byteBudget ?? createCaptureDownloadBudget() }; const assetsDir = join(outputDir, "assets"); mkdirSync(assetsDir, { recursive: true }); @@ -409,7 +417,12 @@ export async function downloadAssets( const results = await Promise.allSettled( batch.map(async ({ url, isPoster, catalog }) => { const parsedUrl = new URL(url); - const buffer = await fetchBuffer(url, Math.min(10_000, remainingMs)); + const buffer = await fetchBuffer( + url, + Math.min(10_000, remainingMs), + 20 * 1024 * 1024, + options.byteBudget, + ); if (!buffer) { drops.unavailable++; return null; @@ -474,7 +487,12 @@ export async function downloadAssets( if (remainingMs <= 0) { drops["budget-exhausted"]++; } else { - const buffer = await fetchBuffer(tokens.ogImage, Math.min(10_000, remainingMs)); + const buffer = await fetchBuffer( + tokens.ogImage, + Math.min(10_000, remainingMs), + 20 * 1024 * 1024, + options.byteBudget, + ); const ext = buffer && (await captureImageExtension(buffer)); if (!buffer || !ext) { drops.unavailable++; @@ -520,6 +538,7 @@ export async function downloadAndRewriteFonts( outputDir: string, options: DownloadBudgetOptions = {}, ): Promise<{ css: string; drops: AssetDropCounts }> { + options = { ...options, byteBudget: options.byteBudget ?? createCaptureDownloadBudget() }; const assetsDir = join(outputDir, "assets", "fonts"); mkdirSync(assetsDir, { recursive: true }); const drops = noDrops(); @@ -559,6 +578,7 @@ export async function downloadAndRewriteFonts( return aLatin - bLatin; }); + const usedFontNames = new Set(); let rewritten = css; let count = 0; @@ -582,10 +602,15 @@ export async function downloadAndRewriteFonts( count++; try { - const buffer = await fetchBuffer(fontUrl, Math.min(10_000, remainingMs)); + const buffer = await fetchBuffer( + fontUrl, + Math.min(10_000, remainingMs), + 10 * 1024 * 1024, + options.byteBudget, + ); const extension = buffer && captureFontExtension(buffer); if (buffer && extension) { - const filename = captureFontFilename(fontUrl, extension); + const filename = captureFontFilename(fontUrl, extension, usedFontNames); const localPath = join(assetsDir, filename); const relativePath = `assets/fonts/${filename}`; writeFileSync(localPath, buffer); @@ -691,7 +716,13 @@ export async function safeFetch(url: string, init?: RequestInit): Promise { +async function fetchBuffer( + url: string, + timeoutMs: number, + maxBytes: number, + budget: DownloadByteBudget = createCaptureDownloadBudget(), +): Promise { + if (budget.remainingBytes <= 0) return null; try { const res = await safeFetch(url, { signal: AbortSignal.timeout(timeoutMs), @@ -703,8 +734,7 @@ async function fetchBuffer(url: string, timeoutMs = 10_000): Promise { + it("keeps canonicalized font names distinct on case-insensitive filesystems", () => { + const used = new Set(); + expect(captureFontFilename("https://fonts.example/site.ttf", ".woff2", used)).toBe( + "site.woff2", + ); + expect(captureFontFilename("https://fonts.example/site.woff2", ".woff2", used)).toBe( + "site-2.woff2", + ); + expect(captureFontFilename("https://fonts.example/SITE.woff2", ".woff2", used)).toBe( + "SITE-3.woff2", + ); + }); it("accepts the existing catalog WOFF2 font without changing its bytes", () => { expect(captureFontExtension(font)).toBe(".woff2"); expect(captureFontFilename("https://fonts.example/font-0.woff2", ".woff2")).toBe( diff --git a/packages/cli/src/capture/captureFontValidation.ts b/packages/cli/src/capture/captureFontValidation.ts index 06728a17e3..87c66d7c10 100644 --- a/packages/cli/src/capture/captureFontValidation.ts +++ b/packages/cli/src/capture/captureFontValidation.ts @@ -44,7 +44,7 @@ export function captureFontExtension(bytes: Buffer): string | null { } /** Preserve ordinary names; remote URL spelling must never select a Windows device or stream. */ -export function captureFontFilename(url: string, extension: string): string { +function preferredFontFilename(url: string, extension: string): string { const basename = new URL(url).pathname.split("/").pop() ?? ""; const dot = basename.lastIndexOf("."); const stem = dot > 0 ? basename.slice(0, dot) : basename; @@ -57,3 +57,16 @@ export function captureFontFilename(url: string, extension: string): string { } return `font-${createHash("sha256").update(url).digest("hex").slice(0, 16)}${extension}`; } + +export function captureFontFilename( + url: string, + extension: string, + used = new Set(), +): string { + const preferred = preferredFontFilename(url, extension); + const stem = preferred.slice(0, -extension.length); + let name = preferred; + for (let n = 2; used.has(name.toLowerCase()); n++) name = `${stem}-${n}${extension}`; + used.add(name.toLowerCase()); + return name; +} diff --git a/packages/cli/src/capture/captureImageValidation.test.ts b/packages/cli/src/capture/captureImageValidation.test.ts index a297f47c38..a17d9989f5 100644 --- a/packages/cli/src/capture/captureImageValidation.test.ts +++ b/packages/cli/src/capture/captureImageValidation.test.ts @@ -9,6 +9,40 @@ function svg(content: string): Buffer { } describe("captured image validation", () => { + it("validates embedded raster bytes and rejects false MIME claims", async () => { + const png = await sharp({ create: { width: 2, height: 2, channels: 4, background: "red" } }) + .png() + .toBuffer(); + expect( + await captureImageExtension( + svg(``), + ), + ).toBe(".svg"); + expect( + await captureImageExtension(svg('')), + ).toBeNull(); + expect( + await captureImageExtension( + svg(``), + ), + ).toBeNull(); + }); + + it("limits the aggregate decoded pixel count of embedded rasters", async () => { + const png = await sharp({ + create: { width: 5000, height: 5000, channels: 3, background: "red" }, + }) + .png() + .toBuffer(); + const encoded = png.toString("base64"); + expect( + await captureImageExtension( + svg( + ``, + ), + ), + ).toBeNull(); + }); it("recognizes actual PNG bytes without needing a trusted URL or content type", async () => { const bytes = await sharp({ create: { width: 2, height: 2, channels: 4, background: "red" } }) .png() diff --git a/packages/cli/src/capture/captureImageValidation.ts b/packages/cli/src/capture/captureImageValidation.ts index 70345724ce..9234a623fe 100644 --- a/packages/cli/src/capture/captureImageValidation.ts +++ b/packages/cli/src/capture/captureImageValidation.ts @@ -8,14 +8,19 @@ const SVG_ELEMENTS = new Set( .split(" "), ); -function localImageReference(value: string): boolean { +function localImageReference(value: string, embedded: Set): boolean { const ref = value.trim(); - return ( - ref.startsWith("#") || /^data:image\/(?:png|jpeg|gif|webp);base64,[a-z0-9+/=\s]+$/i.test(ref) - ); + if (ref.startsWith("#")) return true; + if (!/^data:image\/(?:png|jpeg|gif|webp);base64,[a-z0-9+/=\s]+$/i.test(ref)) return false; + embedded.add(ref); + return true; } -function passiveCss(source: string, context: "stylesheet" | "declarationList" | "value"): boolean { +function passiveCss( + source: string, + context: "stylesheet" | "declarationList" | "value", + embedded: Set, +): boolean { // Escapes can disguise fetch-bearing tokens; keep the accepted spelling unambiguous. if (source.includes("\\")) return false; let safe = true; @@ -29,7 +34,7 @@ function passiveCss(source: string, context: "stylesheet" | "declarationList" | ) ) safe = false; - if (node.type === "Url" && !localImageReference(node.value)) safe = false; + if (node.type === "Url" && !localImageReference(node.value, embedded)) safe = false; if ( node.type === "Atrule" && !["media", "supports", "keyframes"].includes(node.name.toLowerCase()) @@ -43,7 +48,7 @@ function passiveCss(source: string, context: "stylesheet" | "declarationList" | } /** Accept passive SVG without reserializing it or losing theme-dependent paint. */ -function passiveSvg(bytes: Buffer): boolean { +function passiveSvg(bytes: Buffer, embedded: Set): boolean { const source = bytes.toString("utf8"); if (/ 10000) return false; return elements.every((element) => { if (!SVG_ELEMENTS.has(element.localName.toLowerCase())) return false; - if (element.localName === "style" && !passiveCss(element.textContent ?? "", "stylesheet")) + if ( + element.localName === "style" && + !passiveCss(element.textContent ?? "", "stylesheet", embedded) + ) return false; return element .getAttributeNames() .every((attribute: string) => - passiveSvgAttribute(attribute.toLowerCase(), element.getAttribute(attribute) ?? ""), + passiveSvgAttribute( + attribute.toLowerCase(), + element.getAttribute(attribute) ?? "", + embedded, + ), ); }); } -function passiveSvgAttribute(name: string, value: string): boolean { +function passiveSvgAttribute(name: string, value: string, embedded: Set): boolean { if (name.startsWith("on") || name === "xml:base") return false; - if (["href", "xlink:href", "src"].includes(name)) return localImageReference(value); - if (name === "style") return passiveCss(value, "declarationList"); - return !/url\s*\(|\\/i.test(value) || passiveCss(value, "value"); + if (["href", "xlink:href", "src"].includes(name)) return localImageReference(value, embedded); + if (name === "style") return passiveCss(value, "declarationList", embedded); + return !/url\s*\(|\\/i.test(value) || passiveCss(value, "value", embedded); } async function validIco(bytes: Buffer): Promise { @@ -125,11 +137,15 @@ function validIconDimensions(width: number, height: number): boolean { export async function captureImageExtension(bytes: Buffer): Promise { if (await validIco(bytes)) return ".ico"; try { + const embedded = new Set(); // Reject active XML before passing it to an image decoder. - if (/^\s* = { jpeg: ".jpg", png: ".png", @@ -147,3 +163,25 @@ export async function captureImageExtension(bytes: Buffer): Promise): Promise { + let remainingBytes = 10 * 1024 * 1024; + let remainingPixels = 40_000_000; + for (const reference of references) { + const comma = reference.indexOf(","); + const type = reference.slice(11, reference.indexOf(";")).toLowerCase(); + const encoded = reference.slice(comma + 1).replace(/\s/g, ""); + if (encoded.length > Math.ceil(remainingBytes / 3) * 4) return false; + const bytes = Buffer.from(encoded, "base64"); + remainingBytes -= bytes.length; + if (remainingBytes < 0) return false; + const image = sharp(bytes, { limitInputPixels: remainingPixels }); + const metadata = await image.metadata(); + if (metadata.format !== type || !metadata.width || !metadata.height) return false; + const pixels = metadata.width * metadata.height * (metadata.pages ?? 1); + remainingPixels -= pixels; + if (remainingPixels < 0) return false; + await image.resize(1, 1).raw().toBuffer(); + } + return true; +} diff --git a/packages/cli/src/capture/index.ts b/packages/cli/src/capture/index.ts index 848d8776a5..b29285de37 100644 --- a/packages/cli/src/capture/index.ts +++ b/packages/cli/src/capture/index.ts @@ -1,3 +1,4 @@ +import { createCaptureDownloadBudget } from "./readBoundedResponse.js"; /** * Website capture orchestrator. * @@ -608,8 +609,10 @@ export async function captureWebsite( // `budget-exhausted` for every one of them replaces a warning string that could only ever // say "some". A zero budget means it breaks on the first url, so this costs no network. phase("fonts", "started"); + const downloadByteBudget = createCaptureDownloadBudget(); const fontPass = await downloadAndRewriteFonts(extracted.headHtml, outputDir, { remainingMs, + byteBudget: downloadByteBudget, }); extracted.headHtml = fontPass.css; phase( @@ -681,6 +684,7 @@ export async function captureWebsite( progress("assets", "Downloading assets..."); const assetPass = await downloadAssets(tokens, outputDir, catalogedAssets, faviconLinks, { remainingMs, + byteBudget: downloadByteBudget, }); assets = assetPass.assets; assetDrops = assetPass.drops; diff --git a/packages/cli/src/capture/readBoundedResponse.test.ts b/packages/cli/src/capture/readBoundedResponse.test.ts index 218e1e025e..71cf592218 100644 --- a/packages/cli/src/capture/readBoundedResponse.test.ts +++ b/packages/cli/src/capture/readBoundedResponse.test.ts @@ -2,6 +2,15 @@ import { describe, expect, it, vi } from "vitest"; import { readBoundedResponse } from "./readBoundedResponse.js"; describe("bounded capture responses", () => { + it("shares a byte budget across concurrent responses", async () => { + const budget = { remainingBytes: 5 }; + const result = await Promise.all([ + readBoundedResponse(new Response(new Uint8Array(3)), 4, budget), + readBoundedResponse(new Response(new Uint8Array(3)), 4, budget), + ]); + expect(result.filter((value) => value !== null)).toHaveLength(1); + expect(budget.remainingBytes).toBe(0); + }); it("preserves exact bytes at the limit across chunks", async () => { const body = new ReadableStream({ start(controller) { diff --git a/packages/cli/src/capture/readBoundedResponse.ts b/packages/cli/src/capture/readBoundedResponse.ts index b06aa7ee2d..b88d162fd8 100644 --- a/packages/cli/src/capture/readBoundedResponse.ts +++ b/packages/cli/src/capture/readBoundedResponse.ts @@ -1,14 +1,24 @@ +export interface DownloadByteBudget { + remainingBytes: number; +} + +/** Shared by the font and image passes of one website capture. */ +export function createCaptureDownloadBudget(): DownloadByteBudget { + return { remainingBytes: 100 * 1024 * 1024 }; +} + /** Read a download without trusting Content-Length or retaining an oversized response. */ export async function readBoundedResponse( response: Response, maxBytes: number, + budget?: DownloadByteBudget, ): Promise { if (!response.body) return null; const reader = response.body.getReader(); let complete = false; try { const declared = Number(response.headers.get("content-length")); - if (declared > maxBytes) return null; + if (declared > Math.min(maxBytes, budget?.remainingBytes ?? maxBytes)) return null; const chunks: Uint8Array[] = []; let total = 0; while (true) { @@ -17,6 +27,11 @@ export async function readBoundedResponse( complete = true; return Buffer.concat(chunks, total); } + if (budget) { + const available = budget.remainingBytes; + budget.remainingBytes = Math.max(0, available - value.byteLength); + if (value.byteLength > available) return null; + } total += value.byteLength; if (total > maxBytes) return null; chunks.push(value);