From a5e340f872967572bc7c9ea77f8aebc35ff40435 Mon Sep 17 00:00:00 2001 From: Gal Ankonina Date: Mon, 15 Jun 2026 17:10:39 +0300 Subject: [PATCH 1/2] =?UTF-8?q?fix(triage):=20fail=20safe=20on=20code/SAST?= =?UTF-8?q?=20findings=20=E2=80=94=20never=20a=20speculative=20`real`=20(A?= =?UTF-8?q?DR-0051/0052)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Quick-read synthesizer shipped fake CRITICAL `real` verdicts on SAST/code findings. Root cause (Mealie round-3 audit): the exposure analyzer reasons about a code finding from the file PATH, not the file's code, and hedges ("Likely reachable … needs verification to confirm") — but `_classify_reachable` matched the affirmative keywords ("likely"/"reachable") → "yes", and with `internet_facing=true` the dependency-shaped projection emitted `real` 0.82. The Deep dive that would open the file:line never ran (no repo profile/clone), so the speculation shipped as the final verdict. Four fail-safes, all upholding ADR-0051 §10 (never false-clear, never false-confirm): - Hedge-aware reachability: "likely / suggests / appears / needs verification" classify as undetermined, never a confident reachable. `likely` is a probability, not a confirmation — removed from the affirmative set. - Code/SAST findings defer: `synthesize_triage` gains `finding_type`; a `code` finding can't be confidently cleared OR confirmed from the Quick read (it never opens the file), so it returns `needs_review`, which auto-escalates to the file-reading Deep dive. Dependency/posture keep the existing projection. - Triage never strands: a failed prerequisite (e.g. the exposure agent timing out on a deploy-time migration file) degrades to `needs_review` instead of aborting with no verdict — the prior behavior left the CLI to poll-timeout and exit 1. Triage always lands a verdict; never a silent clear, never a crash. - Deep-dive grounded-path guard: a `trace_path` `reached=yes` with no file:line hop is ungrounded speculation → `needs_review`, not a confident `real` (symmetric to the disproof challenge on the clearing side). Verified: all 5 Mealie noise findings now triage to `needs_review` (zero `real`), both deterministically against the recorded exposure inputs and live end-to-end through the daemon. The two prior `real` false positives and the migration crash are gone. Also cuts the 0.2.2 release (VERSION + backend + cliffsec CLI + CHANGELOG) so the triage-first `cliffsec fix` flow (#276) reaches installed users. Co-Authored-By: Claude Opus 4.8 (1M context) --- CHANGELOG.md | 28 +++++ VERSION | 2 +- .../agents/runtime/triage_synthesizer.py | 111 +++++++++++++++++- backend/cliff/agents/triage_deep/runner.py | 31 +++++ backend/cliff/agents/triage_runner.py | 31 ++++- backend/pyproject.toml | 2 +- backend/tests/agents/test_deep_dive_runner.py | 22 +++- backend/tests/agents/test_triage_runner.py | 32 ++++- .../tests/agents/test_triage_synthesizer.py | 89 ++++++++++++++ backend/uv.lock | 2 +- cli/cliff_cli/__init__.py | 2 +- cli/pyproject.toml | 2 +- cli/uv.lock | 2 +- 13 files changed, 334 insertions(+), 22 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 93344d67..883c2468 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,8 +7,36 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +## [0.2.2] - 2026-06-15 + +Triage hardening for SAST/code findings — the Plan-gate bright line ("a +report must never carry a false positive") now holds for code findings the +way it already did for dependencies. Plus the `cliffsec` CLI release that +carries the triage-first `fix` flow to users. + ### Fixed +- **Code/SAST triage no longer ships fake `real` verdicts.** The Quick-read + synthesizer projected a confident `real` for code findings whenever the + exposure analyzer's free-text reachability *looked* affirmative — including + pure speculation like "likely reachable … needs verification to confirm" + (the analyzer reasons from the file path, never opening the flagged line). + On a SAST-heavy repo that shipped fake CRITICAL SQL-injection findings. Two + fail-safes now guard the verdict: (1) reachability classification is + hedge-aware — "likely / suggests / appears / needs verification" is treated + as *undetermined*, never a confident reachable; (2) a `code` finding can't + be confidently cleared OR confirmed from the Quick read at all — it defers + to `needs_review`, which auto-escalates to the file-reading Deep dive + (ADR-0052), where the `file:line` is actually opened before any verdict. +- **Triage never strands on a failed prerequisite.** When the enricher or + exposure agent failed (e.g. a 150s timeout on a deploy-time migration file), + triage aborted with no verdict — leaving the CLI to time out and exit 1. It + now degrades to a `needs_review` verdict landed in the sidebar and chat + card. Triage always produces a verdict; never a silent clear, never a crash. +- **Deep dive won't confirm an ungrounded path.** A `trace_path` result that + claims `reached=yes` but cites no `file:line` hop is now routed to + `needs_review` instead of proceeding to a confident `real` — the symmetric + guard to the disproof challenge on the clearing side. - **OpenAI BYOK save now works.** The validator probe was hardcoded to `gpt-5`, which is in OpenAI's reasoning-model family and rejects `max_tokens` with a 400. The non-classified-4xx catch-all rendered diff --git a/VERSION b/VERSION index 0c62199f..ee1372d3 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -0.2.1 +0.2.2 diff --git a/backend/cliff/agents/runtime/triage_synthesizer.py b/backend/cliff/agents/runtime/triage_synthesizer.py index da77504f..f354d188 100644 --- a/backend/cliff/agents/runtime/triage_synthesizer.py +++ b/backend/cliff/agents/runtime/triage_synthesizer.py @@ -21,6 +21,24 @@ ``NEEDS_REVIEW_CONFIDENCE`` threshold or exploitability is ``unknown`` (ADR-0051 §9). The threshold is a tunable constant, not a magic number frozen in the branches. + +Two fail-safes guard against shipping a confident verdict the quick read hasn't +earned (the Plan-gate bright line — a report must never carry a false positive): + +1. **Speculative reachability never confirms.** The exposure analyzer's + free-text ``reachable`` is classified hedge-aware: phrasing like "likely … + needs verification to confirm" is *speculation*, not a confirmed reachable, + so it maps to ``unknown`` (→ needs_review), never a confident ``real``. + +2. **Code/SAST findings defer.** For a ``code`` finding the exposure analyzer + reasons from the file PATH, never the file's code — so the quick read can't + responsibly clear OR confirm it. It defers to ``needs_review``, which + auto-escalates to the file-reading Deep dive (ADR-0052), where the flagged + ``file:line`` is actually opened before any verdict. The dependency-shaped + projection above (abstention → false_positive, reachable+facing → real) + assumes a CVE/advisory model that does not hold for code findings, where the + enricher always abstains (there is no CVE) — applying it to code is what + shipped fake CRITICAL SQL-injection verdicts. """ from __future__ import annotations @@ -64,12 +82,43 @@ "indeterminate", "uncertain", ) +#: Hedge / speculation markers. The exposure analyzer flagging a sink "likely +#: reachable … needs verification to confirm" has NOT confirmed reachability — +#: it has guessed. Treat such phrasing as ``unknown`` (→ needs_review), never a +#: confident reachable. ``likely`` lives here, NOT in affirmatives: "likely +#: reachable" is a probability, not a confirmation, and promoting it to a +#: confident ``real`` is exactly the false positive this guards against. +_REACH_HEDGE = ( + "likely", + "probabl", # probable / probably + "suggest", # suggests / suggesting + "appears", + "seems", + "may ", # "may be reachable" — trailing space avoids matching e.g. "mayhem" + "might", + "could", + "would ", # "would need to trace" + "potential", + "presum", # presumably + "assume", + "needs verification", + "need to verify", + "needs confirmation", + "to confirm", + "not confirmed", + "unverified", + "needs analysis", + "need to trace", + "needs further", + "requires verification", + "cannot confirm", + "unable to confirm", +) _REACH_AFFIRMATIVE = ( "reachable", "yes", "true", "confirmed", - "likely", "direct", "exploitable", ) @@ -79,13 +128,18 @@ def _classify_reachable(reachable: str | None) -> str: """Map the analyzer's free-text ``reachable`` to ``yes`` / ``no`` / ``unknown``. An empty value means the analyzer ran but couldn't determine reachability → ``unknown`` (the *missing exposure* case is handled by the - caller before this is reached).""" + caller before this is reached). + + Hedge-aware: speculation ("likely … needs verification to confirm") is + classified ``unknown``, BEFORE the affirmative keywords are tried, so a + sink the analyzer merely *guessed* is reachable never projects to a + confident ``real`` (the Plan-gate bright line).""" if not reachable: return "unknown" s = reachable.strip().lower() if any(k in s for k in _REACH_NEGATIVE) or s in ("no", "false", "none"): return "no" - if any(k in s for k in _REACH_UNCERTAIN): + if any(k in s for k in _REACH_UNCERTAIN) or any(k in s for k in _REACH_HEDGE): return "unknown" if any(k in s for k in _REACH_AFFIRMATIVE): return "yes" @@ -103,8 +157,14 @@ def _enricher_abstained(enrichment: dict[str, Any] | None) -> bool: def synthesize_triage( enrichment: dict[str, Any] | None, exposure: dict[str, Any] | None, + finding_type: str = "dependency", ) -> TriageOutput: - """Project a triage verdict from recorded enricher + exposure output.""" + """Project a triage verdict from recorded enricher + exposure output. + + ``finding_type`` is the scanner finding's class (``dependency`` / ``code`` / + ``posture``). A ``code`` finding defers to the Deep dive — see + ``_code_finding_deferral`` — because the quick read can't open its + ``file:line``; every other type uses the dependency-shaped projection.""" known_exploits = bool((enrichment or {}).get("known_exploits")) # Missing exposure entirely → never a confident clear (ADR-0051 §10). @@ -126,6 +186,13 @@ def synthesize_triage( ], ) + # Code/SAST findings: the quick read reasons from the file PATH, not the + # file's code, so its reachability is speculation — a confident verdict needs + # the flagged file:line opened (the Deep dive's job). Defer to needs_review, + # which auto-escalates. Never a confident `real`/clear from a code finding. + if finding_type == "code": + return _code_finding_deferral(exposure) + reachable_raw = exposure.get("reachable") reached = _classify_reachable(reachable_raw if isinstance(reachable_raw, str) else None) internet_facing = exposure.get("internet_facing") @@ -200,6 +267,42 @@ def synthesize_triage( ) +def _code_finding_deferral(exposure: dict[str, Any]) -> TriageOutput: + """Verdict for a ``code``/SAST finding: defer to the Deep dive. + + The quick read's reachability for a code finding is speculation — the + exposure analyzer reasons from the file path, never opening the flagged line + — so it can neither confirm a ``real`` nor responsibly clear a + ``false_positive``. It returns ``needs_review``, which auto-escalates to the + file-reading Deep dive (ADR-0052 / ``decide_escalation``); when no Deep dive + can run, the honest "needs your review" stands rather than a fabricated + verdict. The analyzer's hunch is preserved as context, never promoted. + """ + evidence = exposure.get("reachability_evidence") + reachable_raw = exposure.get("reachable") + hint = evidence or (reachable_raw if isinstance(reachable_raw, str) else None) + return TriageOutput( + verdict="needs_review", + confidence=_CONF_NEEDS_REVIEW, + exploitability={ + "exploitable": "unknown", + "reason": "Confirming whether untrusted input reaches this code requires " + "reading the flagged line — deferred to deep analysis (the quick read " + "reasons from the file path, not the code).", + }, + checks=[ + { + "eyebrow": "REACHABILITY", + "result": "Needs a code read", + "kind": "warn", + "detail": hint + or "The flagged line wasn't opened — escalated for a file-level " + "trace before any verdict.", + } + ], + ) + + def _build_reachability(reached: str, evidence: str | None) -> dict[str, Any] | None: if reached == "no": return {"reached": False, "path": [], "summary": evidence or "No path found."} diff --git a/backend/cliff/agents/triage_deep/runner.py b/backend/cliff/agents/triage_deep/runner.py index b76d5971..a32f8579 100644 --- a/backend/cliff/agents/triage_deep/runner.py +++ b/backend/cliff/agents/triage_deep/runner.py @@ -89,6 +89,13 @@ def _map_reach(reach: dict) -> TriageReachability: return TriageReachability(reached=reach.get("reached") == "yes", path=nodes) +def _has_grounded_path(reach: dict) -> bool: + """Whether a ``reached=yes`` trace is GROUNDED — cites at least one hop with a + real ``file``. An empty (or file-less) path is a speculative "yes", not + confirmed reachability, and must not project to a confident ``real``.""" + return any(n.get("file") for n in (reach.get("path") or [])) + + def _kill_corroborated(ro: dict, facts: dict, repo_knowledge: dict) -> bool: """Whether a rule_out kill is backed by a structural signal (ADR-0052). @@ -366,6 +373,30 @@ def prov(exit_stage: str) -> TriageProvenance: provenance=prov("trace_path"), ) + # A "yes" with no grounded path (no file:line hop) is a SPECULATIVE + # reachable, not a confirmed one — the trace prompt's "no hop, and no + # guard, without a file:line you actually read". Honor that in code: + # route to needs_review instead of proceeding to a confident `real`, so + # an ungrounded "yes" can't false-flag (the symmetric guard to the + # disproof challenge on the clearing side). Plan/challenge would have + # nothing concrete to work from anyway. + if not _has_grounded_path(reach): + return TriageOutput( + verdict="needs_review", + confidence=_CONF_UNKNOWN, + reachability=_map_reach(reach), + checks=[ + TriageCheck( + eyebrow="Reachability", + result="Claimed reachable without a traced path", + kind="warn", + detail="No file:line hop was recorded — routed to review " + "rather than confirmed as reachable.", + ) + ], + provenance=prov("trace_path"), + ) + # 4. Plan the exploit (strong) — reachable-but-no-exploit = hardening. plan = await self._stages.plan( _deps(clone_dir, finding, {**base, "facts": facts, "reachability": reach}), diff --git a/backend/cliff/agents/triage_runner.py b/backend/cliff/agents/triage_runner.py index 40e2b4d9..ae208e7e 100644 --- a/backend/cliff/agents/triage_runner.py +++ b/backend/cliff/agents/triage_runner.py @@ -115,8 +115,12 @@ async def run_triage( ai_env: dict[str, str] | None = None, model_full_id: str | None = None, ) -> TriageOutput | None: - """Run triage for *workspace*'s finding. Returns the verdict, or ``None`` - when a prerequisite run failed (the derivation surfaces a Retry CTA). + """Run triage for *workspace*'s finding. + + Scanner triage always lands a verdict: a failed prerequisite degrades to + ``needs_review`` rather than crashing (so the CLI never strands on a + poll-timeout). The report path may still return ``None`` if the report + triager produced no verdict. ``ai_env`` + ``model_full_id`` (the canonical AI state) enable the agentic Deep dive on escalation; without them triage stays the Quick read (ADR-0052). @@ -156,6 +160,7 @@ async def _run_scanner_triage( ai_env: dict[str, str] | None = None, model_full_id: str | None = None, ) -> TriageOutput | None: + prereq_failed = False for agent_type in ("finding_enricher", "exposure_analyzer"): result = await executor.execute( workspace.id, @@ -166,18 +171,34 @@ async def _run_scanner_triage( ) status = getattr(result, "status", None) if result is None or status in _TERMINAL_FAILURE_STATUSES: + # A failed prerequisite (e.g. the exposure agent timing out on a + # deploy-time SQL file) must NOT abort with no verdict — that strands + # the CLI on a poll-timeout (exit 1) and leaves the UI without a + # result. Degrade to a verdict synthesized from whatever DID run: a + # failed run leaves no structured_output, so the synthesizer's + # undetermined path yields needs_review. Triage always lands a + # verdict; never a silent clear, never a crash. logger.warning( - "Triage aborted: %s ended status=%s for workspace %s", + "Triage prerequisite %s ended status=%s for workspace %s — " + "degrading to needs_review", agent_type, status, workspace.id, ) - return None + prereq_failed = True + break latest = await list_latest_runs_by_workspace_ids(db, [workspace.id]) runs = latest.get(workspace.id, {}) enrichment = _structured_output(runs, "finding_enricher") exposure = _structured_output(runs, "exposure_analyzer") - quick = synthesize_triage(enrichment, exposure) + quick = synthesize_triage(enrichment, exposure, finding_type=finding.type) + + # Partial inputs from a failed prerequisite → land the degraded needs_review + # and stop. Never escalate a partial quick read to the Deep dive (the same + # provider trouble that just failed a prerequisite would likely stall it too). + if prereq_failed: + await _persist_synthesis(db, workspace.id, quick) + return quick # Escalate to the agentic Deep dive when warranted (ADR-0052). Best-effort: # any failure keeps the cheap Quick-read verdict — triage never breaks. diff --git a/backend/pyproject.toml b/backend/pyproject.toml index 0cd36959..30e5437b 100644 --- a/backend/pyproject.toml +++ b/backend/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "cliff" -version = "0.2.1" +version = "0.2.2" description = "Cliff - Self-hosted cybersecurity remediation copilot" requires-python = ">=3.11" license = "AGPL-3.0-only" diff --git a/backend/tests/agents/test_deep_dive_runner.py b/backend/tests/agents/test_deep_dive_runner.py index 9d3025c9..35f62373 100644 --- a/backend/tests/agents/test_deep_dive_runner.py +++ b/backend/tests/agents/test_deep_dive_runner.py @@ -234,11 +234,31 @@ async def test_full_real_verdict_when_challenge_holds(): assert out.provenance.model_tiers["challenge"] == "judge" -async def test_challenge_downgrade_lowers_verdict(): +async def test_reached_yes_without_grounded_path_is_needs_review(): + """A trace that claims reached=yes but records NO file:line hop has not + CONFIRMED reachability — it's a speculative "yes" (the prompt's "no hop + without a file:line you actually read"). Route to needs_review rather than + proceed to a confident `real` (the Plan-gate bright line) — and stop before + plan/challenge, since there's no grounded path to plan against or challenge.""" stages = DeepDiveStages( gather=_stage({}), rule_out=_stage({"killed": False}), trace=_stage({"reached": "yes", "path": []}), + ) + out = await _run(stages) + assert out.verdict == "needs_review" + assert out.provenance.exit_stage == "trace_path" + assert "plan_exploit" not in out.provenance.steps_run + assert "challenge" not in out.provenance.steps_run + + +async def test_challenge_downgrade_lowers_verdict(): + stages = DeepDiveStages( + gather=_stage({}), + rule_out=_stage({"killed": False}), + # Grounded path (a real file:line hop) so this exercises the challenge + # downgrade, not the ungrounded-reachability guard above. + trace=_stage({"reached": "yes", "path": [{"file": "a.py", "line": 9, "role": "sink"}]}), plan=_stage({"hypotheses": [], "no_credible_exploit": False}), challenge=_challenge( Challenge( diff --git a/backend/tests/agents/test_triage_runner.py b/backend/tests/agents/test_triage_runner.py index 1f3e54ea..ffbb8af6 100644 --- a/backend/tests/agents/test_triage_runner.py +++ b/backend/tests/agents/test_triage_runner.py @@ -57,13 +57,16 @@ async def execute(self, workspace_id, agent_type, db, **_kwargs): # noqa: ANN00 ) -async def _make_workspace(db, *, source_type: str = "trivy") -> object: +async def _make_workspace( + db, *, source_type: str = "trivy", finding_type: str = "dependency" +) -> object: finding = await create_finding( db, FindingCreate( source_type=source_type, source_id="vuln-001", title="CVE-2026-0001 in libfoo", + type=finding_type, ), ) ws = await create_workspace(db, WorkspaceCreate(finding_id=finding.id)) @@ -154,14 +157,31 @@ async def test_triage_does_not_advance_finding_status(db) -> None: assert refreshed.status == "new" -async def test_failed_exposure_aborts_without_clearing(db) -> None: - """A failed exposure run produces no verdict (the derivation surfaces a - Retry affordance) — never a silent clear.""" +async def test_failed_exposure_degrades_to_needs_review(db) -> None: + """A failed prerequisite (e.g. the exposure agent timing out) must NOT abort + triage with no verdict — that strands the CLI on a poll-timeout (exit 1) and + leaves a dead UI. It degrades to a `needs_review` verdict, landed in both the + sidebar and the chat card. Never a silent clear; never a crash.""" _finding, ws = await _make_workspace(db) executor = _StubExecutor( {"finding_enricher": _ENRICH_REAL}, fail={"exposure_analyzer"} ) triage = await run_triage(executor, db, ws, env_vars={}) - assert triage is None + assert triage is not None + assert triage.verdict == "needs_review" # never a clear, never None sidebar = await get_sidebar(db, ws.id) - assert sidebar is None or sidebar.triage is None + assert sidebar is not None and sidebar.triage is not None + assert sidebar.triage["verdict"] == "needs_review" + + +async def test_code_finding_defers_to_needs_review(db) -> None: + """A code/SAST finding the exposure analyzer flagged as reachable+facing must + NOT ship as a confident `real` from the quick read — `run_triage` threads + `finding.type` so the synthesizer defers it to needs_review (→ Deep dive).""" + _finding, ws = await _make_workspace(db, source_type="semgrep", finding_type="code") + executor = _StubExecutor( + {"finding_enricher": _ENRICH_REAL, "exposure_analyzer": _EXPOSURE_REAL} + ) + triage = await run_triage(executor, db, ws, env_vars={}) + assert triage is not None + assert triage.verdict == "needs_review" # deferred, not the dependency `real` diff --git a/backend/tests/agents/test_triage_synthesizer.py b/backend/tests/agents/test_triage_synthesizer.py index 3bcf6a6c..818c7ab9 100644 --- a/backend/tests/agents/test_triage_synthesizer.py +++ b/backend/tests/agents/test_triage_synthesizer.py @@ -187,3 +187,92 @@ def test_output_always_has_proof_checks() -> None: out = synthesize_triage(enrichment, exposure) assert out.checks, f"{_label}: synthesis must emit at least one proof row" assert all(c.kind in {"pass", "warn", "fail", "info"} for c in out.checks) + + +# --------------------------------------------------------------------------- +# Speculative reachability must never become a confident verdict (the Plan-gate +# bright line). The exposure analyzer reasons about code findings from the file +# PATH, not the file's code — so a hedged "likely reachable … needs verification" +# is speculation, not a confirmed reachable, and must not project to `real`. +# --------------------------------------------------------------------------- + +# The real-world false positive: SAST SQLi the analyzer never read, hedged. +_EXPOSURE_HEDGED_REACHABLE = { + "reachable": "Likely reachable - this is likely part of the search/query " + "functionality, but the exact call chain needs verification to confirm " + "user-controlled data reaches this code path.", + "internet_facing": True, + "reachability_evidence": "The file path suggests dynamic query construction; " + "need to trace whether user input flows into the text() parameters.", +} + + +def test_hedged_reachability_is_not_promoted_to_real() -> None: + """A dependency finding whose reachability is only *speculated* ("likely … + needs verification to confirm") must not be classified as a confident + reachable — it routes to needs_review, never a confident `real`.""" + out = synthesize_triage(_WITH_CVE, _EXPOSURE_HEDGED_REACHABLE) + assert out.verdict == "needs_review" + assert out.verdict != "real" + + +@pytest.mark.parametrize( + "reachable", + [ + "Likely reachable - needs verification to confirm", + "The file path suggests this may be reachable", + "This appears reachable but the call chain needs analysis", + "Could be reachable; would need to trace the entry point", + ], + ids=["likely-needs-verify", "suggests-may", "appears-needs-analysis", "could-would"], +) +def test_speculative_phrasings_classify_as_uncertain(reachable: str) -> None: + """Hedge language never yields a confident `real`, regardless of facing.""" + out = synthesize_triage(_WITH_CVE, {"reachable": reachable, "internet_facing": True}) + assert out.verdict != "real" + + +# --------------------------------------------------------------------------- +# Code/SAST findings: the quick read can't open the flagged file:line, so it +# DEFERS (needs_review → auto-escalates to the file-reading Deep dive). It never +# emits a confident verdict for a code finding from path-level speculation — +# neither a `real` nor a `false_positive`/`unexploitable` clear. +# --------------------------------------------------------------------------- + + +def test_code_finding_speculative_reachable_defers_not_real() -> None: + """The recipe.py false positive: a code finding the analyzer flagged as + reachable+internet-facing must NOT ship as a confident `real` — it defers.""" + out = synthesize_triage( + _WITH_CVE, + {"reachable": "Reachable from the public search API", "internet_facing": True}, + finding_type="code", + ) + assert out.verdict == "needs_review" + assert out.exploitability is not None + assert out.exploitability.exploitable == "unknown" + + +def test_code_finding_abstained_unknown_defers_not_false_positive() -> None: + """A code finding with no advisory (always true for SAST) + undetermined + reachability must NOT be cleared as `false_positive` from the quick read — + clearing requires a file read (the Deep dive). It defers to needs_review.""" + out = synthesize_triage( + _ABSTAINED, + {"reachable": "unclear", "internet_facing": None}, + finding_type="code", + ) + assert out.verdict == "needs_review" + + +def test_dependency_finding_still_clears_and_confirms() -> None: + """The code-finding deferral must not regress dependency triage: a dependency + finding still projects to its terminal verdicts (default type='dependency').""" + real = synthesize_triage( + _WITH_CVE, + {"reachable": "Reachable from the public upload API", "internet_facing": True}, + finding_type="dependency", + ) + assert real.verdict == "real" + fp = synthesize_triage(_ABSTAINED, {"reachable": "unclear", "internet_facing": None}) + assert fp.verdict == "false_positive" # default finding_type is 'dependency' diff --git a/backend/uv.lock b/backend/uv.lock index 959f2307..54bdfb39 100644 --- a/backend/uv.lock +++ b/backend/uv.lock @@ -502,7 +502,7 @@ wheels = [ [[package]] name = "cliff" -version = "0.2.1" +version = "0.2.2" source = { editable = "." } dependencies = [ { name = "aiosqlite" }, diff --git a/cli/cliff_cli/__init__.py b/cli/cliff_cli/__init__.py index 908db406..c3316415 100644 --- a/cli/cliff_cli/__init__.py +++ b/cli/cliff_cli/__init__.py @@ -5,4 +5,4 @@ state rather than just success/failure. """ -__version__ = "0.2.1" +__version__ = "0.2.2" diff --git a/cli/pyproject.toml b/cli/pyproject.toml index 46e72e2f..824bc018 100644 --- a/cli/pyproject.toml +++ b/cli/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "cliffsec" -version = "0.2.1" +version = "0.2.2" description = "Agent-friendly CLI for Cliff — drive the full remediation loop from Claude Code or any agent." requires-python = ">=3.11" license = "AGPL-3.0-only" diff --git a/cli/uv.lock b/cli/uv.lock index 2a9ec8df..74b237e2 100644 --- a/cli/uv.lock +++ b/cli/uv.lock @@ -38,7 +38,7 @@ wheels = [ [[package]] name = "cliffsec" -version = "0.2.1" +version = "0.2.2" source = { editable = "." } dependencies = [ { name = "click" }, From 48ed13b937e27f6b1eef44e89480d1e108be2c18 Mon Sep 17 00:00:00 2001 From: Gal Ankonina Date: Mon, 15 Jun 2026 18:17:18 +0300 Subject: [PATCH 2/2] =?UTF-8?q?fix(triage):=20address=20review=20=E2=80=94?= =?UTF-8?q?=20hedged-negative=20false-clear,=20secret=20deferral,=20prereq?= =?UTF-8?q?=20robustness?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Code-review pass (CodeRabbit + a multi-angle finder sweep) surfaced four more defects in the same false-verdict class; all fixed with tests: - **Hedged negatives were confidently cleared.** `_classify_reachable` checked NEGATIVE before HEDGE, so "cannot confirm it is not reachable" / "appears unreachable but unverified" matched "not reachable" → `unexploitable` 0.88 — a false-clear of a possibly-real finding. Hedging/uncertainty is now checked FIRST and dominates any negative/affirmative keyword; a clean "no path found" still clears. (Combined `_REACH_UNCERTAIN`+`_REACH_HEDGE` into one pass.) - **`secret` findings fell through to the dependency projection.** A leaked secret has no CVE → the enricher abstains → the projection cleared it as `false_positive`, false-clearing a real secret. The deferral now covers `code` AND `secret` (no-advisory-model types); `dependency`/`posture` keep the projection. `_code_finding_deferral` → `_deferred_quick_verdict`, type-aware. - **Deep-dive grounded-path guard required `file` only.** Now requires a real `file:line` (int line > 0), matching the trace prompt's "no hop without a file:line you actually read" — a `{file, line: null}` hop no longer counts. - **Prereq-failure degrade hardened.** Degrade on ANY non-`completed` status (not just failed/rate_limited — e.g. awaiting_permission), and synthesize the degraded verdict from THIS run's inputs only (drop a prior attempt's stale enricher/exposure output) so a degraded verdict can't be a confident projection from stale data. Full agents suite: 249 passed. ADR-0051 amendment + CHANGELOG updated. Co-Authored-By: Claude Opus 4.8 (1M context) --- CHANGELOG.md | 21 +++- .../agents/runtime/triage_synthesizer.py | 116 +++++++++++------- backend/cliff/agents/triage_deep/runner.py | 13 +- backend/cliff/agents/triage_runner.py | 23 +++- backend/tests/agents/test_deep_dive_runner.py | 14 +++ backend/tests/agents/test_triage_runner.py | 64 +++++++++- .../tests/agents/test_triage_synthesizer.py | 48 ++++++++ 7 files changed, 242 insertions(+), 57 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 883c2468..6497707b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,15 +24,28 @@ carries the triage-first `fix` flow to users. On a SAST-heavy repo that shipped fake CRITICAL SQL-injection findings. Two fail-safes now guard the verdict: (1) reachability classification is hedge-aware — "likely / suggests / appears / needs verification" is treated - as *undetermined*, never a confident reachable; (2) a `code` finding can't - be confidently cleared OR confirmed from the Quick read at all — it defers - to `needs_review`, which auto-escalates to the file-reading Deep dive - (ADR-0052), where the `file:line` is actually opened before any verdict. + as *undetermined*, never a confident reachable; (2) a `code` (and `secret`) + finding can't be confidently cleared OR confirmed from the Quick read at all + — it defers to `needs_review`, which auto-escalates to the file-reading Deep + dive (ADR-0052), where the `file:line` is actually opened before any verdict. +- **Hedged negatives are no longer confidently cleared.** Reachability + classification now checks hedging *before* the negative keywords, so + "cannot confirm it is not reachable" / "appears unreachable but unverified" + route to `needs_review` instead of a confident `unexploitable` — closing the + same false-clear on the clearing side. A clean "no path found" still clears. +- **`secret` findings defer like code.** A leaked-secret finding has no CVE + (the enricher always abstains), so the dependency projection would clear it + as `false_positive` — false-clearing a real secret. Secrets now defer to + `needs_review`. (`posture` keeps the projection — it's a deterministic Cliff + check, real by definition.) - **Triage never strands on a failed prerequisite.** When the enricher or exposure agent failed (e.g. a 150s timeout on a deploy-time migration file), triage aborted with no verdict — leaving the CLI to time out and exit 1. It now degrades to a `needs_review` verdict landed in the sidebar and chat card. Triage always produces a verdict; never a silent clear, never a crash. + The degrade triggers on *any* non-`completed` prerequisite status (not just + `failed`/`rate_limited`), and the degraded verdict is computed without stale + output from a prior triage attempt in the same workspace. - **Deep dive won't confirm an ungrounded path.** A `trace_path` result that claims `reached=yes` but cites no `file:line` hop is now routed to `needs_review` instead of proceeding to a confident `real` — the symmetric diff --git a/backend/cliff/agents/runtime/triage_synthesizer.py b/backend/cliff/agents/runtime/triage_synthesizer.py index f354d188..9d6b2cce 100644 --- a/backend/cliff/agents/runtime/triage_synthesizer.py +++ b/backend/cliff/agents/runtime/triage_synthesizer.py @@ -30,15 +30,18 @@ needs verification to confirm" is *speculation*, not a confirmed reachable, so it maps to ``unknown`` (→ needs_review), never a confident ``real``. -2. **Code/SAST findings defer.** For a ``code`` finding the exposure analyzer - reasons from the file PATH, never the file's code — so the quick read can't - responsibly clear OR confirm it. It defers to ``needs_review``, which - auto-escalates to the file-reading Deep dive (ADR-0052), where the flagged - ``file:line`` is actually opened before any verdict. The dependency-shaped - projection above (abstention → false_positive, reachable+facing → real) - assumes a CVE/advisory model that does not hold for code findings, where the - enricher always abstains (there is no CVE) — applying it to code is what - shipped fake CRITICAL SQL-injection verdicts. +2. **No-advisory-model findings defer.** ``code`` and ``secret`` findings have + no CVE/advisory (the enricher always abstains), and for code the exposure + analyzer reasons from the file PATH, never the file's code — so the quick + read can't responsibly clear OR confirm them. They defer to ``needs_review``, + which auto-escalates to the file-reading Deep dive (ADR-0052), where the + flagged ``file:line`` is opened before any verdict. The dependency-shaped + projection below (abstention → false_positive, reachable+facing → real) + assumes a CVE/advisory model: applying it to code shipped fake CRITICAL + SQL-injection ``real`` verdicts, and to a secret it would false-clear a real + leaked credential as ``false_positive``. ``dependency`` (CVE model) and + ``posture`` (deterministic Cliff check, real by definition) keep the + projection. """ from __future__ import annotations @@ -52,6 +55,12 @@ #: Tuned against the eval harness — see backend/tests/agents/eval. NEEDS_REVIEW_CONFIDENCE = 0.70 +#: Scanner finding types the quick read must NOT confidently judge — first-party +#: findings with no CVE/advisory model, where the dependency projection's +#: abstention + reachability semantics don't hold. They defer to the Deep dive. +#: (`dependency` keeps the projection; `posture` is a deterministic Cliff check.) +_DEFER_TYPES = frozenset({"code", "secret"}) + # Confidence anchors per verdict (all confident verdicts sit at/above the # threshold; every needs_review sits below it — see the §9 invariant test). _CONF_REAL = 0.82 @@ -122,6 +131,9 @@ "direct", "exploitable", ) +#: Anything uncertain OR hedged is undetermined. One set, one membership pass — +#: the two source tuples stay separate only for their distinct doc-comments. +_REACH_UNKNOWN = _REACH_UNCERTAIN + _REACH_HEDGE def _classify_reachable(reachable: str | None) -> str: @@ -130,17 +142,20 @@ def _classify_reachable(reachable: str | None) -> str: reachability → ``unknown`` (the *missing exposure* case is handled by the caller before this is reached). - Hedge-aware: speculation ("likely … needs verification to confirm") is - classified ``unknown``, BEFORE the affirmative keywords are tried, so a - sink the analyzer merely *guessed* is reachable never projects to a - confident ``real`` (the Plan-gate bright line).""" + Uncertainty/hedging is checked FIRST — it dominates any negative or + affirmative keyword the same text also contains. So a hedged NEGATIVE + ("cannot confirm it is not reachable", "appears unreachable but unverified") + is NOT confidently cleared as a no-path, and a hedged AFFIRMATIVE ("likely + reachable … needs verification") is NOT confirmed as reachable. Only a clean, + un-hedged statement reaches the negative/affirmative checks (the Plan-gate + bright line: speculation never becomes a confident verdict).""" if not reachable: return "unknown" s = reachable.strip().lower() + if any(k in s for k in _REACH_UNKNOWN): + return "unknown" if any(k in s for k in _REACH_NEGATIVE) or s in ("no", "false", "none"): return "no" - if any(k in s for k in _REACH_UNCERTAIN) or any(k in s for k in _REACH_HEDGE): - return "unknown" if any(k in s for k in _REACH_AFFIRMATIVE): return "yes" return "unknown" @@ -162,9 +177,10 @@ def synthesize_triage( """Project a triage verdict from recorded enricher + exposure output. ``finding_type`` is the scanner finding's class (``dependency`` / ``code`` / - ``posture``). A ``code`` finding defers to the Deep dive — see - ``_code_finding_deferral`` — because the quick read can't open its - ``file:line``; every other type uses the dependency-shaped projection.""" + ``secret`` / ``posture``). ``code`` and ``secret`` findings defer to the Deep + dive — see ``_deferred_quick_verdict`` — because they have no advisory model + the projection can rest on; ``dependency`` and ``posture`` use the + dependency-shaped projection below.""" known_exploits = bool((enrichment or {}).get("known_exploits")) # Missing exposure entirely → never a confident clear (ADR-0051 §10). @@ -186,12 +202,15 @@ def synthesize_triage( ], ) - # Code/SAST findings: the quick read reasons from the file PATH, not the - # file's code, so its reachability is speculation — a confident verdict needs - # the flagged file:line opened (the Deep dive's job). Defer to needs_review, - # which auto-escalates. Never a confident `real`/clear from a code finding. - if finding_type == "code": - return _code_finding_deferral(exposure) + # First-party findings with no advisory model (`code`, `secret`) can't be + # confidently cleared OR confirmed by the dependency-shaped projection below: + # the enricher always abstains (no CVE), and the quick read reasons from the + # file path, not the file's code. Defer to needs_review (→ auto-escalates to + # the file-reading Deep dive / a human). Never a confident verdict from + # speculation. `dependency` (CVE model) and `posture` (deterministic Cliff + # check) keep the projection. + if finding_type in _DEFER_TYPES: + return _deferred_quick_verdict(finding_type, exposure) reachable_raw = exposure.get("reachable") reached = _classify_reachable(reachable_raw if isinstance(reachable_raw, str) else None) @@ -267,37 +286,50 @@ def synthesize_triage( ) -def _code_finding_deferral(exposure: dict[str, Any]) -> TriageOutput: - """Verdict for a ``code``/SAST finding: defer to the Deep dive. +def _deferred_quick_verdict(finding_type: str, exposure: dict[str, Any]) -> TriageOutput: + """Verdict for a finding the quick read can't responsibly judge (``code`` / + ``secret``): defer to the Deep dive. - The quick read's reachability for a code finding is speculation — the - exposure analyzer reasons from the file path, never opening the flagged line - — so it can neither confirm a ``real`` nor responsibly clear a - ``false_positive``. It returns ``needs_review``, which auto-escalates to the - file-reading Deep dive (ADR-0052 / ``decide_escalation``); when no Deep dive - can run, the honest "needs your review" stands rather than a fabricated + These are first-party findings with no advisory model, so the + dependency-shaped projection (abstention → false_positive, reachable+facing + → real) doesn't fit — applying it false-clears real secrets and fake-confirms + speculative code reachability. Return ``needs_review``, which auto-escalates + to the file-reading Deep dive (ADR-0052 / ``decide_escalation``); when no Deep + dive can run, the honest "needs your review" stands rather than a fabricated verdict. The analyzer's hunch is preserved as context, never promoted. """ + if finding_type == "secret": + reason = ( + "A flagged secret needs review to confirm it's a true exposure — not a " + "placeholder, test fixture, or already-rotated value. The quick read " + "can't clear it." + ) + check_result = "Needs your review" + detail_default = "A potential secret was flagged — escalated for review." + else: # code / SAST + reason = ( + "Confirming whether untrusted input reaches this code requires reading " + "the flagged line — deferred to deep analysis (the quick read reasons " + "from the file path, not the code)." + ) + check_result = "Needs a code read" + detail_default = ( + "The flagged line wasn't opened — escalated for a file-level trace " + "before any verdict." + ) evidence = exposure.get("reachability_evidence") reachable_raw = exposure.get("reachable") hint = evidence or (reachable_raw if isinstance(reachable_raw, str) else None) return TriageOutput( verdict="needs_review", confidence=_CONF_NEEDS_REVIEW, - exploitability={ - "exploitable": "unknown", - "reason": "Confirming whether untrusted input reaches this code requires " - "reading the flagged line — deferred to deep analysis (the quick read " - "reasons from the file path, not the code).", - }, + exploitability={"exploitable": "unknown", "reason": reason}, checks=[ { "eyebrow": "REACHABILITY", - "result": "Needs a code read", + "result": check_result, "kind": "warn", - "detail": hint - or "The flagged line wasn't opened — escalated for a file-level " - "trace before any verdict.", + "detail": hint or detail_default, } ], ) diff --git a/backend/cliff/agents/triage_deep/runner.py b/backend/cliff/agents/triage_deep/runner.py index a32f8579..79e9c576 100644 --- a/backend/cliff/agents/triage_deep/runner.py +++ b/backend/cliff/agents/triage_deep/runner.py @@ -91,9 +91,16 @@ def _map_reach(reach: dict) -> TriageReachability: def _has_grounded_path(reach: dict) -> bool: """Whether a ``reached=yes`` trace is GROUNDED — cites at least one hop with a - real ``file``. An empty (or file-less) path is a speculative "yes", not - confirmed reachability, and must not project to a confident ``real``.""" - return any(n.get("file") for n in (reach.get("path") or [])) + real ``file:line`` (the trace prompt's "no hop … without a file:line you + actually read"). An empty path, or hops with no concrete ``file:line``, is a + speculative "yes", not confirmed reachability, and must not project to a + confident ``real``.""" + for hop in reach.get("path") or []: + file = hop.get("file") + line = hop.get("line") + if isinstance(file, str) and file.strip() and isinstance(line, int) and line > 0: + return True + return False def _kill_corroborated(ro: dict, facts: dict, repo_knowledge: dict) -> bool: diff --git a/backend/cliff/agents/triage_runner.py b/backend/cliff/agents/triage_runner.py index ae208e7e..bc24842c 100644 --- a/backend/cliff/agents/triage_runner.py +++ b/backend/cliff/agents/triage_runner.py @@ -45,8 +45,6 @@ #: real agent run — the chat card + sidebar are written here directly. SYNTHESIZER_AGENT_TYPE = "triage_synthesizer" -_TERMINAL_FAILURE_STATUSES = ("failed", "rate_limited") - _VERDICT_LABEL = { "real": "Real risk", "unexploitable": "Not exploitable", @@ -161,6 +159,7 @@ async def _run_scanner_triage( model_full_id: str | None = None, ) -> TriageOutput | None: prereq_failed = False + failed_agent: str | None = None for agent_type in ("finding_enricher", "exposure_analyzer"): result = await executor.execute( workspace.id, @@ -170,7 +169,10 @@ async def _run_scanner_triage( env_vars=env_vars, ) status = getattr(result, "status", None) - if result is None or status in _TERMINAL_FAILURE_STATUSES: + # Proceed ONLY on a completed prerequisite. Any other result status — + # failed, rate_limited, awaiting_permission, or a future addition — is a + # non-completion and must degrade, not be mistaken for success. + if result is None or status != "completed": # A failed prerequisite (e.g. the exposure agent timing out on a # deploy-time SQL file) must NOT abort with no verdict — that strands # the CLI on a poll-timeout (exit 1) and leaves the UI without a @@ -184,6 +186,7 @@ async def _run_scanner_triage( agent_type, status, workspace.id, ) prereq_failed = True + failed_agent = agent_type break latest = await list_latest_runs_by_workspace_ids(db, [workspace.id]) @@ -191,6 +194,18 @@ async def _run_scanner_triage( enrichment = _structured_output(runs, "finding_enricher") exposure = _structured_output(runs, "exposure_analyzer") + if prereq_failed: + # Don't synthesize from a PRIOR attempt's stale output: the failed agent + # produced no fresh output this run, and an enricher failure breaks the + # loop before exposure runs at all — so ``list_latest_runs`` could hand + # back a confident exposure from an earlier triage and project a false + # ``real``. Drop the failed agent's input (and everything downstream of + # it) so the synthesizer lands on its missing-input needs_review. + if failed_agent == "finding_enricher": + enrichment = exposure = None + else: # exposure_analyzer + exposure = None + quick = synthesize_triage(enrichment, exposure, finding_type=finding.type) # Partial inputs from a failed prerequisite → land the degraded needs_review @@ -251,7 +266,7 @@ async def _run_report_triage( env_vars=env_vars, ) status = getattr(result, "status", None) - if result is None or status in _TERMINAL_FAILURE_STATUSES: + if result is None or status != "completed": # proceed only on completion logger.warning( "Report triage aborted: status=%s for workspace %s", status, workspace.id ) diff --git a/backend/tests/agents/test_deep_dive_runner.py b/backend/tests/agents/test_deep_dive_runner.py index 35f62373..68586e9c 100644 --- a/backend/tests/agents/test_deep_dive_runner.py +++ b/backend/tests/agents/test_deep_dive_runner.py @@ -252,6 +252,20 @@ async def test_reached_yes_without_grounded_path_is_needs_review(): assert "challenge" not in out.provenance.steps_run +async def test_reached_yes_with_file_but_no_line_is_needs_review(): + """A hop citing a `file` but no `line` is not a grounded `file:line` — the + trace prompt demands the line you actually read. Still speculative → route to + needs_review, never a confident `real`.""" + stages = DeepDiveStages( + gather=_stage({}), + rule_out=_stage({"killed": False}), + trace=_stage({"reached": "yes", "path": [{"file": "a.py", "line": None, "role": "sink"}]}), + ) + out = await _run(stages) + assert out.verdict == "needs_review" + assert out.provenance.exit_stage == "trace_path" + + async def test_challenge_downgrade_lowers_verdict(): stages = DeepDiveStages( gather=_stage({}), diff --git a/backend/tests/agents/test_triage_runner.py b/backend/tests/agents/test_triage_runner.py index ffbb8af6..78b10f64 100644 --- a/backend/tests/agents/test_triage_runner.py +++ b/backend/tests/agents/test_triage_runner.py @@ -13,6 +13,7 @@ from __future__ import annotations from datetime import UTC, datetime +from types import SimpleNamespace import pytest @@ -33,9 +34,16 @@ class _StubExecutor: """Simulates ``AgentExecutor.execute`` by writing a completed agent_run row with canned structured output for each agent type.""" - def __init__(self, outputs: dict[str, dict], *, fail: set[str] | None = None): + def __init__( + self, + outputs: dict[str, dict], + *, + fail: set[str] | None = None, + statuses: dict[str, str] | None = None, + ): self.outputs = outputs self.fail = fail or set() + self.statuses = statuses or {} self.calls: list[str] = [] async def check_not_busy(self, db, workspace_id): # noqa: ANN001 @@ -43,18 +51,29 @@ async def check_not_busy(self, db, workspace_id): # noqa: ANN001 async def execute(self, workspace_id, agent_type, db, **_kwargs): # noqa: ANN001 self.calls.append(agent_type) - status = "failed" if agent_type in self.fail else "completed" + if agent_type in self.statuses: + result_status = self.statuses[agent_type] + elif agent_type in self.fail: + result_status = "failed" + else: + result_status = "completed" + # The DB run status enum is narrower than the AgentExecutionResult status + # enum (`awaiting_permission` is result-only), so persist a valid DB state + # and surface the requested status on the returned result. + db_valid = ("completed", "failed", "rate_limited") + db_status = result_status if result_status in db_valid else "running" run = await create_agent_run( db, workspace_id, AgentRunCreate(agent_type=agent_type, status="running") ) - return await update_agent_run( + await update_agent_run( db, run.id, AgentRunUpdate( - status=status, + status=db_status, structured_output=self.outputs.get(agent_type, {}), ), ) + return SimpleNamespace(status=result_status) async def _make_workspace( @@ -174,6 +193,43 @@ async def test_failed_exposure_degrades_to_needs_review(db) -> None: assert sidebar.triage["verdict"] == "needs_review" +async def test_prereq_failure_ignores_stale_prior_runs(db) -> None: + """A failed prerequisite must degrade to needs_review even when a PRIOR + successful exposure run (confident, real-looking) lingers in the workspace. + `list_latest_runs` would otherwise hand that stale output to the synthesizer + and project a confident `real` on a failed triage — never synthesize a + verdict from stale data.""" + _finding, ws = await _make_workspace(db) + # Seed a stale, confident-looking exposure run from a prior attempt. + prior = await create_agent_run( + db, ws.id, AgentRunCreate(agent_type="exposure_analyzer", status="running") + ) + await update_agent_run( + db, prior.id, AgentRunUpdate(status="completed", structured_output=_EXPOSURE_REAL) + ) + # This attempt's enricher fails → the loop breaks before a fresh exposure run, + # leaving the stale one as the latest. + executor = _StubExecutor({}, fail={"finding_enricher"}) + triage = await run_triage(executor, db, ws, env_vars={}) + assert triage is not None + assert triage.verdict == "needs_review" # NOT `real` from the stale exposure + + +async def test_non_completed_prereq_status_degrades(db) -> None: + """Only a `completed` prerequisite may proceed. A non-completed, non-failed + status (e.g. `awaiting_permission`) must also degrade — even if that run + recorded a confident, real-looking output that ``list_latest_runs`` (which + doesn't filter by status) would otherwise hand to the synthesizer.""" + _finding, ws = await _make_workspace(db) + executor = _StubExecutor( + {"finding_enricher": _ENRICH_REAL, "exposure_analyzer": _EXPOSURE_REAL}, + statuses={"exposure_analyzer": "awaiting_permission"}, + ) + triage = await run_triage(executor, db, ws, env_vars={}) + assert triage is not None + assert triage.verdict == "needs_review" # NOT `real` from the un-completed run + + async def test_code_finding_defers_to_needs_review(db) -> None: """A code/SAST finding the exposure analyzer flagged as reachable+facing must NOT ship as a confident `real` from the quick read — `run_triage` threads diff --git a/backend/tests/agents/test_triage_synthesizer.py b/backend/tests/agents/test_triage_synthesizer.py index 818c7ab9..5680ff84 100644 --- a/backend/tests/agents/test_triage_synthesizer.py +++ b/backend/tests/agents/test_triage_synthesizer.py @@ -240,6 +240,54 @@ def test_speculative_phrasings_classify_as_uncertain(reachable: str) -> None: # --------------------------------------------------------------------------- +@pytest.mark.parametrize( + "reachable", + [ + "cannot confirm it is not reachable", + "probably unreachable but needs verification", + "appears to have no path, though unverified", + "likely not reachable, but the call chain is unclear", + ], + ids=["cannot-confirm-not", "probably-unreachable", "appears-nopath", "likely-not"], +) +def test_hedged_negative_is_not_confidently_cleared(reachable: str) -> None: + """A HEDGED negative ("cannot confirm it is not reachable", "appears + unreachable but unverified") is uncertainty, not a clear no-path — it must + NOT be confidently cleared as `unexploitable`. Hedging dominates a negative + substring, so the verdict routes to needs_review.""" + out = synthesize_triage(_WITH_CVE, {"reachable": reachable, "internet_facing": True}) + assert out.verdict != "unexploitable" + assert out.verdict == "needs_review" + + +def test_clean_no_path_still_clears_unexploitable() -> None: + """The fix must not over-trigger: a confident, un-hedged no-path still clears.""" + out = synthesize_triage( + _WITH_CVE, {"reachable": "No path found from any entrypoint", "internet_facing": False} + ) + assert out.verdict == "unexploitable" + + +def test_secret_finding_abstained_defers_not_false_positive() -> None: + """A `secret` finding has no advisory (the enricher always abstains), so the + dependency projection would clear it as `false_positive` — false-clearing a + real leaked secret. Like code, a secret defers to needs_review (the same + no-advisory-model class).""" + out = synthesize_triage( + _ABSTAINED, {"reachable": "unclear", "internet_facing": None}, finding_type="secret" + ) + assert out.verdict == "needs_review" + + +def test_secret_finding_speculative_reachable_defers_not_real() -> None: + out = synthesize_triage( + _WITH_CVE, + {"reachable": "Reachable from the public API", "internet_facing": True}, + finding_type="secret", + ) + assert out.verdict == "needs_review" + + def test_code_finding_speculative_reachable_defers_not_real() -> None: """The recipe.py false positive: a code finding the analyzer flagged as reachable+internet-facing must NOT ship as a confident `real` — it defers."""