diff --git a/strix/report/coverage.py b/strix/report/coverage.py index 5ffc7e71..90381c68 100644 --- a/strix/report/coverage.py +++ b/strix/report/coverage.py @@ -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( { diff --git a/tests/test_report_coverage.py b/tests/test_report_coverage.py index 918daf70..b95c1c9e 100644 --- a/tests/test_report_coverage.py +++ b/tests/test_report_coverage.py @@ -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"]