docs(readme): reconcile vulnerability-pattern documentation with all analyzer emitters - #476
evgenyt-res wants to merge 8 commits into
Conversation
The static_patterns_agent_snooping analyzer (PR NVIDIA#96) added AS1 (Agent Config Directory Access), AS2 (MCP Config File Access), and AS3 (Skill Enumeration) but the README's Vulnerability Patterns section and pattern/category totals were never updated to reflect them. Signed-off-by: Evgeny Talaevsky <evgeny.talaevsky@residenthome.com>
rng1995
left a comment
rng1995
left a comment
There was a problem hiding this comment.
[SkillSpector Review]
Reviewed current head 2e058eca3a8a2b40022400676f9f602835183a5a.
Changes requested: README.md:27 and line 357 change the advertised inventory to 74 patterns, but the tables below already contain 75 ID rows. They are also incomplete relative to the current detectors: RULE_ID_TO_CATEGORY maps 77 static IDs, including omitted E5, TM4, SC7, TT6, SSRF1-SSRF3, and DS1-DS4, while the README separately lists nine AST IDs. Please recompute the total using a clearly defined source of truth, reconcile the missing rows or explicitly document exclusions, and make the final “all detected patterns” claim accurate.
The hosted required checks pass. The branch is behind main, so it would still require an update and revalidation after the documentation is corrected.
Total is 87 patterns (77 static IDs in RULE_ID_TO_CATEGORY + 10 AST IDs in behavioral_ast.py) across 20 categories. Adds missing E5, SC7, TM4, TT6, and AST10 rows to their existing tables, and adds new Server-Side Request Forgery (SSRF1-3) and Insecure Deserialization (DS1-4) sections that had no README coverage. Signed-off-by: Evgeny Talaevsky <evgeny.talaevsky@residenthome.com>
c2e8faa to
713b589
Compare
|
@rng1995 pushed a fix for the pattern/category count reconciliation you flagged (now 87 patterns / 20 categories, verified against |
rng1995
left a comment
rng1995
left a comment
There was a problem hiding this comment.
[SkillSpector Review]
Re-reviewed current head 713b5892ed882c5226a8ac66b7417b9bd0fcc5ed. The prior inventory mismatch is resolved. At this exact head the README contains 87 distinct rule rows—77 registry-backed static IDs plus AST1–AST10—and exactly 20 pattern sections, including every ID previously omitted. The listed severities and categories match the current analyzers. I found no remaining required documentation change.
All required checks pass and there are no unresolved review threads. GitHub reports BEHIND, so update and revalidate before merging.
rng1995
left a comment
rng1995
left a comment
There was a problem hiding this comment.
[SkillSpector Review]
Re-reviewed current head 6a6cbda2dfbdef90eac9a1e011e74c2b18662157 after it changed during submission. The synchronization does not alter the one-file PR delta. The previously listed E5/SC7/TM4/TT6/SSRF/DS/AST omissions are now documented, but the inventory is still incomplete: the central registry used for the 87 count omits additional user-visible analyzer rule families. The inline finding identifies ten concrete missing IDs and the needed source-of-truth correction.
The required test-unit check is still running, and the inaccurate inventory plus GitHub's BLOCKED merge state block merge.
|
@rng1995 pushed a fix reconciling the README against every analyzer emitter (not just |
Reconcile the README's pattern/category totals against every analyzer emitter, not just RULE_ID_TO_CATEGORY. Adds the previously undocumented AE1-AE6 (analysis evasion), BH1-BH3 (bundled execution surface), PE4-PE5 (privilege escalation), and RP1-RP3 (MCP rug pull) findings. 87/20 -> 101/23 patterns/categories. Signed-off-by: Evgeny Talaevsky <evgeny.talaevsky@residenthome.com>
557d419 to
f9e2d7e
Compare
rng1995
left a comment
rng1995
left a comment
There was a problem hiding this comment.
[SkillSpector Review]
Re-reviewed current head 8b30718d59dfda988c82014b2e2412c9d987304f. The prior completeness/count blocker is resolved: this snapshot documents 101 unique rule IDs across 23 sections and includes AE1-AE6, BH1-BH3, PE4-PE5, and RP1-RP3; the prior thread is resolved and exact-head checks pass.
One documentation correction remains. README.md:519-527 does not match the analyzer's emitted severities at this same head: AST1 is HIGH, not CRITICAL; AST3 and AST4 are MEDIUM, not HIGH; and AST7 is LOW, not MEDIUM (src/skillspector/nodes/analyzers/behavioral_ast.py:148-158). Since this PR presents the tables as the complete vulnerability inventory, align those rows with runtime output and add or derive a source-of-truth check so severity drift cannot recur.
The branch is also behind main; re-sync and revalidate after correction.
Corrects README severities that no longer matched _RULE_SEVERITIES and hardcoded Severity.* values in the analyzers: - AST1 CRITICAL->HIGH, AST3/AST4 HIGH->MEDIUM, AST7 MEDIUM->LOW - EA1/EA2 HIGH->MEDIUM, EA3 MEDIUM->LOW - MP1 HIGH->MEDIUM - RA1 CRITICAL->HIGH, RA2 HIGH->MEDIUM Verified every remaining README severity against literal Severity assignments in src/skillspector/nodes/analyzers/**. Signed-off-by: Evgeny Talaevsky <evgeny.talaevsky@residenthome.com>
Signed-off-by: Evgeny Talaevsky <evgeny.talaevsky@residenthome.com> # Conflicts: # README.md
|
Pushed two more fixes:
@rng1995 ready for another look. |
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
The broader README changes and revised totals are not aligned with the stated PR scope and require clarification or narrowing.
Get a fresh assessment by requesting another Copilot review.
Review effort: Lite
Findings: 1
What changed in this PR
Updates README.md to document Agent Snooping patterns and broader vulnerability catalog changes.
Changes:
- Adds AS1–AS3 Agent Snooping documentation.
- Updates totals to 102 patterns across 23 categories.
- Adds additional pattern entries and severity changes.
| File | Summary |
|---|---|
README.md |
Updates vulnerability-pattern documentation and summary counts. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
|
||
| - **Multi-format input**: Scan Git repos, URLs, zip files, directories, or single files | ||
| - **71 vulnerability patterns** across 17 categories: prompt injection, data exfiltration, privilege escalation, supply chain, excessive agency, output handling, system prompt leakage, memory poisoning, tool misuse, rogue agent, anti-refusal, trigger abuse, dangerous code (AST), taint tracking, YARA signatures, MCP least privilege, and MCP tool poisoning | ||
| - **102 vulnerability patterns** across 23 categories: prompt injection, data exfiltration, privilege escalation, supply chain, excessive agency, output handling, system prompt leakage, memory poisoning, tool misuse, rogue agent, agent snooping, anti-refusal, trigger abuse, dangerous code (AST), taint tracking, server-side request forgery, insecure deserialization, YARA signatures, MCP least privilege, MCP tool poisoning, bundled execution surface, analysis evasion, and MCP rug pull |
evgenyt-res
changed the title
|
@copilot-pull-request-reviewer Updated the PR title/description to reflect the actual (reviewer-driven) scope rather than narrowing the diff back down - narrowing would just reintroduce the incompleteness rng1995 flagged across the last several rounds. See the updated description for the full rationale and change list. |
Problem
PR #96 (closing #75) added the
static_patterns_agent_snoopinganalyzer with three new rule IDs - AS1 (Agent Config Directory Access), AS2 (MCP Config File Access), AS3 (Skill Enumeration) - but the README's Vulnerability Patterns section was never updated. The pattern/category counts still read "71 vulnerability patterns across 17 categories" and there is no "Agent Snooping" table, so users reading the README have no way to discover these three checks.Scope note: this started as an AS1-AS3-only doc fix. Review (rng1995) correctly pointed out across several rounds that the README's totals were being computed against
RULE_ID_TO_CATEGORYas if it were an exhaustive source of truth, when it isn't - several analyzer families emit real, user-visible findings with no README row at all. The scope grew to close every gap found, rather than leave the doc newly-fixed-but-still-incomplete.Change
pattern_defaults.py: HIGH, HIGH, MEDIUM), placed after "Rogue Agent".mainitself).Severityvalues: AST1/AST3/AST4/AST7, EA1/EA2/EA3, MP1, RA1/RA2 - verified every remaining row againstsrc/skillspector/nodes/analyzers/**with no further mismatches found.No code changes - documentation only.