From daff8328a7db035b3d52784e23299754640f6a62 Mon Sep 17 00:00:00 2001 From: GautamSharma99 Date: Thu, 30 Jul 2026 18:39:10 +0530 Subject: [PATCH] fix: bound bulk-scan inventories --- sdk/typescript/README.md | 5 + sdk/typescript/src/bulk-scan-discovery.ts | 7 + sdk/typescript/src/bulk-scan-limits.ts | 8 + sdk/typescript/src/multiscan.ts | 208 +++++++++++++----- .../tests-ts/bulk-scan-discovery.test.ts | 37 ++++ sdk/typescript/tests-ts/multiscan.test.ts | 47 ++++ 6 files changed, 258 insertions(+), 54 deletions(-) create mode 100644 sdk/typescript/src/bulk-scan-limits.ts diff --git a/sdk/typescript/README.md b/sdk/typescript/README.md index 9e1f4a51..8863bb8b 100644 --- a/sdk/typescript/README.md +++ b/sdk/typescript/README.md @@ -405,6 +405,11 @@ service,https://github.com/acme/service.git,0123456789abcdef0123456789abcdef0123 `--workers` limits concurrent scans and `--max-attempts` retries failures. Results remain under `--output-dir`; rerun the same command to resume. +Bulk-scan inventories are limited to 8 MiB and 1,000 repositories. Interactive +GitHub discovery applies the same repository limit and stops before creating an +inventory if the selected account exceeds it. Split larger campaigns into +separate repository lists and output directories. + ### Scan history and reruns `npx @openai/codex-security scans list` lists scans for the current repository. Pass a diff --git a/sdk/typescript/src/bulk-scan-discovery.ts b/sdk/typescript/src/bulk-scan-discovery.ts index cfad210a..bf08851d 100644 --- a/sdk/typescript/src/bulk-scan-discovery.ts +++ b/sdk/typescript/src/bulk-scan-discovery.ts @@ -8,6 +8,10 @@ import { promisify } from "node:util"; import { confirm, input, search } from "@inquirer/prompts"; import { Octokit } from "@octokit/core"; import Papa from "papaparse"; +import { + bulkScanRepositoryLimitError, + MAX_BULK_SCAN_REPOSITORIES, +} from "./bulk-scan-limits.js"; import { resolveTrustedExecutable } from "./trusted-executable.js"; const execFile = promisify(execFileCallback); @@ -270,6 +274,9 @@ async function discoverGitHubRepositories( url: `https://${host}/${repository.nameWithOwner}.git`, revision: repository.defaultBranchRef.target.oid.toLowerCase(), }); + if (repositories.length > MAX_BULK_SCAN_REPOSITORIES) { + throw bulkScanRepositoryLimitError(); + } } cursor = connection.pageInfo.hasNextPage ? connection.pageInfo.endCursor diff --git a/sdk/typescript/src/bulk-scan-limits.ts b/sdk/typescript/src/bulk-scan-limits.ts new file mode 100644 index 00000000..d1211634 --- /dev/null +++ b/sdk/typescript/src/bulk-scan-limits.ts @@ -0,0 +1,8 @@ +export const MAX_BULK_SCAN_INVENTORY_BYTES = 8 * 1_024 * 1_024; +export const MAX_BULK_SCAN_REPOSITORIES = 1_000; + +export function bulkScanRepositoryLimitError(): Error { + return new Error( + "Bulk scans support at most 1,000 repositories per campaign. Split the repository list into smaller inventories.", + ); +} diff --git a/sdk/typescript/src/multiscan.ts b/sdk/typescript/src/multiscan.ts index b4a0fa7a..5ac22744 100644 --- a/sdk/typescript/src/multiscan.ts +++ b/sdk/typescript/src/multiscan.ts @@ -1,5 +1,6 @@ import { execFile as execFileCallback } from "node:child_process"; import { randomUUID } from "node:crypto"; +import { createReadStream } from "node:fs"; import { lstat, mkdir, @@ -8,13 +9,20 @@ import { realpath, rename, rm, + stat, truncate, writeFile, } from "node:fs/promises"; import { dirname, isAbsolute, join, relative, resolve, sep } from "node:path"; +import { Transform } from "node:stream"; import { promisify } from "node:util"; import Papa from "papaparse"; import type { CodexSecurity } from "./api.js"; +import { + bulkScanRepositoryLimitError, + MAX_BULK_SCAN_INVENTORY_BYTES, + MAX_BULK_SCAN_REPOSITORIES, +} from "./bulk-scan-limits.js"; import type { CodexSecurityConfig } from "./config.js"; import type { ScanCost } from "./cost.js"; import type { ScanMode } from "./targets.js"; @@ -82,10 +90,11 @@ export async function runMultiscan( if (!Number.isSafeInteger(options.maxAttempts) || options.maxAttempts < 1) { throw new Error("Multiscan max attempts must be a positive integer."); } - const tasks = parseInventory( - await readFile(options.inputPath, "utf8"), + const tasks = await parseInventory( + options.inputPath, dirname(resolve(options.inputPath)), options.mode, + options.signal, ); const output = resolve(options.outputDir); await ensureOutputDirectory(output); @@ -347,19 +356,106 @@ async function hasArtifacts(path: string): Promise { } } -function parseInventory( - source: string, +async function parseInventory( + inputPath: string, directory: string, defaultMode: ScanMode, -): MultiscanTask[] { - const { data: rows, errors } = Papa.parse(source, { - delimiter: ",", - skipEmptyLines: "greedy", - }); - if (errors.length > 0) { - throw new Error(`Multiscan CSV could not be parsed: ${errors[0]!.message}`); + signal?: AbortSignal, +): Promise { + signal?.throwIfAborted(); + const metadata = await stat(inputPath); + signal?.throwIfAborted(); + if (!metadata.isFile()) { + throw new Error("Multiscan inventory must be a regular CSV file."); + } + if (metadata.size > MAX_BULK_SCAN_INVENTORY_BYTES) { + throw new Error("Multiscan CSV must not exceed 8 MiB."); } - const headers = rows.shift(); + + const tasks: MultiscanTask[] = []; + const seen = new Set(); + let headers: string[] | undefined; + let bytesRead = 0; + const input = createReadStream(inputPath, { + highWaterMark: 64 * 1_024, + ...(signal === undefined ? {} : { signal }), + }); + const bounded = new Transform({ + transform(chunk: Buffer, _encoding, callback) { + bytesRead += chunk.length; + callback( + bytesRead > MAX_BULK_SCAN_INVENTORY_BYTES + ? new Error("Multiscan CSV must not exceed 8 MiB.") + : null, + chunk, + ); + }, + }); + input.pipe(bounded); + + return await new Promise((resolveTasks, rejectTasks) => { + let settled = false; + const fail = (error: unknown): void => { + if (settled) return; + settled = true; + const failure = error instanceof Error ? error : new Error(String(error)); + input.destroy(); + bounded.destroy(); + rejectTasks(failure); + }; + input.once("error", fail); + bounded.once("error", fail); + + Papa.parse(bounded as unknown as Papa.LocalFile, { + delimiter: ",", + skipEmptyLines: "greedy", + beforeFirstChunk: (chunk) => + chunk.charCodeAt(0) === 0xfeff ? chunk.slice(1) : chunk, + step: ({ data: fields, errors }) => { + try { + signal?.throwIfAborted(); + if (errors.length > 0) { + throw new Error( + `Multiscan CSV could not be parsed: ${errors[0]!.message}`, + ); + } + if (headers === undefined) { + headers = fields; + validateInventoryHeaders(headers); + return; + } + if (tasks.length >= MAX_BULK_SCAN_REPOSITORIES) { + throw bulkScanRepositoryLimitError(); + } + tasks.push( + parseInventoryRow(fields, headers, directory, defaultMode, seen), + ); + } catch (error) { + fail(error); + } + }, + complete: () => { + if (settled) return; + try { + signal?.throwIfAborted(); + if (headers === undefined) validateInventoryHeaders(undefined); + if (tasks.length === 0) { + throw new Error( + "Multiscan CSV must contain at least one repository.", + ); + } + settled = true; + resolveTasks(tasks); + } catch (error) { + fail(error); + } + }, + error: fail, + }); + }); +} + +function validateInventoryHeaders(headers: string[] | undefined): void { if ( headers === undefined || !["id", "repository", "revision"].every((name) => headers.includes(name)) || @@ -369,48 +465,52 @@ function parseInventory( "Multiscan CSV requires id, repository, and revision columns.", ); } - if (rows.length === 0) - throw new Error("Multiscan CSV must contain at least one repository."); - const seen = new Set(); - return rows.map((fields) => { - if (fields.length !== headers.length) { - throw new Error("Multiscan CSV rows must match their header columns."); - } - const get = (name: string): string => - fields[headers.indexOf(name)]?.trim() ?? ""; - const id = get("id"); - if (!/^[A-Za-z0-9][A-Za-z0-9._-]{0,127}$/u.test(id)) { - throw new Error("Multiscan task IDs must be safe, unique path names."); - } - if (seen.has(id.toLowerCase())) - throw new Error("Multiscan task IDs must be unique."); - seen.add(id.toLowerCase()); - const revision = get("revision").toLowerCase(); - if (!/^(?:[0-9a-f]{40}|[0-9a-f]{64})$/u.test(revision)) { - throw new Error("Multiscan revisions must be full immutable Git SHAs."); - } - const mode = get("mode") || defaultMode; - if (mode !== "standard" && mode !== "deep") { - throw new Error("Multiscan mode must be standard or deep."); - } - const scope = get("scope"); - if ( - scope && - (isAbsolute(scope) || - scope.includes("\\") || - scope.split("/").includes("..") || - scope.includes("\0")) - ) { - throw new Error("Multiscan scope must stay inside its repository."); - } - return { - id, - repository: normalizeRepository(get("repository"), directory), - revision, - mode, - ...(scope ? { scope } : {}), - }; - }); +} + +function parseInventoryRow( + fields: string[], + headers: string[], + directory: string, + defaultMode: ScanMode, + seen: Set, +): MultiscanTask { + if (fields.length !== headers.length) { + throw new Error("Multiscan CSV rows must match their header columns."); + } + const get = (name: string): string => + fields[headers.indexOf(name)]?.trim() ?? ""; + const id = get("id"); + if (!/^[A-Za-z0-9][A-Za-z0-9._-]{0,127}$/u.test(id)) { + throw new Error("Multiscan task IDs must be safe, unique path names."); + } + if (seen.has(id.toLowerCase())) + throw new Error("Multiscan task IDs must be unique."); + seen.add(id.toLowerCase()); + const revision = get("revision").toLowerCase(); + if (!/^(?:[0-9a-f]{40}|[0-9a-f]{64})$/u.test(revision)) { + throw new Error("Multiscan revisions must be full immutable Git SHAs."); + } + const mode = get("mode") || defaultMode; + if (mode !== "standard" && mode !== "deep") { + throw new Error("Multiscan mode must be standard or deep."); + } + const scope = get("scope"); + if ( + scope && + (isAbsolute(scope) || + scope.includes("\\") || + scope.split("/").includes("..") || + scope.includes("\0")) + ) { + throw new Error("Multiscan scope must stay inside its repository."); + } + return { + id, + repository: normalizeRepository(get("repository"), directory), + revision, + mode, + ...(scope ? { scope } : {}), + }; } function normalizeRepository(repository: string, directory: string): string { diff --git a/sdk/typescript/tests-ts/bulk-scan-discovery.test.ts b/sdk/typescript/tests-ts/bulk-scan-discovery.test.ts index a8337649..e1e24c42 100644 --- a/sdk/typescript/tests-ts/bulk-scan-discovery.test.ts +++ b/sdk/typescript/tests-ts/bulk-scan-discovery.test.ts @@ -15,6 +15,7 @@ import { type BulkScanDiscoveryDependencies, type BulkScanPrompt, } from "../src/bulk-scan-discovery.js"; +import { MAX_BULK_SCAN_REPOSITORIES } from "../src/bulk-scan-limits.js"; const temporaryDirectories: string[] = []; const NOW = Date.parse("2026-07-22T12:00:00.000Z"); @@ -255,6 +256,42 @@ describe("bulk scan repository discovery", () => { expect(csv).not.toContain("acme--old"); }); + test("enforces the discovery repository limit before creating an inventory", async () => { + const atLimit = Array.from( + { length: MAX_BULK_SCAN_REPOSITORIES }, + (_value, index) => ({ fullName: `acme/service-${index}` }), + ); + const acceptedRoot = await temporaryDirectory(); + const accepted = discoveryDependencies(acceptedRoot, { + repositories: atLimit, + }); + accepted.prompt.confirms = [false]; + expect(await runBulkScanWizard(accepted.dependencies)).toBeNull(); + expect(accepted.prompt.messages.join("")).toContain( + `Found ${MAX_BULK_SCAN_REPOSITORIES} repositories`, + ); + expect( + accepted.requests.filter(({ path }) => path === "/graphql"), + ).toHaveLength(10); + + const overflowRoot = await temporaryDirectory(); + const overflow = discoveryDependencies(overflowRoot, { + repositories: [ + ...atLimit, + { fullName: `acme/service-${MAX_BULK_SCAN_REPOSITORIES}` }, + ], + }); + await expect(runBulkScanWizard(overflow.dependencies)).rejects.toThrow( + "at most 1,000 repositories", + ); + expect( + overflow.requests.filter(({ path }) => path === "/graphql"), + ).toHaveLength(11); + await expect( + lstat(join(overflowRoot, "security-scans")), + ).rejects.toMatchObject({ code: "ENOENT" }); + }); + test("shows all repositories and scans only the selected ones", async () => { const root = await temporaryDirectory(); const { dependencies, prompt } = discoveryDependencies(root, { diff --git a/sdk/typescript/tests-ts/multiscan.test.ts b/sdk/typescript/tests-ts/multiscan.test.ts index e739ed74..196811b6 100644 --- a/sdk/typescript/tests-ts/multiscan.test.ts +++ b/sdk/typescript/tests-ts/multiscan.test.ts @@ -2,18 +2,24 @@ import { execFileSync } from "node:child_process"; import { access, appendFile, + lstat, mkdir, mkdtemp, readFile, readdir, rm, symlink, + truncate, writeFile, } from "node:fs/promises"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { afterEach, describe, expect, test } from "bun:test"; import type { ScanResult } from "../src/result.js"; +import { + MAX_BULK_SCAN_INVENTORY_BYTES, + MAX_BULK_SCAN_REPOSITORIES, +} from "../src/bulk-scan-limits.js"; import { buildGitHubCredentialArgs, runMultiscan } from "../src/multiscan.js"; import { resolveTrustedExecutable } from "../src/trusted-executable.js"; @@ -263,6 +269,47 @@ describe("multiscan", () => { expect(scans).toBe(0); }); + test("rejects oversized and excessive inventories before creating campaign state", async () => { + for (const prepare of [ + async (path: string) => { + await writeFile(path, "id,repository,revision\n"); + await truncate(path, MAX_BULK_SCAN_INVENTORY_BYTES + 1); + }, + async (path: string) => { + const rows = Array.from( + { length: MAX_BULK_SCAN_REPOSITORIES + 1 }, + (_value, index) => + `repository-${index},.,0123456789abcdef0123456789abcdef01234567`, + ); + await writeFile(path, `id,repository,revision\n${rows.join("\n")}\n`); + }, + ]) { + const paths = await fixture(); + await prepare(paths.input); + let clients = 0; + await expect( + runMultiscan({ + ...options( + paths, + client(async () => { + throw new Error("scan must not start"); + }), + ), + createSecurity: () => { + clients += 1; + return client(async () => { + throw new Error("scan must not start"); + }); + }, + }), + ).rejects.toThrow(/8 MiB|at most 1,000 repositories/u); + expect(clients).toBe(0); + await expect(lstat(paths.output)).rejects.toMatchObject({ + code: "ENOENT", + }); + } + }); + test("materializes the pinned commit, applies row options, and removes its checkout", async () => { const paths = await fixture(); const source = await repository(paths.root, "payments");