Skip to content

Fix two silent consistency-override security false-negatives (Stage-1 + Stage-2) - #195

Merged
gadievron merged 3 commits into
masterfrom
pr-2a-consistency-override-fixes
Jul 26, 2026
Merged

Fix two silent consistency-override security false-negatives (Stage-1 + Stage-2)#195
gadievron merged 3 commits into
masterfrom
pr-2a-consistency-override-fixes

Conversation

@gadievron

Copy link
Copy Markdown
Collaborator

Fix two silent consistency-override security false-negatives (Stage-1 + Stage-2)

Branch: pr-2a-consistency-override-fixes (3 commits off master/d4caf8a) · Target: knostic/OpenAnt:master
Diff: 4 files, +204 (2 fixes +44, 3 guard tests +160) · Not yet pushed — awaiting approval.

Problem

Both stages of the pipeline can silently downgrade a real vulnerable finding to safe via pattern-consistency, so the scan reports clean and exits 0.

  1. Stage 1 (utilities/stage1_consistency.py): run_stage1_consistency_check applies LLM-proposed verdict overrides unconditionally (result["verdict"] = new_verdict). A same-signature-pattern sibling that is SAFE can overwrite a real VULNERABLE finding. At Stage 1 there is no per-finding exploit/verification evidence — only pattern similarity, the weakest signal in the pipeline.
  2. Stage 2 (utilities/finding_verifier.py): the override guard _has_conclusive_exploit_path protects only conclusively-broken paths. A conclusively-exploitable finding (sink reached, attacker control full/partial, path unbroken) is not protected → it can be overwritten to a sibling's safe verdict. One-sided guard.

Both are silent false-negatives — the cardinal sin for a SAST tool.

Fix (directional, purely subtractive)

Make consistency-override directional: upgrades and lateral moves flow freely; only a downgrade-to-safe with weaker-than-existing evidence is blocked.

  • Stage 1: block VULNERABLE/BYPASSABLE → SAFE/PROTECTED downgrades; record stage1_consistency_downgrade_blocked for audit.
  • Stage 2: add the mirror predicate _has_conclusive_exploitable_path (defaults require proof — a missing field never reads as exploitable) + a direction-aware guard blocking a downgrade-to-safe of a conclusively-exploitable finding; record consistency_downgrade_blocked. The existing broken-path guard is untouched, and a legitimate vulnerable→bypassable refinement still applies.

Purely subtractive: it only declines to apply a proposed downgrade — it never synthesizes an upgrade (no group-inflation false positives).

Testing

  • New guard tests: tests/test_stage1_consistency_no_downgrade.py (4, incl. the cross-dir coupling test), tests/test_verifier_consistency_no_exploitable_downgrade.py (5) — 9 pass on this d4caf8a base. On the pre-fix code the 6 downgrade-block assertions FAIL (verifier 5/5, stage1 1/3); the other 2 stage1 tests assert unchanged behaviors (grouping, upgrade) and correctly pass on base — the standard RED/GREEN split for regression guards.
  • On the fork where developed: full suite 2299 passed / 0 failed; two runnable repros flip RED→GREEN.

Why it's safe (blast radius)

  • Casing-consistent (both sides normalized), no crash paths (isinstance/None-guarded), audit keys serialize freely (normalize_results keeps all dict keys; no strict schema).
  • Trade-off: blocking a downgrade can retain a false positive (deferred to the downstream FP filter) rather than drop a true positive — the correct direction for a security tool. Every retained finding carries an audit breadcrumb.

Prior art / provenance

  • Independently corroborated in the OpenAnt KB (openant-kb) as grade-A facts, re-derived at the KB pin c81c0e1 (Stage-1 stage1_consistency.py:241 unguarded override; Stage-2 one-sided _has_conclusive_exploit_path). Not in the KB's already-fixed / Do-Not-Reintroduce set.
  • Present at d4caf8a (this PR's base) in identical form — verified.

Reviewer notes

  • The two commits are independent (Stage-1 and Stage-2) and can be reviewed separately.
  • Interaction note for a concurrent "Add Swift" PR: Swift's same-basename cross-file findings feed Stage-1 basename grouping, where this guard can retain a Swift false positive (recall↑, precision↓, bounded, audited). Stage-2 groups by full path and is immune. Covered by the regression test test_same_basename_cross_directory_findings_group_and_downgrade_is_blocked. Not a blocker.
  • Operational triage hook (make the recommendation runnable): after a scan, list every retained downgrade for review with
    python -c "import json,sys; [print(r.get('route_key'), r.get('stage1_consistency_downgrade_blocked') or r.get('consistency_downgrade_blocked')) for r in json.load(open(sys.argv[1]))['results'] if 'stage1_consistency_downgrade_blocked' in r or 'consistency_downgrade_blocked' in r]" results.json
    (or jq '.results[] | select(.stage1_consistency_downgrade_blocked or .consistency_downgrade_blocked) | {route_key, stage1_consistency_downgrade_blocked, consistency_downgrade_blocked}' results.json).

Rollback

Git-revert-clean: the two commits touch only stage1_consistency.py + finding_verifier.py (+ their tests),
disjoint from every other in-flight change. Reverting restores the prior behavior exactly — with the caveat
that a revert re-opens the two silent vulnerable→safe false-negatives (that is the behavior being fixed).
Note the fix trades toward retaining a false positive over dropping a true positive; if a downstream consumer
mis-handles the new *_downgrade_blocked audit keys, revert is safe and immediate.

gadievron and others added 3 commits July 26, 2026 22:49
…ngrading VULNERABLE->SAFE

At Stage 1 there is no per-finding exploit evidence, only pattern similarity
(the weakest signal). run_stage1_consistency_check applied verdict overrides
unconditionally, so a same-signature-pattern sibling that is SAFE could overwrite
a real VULNERABLE finding -> the scan reports clean (silent security false-negative).

Block SURFACED(VULNERABLE/BYPASSABLE)->SAFEISH(SAFE/PROTECTED) downgrades and record
the rejected suggestion for audit; upgrades and lateral moves are unchanged.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ely-exploitable finding to safe

_has_conclusive_exploit_path only protects conclusively-BROKEN paths, so a
conclusively-EXPLOITABLE finding (sink reached, attacker control, path unbroken)
could be overwritten to a sibling's 'safe' verdict by the consistency resolver.

Add the mirror predicate _has_conclusive_exploitable_path (defaults require proof)
and a direction-aware guard that blocks a downgrade-to-safe of such a finding.
Existing broken-path guard untouched; a legitimate vulnerable->bypassable refinement
still applies.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
… (Swift-relevant)

Stage-1 groups by basename, so same-filename findings in different directories group.
This regression test asserts the downgrade-block + audit breadcrumb on that vector - the
interaction a concurrent Swift-parser PR would exercise (mechanism is language-agnostic).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@gadievron
gadievron merged commit 2dd143c into master Jul 26, 2026
5 of 9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant