Skip to content

docs(readme): document AS1-AS3 agent snooping patterns - #476

Open
evgenyt-res wants to merge 6 commits into
NVIDIA:mainfrom
evgenyt-res:fix/document-as1-as2-as3-patterns
Open

evgenyt-res wants to merge 6 commits into
NVIDIA:mainfrom
evgenyt-res:fix/document-as1-as2-as3-patterns

Conversation

@evgenyt-res

Copy link
Copy Markdown

Problem

PR #96 (closing #75) added the static_patterns_agent_snooping analyzer 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.

Change

  • Add an "Agent Snooping (3 patterns)" table (AS1/AS2/AS3, severities matching pattern_defaults.py: HIGH, HIGH, MEDIUM) placed after "Rogue Agent", matching the analyzer's registration order.
  • Bump the two pattern/category totals from 71/17 to 74/18 in the Features summary and the Vulnerability Patterns section intro, and add "agent snooping" to the category list.

No code changes - documentation only.

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 rng1995 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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.

@evgenyt-res
evgenyt-res requested a review from rng1995 September 14, 2026 08:11
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>
@evgenyt-res
evgenyt-res force-pushed the fix/document-as1-as2-as3-patterns branch from c2e8faa to 713b589 Compare September 14, 2026 08:11
@evgenyt-res

Copy link
Copy Markdown
Author

@rng1995 pushed a fix for the pattern/category count reconciliation you flagged (now 87 patterns / 20 categories, verified against RULE_ID_TO_CATEGORY + AST IDs with no missing/extra rows). Could you take another look?

@rng1995 rng1995 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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 rng1995 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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.

Comment thread README.md Outdated
@evgenyt-res

Copy link
Copy Markdown
Author

@rng1995 pushed a fix reconciling the README against every analyzer emitter (not just RULE_ID_TO_CATEGORY): added AE1-AE6 (Analysis Evasion), BH1-BH3 (Bundled Execution Surface), PE4-PE5 (Privilege Escalation), and RP1-RP3 (MCP Rug Pull), bumping the totals to 101 patterns / 23 categories. Resolved the outstanding thread and re-requesting review - could you take another look?

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>
@evgenyt-res
evgenyt-res force-pushed the fix/document-as1-as2-as3-patterns branch from 557d419 to f9e2d7e Compare September 15, 2026 09:45
@evgenyt-res
evgenyt-res requested a review from rng1995 September 16, 2026 09:38

@rng1995 rng1995 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants