diff --git a/sdk/typescript/src/multiscan.ts b/sdk/typescript/src/multiscan.ts index 9704014c..794ce01b 100644 --- a/sdk/typescript/src/multiscan.ts +++ b/sdk/typescript/src/multiscan.ts @@ -16,7 +16,7 @@ import { hostname } from "node:os"; import { dirname, isAbsolute, join, relative, resolve, sep } from "node:path"; import { promisify } from "node:util"; import Papa from "papaparse"; -import type { CodexSecurity } from "./api.js"; +import type { CodexSecurity, ScanWarningDetails } from "./api.js"; import type { CodexSecurityConfig } from "./config.js"; import type { ScanCost } from "./cost.js"; import { redactedErrorMessage } from "./errors.js"; @@ -43,6 +43,15 @@ interface MultiscanTask { prompt?: string; } +/** + * A warning the scan raised while still completing. `kind` is carried so a + * consumer can single out drift without matching on message text. + */ +interface MultiscanWarning { + message: string; + kind?: ScanWarningDetails["kind"]; +} + interface MultiscanReceipt extends MultiscanTask { status: "completed" | "completed_with_incomplete_coverage" | "failed"; attempt: number; @@ -51,6 +60,13 @@ interface MultiscanReceipt extends MultiscanTask { cost?: ScanCost; error?: string; warning?: string; + /** + * Warnings the scan itself raised, distinct from `warning` above. That one + * is derived locally from coverage and its presence flips `status` to + * `completed_with_incomplete_coverage`; these do not, because a repository + * whose target drifted still has complete coverage. + */ + scanWarnings?: MultiscanWarning[]; } export interface MultiscanOptions { @@ -197,6 +213,9 @@ async function runCampaign( let warning: string | undefined; let coverage: CoverageDocument["completeness"] | undefined; let cost: Readonly | null = null; + // Per attempt, so a retry does not inherit the previous attempt's + // warnings alongside its own. + const scanWarnings: MultiscanWarning[] = []; try { await mkdir(dirname(scanDir), { recursive: true, mode: 0o700 }); await rm(checkout, { recursive: true, force: true }); @@ -232,6 +251,18 @@ async function runCampaign( ...(options.postScanPrompt === undefined ? {} : { postScanPrompt: options.postScanPrompt }), + // Without an observer the scan's own warnings are dropped. A + // repository whose target drifted mid-run still completes with + // complete coverage, so it lands in the ledger as "completed" with + // nothing recording that the results describe a tree that moved. + // Redacted on the way in for the same reason failures are: this is + // written to a file. + onWarning: (warning, details) => { + scanWarnings.push({ + message: redactedErrorMessage(warning), + ...(details === undefined ? {} : { kind: details.kind }), + }); + }, ...(options.signal === undefined ? {} : { signal: options.signal }), }); cost = result.cost; @@ -267,6 +298,7 @@ async function runCampaign( ...(cost === null ? {} : { cost }), ...(failure === undefined ? {} : { error: failure }), ...(warning === undefined ? {} : { warning }), + ...(scanWarnings.length === 0 ? {} : { scanWarnings }), })}\n`, ); options.onProgress?.({ diff --git a/sdk/typescript/tests-ts/multiscan.test.ts b/sdk/typescript/tests-ts/multiscan.test.ts index 3ef41a0b..48586fa4 100644 --- a/sdk/typescript/tests-ts/multiscan.test.ts +++ b/sdk/typescript/tests-ts/multiscan.test.ts @@ -1533,4 +1533,99 @@ describe("multiscan", () => { { id: "complete", status: "completed", attempt: 1, coverage: "complete" }, ]); }); + + test("records scan warnings in the receipt for a repository that still completed", async () => { + const paths = await fixture(); + const source = await repository(paths.root, "drifted"); + await writeFile( + paths.input, + `id,repository,revision +drifted,${source.path},${source.revision} +`, + ); + + const summary = await runMultiscan( + options( + paths, + client(async (_target, scanOptions) => { + scanOptions?.onWarning?.("Scan target changed during the run.", { + kind: "target_changed", + }); + return await completedScan(scanOptions?.outputDir ?? paths.output); + }), + { maxAttempts: 1 }, + ), + ); + + expect(summary).toMatchObject({ total: 1, completed: 1, failed: 0 }); + // The repository did complete, so the status is right; without the warning + // beside it the receipt says a drifted tree was scanned cleanly. + expect(await results(summary.resultsPath)).toMatchObject([ + { + id: "drifted", + status: "completed", + scanWarnings: [ + { + message: "Scan target changed during the run.", + kind: "target_changed", + }, + ], + }, + ]); + }); + + test("omits scanWarnings when a repository raised none", async () => { + const paths = await fixture(); + const source = await repository(paths.root, "quiet"); + await writeFile( + paths.input, + `id,repository,revision +quiet,${source.path},${source.revision} +`, + ); + + const summary = await runMultiscan( + options( + paths, + client(async (_target, scanOptions) => + completedScan(scanOptions?.outputDir ?? paths.output), + ), + { maxAttempts: 1 }, + ), + ); + + const [receipt] = await results(summary.resultsPath); + expect(receipt).toMatchObject({ id: "quiet", status: "completed" }); + expect(receipt).not.toHaveProperty("scanWarnings"); + }); + + test("redacts credential-shaped values in recorded scan warnings", async () => { + const paths = await fixture(); + const source = await repository(paths.root, "leaky"); + await writeFile( + paths.input, + `id,repository,revision +leaky,${source.path},${source.revision} +`, + ); + + const summary = await runMultiscan( + options( + paths, + client(async (_target, scanOptions) => { + scanOptions?.onWarning?.( + "Remote rejected token=ghp_0123456789abcdefghijklmnopqrstuvwxyz.", + ); + return await completedScan(scanOptions?.outputDir ?? paths.output); + }), + { maxAttempts: 1 }, + ), + ); + + const [receipt] = await results(summary.resultsPath); + const [warning] = (receipt as { scanWarnings: { message: string }[] }).scanWarnings; + // The ledger is a file on disk, so warnings get the same treatment errors do. + expect(warning.message).not.toContain("ghp_0123456789"); + expect(warning.message).toContain("[redacted]"); + }); });