From b4018019fbfb7dafd9ab9a770180c86010543aee Mon Sep 17 00:00:00 2001 From: Abdelrahman Mostafa Date: Tue, 6 Oct 2026 12:14:25 +0300 Subject: [PATCH] fix(coverage): match risk-class phrasings by whole word, not substring _entry_is_about matched each phrasing word as a substring of the flattened row, so a short class token matched inside unrelated words - "rce" in "brute force"/"enforcement", "idor" in "corridor". An agent carrying a skill could then record an unrelated row and the gap detector would report that class as assessed, so coverage.json claimed work nobody did. Match whole words instead (word-set membership), with a forward +s plural allowance so "service worker" still matches a "service workers" row and "password" a "passwords" row. The reverse plural is deliberately not allowed: it would let a row word "xs" satisfy the "xss" alias and let an XS-Leaks row mask an XSS gap. Resolves #1447. --- strix/report/coverage.py | 30 +++++++++++++++++++++++++++--- tests/test_report_coverage.py | 29 +++++++++++++++++++++++++++++ 2 files changed, 56 insertions(+), 3 deletions(-) diff --git a/strix/report/coverage.py b/strix/report/coverage.py index d3e70e04..bcca1c33 100644 --- a/strix/report/coverage.py +++ b/strix/report/coverage.py @@ -309,10 +309,34 @@ def cwe_for_skill(skill: str) -> tuple[str, ...]: return vclass.cwe if vclass else () +def _term_in_words(term: str, words: set[str]) -> bool: + """A phrasing word matches a row word exactly, or as its simple plural. + + Matching whole words, not substrings, is what stops a short class token + like ``rce`` matching inside ``brute force`` / ``enforcement`` or ``idor`` + inside ``corridor``. The plural allowance (``worker`` matches a ``workers`` + row, ``password`` a ``passwords`` row) keeps a trivial inflection from + opening a false gap. + + Only the forward direction (phrasing term + ``s``) is allowed, never the + reverse (row word + ``s`` == term): the reverse would let the row word + ``xs`` satisfy the ``xss`` class alias (``"xs" + "s" == "xss"``), so an + unrelated XS-Leaks row would falsely mark XSS covered. A phrasing that + needs to match a singular row carries the singular spelling as its own + alias instead. + """ + return term in words or f"{term}s" in words + + def _entry_is_about(entry: dict[str, Any], phrasings: list[list[str]]) -> bool: - """True when a ledger row plausibly concerns any phrasing of a risk class.""" - haystack = _normalized(f"{entry.get('risk_area', '')} {entry.get('surface', '')}") - return any(all(term in haystack for term in terms) for terms in phrasings) + """True when a ledger row names any phrasing of a risk class. + + A phrasing matches when every one of its words appears as a **word** in the + row (``risk_area`` + ``surface``) — not as a substring, so "brute force" + never counts as ``rce`` coverage and "corridor" never as ``idor``. + """ + words = set(_normalized(f"{entry.get('risk_area', '')} {entry.get('surface', '')}").split()) + return any(all(_term_in_words(term, words) for term in terms) for terms in phrasings) def _entry_authored_by(entry: dict[str, Any], owner_ids: set[str]) -> bool: diff --git a/tests/test_report_coverage.py b/tests/test_report_coverage.py index d53d298d..c0ede09d 100644 --- a/tests/test_report_coverage.py +++ b/tests/test_report_coverage.py @@ -86,6 +86,35 @@ def test_assigned_risk_skill_without_coverage_becomes_a_gap() -> None: assert [gap["risk_area"] for gap in gaps] == ["sql injection"] +def test_substring_match_does_not_hide_a_gap() -> None: + """A short class token matches whole words, not substrings: 'brute force' is + not RCE coverage, 'enforcement' is not 'rce', 'corridor' is not 'idor'.""" + cases = [ + ("rce", "POST /login", "brute force protection"), + ("rce", "GET /api/resources/{id}", "object-level authorization"), + ("rce", "Access control enforcement", "authorization"), + ("idor", "GET /building/corridor/{id}", "information disclosure"), + ] + for skill, surface, risk in cases: + doc = _document( + entries=[_entry(surface=surface, risk_area=risk)], + agent_graph=_graph(metadata={"agent-1": {"skills": [skill]}}), + ) + classes = [g["risk_area"] for g in doc["gaps"] if g["kind"] == "unrecorded_risk_class"] + assert skill.replace("_", " ") in classes, (skill, surface, risk, classes) + + +def test_simple_plural_still_counts_as_coverage() -> None: + """Whole-word matching tolerates a trivial plural, so 'passwords' / + 'credentials' still cover weak_password_detection.""" + doc = _document( + entries=[_entry(risk_area="weak passwords and credentials reviewed", surface="/login")], + agent_graph=_graph(metadata={"agent-1": {"skills": ["weak_password_detection"]}}), + ) + + assert not [g for g in doc["gaps"] if g["kind"] == "unrecorded_risk_class"] + + def test_recorded_risk_class_is_not_reported_as_a_gap() -> None: doc = _document( entries=[_entry(risk_area="SQL injection", surface="GET /search?q=")],