From 5c80081cb7fcaa5c36921e6069b55cca042e0e75 Mon Sep 17 00:00:00 2001 From: Hai Duong Date: Fri, 31 Jul 2026 14:32:17 +0700 Subject: [PATCH 1/3] fix: salvage optional finding fields instead of discarding the finding The unsealed-findings recovery path already salvages malformed writeup, remediationTests, and preventiveControls by dropping just that field and keeping the finding, but any other schema mismatch falls through to the outer handler and discards the entire finding. Two concrete cases hit this without any of the finding's actual content being unrecoverable: - taxonomy.cwe is optional in the hand-rolled validator (defaults to an empty list) but required as a key by the JSON schema, so a finding missing the key outright is silently dropped in full. - codeEvidence is optional at the finding level, but a single malformed entry (e.g. missing `explanation`) currently fails the whole finding instead of just that auxiliary field, unlike its sibling optional fields. This extends the existing salvage pattern to both: codeEvidence joins the existing auxiliary-field recovery list, and a missing taxonomy.cwe is backfilled to an empty list. Dropping codeEvidence can leave dangling evidenceRefs in rootCause/validation/attackPath, so those are pruned in the same pass and a recovery warning is recorded. identity and confidence.rationale are deliberately left as hard requirements in both validators; there is no safe default for identity, and confidence.rationale is a real content requirement rather than a validator mismatch. --- .../scripts/finalize_scan_contract.py | 30 +++++++- sdk/typescript/tests-ts/scan-recovery.test.ts | 77 +++++++++++++++++++ 2 files changed, 106 insertions(+), 1 deletion(-) diff --git a/sdk/typescript/_bundled_plugin/scripts/finalize_scan_contract.py b/sdk/typescript/_bundled_plugin/scripts/finalize_scan_contract.py index 3119005..e7e0e70 100644 --- a/sdk/typescript/_bundled_plugin/scripts/finalize_scan_contract.py +++ b/sdk/typescript/_bundled_plugin/scripts/finalize_scan_contract.py @@ -800,7 +800,7 @@ def _recover_unsealed_findings( name: _require_dict( finding_properties, name, "findings.schema.properties.findings.items.properties" ) - for name in ("remediationTests", "preventiveControls") + for name in ("remediationTests", "preventiveControls", "codeEvidence") } scan = _require_dict(manifest, "scan", "manifest") scan_id = _require_str(scan, "id", "manifest.scan") @@ -872,6 +872,34 @@ def _recover_unsealed_findings( warnings.append( f"Skipped malformed {auxiliary} for finding {index + 1}: {exc}." ) + + taxonomy = finding.get("taxonomy") + if isinstance(taxonomy, dict) and "cwe" not in taxonomy: + taxonomy["cwe"] = [] + warnings.append( + f"Recovered finding {index + 1}: backfilled missing taxonomy.cwe with an empty list." + ) + + known_evidence_ids = { + evidence["id"] + for evidence in finding.get("codeEvidence") or [] + if isinstance(evidence, dict) and isinstance(evidence.get("id"), str) + } + for section_name in ("rootCause", "validation", "attackPath"): + section = finding.get(section_name) + if not isinstance(section, dict): + continue + refs = section.get("evidenceRefs") + if not isinstance(refs, list): + continue + kept_refs = [ref for ref in refs if ref in known_evidence_ids] + if len(kept_refs) != len(refs): + section["evidenceRefs"] = kept_refs + warnings.append( + f"Recovered finding {index + 1}: dropped dangling {section_name}.evidenceRefs " + "after codeEvidence recovery." + ) + _validate_schema_node(finding, finding_schema, context) except ContractError as exc: warning = f"Skipped malformed finding {index + 1}: {exc}." diff --git a/sdk/typescript/tests-ts/scan-recovery.test.ts b/sdk/typescript/tests-ts/scan-recovery.test.ts index 2da5fb9..bf1622c 100644 --- a/sdk/typescript/tests-ts/scan-recovery.test.ts +++ b/sdk/typescript/tests-ts/scan-recovery.test.ts @@ -749,6 +749,83 @@ describe("malformed scan artifact recovery", () => { } }); + test("keeps findings while recovering missing taxonomy CWE and malformed code evidence", async () => { + const fixture = await startDraftScan(); + const path = join(fixture.scanDir, "findings.json"); + const document = await readJson(path); + const valid = document.findings[0]!; + + const missingCwe = structuredClone(valid); + missingCwe.identity.anchor = "missing-cwe"; + const missingCweTaxonomy = (missingCwe as Record)[ + "taxonomy" + ] as Partial<{ cwe: string[] }>; + delete missingCweTaxonomy.cwe; + + const malformedEvidence = structuredClone(valid); + malformedEvidence.identity.anchor = "malformed-evidence"; + (malformedEvidence as Record)["codeEvidence"] = [ + // Missing the required `explanation` field. + { + id: "ev1", + label: "sink", + path: "src/extract.py", + startLine: 1, + code: "x = 1", + }, + ]; + (malformedEvidence as Record)["rootCause"] = { + summary: "Dangling reference regression check.", + evidenceRefs: ["ev1"], + }; + + document.findings.push(missingCwe, malformedEvidence); + await writeJson(path, document); + + const completed = await completeScan(fixture); + + expect(completed.progress.status).toBe("complete"); + expect(completed.findingCount).toBe(3); + expect(completed.warnings).toHaveLength(3); + expect( + completed.warnings.some( + (warning) => + warning.includes("Recovered finding") && + warning.includes("backfilled missing taxonomy.cwe"), + ), + ).toBe(true); + expect( + completed.warnings.some((warning) => + warning.startsWith("Skipped malformed codeEvidence for finding"), + ), + ).toBe(true); + expect( + completed.warnings.some( + (warning) => + warning.includes("Recovered finding") && + warning.includes("dropped dangling rootCause.evidenceRefs"), + ), + ).toBe(true); + + const recovered = (await readJson(path)).findings; + const missingCweRecovered = recovered.find( + (finding) => finding?.identity.anchor === "missing-cwe", + ); + expect(missingCweRecovered).toBeDefined(); + expect( + (missingCweRecovered as Record)?.["taxonomy"], + ).toMatchObject({ cwe: [] }); + + const malformedEvidenceRecovered = recovered.find( + (finding) => finding?.identity.anchor === "malformed-evidence", + ); + expect(malformedEvidenceRecovered).toBeDefined(); + expect(malformedEvidenceRecovered).not.toHaveProperty("codeEvidence"); + expect( + (malformedEvidenceRecovered as Record)?.["rootCause"], + ).toMatchObject({ evidenceRefs: [] }); + }); + test("keeps verified coverage receipts and downgrades invalid coverage", async () => { const fixture = await startDraftScan(); const path = join(fixture.scanDir, "coverage.json"); From e09e0dbdeeff8e9d40a07afe4f2967ed99cd4af3 Mon Sep 17 00:00:00 2001 From: Hai Duong Date: Fri, 31 Jul 2026 14:44:32 +0700 Subject: [PATCH 2/3] fix: recover code evidence ahead of whole-finding validation The previous change recovered codeEvidence only for schema mismatches the hand-rolled _validate_finding validator doesn't itself check (a missing explanation). Any malformation _validate_finding does check directly -- a non-array codeEvidence, a non-object entry, a missing id, or a duplicate id -- still raised inside _validate_finding before the codeEvidence recovery ran, discarding the whole finding exactly as before. Move codeEvidence validation (schema check plus the same duplicate-id check _validate_finding performs) ahead of _validate_finding, so any malformation is caught and the field dropped -- never the finding -- regardless of which validator would have caught it. Prune dangling evidenceRefs before _validate_finding as well, since that function's own referential check would otherwise fail on the now-removed ids. Also prune the documented nested attackPath.dataflow.evidenceRefs location, not only the top-level attackPath.evidenceRefs sibling field, so a stripped codeEvidence entry cannot leave a dangling reference findings.json still contains no consumer resolves. --- .../scripts/finalize_scan_contract.py | 86 +++++++++++++------ sdk/typescript/tests-ts/scan-recovery.test.ts | 71 +++++++++++++-- 2 files changed, 125 insertions(+), 32 deletions(-) diff --git a/sdk/typescript/_bundled_plugin/scripts/finalize_scan_contract.py b/sdk/typescript/_bundled_plugin/scripts/finalize_scan_contract.py index e7e0e70..35e5ab2 100644 --- a/sdk/typescript/_bundled_plugin/scripts/finalize_scan_contract.py +++ b/sdk/typescript/_bundled_plugin/scripts/finalize_scan_contract.py @@ -796,11 +796,14 @@ def _recover_unsealed_findings( writeup_schema = _require_dict( finding_properties, "writeup", "findings.schema.properties.findings.items.properties" ) + code_evidence_schema = _require_dict( + finding_properties, "codeEvidence", "findings.schema.properties.findings.items.properties" + ) auxiliary_schemas = { name: _require_dict( finding_properties, name, "findings.schema.properties.findings.items.properties" ) - for name in ("remediationTests", "preventiveControls", "codeEvidence") + for name in ("remediationTests", "preventiveControls") } scan = _require_dict(manifest, "scan", "manifest") scan_id = _require_str(scan, "id", "manifest.scan") @@ -842,35 +845,25 @@ def _recover_unsealed_findings( ) finding_id = finding["findingId"] previous_position = finding_positions.get(finding_id) - _validate_finding(finding, context) - if "writeup" in finding: - try: - _validate_schema_node(finding["writeup"], writeup_schema, f"{context}.writeup") - report_path = finding["writeup"]["reportPath"] - previous_writeup = ( - recovered[previous_position].get("writeup") - if previous_position is not None - else None - ) - if report_path in writeup_paths and ( - previous_writeup is None or previous_writeup["reportPath"] != report_path - ): - raise ContractError(f"{context}.writeup.reportPath: duplicate report path") - _require_scan_local_file(scan_dir, report_path, f"{context}.writeup.reportPath") - except ContractError as exc: - finding.pop("writeup") - warnings.append(f"Skipped malformed writeup for finding {index + 1}: {exc}.") - for auxiliary, auxiliary_schema in auxiliary_schemas.items(): - if auxiliary not in finding: - continue + + if "codeEvidence" in finding: try: _validate_schema_node( - finding[auxiliary], auxiliary_schema, f"{context}.{auxiliary}" + finding["codeEvidence"], code_evidence_schema, f"{context}.codeEvidence" ) + seen_evidence_ids: set[str] = set() + for evidence_index, evidence in enumerate(finding["codeEvidence"]): + evidence_id = evidence["id"] + if evidence_id in seen_evidence_ids: + raise ContractError( + f"{context}.codeEvidence[{evidence_index}].id: " + "duplicate code-evidence id" + ) + seen_evidence_ids.add(evidence_id) except ContractError as exc: - finding.pop(auxiliary) + finding.pop("codeEvidence") warnings.append( - f"Skipped malformed {auxiliary} for finding {index + 1}: {exc}." + f"Skipped malformed codeEvidence for finding {index + 1}: {exc}." ) taxonomy = finding.get("taxonomy") @@ -885,8 +878,17 @@ def _recover_unsealed_findings( for evidence in finding.get("codeEvidence") or [] if isinstance(evidence, dict) and isinstance(evidence.get("id"), str) } - for section_name in ("rootCause", "validation", "attackPath"): - section = finding.get(section_name) + attack_path = finding.get("attackPath") + evidence_ref_sections = [ + (finding.get("rootCause"), "rootCause"), + (finding.get("validation"), "validation"), + (attack_path, "attackPath"), + ] + if isinstance(attack_path, dict): + evidence_ref_sections.append( + (attack_path.get("dataflow"), "attackPath.dataflow") + ) + for section, section_name in evidence_ref_sections: if not isinstance(section, dict): continue refs = section.get("evidenceRefs") @@ -900,6 +902,36 @@ def _recover_unsealed_findings( "after codeEvidence recovery." ) + _validate_finding(finding, context) + if "writeup" in finding: + try: + _validate_schema_node(finding["writeup"], writeup_schema, f"{context}.writeup") + report_path = finding["writeup"]["reportPath"] + previous_writeup = ( + recovered[previous_position].get("writeup") + if previous_position is not None + else None + ) + if report_path in writeup_paths and ( + previous_writeup is None or previous_writeup["reportPath"] != report_path + ): + raise ContractError(f"{context}.writeup.reportPath: duplicate report path") + _require_scan_local_file(scan_dir, report_path, f"{context}.writeup.reportPath") + except ContractError as exc: + finding.pop("writeup") + warnings.append(f"Skipped malformed writeup for finding {index + 1}: {exc}.") + for auxiliary, auxiliary_schema in auxiliary_schemas.items(): + if auxiliary not in finding: + continue + try: + _validate_schema_node( + finding[auxiliary], auxiliary_schema, f"{context}.{auxiliary}" + ) + except ContractError as exc: + finding.pop(auxiliary) + warnings.append( + f"Skipped malformed {auxiliary} for finding {index + 1}: {exc}." + ) _validate_schema_node(finding, finding_schema, context) except ContractError as exc: warning = f"Skipped malformed finding {index + 1}: {exc}." diff --git a/sdk/typescript/tests-ts/scan-recovery.test.ts b/sdk/typescript/tests-ts/scan-recovery.test.ts index bf1622c..d772b05 100644 --- a/sdk/typescript/tests-ts/scan-recovery.test.ts +++ b/sdk/typescript/tests-ts/scan-recovery.test.ts @@ -779,14 +779,49 @@ describe("malformed scan artifact recovery", () => { evidenceRefs: ["ev1"], }; - document.findings.push(missingCwe, malformedEvidence); + // A codeEvidence malformation caught by the hand-rolled validator itself + // (duplicate ids), not only by the JSON-schema pass, plus a dangling + // reference in the nested `attackPath.dataflow.evidenceRefs` location. + const duplicateEvidence = structuredClone(valid); + duplicateEvidence.identity.anchor = "duplicate-evidence"; + (duplicateEvidence as Record)["codeEvidence"] = [ + { + id: "ev1", + label: "a", + path: "src/extract.py", + startLine: 1, + code: "x = 1", + explanation: "first", + }, + { + id: "ev1", + label: "b", + path: "src/extract.py", + startLine: 2, + code: "y = 2", + explanation: "second", + }, + ]; + (duplicateEvidence as Record)["attackPath"] = { + summary: "Nested dangling reference regression check.", + dataflow: { + summary: "source -> sink", + source: "input", + sink: "output", + outcome: "impact", + evidenceRefs: ["ev1"], + }, + evidenceRefs: ["ev1"], + }; + + document.findings.push(missingCwe, malformedEvidence, duplicateEvidence); await writeJson(path, document); const completed = await completeScan(fixture); expect(completed.progress.status).toBe("complete"); - expect(completed.findingCount).toBe(3); - expect(completed.warnings).toHaveLength(3); + expect(completed.findingCount).toBe(4); + expect(completed.warnings).toHaveLength(6); expect( completed.warnings.some( (warning) => @@ -795,10 +830,10 @@ describe("malformed scan artifact recovery", () => { ), ).toBe(true); expect( - completed.warnings.some((warning) => + completed.warnings.filter((warning) => warning.startsWith("Skipped malformed codeEvidence for finding"), ), - ).toBe(true); + ).toHaveLength(2); expect( completed.warnings.some( (warning) => @@ -806,6 +841,20 @@ describe("malformed scan artifact recovery", () => { warning.includes("dropped dangling rootCause.evidenceRefs"), ), ).toBe(true); + expect( + completed.warnings.some( + (warning) => + warning.includes("Recovered finding") && + warning.includes("dropped dangling attackPath.evidenceRefs"), + ), + ).toBe(true); + expect( + completed.warnings.some( + (warning) => + warning.includes("Recovered finding") && + warning.includes("dropped dangling attackPath.dataflow.evidenceRefs"), + ), + ).toBe(true); const recovered = (await readJson(path)).findings; const missingCweRecovered = recovered.find( @@ -824,6 +873,18 @@ describe("malformed scan artifact recovery", () => { expect( (malformedEvidenceRecovered as Record)?.["rootCause"], ).toMatchObject({ evidenceRefs: [] }); + + const duplicateEvidenceRecovered = recovered.find( + (finding) => finding?.identity.anchor === "duplicate-evidence", + ); + expect(duplicateEvidenceRecovered).toBeDefined(); + expect(duplicateEvidenceRecovered).not.toHaveProperty("codeEvidence"); + expect( + (duplicateEvidenceRecovered as Record)?.["attackPath"], + ).toMatchObject({ + evidenceRefs: [], + dataflow: { evidenceRefs: [] }, + }); }); test("keeps verified coverage receipts and downgrades invalid coverage", async () => { From 1119e188189b91437100c1629199ce6b66d5dee1 Mon Sep 17 00:00:00 2001 From: Hai Duong Date: Fri, 31 Jul 2026 17:34:47 +0700 Subject: [PATCH 3/3] fix: guard evidenceRefs pruning against non-string references The dangling-evidenceRefs pruning added alongside codeEvidence recovery tested set membership directly (`ref in known_evidence_ids`) without first checking that each reference was a string. An evidenceRefs entry that is a JSON object or array is unhashable, so the membership test raises TypeError instead of the ContractError the recovery loop and CLI are prepared to handle -- one malformed optional reference now aborted scan completion with a traceback instead of being recovered or discarded like every other malformed field in this path. Guard the membership test with an isinstance check first so non-string entries (objects, arrays, or otherwise-hashable non-string values) are pruned the same way missing or unknown references already are, rather than crashing. --- .../scripts/finalize_scan_contract.py | 4 +- sdk/typescript/tests-ts/scan-recovery.test.ts | 57 +++++++++++++++++++ 2 files changed, 60 insertions(+), 1 deletion(-) diff --git a/sdk/typescript/_bundled_plugin/scripts/finalize_scan_contract.py b/sdk/typescript/_bundled_plugin/scripts/finalize_scan_contract.py index 35e5ab2..67f8e8b 100644 --- a/sdk/typescript/_bundled_plugin/scripts/finalize_scan_contract.py +++ b/sdk/typescript/_bundled_plugin/scripts/finalize_scan_contract.py @@ -894,7 +894,9 @@ def _recover_unsealed_findings( refs = section.get("evidenceRefs") if not isinstance(refs, list): continue - kept_refs = [ref for ref in refs if ref in known_evidence_ids] + kept_refs = [ + ref for ref in refs if isinstance(ref, str) and ref in known_evidence_ids + ] if len(kept_refs) != len(refs): section["evidenceRefs"] = kept_refs warnings.append( diff --git a/sdk/typescript/tests-ts/scan-recovery.test.ts b/sdk/typescript/tests-ts/scan-recovery.test.ts index d772b05..49d0dcd 100644 --- a/sdk/typescript/tests-ts/scan-recovery.test.ts +++ b/sdk/typescript/tests-ts/scan-recovery.test.ts @@ -887,6 +887,63 @@ describe("malformed scan artifact recovery", () => { }); }); + test("prunes non-string evidenceRefs entries instead of crashing recovery", async () => { + const fixture = await startDraftScan(); + const path = join(fixture.scanDir, "findings.json"); + const document = await readJson(path); + const valid = document.findings[0]!; + + // codeEvidence is malformed so it gets stripped, and evidenceRefs mixes a + // valid string with unhashable (object/array) and non-string (number) + // garbage that a malfunctioning producer could emit. Recovery must prune + // these rather than crash when testing set membership. + const garbageRefs = structuredClone(valid); + garbageRefs.identity.anchor = "garbage-evidence-refs"; + (garbageRefs as Record)["codeEvidence"] = [ + { + id: "ev1", + label: "sink", + path: "src/extract.py", + startLine: 1, + code: "x = 1", + }, + ]; + (garbageRefs as Record)["rootCause"] = { + summary: "Non-string evidenceRefs regression check.", + evidenceRefs: ["ev1", { nested: "garbage" }, ["nested", "garbage"], 42], + }; + + document.findings.push(garbageRefs); + await writeJson(path, document); + + const completed = await completeScan(fixture); + + expect(completed.progress.status).toBe("complete"); + expect(completed.findingCount).toBe(2); + expect( + completed.warnings.some((warning) => + warning.startsWith("Skipped malformed codeEvidence for finding"), + ), + ).toBe(true); + expect( + completed.warnings.some( + (warning) => + warning.includes("Recovered finding") && + warning.includes("dropped dangling rootCause.evidenceRefs"), + ), + ).toBe(true); + + const recovered = (await readJson(path)).findings; + const garbageRefsRecovered = recovered.find( + (finding) => finding?.identity.anchor === "garbage-evidence-refs", + ); + expect(garbageRefsRecovered).toBeDefined(); + expect(garbageRefsRecovered).not.toHaveProperty("codeEvidence"); + expect( + (garbageRefsRecovered as Record)?.["rootCause"], + ).toMatchObject({ evidenceRefs: [] }); + }); + test("keeps verified coverage receipts and downgrades invalid coverage", async () => { const fixture = await startDraftScan(); const path = join(fixture.scanDir, "coverage.json");