fix: salvage optional finding fields instead of discarding the finding

[Haiduongcable]

Summary

finalize_scan_contract.py already salvages a malformed writeup, remediationTests, or
preventiveControls field by dropping just that field and keeping the finding. Any other schema
mismatch falls through to the outer handler and discards the entire finding instead.

Two concrete cases hit this even though the finding's actual content is otherwise fine:

• 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 (missing
  explanation, wrong shape, missing id, duplicate id, etc.) currently fails the whole finding
  instead of just that auxiliary field, unlike its sibling optional fields.

A real, otherwise-valid finding can vanish from report.md, findings.json, SARIF, and CSV because
one optional field was malformed, with only a generic completion warning as a trace.

Changes

• Backfill a missing taxonomy.cwe key to an empty list before final schema validation, matching
  the hand-rolled validator's own tolerant default.
• Extend the existing auxiliary-field salvage pattern to codeEvidence: a malformed entry drops
  just that field, keeping the finding.
• Move codeEvidence recovery ahead of _validate_finding. The hand-rolled validator independently
  rejects some codeEvidence malformations (non-array, non-object entries, missing id, duplicate
  id) before the schema-based recovery would otherwise run, so validating/stripping codeEvidence
  first ensures every malformation type is salvaged consistently, not only the ones a bare
  JSON-schema check would catch.
• Dropping codeEvidence can leave dangling evidenceRefs in rootCause, validation, and
  attackPath — including the documented nested attackPath.dataflow.evidenceRefs location. These
  are pruned in the same pass, with a recovery warning recorded, so the sealed contract never
  references removed evidence.

Deliberately not changed

identity and confidence.rationale remain hard requirements in both validators. There is no safe
default for identity, and confidence.rationale is a genuine content requirement rather than a
validator mismatch, so a finding missing either is still (correctly) discarded.

Verification

Reproduced directly against finalize_scan_contract.py and the bundled example finding before any
change: a missing taxonomy.cwe and five distinct malformed-codeEvidence shapes each discarded
the whole finding. After the fix, all seven cases are recovered — finding kept, offending
field/reference stripped, warning recorded — while identity/confidence.rationale cases are
unchanged.

Added a regression case to tests-ts/scan-recovery.test.ts covering all of the above, including the
nested attackPath.dataflow.evidenceRefs case. Confirmed the test fails against unpatched
upstream/main source (findingCount 4 → 1, all three malformed findings fully discarded) before
passing against the fix.

Full local gate: pnpm run types, pnpm run test (702 pass / 5 skip, 0 fail), pnpm run format,
pnpm run build, and the packaging check (pnpm pack + check:package) all pass.

[Haiduongcable]

Fork CI is green on this exact commit, run within the fork so the full matrix executes without
requiring workflow approval: Haiduongcable#3

Commit: e09e0dbdeeff8e9d40a07afe4f2967ed99cd4af3

Check	Result	Duration
ubuntu-latest / node-22	pass	1m27s
ubuntu-latest / node-24	pass	1m30s
ubuntu-latest / node-24.0.0	pass	1m45s
ubuntu-latest / node-26	pass	1m44s
ubuntu-latest / node-26.0.0	pass	1m41s
macos-latest / node-22	pass	2m41s
windows-latest / node-22	pass	4m52s
linux-amd64	pass	1m27s

Local verification (sdk/typescript):

• pnpm run types — clean
• pnpm run test — 702 pass / 5 skip / 0 fail (701 pass / 5 skip on unpatched upstream/main; the
  new regression case fails against unpatched source before passing against this change)
• pnpm run format — clean
• pnpm run build — clean
• pnpm pack + pnpm run check:package — clean

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e09e0dbdee

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Reject unhashable evidence references before set lookup

When a rootCause, validation, attackPath, or nested dataflow evidenceRefs array contains a JSON object or array, ref in known_evidence_ids attempts to hash it and raises TypeError. The recovery loop and CLI only catch ContractError, so one malformed optional reference now aborts scan completion with a traceback instead of recovering or discarding that finding as before; validate each reference as a string before the set lookup.

Useful? React with 👍 / 👎.

[riju568]

Issue & Root Cause Analysis Problem When there is a little bit of the schema mismatch on the optional fields (taxonomy.cwe not exists or garbage value is there for elements in codeEvidence), the whole findings have been completely dropped in finalizescancontract.py. The New bug noticed during code review When we try to prune orphaned evidenceRefs after cleaning out the ill-formed codeEvidence, we use the lookup on sets ref in knownevidenceids. If one of the elements of a list of evidenceRefs is not hashable (due to ill-formed LLM or scanner's garbage output contains either dictionary/object or a nested list).

Then we got TypeError.

As we have only wrapped contract checks within ContractError handler, this new type of error will crack the cli application down and leave us with trace backs.

Python logic after changes (finalizescancontract.py) In the references pruning logic used for root Cause, validation and attack Path (including in attackPath.dataflow), remove such references:
in the logic change

V

known_evidence_ids = {
ev["id"]
for ev in finding.get("codeEvidence", [])
if isinstance(ev, dict) and isinstance(ev.get("id"), str)
}

def prune_evidence_refs(container: dict) -> bool:
"""Safely prune unhashable or missing evidence references from a container dict."""
refs = container.get("evidenceRefs")
if not isinstance(refs, list):
return False

modified = False
kept_refs = []
for ref in refs:
    # Check that ref is a string before checking set membership to prevent TypeError
    if isinstance(ref, str) and ref in known_evidence_ids:
        kept_refs.append(ref)
    else:
        modified = True

if modified:
    if kept_refs:
        container["evidenceRefs"] = kept_refs
    else:
        container.pop("evidenceRefs", None)

return modified


-----------------------------------------------------------------------

[riju568]

known_evidence_ids = {
ev["id"]
for ev in finding.get("codeEvidence", [])
if isinstance(ev, dict) and isinstance(ev.get("id"), str)
}

def prune_evidence_refs(container: dict) -> bool:
"""Safely prune unhashable or missing evidence references from a container dict."""
refs = container.get("evidenceRefs")
if not isinstance(refs, list):
return False

modified = False
kept_refs = []
for ref in refs:
# Check that ref is a string before checking set membership to prevent TypeError
if isinstance(ref, str) and ref in known_evidence_ids:
kept_refs.append(ref)
else:
modified = True

if modified:
if kept_refs:
container["evidenceRefs"] = kept_refs
else:
container.pop("evidenceRefs", None)

return modified

finalize_scan_contract.py

[Haiduongcable]

Addressed the automated review finding above (P1: unhashable evidence references before set lookup).

Root cause: 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. known_evidence_ids is a Python set, and an evidenceRefs entry that is a JSON
object or array is unhashable, so the membership test itself raises TypeError. The recovery loop
and the CLI are only prepared to catch ContractError, so one malformed optional reference aborted
scan completion with a raw traceback instead of being recovered or discarded like every other
malformed field on this path.

Fix: guard the membership test with isinstance(ref, str) first, so non-string entries — an
object, an array, or any other non-string but otherwise-hashable value — are pruned the same way a
missing or unknown reference already is, instead of crashing:

kept_refs = [
    ref for ref in refs if isinstance(ref, str) and ref in known_evidence_ids
]

Verification:

• Reproduced the exact crash directly against _recover_unsealed_findings before making any change:
  a codeEvidence entry malformed enough to be stripped, plus an evidenceRefs list containing a
  dict, raised TypeError: unhashable type: 'dict', uncaught.
• Added a regression case to tests-ts/scan-recovery.test.ts covering non-string evidenceRefs
  entries (object, array, and a hashable-but-non-string value). Confirmed it fails against the
  pre-fix commit with the same uncaught TypeError propagating out of the workbench subprocess,
  then passes against the fix.
• Full local gate: pnpm run types, pnpm run test (708 pass / 5 skip, 0 fail), pnpm run format,
  pnpm run build all pass.
• Fork CI green on 7a8f60ecc1e56ff685a057e7c58befd9623a51e6:

Check	Result	Duration
ubuntu-latest / node-22	pass	1m46s
ubuntu-latest / node-24	pass	1m44s
ubuntu-latest / node-24.0.0	pass	1m37s
ubuntu-latest / node-26	pass	1m46s
ubuntu-latest / node-26.0.0	pass	1m44s
macos-latest / node-22	pass	1m57s
windows-latest / node-22	pass	4m51s
linux-amd64	pass	1m40s

(Haiduongcable#3)

[Haiduongcable]

@riju568 thanks for the writeup and the proposed patch — the isinstance guard was exactly the
right direction, and having the failure mode spelled out that clearly made it quick to reproduce and
confirm before fixing. Addressed above, with a regression test that fails against the pre-fix commit
and a fresh green CI run.