From 2b413da544809d9ef264c827c30a39bba7c0ff7c Mon Sep 17 00:00:00 2001 From: yoni Date: Fri, 25 Sep 2026 07:06:21 +0000 Subject: [PATCH] Keep prepared candidates and staleness consistent across revisions A report revision whose locations can no longer form a candidate now still supersedes any recorded preparation instead of leaving a ready result pointing at superseded locations. Preparation results carry the candidate that was actually verified so SARIF emits the anchor-corrected edits rather than comparing the stored draft's digest against a re-anchored one. --- strix/fix/contracts.py | 1 + strix/fix/prepare.py | 1 + strix/report/sarif.py | 16 +++++++++------- strix/tools/reporting/tool.py | 1 + tests/test_reporting_fields.py | 24 ++++++++++++++++++++++++ tests/test_sarif.py | 34 ++++++++++++++++++++++++++++++++++ 6 files changed, 70 insertions(+), 7 deletions(-) diff --git a/strix/fix/contracts.py b/strix/fix/contracts.py index 98a52a78a..5628d9993 100644 --- a/strix/fix/contracts.py +++ b/strix/fix/contracts.py @@ -196,6 +196,7 @@ class FixPreparationResultV1(ContractModel): state: PreparationState stop_reason: str source_identity: SourceIdentity | None + candidate: FixCandidateV1 candidate_digest: str final_file_manifest: list[FileManifestEntry] = [] artifact_ref: str | None = None diff --git a/strix/fix/prepare.py b/strix/fix/prepare.py index cdd6d51fc..9f80df0d2 100644 --- a/strix/fix/prepare.py +++ b/strix/fix/prepare.py @@ -353,6 +353,7 @@ def _result( state=state, stop_reason=reason, source_identity=context.candidate.source_identity, + candidate=context.candidate, candidate_digest=context.candidate.digest(), final_file_manifest=manifest or [], artifact_ref=artifact_ref, diff --git a/strix/report/sarif.py b/strix/report/sarif.py index fcf9c3f77..7bcad67c1 100644 --- a/strix/report/sarif.py +++ b/strix/report/sarif.py @@ -29,9 +29,9 @@ Design notes: like URIs, absolute paths, or traversal patterns are rejected rather than emitted as invalid code-scanning alerts. * Findings whose fix candidate completed verified preparation - (``fix_preparation.state == "ready"`` with a matching - ``candidate_digest``) are emitted as SARIF ``fixes`` so code-scanning - can render a one-click suggested change. + (``fix_preparation.state == "ready"`` with a ``candidate_digest`` + matching the candidate the result carries) are emitted as SARIF + ``fixes`` so code-scanning can render a one-click suggested change. * Endpoint / target-only findings (typical of DAST) carry a SARIF ``logicalLocations`` entry so the finding keeps a meaningful anchor even without a source file + line. @@ -591,14 +591,16 @@ def _build_fixes(report: dict[str, Any]) -> list[dict[str, Any]] | None: """Build SARIF ``fixes`` from a verified prepared finding. SARIF consumers can apply ``fixes`` automatically. Strix emits them only - when preparation recorded a ``ready`` result whose ``candidate_digest`` - still matches the stored fix candidate — so the emitted replacements are - exactly what preparation verified, never a stale or diverged draft. + when preparation recorded a ``ready`` result. The result carries the + candidate preparation verified — after anchoring corrected its reported + lines — and its ``candidate_digest`` must match that candidate, so the + emitted replacements are exactly what preparation verified, never a + stale or diverged draft. """ preparation = report.get("fix_preparation") if not isinstance(preparation, dict) or preparation.get("state") != "ready": return None - raw_candidate = report.get("fix_candidate") + raw_candidate = preparation.get("candidate") or report.get("fix_candidate") if not isinstance(raw_candidate, dict): return None try: diff --git a/strix/tools/reporting/tool.py b/strix/tools/reporting/tool.py index a9888364a..e5cdb4ad7 100644 --- a/strix/tools/reporting/tool.py +++ b/strix/tools/reporting/tool.py @@ -466,6 +466,7 @@ def _refresh_fix_candidate( candidate = _build_fix_candidate(report_state, {**existing, **changes}) if candidate is not None: changes["fix_candidate"] = candidate + if candidate is not None or existing.get("fix_candidate") or existing.get("fix_preparation"): changes["fix_preparation"] = { "state": "stale", "stop_reason": "The finding or draft candidate changed after preparation.", diff --git a/tests/test_reporting_fields.py b/tests/test_reporting_fields.py index 46d55af34..b07ee39fa 100644 --- a/tests/test_reporting_fields.py +++ b/tests/test_reporting_fields.py @@ -1981,6 +1981,30 @@ def test_update_refuses_code_locations_it_cannot_use(report_state: ReportState) assert any("start_line" in error for error in result["errors"]) +def test_update_marks_preparation_stale_when_candidate_cannot_be_rebuilt( + report_state: ReportState, +) -> None: + """A revision whose locations cannot form a candidate still supersedes the + preparation recorded for the draft it replaced.""" + _seed_weak_report(report_state) + report = report_state.vulnerability_reports[0] + candidate = {"security_invariant": "Parametrize the query.", "draft_edits": []} + report["fix_candidate"] = candidate + report["fix_preparation"] = {"state": "ready", "candidate_digest": "0" * 64} + + result = _do_update( + report_id="vuln-0009", + update_reason="Repointing the finding to the validated handler.", + fields={"code_locations": [{"file": "./files.py", "start_line": 4, "end_line": 9}]}, + ) + + assert result["success"] is True + assert report["code_locations"] == [{"file": "./files.py", "start_line": 4, "end_line": 9}] + assert report["fix_preparation"]["state"] == "stale" + assert "changed after preparation" in report["fix_preparation"]["stop_reason"] + assert report["fix_candidate"] == candidate + + def _seed_saved_report(report_state: ReportState) -> Path: """The weak report, written to disk the way a filed finding is.""" _seed_weak_report(report_state) diff --git a/tests/test_sarif.py b/tests/test_sarif.py index affe5034a..fae302c18 100644 --- a/tests/test_sarif.py +++ b/tests/test_sarif.py @@ -183,6 +183,40 @@ def test_write_sarif_builds_fixes_from_ready_preparation(tmp_path: Path) -> None assert replacement["insertedContent"]["text"] == 'query = "SELECT * FROM u WHERE id=%s"' +def test_write_sarif_emits_verified_candidate_from_preparation(tmp_path: Path) -> None: + # Preparation re-anchors the draft to the lines its block actually occupies, + # so the result carries the verified candidate and SARIF emits those lines + # rather than the stale positions the report's draft still records. + verified = _fix_candidate( + draft_edits=[ + { + "file": "app.py", + "start_line": 9, + "end_line": 9, + "before": 'query = "SELECT * FROM u WHERE id=" + uid', + "after": 'query = "SELECT * FROM u WHERE id=%s"', + } + ] + ) + write_sarif( + tmp_path, + [ + _finding( + fix_candidate=_fix_candidate(), + fix_preparation={ + "state": "ready", + "candidate": verified, + "candidate_digest": FixCandidateV1.model_validate(verified).digest(), + }, + ) + ], + ) + result = _read(tmp_path)["runs"][0]["results"][0] + replacement = result["fixes"][0]["artifactChanges"][0]["replacements"][0] + assert replacement["deletedRegion"]["startLine"] == 9 + assert replacement["deletedRegion"]["endLine"] == 9 + + def test_write_sarif_suppresses_fixes_when_prepared_candidate_diverged( tmp_path: Path, ) -> None: