mirror of
https://github.com/usestrix/strix.git
synced 2026-08-16 17:27:26 +02:00
coverage: stop publishing two gaps that are not gaps
A real scan surfaced both. An agent carrying path_traversal_lfi_rfi records "Path Traversal", not the skill's filename, so requiring every token of the leaf published "treat it as unexamined" for a class that had been tested and had a finding filed against it; 18 multi-token skills were exposed. Each vulnerability skill now declares how it can appear in a ledger row, with a test that a new skill must do the same. The root agent delegates rather than tests, so it recorded nothing on every clean run and every report carried a false Not Covered line. It is exempt while it has children, and only when the snapshot identifies one unambiguous root -- a root that worked alone is still held to the rule.
This commit is contained in:
+69
-18
@@ -65,21 +65,57 @@ _INCOMPLETE_RUN_STATUSES = frozenset({"failed", "interrupted", "stopped", "runni
|
||||
#: so holding one implies no coverage obligation.
|
||||
_RISK_SKILL_CATEGORY = "vulnerabilities"
|
||||
|
||||
#: Alternate phrasings for skills whose common name and whose canonical risk
|
||||
#: wording share no words. Matching a skill to a ledger row is textual, so
|
||||
#: without these an agent that recorded "object-level authorization" would be
|
||||
#: reported as never having looked at ``idor``. Only genuine synonyms belong
|
||||
#: here — a wrong alias suppresses a real gap, which is the costlier error.
|
||||
_SKILL_ALIASES: dict[str, tuple[str, ...]] = {
|
||||
"idor": ("object level authorization", "bola", "broken object level"),
|
||||
"xss": ("cross site scripting",),
|
||||
"csrf": ("cross site request forgery",),
|
||||
"ssrf": ("server side request forgery",),
|
||||
"rce": ("remote code execution", "command injection"),
|
||||
"sqli": ("sql injection",),
|
||||
"xxe": ("xml external entity",),
|
||||
"lfi": ("local file inclusion", "path traversal"),
|
||||
"ssti": ("server side template injection",),
|
||||
#: How each vulnerability skill can legitimately appear in a ledger row.
|
||||
#:
|
||||
#: Matching a skill to a row is textual, and a skill's filename is not how a
|
||||
#: pentester writes the class down: an agent carrying ``path_traversal_lfi_rfi``
|
||||
#: records "Path Traversal", and one carrying ``weak_password_detection``
|
||||
#: records "weak password policy". A row matches when it contains every word
|
||||
#: of *any one* phrasing here. Skills absent from this map fall back to their
|
||||
#: own words, so a new skill is merely matched strictly, never crashed on —
|
||||
#: but add an entry, because a false gap asserts something untrue in a report.
|
||||
_SKILL_PHRASINGS: dict[str, tuple[str, ...]] = {
|
||||
"authentication_jwt": ("authentication", "jwt", "session"),
|
||||
"broken_function_level_authorization": (
|
||||
"function level authorization",
|
||||
"authorization",
|
||||
"access control",
|
||||
"privilege escalation",
|
||||
),
|
||||
"business_logic": ("business logic", "logic flaw"),
|
||||
"csrf": ("csrf", "cross site request forgery"),
|
||||
"header_injection": ("header injection", "host header", "crlf"),
|
||||
"http_request_smuggling": ("request smuggling", "desync"),
|
||||
"idor": ("idor", "object level authorization", "bola", "direct object reference"),
|
||||
"information_disclosure": (
|
||||
"information disclosure",
|
||||
"information leak",
|
||||
"sensitive data",
|
||||
"data exposure",
|
||||
),
|
||||
"insecure_deserialization": ("deserialization",),
|
||||
"insecure_file_uploads": ("file upload",),
|
||||
"llm_prompt_injection": ("prompt injection",),
|
||||
"mass_assignment": ("mass assignment", "parameter binding"),
|
||||
"nosql_injection": ("nosql",),
|
||||
"open_redirect": ("redirect",),
|
||||
"path_traversal_lfi_rfi": (
|
||||
"path traversal",
|
||||
"directory traversal",
|
||||
"file inclusion",
|
||||
"lfi",
|
||||
"rfi",
|
||||
),
|
||||
"prototype_pollution": ("prototype pollution",),
|
||||
"race_conditions": ("race condition", "toctou"),
|
||||
"rce": ("rce", "remote code execution", "code execution", "command injection"),
|
||||
"sql_injection": ("sql injection", "sqli"),
|
||||
"ssrf": ("ssrf", "server side request forgery"),
|
||||
"ssti": ("ssti", "template injection"),
|
||||
"subdomain_takeover": ("subdomain takeover",),
|
||||
"weak_password_detection": ("password", "credential", "brute force"),
|
||||
"xss": ("xss", "cross site scripting", "script injection"),
|
||||
"xxe": ("xxe", "xml external entity", "xml entity"),
|
||||
}
|
||||
|
||||
|
||||
@@ -129,6 +165,13 @@ def agents_from_graph(graph: dict[str, Any]) -> list[dict[str, Any]]:
|
||||
names: dict[str, Any] = raw_names if isinstance(raw_names, dict) else {}
|
||||
raw_metadata = graph.get("metadata")
|
||||
metadata: dict[str, Any] = raw_metadata if isinstance(raw_metadata, dict) else {}
|
||||
raw_parents = graph.get("parent_of")
|
||||
parents: dict[str, Any] = raw_parents if isinstance(raw_parents, dict) else {}
|
||||
# Only an unambiguous root earns the exemption below. A snapshot with no
|
||||
# parent links at all makes every agent look parentless, and excusing all
|
||||
# of them would silently delete the silent-agent check.
|
||||
parentless = [agent_id for agent_id in statuses if not parents.get(agent_id)]
|
||||
root_id = parentless[0] if len(parentless) == 1 else None
|
||||
|
||||
agents: list[dict[str, Any]] = []
|
||||
for agent_id, status in statuses.items():
|
||||
@@ -143,6 +186,7 @@ def agents_from_graph(graph: dict[str, Any]) -> list[dict[str, Any]]:
|
||||
"status": str(status),
|
||||
"skills": [str(skill) for skill in skills],
|
||||
"task": str(meta.get("task") or ""),
|
||||
"is_root": agent_id == root_id,
|
||||
}
|
||||
)
|
||||
agents.sort(key=lambda agent: str(agent["agent_name"]))
|
||||
@@ -151,7 +195,7 @@ def agents_from_graph(graph: dict[str, Any]) -> list[dict[str, Any]]:
|
||||
|
||||
def _skill_phrasings(skill: str) -> list[list[str]]:
|
||||
"""Word lists that would each count as a ledger row naming *skill*."""
|
||||
phrasings = [skill, *_SKILL_ALIASES.get(skill, ())]
|
||||
phrasings = _SKILL_PHRASINGS.get(skill) or (skill,)
|
||||
return [terms for phrase in phrasings if (terms := _normalized(phrase).split())]
|
||||
|
||||
|
||||
@@ -204,11 +248,18 @@ def skill_coverage_gaps(
|
||||
def _silent_agent_gaps(
|
||||
entries: list[dict[str, Any]], agents: list[dict[str, Any]]
|
||||
) -> list[dict[str, Any]]:
|
||||
"""Agents that ran and recorded nothing at all."""
|
||||
"""Agents that ran and recorded nothing at all.
|
||||
|
||||
The root agent is exempt while it has children: it delegates and
|
||||
reconciles rather than testing, so flagging it on every clean scan would
|
||||
put a permanent false line in the report and teach readers to skip the
|
||||
section. A root that ran alone tested alone, and is held to the rule.
|
||||
"""
|
||||
recorded_ids = {str(entry.get("agent_id")) for entry in entries if entry.get("agent_id")}
|
||||
delegated = len(agents) > 1
|
||||
gaps: list[dict[str, Any]] = []
|
||||
for agent in agents:
|
||||
if agent["agent_id"] in recorded_ids:
|
||||
if agent["agent_id"] in recorded_ids or (agent["is_root"] and delegated):
|
||||
continue
|
||||
gaps.append(
|
||||
{
|
||||
|
||||
@@ -6,11 +6,13 @@ import json
|
||||
from typing import TYPE_CHECKING, Any
|
||||
|
||||
from strix.report.coverage import (
|
||||
_SKILL_PHRASINGS,
|
||||
build_coverage_document,
|
||||
read_agent_graph,
|
||||
render_coverage_markdown,
|
||||
write_coverage,
|
||||
)
|
||||
from strix.skills import get_available_skills
|
||||
|
||||
|
||||
if TYPE_CHECKING:
|
||||
@@ -201,3 +203,80 @@ def test_read_agent_graph_loads_a_snapshot(tmp_path: Path) -> None:
|
||||
(tmp_path / "agents.json").write_text(json.dumps(_graph()), encoding="utf-8")
|
||||
|
||||
assert read_agent_graph(tmp_path)["names"] == {"agent-1": "authz-tester"}
|
||||
|
||||
|
||||
def test_multi_token_skill_matches_how_a_pentester_writes_it() -> None:
|
||||
"""An agent carrying path_traversal_lfi_rfi records "Path Traversal".
|
||||
|
||||
Requiring the skill's filename verbatim published a false gap for a class
|
||||
that had been tested and even had a finding filed against it.
|
||||
"""
|
||||
doc = _document(
|
||||
entries=[_entry(risk_area="Path Traversal / Directory Traversal", surface="/download")],
|
||||
agent_graph=_graph(metadata={"agent-1": {"skills": ["path_traversal_lfi_rfi"]}}),
|
||||
)
|
||||
|
||||
assert not [gap for gap in doc["gaps"] if gap["kind"] == "unrecorded_risk_class"]
|
||||
|
||||
|
||||
def test_every_vulnerability_skill_declares_its_phrasings() -> None:
|
||||
"""A new skill without phrasings would be matched by its filename alone,
|
||||
which is how the false gap above got published."""
|
||||
missing = set(get_available_skills()["vulnerabilities"]) - set(_SKILL_PHRASINGS)
|
||||
|
||||
assert not missing, f"add ledger phrasings for: {sorted(missing)}"
|
||||
|
||||
|
||||
def test_declared_phrasings_name_real_skills() -> None:
|
||||
stale = set(_SKILL_PHRASINGS) - set(get_available_skills()["vulnerabilities"])
|
||||
|
||||
assert not stale, f"phrasings for skills that no longer exist: {sorted(stale)}"
|
||||
|
||||
|
||||
def _delegating_graph(**overrides: Any) -> dict[str, Any]:
|
||||
base: dict[str, Any] = {
|
||||
"statuses": {"root": "completed", "agent-1": "completed"},
|
||||
"names": {"root": "Root Agent", "agent-1": "authz-tester"},
|
||||
"parent_of": {"agent-1": "root"},
|
||||
"metadata": {"agent-1": {"skills": ["idor"]}},
|
||||
}
|
||||
base.update(overrides)
|
||||
return base
|
||||
|
||||
|
||||
def test_delegating_root_agent_is_not_a_coverage_gap() -> None:
|
||||
"""The root delegates and reconciles; it is not a tester that went quiet.
|
||||
Flagging it would put the same false line in every clean report."""
|
||||
doc = _document(agent_graph=_delegating_graph())
|
||||
|
||||
silent = [gap for gap in doc["gaps"] if gap["kind"] == "agent_recorded_no_coverage"]
|
||||
assert silent == []
|
||||
|
||||
|
||||
def test_a_subagent_that_records_nothing_is_still_a_gap() -> None:
|
||||
doc = _document(
|
||||
agent_graph=_delegating_graph(
|
||||
statuses={"root": "completed", "agent-1": "completed", "agent-2": "completed"},
|
||||
names={"root": "Root Agent", "agent-1": "authz-tester", "agent-2": "recon"},
|
||||
parent_of={"agent-1": "root", "agent-2": "root"},
|
||||
)
|
||||
)
|
||||
|
||||
silent = [gap for gap in doc["gaps"] if gap["kind"] == "agent_recorded_no_coverage"]
|
||||
assert [gap["agent_name"] for gap in silent] == ["recon"]
|
||||
|
||||
|
||||
def test_a_root_that_worked_alone_is_held_to_the_rule() -> None:
|
||||
"""With no subagents there is nobody else the testing could have come
|
||||
from, so silence is a real gap."""
|
||||
doc = _document(
|
||||
entries=[],
|
||||
agent_graph={
|
||||
"statuses": {"root": "completed"},
|
||||
"names": {"root": "Root Agent"},
|
||||
"parent_of": {},
|
||||
},
|
||||
)
|
||||
|
||||
silent = [gap for gap in doc["gaps"] if gap["kind"] == "agent_recorded_no_coverage"]
|
||||
assert [gap["agent_name"] for gap in silent] == ["Root Agent"]
|
||||
|
||||
Reference in New Issue
Block a user