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..b2f35f7f9 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 @@ -77,11 +77,9 @@ export async function validateReducerArtifacts(input: { } else { validateRetainedFindings(result, [], previous); } - const previousFindingIds = new Set((previous?.findings ?? []).map(scanFindingIdentity)); + const retainedFindings = previousFindingAssignments(result, previous); return { - newFindings: result.findings.filter((finding) => ( - !previousFindingIds.has(scanFindingIdentity(finding)) - )).length + newFindings: result.findings.filter((finding) => !retainedFindings.has(finding)).length }; } @@ -97,23 +95,11 @@ export function reconcileDeepReduction( if (source.scanId !== result.scanId) throw new Error("Deep reduction source belongs to a different scan."); if (source.complete === false) throw new Error("Deep reduction source is only a checkpoint, not a complete result."); } - validateRetainedFindings(result, discoveries.map((discovery) => discovery.result), previous ?? undefined); - retainSourceFindings(result, { discoveries, previous }); - const unmatched = new Set(result.findings); - for (const finding of previous?.findings ?? []) { - const previousRefs = findingSourceIds(finding); - const retained = ( - previousRefs.length > 0 - ? result.findings.find((current) => ( - findingSourceIds(current).some((ref) => previousRefs.includes(ref)) - )) - : undefined - ) ?? [...unmatched].find((current) => ( - scanFindingIdentity(current) === scanFindingIdentity(finding) - )); - if (retained) { + validateReductionHasFindings(result, discoveries.map((discovery) => discovery.result), previous ?? undefined); + retainSourceFindings(result, { discoveries, previous }, false); + for (const [retained, previousFindings] of previousFindingAssignments(result, previous)) { + for (const finding of previousFindings) { preserveFindingDetails(retained, finding); - unmatched.delete(retained); } } retainSourceFindings(result, { discoveries, previous }); @@ -167,7 +153,11 @@ function findingSourceIds(finding: Record): string[] { )); } -function retainSourceFindings(result: ScanDraftInput, inputs: DeepReductionSources): void { +function retainSourceFindings( + result: ScanDraftInput, + inputs: DeepReductionSources, + requireComplete = true, +): void { type Finding = Record; const sources = new Map(); for (const discovery of inputs.discoveries) { @@ -209,12 +199,12 @@ function retainSourceFindings(result: ScanDraftInput, inputs: DeepReductionSourc } } const missing = [...sources.keys()].filter((id) => !claimed.has(id)); - if (missing.length) throw new Error(`Deep reduction left unaccounted source findings: ${missing.join(", ")}.`); + if (requireComplete && missing.length) { + throw new Error(`Deep reduction left unaccounted source findings: ${missing.join(", ")}.`); + } } - -/** Preserve previously accepted identities and never discard every reported finding. */ -export function validateRetainedFindings( +function validateReductionHasFindings( result: ScanDraftInput, sources: ScanDraftInput[], previous?: ScanDraftInput @@ -226,16 +216,44 @@ export function validateRetainedFindings( ) { throw new Error("Deep reduction discarded every accepted Standard scan finding."); } +} - const currentFindingIds = new Set(result.findings.map(scanFindingIdentity)); +function previousFindingAssignments( + result: ScanDraftInput, + previous?: ScanDraftInput | null, +): Map { + const assignments = new Map(); for (const finding of previous?.findings ?? []) { - if (currentFindingIds.has(scanFindingIdentity(finding))) continue; - throw Object.assign(new Error( - "Deep reduction discarded or changed a previously accepted finding identity." - ), { - code: "merge_traceability_unstable_candidate_id" - }); + const previousRefs = findingSourceIds(finding); + const matches = previousRefs.length > 0 + ? result.findings.filter((current) => { + const currentRefs = new Set(findingSourceIds(current)); + return previousRefs.every((ref) => currentRefs.has(ref)); + }) + : result.findings.filter((current) => scanFindingIdentity(current) === scanFindingIdentity(finding)); + if (matches.length !== 1) { + throw Object.assign(new Error( + "Deep reduction discarded, split, or ambiguously reassigned a previously accepted finding." + ), { + code: "merge_traceability_unstable_candidate_id" + }); + } + const retained = matches[0]!; + const assigned = assignments.get(retained) ?? []; + assigned.push(finding); + assignments.set(retained, assigned); } + return assignments; +} + +/** Preserve previous source lineage (or legacy identity) and never discard every finding. */ +export function validateRetainedFindings( + result: ScanDraftInput, + sources: ScanDraftInput[], + previous?: ScanDraftInput +): void { + validateReductionHasFindings(result, sources, previous); + previousFindingAssignments(result, previous); } function parseStoredScanDraft( diff --git a/plugins/codex-security/mcp-app/tests/test_deep_scan_artifact_validation.mjs b/plugins/codex-security/mcp-app/tests/test_deep_scan_artifact_validation.mjs index e5c3e6b65..b7ee2ac0e 100644 --- a/plugins/codex-security/mcp-app/tests/test_deep_scan_artifact_validation.mjs +++ b/plugins/codex-security/mcp-app/tests/test_deep_scan_artifact_validation.mjs @@ -33,6 +33,7 @@ const root = await realpath(await mkdtemp(path.join(tmpdir(), "deep-scan-artifac try { await testDiscoveryValidation(root); await testReducerValidation(root); + await testReducerLineageConvergence(root); await testEmptyDiscoveryAndReduction(root); } finally { await rm(root, { recursive: true, force: true }); @@ -384,13 +385,13 @@ async function testReducerValidation(root) { provenance: { source: "local_plugin", sourceFindingIds: ["origin:b"] }, }, ])); - await validateReducerArtifacts({ + assert.deepEqual(await validateReducerArtifacts({ artifacts, artifactDir, resultPath, reducerId: "dedup-colliding-previous", sources: collidingSources, - }, scanId); + }, scanId), { newFindings: 0 }); const reconciledCollisions = JSON.parse(await readFile(resultPath, "utf8")).findings; assert.deepEqual( reconciledCollisions.map((item) => item.provenance.sourceFindingIds), @@ -521,6 +522,170 @@ async function testReducerValidation(root) { } } +async function testReducerLineageConvergence(root) { + const artifacts = await createLayout(path.join(root, "lineage-convergence")); + const artifactDir = path.join(artifacts.dedupRoot, "dedup-lineage", "output"); + const resultPath = path.join(artifactDir, "result.json"); + await mkdir(artifactDir, { recursive: true }); + + const originalA = finding("alias-a", "src/alias-a.js"); + const originalB = finding("alias-b", "src/alias-b.js"); + const previousA = { + ...originalA, + provenance: { + source: "local_plugin", + sourceFindingIds: ["alias:a"], + sourceFindings: [{ id: "alias:a", finding: originalA }] + } + }; + const previousB = { + ...originalB, + provenance: { + source: "local_plugin", + sourceFindingIds: ["alias:b"], + sourceFindings: [{ id: "alias:b", finding: originalB }] + } + }; + const previousAliases = draft([previousA, previousB]); + + await writeResult(resultPath, draft([{ + ...originalA, + provenance: { + source: "local_plugin", + sourceFindingIds: ["alias:a", "alias:b"] + } + }])); + assert.deepEqual(await validateReducerArtifacts({ + artifacts, + artifactDir, + resultPath, + reducerId: "dedup-alias-collapse", + sources: { discoveries: [], previous: previousAliases } + }, scanId), { newFindings: 0 }); + const reconciledAlias = JSON.parse(await readFile(resultPath, "utf8")).findings[0]; + assert.deepEqual(reconciledAlias.provenance.sourceFindingIds, ["alias:a", "alias:b"]); + assert.equal(reconciledAlias.provenance.previousFindings[0].identity.anchor, "alias-b"); + + const previousResultPath = path.join( + artifacts.dedupRoot, + "dedup-lineage-previous", + "output", + "result.json" + ); + await mkdir(path.dirname(previousResultPath), { recursive: true }); + await writeResult(previousResultPath, previousAliases); + assert.deepEqual(await validateReducerArtifacts({ + artifacts, + artifactDir, + resultPath, + reducerId: "dedup-alias-collapse-recovery", + previousReducerResultPath: previousResultPath + }, scanId), { newFindings: 0 }); + + const freshEvidence = finding("fresh-evidence", "src/fresh-evidence.js"); + const freshSources = [{ workerId: "worker-fresh", result: draft([freshEvidence]) }]; + const previousCombined = { + ...originalA, + provenance: { + source: "local_plugin", + sourceFindingIds: ["alias:a", "alias:b"], + sourceFindings: [ + { id: "alias:a", finding: originalA }, + { id: "alias:b", finding: originalB } + ] + } + }; + await writeResult(resultPath, draft([ + { + ...originalA, + provenance: { source: "local_plugin", sourceFindingIds: ["alias:a"] } + }, + { + ...originalB, + provenance: { + source: "local_plugin", + sourceFindingIds: ["alias:b", "worker-fresh:0"] + } + } + ])); + await assert.rejects(validateReducerArtifacts({ + artifacts, + artifactDir, + resultPath, + reducerId: "dedup-lineage-split", + sources: { discoveries: freshSources, previous: draft([previousCombined]) } + }, scanId), (error) => ( + error.code === "merge_traceability_unstable_candidate_id" + && /split/.test(error.message) + )); + + await writeResult(resultPath, draft([{ + ...finding("merged-root", "src/merged-root.js"), + provenance: { + source: "local_plugin", + sourceFindingIds: ["alias:a", "alias:b", "worker-fresh:0"] + } + }])); + assert.deepEqual(await validateReducerArtifacts({ + artifacts, + artifactDir, + resultPath, + reducerId: "dedup-existing-root-new-evidence", + sources: { discoveries: freshSources, previous: previousAliases } + }, scanId), { newFindings: 0 }); + + await writeResult(resultPath, draft([ + { + ...originalA, + provenance: { + source: "local_plugin", + sourceFindingIds: ["alias:a", "alias:b"] + } + }, + { + ...freshEvidence, + provenance: { + source: "local_plugin", + sourceFindingIds: ["worker-fresh:0"] + } + } + ])); + assert.deepEqual(await validateReducerArtifacts({ + artifacts, + artifactDir, + resultPath, + reducerId: "dedup-genuinely-new-root", + sources: { discoveries: freshSources, previous: previousAliases } + }, scanId), { newFindings: 1 }); + + const legacyPrevious = { + ...originalA, + provenance: { source: "local_plugin" } + }; + await writeResult(resultPath, draft([originalA])); + assert.deepEqual(await validateReducerArtifacts({ + artifacts, + artifactDir, + resultPath, + reducerId: "dedup-legacy-identity", + sources: { discoveries: [], previous: draft([legacyPrevious]) } + }, scanId), { newFindings: 0 }); + await writeResult(resultPath, draft([{ + ...finding("legacy-renamed", "src/alias-a.js"), + provenance: { + source: "local_plugin", + sourceFindingIds: ["previous:0"] + } + }])); + await assert.rejects(validateReducerArtifacts({ + artifacts, + artifactDir, + resultPath, + reducerId: "dedup-legacy-renamed", + sources: { discoveries: [], previous: draft([legacyPrevious]) } + }, scanId), (error) => error.code === "merge_traceability_unstable_candidate_id"); +} + async function testEmptyDiscoveryAndReduction(root) { const artifacts = await createLayout(path.join(root, "empty")); const worker = await createWorker(