mirror of
https://github.com/usestrix/strix.git
synced 2026-10-02 02:13:43 +00:00
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.
This commit is contained in:
parent
49eca20a1a
commit
2b413da544
6 changed files with 70 additions and 7 deletions
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
|
|
|
|||
|
|
@ -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:
|
||||
|
|
|
|||
|
|
@ -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.",
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
|
|
|||
|
|
@ -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:
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue