mirror of
https://github.com/usestrix/strix.git
synced 2026-10-12 03:47:56 +00:00
Delete: any agent may withdraw a report; rollback write failures surface as ReportRollbackError; viewer shows pending state
This commit is contained in:
parent
7baa127e93
commit
554067de7c
6 changed files with 98 additions and 88 deletions
|
|
@ -19,12 +19,15 @@ export default function VulnReportDeleteRenderer({ args, result, status }: ToolR
|
|||
const title = typeof res?.title === "string" ? res.title : "";
|
||||
const severity = typeof res?.severity === "string" ? res.severity.toLowerCase() : "";
|
||||
const failed = res != null ? !ok : status === "failed" || status === "error";
|
||||
const pending = res == null && !failed;
|
||||
const label = pending ? "withdrawing report\u2026" : failed ? "report not withdrawn" : "report withdrawn";
|
||||
const labelColor = pending ? "text-[#888]" : failed ? "text-red-400/80" : "text-yellow-400/80";
|
||||
|
||||
return (
|
||||
<div className="space-y-2">
|
||||
<div className="flex items-center gap-2 flex-wrap">
|
||||
<span className={`font-semibold text-sm ${failed ? "text-red-400/80" : "text-yellow-400/80"}`}>
|
||||
{failed ? "report not withdrawn" : "report withdrawn"}
|
||||
<span className={`font-semibold text-sm ${labelColor}`} aria-live="polite">
|
||||
{label}
|
||||
</span>
|
||||
{reportId && <span className="text-[#555] font-mono text-[13px]">{reportId}</span>}
|
||||
{severity && (
|
||||
|
|
|
|||
File diff suppressed because one or more lines are too long
|
|
@ -6,7 +6,7 @@
|
|||
<meta name="viewport" content="width=device-width, initial-scale=1.0" />
|
||||
<meta name="color-scheme" content="dark" />
|
||||
<title>Strix Results</title>
|
||||
<script type="module" crossorigin src="./assets/index-DNPiqLUu.js"></script>
|
||||
<script type="module" crossorigin src="./assets/index-DbPDzbcJ.js"></script>
|
||||
<link rel="stylesheet" crossorigin href="./assets/index-CrLE9rrV.css">
|
||||
</head>
|
||||
<body>
|
||||
|
|
|
|||
|
|
@ -36,16 +36,20 @@ _global_report_state: Optional["ReportState"] = None
|
|||
_CONTROL_CHARS = re.compile(r"[\x00-\x1f\x7f]+")
|
||||
|
||||
|
||||
class ReportOwnershipError(PermissionError):
|
||||
"""Raised when an agent tries to withdraw a report another agent filed."""
|
||||
class ReportRollbackError(RuntimeError):
|
||||
"""Raised when a failed deletion could not rewrite the on-disk indexes.
|
||||
|
||||
def __init__(self, report_id: str, *, filed_by: str, caller: str | None) -> None:
|
||||
The report is still on file in memory; the indexes are rewritten from
|
||||
memory on the next save.
|
||||
"""
|
||||
|
||||
def __init__(self, report_id: str, *, cause: BaseException) -> None:
|
||||
self.report_id = report_id
|
||||
self.filed_by = filed_by
|
||||
self.caller = caller
|
||||
self.cause = cause
|
||||
super().__init__(
|
||||
f"Report '{report_id}' was filed by agent {filed_by}, not by "
|
||||
f"{caller or 'the caller'}; only the agent that filed a report can withdraw it"
|
||||
f"Deletion of report '{report_id}' failed ({cause}) and the on-disk indexes "
|
||||
"could not be restored; the report is still on file and the indexes are "
|
||||
"rewritten on the next save"
|
||||
)
|
||||
|
||||
|
||||
|
|
@ -558,27 +562,23 @@ class ReportState:
|
|||
) -> dict[str, Any] | None:
|
||||
"""Withdraw a report from the run, keeping a record of the withdrawal.
|
||||
|
||||
Returns the removed report, or ``None`` when the id is unknown. Only the
|
||||
agent that filed a report may withdraw it (:class:`ReportOwnershipError`
|
||||
otherwise). The report leaves ``vulnerability_reports`` and its rendered
|
||||
artifacts; the run record keeps who withdrew it and why, so the id is
|
||||
Returns the removed report, or ``None`` when the id is unknown. Every
|
||||
agent in a run shares one trust boundary, so any of them may withdraw
|
||||
any report (just as any of them may revise one); the run record keeps
|
||||
who filed it, who withdrew it and why. The report leaves
|
||||
``vulnerability_reports`` and its rendered artifacts, and its id is
|
||||
never reissued.
|
||||
|
||||
The rewritten artifacts and the ``vulnerability_deleted_callback`` must
|
||||
both accept the deletion first: if either fails the report is put back
|
||||
and the error propagates, so the deletion can be retried.
|
||||
and the error propagates, so the deletion can be retried
|
||||
(:class:`ReportRollbackError` when the indexes could not be put back).
|
||||
"""
|
||||
report = next((r for r in self.vulnerability_reports if r.get("id") == report_id), None)
|
||||
if report is None:
|
||||
logger.warning("cannot delete unknown vulnerability report %s", report_id)
|
||||
return None
|
||||
|
||||
filed_by = report.get("agent_id")
|
||||
if filed_by and deleted_by_agent_id != filed_by:
|
||||
raise ReportOwnershipError(
|
||||
report_id, filed_by=str(filed_by), caller=deleted_by_agent_id
|
||||
)
|
||||
|
||||
entry: dict[str, Any] = {
|
||||
"id": report_id,
|
||||
"title": report.get("title"),
|
||||
|
|
@ -608,7 +608,7 @@ class ReportState:
|
|||
self._write_artifacts()
|
||||
if self.vulnerability_deleted_callback:
|
||||
self.vulnerability_deleted_callback({**report, "deletion": entry})
|
||||
except Exception:
|
||||
except Exception as exc:
|
||||
self.vulnerability_reports.insert(position, report)
|
||||
if was_saved:
|
||||
self._saved_vuln_ids.add(report_id)
|
||||
|
|
@ -616,7 +616,13 @@ class ReportState:
|
|||
self.run_record["deleted_vulnerability_reports"] = history
|
||||
else:
|
||||
self.run_record.pop("deleted_vulnerability_reports", None)
|
||||
self._save_artifacts()
|
||||
try:
|
||||
self._write_artifacts()
|
||||
except Exception as rollback_exc:
|
||||
logger.exception(
|
||||
"could not restore artifacts after failed deletion of %s", report_id
|
||||
)
|
||||
raise ReportRollbackError(report_id, cause=exc) from rollback_exc
|
||||
raise
|
||||
|
||||
md_path = self.get_run_dir() / "vulnerabilities" / f"{report_id}.md"
|
||||
|
|
|
|||
|
|
@ -720,7 +720,7 @@ def _do_delete(
|
|||
),
|
||||
}
|
||||
|
||||
from strix.report.state import ReportOwnershipError, get_global_report_state
|
||||
from strix.report.state import ReportRollbackError, get_global_report_state
|
||||
|
||||
report_state = get_global_report_state()
|
||||
if report_state is None:
|
||||
|
|
@ -736,23 +736,18 @@ def _do_delete(
|
|||
deleted_by_agent_id=agent_id,
|
||||
deleted_by_agent_name=agent_name,
|
||||
)
|
||||
except ReportOwnershipError as e:
|
||||
return {
|
||||
"success": False,
|
||||
"error": (
|
||||
f"{e!s}. If you have disproved another agent's finding, send that agent "
|
||||
"your counterevidence so it can withdraw the report itself."
|
||||
),
|
||||
"report_id": report_id,
|
||||
}
|
||||
except Exception as e:
|
||||
logger.exception("delete_vulnerability_report persistence failed")
|
||||
if isinstance(e, ReportRollbackError):
|
||||
outcome = (
|
||||
f"{e.cause!s}. The report is still on file, but its on-disk indexes could "
|
||||
"not be rewritten and may be stale until the next report is saved"
|
||||
)
|
||||
else:
|
||||
outcome = f"{e!s}. The report is still on file"
|
||||
return {
|
||||
"success": False,
|
||||
"error": (
|
||||
f"Failed to delete report '{report_id}': {e!s}. "
|
||||
"The report is still on file; retry the deletion."
|
||||
),
|
||||
"error": f"Failed to delete report '{report_id}': {outcome}; retry the deletion.",
|
||||
"report_id": report_id,
|
||||
}
|
||||
if deleted is None:
|
||||
|
|
@ -1654,11 +1649,10 @@ async def delete_vulnerability_report(
|
|||
impact and the counterevidence come down together. Do NOT rewrite a
|
||||
report into a "retracted" or "false positive" note either: delete it.
|
||||
|
||||
You can only withdraw a report you filed; a report filed by another
|
||||
agent is refused, so send that agent your counterevidence instead. Call
|
||||
``get_report`` first to read what it claims, then state in
|
||||
Only withdraw a report you have re-tested yourself, whoever filed it.
|
||||
Call ``get_report`` first to read what it claims, then state in
|
||||
``delete_reason`` exactly what disproved it, so the scan history shows
|
||||
why the finding is gone. A withdrawn report cannot be restored; if new
|
||||
who withdrew the finding and why. A withdrawn report cannot be restored; if new
|
||||
evidence later proves the issue, file it again.
|
||||
|
||||
Args:
|
||||
|
|
|
|||
|
|
@ -13,7 +13,8 @@ from strix.report.dedupe import (
|
|||
_prepare_report_for_comparison,
|
||||
check_duplicate,
|
||||
)
|
||||
from strix.report.state import ReportOwnershipError, ReportState, set_global_report_state
|
||||
from strix.report.state import ReportRollbackError, ReportState, set_global_report_state
|
||||
from strix.report.writer import write_run_record
|
||||
from strix.tools.finish.tool import finish_scan
|
||||
from strix.tools.reporting import tool as reporting_tool
|
||||
from strix.tools.reporting.tool import (
|
||||
|
|
@ -2061,61 +2062,28 @@ def _assert_report_still_filed(
|
|||
assert report_state._next_report_id() == "vuln-0010"
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
("agent_id", "agent_name"),
|
||||
[("834f79fb", "Validation Agent"), (None, None)],
|
||||
ids=["another agent", "no caller identity"],
|
||||
)
|
||||
def test_delete_refuses_a_report_the_caller_did_not_file(
|
||||
report_state: ReportState, agent_id: str | None, agent_name: str | None
|
||||
) -> None:
|
||||
"""Report ids are sequential and listed to every agent; knowing one is not owning it."""
|
||||
def test_any_agent_in_the_run_may_delete_a_report(report_state: ReportState) -> None:
|
||||
"""Agents in a run share one trust boundary: a report another agent filed can be
|
||||
withdrawn once disproved, and the history keeps both identities apart."""
|
||||
run_dir = _seed_saved_report(report_state)
|
||||
original = dict(report_state.vulnerability_reports[0])
|
||||
calls: list[dict[str, Any]] = []
|
||||
report_state.vulnerability_deleted_callback = calls.append
|
||||
|
||||
result = _do_delete(
|
||||
report_id="vuln-0009",
|
||||
delete_reason="Disproved.",
|
||||
agent_id=agent_id,
|
||||
agent_name=agent_name,
|
||||
agent_id="834f79fb",
|
||||
agent_name="Validation Agent",
|
||||
)
|
||||
|
||||
assert result["success"] is False
|
||||
assert "filed by agent aaaa1111" in result["error"]
|
||||
assert "send that agent your counterevidence" in result["error"]
|
||||
assert calls == [], "a refused deletion never reaches persistence"
|
||||
_assert_report_still_filed(report_state, run_dir, original)
|
||||
|
||||
|
||||
def test_delete_refuses_a_foreign_report_at_the_state_layer(report_state: ReportState) -> None:
|
||||
_seed_saved_report(report_state)
|
||||
|
||||
with pytest.raises(ReportOwnershipError) as excinfo:
|
||||
report_state.delete_vulnerability_report(
|
||||
"vuln-0009", delete_reason="Disproved.", deleted_by_agent_id="834f79fb"
|
||||
)
|
||||
|
||||
assert excinfo.value.filed_by == "aaaa1111"
|
||||
assert excinfo.value.caller == "834f79fb"
|
||||
assert len(report_state.vulnerability_reports) == 1
|
||||
|
||||
|
||||
def test_delete_allows_a_report_filed_without_an_agent(report_state: ReportState) -> None:
|
||||
"""A report with no filer on record (host-side or legacy) has no owner to check."""
|
||||
_seed_saved_report(report_state)
|
||||
del report_state.vulnerability_reports[0]["agent_id"]
|
||||
|
||||
deleted = report_state.delete_vulnerability_report(
|
||||
"vuln-0009", delete_reason="Disproved.", deleted_by_agent_id="834f79fb"
|
||||
)
|
||||
|
||||
assert deleted is not None
|
||||
assert result["success"] is True
|
||||
assert report_state.vulnerability_reports == []
|
||||
history = report_state.run_record["deleted_vulnerability_reports"]
|
||||
assert history[0]["agent_id"] == "834f79fb"
|
||||
assert "filed_by_agent_id" not in history[0]
|
||||
assert [c["id"] for c in calls] == ["vuln-0009"]
|
||||
assert not (run_dir / "vulnerabilities" / "vuln-0009.md").exists()
|
||||
(deletion,) = report_state.run_record["deleted_vulnerability_reports"]
|
||||
assert deletion["filed_by_agent_id"] == "aaaa1111"
|
||||
assert deletion["agent_id"] == "834f79fb"
|
||||
assert deletion["agent_name"] == "Validation Agent"
|
||||
|
||||
|
||||
def test_delete_reports_persistence_failure_and_keeps_the_report(
|
||||
|
|
@ -2169,6 +2137,45 @@ def test_delete_reports_an_artifact_write_failure_and_keeps_the_report(
|
|||
_assert_report_still_filed(report_state, run_dir, original)
|
||||
|
||||
|
||||
def test_delete_reports_when_the_rollback_itself_cannot_be_written(
|
||||
report_state: ReportState, monkeypatch: pytest.MonkeyPatch
|
||||
) -> None:
|
||||
"""A disk that stays full: memory is restored, the caller is told the indexes
|
||||
may be stale, and the next save rewrites them."""
|
||||
run_dir = _seed_saved_report(report_state)
|
||||
original = dict(report_state.vulnerability_reports[0])
|
||||
calls: list[dict[str, Any]] = []
|
||||
report_state.vulnerability_deleted_callback = calls.append
|
||||
disk_full = True
|
||||
|
||||
def write_run_record_while_disk_full(run_dir_: Path, record: dict[str, Any]) -> None:
|
||||
if disk_full:
|
||||
raise OSError(28, "No space left on device")
|
||||
write_run_record(run_dir_, record)
|
||||
|
||||
monkeypatch.setattr("strix.report.state.write_run_record", write_run_record_while_disk_full)
|
||||
|
||||
with pytest.raises(ReportRollbackError) as excinfo:
|
||||
report_state.delete_vulnerability_report(
|
||||
"vuln-0009", delete_reason="Disproved.", deleted_by_agent_id="aaaa1111"
|
||||
)
|
||||
|
||||
assert isinstance(excinfo.value.cause, OSError)
|
||||
assert calls == []
|
||||
assert report_state.vulnerability_reports == [original]
|
||||
assert "vuln-0009" in report_state._saved_vuln_ids
|
||||
assert "deleted_vulnerability_reports" not in report_state.run_record
|
||||
|
||||
result = _do_delete(report_id="vuln-0009", delete_reason="Disproved.", agent_id="aaaa1111")
|
||||
assert result["success"] is False
|
||||
assert "No space left on device" in result["error"]
|
||||
assert "may be stale" in result["error"]
|
||||
|
||||
disk_full = False
|
||||
report_state.save_run_data()
|
||||
_assert_report_still_filed(report_state, run_dir, original)
|
||||
|
||||
|
||||
def test_delete_can_be_retried_after_persistence_recovers(report_state: ReportState) -> None:
|
||||
run_dir = _seed_saved_report(report_state)
|
||||
outcomes = iter([RuntimeError("persistence failed"), None])
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue