diff --git a/src/image-evidence.ts b/src/image-evidence.ts index 21a067e..42686c5 100644 --- a/src/image-evidence.ts +++ b/src/image-evidence.ts @@ -1,30 +1,44 @@ +import { PNG } from "pngjs"; + +import { + SCREENSHOT_MAX_SOURCE_PIXELS, + hasPngSignature, + readPngDeclaredDimensions +} from "./screenshot-image.js"; + +// A noisy 4K RGBA frame is roughly 32 MiB before PNG compression, so this +// admits realistic screenshot payloads while bounding decoder input. +const SCREENSHOT_MAX_BYTES = 32 * 1024 * 1024; + export function screenshotEvidenceError(relativePath: string, bytes: Buffer): string | null { const extension = relativePath.toLowerCase().split(".").pop() ?? ""; - if (extension === "png") { - return hasPrefix(bytes, [0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a]) - ? null - : "expected PNG signature"; + if (extension !== "png") { + return `unsupported screenshot extension .${extension || "unknown"}; only decoded PNG evidence is supported`; } - if (extension === "jpg" || extension === "jpeg") { - return hasPrefix(bytes, [0xff, 0xd8, 0xff]) ? null : "expected JPEG signature"; + if (!hasPngSignature(bytes)) { + return "expected PNG signature"; } - if (extension === "webp") { - return bytes.length >= 12 - && bytes.subarray(0, 4).toString("ascii") === "RIFF" - && bytes.subarray(8, 12).toString("ascii") === "WEBP" - ? null - : "expected WEBP signature"; + if (bytes.length > SCREENSHOT_MAX_BYTES) { + return `PNG byte size exceeds ${SCREENSHOT_MAX_BYTES} byte limit`; } - if (extension === "gif") { - const signature = bytes.subarray(0, 6).toString("ascii"); - return signature === "GIF87a" || signature === "GIF89a" ? null : "expected GIF signature"; + const declaredDimensions = readPngDeclaredDimensions(bytes); + if (declaredDimensions) { + const dimensionsError = pngDimensionsError(declaredDimensions.width, declaredDimensions.height); + if (dimensionsError) { + return dimensionsError; + } } - return `unsupported screenshot extension .${extension || "unknown"}`; + try { + const decoded = PNG.sync.read(bytes, { checkCRC: true }); + return pngDimensionsError(decoded.width, decoded.height); + } catch { + return "could not decode PNG evidence"; + } } export function assertScreenshotEvidence(relativePath: string, bytes: Buffer): void { @@ -34,6 +48,12 @@ export function assertScreenshotEvidence(relativePath: string, bytes: Buffer): v } } -function hasPrefix(bytes: Buffer, prefix: number[]): boolean { - return bytes.length >= prefix.length && prefix.every((value, index) => bytes[index] === value); +function pngDimensionsError(width: number, height: number): string | null { + if (width === 0 || height === 0) { + return "PNG dimensions must be greater than zero"; + } + if (width * height > SCREENSHOT_MAX_SOURCE_PIXELS) { + return `PNG pixel count exceeds ${SCREENSHOT_MAX_SOURCE_PIXELS} pixel limit`; + } + return null; } diff --git a/src/redaction.ts b/src/redaction.ts index 7ef8495..6fc086e 100644 --- a/src/redaction.ts +++ b/src/redaction.ts @@ -3,6 +3,8 @@ import { realpathSync } from "node:fs"; import path from "node:path"; import { PNG } from "pngjs"; +import { SCREENSHOT_MAX_SOURCE_PIXELS, readPngDeclaredDimensions } from "./screenshot-image.js"; + // Single source of truth for public-safety redaction patterns. Both the Codex // actor trace (src/codex-app-server.ts) and the run-bundle scanner/redactor // (src/run.ts) use these so the denylist cannot drift between producers and the @@ -136,9 +138,7 @@ const SCREENSHOT_MAX_WIDTH_DEFAULT = 96; const SCREENSHOT_MAX_WIDTH_CAP = 128; // Reject absurd source dimensions before decode so a crafted IHDR cannot OOM the // process before the try/catch can fall back to a placeholder. -const SCREENSHOT_MAX_SOURCE_PIXELS = 50_000_000; const SCREENSHOT_PLACEHOLDER_GRAY = 128; -const PNG_SIGNATURE_BE = 0x89_50_4e_47; /** * A redacted screenshot safe to persist to a public run bundle. `buffer` is @@ -217,13 +217,9 @@ function effectiveBlurRadius(outW: number): number { // the pixel count exceeds the cap. A too-short or non-PNG buffer returns false // and falls through to PNG.sync.read, which throws and lands on the placeholder. function sourcePixelsExceedCap(buf: Buffer): boolean { - if (buf.length < 24 || buf.readUInt32BE(0) !== PNG_SIGNATURE_BE) { - return false; - } - // IHDR is the first chunk: width at byte 16, height at byte 20 (big-endian). - const width = buf.readUInt32BE(16); - const height = buf.readUInt32BE(20); - return width * height > SCREENSHOT_MAX_SOURCE_PIXELS; + const dimensions = readPngDeclaredDimensions(buf); + return dimensions !== null + && dimensions.width * dimensions.height > SCREENSHOT_MAX_SOURCE_PIXELS; } function placeholderScreenshot(maxWidth: number): RedactedScreenshot { diff --git a/src/screenshot-image.ts b/src/screenshot-image.ts new file mode 100644 index 0000000..61cbf74 --- /dev/null +++ b/src/screenshot-image.ts @@ -0,0 +1,36 @@ +const PNG_SIGNATURE = Buffer.from([0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a]); +const PNG_IHDR_LENGTH = 13; + +// Shared by validation and redaction so their pre-decode allocation guard +// cannot drift. +export const SCREENSHOT_MAX_SOURCE_PIXELS = 50_000_000; + +export interface PngDimensions { + width: number; + height: number; +} + +export function hasPngSignature(bytes: Buffer): boolean { + return bytes.length >= PNG_SIGNATURE.length + && bytes.subarray(0, PNG_SIGNATURE.length).equals(PNG_SIGNATURE); +} + +/** + * Read dimensions from a structurally positioned PNG IHDR without decoding. + * CRC and complete-file validity remain the decoder's responsibility. + */ +export function readPngDeclaredDimensions(bytes: Buffer): PngDimensions | null { + if ( + !hasPngSignature(bytes) + || bytes.length < 24 + || bytes.readUInt32BE(8) !== PNG_IHDR_LENGTH + || bytes.subarray(12, 16).toString("ascii") !== "IHDR" + ) { + return null; + } + + return { + width: bytes.readUInt32BE(16), + height: bytes.readUInt32BE(20) + }; +} diff --git a/tests/actor-conformance.test.ts b/tests/actor-conformance.test.ts index b3928bd..bc975a5 100644 --- a/tests/actor-conformance.test.ts +++ b/tests/actor-conformance.test.ts @@ -16,11 +16,9 @@ import { type ScriptedPageLike } from "../src/scripted-browser-actor.js"; import { buildClaudeSession, buildCodexResult, buildPiSession, fixturePersona } from "./actor-fixtures.js"; +import { syntheticPng1x1 } from "./image-fixtures.js"; -const PNG_1X1 = Buffer.from( - "iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAIAAACQd1PeAAAADUlEQVR42mP8z8BQDwAFgwJ/lp9J1wAAAABJRU5ErkJggg==", - "base64" -); +const PNG_1X1 = syntheticPng1x1(); // The shared contract every adapter's ActorTrace must satisfy. This is what makes // the harnesses interchangeable (ADR step 7): one persona run through codex and pi diff --git a/tests/image-evidence.test.ts b/tests/image-evidence.test.ts new file mode 100644 index 0000000..d511b69 --- /dev/null +++ b/tests/image-evidence.test.ts @@ -0,0 +1,71 @@ +import { describe, expect, it } from "vitest"; +import { PNG } from "pngjs"; + +import { assertScreenshotEvidence, screenshotEvidenceError } from "../src/image-evidence.js"; + +function encodePng(width = 2, height = 2): Buffer { + const png = new PNG({ width, height }); + png.data.fill(255); + return PNG.sync.write(png); +} + +describe("screenshot evidence", () => { + it("accepts a valid small PNG", () => { + const bytes = encodePng(); + + expect(screenshotEvidenceError("screenshots/tiny.png", bytes)).toBeNull(); + expect(() => assertScreenshotEvidence("screenshots/tiny.png", bytes)).not.toThrow(); + }); + + it("rejects text saved with a PNG extension", () => { + expect(screenshotEvidenceError("screenshots/not-an-image.png", Buffer.from("not an image"))) + .toBe("expected PNG signature"); + }); + + it("rejects a truncated PNG that still has a valid signature", () => { + const truncated = encodePng().subarray(0, 24); + + expect(screenshotEvidenceError("screenshots/truncated.png", truncated)) + .toBe("could not decode PNG evidence"); + }); + + it("rejects PNG dimensions that exceed the pixel limit before decoding", () => { + const oversized = Buffer.from(encodePng()); + oversized.writeUInt32BE(100_000, 16); + oversized.writeUInt32BE(100_000, 20); + + expect(screenshotEvidenceError("screenshots/oversized.png", oversized)) + .toBe("PNG pixel count exceeds 50000000 pixel limit"); + }); + + it("rejects PNG payloads that exceed the byte limit before decoding", () => { + const oversized = Buffer.alloc(32 * 1024 * 1024 + 1); + encodePng().copy(oversized); + + expect(screenshotEvidenceError("screenshots/oversized.png", oversized)) + .toBe("PNG byte size exceeds 33554432 byte limit"); + }); + + it("rejects zero declared dimensions", () => { + const emptyWidth = Buffer.from(encodePng()); + emptyWidth.writeUInt32BE(0, 16); + + expect(screenshotEvidenceError("screenshots/zero-width.png", emptyWidth)) + .toBe("PNG dimensions must be greater than zero"); + }); + + it.each([ + ["jpg", Buffer.from([0xff, 0xd8, 0xff])], + ["jpeg", Buffer.from([0xff, 0xd8, 0xff])], + ["webp", Buffer.from("RIFF0000WEBP", "ascii")], + ["gif", Buffer.from("GIF89a", "ascii")] + ])("rejects signature-only .%s evidence", (extension, bytes) => { + expect(screenshotEvidenceError(`screenshots/image.${extension}`, bytes)) + .toBe(`unsupported screenshot extension .${extension}; only decoded PNG evidence is supported`); + }); + + it("rejects an unknown screenshot extension with a clear message", () => { + expect(screenshotEvidenceError("screenshots/tiny.bmp", encodePng())) + .toBe("unsupported screenshot extension .bmp; only decoded PNG evidence is supported"); + }); +}); diff --git a/tests/image-fixtures.ts b/tests/image-fixtures.ts new file mode 100644 index 0000000..11934b7 --- /dev/null +++ b/tests/image-fixtures.ts @@ -0,0 +1,6 @@ +export const SYNTHETIC_PNG_1X1_BASE64 = + "iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAAC0lEQVR4AWP4DwQACfsD/c8LaHIAAAAASUVORK5CYII="; + +export function syntheticPng1x1(): Buffer { + return Buffer.from(SYNTHETIC_PNG_1X1_BASE64, "base64"); +} diff --git a/tests/observer.test.ts b/tests/observer.test.ts index cbfa317..858a8f0 100644 --- a/tests/observer.test.ts +++ b/tests/observer.test.ts @@ -10,11 +10,9 @@ import { createProgram } from "../src/program.js"; import { attachObserverRuntimeStreamUrls, renderObserver, serveObserver } from "../src/observer.js"; import { OBSERVER_DATA_SCHEMA } from "../src/observer-data.js"; import { runDryRun } from "../src/run.js"; +import { syntheticPng1x1 } from "./image-fixtures.js"; -const PNG_1X1 = Buffer.from( - "iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAIAAACQd1PeAAAADUlEQVR42mP8z8BQDwAFgwJ/lp9J1wAAAABJRU5ErkJggg==", - "base64" -); +const PNG_1X1 = syntheticPng1x1(); async function withRunBundle(callback: (cwd: string) => Promise): Promise { const tempRoot = await mkdtemp(path.join(os.tmpdir(), "humanish-observer-fixture-")); @@ -986,7 +984,8 @@ describe("observer rendering", () => { const screenshotResponse = await fetch(screenshotUrl); expect(screenshotResponse.status).toBe(200); expect(screenshotResponse.headers.get("content-type")).toBe("image/png"); - expect(Buffer.from(await screenshotResponse.arrayBuffer()).subarray(0, 8)).toEqual(PNG_1X1.subarray(0, 8)); + expect(Buffer.from(await screenshotResponse.arrayBuffer()).subarray(0, 8)) + .toEqual(PNG_1X1.subarray(0, 8)); } finally { await server.close(); } diff --git a/tests/run.test.ts b/tests/run.test.ts index c4cb595..40d352e 100644 --- a/tests/run.test.ts +++ b/tests/run.test.ts @@ -26,8 +26,10 @@ import { type RunSubjectProvenance, type RunSubjectStateStepRecord } from "../src/run.js"; +import { SYNTHETIC_PNG_1X1_BASE64, syntheticPng1x1 } from "./image-fixtures.js"; const execFileAsync = promisify(execFile); +const PNG_1X1 = syntheticPng1x1(); function isNodeErrorCode(error: unknown, ...codes: string[]): boolean { return error instanceof Error @@ -135,7 +137,7 @@ async function writeFakeBrowserCommand(cwd: string): Promise { " process.stderr.write('missing screenshot arg\\n');", " process.exit(2);", "}", - "const png = Buffer.from('89504e470d0a1a0a0000000d4948445200000001000000010802000000907753de0000000c49444154789c6360f8cf000000040003027e7b040000000049454e44ae426082', 'hex');", + `const png = Buffer.from('${SYNTHETIC_PNG_1X1_BASE64}', 'base64');`, "fs.writeFileSync(screenshotArg.slice('--screenshot='.length), png);", "process.exit(0);" ].join("\n"), @@ -787,7 +789,7 @@ describe("dry-run bundles", () => { }); }); - it("rejects referenced screenshot files that are not valid image evidence", async () => { + it("rejects referenced screenshot files that have a valid PNG signature but cannot be decoded", async () => { await withFixtureCopy(async (cwd) => { const run = await runDryRun({ cwd, @@ -797,9 +799,9 @@ describe("dry-run bundles", () => { expect(run.ok).toBe(true); const runRoot = path.join(cwd, ".humanish/runs/invalid-screenshot-regression"); - const screenshotPath = "screenshots/not-a-real-png.png"; + const screenshotPath = "screenshots/truncated.png"; await mkdir(path.join(runRoot, "screenshots"), { recursive: true }); - await writeFile(path.join(runRoot, screenshotPath), "this file is non-empty but not an image", "utf8"); + await writeFile(path.join(runRoot, screenshotPath), PNG_1X1.subarray(0, 24)); const bundlePath = path.join(runRoot, "run.json"); const bundle = JSON.parse(await readFile(bundlePath, "utf8")) as { @@ -819,7 +821,7 @@ describe("dry-run bundles", () => { const verify = await verifyRun(cwd, "invalid-screenshot-regression"); expect(verify.ok).toBe(false); expect(verify.checks.find((check) => check.name === "local evidence artifacts exist")?.message) - .toContain("screenshots/not-a-real-png.png (expected PNG signature)"); + .toContain("screenshots/truncated.png (could not decode PNG evidence)"); }); }); diff --git a/tests/scripted-browser-actor.test.ts b/tests/scripted-browser-actor.test.ts index f07de71..ac1ddce 100644 --- a/tests/scripted-browser-actor.test.ts +++ b/tests/scripted-browser-actor.test.ts @@ -17,11 +17,9 @@ import { type ScriptedLocatorLike, type ScriptedPageLike } from "../src/scripted-browser-actor.js"; +import { syntheticPng1x1 } from "./image-fixtures.js"; -const PNG_1X1 = Buffer.from( - "iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAIAAACQd1PeAAAADUlEQVR42mP8z8BQDwAFgwJ/lp9J1wAAAABJRU5ErkJggg==", - "base64" -); +const PNG_1X1 = syntheticPng1x1(); // --------------------------------------------------------------------------- // Fake browser: a tiny in-memory "app" behind the structural seams, driven by diff --git a/tests/scripted-browser-lab.test.ts b/tests/scripted-browser-lab.test.ts index 4e337e9..725fd70 100644 --- a/tests/scripted-browser-lab.test.ts +++ b/tests/scripted-browser-lab.test.ts @@ -20,12 +20,10 @@ import { type ScriptedBrowserLabHooks } from "../src/scripted-browser-lab.js"; import type { ScriptedBrowserLike, ScriptedBrowserSessionResult, ScriptedLocatorLike, ScriptedPageLike } from "../src/scripted-browser-actor.js"; +import { syntheticPng1x1 } from "./image-fixtures.js"; const ROOT = process.cwd(); -const PNG_1X1 = Buffer.from( - "iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAIAAACQd1PeAAAADUlEQVR42mP8z8BQDwAFgwJ/lp9J1wAAAABJRU5ErkJggg==", - "base64" -); +const PNG_1X1 = syntheticPng1x1(); // --------------------------------------------------------------------------- // Fakes + fixtures. The fake browser drives the REAL step executor and writes