From 23afbbd11a4d673cfe90f670fd497172a0f7b285 Mon Sep 17 00:00:00 2001 From: cjc0013 Date: Sat, 29 Aug 2026 01:28:46 -0400 Subject: [PATCH] fix: keep severity calibration consistent across scan modes --- .../src/deep-scan/artifact-validation.ts | 55 ++++++++++++++++++- .../mcp-app/templates/deep-scan/dedup.md | 2 + .../tests/test_artifact_deep_reducer.mjs | 47 ++++++++++++++++ 3 files changed, 102 insertions(+), 2 deletions(-) diff --git a/plugins/codex-security/mcp-app/src/deep-scan/artifact-validation.ts b/plugins/codex-security/mcp-app/src/deep-scan/artifact-validation.ts index e26dd1cb3..b897722ad 100644 --- a/plugins/codex-security/mcp-app/src/deep-scan/artifact-validation.ts +++ b/plugins/codex-security/mcp-app/src/deep-scan/artifact-validation.ts @@ -117,6 +117,7 @@ export function reconcileDeepReduction( } } retainSourceFindings(result, { discoveries, previous }); + validateSeverityReconciliation(result, { discoveries, previous }); result.coverage = preserveScanCoverage(result.coverage, [ ...discoveries.map((discovery) => discovery.result.coverage), ...(previous ? [previous.coverage] : []), @@ -167,8 +168,9 @@ function findingSourceIds(finding: Record): string[] { )); } -function retainSourceFindings(result: ScanDraftInput, inputs: DeepReductionSources): void { - type Finding = Record; +type Finding = Record; + +function sourceFindingsById(inputs: DeepReductionSources): Map { const sources = new Map(); for (const discovery of inputs.discoveries) { for (const [index, finding] of discovery.result.findings.entries()) { @@ -186,6 +188,11 @@ function retainSourceFindings(result: ScanDraftInput, inputs: DeepReductionSourc sources.set(`previous:${index}`, finding); } } + return sources; +} + +function retainSourceFindings(result: ScanDraftInput, inputs: DeepReductionSources): void { + const sources = sourceFindingsById(inputs); const claimed = new Set(); for (const finding of result.findings) { const provenance = finding.provenance as Finding; @@ -212,6 +219,50 @@ function retainSourceFindings(result: ScanDraftInput, inputs: DeepReductionSourc if (missing.length) throw new Error(`Deep reduction left unaccounted source findings: ${missing.join(", ")}.`); } +function validateSeverityReconciliation( + result: ScanDraftInput, + inputs: DeepReductionSources, +): void { + const sources = sourceFindingsById(inputs); + for (const finding of result.findings) { + const sourceLevels = new Set( + findingSourceIds(finding).map((id) => severityLevel(sources.get(id))), + ); + sourceLevels.delete(undefined); + const outputLevel = severityLevel(finding); + if ( + outputLevel === undefined + || sourceLevels.size === 0 + || (sourceLevels.size === 1 && sourceLevels.has(outputLevel)) + ) { + continue; + } + + const severity = finding.severity as Finding; + if ( + !isNonEmptyText(severity.rationale) + || !isNonEmptyText(severity.changeConditions) + ) { + throw new Error( + "Deep reduction changed or reconciled conflicting Standard scan severities " + + "without recording severity.rationale and severity.changeConditions." + ); + } + } +} + +function severityLevel(finding: Finding | undefined): string | undefined { + if (!finding || typeof finding.severity !== "object" || finding.severity === null) { + return undefined; + } + const level = (finding.severity as Finding).level; + return typeof level === "string" ? level : undefined; +} + +function isNonEmptyText(value: unknown): value is string { + return typeof value === "string" && value.trim().length > 0; +} + /** Preserve previously accepted identities and never discard every reported finding. */ export function validateRetainedFindings( diff --git a/plugins/codex-security/mcp-app/templates/deep-scan/dedup.md b/plugins/codex-security/mcp-app/templates/deep-scan/dedup.md index b32585c22..ec8542291 100644 --- a/plugins/codex-security/mcp-app/templates/deep-scan/dedup.md +++ b/plugins/codex-security/mcp-app/templates/deep-scan/dedup.md @@ -14,6 +14,8 @@ Do not merge findings merely because they share a subsystem, CWE, route or file For a valid merge, synthesize one stronger finding while preserving every materially useful non-redundant detail: narrower exploit framings, affected subpaths, preconditions, distinct source/control/sink nuances, contradictory or strengthening evidence, affected locations, and remediation-relevant subcases. Omit only genuinely duplicate or superseded detail. Preserve previously established finding identities. +Severity is determined by evidence, not by scan mode, worker count, repeated labels, or stronger wording. Compare the source findings' attacker, impact, likelihood, prerequisites, controls, counterevidence, and concrete outcome before choosing the merged severity. Preserve their severity when that evidence is materially equivalent. If source severities conflict, or the merged severity differs from every source severity, record the specific additional or limiting source-backed evidence that resolves the difference in `severity.rationale`, and record what evidence would change that calibration in `severity.changeConditions`. The host rejects an unexplained severity conflict or change. + Account for every input finding using the host-supplied `provenance.sourceFindingIds`. Copy the refs for retained findings; union them for a valid merge, preserving previous refs. Never invent, omit, or reuse a ref across independent output findings. The host retains original source payloads and rejects unaccounted input. Identity collisions do not establish that findings are duplicates. Combine the complete coverage, exclusions, deferred work, open questions, threat-model context, and optional scope from the Standard results without dropping meaningful information. Keep coverage partial whenever any deferred work or follow-up surface requires it. You cannot resolve or reject a source finding without inspecting code, which is outside this reducer's role. diff --git a/plugins/codex-security/mcp-app/tests/test_artifact_deep_reducer.mjs b/plugins/codex-security/mcp-app/tests/test_artifact_deep_reducer.mjs index 2dd7c4cac..76066cd60 100644 --- a/plugins/codex-security/mcp-app/tests/test_artifact_deep_reducer.mjs +++ b/plugins/codex-security/mcp-app/tests/test_artifact_deep_reducer.mjs @@ -130,6 +130,14 @@ try { threatModel: { summary: "Requests reach shared and independent code." }, scope: { summary: "Shared and independent request handling." } }); + await assert.rejects( + recordCodexSecurityDeepReduction(context, { + ...merged, + findings: [{ ...shared, severity: { level: "medium" } }, independent], + }), + /without recording severity\.rationale and severity\.changeConditions/, + "a reducer cannot silently recalibrate unanimous Standard results", + ); const outcome = await recordCodexSecurityDeepReduction(context, merged); const mergedWithSources = { ...merged, @@ -202,6 +210,45 @@ try { await assert.rejects(recordCodexSecurityDeepReduction(collisionContext, draft([{ ...shared, provenance: { ...shared.provenance, sourceFindingIds: ["unassigned:0"] }, }])), /unknown source finding/); + + const mediumShared = { + ...shared, + severity: { level: "medium" }, + }; + const severityWorker = await createWorker({ + workersRoot, label: "discovery-severity", id: "worker-severity", + result: draft([mediumShared]), completionSequence: 6, + }); + const severityRoot = path.join(dedupRoot, "dedup-severity", "output"); + await mkdir(severityRoot, { recursive: true }); + const severityContext = { + ...context, root: severityRoot, + deepReducer: { scanRoot, claimedWorkers: [first, severityWorker] }, + }; + const severitySourceFindingIds = ["worker-001:0", "worker-severity:0"]; + await assert.rejects( + recordCodexSecurityDeepReduction(severityContext, draft([{ + ...shared, + provenance: { ...shared.provenance, sourceFindingIds: severitySourceFindingIds }, + }])), + /without recording severity\.rationale and severity\.changeConditions/, + "conflicting Standard severities require an explicit evidence-based resolution", + ); + const reconciledSeverity = { + ...shared, + provenance: { ...shared.provenance, sourceFindingIds: severitySourceFindingIds }, + severity: { + level: "high", + rationale: "The retained source establishes direct attacker reachability and impact.", + changeConditions: "Constrained reachability or impact would lower the severity.", + }, + }; + await recordCodexSecurityDeepReduction(severityContext, draft([reconciledSeverity])); + const severityOutput = JSON.parse( + await readFile(path.join(severityRoot, "result.json"), "utf8"), + ); + assert.equal(severityOutput.findings[0].severity.level, "high"); + assert.equal(severityOutput.findings[0].provenance.sourceFindings.length, 2); await assert.rejects( readFile(path.join(scanRoot, "artifacts", "02_discovery", "candidate_ledger.jsonl")), { code: "ENOENT" }