diff --git a/packages/coding-agent/src/security/comparison.ts b/packages/coding-agent/src/security/comparison.ts index 3485e80be..582aebed4 100644 --- a/packages/coding-agent/src/security/comparison.ts +++ b/packages/coding-agent/src/security/comparison.ts @@ -3,7 +3,7 @@ import type { SecurityComparisonReport, SecurityFinding, SecurityFindingMatch, S export interface SecurityDifferentialFindingMatch { referenceFindingId: string; candidateFindingId: string; - basis: "fingerprint" | "rule_location"; + basis: "fingerprint" | "rule_location" | "taxonomy_location"; } export interface SecurityDifferentialFindingSummary { findingId: string; @@ -58,6 +58,39 @@ function fallbackKey(finding: SecurityFinding): string | undefined { return `${finding.ruleId.trim().toLowerCase()}\u0000${location}`; } +function normalizedPath(value: string): string { + return value.replaceAll("\\", "/").replace(/^\.\//, "").toLowerCase(); +} + +function findingLocations(finding: SecurityFinding) { + return finding.occurrences.flatMap(occurrence => occurrence.locations); +} + +function taxonomyLocationMatch(reference: SecurityFinding, candidate: SecurityFinding): boolean { + const referenceCwes = new Set(reference.taxonomy.cwe.map(value => value.trim().toUpperCase())); + if ( + referenceCwes.size === 0 || + !candidate.taxonomy.cwe.some(value => referenceCwes.has(value.trim().toUpperCase())) + ) { + return false; + } + for (const referenceLocation of findingLocations(reference)) { + for (const candidateLocation of findingLocations(candidate)) { + if (normalizedPath(referenceLocation.path) !== normalizedPath(candidateLocation.path)) continue; + const referenceEnd = referenceLocation.endLine ?? referenceLocation.startLine; + const candidateEnd = candidateLocation.endLine ?? candidateLocation.startLine; + if ( + Math.max(referenceLocation.startLine, candidateLocation.startLine) <= + Math.min(referenceEnd, candidateEnd) || + Math.abs(referenceLocation.startLine - candidateLocation.startLine) <= 3 + ) { + return true; + } + } + } + return false; +} + function findingSummary(finding: SecurityFinding): SecurityDifferentialFindingSummary { const location = finding.occurrences.flatMap(occurrence => occurrence.locations)[0]; return { @@ -104,31 +137,46 @@ export function compareSecurityProducers( candidateByFallback.set(key, bucket); } const usedCandidateIds = new Set(); + const matchedReferenceIds = new Set(); const matches: SecurityDifferentialFindingMatch[] = []; + const addMatch = ( + referenceFinding: SecurityFinding, + candidateFinding: SecurityFinding, + basis: SecurityDifferentialFindingMatch["basis"], + ) => { + matchedReferenceIds.add(referenceFinding.id); + usedCandidateIds.add(candidateFinding.id); + matches.push({ + referenceFindingId: referenceFinding.id, + candidateFindingId: candidateFinding.id, + basis, + }); + }; for (const referenceFinding of reference.findings) { const exact = candidateByFingerprint.get(referenceFinding.fingerprint); - if (exact && !usedCandidateIds.has(exact.id)) { - usedCandidateIds.add(exact.id); - matches.push({ - referenceFindingId: referenceFinding.id, - candidateFindingId: exact.id, - basis: "fingerprint", - }); - continue; - } + if (exact && !usedCandidateIds.has(exact.id)) addMatch(referenceFinding, exact, "fingerprint"); + } + for (const referenceFinding of reference.findings) { + if (matchedReferenceIds.has(referenceFinding.id)) continue; const key = fallbackKey(referenceFinding); const fallback = key ? candidateByFallback.get(key)?.find(finding => !usedCandidateIds.has(finding.id)) : undefined; - if (!fallback) continue; - usedCandidateIds.add(fallback.id); - matches.push({ - referenceFindingId: referenceFinding.id, - candidateFindingId: fallback.id, - basis: "rule_location", - }); + if (fallback) addMatch(referenceFinding, fallback, "rule_location"); + } + const unmatchedReferences = reference.findings.filter(finding => !matchedReferenceIds.has(finding.id)); + for (const referenceFinding of unmatchedReferences) { + const compatibleCandidates = candidate.findings.filter( + finding => !usedCandidateIds.has(finding.id) && taxonomyLocationMatch(referenceFinding, finding), + ); + if (compatibleCandidates.length !== 1) continue; + const compatibleCandidate = compatibleCandidates[0]; + const compatibleReferences = unmatchedReferences.filter( + finding => !matchedReferenceIds.has(finding.id) && taxonomyLocationMatch(finding, compatibleCandidate), + ); + if (compatibleReferences.length !== 1) continue; + addMatch(referenceFinding, compatibleCandidate, "taxonomy_location"); } - const matchedReferenceIds = new Set(matches.map(match => match.referenceFindingId)); const referenceOnlyFindingIds = reference.findings .filter(finding => !matchedReferenceIds.has(finding.id)) .map(finding => finding.id); diff --git a/packages/coding-agent/src/security/contracts/types.ts b/packages/coding-agent/src/security/contracts/types.ts index b3ae9931e..8ecd779b4 100644 --- a/packages/coding-agent/src/security/contracts/types.ts +++ b/packages/coding-agent/src/security/contracts/types.ts @@ -241,7 +241,7 @@ export interface SecurityFindingMatch { afterFindingId?: string; fingerprint: string; status: "unchanged" | "new" | "resolved"; - matchBasis?: "fingerprint" | "rule_location"; + matchBasis?: "fingerprint" | "rule_location" | "taxonomy_location"; } export interface SecurityComparisonReport { diff --git a/packages/coding-agent/test/security/comparison.test.ts b/packages/coding-agent/test/security/comparison.test.ts index 7f3d48358..770a0c730 100644 --- a/packages/coding-agent/test/security/comparison.test.ts +++ b/packages/coding-agent/test/security/comparison.test.ts @@ -2,7 +2,14 @@ import { describe, expect, test } from "bun:test"; import type { SecurityFinding, SecurityScanBundle } from "../../src/security"; import { compareSecurityLineage, compareSecurityProducers } from "../../src/security"; -function finding(id: string, fingerprint: string, ruleId: string, path: string, startLine: number): SecurityFinding { +function finding( + id: string, + fingerprint: string, + ruleId: string, + path: string, + startLine: number, + cwe: string[] = [], +): SecurityFinding { return { id, scanId: "placeholder", @@ -12,7 +19,7 @@ function finding(id: string, fingerprint: string, ruleId: string, path: string, summary: id, severity: { level: "high" }, confidence: { level: "high" }, - taxonomy: { category: "test", cwe: [] }, + taxonomy: { category: "test", cwe }, occurrences: [{ id: `occ-${id}`, locations: [{ path, startLine }], evidenceIds: [] }], evidence: [], validation: { status: "unvalidated", evidenceIds: [] }, @@ -96,6 +103,37 @@ describe("security comparison", () => { ]); }); + test("matches producer-neutral taxonomy and nearby source locations only when unambiguous", () => { + const reference = bundle("secscan_reference", [ + finding("ref-cmd", "official-fp", "official.command", "src/command.ts", 3, ["CWE-78"]), + ]); + const candidate = bundle("secscan_candidate", [ + finding("cand-cmd", "native-fp", "native.shell", "./src/command.ts", 5, ["cwe-78"]), + ]); + const report = compareSecurityProducers(reference, candidate); + expect(report.matches).toEqual([ + { + referenceFindingId: "ref-cmd", + candidateFindingId: "cand-cmd", + basis: "taxonomy_location", + }, + ]); + }); + + test("leaves ambiguous taxonomy and location candidates unmatched", () => { + const reference = bundle("secscan_reference", [ + finding("ref-one", "ref-one-fp", "official.one", "src/shared.ts", 10, ["CWE-89"]), + finding("ref-two", "ref-two-fp", "official.two", "src/shared.ts", 12, ["CWE-89"]), + ]); + const candidate = bundle("secscan_candidate", [ + finding("cand", "cand-fp", "native.sql", "src/shared.ts", 11, ["CWE-89"]), + ]); + const report = compareSecurityProducers(reference, candidate); + expect(report.matches).toEqual([]); + expect(report.referenceOnlyFindingIds).toEqual(["ref-one", "ref-two"]); + expect(report.candidateOnlyFindingIds).toEqual(["cand"]); + }); + test("lineage classifies unchanged, resolved, and introduced findings", () => { const before = bundle("secscan_before", [ finding("before-shared", "fp-shared", "rule.shared", "src/a.ts", 1),