diff --git a/sdk/typescript/README.md b/sdk/typescript/README.md index 9e1f4a51..71046f8d 100644 --- a/sdk/typescript/README.md +++ b/sdk/typescript/README.md @@ -440,6 +440,10 @@ checkout so a fixed vulnerability can be checked again. cause; `scans match --all` matches all completed scans of the current repository, including other worktrees and clones. Saved matches appear in `scans show` and are reused unless `--force` is passed. Scans without sealed artifacts are skipped. +Automatic history matching loads at most 64 scan pairs per workbench page and +32 findings or 512 KiB per finding page. Each model comparison receives one +bounded page from each scan; cross-page matches are reconciled before the pair +is saved. Match results larger than 1 MiB are rejected with an actionable error. `scans compare BEFORE_SCAN_ID AFTER_SCAN_ID` automatically matches findings by root cause, reuses saved matches, and reports findings as new, persisting, diff --git a/sdk/typescript/_bundled_plugin/references/final-report.md b/sdk/typescript/_bundled_plugin/references/final-report.md index 9e5f555e..755c53f9 100644 --- a/sdk/typescript/_bundled_plugin/references/final-report.md +++ b/sdk/typescript/_bundled_plugin/references/final-report.md @@ -45,6 +45,11 @@ When there are no reportable findings, include a short `No findings` section tha When there are reportable findings, render them as readable markdown findings rather than raw JSON or a dumped schema object. Order findings from highest severity to lowest severity: `critical`, then `high`, then `medium`, then `low`. +`informational` is outside the reportable severity set, so those findings are not +detailed in the report. They are still sealed into `findings.json` and included in +the SARIF and CSV exports, so the report records how many were held back rather +than omitting them silently. + Use a separate finding entry for each independently attackable source/control/sink instance. Do not combine sibling routes, templates, query builders, parser operations, auth/object-access endpoints, or shared-helper callers into one representative finding solely for readability; if grouping helps, add a short grouped summary after the individual finding entries. If validation or attack-path analysis provides a broad family row with multiple independently triggerable sink, parser, helper, API-mode, or protected-action lines, split it into child final findings before writing the report. Multiple affected lines inside one finding are appropriate for one inseparable proof tuple, such as a wrapper plus its shared sink, but not as a substitute for separate findings when sibling operations can be triggered independently. diff --git a/sdk/typescript/_bundled_plugin/scripts/report_projection.py b/sdk/typescript/_bundled_plugin/scripts/report_projection.py index e1e9f45c..5578bae7 100644 --- a/sdk/typescript/_bundled_plugin/scripts/report_projection.py +++ b/sdk/typescript/_bundled_plugin/scripts/report_projection.py @@ -363,6 +363,23 @@ def _code_evidence_lines(evidence: list[dict[str, Any]]) -> list[str]: return lines +def _unreported_findings_note(findings: list[dict[str, Any]]) -> str: + """Describe findings held back from this report by the reportable-severity gate. + + They remain sealed in findings.json and exported to SARIF and CSV, so without + this line the three artifacts of one scan contradict each other and the one a + human reads is the one missing data. + """ + count = len(findings) + subject = "finding is" if count == 1 else "findings are" + pronoun = "It remains" if count == 1 else "They remain" + return ( + f"{count} {subject} outside the reportable severity set " + f"({_severity_mix(findings)}) and not detailed here. " + f"{pronoun} recorded in `findings.json` and in the SARIF and CSV exports." + ) + + def _severity_mix(findings: list[dict[str, Any]]) -> str: counts = Counter(finding["severity"]["level"] for finding in findings) return ( @@ -588,6 +605,11 @@ def build_report_markdown( ), key=_finding_sort_key, ) + unreported = [ + finding + for finding in findings_document["findings"] + if finding["severity"]["level"] not in REPORTABLE_SEVERITIES + ] writeup_paths = [_writeup_report_path(finding) for finding in findings] duplicate_writeup_paths = sorted( path @@ -719,6 +741,8 @@ def build_report_markdown( f"| {finding_link} | {finding['severity']['level']} " f"| {finding['confidence']['level']} | {writeup_link} |" ) + if unreported: + lines.extend(["", _unreported_findings_note(unreported)]) lines.extend( [ "", @@ -746,6 +770,8 @@ def build_report_markdown( "No reportable findings survived the canonical discovery, validation, and reportability gates.", ] ) + if unreported: + lines.extend(["", _unreported_findings_note(unreported)]) if hardening_portfolio_path is not None: lines.extend( [ diff --git a/sdk/typescript/_bundled_plugin/scripts/workbench_cli.py b/sdk/typescript/_bundled_plugin/scripts/workbench_cli.py index 8e0c38e9..38a75e92 100644 --- a/sdk/typescript/_bundled_plugin/scripts/workbench_cli.py +++ b/sdk/typescript/_bundled_plugin/scripts/workbench_cli.py @@ -152,6 +152,12 @@ def parse_args(description: str) -> argparse.Namespace: list_unmatched_scan_pairs = subparsers.add_parser("list-unmatched-scan-pairs") list_unmatched_scan_pairs.add_argument("--repository", required=True) list_unmatched_scan_pairs.add_argument("--force", action="store_true") + list_unmatched_scan_pairs.add_argument("--offset", type=non_negative_int, default=0) + list_unmatched_scan_pairs.add_argument("--completed-before") + + get_scan_matching_inputs = subparsers.add_parser("get-scan-matching-inputs") + get_scan_matching_inputs.add_argument("--scan-id", required=True) + get_scan_matching_inputs.add_argument("--offset", type=non_negative_int, default=0) register_cli_scan = subparsers.add_parser("register-cli-scan") register_cli_scan.add_argument("--scan-dir", required=True) @@ -168,12 +174,15 @@ def parse_args(description: str) -> argparse.Namespace: compare_scans.add_argument("--before-scan-id", required=True) compare_scans.add_argument("--after-scan-id", required=True) compare_scans.add_argument("--include-matching-inputs", action="store_true") + compare_scans.add_argument("--include-matching-status", action="store_true") compare_scans.add_argument("--require-matches", action="store_true") save_scan_comparison = subparsers.add_parser("save-scan-comparison") save_scan_comparison.add_argument("--before-scan-id", required=True) save_scan_comparison.add_argument("--after-scan-id", required=True) - save_scan_comparison.add_argument("--matches-json", required=True) + matches = save_scan_comparison.add_mutually_exclusive_group(required=True) + matches.add_argument("--matches-json") + matches.add_argument("--matches-file") list_global_findings = subparsers.add_parser("list-global-findings") list_global_findings.add_argument("--query") diff --git a/sdk/typescript/_bundled_plugin/scripts/workbench_constants.py b/sdk/typescript/_bundled_plugin/scripts/workbench_constants.py index 4ddd91f3..08b5e704 100644 --- a/sdk/typescript/_bundled_plugin/scripts/workbench_constants.py +++ b/sdk/typescript/_bundled_plugin/scripts/workbench_constants.py @@ -42,6 +42,9 @@ PATCH_ARTIFACT_MAX_BYTES = 2 * 1024 * 1024 FINDINGS_RESULT_LIMIT = 20 FINDINGS_PAGE_MAX = 20 +MATCHING_PAIR_PAGE_MAX = 64 +MATCHING_FINDING_PAGE_MAX = 32 +MATCHING_INPUT_PAGE_BYTES = 512 * 1024 FINDING_DETAILS_PREVIEW_BYTES = 16_000 FINDING_ROOT_CAUSE_PREVIEW_BYTES = 2_000 FINDING_VALIDATION_PREVIEW_BYTES = 3_000 diff --git a/sdk/typescript/_bundled_plugin/scripts/workbench_db.py b/sdk/typescript/_bundled_plugin/scripts/workbench_db.py index ebee840c..326b3d23 100644 --- a/sdk/typescript/_bundled_plugin/scripts/workbench_db.py +++ b/sdk/typescript/_bundled_plugin/scripts/workbench_db.py @@ -3568,9 +3568,16 @@ def main() -> None: result = scan_history.list_unmatched_scan_pairs( connection, args, - backfill_finding_details=backfill_legacy_finding_details, read_coverage=coverage_for_comparison, ) + elif args.command == "get-scan-matching-inputs": + result = scan_history.get_scan_matching_inputs( + connection, + args, + require_scan=require_scan, + read_coverage=coverage_for_comparison, + backfill_finding_details=backfill_legacy_finding_details, + ) elif args.command == "register-cli-scan": result = register_cli_scan(connection, args) elif args.command == "get-scan-recipe": @@ -3583,6 +3590,7 @@ def main() -> None: read_coverage=coverage_for_comparison, backfill_finding_details=backfill_legacy_finding_details, include_matching_inputs=args.include_matching_inputs, + include_matching_status=args.include_matching_status, require_matches=args.require_matches, ) elif args.command == "save-scan-comparison": diff --git a/sdk/typescript/_bundled_plugin/scripts/workbench_scan_history.py b/sdk/typescript/_bundled_plugin/scripts/workbench_scan_history.py index c92f1490..80e6c7ce 100644 --- a/sdk/typescript/_bundled_plugin/scripts/workbench_scan_history.py +++ b/sdk/typescript/_bundled_plugin/scripts/workbench_scan_history.py @@ -3,6 +3,7 @@ import argparse import fnmatch import json +import math import os import sqlite3 from pathlib import Path, PurePosixPath @@ -10,7 +11,12 @@ from urllib.parse import urlsplit from report_projection import SEVERITY_ORDER -from workbench_constants import FINDINGS_PAGE_MAX +from workbench_constants import ( + FINDINGS_PAGE_MAX, + MATCHING_FINDING_PAGE_MAX, + MATCHING_INPUT_PAGE_BYTES, + MATCHING_PAIR_PAGE_MAX, +) from workbench_target import git_output @@ -271,7 +277,6 @@ def list_unmatched_scan_pairs( connection: sqlite3.Connection, args: argparse.Namespace, *, - backfill_finding_details: Callable[[sqlite3.Connection, sqlite3.Row], None], read_coverage: Callable[[sqlite3.Row], dict[str, Any]], ) -> dict[str, Any]: repository = Path(args.repository).expanduser().resolve() @@ -282,11 +287,15 @@ def list_unmatched_scan_pairs( """, (str(repository), str(repository)), ).fetchone() + completed = connection.execute( + "SELECT * FROM scans WHERE status = 'complete'" + + (" AND completed_at <= ?" if args.completed_before is not None else "") + + " ORDER BY started_at, id", + (() if args.completed_before is None else (args.completed_before,)), + ) selected = [ scan - for scan in connection.execute( - "SELECT * FROM scans WHERE status = 'complete' ORDER BY started_at, id" - ) + for scan in completed if Path(scan["target_path"]).resolve() == repository or _same_repository(scan, requested) ] @@ -302,42 +311,41 @@ def list_unmatched_scan_pairs( (row["before_scan_id"], row["after_scan_id"]) for row in connection.execute("SELECT before_scan_id, after_scan_id FROM scan_comparisons") } - batches = [] - skipped = 0 - backfilled: set[str] = set() - for index, after in enumerate(available): - previous = [ - before - for before in available[:index] - if args.force or (before["id"], after["id"]) not in saved_pairs - ] - skipped += index - len(previous) - if not previous: - continue - for scan in (*previous, after): - if scan["id"] not in backfilled: - backfill_finding_details(connection, scan) - backfilled.add(scan["id"]) - batches.append( - { - "afterFindings": [ - _matching_input(row) for row in _scan_findings(connection, after["id"]).values() - ], - "afterScanId": after["id"], - "beforeScans": [ - { - "findings": [ - _matching_input(row) - for row in _scan_findings(connection, before["id"]).values() - ], - "scanId": before["id"], - } - for before in previous - ], - } + available_indexes = {scan["id"]: index for index, scan in enumerate(available)} + skipped = ( + 0 + if args.force + else sum( + 1 + for before_id, after_id in saved_pairs + if before_id in available_indexes + and after_id in available_indexes + and available_indexes[before_id] < available_indexes[after_id] ) + ) + pair_count = len(available) * (len(available) - 1) // 2 + pair_index = min(args.offset, pair_count) + after_index = (1 + math.isqrt(1 + 8 * pair_index)) // 2 + before_index = pair_index - after_index * (after_index - 1) // 2 + pairs = [] + while pair_index < pair_count and len(pairs) < MATCHING_PAIR_PAGE_MAX: + before = available[before_index] + after = available[after_index] + if args.force or (before["id"], after["id"]) not in saved_pairs: + pairs.append( + { + "afterScanId": after["id"], + "beforeScanId": before["id"], + } + ) + pair_index += 1 + before_index += 1 + if before_index == after_index: + after_index += 1 + before_index = 0 return { - "batches": batches, + "nextOffset": pair_index if pair_index < pair_count else None, + "pairs": pairs, "repository": str(repository), "scanCount": len(selected), "skippedPairs": skipped, @@ -345,6 +353,76 @@ def list_unmatched_scan_pairs( } +def get_scan_matching_inputs( + connection: sqlite3.Connection, + args: argparse.Namespace, + *, + require_scan: Callable[[sqlite3.Connection, str], sqlite3.Row], + read_coverage: Callable[[sqlite3.Row], dict[str, Any]], + backfill_finding_details: Callable[[sqlite3.Connection, sqlite3.Row], None], +) -> dict[str, Any]: + scan = require_scan(connection, args.scan_id) + if scan["status"] != "complete": + raise SystemExit("Only completed scans can be matched.") + read_coverage(scan) + backfill_finding_details(connection, scan) + if connection.execute( + """ + SELECT 1 + FROM finding_occurrences + WHERE scan_id = ? AND length(CAST(details_json AS BLOB)) > ? + LIMIT 1 + """, + (scan["id"], MATCHING_INPUT_PAGE_BYTES), + ).fetchone(): + raise SystemExit( + "A scan finding exceeds the 512 KiB automatic matching input limit." + ) + total_findings = connection.execute( + "SELECT COUNT(*) FROM finding_occurrences WHERE scan_id = ?", + (scan["id"],), + ).fetchone()[0] + rows = _scan_finding_page( + connection, + scan["id"], + offset=args.offset, + limit=MATCHING_FINDING_PAGE_MAX, + ) + findings = [] + encoded_bytes = 2 + next_offset = min(args.offset, total_findings) + for row in rows: + finding = _matching_input(row) + item_bytes = len( + json.dumps( + finding, + allow_nan=False, + ensure_ascii=False, + separators=(",", ":"), + ).encode("utf-8") + ) + if item_bytes > MATCHING_INPUT_PAGE_BYTES: + raise SystemExit( + "A scan finding exceeds the 512 KiB automatic matching input limit." + ) + separator_bytes = 1 if findings else 0 + if encoded_bytes + separator_bytes + item_bytes > MATCHING_INPUT_PAGE_BYTES: + if not findings: + raise SystemExit( + "A scan finding exceeds the 512 KiB automatic matching input limit." + ) + break + findings.append(finding) + encoded_bytes += separator_bytes + item_bytes + next_offset += 1 + return { + "findings": findings, + "nextOffset": next_offset if next_offset < total_findings else None, + "scanId": scan["id"], + "totalFindings": total_findings, + } + + def compare_scans( connection: sqlite3.Connection, args: argparse.Namespace, @@ -353,6 +431,7 @@ def compare_scans( read_coverage: Callable[[sqlite3.Row], dict[str, Any]], backfill_finding_details: Callable[[sqlite3.Connection, sqlite3.Row], None] | None = None, include_matching_inputs: bool = False, + include_matching_status: bool = False, require_matches: bool = False, ) -> dict[str, Any]: before = require_scan(connection, args.before_scan_id) @@ -478,8 +557,9 @@ def compare_scans( "repository": before["target_path"], "summary": summary, } - if include_matching_inputs: + if include_matching_inputs or include_matching_status: result["matchingCached"] = cached is not None + if include_matching_inputs: result["matchingInputs"] = { "before": [_matching_input(row) for row in before_findings.values()], "after": [_matching_input(row) for row in after_findings.values()], @@ -507,8 +587,15 @@ def save_scan_comparison( before_findings = _scan_findings(connection, before["id"]) after_findings = _scan_findings(connection, after["id"]) try: - payload = json.loads(args.matches_json) - except (TypeError, ValueError) as exc: + if args.matches_file is None: + serialized = args.matches_json + else: + with open(args.matches_file, "rb") as matches: + serialized = matches.read(1024 * 1024 + 1) + if len(serialized) > 1024 * 1024: + raise ValueError("Scan comparison matches exceed the 1 MiB safety limit.") + payload = json.loads(serialized) + except (OSError, TypeError, ValueError) as exc: raise SystemExit("Scan comparison matches must be a valid JSON object.") from exc if not isinstance(payload, dict) or set(payload) != {"matches", "uncertain"}: raise SystemExit("Scan comparison matches must contain matches and uncertain arrays.") @@ -781,12 +868,44 @@ def _scan_findings(connection: sqlite3.Connection, scan_id: str) -> dict[str, sq FROM finding_occurrences AS occurrences LEFT JOIN finding_triage AS triage ON triage.occurrence_id = occurrences.id WHERE occurrences.scan_id = ? + ORDER BY occurrences.id """, (scan_id,), ) return {row["finding_id"]: row for row in rows} +def _scan_finding_page( + connection: sqlite3.Connection, + scan_id: str, + *, + offset: int, + limit: int, +) -> list[sqlite3.Row]: + return list( + connection.execute( + """ + SELECT occurrences.*, + COALESCE(triage.status, 'open') AS triage_status, triage.close_reason, + ( + SELECT locations.relative_path + FROM finding_locations AS locations + WHERE locations.occurrence_id = occurrences.id + ORDER BY CASE WHEN locations.role = 'root_control' THEN 0 ELSE 1 END, + locations.sort_order + LIMIT 1 + ) AS relative_path + FROM finding_occurrences AS occurrences + LEFT JOIN finding_triage AS triage ON triage.occurrence_id = occurrences.id + WHERE occurrences.scan_id = ? + ORDER BY occurrences.id + LIMIT ? OFFSET ? + """, + (scan_id, limit, offset), + ) + ) + + def scan_covers_path( scan: sqlite3.Row, *, diff --git a/sdk/typescript/src/cli.ts b/sdk/typescript/src/cli.ts index 1abe8302..d70f3fd7 100644 --- a/sdk/typescript/src/cli.ts +++ b/sdk/typescript/src/cli.ts @@ -9,7 +9,16 @@ import { realpathSync, writeSync, } from "node:fs"; -import { mkdir, readFile, realpath, stat, writeFile } from "node:fs/promises"; +import { + mkdir, + mkdtemp, + readFile, + realpath, + rm, + stat, + writeFile, +} from "node:fs/promises"; +import { tmpdir } from "node:os"; import { basename, dirname, @@ -77,6 +86,7 @@ import { import { matchScanFindings, type ScanComparisonInput, + type ScanComparisonResult, } from "./scan-comparison.js"; import { renderScanHistory, @@ -230,20 +240,33 @@ interface ExportArguments { pythonPath?: string; } -interface MatchingBatch { +interface MatchingPair { afterScanId: string; - afterFindings: ScanComparisonInput["after"]; - beforeScans: { scanId: string; findings: ScanComparisonInput["before"] }[]; + beforeScanId: string; } -type MatchingPlan = JsonObject & { +type MatchingPlanPage = JsonObject & { + nextOffset: number | null; + pairs: (JsonObject & MatchingPair)[]; repository: string; scanCount: number; unavailableScans: number; skippedPairs: number; - batches: (JsonObject & MatchingBatch)[]; }; +type MatchingInputPage = JsonObject & { + findings: ScanComparisonInput["before"]; + nextOffset: number | null; + scanId: string; + totalFindings: number; +}; + +const MAX_MATCH_RESULT_BYTES = 1024 * 1024; +const MAX_INLINE_MATCH_RESULT_BYTES = 16 * 1024; +const MAX_GROUPED_MATCH_INPUT_BYTES = 512 * 1024; +const MAX_GROUPED_MATCH_PAIRS = 1024; +const MAX_GROUPED_MATCH_INPUT_CONCURRENCY = 8; + interface SkillCommandOutput { readonly command: "validate" | "patch"; readonly stdout: Writable; @@ -623,23 +646,22 @@ export async function main( beforeId, "--after-scan-id", afterId, - "--include-matching-inputs", + "--include-matching-status", ], async ({ matchingCached, matchingInputs, ...comparison }) => { - if (matchingCached && !force) return comparison; - return await dependencies.runWorkbench([ - "save-scan-comparison", - "--before-scan-id", + if (matchingCached !== false && !force) return comparison; + const matching = + matchingInputs === undefined + ? await matchScanPairInBatches(dependencies, beforeId, afterId) + : await dependencies.matchFindings( + matchingInputs as JsonObject & ScanComparisonInput, + ); + return await saveScanComparison( + dependencies, beforeId, - "--after-scan-id", afterId, - "--matches-json", - JSON.stringify( - await dependencies.matchFindings( - matchingInputs as JsonObject & ScanComparisonInput, - ), - ), - ]); + matching, + ); }, ); const presentHistory = ( @@ -857,7 +879,9 @@ export async function main( try { if (options.all) { return presentHistory( - await matchAllScans(dependencies, options.force), + await matchAllScans(dependencies, options.force, (warning) => { + errorOutput.write(`codex-security: warning: ${warning}\n`); + }), "match-all", format, ); @@ -1959,74 +1983,101 @@ function validateCliArguments( async function matchAllScans( dependencies: CliDependencies, force: boolean, + onWarning?: (warning: string) => void, ): Promise { - const result = (await dependencies.runWorkbench([ - "list-unmatched-scan-pairs", - "--repository", - dependencies.currentDirectory(), - ...(force ? ["--force"] : []), - ])) as MatchingPlan; - const { repository, scanCount, unavailableScans, skippedPairs, batches } = - result; - + let repository = dependencies.currentDirectory(); + let scanCount = 0; + let unavailableScans = 0; + let skippedPairs = 0; let matchedPairs = 0; let findingMatches = 0; - for (const { afterScanId, afterFindings, beforeScans } of batches) { - const before = beforeScans.flatMap(({ findings }) => findings); - const matching = - before.length === 0 || afterFindings.length === 0 - ? { matches: [], uncertain: [] } - : await dependencies.matchFindings( - { before, after: afterFindings }, - { allowHistoricalUncertainty: true }, - ); - const comparisons = beforeScans.map(({ scanId, findings }) => { - const beforeIds = new Set( - findings.map(({ occurrenceId }) => occurrenceId), - ); - const matches = matching.matches.flatMap((match) => { - const beforeOccurrenceIds = match.beforeOccurrenceIds.filter((id) => - beforeIds.has(id), + let unmatchedBatches = 0; + let firstFailure: unknown; + let offset = 0; + let firstPage = true; + const completedBefore = new Date().toISOString(); + let pendingPairs: MatchingPair[] = []; + const matchPendingPairs = async (): Promise => { + if (pendingPairs.length === 0) return; + const group = pendingPairs; + pendingPairs = []; + const afterScanId = group[0]!.afterScanId; + const groupedMatching = + group.length > 1 + ? matchScanGroupInOneBatch(dependencies, group, afterScanId).catch( + () => null, + ) + : undefined; + for (const { beforeScanId } of group) { + try { + const matching = + (await groupedMatching)?.get(beforeScanId) ?? + (await matchScanPairInBatches( + dependencies, + beforeScanId, + afterScanId, + )); + await saveScanComparison( + dependencies, + beforeScanId, + afterScanId, + matching, ); - return beforeOccurrenceIds.length === 0 - ? [] - : [{ ...match, beforeOccurrenceIds }]; - }); - const uncertain = matching.uncertain.filter(({ beforeOccurrenceId }) => - beforeIds.has(beforeOccurrenceId), - ); - const matchedAfter = new Set( - matches.flatMap(({ afterOccurrenceIds }) => afterOccurrenceIds), - ); + matchedPairs += 1; + findingMatches += matching.matches.reduce( + (count, { beforeOccurrenceIds, afterOccurrenceIds }) => + count + beforeOccurrenceIds.length * afterOccurrenceIds.length, + 0, + ); + } catch (error) { + unmatchedBatches += 1; + firstFailure ??= error; + onWarning?.( + `Could not match findings against scan ${afterScanId}: ${cliErrorMessage(error)}`, + ); + } + } + }; + while (true) { + const page = (await dependencies.runWorkbench([ + "list-unmatched-scan-pairs", + "--repository", + dependencies.currentDirectory(), + "--offset", + String(offset), + "--completed-before", + completedBefore, + ...(force ? ["--force"] : []), + ])) as MatchingPlanPage; + if (firstPage) { + ({ repository, scanCount, unavailableScans, skippedPairs } = page); + firstPage = false; + } + for (const pair of page.pairs) { if ( - uncertain.some(({ afterOccurrenceId }) => - matchedAfter.has(afterOccurrenceId), - ) + pendingPairs.length > 0 && + pendingPairs[0]!.afterScanId !== pair.afterScanId ) { - throw new CodexSecurityError( - "Scan matching returned conflicting confirmed and uncertain findings.", - ); + await matchPendingPairs(); } - return { scanId, matches, uncertain }; - }); - for (const { scanId, matches, uncertain } of comparisons) { - await dependencies.runWorkbench([ - "save-scan-comparison", - "--before-scan-id", - scanId, - "--after-scan-id", - afterScanId, - "--matches-json", - JSON.stringify({ matches, uncertain }), - ]); - matchedPairs += 1; - findingMatches += matches.reduce( - (count, { beforeOccurrenceIds, afterOccurrenceIds }) => - count + beforeOccurrenceIds.length * afterOccurrenceIds.length, - 0, + pendingPairs.push(pair); + if (pendingPairs.length >= MAX_GROUPED_MATCH_PAIRS) { + await matchPendingPairs(); + } + } + if (page.nextOffset === null) break; + if (!Number.isSafeInteger(page.nextOffset) || page.nextOffset <= offset) { + throw new CodexSecurityError( + "Scan matching returned an invalid pagination cursor.", ); } + offset = page.nextOffset; } + await matchPendingPairs(); + // A failure that left nothing matched is reported rather than presented as a + // successful no-op, so an unwritable workbench or a rejected credential still + // surfaces instead of being reduced to a warning. + if (matchedPairs === 0 && firstFailure !== undefined) throw firstFailure; return { repository, scanCount, @@ -2034,6 +2085,317 @@ async function matchAllScans( matchedPairs, skippedPairs, findingMatches, + ...(unmatchedBatches === 0 ? {} : { unmatchedBatches }), + }; +} + +async function matchScanGroupInOneBatch( + dependencies: CliDependencies, + pairs: readonly MatchingPair[], + afterScanId: string, +): Promise | null> { + const after = await matchingInputPage(dependencies, afterScanId, 0); + const previous: Array<{ + scanId: string; + page: Awaited>; + }> = new Array(pairs.length); + let nextPair = 0; + let failed = false; + let failure: unknown; + await Promise.all( + Array.from( + { length: Math.min(pairs.length, MAX_GROUPED_MATCH_INPUT_CONCURRENCY) }, + async () => { + while (!failed && nextPair < pairs.length) { + const index = nextPair++; + const beforeScanId = pairs[index]!.beforeScanId; + try { + previous[index] = { + scanId: beforeScanId, + page: await matchingInputPage(dependencies, beforeScanId, 0), + }; + } catch (error) { + failed = true; + failure = error; + } + } + }, + ), + ); + if (failed) throw failure; + if ( + after.nextOffset !== null || + previous.some(({ page }) => page.nextOffset !== null) + ) { + return null; + } + const input = { + before: previous.flatMap(({ page }) => page.findings), + after: after.findings, + }; + if ( + Buffer.byteLength(JSON.stringify(input)) > MAX_GROUPED_MATCH_INPUT_BYTES + ) { + return null; + } + const matching = + input.before.length === 0 || input.after.length === 0 + ? { matches: [], uncertain: [] } + : await dependencies.matchFindings(input, { + allowHistoricalUncertainty: true, + }); + if (Buffer.byteLength(JSON.stringify(matching)) > MAX_MATCH_RESULT_BYTES) { + throw oversizedAutomaticMatchError(); + } + return new Map( + previous.map(({ scanId, page }) => { + const occurrenceIds = new Set( + page.findings.map(({ occurrenceId }) => occurrenceId), + ); + const confirmed = matching.matches + .map((match) => ({ + ...match, + beforeOccurrenceIds: match.beforeOccurrenceIds.filter((id) => + occurrenceIds.has(id), + ), + })) + .filter(({ beforeOccurrenceIds }) => beforeOccurrenceIds.length > 0); + const uncertain = matching.uncertain.filter(({ beforeOccurrenceId }) => + occurrenceIds.has(beforeOccurrenceId), + ); + return [scanId, reconcileMatchingBatches(confirmed, uncertain)]; + }), + ); +} + +async function saveScanComparison( + dependencies: CliDependencies, + beforeScanId: string, + afterScanId: string, + matching: ScanComparisonResult, +): Promise { + const serialized = JSON.stringify(matching); + const bytes = Buffer.byteLength(serialized); + if (bytes > MAX_MATCH_RESULT_BYTES) throw oversizedAutomaticMatchError(); + + const arguments_ = [ + "save-scan-comparison", + "--before-scan-id", + beforeScanId, + "--after-scan-id", + afterScanId, + ]; + if (bytes <= MAX_INLINE_MATCH_RESULT_BYTES) { + return await dependencies.runWorkbench([ + ...arguments_, + "--matches-json", + serialized, + ]); + } + + const temporary = await mkdtemp(join(tmpdir(), "codex-security-matches-")); + const path = join(temporary, "matches.json"); + try { + await writeFile(path, serialized, { mode: 0o600, flag: "wx" }); + return await dependencies.runWorkbench([ + ...arguments_, + "--matches-file", + path, + ]); + } finally { + await rm(temporary, { force: true, recursive: true }); + } +} + +async function matchScanPairInBatches( + dependencies: CliDependencies, + beforeScanId: string, + afterScanId: string, +): Promise { + let beforePage = await matchingInputPage(dependencies, beforeScanId, 0); + const firstAfterPage = await matchingInputPage(dependencies, afterScanId, 0); + if ( + beforePage.findings.length === 0 || + firstAfterPage.findings.length === 0 + ) { + return { matches: [], uncertain: [] }; + } + + const confirmed: ScanComparisonResult["matches"] = []; + const uncertain = new Map< + string, + ScanComparisonResult["uncertain"][number] + >(); + while (true) { + let afterPage = firstAfterPage; + while (true) { + const result = await dependencies.matchFindings( + { + before: beforePage.findings, + after: afterPage.findings, + }, + { allowHistoricalUncertainty: true }, + ); + confirmed.push(...result.matches); + for (const candidate of result.uncertain) { + const key = JSON.stringify([ + candidate.beforeOccurrenceId, + candidate.afterOccurrenceId, + ]); + const existing = uncertain.get(key); + if (existing === undefined || candidate.reason < existing.reason) { + uncertain.set(key, candidate); + } + } + const reconciled = reconcileMatchingBatches(confirmed, [ + ...uncertain.values(), + ]); + if ( + Buffer.byteLength(JSON.stringify(reconciled)) > MAX_MATCH_RESULT_BYTES + ) { + throw oversizedAutomaticMatchError(); + } + confirmed.splice(0, confirmed.length, ...reconciled.matches); + if (afterPage.nextOffset === null) break; + afterPage = await matchingInputPage( + dependencies, + afterScanId, + afterPage.nextOffset, + ); + } + if (beforePage.nextOffset === null) break; + beforePage = await matchingInputPage( + dependencies, + beforeScanId, + beforePage.nextOffset, + ); + } + return reconcileMatchingBatches(confirmed, [...uncertain.values()]); +} + +function oversizedAutomaticMatchError(): CodexSecurityError { + return new CodexSecurityError( + "Automatic scan matching produced more than 1 MiB of match data. Review this scan pair manually.", + ); +} + +async function matchingInputPage( + dependencies: CliDependencies, + scanId: string, + offset: number, +): Promise { + const page = (await dependencies.runWorkbench([ + "get-scan-matching-inputs", + "--scan-id", + scanId, + "--offset", + String(offset), + ])) as MatchingInputPage; + if ( + page.scanId !== scanId || + !Array.isArray(page.findings) || + (page.nextOffset !== null && + (!Number.isSafeInteger(page.nextOffset) || page.nextOffset <= offset)) + ) { + throw new CodexSecurityError( + "Scan matching returned an invalid finding page.", + ); + } + return page; +} + +function reconcileMatchingBatches( + confirmed: ScanComparisonResult["matches"], + uncertain: ScanComparisonResult["uncertain"], +): ScanComparisonResult { + const parents = new Map(); + const find = (value: string): string => { + const parent = parents.get(value); + if (parent === undefined) { + parents.set(value, value); + return value; + } + if (parent === value) return value; + const root = find(parent); + parents.set(value, root); + return root; + }; + const union = (left: string, right: string): void => { + const leftRoot = find(left); + const rightRoot = find(right); + if (leftRoot !== rightRoot) { + parents.set( + leftRoot < rightRoot ? rightRoot : leftRoot, + leftRoot < rightRoot ? leftRoot : rightRoot, + ); + } + }; + for (const match of confirmed) { + const nodes = [ + ...match.beforeOccurrenceIds.map((id) => `before:${id}`), + ...match.afterOccurrenceIds.map((id) => `after:${id}`), + ]; + for (const node of nodes.slice(1)) union(nodes[0]!, node); + } + + const groups = new Map< + string, + { + before: Set; + after: Set; + reasons: Set; + } + >(); + for (const match of confirmed) { + const root = find(`before:${match.beforeOccurrenceIds[0]!}`); + const group = groups.get(root) ?? { + before: new Set(), + after: new Set(), + reasons: new Set(), + }; + match.beforeOccurrenceIds.forEach((id) => group.before.add(id)); + match.afterOccurrenceIds.forEach((id) => group.after.add(id)); + group.reasons.add(match.reason); + groups.set(root, group); + } + const matches = [...groups.values()] + .map(({ before, after, reasons }) => ({ + beforeOccurrenceIds: [...before].sort(), + afterOccurrenceIds: [...after].sort(), + confidence: "high" as const, + reason: [...reasons].sort()[0]!, + })) + .sort( + (left, right) => + left.beforeOccurrenceIds[0]!.localeCompare( + right.beforeOccurrenceIds[0]!, + ) || + left.afterOccurrenceIds[0]!.localeCompare(right.afterOccurrenceIds[0]!), + ); + const matchedBefore = new Set( + matches.flatMap(({ beforeOccurrenceIds }) => beforeOccurrenceIds), + ); + const matchedAfter = new Set( + matches.flatMap(({ afterOccurrenceIds }) => afterOccurrenceIds), + ); + if ( + uncertain.some( + ({ beforeOccurrenceId, afterOccurrenceId }) => + matchedBefore.has(beforeOccurrenceId) || + matchedAfter.has(afterOccurrenceId), + ) + ) { + throw new CodexSecurityError( + "Scan matching returned conflicting confirmed and uncertain findings.", + ); + } + return { + matches, + uncertain: uncertain.sort( + (left, right) => + left.beforeOccurrenceId.localeCompare(right.beforeOccurrenceId) || + left.afterOccurrenceId.localeCompare(right.afterOccurrenceId), + ), }; } diff --git a/sdk/typescript/src/scan-history-renderer.ts b/sdk/typescript/src/scan-history-renderer.ts index 1c4e79a0..5ba61e22 100644 --- a/sdk/typescript/src/scan-history-renderer.ts +++ b/sdk/typescript/src/scan-history-renderer.ts @@ -30,6 +30,20 @@ const SEVERITY_COLORS: Record = { INFORMATIONAL: 37, }; +const SEVERITY_ORDER = Object.keys(SEVERITY_COLORS); +const MAX_KNOWN_SINCE_LENGTH = 32; +// Severity badges occupy a fixed column, so the widest label decides the width +// and the finding indent that lines up under it. INFORMATIONAL is abbreviated to +// keep that column narrow; spelling it out costs every finding title five +// characters, which matters most at the 48-column minimum. +const SEVERITY_LABELS: Record = { INFORMATIONAL: "INFO" }; +const SEVERITY_BADGE_WIDTH = Math.max( + ...Object.keys(SEVERITY_COLORS).map( + (severity) => (SEVERITY_LABELS[severity] ?? severity).length, + ), +); +const FINDING_INDENT = 4 + SEVERITY_BADGE_WIDTH + 2; + const KNOWN_SINCE_DATE = new Intl.DateTimeFormat("en-US", { month: "short", day: "numeric", @@ -43,11 +57,47 @@ function clean(value: unknown): string { .replace(/[\u0000-\u001F\u007F-\u009F]/g, " "); } +// The workbench response is only checked for being a JSON object before it +// reaches this renderer, so every field read here is treated as optional. A +// history view degrades rather than aborting the command on a payload the +// installed plugin did not produce. +function record(value: unknown): JsonObject { + return typeof value === "object" && value !== null && !Array.isArray(value) + ? (value as JsonObject) + : {}; +} + +function records(value: unknown): JsonObject[] { + return Array.isArray(value) + ? value.filter( + (entry): entry is JsonObject => + typeof entry === "object" && entry !== null && !Array.isArray(entry), + ) + : []; +} + +function text(value: unknown, fallback = ""): string { + return value === undefined || value === null ? fallback : clean(value); +} + function findingSeverity(finding: JsonObject): string { const severity = finding["severity"]; - return clean( - typeof severity === "string" ? severity : (severity as JsonObject)["level"], - ).toUpperCase(); + const level = + typeof severity === "string" ? severity : record(severity)["level"]; + return text(level).toUpperCase(); +} + +function severityRank(finding: JsonObject): number { + const index = SEVERITY_ORDER.indexOf(findingSeverity(finding)); + return index < 0 ? Number.MAX_SAFE_INTEGER : index; +} + +function knownSinceLabel(value: unknown): string { + const raw = clean(value); + const parsed = Date.parse(raw); + return Number.isFinite(parsed) + ? KNOWN_SINCE_DATE.format(parsed) + : raw.slice(0, MAX_KNOWN_SINCE_LENGTH); } export function renderScanHistory( @@ -95,59 +145,70 @@ export function renderScanHistory( const finding = (entry: JsonObject, includeReason = true): void => { const severity = findingSeverity(entry); const title = clean(entry["title"]); - const badge = paint(severity.padEnd(8), SEVERITY_COLORS[severity] ?? 37); - wrap(title, 14, ` ${badge} `); + const badge = paint( + (SEVERITY_LABELS[severity] ?? severity) + .slice(0, SEVERITY_BADGE_WIDTH) + .padEnd(SEVERITY_BADGE_WIDTH), + SEVERITY_COLORS[severity] ?? 37, + ); + wrap(title, FINDING_INDENT, ` ${badge} `); const before = entry["beforeOccurrenceIds"] as string[] | undefined; const after = entry["afterOccurrenceIds"] as string[] | undefined; const grouped = before?.length || after?.length ? ` ${accent("·")} ${before?.length ?? 1} → ${after?.length ?? 1}` : ""; - const matches = entry["matches"] as JsonObject[] | undefined; + const matches = records(entry["matches"]); const knownScanIds = entry["knownScanIds"] as string[] | undefined; const knownScans = knownScanIds?.length ? ` in ${clean(knownScanIds[0]).slice(0, 8)}${knownScanIds.length > 1 ? ` … ${clean(knownScanIds[knownScanIds.length - 1]).slice(0, 8)}` : ""}` : ""; const knownSince = - command === "show" && matches?.length && entry["knownSince"] - ? ` ${accent("·")} ${strong(`Known since ${KNOWN_SINCE_DATE.format(new Date(clean(entry["knownSince"])))}`)}${knownScans}` + command === "show" && matches.length && entry["knownSince"] + ? ` ${accent("·")} ${strong(`Known since ${knownSinceLabel(entry["knownSince"])}`)}${knownScans}` : ""; - const location = (entry["locations"] as JsonObject[] | undefined)?.[0]; + const location = records(entry["locations"])[0]; const path = entry["path"] ?? `${location?.["path"]}${location?.["startLine"] ? `:${location["startLine"]}` : ""}`; - lines.push(` ${dim(clean(path))}${grouped}${knownSince}`); + lines.push( + `${" ".repeat(FINDING_INDENT)}${dim(clean(path))}${grouped}${knownSince}`, + ); const showLinkedFindings = command !== "show" || options.showLinkedFindings; - if (matches?.length && showLinkedFindings) { - lines.push(` ${accent("↔")} ${strong("LINKED FINDINGS")}`); + if (matches.length && showLinkedFindings) { + lines.push( + `${" ".repeat(FINDING_INDENT)}${accent("↔")} ${strong("LINKED FINDINGS")}`, + ); for (const match of matches) { lines.push( - ` ${strong("MATCHED SCAN")} ${accent(clean(match["scanId"]).slice(0, 8))}`, + `${" ".repeat(FINDING_INDENT + 2)}${strong("MATCHED SCAN")} ${accent(clean(match["scanId"]).slice(0, 8))}`, ); - wrap(`↳ ${clean(match["title"])}`, 18); + wrap(`↳ ${clean(match["title"])}`, FINDING_INDENT + 4); } } const reason = entry["matchReason"] ?? entry["reason"] ?? - (matches?.length + (matches.length ? [...new Set(matches.map((match) => clean(match["reason"])))].join( "; ", ) : undefined); - if (includeReason && reason && (!matches?.length || showLinkedFindings)) { - if (matches?.length) { - lines.push(` ${strong("SAME ROOT CAUSE")}`); - wrap(clean(reason), 18); + if (includeReason && reason && (!matches.length || showLinkedFindings)) { + if (matches.length) { + lines.push( + `${" ".repeat(FINDING_INDENT + 2)}${strong("SAME ROOT CAUSE")}`, + ); + wrap(clean(reason), FINDING_INDENT + 4); } else { - wrap(`↳ ${clean(reason)}`, 14); + wrap(`↳ ${clean(reason)}`, FINDING_INDENT); } } }; if (command === "list") { - const scans = (result["scans"] as JsonObject[]).filter((scan) => { - if ((scan["progress"] as JsonObject)["status"] !== "running") { + const scans = records(result["scans"]).filter((scan) => { + if (record(scan["progress"])["status"] !== "running") { return true; } const updated = Date.parse(scan["updatedAt"] as string); @@ -165,7 +226,7 @@ export function renderScanHistory( ), ); const latest = scans.find( - (scan) => (scan["progress"] as JsonObject)["status"] === "complete", + (scan) => record(scan["progress"])["status"] === "complete", )?.["findingCount"]; const multipleRepositories = options.repository === undefined && @@ -181,7 +242,7 @@ export function renderScanHistory( ); } for (const scan of scans) { - const status = clean((scan["progress"] as JsonObject)["status"]); + const status = text(record(scan["progress"])["status"], "unknown"); const complete = status === "complete"; const statusColor = complete ? 32 : status === "running" ? 36 : 31; const statusLabel = paint( @@ -203,11 +264,11 @@ export function renderScanHistory( } } } else if (command === "show") { - const status = clean((result["progress"] as JsonObject)["status"]); + const status = text(record(result["progress"])["status"], "unknown"); const statusColor = status === "complete" ? 32 : status === "running" ? 36 : 31; lines.push( - ` ${strong(clean(basename(result["targetPath"] as string)))} ${accent("·")} ${clean(result["scanId"])}`, + ` ${strong(clean(basename(text(result["targetPath"]))))} ${accent("·")} ${clean(result["scanId"])}`, ` ${paint(`${status === "complete" ? "✓" : "●"} ${status.toUpperCase()}`, statusColor)} ${accent("·")} ${clean(result["mode"])}`, ); if (result["failureMessage"]) { @@ -226,8 +287,8 @@ export function renderScanHistory( ` ${strong("PARENT SCAN")} ${clean(result["parentScanId"]).slice(0, 8)}`, ); } - const summary = result["severityCounts"] as JsonObject | undefined; - if (summary) { + const summary = record(result["severityCounts"]); + if (Object.keys(summary).length > 0) { lines.push( ` ${Object.entries(summary) .filter(([, count]) => count) @@ -241,19 +302,17 @@ export function renderScanHistory( .join(` ${accent("·")} `)}`, ); } - const recipe = result["recipe"] as JsonObject | undefined; - const config = recipe?.["config"] as JsonObject | undefined; - if (config && Object.keys(config).length > 0) { + const recipe = record(result["recipe"]); + const config = record(recipe["config"]); + if (Object.keys(config).length > 0) { lines.push( ` ${strong("CONFIGURATION")} ${Object.entries(config) .map(([key, value]) => `${clean(key)}=${clean(value)}`) .join(` ${accent("·")} `)}`, ); } - const coverage = (result["progress"] as JsonObject)["coverage"] as - | JsonObject - | undefined; - if (coverage) { + const coverage = record(record(result["progress"])["coverage"]); + if (Object.keys(coverage).length > 0) { const parts = [ ...(coverage["worklistRows"] == null ? [] @@ -270,16 +329,19 @@ export function renderScanHistory( ); } } - const knowledgeBase = recipe?.["knowledgeBasePaths"] as - | string[] - | undefined; - if (knowledgeBase?.length) { + const savedKnowledgeBase = recipe["knowledgeBasePaths"]; + const knowledgeBase = Array.isArray(savedKnowledgeBase) + ? savedKnowledgeBase.filter( + (path): path is string => typeof path === "string", + ) + : []; + if (knowledgeBase.length > 0) { lines.push( ` ${strong("KNOWLEDGE BASE")} ${knowledgeBase.map((path) => dim(clean(path))).join(", ")}`, ); } - const artifacts = result["artifacts"] as JsonObject | undefined; - if (artifacts && Object.keys(artifacts).length > 0) { + const artifacts = record(result["artifacts"]); + if (Object.keys(artifacts).length > 0) { lines.push(` ${strong("ARTIFACTS")}`); const scanDirectory = result["scanDir"] as string | undefined; if (scanDirectory) lines.push(` ${dim(clean(scanDirectory))}`); @@ -293,7 +355,7 @@ export function renderScanHistory( ); } } - const findings = result["findings"] as JsonObject[]; + const findings = records(result["findings"]); if (findings.length > 0) { const count = typeof result["findingCount"] === "number" @@ -314,20 +376,18 @@ export function renderScanHistory( } } else if (command === "compare") { if (result["repository"]) { - lines.push( - ` ${strong(clean(basename(result["repository"] as string)))}`, - ); + lines.push(` ${strong(clean(basename(text(result["repository"]))))}`); } lines.push( ` ${clean(result["beforeScanId"]).slice(0, 8)} → ${clean(result["afterScanId"]).slice(0, 8)}`, ); - const coverage = (result["coverage"] as JsonObject)["afterCompleteness"]; + const coverage = record(result["coverage"])["afterCompleteness"]; if (coverage !== "complete") { lines.push( ` ${paint(`⚠ Follow-up coverage is ${clean(coverage)}; resolved findings cannot be confirmed.`, 33)}`, ); } - const findings = (result["findings"] as JsonObject[]).map((entry) => + const findings = records(result["findings"]).map((entry) => entry["status"] === "unknown" && entry["reason"] === "The affected path was excluded or outside the later scope." @@ -338,12 +398,11 @@ export function renderScanHistory( (entry) => entry["status"] === "not_rescanned", ).length; const summary: JsonObject = { - ...(result["summary"] as JsonObject), + ...record(result["summary"]), ...(notRescanned ? { unknown: - Number((result["summary"] as JsonObject)["unknown"] ?? 0) - - notRescanned, + Number(record(result["summary"])["unknown"] ?? 0) - notRescanned, not_rescanned: notRescanned, } : {}), @@ -370,11 +429,7 @@ export function renderScanHistory( (entry) => String(entry["status"]).toLowerCase() === status, ); if (group.length === 0) continue; - group.sort( - (first, second) => - Object.keys(SEVERITY_COLORS).indexOf(findingSeverity(first)) - - Object.keys(SEVERITY_COLORS).indexOf(findingSeverity(second)), - ); + group.sort((first, second) => severityRank(first) - severityRank(second)); const style = STATUS_STYLES[status]!; const title = `${style.icon} ${status[0]!.toUpperCase()}${status.slice(1).replaceAll("_", " ")}`; const heading = `${title} (${group.length} finding${group.length === 1 ? "" : "s"})`; @@ -390,7 +445,7 @@ export function renderScanHistory( } } else { lines.push( - ` ${strong(clean(basename(result["repository"] as string)))}`, + ` ${strong(clean(basename(text(result["repository"]))))}`, "", ` ${paint("●", 36)} ${clean(result["scanCount"])} scans ${paint("↔", 36)} ${clean(result["matchedPairs"])} comparisons ${paint("◆", 32)} ${clean(result["findingMatches"])} root-cause matches`, ); @@ -399,6 +454,14 @@ export function renderScanHistory( ` ${paint(`${clean(result["unavailableScans"])} scans unavailable`, 33)}`, ); } + if (result["unmatchedBatches"]) { + lines.push( + ` ${paint( + `${clean(result["unmatchedBatches"])} comparisons could not be matched; rerun to retry`, + 33, + )}`, + ); + } } return `${lines.join("\n")}\n\n`; diff --git a/sdk/typescript/tests-ts/api.test.ts b/sdk/typescript/tests-ts/api.test.ts index 13e73152..e025dc72 100644 --- a/sdk/typescript/tests-ts/api.test.ts +++ b/sdk/typescript/tests-ts/api.test.ts @@ -3901,7 +3901,7 @@ if (process.argv.slice(2).join(" ") !== "login status") { await writeFile( fakeCodex, ` -import { appendFileSync } from "node:fs"; +import { appendFileSync, writeSync } from "node:fs"; const args = process.argv.slice(2).join(" "); if (args === "login --with-api-key") { @@ -3913,10 +3913,11 @@ if (args === "login --with-api-key") { appendFileSync(${JSON.stringify(keyLog)}, apiKey); } } else if (args === "login") { - console.error("Open https://auth.example.test/login"); + writeSync(2, "Open https://auth.example.test/login\\n"); } else { process.exitCode = 2; } +process.exit(process.exitCode ?? 0); `, ); let codexOptions: CodexOptions | null = null; diff --git a/sdk/typescript/tests-ts/cli-workbench.test.ts b/sdk/typescript/tests-ts/cli-workbench.test.ts index 90156a43..1e2c5245 100644 --- a/sdk/typescript/tests-ts/cli-workbench.test.ts +++ b/sdk/typescript/tests-ts/cli-workbench.test.ts @@ -1,8 +1,11 @@ -import { resolve } from "node:path"; +import { spawnSync } from "node:child_process"; +import { readFile, stat } from "node:fs/promises"; +import { join, resolve } from "node:path"; import { describe, expect, test } from "bun:test"; import type { CodexSecurityConfig, JsonObject } from "../src/index.js"; -import { DiffTarget } from "../src/index.js"; +import { CodexSecurityError, DiffTarget } from "../src/index.js"; import { main } from "../src/cli.js"; +import { PLUGIN_ROOT } from "./plugin-root.js"; import { capture, dependencies, @@ -121,7 +124,7 @@ describe("CLI workbench", () => { "before", "--after-scan-id", "after", - "--include-matching-inputs", + "--include-matching-status", ], { comparable: true, @@ -139,7 +142,7 @@ describe("CLI workbench", () => { "before", "--after-scan-id", "after", - "--include-matching-inputs", + "--include-matching-status", ], { comparable: true, @@ -245,23 +248,106 @@ describe("CLI workbench", () => { expect(calls).toEqual(["compare-scans"]); }); + test("pages explicit scan comparisons without requesting unbounded inputs", async () => { + const calls: Array = []; + const stdout = capture(); + expect( + await main( + ["scans", "compare", "before", "after", "--json"], + stdout.stream, + capture().stream, + dependencies({ + onWorkbench: (args): JsonObject => { + calls.push(args); + if (args[0] === "compare-scans") { + return { matchingCached: false }; + } + if (args[0] === "get-scan-matching-inputs") { + return { + scanId: args[2]!, + findings: [{ occurrenceId: args[2]! }], + nextOffset: null, + totalFindings: 1, + }; + } + return { summary: { persisting: 1 } }; + }, + onMatch: async () => ({ + matches: [ + { + beforeOccurrenceIds: ["before"], + afterOccurrenceIds: ["after"], + confidence: "high", + reason: "Same root cause.", + }, + ], + uncertain: [], + }), + }), + ), + ).toBe(0); + expect(calls.map(([command]) => command)).toEqual([ + "compare-scans", + "get-scan-matching-inputs", + "get-scan-matching-inputs", + "save-scan-comparison", + ]); + expect(calls[0]).toContain("--include-matching-status"); + expect(calls[0]).not.toContain("--include-matching-inputs"); + expect(JSON.parse(stdout.text())).toEqual({ summary: { persisting: 1 } }); + }); + + test("passes large valid match results through a private temporary file", async () => { + const reason = "safe reason ".repeat(2_000); + let matchesPath: string | undefined; + expect( + await main( + ["scans", "match", "before", "after", "--json"], + capture().stream, + capture().stream, + dependencies({ + onWorkbench: async (args): Promise => { + if (args[0] === "compare-scans") { + return { + matchingCached: false, + matchingInputs: { + before: [{ occurrenceId: "before" }], + after: [{ occurrenceId: "after" }], + }, + }; + } + expect(args[5]).toBe("--matches-file"); + matchesPath = args[6]!; + expect( + JSON.parse(await readFile(matchesPath, "utf8")), + ).toMatchObject({ matches: [{ reason }] }); + return { summary: { persisting: 1 } }; + }, + onMatch: async () => ({ + matches: [ + { + beforeOccurrenceIds: ["before"], + afterOccurrenceIds: ["after"], + confidence: "high", + reason, + }, + ], + uncertain: [], + }), + }), + ), + ).toBe(0); + expect(matchesPath).toBeDefined(); + await expect(stat(matchesPath!)).rejects.toMatchObject({ code: "ENOENT" }); + }); + test("matches all scans once per later scan", async () => { const finding = (occurrenceId: string) => ({ occurrenceId }); - const batches = [ - { - afterScanId: "scan-b", - afterFindings: [finding("b")], - beforeScans: [{ scanId: "scan-a", findings: [finding("a")] }], - }, - { - afterScanId: "scan-c", - afterFindings: [finding("c"), finding("c-shared")], - beforeScans: [ - { scanId: "scan-a", findings: [finding("a")] }, - { scanId: "scan-b", findings: [finding("b")] }, - ], - }, - ]; + const findings = new Map([ + ["scan-a", [finding("a")]], + ["scan-b", [finding("b")]], + ["scan-c", [finding("c"), finding("c-shared")]], + ]); const calls: Array = []; let matcherCalls = 0; const stdout = capture(); @@ -274,53 +360,90 @@ describe("CLI workbench", () => { dependencies({ onWorkbench: (args): JsonObject => { calls.push(args); - return args[0] === "list-unmatched-scan-pairs" - ? { - repository: "/current/repository", - scanCount: 5, - unavailableScans: 2, - skippedPairs: 1, - batches, - } - : {}; + if (args[0] === "list-unmatched-scan-pairs") { + return { + repository: "/current/repository", + scanCount: 5, + unavailableScans: 2, + skippedPairs: 1, + nextOffset: null, + pairs: [ + { beforeScanId: "scan-a", afterScanId: "scan-b" }, + { beforeScanId: "scan-a", afterScanId: "scan-c" }, + { beforeScanId: "scan-b", afterScanId: "scan-c" }, + ], + }; + } + if (args[0] === "get-scan-matching-inputs") { + const scanId = args[2]!; + return { + scanId, + findings: findings.get(scanId)!, + nextOffset: null, + totalFindings: findings.get(scanId)!.length, + }; + } + return {}; }, onMatch: async (input) => { matcherCalls += 1; - return input.after[0]?.occurrenceId === "b" - ? { - matches: [ - { - beforeOccurrenceIds: ["a"], - afterOccurrenceIds: ["b"], - confidence: "high", - reason: "Same root cause.", - }, - ], - uncertain: [], - } - : { - matches: [ - { - beforeOccurrenceIds: ["a", "b"], - afterOccurrenceIds: ["c"], - confidence: "high", - reason: "Same root cause.", - }, - { - beforeOccurrenceIds: ["a"], - afterOccurrenceIds: ["c-shared"], - confidence: "high", - reason: "Same root cause.", - }, - ], - uncertain: [ - { - beforeOccurrenceId: "b", - afterOccurrenceId: "c-shared", - reason: "Possibly the same root cause.", - }, - ], - }; + const before = input.before[0]?.occurrenceId; + const after = input.after[0]?.occurrenceId; + if (before === "a" && after === "b") { + return { + matches: [ + { + beforeOccurrenceIds: ["a"], + afterOccurrenceIds: ["b"], + confidence: "high", + reason: "Same root cause.", + }, + ], + uncertain: [], + }; + } + if (before === "a" && after === "c") { + return { + matches: [ + { + beforeOccurrenceIds: ["a"], + afterOccurrenceIds: ["c"], + confidence: "high", + reason: "Same root cause.", + }, + { + beforeOccurrenceIds: ["a"], + afterOccurrenceIds: ["c-shared"], + confidence: "high", + reason: "Same root cause.", + }, + ...(input.before.some( + ({ occurrenceId }) => occurrenceId === "b", + ) + ? [ + { + beforeOccurrenceIds: ["b"], + afterOccurrenceIds: ["c"], + confidence: "high" as const, + reason: "Same root cause.", + }, + ] + : []), + ], + uncertain: [], + }; + } + return { + matches: [ + { + beforeOccurrenceIds: ["b"], + afterOccurrenceIds: ["c"], + confidence: "high", + reason: "Same root cause.", + }, + ], + uncertain: [], + }; }, }), ), @@ -330,10 +453,17 @@ describe("CLI workbench", () => { "list-unmatched-scan-pairs", "--repository", "/current/repository", + "--offset", + "0", + "--completed-before", + expect.any(String), "--force", ]); + const saves = calls.filter( + ([command]) => command === "save-scan-comparison", + ); expect( - calls.slice(1).map((args) => ({ + saves.map((args) => ({ before: args[2], after: args[4], result: JSON.parse(args[6]!), @@ -345,8 +475,10 @@ describe("CLI workbench", () => { after: "scan-c", result: { matches: [ - { beforeOccurrenceIds: ["a"], afterOccurrenceIds: ["c"] }, - { beforeOccurrenceIds: ["a"], afterOccurrenceIds: ["c-shared"] }, + { + beforeOccurrenceIds: ["a"], + afterOccurrenceIds: ["c", "c-shared"], + }, ], uncertain: [], }, @@ -356,7 +488,7 @@ describe("CLI workbench", () => { after: "scan-c", result: { matches: [{ beforeOccurrenceIds: ["b"] }], - uncertain: [{ beforeOccurrenceId: "b" }], + uncertain: [], }, }, ]); @@ -370,31 +502,363 @@ describe("CLI workbench", () => { }); }); - test("saves empty comparisons without starting Codex", async () => { + test("keeps one later-scan matching group across workbench pair pages", async () => { + const pairCount = 65; const calls: Array = []; - const deps = dependencies({ - onWorkbench: (args): JsonObject => { - calls.push(args); - return args[0] === "list-unmatched-scan-pairs" - ? { - repository: "/repo", - scanCount: 2, - unavailableScans: 0, - skippedPairs: 0, - batches: [ + let matcherCalls = 0; + let activeInputLoads = 0; + let maximumInputLoads = 0; + const stdout = capture(); + + expect( + await main( + ["scans", "match", "--all", "--json"], + stdout.stream, + capture().stream, + dependencies({ + onWorkbench: async (args): Promise => { + calls.push(args); + if (args[0] === "list-unmatched-scan-pairs") { + const offset = Number(args[4]); + return { + repository: "/repo", + scanCount: pairCount + 1, + unavailableScans: 0, + skippedPairs: 0, + nextOffset: offset === 0 ? 64 : null, + pairs: Array.from( + { length: offset === 0 ? 64 : 1 }, + (_, index) => ({ + beforeScanId: `before-${offset + index}`, + afterScanId: "after", + }), + ), + }; + } + if (args[0] !== "get-scan-matching-inputs") return {}; + const scanId = args[2]!; + activeInputLoads += 1; + maximumInputLoads = Math.max(maximumInputLoads, activeInputLoads); + await Promise.resolve(); + activeInputLoads -= 1; + return { + scanId, + findings: [{ occurrenceId: scanId }], + nextOffset: null, + totalFindings: 1, + }; + }, + onMatch: async (input) => { + matcherCalls += 1; + return { + matches: [ { - afterScanId: "after", - afterFindings: [], - beforeScans: [ - { - scanId: "before", - findings: [{ occurrenceId: "before" }], - }, - ], + beforeOccurrenceIds: input.before.map( + ({ occurrenceId }) => occurrenceId, + ), + afterOccurrenceIds: ["after"], + confidence: "high", + reason: "Same root cause.", }, ], + uncertain: [], + }; + }, + }), + ), + ).toBe(0); + + expect(matcherCalls).toBe(1); + expect(maximumInputLoads).toBeLessThanOrEqual(8); + expect( + calls + .filter(([command]) => command === "list-unmatched-scan-pairs") + .map((args) => args[4]), + ).toEqual(["0", "64"]); + expect( + calls.filter(([command]) => command === "save-scan-comparison"), + ).toHaveLength(pairCount); + expect(JSON.parse(stdout.text())).toMatchObject({ + matchedPairs: pairCount, + findingMatches: pairCount, + }); + }); + + test("freezes completed scans and skips cached pair pages in one workbench call", () => { + const python = Bun.which("python3") ?? Bun.which("python"); + expect(python).not.toBeNull(); + const result = spawnSync( + python!, + [ + "-I", + "-B", + "-c", + [ + "import argparse, sqlite3, sys", + "sys.path.insert(0, sys.argv[1])", + "from workbench_scan_history import list_unmatched_scan_pairs", + "connection = sqlite3.connect(':memory:')", + "connection.row_factory = sqlite3.Row", + "connection.executescript('CREATE TABLE security_targets (id TEXT, current_path TEXT); CREATE TABLE scans (id TEXT, target_path TEXT, status TEXT, started_at TEXT, completed_at TEXT); CREATE TABLE scan_comparisons (before_scan_id TEXT, after_scan_id TEXT);')", + "scans = [(f'scan-{index:03}', '/repo', 'complete', f'2026-01-01T00:{index:03}Z', '2026-01-01T00:00:00Z') for index in range(100)]", + "connection.executemany('INSERT INTO scans VALUES (?, ?, ?, ?, ?)', scans)", + "saved = [(before[0], after[0]) for position, after in enumerate(scans) for before in scans[:position]]", + "connection.executemany('INSERT INTO scan_comparisons VALUES (?, ?)', saved)", + "connection.execute(\"INSERT INTO scans VALUES ('new-scan', '/repo', 'complete', '2025-01-01T00:00:00Z', '2027-01-01T00:00:00Z')\")", + "args = argparse.Namespace(repository='/repo', force=False, offset=0, completed_before='2026-06-01T00:00:00Z')", + "result = list_unmatched_scan_pairs(connection, args, read_coverage=lambda scan: {})", + "assert result['scanCount'] == 100, result", + "assert result['skippedPairs'] == len(saved), result", + "assert result['pairs'] == [] and result['nextOffset'] is None, result", + "connection.execute('DELETE FROM scan_comparisons WHERE before_scan_id = ? AND after_scan_id = ?', (scans[-2][0], scans[-1][0]))", + "result = list_unmatched_scan_pairs(connection, args, read_coverage=lambda scan: {})", + "assert result['pairs'] == [{'beforeScanId': scans[-2][0], 'afterScanId': scans[-1][0]}], result", + "assert result['nextOffset'] is None, result", + ].join("\n"), + join(PLUGIN_ROOT, "scripts"), + ], + { encoding: "utf8", timeout: 10_000 }, + ); + expect(result.status, result.stderr).toBe(0); + }); + + test("pages match-all history and reconciles bounded finding batches", async () => { + const calls: Array = []; + const matcherInputs: Array<{ + before: string[]; + after: string[]; + }> = []; + const pages = new Map([ + ["before:0", { findings: [{ occurrenceId: "b-1" }], nextOffset: 1 }], + ["before:1", { findings: [{ occurrenceId: "b-2" }], nextOffset: null }], + ["after:0", { findings: [{ occurrenceId: "a-1" }], nextOffset: 1 }], + ["after:1", { findings: [{ occurrenceId: "a-2" }], nextOffset: null }], + ]); + const stdout = capture(); + + expect( + await main( + ["scans", "match", "--all", "--json"], + stdout.stream, + capture().stream, + dependencies({ + onWorkbench: (args): JsonObject => { + calls.push(args); + if (args[0] === "list-unmatched-scan-pairs") { + const offset = Number(args[4]); + return { + repository: "/repo", + scanCount: 2, + unavailableScans: 0, + skippedPairs: 0, + nextOffset: offset === 0 ? 64 : null, + pairs: + offset === 0 + ? [] + : [{ beforeScanId: "before", afterScanId: "after" }], + }; + } + if (args[0] === "get-scan-matching-inputs") { + const scanId = args[2]!; + const offset = Number(args[4]); + return { + scanId, + totalFindings: 2, + ...pages.get(`${scanId}:${offset}`)!, + }; + } + return {}; + }, + onMatch: async (input) => { + const before = input.before.map(({ occurrenceId }) => occurrenceId); + const after = input.after.map(({ occurrenceId }) => occurrenceId); + matcherInputs.push({ before, after }); + if ( + (before[0] === "b-1" && after[0] === "a-1") || + (before[0] === "b-1" && after[0] === "a-2") || + (before[0] === "b-2" && after[0] === "a-2") + ) { + return { + matches: [ + { + beforeOccurrenceIds: before, + afterOccurrenceIds: after, + confidence: "high", + reason: `${before[0]} matches ${after[0]}`, + }, + ], + uncertain: [], + }; + } + return { matches: [], uncertain: [] }; + }, + }), + ), + ).toBe(0); + + expect( + calls + .filter(([command]) => command === "list-unmatched-scan-pairs") + .map((args) => args[4]), + ).toEqual(["0", "64"]); + expect(matcherInputs).toEqual([ + { before: ["b-1"], after: ["a-1"] }, + { before: ["b-1"], after: ["a-2"] }, + { before: ["b-2"], after: ["a-1"] }, + { before: ["b-2"], after: ["a-2"] }, + ]); + const save = calls.find(([command]) => command === "save-scan-comparison")!; + expect(JSON.parse(save[6]!)).toEqual({ + matches: [ + { + beforeOccurrenceIds: ["b-1", "b-2"], + afterOccurrenceIds: ["a-1", "a-2"], + confidence: "high", + reason: "b-1 matches a-1", + }, + ], + uncertain: [], + }); + expect(JSON.parse(stdout.text())).toMatchObject({ + matchedPairs: 1, + findingMatches: 4, + }); + }); + + test("rejects oversized accumulated match-all results before persistence", async () => { + const calls: Array = []; + const stderr = capture(); + expect( + await main( + ["scans", "match", "--all"], + capture().stream, + stderr.stream, + dependencies({ + onWorkbench: (args): JsonObject => { + calls.push(args); + if (args[0] === "list-unmatched-scan-pairs") { + return { + repository: "/repo", + scanCount: 2, + unavailableScans: 0, + skippedPairs: 0, + nextOffset: null, + pairs: [{ beforeScanId: "before", afterScanId: "after" }], + }; + } + const scanId = args[2]!; + return { + scanId, + findings: [{ occurrenceId: scanId }], + nextOffset: null, + totalFindings: 1, + }; + }, + onMatch: async () => ({ + matches: [ + { + beforeOccurrenceIds: ["before"], + afterOccurrenceIds: ["after"], + confidence: "high", + reason: "x".repeat(1024 * 1024), + }, + ], + uncertain: [], + }), + }), + ), + ).toBe(2); + expect(stderr.text()).toContain("more than 1 MiB"); + expect(calls.some(([command]) => command === "save-scan-comparison")).toBe( + false, + ); + }); + + test("caps reconciled matches instead of duplicated page-pair responses", async () => { + const calls: Array = []; + const stdout = capture(); + const reason = "same root cause ".repeat(20_000); + + expect( + await main( + ["scans", "match", "--all", "--json"], + stdout.stream, + capture().stream, + dependencies({ + onWorkbench: (args): JsonObject => { + calls.push(args); + if (args[0] === "list-unmatched-scan-pairs") { + return { + repository: "/repo", + scanCount: 2, + unavailableScans: 0, + skippedPairs: 0, + nextOffset: null, + pairs: [{ beforeScanId: "before", afterScanId: "after" }], + }; } - : {}; + if (args[0] !== "get-scan-matching-inputs") return {}; + const scanId = args[2]!; + const offset = Number(args[4]); + return { + scanId, + findings: [{ occurrenceId: `${scanId}-${offset}` }], + nextOffset: offset === 0 ? 1 : null, + totalFindings: 2, + }; + }, + onMatch: async (input) => ({ + matches: [ + { + beforeOccurrenceIds: input.before.map( + ({ occurrenceId }) => occurrenceId, + ), + afterOccurrenceIds: input.after.map( + ({ occurrenceId }) => occurrenceId, + ), + confidence: "high", + reason, + }, + ], + uncertain: [], + }), + }), + ), + ).toBe(0); + expect( + calls.some( + (args) => + args[0] === "save-scan-comparison" && args.includes("--matches-file"), + ), + ).toBe(true); + expect(JSON.parse(stdout.text())).toMatchObject({ matchedPairs: 1 }); + }); + + test("saves empty comparisons without starting Codex", async () => { + const calls: Array = []; + const deps = dependencies({ + onWorkbench: (args): JsonObject => { + calls.push(args); + if (args[0] === "list-unmatched-scan-pairs") { + return { + repository: "/repo", + scanCount: 2, + unavailableScans: 0, + skippedPairs: 0, + nextOffset: null, + pairs: [{ beforeScanId: "before", afterScanId: "after" }], + }; + } + if (args[0] === "get-scan-matching-inputs") { + const scanId = args[2]!; + return { + scanId, + findings: scanId === "before" ? [{ occurrenceId: "before" }] : [], + nextOffset: null, + totalFindings: scanId === "before" ? 1 : 0, + }; + } + return {}; }, }); deps.matchFindings = async () => { @@ -409,7 +873,8 @@ describe("CLI workbench", () => { deps, ), ).toBe(0); - expect(JSON.parse(calls[1]![6]!)).toEqual({ matches: [], uncertain: [] }); + const save = calls.find(([command]) => command === "save-scan-comparison")!; + expect(JSON.parse(save[6]!)).toEqual({ matches: [], uncertain: [] }); }); test("does not save conflicting confirmed and uncertain matches", async () => { @@ -423,22 +888,28 @@ describe("CLI workbench", () => { dependencies({ onWorkbench: (args): JsonObject => { calls.push(args); + if (args[0] === "list-unmatched-scan-pairs") { + return { + repository: "/repo", + scanCount: 2, + unavailableScans: 0, + skippedPairs: 0, + nextOffset: null, + pairs: [{ beforeScanId: "before", afterScanId: "after" }], + }; + } + const scanId = args[2]!; return { - batches: [ - { - afterScanId: "after", - afterFindings: [{ occurrenceId: "after" }], - beforeScans: [ - { - scanId: "before", - findings: [ - { occurrenceId: "confirmed" }, - { occurrenceId: "uncertain" }, - ], - }, - ], - }, - ], + scanId, + findings: + scanId === "before" + ? [ + { occurrenceId: "confirmed" }, + { occurrenceId: "uncertain" }, + ] + : [{ occurrenceId: "after" }], + nextOffset: null, + totalFindings: scanId === "before" ? 2 : 1, }; }, onMatch: async () => ({ @@ -462,7 +933,221 @@ describe("CLI workbench", () => { ), ).toBe(2); expect(stderr.text()).toContain("conflicting confirmed and uncertain"); - expect(calls).toHaveLength(1); + expect(calls.some(([command]) => command === "save-scan-comparison")).toBe( + false, + ); + }); + + test("falls back per scan pair when a grouped optimization rejects", async () => { + const calls: Array = []; + const stdout = capture(); + const stderr = capture(); + + expect( + await main( + ["scans", "match", "--all", "--json"], + stdout.stream, + stderr.stream, + dependencies({ + onWorkbench: (args): JsonObject => { + calls.push(args); + if (args[0] === "list-unmatched-scan-pairs") { + return { + repository: "/repo", + scanCount: 3, + unavailableScans: 0, + skippedPairs: 0, + nextOffset: null, + pairs: [ + { beforeScanId: "before-broken", afterScanId: "after" }, + { beforeScanId: "before-valid", afterScanId: "after" }, + ], + }; + } + if (args[0] !== "get-scan-matching-inputs") return {}; + const scanId = args[2]!; + if (scanId === "before-broken") { + throw new Error("finding exceeds the page size limit"); + } + return { + scanId, + findings: [{ occurrenceId: scanId }], + nextOffset: null, + totalFindings: 1, + }; + }, + onMatch: async (input) => ({ + matches: [ + { + beforeOccurrenceIds: input.before.map( + ({ occurrenceId }) => occurrenceId, + ), + afterOccurrenceIds: input.after.map( + ({ occurrenceId }) => occurrenceId, + ), + confidence: "high", + reason: "Same root cause.", + }, + ], + uncertain: [], + }), + }), + ), + ).toBe(0); + expect(JSON.parse(stdout.text())).toMatchObject({ + matchedPairs: 1, + unmatchedBatches: 1, + }); + expect(stderr.text()).toContain("finding exceeds the page size limit"); + expect( + calls.filter(([command]) => command === "save-scan-comparison"), + ).toHaveLength(1); + }); + + test("keeps matching later scans after one batch conflicts", async () => { + const calls: Array = []; + const stderr = capture(); + const stdout = capture(); + const findings = new Map([ + [ + "before-conflict", + [{ occurrenceId: "confirmed" }, { occurrenceId: "loose" }], + ], + ["after-conflict", [{ occurrenceId: "after" }]], + ["before-ok", [{ occurrenceId: "ok-before" }]], + ["after-ok", [{ occurrenceId: "ok-after" }]], + ]); + + expect( + await main( + ["scans", "match", "--all", "--json"], + stdout.stream, + stderr.stream, + dependencies({ + onWorkbench: (args): JsonObject => { + calls.push(args); + if (args[0] === "list-unmatched-scan-pairs") { + const offset = Number(args[4]); + return { + repository: "/repo", + scanCount: 3, + unavailableScans: 0, + skippedPairs: 0, + nextOffset: offset === 0 ? 1 : null, + pairs: + offset === 0 + ? [ + { + beforeScanId: "before-conflict", + afterScanId: "after-conflict", + }, + ] + : [ + { + beforeScanId: "before-ok", + afterScanId: "after-ok", + }, + ], + }; + } + if (args[0] === "get-scan-matching-inputs") { + const scanId = args[2]!; + const pageFindings = findings.get(scanId)!; + return { + scanId, + findings: pageFindings, + nextOffset: null, + totalFindings: pageFindings.length, + }; + } + return {}; + }, + onMatch: async (input) => + input.after[0]?.occurrenceId === "after" + ? { + matches: [ + { + beforeOccurrenceIds: ["confirmed"], + afterOccurrenceIds: ["after"], + confidence: "high", + reason: "Same root cause.", + }, + ], + uncertain: [ + { + beforeOccurrenceId: "loose", + afterOccurrenceId: "after", + reason: "Possibly the same root cause.", + }, + ], + } + : { + matches: [ + { + beforeOccurrenceIds: ["ok-before"], + afterOccurrenceIds: ["ok-after"], + confidence: "high", + reason: "Same root cause.", + }, + ], + uncertain: [], + }, + }), + ), + ).toBe(0); + // The conflicting batch is skipped, not saved, and does not stop the run. + expect( + calls.filter((args) => args[0] === "save-scan-comparison").length, + ).toBe(1); + expect( + calls + .filter((args) => args[0] === "list-unmatched-scan-pairs") + .map((args) => args[4]), + ).toEqual(["0", "1"]); + expect(calls.at(-1)?.[2]).toBe("before-ok"); + expect(stderr.text()).toContain("conflicting confirmed and uncertain"); + expect(stderr.text()).toContain("after-conflict"); + expect(JSON.parse(stdout.text())).toEqual({ + repository: "/repo", + scanCount: 3, + unavailableScans: 0, + matchedPairs: 1, + skippedPairs: 0, + findingMatches: 1, + unmatchedBatches: 1, + }); + }); + + test("reports the underlying failure when no batch could be matched", async () => { + const stderr = capture(); + expect( + await main( + ["scans", "match", "--all"], + capture().stream, + stderr.stream, + dependencies({ + onWorkbench: (args): JsonObject => { + if (args[0] === "list-unmatched-scan-pairs") { + return { + repository: "/repo", + scanCount: 2, + unavailableScans: 0, + skippedPairs: 0, + nextOffset: null, + pairs: [ + { beforeScanId: "one-before", afterScanId: "one-after" }, + { beforeScanId: "two-before", afterScanId: "two-after" }, + ], + }; + } + throw new CodexSecurityError( + "cannot open the workbench database at /repo/workbench.sqlite3", + ); + }, + }), + ), + ).toBe(2); + expect(stderr.text()).toContain("cannot open the workbench database"); }); test("force recomputes saved matches", async () => { diff --git a/sdk/typescript/tests-ts/report-projection.test.ts b/sdk/typescript/tests-ts/report-projection.test.ts new file mode 100644 index 00000000..59d2a420 --- /dev/null +++ b/sdk/typescript/tests-ts/report-projection.test.ts @@ -0,0 +1,151 @@ +import { spawnSync } from "node:child_process"; +import { mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { dirname, join } from "node:path"; +import { fileURLToPath } from "node:url"; +import { describe, expect, test } from "bun:test"; + +const pluginRoot = join( + dirname(fileURLToPath(import.meta.url)), + "..", + "_bundled_plugin", +); + +function usablePython(): string | null { + for (const candidate of [process.env["PYTHON"], "python3", "python"]) { + if (candidate === undefined) continue; + const probe = spawnSync( + candidate, + [ + "-c", + "import sys; raise SystemExit(0 if sys.version_info >= (3, 10) else 1)", + ], + { encoding: "utf8" }, + ); + if (probe.status === 0) return candidate; + } + return null; +} + +const python = usablePython(); + +/** Render report.md through the bundled projection with each finding forced to `level`. */ +function renderReport(python: string, levels: readonly string[]): string { + const script = ` +import copy, json, sys +sys.path.insert(0, ${JSON.stringify(join(pluginRoot, "scripts"))}) +import report_projection + +base = ${JSON.stringify(join(pluginRoot, "examples", "completed-scan"))} +def load(name): + with open(base + "/" + name, encoding="utf-8") as handle: + return json.load(handle) + +manifest, coverage, findings = load("scan-manifest.json"), load("coverage.json"), load("findings.json") +levels = json.loads(${JSON.stringify(JSON.stringify(levels))}) +template = copy.deepcopy(findings["findings"][0]) +findings["findings"] = [] +for index, level in enumerate(levels): + entry = copy.deepcopy(template) + entry["severity"]["level"] = level + entry["title"] = "Finding %d" % index + entry["findingId"] = "%s-%d" % (entry.get("findingId", "finding"), index) + if index > 0: + entry.pop("writeup", None) + findings["findings"].append(entry) + +sys.stdout.write(report_projection.build_report_markdown(manifest, findings, coverage)) +`; + const result = spawnSync(python, ["-I", "-B", "-c", script], { + encoding: "utf8", + maxBuffer: 16 * 1024 * 1024, + }); + if (result.status !== 0) { + throw new Error(`report projection failed: ${result.stderr}`); + } + return result.stdout; +} + +describe("report projection severity gate", () => { + // https://github.com/openai/codex-security/issues/48 -- informational findings + // are sealed into findings.json and exported to SARIF and CSV, but excluded + // from report.md, which then claimed the scan produced nothing. + test.skipIf(python === null)( + "records findings held back by the reportable severity gate", + () => { + const text = renderReport(python!, ["informational"]); + expect(text).toContain("### No findings"); + expect(text).toContain( + "1 finding is outside the reportable severity set", + ); + expect(text).toContain("(informational: 1)"); + expect(text).toContain("`findings.json`"); + expect(text).not.toContain("Finding 0"); + }, + ); + + test.skipIf(python === null)( + "records held-back findings alongside reportable ones", + () => { + const text = renderReport(python!, [ + "high", + "informational", + "informational", + ]); + expect(text).not.toContain("### No findings"); + expect(text).toContain("Finding 0"); + expect(text).toContain( + "2 findings are outside the reportable severity set", + ); + expect(text).toContain("(informational: 2)"); + }, + ); + + test.skipIf(python === null)( + "stays silent when every finding is reportable", + () => { + const text = renderReport(python!, ["high", "low"]); + expect(text).not.toContain("outside the reportable severity set"); + expect(text).not.toContain("### No findings"); + }, + ); + + test.skipIf(python === null)( + "keeps the rendered report valid for the format validator", + () => { + const reportDirectory = mkdtempSync( + join(tmpdir(), "codex-security-report-projection-"), + ); + const reportPath = join(reportDirectory, "report.md"); + try { + for (const levels of [["informational"], ["high", "informational"]]) { + writeFileSync(reportPath, renderReport(python!, levels), "utf8"); + const validation = spawnSync( + python!, + [ + "-I", + "-B", + join(pluginRoot, "scripts", "validate_report_format.py"), + "--report-md", + reportPath, + ], + { encoding: "utf8" }, + ); + expect(validation.status).toBe(0); + } + } finally { + rmSync(reportDirectory, { recursive: true, force: true }); + } + }, + ); + + test("documents the exclusion in the final-report reference", () => { + const reference = readFileSync( + join(pluginRoot, "references", "final-report.md"), + "utf8", + ); + expect(reference).toContain( + "`informational` is outside the reportable severity set", + ); + }); +}); diff --git a/sdk/typescript/tests-ts/scan-history-renderer.test.ts b/sdk/typescript/tests-ts/scan-history-renderer.test.ts index 465209d7..bd85ef44 100644 --- a/sdk/typescript/tests-ts/scan-history-renderer.test.ts +++ b/sdk/typescript/tests-ts/scan-history-renderer.test.ts @@ -154,6 +154,28 @@ describe("scan history renderer", () => { } }); + test("ignores malformed persisted knowledge-base paths", () => { + for (const knowledgeBasePaths of [ + "/not/an/array", + { path: "/not/an/array" }, + [null, 42, "/safe/threat-model.md"], + ]) { + const result = { + scanId: "scan-1", + progress: { status: "complete" }, + recipe: { knowledgeBasePaths }, + }; + expect(() => + renderScanHistory(result, "show", { color: false }), + ).not.toThrow(); + if (Array.isArray(knowledgeBasePaths)) { + expect(renderScanHistory(result, "show", { color: false })).toContain( + "/safe/threat-model.md", + ); + } + } + }); + test("keeps repositories visible at narrow and wide terminal widths", () => { const scans = [ { @@ -291,3 +313,226 @@ describe("scan history renderer", () => { } }); }); + +describe("scan history renderer resilience", () => { + // The workbench response is only checked for being a JSON object + // (src/runtime.ts runWorkbench), so a plugin or database that drifts from the + // shape this renderer expects must degrade instead of crashing the command. + const plain = ( + result: Parameters[0], + command: Parameters[1], + options = {}, + ) => + stripVTControlCharacters( + renderScanHistory(result, command, { color: false, ...options }), + ); + + test("renders comparison findings with a missing or null severity", () => { + for (const severity of [undefined, null]) { + const text = plain( + { + beforeScanId: "before-scan", + afterScanId: "after-scan", + coverage: { afterCompleteness: "complete" }, + summary: { resolved: 1 }, + findings: [ + { + ...(severity === undefined ? {} : { severity }), + status: "resolved", + title: "Reflected XSS in the search handler", + locations: [{ path: "src/search.ts", startLine: 10 }], + }, + ], + }, + "compare", + ); + expect(text).toContain("Reflected XSS in the search handler"); + expect(text).toContain("src/search.ts:10"); + } + }); + + test("sorts an unrecognized severity below every known severity", () => { + const text = plain( + { + beforeScanId: "before-scan", + afterScanId: "after-scan", + coverage: { afterCompleteness: "complete" }, + summary: { new: 2 }, + findings: [ + { + status: "new", + severity: "not-a-severity", + title: "Unknown severity finding", + path: "a.ts", + }, + { + status: "new", + severity: "critical", + title: "Critical severity finding", + path: "b.ts", + }, + ], + }, + "compare", + ); + expect(text.indexOf("Critical severity finding")).toBeLessThan( + text.indexOf("Unknown severity finding"), + ); + }); + + test("lists scans that carry no progress record", () => { + const text = plain( + { + scans: [ + { + scanId: "11111111-1111-4111-8111-111111111111", + targetPath: "/repo", + mode: "standard", + startedAt: "2026-07-01T00:00:00Z", + findingCount: 3, + }, + ], + }, + "list", + ); + expect(text).toContain("11111111-1111-4111-8111-111111111111"); + expect(text).toContain("UNKNOWN"); + }); + + test("falls back to the raw value for an unparsable knownSince", () => { + const text = plain( + { + scanId: "scan-1", + targetPath: "/repo", + mode: "standard", + progress: { status: "complete" }, + findings: [ + { + severity: { level: "high" }, + title: "Path traversal", + knownSince: "not-a-timestamp", + matches: [ + { + scanId: "older-scan", + title: "Path traversal", + reason: "same sink", + }, + ], + locations: [{ path: "src/fs.ts", startLine: 22 }], + }, + ], + }, + "show", + { showLinkedFindings: true }, + ); + expect(text).toContain("Known since not-a-timestamp"); + expect(text).toContain("Path traversal"); + }); + + test("renders every command from an empty payload", () => { + for (const command of ["list", "show", "compare", "match-all"] as const) { + expect(() => plain({}, command)).not.toThrow(); + } + }); +}); + +describe("severity badge column", () => { + const severities = ["critical", "high", "medium", "low", "informational"]; + + test("starts every finding title at the same column", () => { + for (const color of [false, true]) { + for (const columns of [48, 96, 120]) { + const text = stripVTControlCharacters( + renderScanHistory( + { + beforeScanId: "before-scan", + afterScanId: "after-scan", + coverage: { afterCompleteness: "complete" }, + summary: { new: severities.length }, + findings: severities.map((severity) => ({ + status: "new", + severity, + title: `Title for ${severity}`, + path: `${severity}.ts`, + })), + }, + "compare", + { color, columns }, + ), + ); + const offsets = severities.map((severity) => { + const line = text + .split("\n") + .find((candidate) => candidate.includes(`Title for ${severity}`))!; + return line.indexOf(`Title for ${severity}`); + }); + expect(new Set(offsets).size).toBe(1); + const path = text + .split("\n") + .find((candidate) => candidate.includes("informational.ts"))!; + expect(path.indexOf("informational.ts")).toBe(offsets[0]!); + } + } + }); + + test("keeps the badge column as wide as its widest label", () => { + const text = stripVTControlCharacters( + renderScanHistory( + { + beforeScanId: "before-scan", + afterScanId: "after-scan", + coverage: { afterCompleteness: "complete" }, + summary: { new: 1 }, + findings: [ + { + status: "new", + severity: "informational", + title: "Informational finding", + path: "a.ts", + }, + ], + }, + "compare", + { color: false }, + ), + ); + expect(text).toContain(" INFO Informational finding"); + expect(text).not.toContain("INFORMATIONAL "); + }); + + test("keeps unrecognized severity labels inside the fixed badge column", () => { + const text = stripVTControlCharacters( + renderScanHistory( + { + beforeScanId: "before-scan", + afterScanId: "after-scan", + coverage: { afterCompleteness: "complete" }, + summary: { new: 2 }, + findings: [ + { + status: "new", + severity: "extraordinary-severity", + title: "Unknown severity finding", + }, + { + status: "new", + severity: "critical", + title: "Known severity finding", + }, + ], + }, + "compare", + { color: false }, + ), + ); + const unknown = text + .split("\n") + .find((line) => line.includes("Unknown severity finding"))!; + const known = text + .split("\n") + .find((line) => line.includes("Known severity finding"))!; + expect(unknown.indexOf("Unknown severity finding")).toBe( + known.indexOf("Known severity finding"), + ); + }); +});