From 6d0bc657c1416ecdaff93a3b338a269ae915bb6b Mon Sep 17 00:00:00 2001 From: mldangelo-oai <269034524+mldangelo-oai@users.noreply.github.com> Date: Sun, 23 Aug 2026 19:39:43 +0000 Subject: [PATCH 1/2] fix(package): scan semantic content for internal references --- sdk/typescript/scripts/check-package.mjs | 245 +++++++++--------- .../scripts/package-internal-references.mjs | 53 ++++ .../scripts/package-tar-entries.mjs | 80 ++++++ .../package-internal-references.test.ts | 80 ++++++ .../tests-ts/package-tar-entries.test.ts | 221 ++++++++++++++++ 5 files changed, 562 insertions(+), 117 deletions(-) create mode 100644 sdk/typescript/scripts/package-internal-references.mjs create mode 100644 sdk/typescript/scripts/package-tar-entries.mjs create mode 100644 sdk/typescript/tests-ts/package-internal-references.test.ts create mode 100644 sdk/typescript/tests-ts/package-tar-entries.test.ts diff --git a/sdk/typescript/scripts/check-package.mjs b/sdk/typescript/scripts/check-package.mjs index 7074bdcd3..8ace3c130 100644 --- a/sdk/typescript/scripts/check-package.mjs +++ b/sdk/typescript/scripts/check-package.mjs @@ -1,10 +1,21 @@ import { spawnSync } from "node:child_process"; import { createHash } from "node:crypto"; -import { readFileSync } from "node:fs"; +import { + chmodSync, + lstatSync, + mkdtempSync, + readFileSync, + readdirSync, + rmSync, +} from "node:fs"; +import { tmpdir } from "node:os"; +import { join, resolve } from "node:path"; import { fileURLToPath } from "node:url"; -import { brotliDecompressSync, gunzipSync } from "node:zlib"; +import { gunzipSync } from "node:zlib"; +import { assertNoInternalReferences } from "./package-internal-references.mjs"; import { assertExpectedGitHead } from "./package-provenance.mjs"; import { packageSmokeTimeouts } from "./package-smoke-timeouts.mjs"; +import { plainTarEntries } from "./package-tar-entries.mjs"; import { regularTarListingLines } from "./package-tar-listing.mjs"; const PACKAGE_SMOKE_PROCESS_TIMEOUT_MS = @@ -22,15 +33,21 @@ if (archive === undefined || args.length > 2) { ); } +const archivePath = resolve(archive); const MAX_EXPANDED_ASSET_BYTES = 32 * 1024 * 1024; -const archiveBytes = gunzipSync(readFileSync(archive), { +const archiveBytes = gunzipSync(readFileSync(archivePath), { maxOutputLength: MAX_EXPANDED_ASSET_BYTES, }); const PUBLIC_LOGO_SHA256 = "9b9c2b09b2fa064611fb62307d321d5c2ea70cf0789f7ce34cdb0fc0d9190b3a"; -const tarOptions = { maxBuffer: archiveBytes.byteLength + 1024 }; +const processEnvironment = { ...process.env }; +delete processEnvironment.TAR_OPTIONS; +const tarOptions = { + env: { ...processEnvironment, LC_ALL: "C" }, + maxBuffer: archiveBytes.byteLength + 1024, +}; function tar(args, encoding = "buffer") { - const result = spawnSync("tar", ["--ignore-zeros", ...args], { + const result = spawnSync("tar", args, { ...tarOptions, encoding, }); @@ -44,59 +61,24 @@ function tar(args, encoding = "buffer") { return result.stdout; } -let offset = 0; -const archiveFiles = new Map(); -for (; offset + 512 <= archiveBytes.byteLength; ) { - const header = archiveBytes.subarray(offset, offset + 512); - if (header.every((byte) => byte === 0)) { - offset += 512; - continue; - } - const name = header.subarray(0, 100).toString("utf8").split("\0", 1)[0]; - const prefix = header.subarray(345, 500).toString("utf8").split("\0", 1)[0]; - const path = prefix === "" ? name : `${prefix}/${name}`; - const sizeField = header - .subarray(124, 136) - .toString("ascii") - .split("\0", 1)[0] - .trim(); - if (!/^[0-7]*$/u.test(sizeField)) { - throw new Error("npm tarball contains an invalid tar entry."); - } - if (path.endsWith("/") && header[156] !== 0x35) { - throw new Error("npm tarball contains an invalid tar entry."); - } - const size = Number.parseInt(sizeField || "0", 8); - const contentsStart = offset + 512; - const nextOffset = contentsStart + Math.ceil(size / 512) * 512; - if (nextOffset > archiveBytes.byteLength) { - throw new Error("npm tarball contains an invalid tar entry."); - } - if (header[156] === 0 || header[156] === 0x30) { - archiveFiles.set( - path, - archiveBytes.subarray(contentsStart, contentsStart + size), - ); - } - offset = nextOffset; -} -if (archiveBytes.subarray(offset).some((byte) => byte !== 0)) { - throw new Error("npm tarball contains trailing tar data."); -} - -function archiveFile(path) { - const contents = archiveFiles.get(path); - if (contents === undefined) { - throw new Error("npm tarball contains an invalid tar entry: " + path + "."); - } - return contents; +function invalidTarEntry() { + throw new Error("npm tarball contains an invalid tar entry."); } -const entries = tar(["-tzf", archive], "utf8").split(/\r?\n/u).filter(Boolean); +const rawEntries = plainTarEntries(archiveBytes); +const entries = tar(["-tzf", archivePath], "utf8") + .split(/\r?\n/u) + .filter(Boolean); const files = new Set(entries); if (files.size !== entries.length) { throw new Error("npm tarball contains duplicate paths."); } +if ( + rawEntries.length !== entries.length || + rawEntries.some(({ path }, index) => path !== entries[index]) +) { + invalidTarEntry(); +} const required = [ "package/package.json", "package/README.md", @@ -138,18 +120,11 @@ if (pluginFiles.size !== pluginPaths.length) { } const pluginEntries = new Set(); -const pluginDirectories = new Set(["package/_bundled_plugin"]); for (const file of pluginFiles) { - const archivePath = `package/_bundled_plugin/${file}`; - pluginEntries.add(archivePath); - if (!files.has(archivePath)) { - throw new Error(`npm tarball is missing ${archivePath}.`); - } - const parts = file.split("/"); - for (let index = 1; index < parts.length; index++) { - pluginDirectories.add( - `package/_bundled_plugin/${parts.slice(0, index).join("/")}`, - ); + const pluginArchivePath = `package/_bundled_plugin/${file}`; + pluginEntries.add(pluginArchivePath); + if (!files.has(pluginArchivePath)) { + throw new Error(`npm tarball is missing ${pluginArchivePath}.`); } } @@ -209,29 +184,20 @@ for (const file of distFiles) { } const unsafePath = /(?:^|\/)\.{1,2}(?:\/|$)/u; for (const file of files) { - const normalized = file.endsWith("/") ? file.slice(0, -1) : file; - const allowed = file.endsWith("/") - ? normalized === "package" || - normalized === "package/bin" || - normalized === "package/dist" || - pluginDirectories.has(normalized) - : allowedRoot.has(normalized) || - distFiles.has(normalized) || - pluginEntries.has(normalized); + const allowed = + allowedRoot.has(file) || distFiles.has(file) || pluginEntries.has(file); if (!allowed || unsafePath.test(file) || file.includes("\\")) { throw new Error(`npm tarball contains an unexpected file: ${file}.`); } } -const listing = tar(["-tvzf", archive], "utf8"); +const listing = tar(["-tvzf", archivePath], "utf8"); const listingLines = regularTarListingLines(listing); if ( listingLines.length !== entries.length || - listingLines.some( - (line, index) => line.startsWith("d") !== entries[index].endsWith("/"), - ) + listingLines.some((line) => !line.startsWith("-")) ) { - throw new Error("npm tarball contains an invalid tar entry."); + invalidTarEntry(); } const launcherPermissions = listingLines[entries.indexOf("package/bin/codex-security.mjs")]?.split( @@ -241,6 +207,86 @@ const launcherPermissions = if ([3, 6, 9].some((index) => launcherPermissions[index] !== "x")) { throw new Error("npm package CLI launcher is not executable."); } + +function extractedArchiveFiles() { + const rawSizes = new Map(); + const expectedPaths = new Map(); + for (const { path, size } of rawEntries) { + if (expectedPaths.get(path) === "directory") invalidTarEntry(); + rawSizes.set(path, size); + expectedPaths.set(path, "file"); + const parts = path.split("/"); + for (let index = 1; index < parts.length; index++) { + const directory = parts.slice(0, index).join("/"); + if (rawSizes.has(directory)) invalidTarEntry(); + expectedPaths.set(directory, "directory"); + } + } + + const extractionRoot = mkdtempSync( + join(tmpdir(), "codex-security-package-check-"), + ); + try { + chmodSync(extractionRoot, 0o700); + tar([ + "--keep-old-files", + "--no-same-owner", + "--no-same-permissions", + "--no-acls", + "--no-xattrs", + "-xzf", + archivePath, + "-C", + extractionRoot, + ]); + + const archiveFiles = new Map(); + let expandedBytes = 0; + function visit(directory, relative = "") { + for (const name of readdirSync(directory)) { + const path = relative === "" ? name : `${relative}/${name}`; + const expectedType = expectedPaths.get(path); + if (expectedType === undefined) invalidTarEntry(); + const extractedPath = join(extractionRoot, path); + const stats = lstatSync(extractedPath); + expectedPaths.delete(path); + + if (expectedType === "directory") { + if (!stats.isDirectory()) invalidTarEntry(); + visit(extractedPath, path); + continue; + } + + if ( + !stats.isFile() || + stats.nlink !== 1 || + stats.size !== rawSizes.get(path) || + stats.size > MAX_EXPANDED_ASSET_BYTES || + expandedBytes > MAX_EXPANDED_ASSET_BYTES - stats.size + ) { + invalidTarEntry(); + } + expandedBytes += stats.size; + archiveFiles.set(path, readFileSync(extractedPath)); + } + } + visit(extractionRoot); + + if (expectedPaths.size !== 0 || archiveFiles.size !== rawEntries.length) { + invalidTarEntry(); + } + return archiveFiles; + } finally { + rmSync(extractionRoot, { force: true, recursive: true }); + } +} + +const archiveFiles = extractedArchiveFiles(); +function archiveFile(path) { + const contents = archiveFiles.get(path); + if (contents === undefined) invalidTarEntry(); + return contents; +} const packageJson = JSON.parse( archiveFile("package/package.json").toString("utf8"), ); @@ -255,40 +301,6 @@ assertExpectedGitHead( process.env.CODEX_SECURITY_EXPECTED_GIT_HEAD, ); -const internalMarker = - /(?:internal\.api\.openai\.org|gateway\.[a-z0-9.-]*internal|\.openai\.org|openai\.firewall\.socket\.dev|socket\x2dfirewall\x2dregistry|openai\.(?:enterprise\.)?slack\.com|app\.slack\.com\/client|(?:app\.notion\.com\/p|notion\.so)\/openai|linear\.app\/openai|(?:github\.com[:/]|api\.github\.com\/repos\/|raw\.githubusercontent\.com\/)openai\/openai(?:\.git)?(?:[^a-z0-9_-]|$)|LicenseRef\x2dProprietary|\/Users\/|\/home\/dev-user|flow\.apps\.openai\.org|(?:^|[^a-z0-9_-])go\/[a-z0-9_-]+)/iu; - -const payloads = [archiveBytes.toString("utf8")]; -const compressedFiles = [...files].filter((file) => /\.br$/iu.test(file)); -const compressedParts = new Map(); -for (const file of files) { - const match = /^(.*\.br)\.part-([0-9]+)$/iu.exec(file); - if (match === null) continue; - const [, name, part] = match; - const parts = compressedParts.get(name) ?? []; - parts.push({ file, part: Number(part) }); - compressedParts.set(name, parts); -} - -function brotliPayload(bytes, file) { - const result = brotliDecompressSync(bytes, { - info: true, - maxOutputLength: MAX_EXPANDED_ASSET_BYTES, - }); - if (result.engine.bytesWritten !== bytes.length) { - throw new Error(`npm tarball contains trailing Brotli data: ${file}.`); - } - return result.buffer; -} - -for (const file of compressedFiles) { - payloads.push(brotliPayload(archiveFile(file), file).toString("utf8")); -} -for (const parts of compressedParts.values()) { - parts.sort((left, right) => left.part - right.part); - const bytes = Buffer.concat(parts.map(({ file }) => archiveFile(file))); - payloads.push(brotliPayload(bytes, parts[0].file).toString("utf8")); -} for (const file of files) { if (/\.png$/iu.test(file)) { const digest = createHash("sha256").update(archiveFile(file)).digest("hex"); @@ -298,16 +310,15 @@ for (const file of files) { } } -for (const contents of payloads) { - if (internalMarker.test(contents)) { - throw new Error("npm tarball contains an internal reference."); - } -} +assertNoInternalReferences(archiveFiles, MAX_EXPANDED_ASSET_BYTES); if (args.length === 1) { const smoke = spawnSync( process.execPath, - [fileURLToPath(new URL("./smoke-package.mjs", import.meta.url)), archive], + [ + fileURLToPath(new URL("./smoke-package.mjs", import.meta.url)), + archivePath, + ], { stdio: "inherit", timeout: PACKAGE_SMOKE_PROCESS_TIMEOUT_MS, diff --git a/sdk/typescript/scripts/package-internal-references.mjs b/sdk/typescript/scripts/package-internal-references.mjs new file mode 100644 index 000000000..372fb36c7 --- /dev/null +++ b/sdk/typescript/scripts/package-internal-references.mjs @@ -0,0 +1,53 @@ +import { brotliDecompressSync } from "node:zlib"; + +const internalMarker = + /(?:internal\.api\.openai\.org|gateway\.[a-z0-9.-]*internal|\.openai\.org|openai\.firewall\.socket\.dev|socket\x2dfirewall\x2dregistry|openai\.(?:enterprise\.)?slack\.com|app\.slack\.com\/client|(?:app\.notion\.com\/p|notion\.so)\/openai|linear\.app\/openai|(?:github\.com[:/]|api\.github\.com\/repos\/|raw\.githubusercontent\.com\/)openai\/openai(?:\.git)?(?:[^a-z0-9_-]|$)|LicenseRef\x2dProprietary|\/Users\/|\/home\/dev-user|flow\.apps\.openai\.org|(?:^|[^a-z0-9_-])go\/[a-z0-9_-]+)/iu; + +function brotliPayload(bytes, file, maxExpandedAssetBytes) { + const result = brotliDecompressSync(bytes, { + info: true, + maxOutputLength: maxExpandedAssetBytes, + }); + if (result.engine.bytesWritten !== bytes.length) { + throw new Error(`npm tarball contains trailing Brotli data: ${file}.`); + } + return result.buffer; +} + +export function assertNoInternalReference(contents) { + if (internalMarker.test(contents.toString("utf8"))) { + throw new Error("npm tarball contains an internal reference."); + } +} + +export function assertNoInternalReferences( + archiveFiles, + maxExpandedAssetBytes, +) { + const compressedParts = new Map(); + + for (const [file, contents] of archiveFiles) { + assertNoInternalReference(Buffer.from(file)); + const match = /^(.*\.br)\.part-([0-9]+)$/iu.exec(file); + if (match !== null) { + const [, name, part] = match; + const parts = compressedParts.get(name) ?? []; + parts.push({ file, part: Number(part), contents }); + compressedParts.set(name, parts); + } else if (/\.br$/iu.test(file)) { + assertNoInternalReference( + brotliPayload(contents, file, maxExpandedAssetBytes), + ); + } else if (!/\.png$/iu.test(file)) { + assertNoInternalReference(contents); + } + } + + for (const parts of compressedParts.values()) { + parts.sort((left, right) => left.part - right.part); + const bytes = Buffer.concat(parts.map(({ contents }) => contents)); + assertNoInternalReference( + brotliPayload(bytes, parts[0].file, maxExpandedAssetBytes), + ); + } +} diff --git a/sdk/typescript/scripts/package-tar-entries.mjs b/sdk/typescript/scripts/package-tar-entries.mjs new file mode 100644 index 000000000..186ba8b5d --- /dev/null +++ b/sdk/typescript/scripts/package-tar-entries.mjs @@ -0,0 +1,80 @@ +import { assertNoInternalReference } from "./package-internal-references.mjs"; + +const blockSize = 512; +const invalidTarEntryError = "npm tarball contains an invalid tar entry."; +function invalidTarEntry() { + throw new Error(invalidTarEntryError); +} + +function headerText(header, start, end) { + const field = header.subarray(start, end); + return field.toString("utf8").split("\0", 1)[0]; +} + +function canonicalSize(header) { + const field = header.subarray(124, 136); + if ( + !field.subarray(0, 11).every((byte) => byte >= 0x30 && byte <= 0x37) || + field[11] !== 0x20 + ) { + invalidTarEntry(); + } + return Number.parseInt(field.subarray(0, 11).toString("ascii"), 8); +} + +export function plainTarEntries(archiveBytes) { + if (archiveBytes.byteLength % blockSize !== 0) invalidTarEntry(); + const entries = []; + let offset = 0; + + while (offset + blockSize <= archiveBytes.byteLength) { + const header = archiveBytes.subarray(offset, offset + blockSize); + if (header.every((byte) => byte === 0)) { + const secondBlock = archiveBytes.subarray( + offset + blockSize, + offset + blockSize * 2, + ); + if ( + secondBlock.byteLength !== blockSize || + secondBlock.some((byte) => byte !== 0) || + archiveBytes.subarray(offset + blockSize * 2).some((byte) => byte !== 0) + ) { + invalidTarEntry(); + } + return entries; + } + + assertNoInternalReference(header); + if ( + header[156] !== 0x30 || + !header.subarray(257, 263).equals(Buffer.from("ustar\0")) || + !header.subarray(263, 265).equals(Buffer.from("00")) + ) { + invalidTarEntry(); + } + + const name = headerText(header, 0, 100); + const prefix = headerText(header, 345, 500); + const path = prefix === "" ? name : `${prefix}/${name}`; + if (name === "" || path.endsWith("/")) { + invalidTarEntry(); + } + assertNoInternalReference(Buffer.from(path)); + + const size = canonicalSize(header); + const contentsEnd = offset + blockSize + size; + const nextOffset = + offset + blockSize + Math.ceil(size / blockSize) * blockSize; + if ( + nextOffset > archiveBytes.byteLength || + archiveBytes.subarray(contentsEnd, nextOffset).some((byte) => byte !== 0) + ) { + invalidTarEntry(); + } + + entries.push({ path, size }); + offset = nextOffset; + } + + invalidTarEntry(); +} diff --git a/sdk/typescript/tests-ts/package-internal-references.test.ts b/sdk/typescript/tests-ts/package-internal-references.test.ts new file mode 100644 index 000000000..807256629 --- /dev/null +++ b/sdk/typescript/tests-ts/package-internal-references.test.ts @@ -0,0 +1,80 @@ +import { describe, expect, test } from "bun:test"; +import { brotliCompressSync, brotliDecompressSync } from "node:zlib"; + +type PackageInternalReferences = { + assertNoInternalReferences: ( + archiveFiles: ReadonlyMap, + maxExpandedAssetBytes: number, + ) => void; +}; + +const { assertNoInternalReferences } = (await import( + new URL("../scripts/package-internal-references.mjs", import.meta.url).href +)) as PackageInternalReferences; + +const maxExpandedAssetBytes = 1024; +const internalReferenceError = "npm tarball contains an internal reference."; + +describe("npm package internal reference checks", () => { + test("ignores marker-like bytes in Brotli metadata", () => { + // Valid Brotli metadata containing Go/1 followed by an empty final block. + const compressed = Buffer.from("6b1100ff476f2f3103", "hex"); + + expect(compressed.toString("utf8")).toMatch( + /(?:^|[^a-z0-9_-])go\/[a-z0-9_-]+/iu, + ); + expect(brotliDecompressSync(compressed)).toEqual(Buffer.alloc(0)); + expect(() => + assertNoInternalReferences( + new Map([["package/payload.br", compressed]]), + maxExpandedAssetBytes, + ), + ).not.toThrow(); + }); + + test("rejects an internal reference in normal text", () => { + expect(() => + assertNoInternalReferences( + new Map([["package/README.md", Buffer.from("See go/example.")]]), + maxExpandedAssetBytes, + ), + ).toThrow(internalReferenceError); + }); + + test("rejects an internal reference in an archive path", () => { + expect(() => + assertNoInternalReferences( + new Map([ + ["package/references/go/example.md", Buffer.from("Public text.")], + ]), + maxExpandedAssetBytes, + ), + ).toThrow(internalReferenceError); + }); + + test("rejects an internal reference in a Brotli payload", () => { + const compressed = brotliCompressSync(Buffer.from("See go/example.")); + + expect(() => + assertNoInternalReferences( + new Map([["package/payload.br", compressed]]), + maxExpandedAssetBytes, + ), + ).toThrow(internalReferenceError); + }); + + test("rejects an internal reference in a partitioned Brotli payload", () => { + const compressed = brotliCompressSync(Buffer.from("See go/example.")); + const split = Math.floor(compressed.length / 2); + + expect(() => + assertNoInternalReferences( + new Map([ + ["package/payload.br.part-000", compressed.subarray(0, split)], + ["package/payload.br.part-001", compressed.subarray(split)], + ]), + maxExpandedAssetBytes, + ), + ).toThrow(internalReferenceError); + }); +}); diff --git a/sdk/typescript/tests-ts/package-tar-entries.test.ts b/sdk/typescript/tests-ts/package-tar-entries.test.ts new file mode 100644 index 000000000..20a54650a --- /dev/null +++ b/sdk/typescript/tests-ts/package-tar-entries.test.ts @@ -0,0 +1,221 @@ +import { describe, expect, test } from "bun:test"; + +type PlainTarEntry = { + path: string; + size: number; +}; + +type PackageTarEntries = { + plainTarEntries: (archiveBytes: Buffer) => PlainTarEntry[]; +}; + +const { plainTarEntries } = (await import( + new URL("../scripts/package-tar-entries.mjs", import.meta.url).href +)) as PackageTarEntries; + +const blockSize = 512; +const invalidTarEntryError = "npm tarball contains an invalid tar entry."; +const internalReferenceError = "npm tarball contains an internal reference."; + +function octal(value: number, width: number, terminator = "\0"): Buffer { + return Buffer.from( + value.toString(8).padStart(width - terminator.length, "0") + terminator, + ); +} + +function tarHeader({ + name, + prefix = "", + size = 0, + type = 0x30, + sizeField = octal(size, 12, " "), + magic = "ustar\0", + version = "00", + user = "", + deviceNumbers = Buffer.alloc(16), + reserved = Buffer.alloc(12), +}: { + name: string; + prefix?: string; + size?: number; + type?: number; + sizeField?: Buffer; + magic?: string; + version?: string; + user?: string; + deviceNumbers?: Buffer; + reserved?: Buffer; +}): Buffer { + const header = Buffer.alloc(blockSize); + header.write(name, 0, 100, "utf8"); + octal(0o644, 8).copy(header, 100); + octal(0, 8).copy(header, 108); + octal(0, 8).copy(header, 116); + sizeField.copy(header, 124); + octal(0, 12).copy(header, 136); + header.fill(0x20, 148, 156); + header[156] = type; + header.write(magic, 257, "binary"); + header.write(version, 263, "binary"); + header.write(user, 265, 32, "utf8"); + deviceNumbers.copy(header, 329, 0, 16); + header.write(prefix, 345, 155, "utf8"); + reserved.copy(header, 500, 0, 12); + const checksum = header.reduce((sum, byte) => sum + byte, 0); + Buffer.from(checksum.toString(8).padStart(6, "0") + "\0 ").copy(header, 148); + return header; +} + +function tarRecord( + contents: Buffer, + options: Omit[0], "size">, +): Buffer { + return Buffer.concat([ + tarHeader({ ...options, size: contents.length }), + contents, + Buffer.alloc( + Math.ceil(contents.length / blockSize) * blockSize - contents.length, + ), + ]); +} + +function archive(...records: Buffer[]): Buffer { + return Buffer.concat([...records, Buffer.alloc(blockSize * 2)]); +} + +describe("plain npm tar entries", () => { + test("accepts canonical ustar files and prefix paths", () => { + const prefix = `package/${"nested/".repeat(13)}deep`; + const longPath = `${prefix}/README.md`; + expect(longPath.length).toBeGreaterThan(100); + + const bytes = archive( + tarRecord(Buffer.from("license"), { name: "package/LICENSE" }), + tarRecord(Buffer.from("readme"), { name: "README.md", prefix }), + ); + + expect(plainTarEntries(bytes)).toEqual([ + { path: "package/LICENSE", size: 7 }, + { path: longPath, size: 6 }, + ]); + }); + + test("rejects every unsupported tar entry type", () => { + for (const type of [ + 0, 0x31, 0x32, 0x35, 0x44, 0x4b, 0x4c, 0x53, 0x67, 0x78, + ]) { + expect(() => + plainTarEntries( + archive( + tarRecord(Buffer.from("x"), { + name: "package/README.md", + type, + }), + ), + ), + ).toThrow(invalidTarEntryError); + } + }); + + test("rejects alternate size encodings", () => { + const base256 = Buffer.alloc(12); + base256[0] = 0x80; + base256[11] = 1; + for (const sizeField of [ + base256, + octal(1, 12), + Buffer.from(" 0000000001 "), + ]) { + expect(() => + plainTarEntries( + archive( + tarRecord(Buffer.from("x"), { + name: "package/README.md", + sizeField, + }), + ), + ), + ).toThrow(invalidTarEntryError); + } + }); + + test("rejects alternate ustar signatures", () => { + for (const options of [ + { magic: "ustar " }, + { magic: "ustar\0", version: " \0" }, + ]) { + expect(() => + plainTarEntries( + archive( + tarRecord(Buffer.from("x"), { + name: "package/README.md", + ...options, + }), + ), + ), + ).toThrow(invalidTarEntryError); + } + }); + + test("scans complete header text fields", () => { + expect(() => + plainTarEntries( + archive( + tarRecord(Buffer.from("clean"), { + name: "package/README.md", + user: "public\0go/example", + }), + ), + ), + ).toThrow(internalReferenceError); + }); + + test("scans complete raw headers", () => { + for (const options of [ + { + deviceNumbers: Buffer.concat([ + Buffer.from("go/example"), + Buffer.alloc(6), + ]), + }, + { reserved: Buffer.from("go/example") }, + ]) { + expect(() => + plainTarEntries( + archive( + tarRecord(Buffer.from("clean"), { + name: "package/README.md", + ...options, + }), + ), + ), + ).toThrow(internalReferenceError); + } + }); + + test("rejects nonzero padding and data after the terminator", () => { + const record = tarRecord(Buffer.from("x"), { + name: "package/README.md", + }); + const badPadding = Buffer.from(record); + badPadding[badPadding.length - 1] = 1; + const secondRecord = tarRecord(Buffer.from("y"), { + name: "package/LICENSE", + }); + + for (const bytes of [ + archive(badPadding), + Buffer.concat([record, Buffer.alloc(blockSize), secondRecord]), + Buffer.concat([ + record, + Buffer.alloc(blockSize * 2), + secondRecord, + Buffer.alloc(blockSize * 2), + ]), + Buffer.concat([archive(record), Buffer.alloc(1)]), + record, + ]) { + expect(() => plainTarEntries(bytes)).toThrow(invalidTarEntryError); + } + }); +}); From fd44228f5b9c8b73b13869e0145586f59198dfe5 Mon Sep 17 00:00:00 2001 From: mldangelo-oai <269034524+mldangelo-oai@users.noreply.github.com> Date: Sun, 23 Aug 2026 20:39:42 +0000 Subject: [PATCH 2/2] fix(package): stream tar archives through stdin --- sdk/typescript/scripts/check-package.mjs | 16 +- .../tests-ts/package-tar-listing.test.ts | 332 ++++++++++++++++++ 2 files changed, 340 insertions(+), 8 deletions(-) diff --git a/sdk/typescript/scripts/check-package.mjs b/sdk/typescript/scripts/check-package.mjs index 8ace3c130..32a75fbcf 100644 --- a/sdk/typescript/scripts/check-package.mjs +++ b/sdk/typescript/scripts/check-package.mjs @@ -35,19 +35,22 @@ if (archive === undefined || args.length > 2) { const archivePath = resolve(archive); const MAX_EXPANDED_ASSET_BYTES = 32 * 1024 * 1024; -const archiveBytes = gunzipSync(readFileSync(archivePath), { +const compressedArchive = readFileSync(archivePath); +const archiveBytes = gunzipSync(compressedArchive, { maxOutputLength: MAX_EXPANDED_ASSET_BYTES, }); +const rawEntries = plainTarEntries(archiveBytes); const PUBLIC_LOGO_SHA256 = "9b9c2b09b2fa064611fb62307d321d5c2ea70cf0789f7ce34cdb0fc0d9190b3a"; const processEnvironment = { ...process.env }; delete processEnvironment.TAR_OPTIONS; const tarOptions = { env: { ...processEnvironment, LC_ALL: "C" }, + input: compressedArchive, maxBuffer: archiveBytes.byteLength + 1024, }; function tar(args, encoding = "buffer") { - const result = spawnSync("tar", args, { + const result = spawnSync("tar", ["--ignore-zeros", ...args], { ...tarOptions, encoding, }); @@ -65,10 +68,7 @@ function invalidTarEntry() { throw new Error("npm tarball contains an invalid tar entry."); } -const rawEntries = plainTarEntries(archiveBytes); -const entries = tar(["-tzf", archivePath], "utf8") - .split(/\r?\n/u) - .filter(Boolean); +const entries = tar(["-tzf", "-"], "utf8").split(/\r?\n/u).filter(Boolean); const files = new Set(entries); if (files.size !== entries.length) { throw new Error("npm tarball contains duplicate paths."); @@ -191,7 +191,7 @@ for (const file of files) { } } -const listing = tar(["-tvzf", archivePath], "utf8"); +const listing = tar(["-tvzf", "-"], "utf8"); const listingLines = regularTarListingLines(listing); if ( listingLines.length !== entries.length || @@ -235,7 +235,7 @@ function extractedArchiveFiles() { "--no-acls", "--no-xattrs", "-xzf", - archivePath, + "-", "-C", extractionRoot, ]); diff --git a/sdk/typescript/tests-ts/package-tar-listing.test.ts b/sdk/typescript/tests-ts/package-tar-listing.test.ts index d2d117280..d97ac08e1 100644 --- a/sdk/typescript/tests-ts/package-tar-listing.test.ts +++ b/sdk/typescript/tests-ts/package-tar-listing.test.ts @@ -1,9 +1,146 @@ +import { spawnSync } from "node:child_process"; +import { createHash } from "node:crypto"; +import { + chmodSync, + existsSync, + mkdirSync, + mkdtempSync, + readFileSync, + realpathSync, + rmSync, + writeFileSync, +} from "node:fs"; +import { tmpdir } from "node:os"; +import { delimiter, join } from "node:path"; +import { fileURLToPath } from "node:url"; +import { gzipSync } from "node:zlib"; import { describe, expect, test } from "bun:test"; const { regularTarListingLines } = (await import( new URL("../scripts/package-tar-listing.mjs", import.meta.url).href )) as { regularTarListingLines: (listing: string) => string[] }; +const blockSize = 512; +const distModules = [ + "api", + "auth", + "bulk-scan-discovery", + "cli", + "codex-prompt", + "component-plan", + "component-scan", + "config", + "contract", + "cost", + "cost-model", + "custom-validation", + "custom-validation-prompt", + "errors", + "index", + "knowledge-base", + "linear", + "models", + "multiscan", + "patch-tui", + "publication", + "publication-events", + "publication-store", + "publish", + "result", + "runtime", + "scan-activity", + "scan-comparison", + "scan-dashboard", + "scan-history-renderer", + "scan-logs", + "scan-sessions", + "targets", + "trusted-executable", + "version", + "windows-path", + "worker-progress", +]; + +function octal(value: number, width: number, terminator = "\0"): Buffer { + return Buffer.from( + value.toString(8).padStart(width - terminator.length, "0") + terminator, + ); +} + +function tarRecord(path: string, contents: Buffer, mode = 0o644): Buffer { + const header = Buffer.alloc(blockSize); + header.write(path, 0, 100, "utf8"); + octal(mode, 8).copy(header, 100); + octal(0, 8).copy(header, 108); + octal(0, 8).copy(header, 116); + octal(contents.length, 12, " ").copy(header, 124); + octal(0, 12).copy(header, 136); + header.fill(0x20, 148, 156); + header[156] = 0x30; + header.write("ustar\0", 257, "binary"); + header.write("00", 263, "binary"); + const checksum = header.reduce((sum, byte) => sum + byte, 0); + Buffer.from(checksum.toString(8).padStart(6, "0") + "\0 ").copy(header, 148); + return Buffer.concat([ + header, + contents, + Buffer.alloc( + Math.ceil(contents.length / blockSize) * blockSize - contents.length, + ), + ]); +} + +function packageTar(trailingZeroBytes = 0): Buffer { + const paths = [ + "package/package.json", + "package/README.md", + "package/LICENSE", + "package/bin/codex-security.mjs", + ...distModules.flatMap((module) => + ["js", "js.map", "d.ts", "d.ts.map"].map( + (extension) => `package/dist/${module}.${extension}`, + ), + ), + "package/_bundled_plugin/.codex-plugin/plugin.json", + ]; + const records = paths.map((path) => { + const contents = + path === "package/package.json" + ? Buffer.from( + JSON.stringify({ + license: "Apache-2.0", + name: "@openai/codex-security", + }), + ) + : path.endsWith(".json") || path.endsWith(".map") + ? Buffer.from("{}\n") + : Buffer.from("fixture\n"); + return tarRecord( + path, + contents, + path === "package/bin/codex-security.mjs" ? 0o755 : 0o644, + ); + }); + return Buffer.concat([ + ...records, + Buffer.alloc(blockSize * 2 + trailingZeroBytes), + ]); +} + +function commandPath(command: string): string { + const lookup = spawnSync( + process.platform === "win32" ? "where.exe" : "which", + [command], + { encoding: "utf8", windowsHide: true }, + ); + if (lookup.error !== undefined) throw lookup.error; + const path = lookup.stdout.split(/\r?\n/u).find(Boolean); + if (lookup.status !== 0 || path === undefined) { + throw new Error(`Could not resolve ${command}.`); + } + return path; +} + describe("npm package tar listings", () => { test("accepts regular entries with Unix or Windows line endings", () => { const file = "-rw-r--r-- package/package.json"; @@ -24,4 +161,199 @@ describe("npm package tar listings", () => { regularTarListingLines("lrwxrwxrwx package/link -> target\r\n"), ).toThrow("npm tarball contains a non-regular entry"); }); + + test("accepts equivalent bounded gzip representations", () => { + const root = mkdtempSync(join(tmpdir(), "codex-package-gzip-test-")); + try { + const tarBytes = packageTar(31 * 1024 * 1024); + const archives = [ + ["default", gzipSync(tarBytes)], + ["level-0", gzipSync(tarBytes, { level: 0 })], + ] as const; + expect(archives[0][1].length).toBeLessThan(1024 * 1024); + expect(archives[1][1].length).toBeGreaterThan(31 * 1024 * 1024); + + const contractPath = join(root, "plugin contract.json"); + writeFileSync( + contractPath, + JSON.stringify({ + externalOwnedExact: [".codex-plugin/plugin.json"], + shippedExact: [], + }), + ); + const environment: NodeJS.ProcessEnv = { ...process.env }; + delete environment["CODEX_SECURITY_EXPECTED_GIT_HEAD"]; + for (const [representation, contents] of archives) { + const archivePath = join(root, `${representation}.tgz`); + writeFileSync(archivePath, contents); + const result = spawnSync( + commandPath("node"), + [ + fileURLToPath( + new URL("../scripts/check-package.mjs", import.meta.url), + ), + archivePath, + contractPath, + ], + { + cwd: root, + encoding: "utf8", + env: environment, + timeout: 30_000, + windowsHide: true, + }, + ); + expect({ + representation, + status: result.status, + stderr: result.stderr, + }).toEqual({ representation, status: 0, stderr: "" }); + } + } finally { + rmSync(root, { force: true, recursive: true }); + } + }); + + test("streams each archive without resolving tar from its directory", () => { + const root = mkdtempSync(join(tmpdir(), "codex-package-tar-test-")); + try { + const archiveDirectory = join( + root, + process.platform === "win32" ? "package archive" : "D: package archive", + ); + mkdirSync(archiveDirectory, { recursive: true }); + const archivePath = join( + archiveDirectory, + process.platform === "win32" + ? "-fixture package.tgz" + : "-fixture: package.tgz", + ); + const contractPath = join(root, "plugin contract.json"); + const logPath = join(root, "tar calls.jsonl"); + const adjacentTarMarker = join(root, "archive tar ran"); + const tarBytes = packageTar(); + const archiveContents = gzipSync(tarBytes, { level: 0 }); + writeFileSync(archivePath, archiveContents); + writeFileSync( + contractPath, + JSON.stringify({ + externalOwnedExact: [".codex-plugin/plugin.json"], + shippedExact: [], + }), + ); + + const nodePath = commandPath("node"); + const environment: NodeJS.ProcessEnv = { ...process.env }; + delete environment["CODEX_SECURITY_EXPECTED_GIT_HEAD"]; + environment["PATH"] = `.${delimiter}${process.env["PATH"] ?? ""}`; + if (process.platform === "win32") { + for (const name of ["tar.com", "tar.exe"]) { + writeFileSync(join(archiveDirectory, name), "not an executable"); + } + } else { + const adjacentTar = join(archiveDirectory, "tar"); + writeFileSync( + adjacentTar, + `#!/usr/bin/env node +require("node:fs").writeFileSync(process.env.ADJACENT_TAR_MARKER, "ran"); +process.exit(99); +`, + ); + chmodSync(adjacentTar, 0o755); + + const proxySource = `#!/usr/bin/env node +const { spawnSync } = require("node:child_process"); +const { createHash } = require("node:crypto"); +const { appendFileSync, readFileSync } = require("node:fs"); +const input = readFileSync(0); +appendFileSync( + process.env.TAR_PROXY_LOG, + JSON.stringify({ + args: process.argv.slice(2), + cwd: process.cwd(), + inputLength: input.length, + inputSha256: createHash("sha256").update(input).digest("hex"), + }) + "\\n", +); +const result = spawnSync(process.env.REAL_TAR, process.argv.slice(2), { + env: process.env, + input, + stdio: ["pipe", "inherit", "inherit"], + windowsHide: true, +}); +if (result.error !== undefined) throw result.error; +process.exit(result.status ?? 1); +`; + const proxyPath = join(root, "tar"); + writeFileSync(proxyPath, proxySource); + chmodSync(proxyPath, 0o755); + environment["ADJACENT_TAR_MARKER"] = adjacentTarMarker; + environment["REAL_TAR"] = commandPath("tar"); + environment["TAR_PROXY_LOG"] = logPath; + } + + const result = spawnSync( + nodePath, + [ + fileURLToPath( + new URL("../scripts/check-package.mjs", import.meta.url), + ), + archivePath, + contractPath, + ], + { + cwd: root, + encoding: "utf8", + env: environment, + timeout: 30_000, + windowsHide: true, + }, + ); + expect(existsSync(adjacentTarMarker)).toBe(false); + expect({ status: result.status, stderr: result.stderr }).toEqual({ + status: 0, + stderr: "", + }); + if (process.platform === "win32") return; + + const calls = readFileSync(logPath, "utf8") + .trim() + .split(/\r?\n/u) + .map( + (line) => + JSON.parse(line) as { + args: string[]; + cwd: string; + inputLength: number; + inputSha256: string; + }, + ); + const archiveSha256 = createHash("sha256") + .update(archiveContents) + .digest("hex"); + expect(calls).toHaveLength(3); + for (const call of calls) { + expect(call.args.filter((arg) => arg === "-")).toEqual(["-"]); + expect(call.args).not.toContain(archivePath); + expect(realpathSync(call.cwd)).toBe(realpathSync(root)); + expect(call.inputLength).toBe(archiveContents.length); + expect(call.inputSha256).toBe(archiveSha256); + } + expect(calls[0]?.args).toEqual(["--ignore-zeros", "-tzf", "-"]); + expect(calls[1]?.args).toEqual(["--ignore-zeros", "-tvzf", "-"]); + expect(calls[2]?.args.slice(0, 9)).toEqual([ + "--ignore-zeros", + "--keep-old-files", + "--no-same-owner", + "--no-same-permissions", + "--no-acls", + "--no-xattrs", + "-xzf", + "-", + "-C", + ]); + } finally { + rmSync(root, { force: true, recursive: true }); + } + }); });