diff --git a/plugins/codex-security/mcp-app/src/artifact-discovery.ts b/plugins/codex-security/mcp-app/src/artifact-discovery.ts index 622a8156e..09ee3303a 100644 --- a/plugins/codex-security/mcp-app/src/artifact-discovery.ts +++ b/plugins/codex-security/mcp-app/src/artifact-discovery.ts @@ -17,7 +17,6 @@ import { type SchemaDocument } from "./artifact-schema-loader.js"; import { candidateSchemaV1 } from "./deep-scan/artifact-contracts.js"; -import { missingPythonHelperMessage, resolvePythonCommand } from "./python_command.js"; const execFileAsync = promisify(execFile); const discoveryComponents = ["artifacts", "02_discovery"] as const; @@ -137,7 +136,7 @@ export async function recordCodexSecurityDiscoveryCandidates( const inventoryComponents = [...discoveryComponents, "in_scope_files.txt"]; const candidateComponents = [...discoveryComponents, "candidate_ledger.jsonl"]; - // Verify the inventory is a context-bound regular file before passing it to Python. + // Verify the inventory is a context-bound regular file before normalization. await readArtifactText(context, inventoryComponents, "discovery review inventory"); const inventoryPath = await artifactDestination( context, @@ -161,12 +160,11 @@ export async function recordCodexSecurityDiscoveryCandidates( mode: 0o600 }); - const pythonCommand = context.pythonCommand ?? await resolvePythonCommand(); try { await execFileAsync( - pythonCommand, + process.execPath, [ - join(pluginRoot, "scripts", "normalize_candidates.py"), + join(pluginRoot, "scripts", "normalize_candidates.mjs"), "--input", temporaryInput, "--out", @@ -184,7 +182,7 @@ export async function recordCodexSecurityDiscoveryCandidates( } ); } catch (error) { - throw discoveryNormalizationError(error, pythonCommand, [ + throw discoveryNormalizationError(error, [ [temporaryInput, "candidate input"], [temporaryDirectory, "private candidate input"], [inventoryPath, "the assigned review inventory"], @@ -226,14 +224,8 @@ export async function listCodexSecurityCandidates( function discoveryNormalizationError( error: unknown, - pythonCommand: string, privateValues: Array ): Error { - const pythonMessage = missingPythonHelperMessage(error, pythonCommand); - if (pythonMessage) { - return new Error(`${discoveryLabel}: ${pythonMessage}`, { cause: error }); - } - const stderr = error && typeof error === "object" && "stderr" in error ? error.stderr : undefined; diff --git a/plugins/codex-security/mcp-app/tests/test_artifact_discovery.mjs b/plugins/codex-security/mcp-app/tests/test_artifact_discovery.mjs index b1c6753f6..0049024ed 100644 --- a/plugins/codex-security/mcp-app/tests/test_artifact_discovery.mjs +++ b/plugins/codex-security/mcp-app/tests/test_artifact_discovery.mjs @@ -34,7 +34,9 @@ const { `data:text/javascript;base64,${Buffer.from(bundle.outputFiles[0].contents).toString("base64")}` ); -const pluginRoot = fileURLToPath(new URL("../../", import.meta.url)); +const pluginRoot = process.env.CODEX_SECURITY_TEST_PLUGIN_ROOT + ? path.resolve(process.env.CODEX_SECURITY_TEST_PLUGIN_ROOT) + : fileURLToPath(new URL("../../../../sdk/typescript/_bundled_plugin/", import.meta.url)); const definitions = JSON.parse(await readFile(path.join( pluginRoot, "schemas", @@ -388,7 +390,8 @@ async function createContext(root, repoRoot, name, layout) { root: artifactRoot, repoRoot, layout, - pluginRoot + pluginRoot, + pythonCommand: path.join(root, "missing-python") }; } diff --git a/plugins/codex-security/mcp-app/tests/test_compact_artifact_server.mjs b/plugins/codex-security/mcp-app/tests/test_compact_artifact_server.mjs index d1af7e3da..f3f9e48f7 100644 --- a/plugins/codex-security/mcp-app/tests/test_compact_artifact_server.mjs +++ b/plugins/codex-security/mcp-app/tests/test_compact_artifact_server.mjs @@ -13,8 +13,7 @@ const applicationRoot = path.resolve( path.dirname(fileURLToPath(import.meta.url)), ".." ); -const pluginRoot = path.resolve(applicationRoot, ".."); -const bundledPluginRoot = process.env.CODEX_SECURITY_TEST_PLUGIN_ROOT +const pluginRoot = process.env.CODEX_SECURITY_TEST_PLUGIN_ROOT ? path.resolve(process.env.CODEX_SECURITY_TEST_PLUGIN_ROOT) : path.resolve(applicationRoot, "../../../sdk/typescript/_bundled_plugin"); const temporaryRoot = await mkdtemp(path.join(tmpdir(), "codex-security-artifact-mcp-")); @@ -30,7 +29,7 @@ try { await testDiscoveryWorkerToolList(runtimeBundle); await testReducerWorkerToolList(runtimeBundle); - const shippedRuntime = path.join(bundledPluginRoot, "mcp", "server.mjs"); + const shippedRuntime = path.join(pluginRoot, "mcp", "server.mjs"); await testParentToolList(shippedRuntime); await testClaimedParentArtifactOperations(shippedRuntime, "shipped"); await testSemanticScanDraftCompletion(shippedRuntime, "shipped"); @@ -1179,7 +1178,7 @@ async function bundleEntrypoint(entrypoint, outfile) { await build({ bundle: true, define: { - __dirname: JSON.stringify(applicationRoot), + __dirname: JSON.stringify(path.join(pluginRoot, "mcp")), "import.meta.url": "__filename" }, entryPoints: [path.join(applicationRoot, entrypoint)], diff --git a/plugins/codex-security/plugin-files.json b/plugins/codex-security/plugin-files.json index 2d0004e21..bf6885699 100644 --- a/plugins/codex-security/plugin-files.json +++ b/plugins/codex-security/plugin-files.json @@ -50,7 +50,7 @@ "scripts/generate_rank_input.py", "scripts/launch_codex_security_mcp", "scripts/launch_codex_security_mcp.cmd", - "scripts/normalize_candidates.py", + "scripts/normalize_candidates.mjs", "scripts/rank_preview.py", "scripts/report_projection.py", "scripts/resolve_security_md.py", diff --git a/plugins/codex-security/scripts/normalize_candidates.py b/plugins/codex-security/scripts/normalize_candidates.py deleted file mode 100644 index a445acd70..000000000 --- a/plugins/codex-security/scripts/normalize_candidates.py +++ /dev/null @@ -1,338 +0,0 @@ -#!/usr/bin/env python3 -"""Validate and combine security-scan candidates into deterministic JSONL.""" - -from __future__ import annotations - -import argparse -import hashlib -import json -import re -import sys -import tempfile -from pathlib import Path, PurePosixPath -from typing import Any - -CWE = re.compile(r"(?i)CWE-(\d+)") -ROLES = { - "entrypoint": 0, - "entrypoint/wrapper": 1, - "source": 2, - "root_control": 3, - "sink": 4, - "concrete_implementation": 5, - "evidence": 6, -} -FIELDS = { - "candidate_id", - "cwe_ids", - "locations", - "summary", - "evidence", - "context", - "instance", -} - - -def text_field(row: dict[str, Any], field: str, *, required: bool = True) -> str | None: - value = row.get(field) - if value is None and not required: - return None - if not isinstance(value, str) or not value.strip(): - raise ValueError(f"{field}: expected a non-empty string") - return value.strip() - - -def cwe_ids(row: dict[str, Any]) -> list[str]: - value = row.get("cwe_ids") - if not isinstance(value, list): - raise ValueError("cwe_ids: expected an array") - found: set[int] = set() - for item in value: - if not isinstance(item, str): - raise ValueError("cwe_ids: expected CWE strings") - match = CWE.fullmatch(item.strip()) - if match is None or int(match[1]) < 1: - raise ValueError(f"cwe_ids: unsupported value {item!r}") - found.add(int(match[1])) - return [f"CWE-{number}" for number in sorted(found)] - - -def relative_file(value: Any, repo_root: Path) -> tuple[str, Path]: - if not isinstance(value, str) or not value or "\0" in value: - raise ValueError("path: expected a non-empty repository-relative path") - raw = value - if sys.platform == "win32": - raw = raw.replace("\\", "/") - path = PurePosixPath(raw) - if ( - path.is_absolute() - or ".." in path.parts - or (sys.platform == "win32" and re.match(r"^[A-Za-z]:", raw)) - ): - raise ValueError("path: expected a repository-relative path without traversal") - resolved = (repo_root / raw).resolve(strict=True) - try: - relative = resolved.relative_to(repo_root).as_posix() - except ValueError as error: - raise ValueError("path: must resolve inside --repo-root") from error - if not resolved.is_file(): - raise ValueError("path: expected a regular file") - return relative, resolved - - -def positive_line(value: Any, field: str) -> int: - if not isinstance(value, int) or isinstance(value, bool) or value < 1: - raise ValueError(f"{field}: expected a positive integer") - return value - - -def normalize_locations( - row: dict[str, Any], repo_root: Path, line_counts: dict[Path, int] -) -> list[dict[str, Any]]: - value = row.get("locations") - if not isinstance(value, list) or not value: - raise ValueError("locations: expected a non-empty array") - normalized: dict[tuple[str, int, int, str], dict[str, Any]] = {} - for item in value: - if not isinstance(item, dict): - raise ValueError("locations: expected location objects") - unknown = set(item) - {"path", "start_line", "end_line", "role"} - if unknown: - raise ValueError(f"locations: unsupported fields {', '.join(sorted(unknown))}") - relative, source = relative_file(item.get("path"), repo_root) - if ( - not relative.strip() - or "\\" in relative - or any(":" in part for part in PurePosixPath(relative).parts) - ): - raise ValueError("path: expected a safe repository-relative POSIX path") - start = positive_line(item.get("start_line"), "start_line") - end = positive_line(item.get("end_line", start), "end_line") - if end < start: - raise ValueError("end_line: must be greater than or equal to start_line") - if source not in line_counts: - line_counts[source] = len(source.read_bytes().splitlines()) - if end > line_counts[source]: - raise ValueError(f"line range {start}-{end} exceeds {relative}:{line_counts[source]}") - role = item.get("role") - if not isinstance(role, str) or role not in ROLES: - raise ValueError(f"role: unsupported value {role!r}") - key = (relative, start, end, role) - normalized[key] = { - "path": relative, - "start_line": start, - "end_line": end, - "role": role, - } - return sorted( - normalized.values(), - key=lambda item: ( - ROLES[item["role"]], - item["path"], - item["start_line"], - item["end_line"], - ), - ) - - -def read_scope(path: Path, repo_root: Path, *, allow_missing: bool = False) -> set[str]: - contents = path.read_bytes().decode("utf-8") - lines = contents.split("\n") - listed_rows = set(lines) - - def is_scope_file(value: str) -> bool: - try: - relative_file(value, repo_root) - except (OSError, ValueError): - return False - return True - - carriage_rows = { - line: (is_scope_file(line), is_scope_file(line.removesuffix("\r"))) - for line in lines - if line.endswith("\r") and line != "\r" and sys.platform != "win32" - } - crlf_evidence = any(line == "\r" for line in lines) or any( - stripped and not literal for literal, stripped in carriage_rows.values() - ) - literal_evidence = any(literal and not stripped for literal, stripped in carriage_rows.values()) - - scope: set[str] = set() - for number, line in enumerate(lines, 1): - if sys.platform == "win32" or line == "\r": - line = line.removesuffix("\r") - elif line.endswith("\r"): - literal, stripped = carriage_rows[line] - if stripped and not literal: - line = line.removesuffix("\r") - elif stripped and literal: - if number == len(lines) and not contents.endswith("\n"): - pass - elif line.removesuffix("\r") in listed_rows: - pass - elif crlf_evidence and not literal_evidence: - line = line.removesuffix("\r") - elif literal_evidence and not crlf_evidence: - pass - else: - raise ValueError(f"in-scope file row {number}: ambiguous carriage-return paths") - elif not literal and crlf_evidence: - line = line.removesuffix("\r") - if not line: - continue - try: - relative, _ = relative_file(line, repo_root) - except (OSError, ValueError) as error: - if allow_missing and isinstance(error, FileNotFoundError): - candidate = PurePosixPath(line) - if candidate.is_absolute() or ".." in candidate.parts or "\0" in line: - raise ValueError(f"in-scope file row {number}: unsafe deleted path") from error - resolved = (repo_root / line).resolve(strict=False) - try: - relative = resolved.relative_to(repo_root).as_posix() - except ValueError as escaped: - raise ValueError( - f"in-scope file row {number}: path escapes repository" - ) from escaped - scope.add(relative) - continue - raise ValueError(f"in-scope file row {number}: {error}") from error - scope.add(relative) - return scope - - -def normalize_candidate( - row: dict[str, Any], repo_root: Path, scope: set[str], line_counts: dict[Path, int] -) -> dict[str, Any]: - unknown = set(row) - FIELDS - if unknown: - raise ValueError(f"unsupported fields {', '.join(sorted(unknown))}") - if "candidate_id" in row: - text_field(row, "candidate_id") - candidate_locations = normalize_locations(row, repo_root, line_counts) - if not any(item["path"] in scope for item in candidate_locations): - raise ValueError("locations: expected at least one in-scope file") - result: dict[str, Any] = { - "cwe_ids": cwe_ids(row), - "locations": candidate_locations, - "summary": text_field(row, "summary"), - "evidence": text_field(row, "evidence"), - } - context = text_field(row, "context", required=False) - if context is not None: - result["context"] = context - instance = text_field(row, "instance", required=False) - if instance is not None: - result["instance"] = instance - return result - - -def identity(row: dict[str, Any]) -> str: - return json.dumps( - { - "cwe_ids": row["cwe_ids"], - "locations": row["locations"], - "instance": row.get("instance"), - }, - ensure_ascii=False, - separators=(",", ":"), - sort_keys=True, - ) - - -def merged_text(group: list[dict[str, Any]], field: str) -> str: - return "\n".join(sorted({item[field] for item in group if field in item})) - - -def combine(rows: list[dict[str, Any]]) -> list[dict[str, Any]]: - groups: dict[str, list[dict[str, Any]]] = {} - for row in rows: - groups.setdefault(identity(row), []).append(row) - combined: list[dict[str, Any]] = [] - for key, group in sorted(groups.items()): - candidate_id = hashlib.sha256(key.encode()).hexdigest()[:16] - - result = { - "candidate_id": f"candidate-{candidate_id}", - "cwe_ids": group[0]["cwe_ids"], - "locations": group[0]["locations"], - "summary": merged_text(group, "summary"), - "evidence": merged_text(group, "evidence"), - } - context = merged_text(group, "context") - if context: - result["context"] = context - if "instance" in group[0]: - result["instance"] = group[0]["instance"] - combined.append(result) - return combined - - -def main() -> None: - parser = argparse.ArgumentParser(description=__doc__) - parser.add_argument("--input", nargs="+", required=True, help="Candidate JSONL inputs.") - parser.add_argument("--out", required=True, help="Combined candidate JSONL output.") - parser.add_argument("--repo-root", required=True, help="Repository root for candidate paths.") - parser.add_argument("--in-scope-files", required=True, help="Repository-relative file list.") - parser.add_argument( - "--allow-missing-in-scope", - action="store_true", - help="Keep deleted Git paths in a diff inventory while validating existing candidate files.", - ) - args = parser.parse_args() - try: - repo_root = Path(args.repo_root).expanduser().resolve(strict=True) - if not repo_root.is_dir(): - raise ValueError("--repo-root: expected a directory") - output = Path(args.out).expanduser().resolve(strict=False) - scope_path = Path(args.in_scope_files).expanduser().resolve(strict=True) - inputs = sorted({Path(value).expanduser().resolve(strict=True) for value in args.input}) - if output in inputs: - raise ValueError("--out: must not also be an input") - if output == scope_path: - raise ValueError("--out: must not replace --in-scope-files") - scope = read_scope(scope_path, repo_root, allow_missing=args.allow_missing_in_scope) - line_counts: dict[Path, int] = {} - rows: list[dict[str, Any]] = [] - for source in inputs: - with source.open(encoding="utf-8") as handle: - for number, line in enumerate(handle, 1): - if not line.strip(): - continue - try: - row = json.loads(line) - if not isinstance(row, dict): - raise ValueError("expected a JSON object") - rows.append(normalize_candidate(row, repo_root, scope, line_counts)) - except (ValueError, TypeError, OSError) as error: - raise ValueError(f"{source} row {number}: {error}") from error - combined = combine(rows) - output.parent.mkdir(parents=True, exist_ok=True) - temporary: Path | None = None - try: - with tempfile.NamedTemporaryFile( - mode="w", - encoding="utf-8", - dir=output.parent, - prefix=f".{output.name}.", - suffix=".tmp", - delete=False, - ) as handle: - temporary = Path(handle.name) - for row in combined: - handle.write( - json.dumps(row, ensure_ascii=False, separators=(",", ":"), sort_keys=True) - + "\n" - ) - temporary.replace(output) - finally: - if temporary is not None: - temporary.unlink(missing_ok=True) - print(f"Combined {len(rows)} candidate rows into {len(combined)} rows in {output}") - except (OSError, ValueError) as error: - print(f"normalize_candidates: {error}", file=sys.stderr) - raise SystemExit(2) from error - - -if __name__ == "__main__": - main() diff --git a/plugins/codex-security/scripts/normalize_candidates.ts b/plugins/codex-security/scripts/normalize_candidates.ts new file mode 100644 index 000000000..264747d26 --- /dev/null +++ b/plugins/codex-security/scripts/normalize_candidates.ts @@ -0,0 +1,374 @@ +import { createHash } from "node:crypto"; +import { + mkdirSync, + mkdtempSync, + readFileSync, + realpathSync, + renameSync, + rmSync, + statSync, + writeFileSync, +} from "node:fs"; +import { dirname, isAbsolute, join, relative, resolve, sep } from "node:path"; +import { fileURLToPath } from "node:url"; +import { parseArgs } from "node:util"; + +const ROLES = [ + "entrypoint", + "entrypoint/wrapper", + "source", + "root_control", + "sink", + "concrete_implementation", + "evidence", +] as const; +const FIELDS = new Set([ + "candidate_id", + "cwe_ids", + "locations", + "summary", + "evidence", + "context", + "instance", +]); +const LOCATION_FIELDS = new Set(["path", "start_line", "end_line", "role"]); + +interface Location { + path: string; + start_line: number; + end_line: number; + role: (typeof ROLES)[number]; +} + +interface NormalizedCandidate { + cwe_ids: string[]; + locations: Location[]; + summary: string; + evidence: string; + context?: string; + instance?: string; +} + +type CombinedCandidate = NormalizedCandidate & { candidate_id: string }; + +function object( + value: unknown, + fields: ReadonlySet, +): Record { + if (value === null || typeof value !== "object" || Array.isArray(value)) { + throw new Error("expected an object"); + } + const extra = Object.keys(value).filter((key) => !fields.has(key)); + if (extra.length) throw new Error(`unsupported fields: ${extra.join(", ")}`); + return value as Record; +} + +function text(value: unknown, field: string): string { + if (typeof value !== "string" || !value.trim()) { + throw new Error(`${field}: expected a non-empty string`); + } + return value.trim(); +} + +function positiveLine(value: unknown, field: string): number { + if (typeof value !== "number" || !Number.isInteger(value) || value < 1) { + throw new Error(`${field}: expected a positive integer`); + } + return value; +} + +function message(error: unknown): string { + return error instanceof Error ? error.message : String(error); +} + +function isMissingFile(error: unknown): boolean { + return error instanceof Error && "code" in error && error.code === "ENOENT"; +} + +function readLines(path: string): string[] { + return new TextDecoder("utf-8", { fatal: true }) + .decode(readFileSync(path)) + .split(/\r?\n/u); +} + +function repoFile(value: unknown, repoRoot: string) { + if (typeof value !== "string" || !value || value.includes("\0")) { + throw new Error("path: expected a repository-relative file"); + } + const raw = + process.platform === "win32" ? value.replaceAll("\\", "/") : value; + if ( + isAbsolute(raw) || + raw.split("/").includes("..") || + (process.platform === "win32" && /^[A-Za-z]:/u.test(raw)) + ) { + throw new Error( + "path: expected a repository-relative path without traversal", + ); + } + const source = realpathSync.native(resolve(repoRoot, raw)); + const path = relative(repoRoot, source); + if (path === ".." || path.startsWith(`..${sep}`) || isAbsolute(path)) { + throw new Error("path: must resolve inside --repo-root"); + } + if (!statSync(source).isFile()) + throw new Error("path: expected a regular file"); + return { path: path.split(sep).join("/"), source }; +} + +function cweIds(value: unknown): string[] { + if (!Array.isArray(value)) throw new Error("cwe_ids: expected an array"); + const found = new Set(); + for (const item of value) { + const match = typeof item === "string" && /^CWE-(\d+)$/iu.exec(item.trim()); + if (!match || BigInt(match[1]!) < 1n) { + throw new Error(`cwe_ids: unsupported value ${JSON.stringify(item)}`); + } + found.add(BigInt(match[1]!)); + } + return [...found] + .sort((left, right) => (left < right ? -1 : left > right ? 1 : 0)) + .map((number) => `CWE-${number}`); +} + +function countLines(path: string): number { + const lines = readFileSync(path, "utf8").split(/\r\n|\r|\n/u); + return lines.length - (lines.at(-1) === "" ? 1 : 0); +} + +function compareStrings(left: string, right: string): number { + return left < right ? -1 : left > right ? 1 : 0; +} + +function normalizeLocations( + value: unknown, + repoRoot: string, + lineCounts: Map, +): Location[] { + if (!Array.isArray(value) || value.length === 0) { + throw new Error("locations: expected a non-empty array"); + } + const locations = new Map(); + for (const raw of value) { + const item = object(raw, LOCATION_FIELDS); + const { path, source } = repoFile(item["path"], repoRoot); + if (!path.trim() || /[\\:]/u.test(path)) { + throw new Error("path: expected a safe repository-relative POSIX path"); + } + const start = positiveLine(item["start_line"], "start_line"); + const end = + item["end_line"] === undefined + ? start + : positiveLine(item["end_line"], "end_line"); + const lineCount = lineCounts.get(source) ?? countLines(source); + lineCounts.set(source, lineCount); + if (end < start || end > lineCount) { + throw new Error( + `invalid line range ${start}-${end} for ${path} (${lineCount} lines)`, + ); + } + const role = ROLES.find((role) => role === item["role"]); + if (role === undefined) throw new Error("role: unsupported value"); + const location = { path, start_line: start, end_line: end, role }; + locations.set(JSON.stringify(location), location); + } + return [...locations.values()].sort( + (left, right) => + ROLES.indexOf(left.role) - ROLES.indexOf(right.role) || + compareStrings(left.path, right.path) || + left.start_line - right.start_line || + left.end_line - right.end_line, + ); +} + +function readScope( + path: string, + repoRoot: string, + allowMissing: boolean, +): Set { + const scope = new Set(); + for (const [index, line] of readLines(path).entries()) { + if (!line) continue; + try { + scope.add(repoFile(line, repoRoot).path); + } catch (error) { + if (!allowMissing || !isMissingFile(error)) { + throw new Error(`in-scope file row ${index + 1}: ${message(error)}`); + } + } + } + return scope; +} + +export function normalizeCandidate( + value: unknown, + repoRoot: string, + scope: Set, + lineCounts: Map, +): NormalizedCandidate { + const row = object(value, FIELDS); + const locations = normalizeLocations(row["locations"], repoRoot, lineCounts); + if (!locations.some((location) => scope.has(location.path))) { + throw new Error("locations: expected at least one in-scope file"); + } + const result: NormalizedCandidate = { + cwe_ids: cweIds(row["cwe_ids"]), + locations, + summary: text(row["summary"], "summary"), + evidence: text(row["evidence"], "evidence"), + }; + for (const field of ["context", "instance"] as const) { + if (row[field] != null) result[field] = text(row[field], field); + } + return result; +} + +function identity({ + cwe_ids, + locations, + instance, +}: NormalizedCandidate): string { + return JSON.stringify({ cwe_ids, locations, instance }); +} + +function mergedText( + rows: NormalizedCandidate[], + field: "summary" | "evidence" | "context", +): string { + const values = rows + .map((row) => row[field]) + .filter((value) => value !== undefined); + return [...new Set(values)].sort().join("\n"); +} + +export function combine(rows: NormalizedCandidate[]): CombinedCandidate[] { + return [...Map.groupBy(rows, identity)] + .sort(([left], [right]) => compareStrings(left, right)) + .map(([key, group]) => { + const { cwe_ids, locations, instance } = group[0]!; + return { + candidate_id: `candidate-${createHash("sha256").update(key).digest("hex").slice(0, 16)}`, + cwe_ids, + locations, + summary: mergedText(group, "summary"), + evidence: mergedText(group, "evidence"), + context: mergedText(group, "context") || undefined, + instance, + }; + }); +} + +function writeCombined( + output: string, + rows: CombinedCandidate[], + inputs: string[], +): void { + try { + const target = realpathSync.native(output); + if (inputs.some((input) => relative(input, target) === "")) { + throw new Error("--out: must not replace an input or the scope file"); + } + } catch (error) { + if (!isMissingFile(error)) throw error; + } + mkdirSync(dirname(output), { recursive: true }); + const directory = mkdtempSync(join(dirname(output), ".normalize-")); + const temporary = join(directory, "candidates.jsonl"); + try { + writeFileSync( + temporary, + rows.map((row) => `${JSON.stringify(row)}\n`).join(""), + { mode: 0o600 }, + ); + renameSync(temporary, output); + } finally { + rmSync(directory, { recursive: true, force: true }); + } +} + +const HELP = `Validate and combine security-scan candidates into deterministic JSONL. + +Usage: normalize_candidates.mjs --input [--input ...] --out --repo-root --in-scope-files [--allow-missing-in-scope] + +Input and scope files use LF or CRLF lines. Missing scope entries are skipped when allowed. +Use full option names. The invoking shell expands home paths.`; + +function main(argv: string[]): void { + const { values } = parseArgs({ + args: argv, + options: { + input: { type: "string", multiple: true }, + out: { type: "string" }, + "repo-root": { type: "string" }, + "in-scope-files": { type: "string" }, + "allow-missing-in-scope": { type: "boolean", default: false }, + help: { type: "boolean", short: "h" }, + }, + }); + if (values.help) { + console.log(HELP); + return; + } + const { + input: inputFiles, + out, + "repo-root": root, + "in-scope-files": scopeFile, + } = values; + if ( + !inputFiles?.length || + out === undefined || + root === undefined || + scopeFile === undefined + ) { + throw new Error( + "--input, --out, --repo-root and --in-scope-files are required", + ); + } + const repoRoot = realpathSync.native(resolve(root)); + if (!statSync(repoRoot).isDirectory()) + throw new Error("--repo-root: expected a directory"); + const scopePath = realpathSync.native(resolve(scopeFile)); + const inputs = [ + ...new Set(inputFiles.map((path) => realpathSync.native(resolve(path)))), + ]; + const output = resolve(out); + const scope = readScope( + scopePath, + repoRoot, + values["allow-missing-in-scope"], + ); + const lineCounts = new Map(); + const rows: NormalizedCandidate[] = []; + for (const source of inputs) { + for (const [index, line] of readLines(source).entries()) { + if (!line.trim()) continue; + try { + rows.push( + normalizeCandidate(JSON.parse(line), repoRoot, scope, lineCounts), + ); + } catch (error) { + throw new Error(`${source} row ${index + 1}: ${message(error)}`); + } + } + } + const combined = combine(rows); + writeCombined(output, combined, [...inputs, scopePath]); + console.log( + `Combined ${rows.length} candidate rows into ${combined.length} rows in ${output}`, + ); +} + +const entrypoint = process.argv[1]; +if ( + entrypoint !== undefined && + realpathSync.native(fileURLToPath(import.meta.url)) === + realpathSync.native(entrypoint) +) { + try { + main(process.argv.slice(2)); + } catch (error) { + console.error(`normalize_candidates: ${message(error)}`); + process.exitCode = 2; + } +} diff --git a/plugins/codex-security/skills/security-diff-scan/SKILL.md b/plugins/codex-security/skills/security-diff-scan/SKILL.md index af59e39c2..10faac5b8 100644 --- a/plugins/codex-security/skills/security-diff-scan/SKILL.md +++ b/plugins/codex-security/skills/security-diff-scan/SKILL.md @@ -34,6 +34,6 @@ For terminal scans without a `scanId`, generate the changed-file list with: /scripts/generate_in_scope_files.py --repo --scope . --diff-base --diff-head --diff-mode --out /in_scope_files.txt ``` -Record candidates with `normalize_candidates.py --input --out /candidate_ledger.jsonl --repo-root --in-scope-files /in_scope_files.txt --allow-missing-in-scope`. Add validation and attack-path decisions to that same file. Following `../../references/final-report.md`, assemble unsealed `scan-manifest.json`, `findings.json`, and `coverage.json` before running `finalize_scan_contract.py --scan-dir --source-root `. +Record candidates with `node normalize_candidates.mjs --input --out /candidate_ledger.jsonl --repo-root --in-scope-files /in_scope_files.txt --allow-missing-in-scope`. Repeat `--input ` for additional inputs. Add validation and attack-path decisions to that same file. Following `../../references/final-report.md`, assemble unsealed `scan-manifest.json`, `findings.json`, and `coverage.json` before running `finalize_scan_contract.py --scan-dir --source-root `. Finish only after every changed file and candidate is accounted for. Return the generated report, actual coverage gaps, and Codex review comments for confirmed findings. diff --git a/plugins/codex-security/tests/test_normalize_candidates.py b/plugins/codex-security/tests/test_normalize_candidates.py deleted file mode 100644 index 94290fcb6..000000000 --- a/plugins/codex-security/tests/test_normalize_candidates.py +++ /dev/null @@ -1,789 +0,0 @@ -from __future__ import annotations - -import json -import subprocess -import sys -from collections.abc import Callable -from pathlib import Path - -import pytest - -SCRIPT = Path(__file__).resolve().parents[1] / "scripts" / "normalize_candidates.py" - - -def write_jsonl(path: Path, rows: list[dict[str, object]]) -> None: - path.write_text("".join(json.dumps(row) + "\n" for row in rows), encoding="utf-8") - - -def location(path: str, line: int, role: str) -> dict[str, object]: - return {"path": path, "start_line": line, "role": role} - - -def candidate( - locations: list[dict[str, object]], - *, - cwes: list[str] | None = None, - summary: str = "Request input reaches an unsafe operation", - evidence: str = "The input is passed to the operation without a check", - context: str | None = None, - instance: str | None = None, -) -> dict[str, object]: - row: dict[str, object] = { - "cwe_ids": ["CWE-89"] if cwes is None else cwes, - "locations": locations, - "summary": summary, - "evidence": evidence, - } - if context is not None: - row["context"] = context - if instance is not None: - row["instance"] = instance - return row - - -def setup_repo(tmp_path: Path) -> tuple[Path, Path]: - repo = tmp_path / "repo" - for path in ("app/routes.py", "app/query.py", "app/export.py", "helpers/shared.py"): - source = repo / path - source.parent.mkdir(parents=True, exist_ok=True) - source.write_text("one\ntwo\nthree\nfour\nfive\n", encoding="utf-8") - scope = tmp_path / "in_scope_files.txt" - scope.write_text("app/routes.py\napp/query.py\napp/export.py\n", encoding="utf-8") - return repo, scope - - -def run_combiner( - tmp_path: Path, - inputs: list[list[dict[str, object]]], - *, - repo: Path | None = None, - scope: Path | None = None, - output_name: str = "combined.jsonl", - allow_missing: bool = False, -) -> tuple[subprocess.CompletedProcess[str], Path]: - if repo is None or scope is None: - repo, scope = setup_repo(tmp_path) - sources: list[Path] = [] - for index, rows in enumerate(inputs): - source = tmp_path / f"candidates-{index}.jsonl" - write_jsonl(source, rows) - sources.append(source) - output = tmp_path / output_name - result = subprocess.run( - [ - sys.executable, - str(SCRIPT), - "--input", - *(str(source) for source in sources), - "--out", - str(output), - "--repo-root", - str(repo), - "--in-scope-files", - str(scope), - *(["--allow-missing-in-scope"] if allow_missing else []), - ], - capture_output=True, - text=True, - ) - return result, output - - -def read_jsonl(path: Path) -> list[dict[str, object]]: - return [json.loads(line) for line in path.read_text(encoding="utf-8").split("\n") if line] - - -@pytest.mark.skipif(sys.platform == "win32", reason="POSIX-only literal file names") -@pytest.mark.parametrize( - "relative", - [ - "app/ leading.py", - "app/trailing .py", - "app/ .py", - "app/ .py", - "app/carriage\rname.py", - "app/trailing-carriage.py\r", - "app/vertical\vname.py", - "app/form\fname.py", - "app/next\u0085name.py", - "app/line\u2028name.py", - "app/paragraph\u2029name.py", - ], -) -def test_scope_preserves_literal_posix_inventory_paths(tmp_path: Path, relative: str) -> None: - repo, scope = setup_repo(tmp_path) - source = repo / relative - source.write_text("one\ntwo\n", encoding="utf-8") - scope.write_bytes((relative + "\n").encode("utf-8")) - - result, output = run_combiner( - tmp_path, - [[candidate([location(relative, 1, "entrypoint")])]], - repo=repo, - scope=scope, - ) - - assert result.returncode == 0, result.stderr - assert read_jsonl(output)[0]["locations"][0]["path"] == relative - - -def test_scope_accepts_crlf_inventory_on_every_platform(tmp_path: Path) -> None: - repo, scope = setup_repo(tmp_path) - scope.write_bytes(b"\r\napp/routes.py\r\napp/query.py\r\n") - - result, output = run_combiner( - tmp_path, - [[candidate([location("app/routes.py", 1, "entrypoint")])]], - repo=repo, - scope=scope, - ) - - assert result.returncode == 0, result.stderr - assert read_jsonl(output)[0]["locations"][0]["path"] == "app/routes.py" - - -def test_diff_scope_accepts_deleted_files_without_weakening_candidate_locations( - tmp_path: Path, -) -> None: - repo, scope = setup_repo(tmp_path) - scope.write_text("app/deleted_guard.py\napp/routes.py\n", encoding="utf-8") - - rejected, _ = run_combiner( - tmp_path, - [[candidate([location("app/routes.py", 1, "entrypoint")])]], - repo=repo, - scope=scope, - ) - assert rejected.returncode == 2 - assert "in-scope file row 1" in rejected.stderr - - accepted, output = run_combiner( - tmp_path, - [[candidate([location("app/routes.py", 1, "entrypoint")])]], - repo=repo, - scope=scope, - allow_missing=True, - ) - assert accepted.returncode == 0, accepted.stderr - assert read_jsonl(output)[0]["locations"][0]["path"] == "app/routes.py" - - missing_candidate, _ = run_combiner( - tmp_path, - [[candidate([location("app/deleted_guard.py", 1, "root_control")])]], - repo=repo, - scope=scope, - output_name="missing-candidate.jsonl", - allow_missing=True, - ) - assert missing_candidate.returncode == 2 - assert "deleted_guard.py" in missing_candidate.stderr - - -def test_diff_scope_rejects_deleted_path_outside_repository(tmp_path: Path) -> None: - repo, scope = setup_repo(tmp_path) - scope.write_text("../deleted_guard.py\napp/routes.py\n", encoding="utf-8") - - result, output = run_combiner( - tmp_path, - [[candidate([location("app/routes.py", 1, "entrypoint")])]], - repo=repo, - scope=scope, - allow_missing=True, - ) - - assert result.returncode == 2 - assert "in-scope file row 1" in result.stderr - assert not output.exists() - - -@pytest.mark.parametrize( - "inventory", - [ - b"app/routes.py\r\napp/query.py\n", - b"app/routes.py\r\napp/query.py", - ], -) -def test_scope_accepts_mixed_and_unterminated_crlf_inventories( - tmp_path: Path, inventory: bytes -) -> None: - repo, scope = setup_repo(tmp_path) - scope.write_bytes(inventory) - - result, output = run_combiner( - tmp_path, - [[candidate([location("app/routes.py", 1, "entrypoint")])]], - repo=repo, - scope=scope, - ) - - assert result.returncode == 0, result.stderr - assert read_jsonl(output)[0]["locations"][0]["path"] == "app/routes.py" - - -@pytest.mark.skipif(sys.platform == "win32", reason="POSIX-only literal file names") -def test_crlf_scope_preserves_paths_when_carriage_return_names_also_exist( - tmp_path: Path, -) -> None: - repo, scope = setup_repo(tmp_path) - (repo / "app/routes.py\r").write_text("one\ntwo\n", encoding="utf-8") - scope.write_bytes(b"app/routes.py\r\napp/query.py\r\n") - - result, output = run_combiner( - tmp_path, - [[candidate([location("app/routes.py", 1, "entrypoint")])]], - repo=repo, - scope=scope, - ) - - assert result.returncode == 0, result.stderr - assert read_jsonl(output)[0]["locations"][0]["path"] == "app/routes.py" - - -@pytest.mark.skipif(sys.platform == "win32", reason="POSIX-only literal file names") -@pytest.mark.parametrize("candidate_path", ["app/routes.py", "app/routes.py\r"]) -def test_lf_scope_preserves_separately_listed_carriage_return_collisions( - tmp_path: Path, candidate_path: str -) -> None: - repo, scope = setup_repo(tmp_path) - for relative in ("app/routes.py\r", "app/query.py\r"): - (repo / relative).write_text("one\ntwo\n", encoding="utf-8") - scope.write_bytes(b"app/query.py\napp/query.py\r\napp/routes.py\napp/routes.py\r\n") - - result, output = run_combiner( - tmp_path, - [[candidate([location(candidate_path, 1, "entrypoint")])]], - repo=repo, - scope=scope, - ) - - assert result.returncode == 0, result.stderr - assert read_jsonl(output)[0]["locations"][0]["path"] == candidate_path - - -@pytest.mark.skipif(sys.platform == "win32", reason="POSIX-only literal file names") -def test_scope_preserves_unterminated_final_carriage_return_filename( - tmp_path: Path, -) -> None: - repo, scope = setup_repo(tmp_path) - relative = "app/routes.py\r" - (repo / relative).write_text("one\ntwo\n", encoding="utf-8") - scope.write_bytes(b"app/query.py\r\napp/routes.py\r") - - result, output = run_combiner( - tmp_path, - [[candidate([location(relative, 1, "entrypoint")])]], - repo=repo, - scope=scope, - ) - - assert result.returncode == 0, result.stderr - assert read_jsonl(output)[0]["locations"][0]["path"] == relative - - -@pytest.mark.skipif(sys.platform == "win32", reason="POSIX-only literal file names") -@pytest.mark.parametrize( - "relative", - ["app/literal\\name.py", "app/C:foo.py", " ", " "], -) -def test_candidate_rejects_paths_incompatible_with_scan_contract( - tmp_path: Path, relative: str -) -> None: - repo, scope = setup_repo(tmp_path) - source = repo / relative - source.write_text("one\ntwo\n", encoding="utf-8") - scope.write_bytes((relative + "\n").encode("utf-8")) - - result, output = run_combiner( - tmp_path, - [[candidate([location(relative, 1, "entrypoint")])]], - repo=repo, - scope=scope, - ) - - assert result.returncode == 2 - assert "safe repository-relative POSIX path" in result.stderr - assert not output.exists() - - -@pytest.mark.skipif(sys.platform == "win32", reason="POSIX-only literal file names") -@pytest.mark.parametrize("candidate_path", ["app/routes.py", "app/routes.py\r"]) -def test_mixed_scope_rejects_colliding_carriage_return_file_names( - tmp_path: Path, candidate_path: str -) -> None: - repo, scope = setup_repo(tmp_path) - (repo / "app/routes.py\r").write_text("one\ntwo\n", encoding="utf-8") - scope.write_bytes(b"app/routes.py\r\napp/query.py\n") - - result, output = run_combiner( - tmp_path, - [[candidate([location(candidate_path, 1, "entrypoint")])]], - repo=repo, - scope=scope, - ) - - assert result.returncode == 2 - assert "ambiguous carriage-return paths" in result.stderr - assert not output.exists() - - -@pytest.mark.skipif(sys.platform == "win32", reason="POSIX-only literal file names") -def test_lf_scope_uses_independent_literal_carriage_return_evidence( - tmp_path: Path, -) -> None: - repo, scope = setup_repo(tmp_path) - relative = "app/routes.py\r" - evidence = "app/literal-evidence.py\r" - for path in (relative, evidence): - (repo / path).write_text("one\ntwo\n", encoding="utf-8") - scope.write_bytes(b"app/routes.py\r\napp/literal-evidence.py\r\napp/query.py\n") - - result, output = run_combiner( - tmp_path, - [[candidate([location(relative, 1, "entrypoint")])]], - repo=repo, - scope=scope, - ) - - assert result.returncode == 0, result.stderr - assert read_jsonl(output)[0]["locations"][0]["path"] == relative - - -@pytest.mark.skipif(sys.platform == "win32", reason="POSIX-only literal file names") -def test_scope_rejects_indistinguishable_carriage_return_inventories( - tmp_path: Path, -) -> None: - repo, scope = setup_repo(tmp_path) - for relative in ("app/routes.py\r", "app/query.py\r"): - (repo / relative).write_text("one\ntwo\n", encoding="utf-8") - scope.write_bytes(b"app/routes.py\r\napp/query.py\r\n") - - result, output = run_combiner( - tmp_path, - [[candidate([location("app/routes.py", 1, "entrypoint")])]], - repo=repo, - scope=scope, - ) - - assert result.returncode == 2 - assert "ambiguous carriage-return paths" in result.stderr - assert not output.exists() - - -@pytest.mark.skipif(sys.platform == "win32", reason="POSIX-only literal file names") -def test_crlf_blank_line_disambiguates_colliding_carriage_return_paths( - tmp_path: Path, -) -> None: - repo, scope = setup_repo(tmp_path) - for relative in ("app/routes.py\r", "app/query.py\r"): - (repo / relative).write_text("one\ntwo\n", encoding="utf-8") - scope.write_bytes(b"\r\napp/routes.py\r\napp/query.py\r\n") - - result, output = run_combiner( - tmp_path, - [[candidate([location("app/routes.py", 1, "entrypoint")])]], - repo=repo, - scope=scope, - ) - - assert result.returncode == 0, result.stderr - assert read_jsonl(output)[0]["locations"][0]["path"] == "app/routes.py" - - -@pytest.mark.skipif(sys.platform != "win32", reason="Windows-native file paths") -def test_windows_scope_normalizes_native_separators(tmp_path: Path) -> None: - repo, scope = setup_repo(tmp_path) - scope.write_bytes(b"app\\routes.py\r\n") - - result, output = run_combiner( - tmp_path, - [[candidate([location("app\\routes.py", 2, "entrypoint")])]], - repo=repo, - scope=scope, - ) - - assert result.returncode == 0, result.stderr - assert read_jsonl(output)[0]["locations"][0]["path"] == "app/routes.py" - - -@pytest.mark.skipif(sys.platform != "win32", reason="Windows-native drive paths") -def test_windows_candidates_reject_drive_qualified_paths(tmp_path: Path) -> None: - result, output = run_combiner( - tmp_path, - [[candidate([location("C:routes.py", 1, "entrypoint")])]], - ) - - assert result.returncode == 2 - assert "repository-relative path without traversal" in result.stderr - assert not output.exists() - - -def test_matching_code_paths_combine_even_when_the_prose_differs( - tmp_path: Path, -) -> None: - first = candidate( - [ - location("app/query.py", 4, "sink"), - location("app/routes.py", 2, "entrypoint"), - location("app/query.py", 3, "root_control"), - ], - cwes=["cwe-089", "CWE-89"], - summary="The request id reaches an interpolated query", - evidence="The query uses an f-string", - context="Intentional training application", - ) - second = candidate( - [ - location("app/query.py", 3, "root_control"), - location("app/routes.py", 2, "entrypoint"), - location("app/query.py", 4, "sink"), - location("app/query.py", 4, "sink"), - ], - summary="SQL injection is reachable from the request", - evidence="The id is inserted directly into SQL", - context="Runs only on localhost", - ) - - result, output = run_combiner(tmp_path, [[first], [second]]) - - assert result.returncode == 0, result.stderr - rows = read_jsonl(output) - assert len(rows) == 1 - assert rows[0]["candidate_id"].startswith("candidate-") - assert rows[0]["cwe_ids"] == ["CWE-89"] - assert rows[0]["locations"] == [ - {"path": "app/routes.py", "start_line": 2, "end_line": 2, "role": "entrypoint"}, - { - "path": "app/query.py", - "start_line": 3, - "end_line": 3, - "role": "root_control", - }, - {"path": "app/query.py", "start_line": 4, "end_line": 4, "role": "sink"}, - ] - assert rows[0]["summary"] == ( - "SQL injection is reachable from the request\nThe request id reaches an interpolated query" - ) - assert rows[0]["evidence"] == "The id is inserted directly into SQL\nThe query uses an f-string" - assert rows[0]["context"] == "Intentional training application\nRuns only on localhost" - - -@pytest.mark.parametrize( - "first_locations,second_locations", - [ - ( - [ - location("app/routes.py", 2, "entrypoint"), - location("helpers/shared.py", 3, "root_control"), - location("app/query.py", 4, "sink"), - ], - [ - location("app/routes.py", 2, "entrypoint"), - location("helpers/shared.py", 3, "root_control"), - location("app/export.py", 4, "sink"), - ], - ), - ( - [ - location("app/routes.py", 2, "entrypoint"), - location("helpers/shared.py", 3, "root_control"), - location("app/query.py", 4, "sink"), - ], - [ - location("app/export.py", 2, "entrypoint"), - location("helpers/shared.py", 3, "root_control"), - location("app/query.py", 4, "sink"), - ], - ), - ], -) -def test_different_reachable_paths_remain_separate( - tmp_path: Path, - first_locations: list[dict[str, object]], - second_locations: list[dict[str, object]], -) -> None: - result, output = run_combiner( - tmp_path, - [[candidate(first_locations), candidate(second_locations)]], - ) - - assert result.returncode == 0, result.stderr - rows = read_jsonl(output) - assert len(rows) == 2 - assert rows[0]["candidate_id"] != rows[1]["candidate_id"] - - -def test_separate_bugs_at_the_same_locations_can_use_an_instance_label( - tmp_path: Path, -) -> None: - locations = [ - location("app/routes.py", 2, "entrypoint"), - location("app/query.py", 4, "sink"), - ] - first = candidate(locations, instance="id parameter") - second = candidate(locations, instance="sort parameter") - - result, output = run_combiner(tmp_path, [[first, second]]) - - assert result.returncode == 0, result.stderr - rows = read_jsonl(output) - assert len(rows) == 2 - assert {row["instance"] for row in rows} == {"id parameter", "sort parameter"} - assert rows[0]["candidate_id"] != rows[1]["candidate_id"] - - -def test_output_is_stable_when_input_and_row_order_changes(tmp_path: Path) -> None: - duplicate_one = candidate( - [ - location("app/routes.py", 2, "entrypoint"), - location("app/query.py", 4, "sink"), - ], - summary="Request id reaches SQL", - evidence="The query interpolates id", - ) - duplicate_two = candidate( - [ - location("app/query.py", 4, "sink"), - location("app/routes.py", 2, "entrypoint"), - ], - summary="SQL is built from request input", - evidence="The id is part of the query string", - ) - separate = candidate( - [ - location("app/export.py", 2, "entrypoint"), - location("app/export.py", 4, "sink"), - ], - cwes=["CWE-22"], - summary="The export path is unsafe", - evidence="The path is joined without a containment check", - ) - - first, first_output = run_combiner( - tmp_path, - [[duplicate_one, separate], [duplicate_two]], - output_name="first.jsonl", - ) - second, second_output = run_combiner( - tmp_path, - [[duplicate_two], [separate, duplicate_one]], - output_name="second.jsonl", - ) - - assert first.returncode == 0, first.stderr - assert second.returncode == 0, second.stderr - assert first_output.read_bytes() == second_output.read_bytes() - - -def test_combined_output_can_be_combined_again_without_changing_it( - tmp_path: Path, -) -> None: - repo, scope = setup_repo(tmp_path) - locations = [ - location("app/routes.py", 2, "entrypoint"), - location("app/query.py", 4, "sink"), - ] - first, first_output = run_combiner( - tmp_path, - [ - [ - candidate(locations, evidence="First trace"), - candidate(locations, evidence="Second trace"), - ] - ], - repo=repo, - scope=scope, - output_name="first.jsonl", - ) - second_output = tmp_path / "second.jsonl" - second = subprocess.run( - [ - sys.executable, - str(SCRIPT), - "--input", - str(first_output), - "--out", - str(second_output), - "--repo-root", - str(repo), - "--in-scope-files", - str(scope), - ], - capture_output=True, - text=True, - ) - - assert first.returncode == 0, first.stderr - assert second.returncode == 0, second.stderr - assert first_output.read_bytes() == second_output.read_bytes() - - -def test_multiline_candidate_text_keeps_its_original_order(tmp_path: Path) -> None: - summary = "Z: attacker reaches the route.\nA: the route then reaches the sink." - evidence = "Z: read the request.\nA: pass it into the query." - context = "Z: enabled in the demo.\nA: runs only on localhost." - row = candidate( - [location("app/routes.py", 2, "entrypoint")], - summary=summary, - evidence=evidence, - context=context, - ) - - result, output = run_combiner(tmp_path, [[row]]) - - assert result.returncode == 0, result.stderr - combined = read_jsonl(output)[0] - assert combined["summary"] == summary - assert combined["evidence"] == evidence - assert combined["context"] == context - - -def test_unknown_cwe_and_supporting_location_outside_scope_are_allowed( - tmp_path: Path, -) -> None: - row = candidate( - [ - location("app/routes.py", 2, "entrypoint"), - location("helpers/shared.py", 3, "root_control"), - ], - cwes=[], - ) - - result, output = run_combiner(tmp_path, [[row]]) - - assert result.returncode == 0, result.stderr - assert read_jsonl(output)[0]["cwe_ids"] == [] - - -@pytest.mark.parametrize( - ("change", "message"), - [ - (lambda row: row.pop("cwe_ids"), "cwe_ids: expected an array"), - ( - lambda row: row.update(cwe_ids=["SQL injection"]), - "cwe_ids: unsupported value", - ), - (lambda row: row.update(locations=[]), "locations: expected a non-empty array"), - ( - lambda row: row["locations"][0].update(path="app/missing.py"), - "missing.py", - ), - ( - lambda row: row["locations"][0].update(path="../outside.py"), - "repository-relative path without traversal", - ), - ( - lambda row: row["locations"][0].update(start_line=6), - "line range 6-6 exceeds app/routes.py:5", - ), - ( - lambda row: row["locations"][0].update(end_line=1), - "greater than or equal", - ), - ( - lambda row: row["locations"][0].update(role="rootControl"), - "role: unsupported value", - ), - ( - lambda row: row["locations"][0].update(line=2), - "locations: unsupported fields line", - ), - ( - lambda row: row.update(technically_validated=True), - "unsupported fields technically_validated", - ), - ( - lambda row: row.update(disposition="reportable"), - "unsupported fields disposition", - ), - ], -) -def test_invalid_candidates_fail_with_the_input_row( - tmp_path: Path, change: Callable[[dict[str, object]], object], message: str -) -> None: - valid = candidate([location("app/routes.py", 2, "entrypoint")]) - invalid = candidate([location("app/routes.py", 2, "entrypoint")]) - change(invalid) - - result, output = run_combiner(tmp_path, [[valid, invalid]]) - - assert result.returncode == 2 - assert "candidates-0.jsonl row 2:" in result.stderr - assert message in result.stderr - assert not output.exists() - - -def test_candidate_with_no_in_scope_location_fails(tmp_path: Path) -> None: - row = candidate([location("helpers/shared.py", 3, "root_control")]) - - result, output = run_combiner(tmp_path, [[row]]) - - assert result.returncode == 2 - assert "expected at least one in-scope file" in result.stderr - assert not output.exists() - - -def test_malformed_json_does_not_replace_an_existing_output(tmp_path: Path) -> None: - repo, scope = setup_repo(tmp_path) - source = tmp_path / "candidates.jsonl" - source.write_text( - json.dumps(candidate([location("app/routes.py", 2, "entrypoint")])) + "\nnot-json\n", - encoding="utf-8", - ) - output = tmp_path / "combined.jsonl" - output.write_text("previous output\n", encoding="utf-8") - - result = subprocess.run( - [ - sys.executable, - str(SCRIPT), - "--input", - str(source), - "--out", - str(output), - "--repo-root", - str(repo), - "--in-scope-files", - str(scope), - ], - capture_output=True, - text=True, - ) - - assert result.returncode == 2 - assert "candidates.jsonl row 2:" in result.stderr - assert output.read_text(encoding="utf-8") == "previous output\n" - - -def test_empty_candidate_input_produces_an_empty_ledger(tmp_path: Path) -> None: - result, output = run_combiner(tmp_path, [[]]) - - assert result.returncode == 0, result.stderr - assert output.read_text(encoding="utf-8") == "" - - -def test_output_cannot_replace_the_in_scope_file_list(tmp_path: Path) -> None: - repo, scope = setup_repo(tmp_path) - source = tmp_path / "candidates.jsonl" - write_jsonl(source, [candidate([location("app/routes.py", 2, "entrypoint")])]) - original_scope = scope.read_text(encoding="utf-8") - - result = subprocess.run( - [ - sys.executable, - str(SCRIPT), - "--input", - str(source), - "--out", - str(scope), - "--repo-root", - str(repo), - "--in-scope-files", - str(scope), - ], - capture_output=True, - text=True, - ) - - assert result.returncode == 2 - assert "--out: must not replace --in-scope-files" in result.stderr - assert scope.read_text(encoding="utf-8") == original_scope diff --git a/sdk/typescript/package.json b/sdk/typescript/package.json index fa937b0c9..0ac005273 100644 --- a/sdk/typescript/package.json +++ b/sdk/typescript/package.json @@ -48,7 +48,7 @@ "build:plugin": "node scripts/build-plugin.mjs", "check:plugin-source": "node scripts/check-plugin-source.mjs", "check:package": "node scripts/check-package.mjs", - "format": "prettier --check --ignore-path .gitignore --ignore-path .prettierignore \"**/*.{cjs,mjs,js,ts,tsx,json,md}\"", + "format": "prettier --check --ignore-path .gitignore --ignore-path .prettierignore \"**/*.{cjs,mjs,js,ts,tsx,json,md}\" \"../../plugins/codex-security/scripts/*.ts\"", "generate:models": "node scripts/generate-models.cjs", "generate:models:check": "node scripts/generate-models.cjs --check", "lint": "tsc --noEmit", diff --git a/sdk/typescript/scripts/build-plugin.mjs b/sdk/typescript/scripts/build-plugin.mjs index 7f8dbbbce..647a3a1c0 100644 --- a/sdk/typescript/scripts/build-plugin.mjs +++ b/sdk/typescript/scripts/build-plugin.mjs @@ -7,6 +7,7 @@ import { readFile, readdir, rm, + writeFile, } from "node:fs/promises"; import { dirname, join, posix, resolve } from "node:path"; import { fileURLToPath, pathToFileURL } from "node:url"; @@ -16,6 +17,7 @@ const scriptDirectory = dirname(fileURLToPath(import.meta.url)); const packageRoot = resolve(scriptDirectory, ".."); const repositoryRoot = resolve(packageRoot, "../.."); const publicManifest = ".codex-plugin/plugin.json"; +const normalizer = "scripts/normalize_candidates.mjs"; const execFileAsync = promisify(execFile); function sourcePath(root, relativePath) { @@ -89,7 +91,9 @@ export async function buildBundledPlugin({ throw new Error("Plugin projection contract contains duplicate paths."); } - const copiedPaths = files.filter((path) => !path.startsWith("mcp/")); + const copiedPaths = files.filter( + (path) => !path.startsWith("mcp/") && path !== normalizer, + ); const sourceFiles = await Promise.all( copiedPaths.map(async (path) => { const file = sourcePath(source, path); @@ -131,6 +135,25 @@ export async function buildBundledPlugin({ await chmod(output, mode); } + if (files.includes(normalizer)) { + const { transform } = await import("esbuild"); + const { code } = await transform( + await readFile( + sourcePath(source, "scripts/normalize_candidates.ts"), + "utf8", + ), + { + loader: "ts", + format: "esm", + target: "node22.13", + banner: "#!/usr/bin/env node", + }, + ); + const output = sourcePath(destination, normalizer); + await mkdir(dirname(output), { recursive: true }); + await writeFile(output, code, "utf8"); + } + const generated = await destinationFiles(destination); const expected = [...files].sort(); if ( diff --git a/sdk/typescript/src/custom-validation-prompt.ts b/sdk/typescript/src/custom-validation-prompt.ts index 5e04eddcc..8c5b0e99e 100644 --- a/sdk/typescript/src/custom-validation-prompt.ts +++ b/sdk/typescript/src/custom-validation-prompt.ts @@ -13,7 +13,7 @@ const SOURCES = { "skills/security-scan/SKILL.md": "5b8f5d7debeca14c6b37e8e7ba737671362b8eb4b7f49e693c99c6bd04bc8fa0", "skills/security-diff-scan/SKILL.md": - "0a4c519ad713585876ea7eb0a8af4b59892c86746f4c69851db9ab347b7fad2f", + "cafd0d2b3efe31ac5ffe04160333a580255471f9e999d27da7641f730ba83ffd", } as const; const DISABLED_TOOLS = [ diff --git a/sdk/typescript/tests-ts/build-plugin.test.ts b/sdk/typescript/tests-ts/build-plugin.test.ts index b1097d920..bf68ebeb9 100644 --- a/sdk/typescript/tests-ts/build-plugin.test.ts +++ b/sdk/typescript/tests-ts/build-plugin.test.ts @@ -167,6 +167,11 @@ describe("bundled plugin build", () => { await writeFixture(source, "scripts/launch", "#!/bin/sh\nexit 0\n"); await chmod(join(source, "scripts", "launch"), 0o755); await writeFixture(source, "schemas/scan.json", "{}\n"); + await writeFixture( + source, + "scripts/normalize_candidates.ts", + 'const message: string = "generated normalizer";\nconsole.log(message);\n', + ); await writeFixture( source, "mcp-app/package.json", @@ -202,6 +207,7 @@ await writeFile(join(output, "server.mjs"), "generated mcp runtime\\n"); "mcp/server.mjs", "schemas/scan.json", "scripts/launch", + "scripts/normalize_candidates.mjs", "sdk/typescript/owned-by-sdk.txt", ], }, @@ -217,6 +223,7 @@ await writeFile(join(output, "server.mjs"), "generated mcp runtime\\n"); "mcp/server.mjs", "schemas/scan.json", "scripts/launch", + "scripts/normalize_candidates.mjs", ]); expect( await readFile(join(destination, "schemas", "scan.json"), "utf8"), @@ -224,6 +231,10 @@ await writeFile(join(output, "server.mjs"), "generated mcp runtime\\n"); expect(await readFile(join(destination, "mcp", "server.mjs"), "utf8")).toBe( "generated mcp runtime\n", ); + const normalizer = await execFileAsync("node", [ + join(destination, "scripts", "normalize_candidates.mjs"), + ]); + expect(normalizer.stdout.trim()).toBe("generated normalizer"); if (process.platform !== "win32") { expect( (await stat(join(destination, "scripts", "launch"))).mode & 0o111, diff --git a/sdk/typescript/tests-ts/compact-diff-scan.test.ts b/sdk/typescript/tests-ts/compact-diff-scan.test.ts index f117cb0c3..82c7108d6 100644 --- a/sdk/typescript/tests-ts/compact-diff-scan.test.ts +++ b/sdk/typescript/tests-ts/compact-diff-scan.test.ts @@ -14,6 +14,7 @@ import { createInterface } from "node:readline"; import { afterEach, describe, expect, test } from "bun:test"; import { loadContract } from "../src/index.js"; import { PLUGIN_ROOT } from "./plugin-root.js"; +import { runNormalizer } from "./support/normalize-candidates.js"; type JsonObject = Record; @@ -389,12 +390,8 @@ describe("compact diff scan", () => { inventory, ]; - expect(python("normalize_candidates.py", ...args).status).toBe(2); - const accepted = python( - "normalize_candidates.py", - ...args, - "--allow-missing-in-scope", - ); + expect(runNormalizer(args).status).toBe(2); + const accepted = runNormalizer([...args, "--allow-missing-in-scope"]); expect(accepted.status, accepted.stderr).toBe(0); const contents = readFileSync(output, "utf8"); expect(contents).toContain("Résumé: missing guard"); @@ -408,11 +405,7 @@ describe("compact diff scan", () => { ]); writeFileSync(inventory, "../escaped.py\nsrc/handler.py\n"); - const escaped = python( - "normalize_candidates.py", - ...args, - "--allow-missing-in-scope", - ); + const escaped = runNormalizer([...args, "--allow-missing-in-scope"]); expect(escaped.status).toBe(2); expect(escaped.stderr).toContain("in-scope file row 1"); }); diff --git a/sdk/typescript/tests-ts/normalize-candidates-filesystem.test.ts b/sdk/typescript/tests-ts/normalize-candidates-filesystem.test.ts new file mode 100644 index 000000000..a7afe3f2f --- /dev/null +++ b/sdk/typescript/tests-ts/normalize-candidates-filesystem.test.ts @@ -0,0 +1,273 @@ +import { spawnSync } from "node:child_process"; +import { + linkSync, + mkdirSync, + mkdtempSync, + readFileSync, + readdirSync, + realpathSync, + rmSync, + statSync, + symlinkSync, + writeFileSync, +} from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { afterEach, describe, expect, test } from "bun:test"; +import { PLUGIN_ROOT } from "./plugin-root.js"; +import { + normalizerArguments, + runNormalizer, + writeSource, +} from "./support/normalize-candidates.js"; + +const temporaryRoots: string[] = []; +const testPosix = process.platform === "win32" ? test.skip : test; +const testWindows = process.platform === "win32" ? test : test.skip; +const directoryLinkType = process.platform === "win32" ? "junction" : "dir"; + +afterEach(() => { + for (const root of temporaryRoots.splice(0)) { + rmSync(root, { recursive: true, force: true }); + } +}); + +function candidate(path = "src/in-scope.ts") { + return { + cwe_ids: ["CWE-79"], + locations: [{ path, start_line: 1, role: "source" }], + summary: "Synthetic candidate", + evidence: "Synthetic evidence", + }; +} + +function fixture() { + const root = realpathSync( + mkdtempSync(join(tmpdir(), "codex-security-normalizer-filesystem-")), + ); + temporaryRoots.push(root); + const repository = join(root, "repository"); + writeSource(repository, "src/in-scope.ts", "one\ntwo\n"); + const inventory = join(root, "in-scope.txt"); + const input = join(root, "candidates.jsonl"); + const output = join(root, "output.jsonl"); + writeFileSync(inventory, "src/in-scope.ts\n"); + writeFileSync(input, `${JSON.stringify(candidate())}\n`); + const args = normalizerArguments([input], output, repository, inventory); + return { root, repository, inventory, input, output, args }; +} + +describe("candidate normalizer filesystem contract", () => { + test("runs through a linked helper directory", () => { + const { root, output, args } = fixture(); + const linked = join(root, "linked-helpers"); + symlinkSync(join(PLUGIN_ROOT, "scripts"), linked, directoryLinkType); + const result = runNormalizer( + args, + join(linked, "normalize_candidates.mjs"), + ); + expect(result.status, result.stderr).toBe(0); + expect(JSON.parse(readFileSync(output, "utf8")).summary).toBe( + "Synthetic candidate", + ); + }); + + test("canonicalizes directory links and preserves hard-link names", () => { + const { inventory, repository, input, output, args } = fixture(); + writeSource(repository, "real/target.ts", "target\n"); + mkdirSync(join(repository, "aliases")); + linkSync( + join(repository, "real", "target.ts"), + join(repository, "aliases", "hard.ts"), + ); + symlinkSync( + join(repository, "real"), + join(repository, "linked"), + directoryLinkType, + ); + writeFileSync(inventory, "linked/target.ts\naliases/hard.ts\n"); + writeFileSync( + input, + JSON.stringify({ + ...candidate(), + locations: [ + { path: "linked/target.ts", start_line: 1, role: "sink" }, + { path: "aliases/hard.ts", start_line: 1, role: "source" }, + ], + }) + "\n", + ); + const result = runNormalizer(args); + expect(result.status, result.stderr).toBe(0); + expect(JSON.parse(readFileSync(output, "utf8")).locations).toMatchObject([ + { path: "aliases/hard.ts" }, + { path: "real/target.ts" }, + ]); + }); + + testPosix( + "rejects directories, broken links, and FIFOs without changing output", + () => { + const { repository, input, output, args } = fixture(); + mkdirSync(join(repository, "src", "directory")); + symlinkSync("missing.ts", join(repository, "src", "broken.ts")); + symlinkSync("loop.ts", join(repository, "src", "loop.ts")); + const fifo = join(repository, "src", "named-pipe"); + const result = spawnSync("mkfifo", [fifo], { encoding: "utf8" }); + expect(result.status, result.stderr).toBe(0); + writeFileSync(output, "existing output\n"); + for (const path of [ + "src/directory", + "src/broken.ts", + "src/loop.ts", + "src/named-pipe", + ]) { + writeFileSync(input, `${JSON.stringify(candidate(path))}\n`); + expect( + runNormalizer(args, undefined, { timeout: 30_000 }).status, + path, + ).toBe(2); + expect(readFileSync(output, "utf8")).toBe("existing output\n"); + } + }, + ); + + test.each(["scope", "candidate"])( + "rejects invalid UTF-8 in %s without changing output", + (kind) => { + const { repository, inventory, input, output, args } = fixture(); + if (kind === "scope") { + writeSource(repository, "src/�.ts", "one\n"); + writeFileSync(input, JSON.stringify(candidate("src/�.ts")) + "\n"); + writeFileSync( + inventory, + Buffer.concat([ + Buffer.from("src/"), + Buffer.from([0xff]), + Buffer.from(".ts\n"), + ]), + ); + } else { + const contents = readFileSync(input); + contents[contents.indexOf("Synthetic candidate")] = 0xff; + writeFileSync(input, contents); + } + writeFileSync(output, "existing output\n"); + expect(runNormalizer(args).status).toBe(2); + expect(readFileSync(output, "utf8")).toBe("existing output\n"); + }, + ); + + test.each(["{", "null", "[]"])( + "rejects invalid candidate document %s without changing output", + (contents) => { + const { input, output, args } = fixture(); + writeFileSync(input, `${contents}\n`); + writeFileSync(output, "existing output\n"); + const result = runNormalizer(args); + expect(result.status, result.stderr).toBe(2); + expect(result.stderr).toContain("row 1"); + expect(readFileSync(output, "utf8")).toBe("existing output\n"); + }, + ); + + test("skips missing scope entries without admitting files outside the repository", () => { + const { root, repository, inventory, input, output, args } = fixture(); + const outside = join(root, "outside"); + mkdirSync(outside); + symlinkSync(outside, join(repository, "linked"), directoryLinkType); + writeFileSync(inventory, "src/in-scope.ts\nlinked/deleted.ts\n"); + expect(runNormalizer(args).status).toBe(2); + const allowed = [...args, "--allow-missing-in-scope"]; + const result = runNormalizer(allowed); + expect(result.status, result.stderr).toBe(0); + const before = readFileSync(output); + writeFileSync(join(outside, "deleted.ts"), "outside\n"); + expect(runNormalizer(allowed).status).toBe(2); + expect(readFileSync(output)).toEqual(before); + writeFileSync(inventory, "src/in-scope.ts\n"); + writeFileSync(input, JSON.stringify(candidate("linked/deleted.ts")) + "\n"); + expect(runNormalizer(allowed).status).toBe(2); + expect(readFileSync(output)).toEqual(before); + }); + + testPosix("replaces an output symlink without changing its target", () => { + const { root, output, args } = fixture(); + const target = join(root, "unrelated.txt"); + writeFileSync(target, "leave this alone\n"); + symlinkSync(target, output); + const result = runNormalizer(args); + expect(result.status, result.stderr).toBe(0); + expect(readFileSync(target, "utf8")).toBe("leave this alone\n"); + expect(JSON.parse(readFileSync(output, "utf8")).summary).toBe( + "Synthetic candidate", + ); + }); + + test("protects candidate inputs and the scope inventory from output replacement", () => { + const { inventory, repository, input } = fixture(); + for (const output of [input, inventory]) { + const before = readFileSync(output); + const result = runNormalizer( + normalizerArguments([input], output, repository, inventory), + ); + expect(result.status, result.stderr).toBe(2); + expect(readFileSync(output)).toEqual(before); + } + }); + + test("writes private files atomically and cleans failed write temporaries", () => { + const { root, output, args } = fixture(); + writeFileSync(output, "existing output\n", { mode: 0o644 }); + const before = readdirSync(root).sort(); + const result = runNormalizer(args); + expect(result.status, result.stderr).toBe(0); + expect(JSON.parse(readFileSync(output, "utf8")).summary).toBe( + "Synthetic candidate", + ); + if (process.platform !== "win32") + expect(statSync(output).mode & 0o777).toBe(0o600); + expect(readdirSync(root).sort()).toEqual(before); + + rmSync(output); + mkdirSync(output); + expect(runNormalizer(args).status).toBe(2); + expect(statSync(output).isDirectory()).toBe(true); + expect(readdirSync(root).sort()).toEqual(before); + }); + + testWindows( + "accepts Windows separators and rejects absolute drive paths", + () => { + const { inventory, input, output, args } = fixture(); + writeFileSync(inventory, "src\\in-scope.ts\r\n"); + writeFileSync( + input, + JSON.stringify(candidate("src\\in-scope.ts")) + "\n", + ); + const result = runNormalizer(args); + expect(result.status, result.stderr).toBe(0); + expect(JSON.parse(readFileSync(output, "utf8")).locations[0].path).toBe( + "src/in-scope.ts", + ); + writeFileSync(input, JSON.stringify(candidate("C:\\outside.ts")) + "\n"); + expect(runNormalizer(args).status).toBe(2); + }, + ); + + testWindows("protects differently cased input and scope paths", () => { + const { inventory, repository, input } = fixture(); + for (const output of [input, inventory]) { + const before = readFileSync(output); + const result = runNormalizer( + normalizerArguments( + [input], + output.toUpperCase(), + repository, + inventory, + ), + ); + expect(result.status, result.stderr).toBe(2); + expect(readFileSync(output)).toEqual(before); + } + }); +}); diff --git a/sdk/typescript/tests-ts/normalize-candidates.property.test.ts b/sdk/typescript/tests-ts/normalize-candidates.property.test.ts new file mode 100644 index 000000000..8eba34e9f --- /dev/null +++ b/sdk/typescript/tests-ts/normalize-candidates.property.test.ts @@ -0,0 +1,85 @@ +import { mkdtempSync, realpathSync, rmSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { expect, test } from "bun:test"; +import fc from "fast-check"; +import { + combine, + normalizeCandidate, +} from "../../../plugins/codex-security/scripts/normalize_candidates.js"; +import { writeSource } from "./support/normalize-candidates.js"; +import { propertyOptions } from "./support/property.js"; + +const text = fc + .string({ unit: "binary", maxLength: 40 }) + .map((value) => ` ${value}x `); +const candidate = fc.record({ + cwe_ids: fc.array( + fc.integer({ min: 1, max: 1000 }).map((value) => `cwe-0${value}`), + { maxLength: 4 }, + ), + locations: fc.array( + fc.record({ + path: fc.constantFrom("src/alpha.ts", "src/é.ts", "src/😀.ts"), + start_line: fc.integer({ min: 1, max: 3 }), + role: fc.constantFrom("source", "sink", "evidence"), + }), + { minLength: 1, maxLength: 4 }, + ), + summary: text, + evidence: text, + context: fc.option(text, { nil: undefined }), + instance: fc.option(text, { nil: undefined }), +}); + +test("normalization is deterministic across row, location, and CWE order and duplicates", () => { + const repository = realpathSync( + mkdtempSync(join(tmpdir(), "codex-security-normalizer-property-")), + ); + const scope = new Set(["src/alpha.ts", "src/é.ts", "src/😀.ts"]); + for (const path of scope) writeSource(repository, path, "one\ntwo\nthree\n"); + const lineCounts = new Map(); + const normalize = (rows: unknown[]) => + combine( + rows.map((row) => normalizeCandidate(row, repository, scope, lineCounts)), + ); + try { + fc.assert( + fc.property( + fc.array(candidate, { minLength: 1, maxLength: 10 }), + (rows) => { + const variants = rows.flatMap((row) => [ + row, + { ...row, context: undefined }, + ]); + const expected = normalize(variants); + const reordered = variants.toReversed().map((row) => ({ + ...row, + cwe_ids: [...row.cwe_ids.toReversed(), ...row.cwe_ids], + locations: [...row.locations.toReversed(), ...row.locations], + })); + expect(JSON.stringify(normalize([...reordered, ...reordered]))).toBe( + JSON.stringify(expected), + ); + expect(new Set(expected.map((row) => row.candidate_id)).size).toBe( + expected.length, + ); + + const changedText = rows.map((row) => ({ + ...row, + candidate_id: "upstream-id", + summary: "Reworded summary", + evidence: "Additional evidence", + context: "Additional context", + })); + expect(normalize(changedText).map((row) => row.candidate_id)).toEqual( + expected.map((row) => row.candidate_id), + ); + }, + ), + propertyOptions, + ); + } finally { + rmSync(repository, { recursive: true, force: true }); + } +}); diff --git a/sdk/typescript/tests-ts/normalize-candidates.test.ts b/sdk/typescript/tests-ts/normalize-candidates.test.ts new file mode 100644 index 000000000..9188e3846 --- /dev/null +++ b/sdk/typescript/tests-ts/normalize-candidates.test.ts @@ -0,0 +1,215 @@ +import { + mkdtempSync, + readFileSync, + realpathSync, + rmSync, + writeFileSync, +} from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { afterEach, describe, expect, test } from "bun:test"; +import { + combine, + normalizeCandidate, +} from "../../../plugins/codex-security/scripts/normalize_candidates.js"; +import { + normalizerArguments, + runNormalizer, + writeSource, +} from "./support/normalize-candidates.js"; + +const temporaryRoots: string[] = []; +const location = { + path: "src/in-scope.ts", + start_line: 1, + role: "source", +} as const; +const candidate = { + cwe_ids: ["CWE-79"], + locations: [location], + summary: "Candidate", + evidence: "Evidence", +}; + +afterEach(() => { + for (const root of temporaryRoots.splice(0)) + rmSync(root, { recursive: true, force: true }); +}); + +function fixture() { + const root = realpathSync( + mkdtempSync(join(tmpdir(), "codex-security-normalizer-")), + ); + temporaryRoots.push(root); + const repository = join(root, "repository"); + writeSource(repository, location.path, "one\ntwo\n"); + writeSource(repository, "src/out-of-scope.ts", "one\n"); + return { root, repository }; +} + +describe("candidate normalizer", () => { + test.each(["\n", "\r\n"])( + "normalizes and combines JSONL with %j line endings", + (newline) => { + const { root, repository } = fixture(); + writeSource(repository, location.path, `one${newline}two${newline}`); + const inventory = join(root, "scope.txt"); + const first = join(root, "first.jsonl"); + const second = join(root, "second.jsonl"); + const output = join(root, "output.jsonl"); + writeFileSync(inventory, `${location.path}${newline}`); + writeFileSync( + first, + [ + JSON.stringify({ + ...candidate, + candidate_id: "discarded-id", + cwe_ids: [" cwe-079 ", "CWE-89"], + summary: " A summary ", + locations: [location, location], + instance: "route:a", + }), + JSON.stringify({ + ...candidate, + summary: "Separate candidate", + instance: "route:b", + }), + ].join(newline), + ); + writeFileSync( + second, + `${JSON.stringify({ ...candidate, cwe_ids: ["CWE-89", "CWE-79"], summary: "B summary", evidence: "More evidence", context: "Context", instance: "route:a" })}${newline}`, + ); + const result = runNormalizer( + normalizerArguments( + [second, first, first], + output, + repository, + inventory, + ), + ); + expect(result.status, result.stderr).toBe(0); + expect(result.stdout).toContain("Combined 3 candidate rows into 2 rows"); + const rows = readFileSync(output, "utf8") + .trim() + .split("\n") + .map((line) => JSON.parse(line)) as ReturnType; + expect(rows).toHaveLength(2); + const merged = rows.find((row) => row.instance === "route:a")!; + expect(merged).toEqual({ + candidate_id: expect.stringMatching(/^candidate-[a-f0-9]{16}$/u), + cwe_ids: ["CWE-79", "CWE-89"], + locations: [{ ...location, end_line: 1 }], + summary: "A summary\nB summary", + evidence: "Evidence\nMore evidence", + context: "Context", + instance: "route:a", + }); + expect( + rows.find((row) => row.instance === "route:b")?.candidate_id, + ).not.toBe(merged.candidate_id); + }, + ); + + test.each(["\n", "\r\n", "\r"])( + "counts source lines separated by %j", + (newline) => { + const { repository } = fixture(); + writeSource(repository, location.path, `one${newline}two${newline}`); + const row = normalizeCandidate( + { ...candidate, locations: [{ ...location, start_line: 2 }] }, + repository, + new Set([location.path]), + new Map(), + ); + expect(row.locations[0]?.end_line).toBe(2); + }, + ); + + test.each([ + { name: "empty locations", patch: { locations: [] } }, + { name: "blank summary", patch: { summary: " \t" } }, + { name: "blank evidence", patch: { evidence: "" } }, + { name: "invalid context", patch: { context: false } }, + { name: "invalid instance", patch: { instance: 1 } }, + { name: "invalid CWE", patch: { cwe_ids: ["CWE-0"] } }, + { + name: "invalid role", + patch: { locations: [{ ...location, role: "unknown" }] }, + }, + { + name: "boolean line", + patch: { locations: [{ ...location, start_line: true }] }, + }, + { + name: "fractional line", + patch: { locations: [{ ...location, start_line: 1.5 }] }, + }, + { + name: "reversed range", + patch: { locations: [{ ...location, start_line: 2, end_line: 1 }] }, + }, + { + name: "line beyond file", + patch: { locations: [{ ...location, start_line: 3 }] }, + }, + { name: "unknown field", patch: { unexpected: true } }, + { + name: "unknown location field", + patch: { locations: [{ ...location, extra: true }] }, + }, + { + name: "path traversal", + patch: { locations: [{ ...location, path: "../outside.ts" }] }, + }, + { + name: "out of scope", + patch: { locations: [{ ...location, path: "src/out-of-scope.ts" }] }, + }, + ])("rejects $name", ({ patch }) => { + const { repository } = fixture(); + expect(() => + normalizeCandidate( + { ...candidate, ...patch }, + repository, + new Set([location.path]), + new Map(), + ), + ).toThrow(); + }); + + test("accepts explicit values for paths containing spaces or leading dashes", () => { + const { root } = fixture(); + const input = "-candidate input.jsonl"; + writeFileSync(join(root, input), `${JSON.stringify(candidate)}\n`); + writeFileSync(join(root, "in scope.txt"), `${location.path}\n`); + const args = [ + `--input=${input}`, + "--out=output.jsonl", + "--repo-root=repository", + "--in-scope-files=in scope.txt", + ]; + const result = runNormalizer(args, undefined, { cwd: root }); + expect(result.status, result.stderr).toBe(0); + expect( + JSON.parse(readFileSync(join(root, "output.jsonl"), "utf8")).summary, + ).toBe("Candidate"); + expect( + runNormalizer([...args, input], undefined, { cwd: root }).status, + ).toBe(2); + }); + + test("shows help and reports missing or unknown arguments", () => { + for (const flag of ["--help", "-h"]) { + const result = runNormalizer([flag]); + expect(result.status, result.stderr).toBe(0); + expect(result.stdout).toContain("Usage:"); + } + for (const args of [[], ["--out"], ["--input"], ["--unknown"]]) { + const result = runNormalizer(args); + expect(result.status).toBe(2); + expect(result.stdout).toBe(""); + expect(result.stderr).not.toBe(""); + } + }); +}); diff --git a/sdk/typescript/tests-ts/runtime.test.ts b/sdk/typescript/tests-ts/runtime.test.ts index 07c142f1f..4df14e334 100644 --- a/sdk/typescript/tests-ts/runtime.test.ts +++ b/sdk/typescript/tests-ts/runtime.test.ts @@ -773,152 +773,6 @@ describe("plugin runtime preparation", () => { ).toBeDefined(); }); - testPosix( - "preserves literal POSIX candidate paths in the bundled plugin", - async () => { - const root = await temporaryDirectory(); - await mkdir(join(root, "source")); - const cases = [ - { path: "source\\candidate.py", contents: "literal candidate\n" }, - { path: " leading.py", contents: "leading whitespace\n" }, - { path: "trailing.py ", contents: "trailing whitespace\n" }, - { path: " ", contents: "single whitespace filename\n" }, - { path: " ", contents: "multiple whitespace filename\n" }, - { path: "C:candidate.py", contents: "literal colon\n" }, - { path: "carriage\rreturn.py", contents: "literal carriage return\n" }, - { path: "vertical\vtab.py", contents: "literal vertical tab\n" }, - { path: "form\ffeed.py", contents: "literal form feed\n" }, - { path: "next\u0085line.py", contents: "literal next line\n" }, - { - path: "unicode\u2028separator.py", - contents: "literal line separator\n", - }, - { - path: "paragraph\u2029separator.py", - contents: "literal paragraph separator\n", - }, - ]; - await Promise.all([ - ...cases.map((item) => writeFile(join(root, item.path), item.contents)), - writeFile(join(root, "source", "candidate.py"), "wrong candidate\n"), - writeFile(join(root, "leading.py"), "wrong leading candidate\n"), - writeFile(join(root, "trailing.py"), "wrong trailing candidate\n"), - ]); - const scopePath = join(root, "in-scope-files.txt"); - await writeFile( - scopePath, - `${cases.map((item) => item.path).join("\n")}\n`, - ); - - const python = Bun.which("python3") ?? Bun.which("python"); - expect(python).not.toBeNull(); - const sourcePlugin = await bundledPluginRoot(); - const projector = new URL( - "../scripts/project-plugin.mjs", - import.meta.url, - ); - const publicManifest = new URL( - "../public-repo/sdk/typescript/plugin.public.json", - import.meta.url, - ); - let bundledPlugin = sourcePlugin; - if (existsSync(projector) && existsSync(publicManifest)) { - const packageRoot = join(root, "package"); - const isolatedProjector = join( - packageRoot, - "scripts", - "project-plugin.mjs", - ); - const isolatedManifest = join( - packageRoot, - "public-repo", - "sdk", - "typescript", - "plugin.public.json", - ); - await Promise.all([ - mkdir(dirname(isolatedProjector), { recursive: true }), - mkdir(dirname(isolatedManifest), { recursive: true }), - ]); - await Promise.all([ - copyFile(projector, isolatedProjector), - copyFile(publicManifest, isolatedManifest), - ]); - const projection = Bun.spawnSync( - [process.execPath, isolatedProjector], - { - cwd: packageRoot, - env: { - ...process.env, - CODEX_SECURITY_PLUGIN_ROOT: sourcePlugin, - }, - stdout: "pipe", - stderr: "pipe", - }, - ); - expect(new TextDecoder().decode(projection.stderr)).toBe(""); - expect(projection.exitCode).toBe(0); - bundledPlugin = join(packageRoot, "_bundled_plugin"); - } - const normalizer = join( - bundledPlugin, - "scripts", - "normalize_candidates.py", - ); - expect(await readFile(normalizer, "utf8")).toBe( - await readFile( - join(sourcePlugin, "scripts", "normalize_candidates.py"), - "utf8", - ), - ); - const result = Bun.spawnSync([ - python!, - "-I", - "-B", - "-c", - [ - "import json, pathlib, runpy, sys", - "module = runpy.run_path(sys.argv[1])", - "root = pathlib.Path(sys.argv[2])", - "scope = module['read_scope'](pathlib.Path(sys.argv[3]), root)", - "finalizer = runpy.run_path(sys.argv[5])", - "results = []", - "for value in json.loads(sys.argv[4]):", - " path, source = module['relative_file'](value, root)", - " candidate = {'cwe_ids': ['CWE-89'], 'locations': [{'path': value, 'start_line': 1, 'role': 'entrypoint'}], 'summary': 'Test finding', 'evidence': 'Test evidence'}", - " try:", - " normalized = module['normalize_candidate'](candidate, root, scope, {})", - " location = normalized['locations'][0]", - " finalizer['_validate_location']({'path': location['path'], 'startLine': location['start_line'], 'endLine': location['end_line'], 'role': location['role']}, 'candidate.locations[0]')", - " except ValueError:", - " contract_valid = False", - " else:", - " contract_valid = True", - " results.append({'path': path, 'contents': source.read_text(encoding='utf-8'), 'inScope': path in scope, 'contractValid': contract_valid})", - "print(json.dumps(results))", - ].join("\n"), - normalizer, - root, - scopePath, - JSON.stringify(cases.map((item) => item.path)), - join(bundledPlugin, "scripts", "finalize_scan_contract.py"), - ]); - - expect(result.exitCode).toBe(0); - expect(JSON.parse(new TextDecoder().decode(result.stdout))).toEqual( - cases.map((item) => ({ - ...item, - inScope: true, - contractValid: - item.path.trim().length > 0 && - !/^[A-Za-z]:/.test(item.path) && - !item.path.includes("\\") && - !/[\u0000-\u001f]/u.test(item.path), - })), - ); - }, - ); - test("uses a configured plugin directory directly", async () => { const root = await temporaryDirectory(); const ambientHome = join(root, ".codex", "plugins", "cache"); diff --git a/sdk/typescript/tests-ts/support/normalize-candidates.ts b/sdk/typescript/tests-ts/support/normalize-candidates.ts new file mode 100644 index 000000000..cd01b2ebc --- /dev/null +++ b/sdk/typescript/tests-ts/support/normalize-candidates.ts @@ -0,0 +1,48 @@ +import { spawnSync, type SpawnSyncOptions } from "node:child_process"; +import { mkdirSync, writeFileSync } from "node:fs"; +import { dirname, join } from "node:path"; +import { fileURLToPath } from "node:url"; + +const normalizer = fileURLToPath( + new URL( + "../../_bundled_plugin/scripts/normalize_candidates.mjs", + import.meta.url, + ), +); + +export function writeSource( + repository: string, + path: string, + contents: string | Uint8Array, +): void { + const output = join(repository, path); + mkdirSync(dirname(output), { recursive: true }); + writeFileSync(output, contents); +} + +export function runNormalizer( + args: string[], + script = normalizer, + options: Pick = {}, +) { + return spawnSync("node", [script, ...args], { ...options, encoding: "utf8" }); +} + +export function normalizerArguments( + inputs: string[], + output: string, + repository: string, + inventory: string, + allowMissing = false, +): string[] { + return [ + ...inputs.flatMap((input) => ["--input", input]), + "--out", + output, + "--repo-root", + repository, + "--in-scope-files", + inventory, + ...(allowMissing ? ["--allow-missing-in-scope"] : []), + ]; +} diff --git a/sdk/typescript/tsconfig.json b/sdk/typescript/tsconfig.json index 18454c78e..2ca9f548a 100644 --- a/sdk/typescript/tsconfig.json +++ b/sdk/typescript/tsconfig.json @@ -1,5 +1,6 @@ { "include": [ + "../../plugins/codex-security/scripts/normalize_candidates.ts", "src/**/*.ts", "src/**/*.tsx", "dashboard/**/*.ts",