feat(skill-scanner): Add detection for 7 new attack vectors#102
Conversation
Based on findings from https://github.com/gricha/dangerous-skills benchmarking 10 malicious skill patterns across 14 models. New detections in scan_skill.py: - Symlinks pointing outside skill directory (critical) - YAML frontmatter hooks that execute shell commands (critical) - !`command` pre-prompt injection syntax (high) - Test file auto-discovery (conftest.py, test_*.py, *.test.js) (high) - npm lifecycle hooks (postinstall) in bundled packages (critical) - PNG metadata text injection (tEXt/iTXt chunks) (high) - Unicode Tag character smuggling (U+E0000 block) (critical) Updated SKILL.md Phase 5 behavioral analysis with structural attack checklist. Updated reference files with new pattern documentation. Co-Authored-By: Claude <noreply@anthropic.com>
plugins/sentry-skills/skills/skill-scanner/scripts/scan_skill.py
Outdated
Show resolved
Hide resolved
plugins/sentry-skills/skills/skill-scanner/scripts/scan_skill.py
Outdated
Show resolved
Hide resolved
plugins/sentry-skills/skills/skill-scanner/scripts/scan_skill.py
Outdated
Show resolved
Hide resolved
- Fix PNG scanner: parse chunk boundaries properly instead of naive byte search. Prevents false positives from compressed image data. - Fix symlink check: use Path.is_relative_to() instead of unsafe string startswith comparison. - Fix package.json read: add encoding param and ValueError to except clause to handle non-UTF-8 files gracefully. Co-Authored-By: Claude <noreply@anthropic.com>
- iTXt chunks have 5 null-separated fields (keyword, comprFlag, comprMethod, langTag, transKeyword, text) — parse all 5 instead of treating like tEXt (which has only 2). - Handle package.json where "scripts" is null instead of missing. Co-Authored-By: Claude <noreply@anthropic.com>
| elif chunk_type == b"iTXt": | ||
| # iTXt: keyword\0comprFlag\0comprMethod\0langTag\0transKeyword\0text | ||
| parts = chunk_data.split(b"\x00", 4) | ||
| if len(parts) >= 5: |
There was a problem hiding this comment.
Bug: The iTXt chunk parsing logic incorrectly checks for 5 parts after splitting on null bytes, but a valid chunk only produces 4. This condition is never met.
Severity: CRITICAL
Suggested Fix
The split operation should be limited to 3 splits to match the 3 null separators in the iTXt spec: parts = chunk_data.split(b"\x00", 3). Consequently, the condition should check if len(parts) >= 4, and the text value should be accessed from parts[3].
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: plugins/sentry-skills/skills/skill-scanner/scripts/scan_skill.py#L499
Potential issue: The code for parsing `iTXt` chunks in PNG files contains a logical
error that prevents it from ever detecting metadata. The code splits the chunk data on
null bytes and then checks if the resulting list `parts` has a length of 5 or more with
`if len(parts) >= 5`. However, according to the PNG specification for `iTXt` chunks,
there are only 3 null separators, which means `split(b"\x00", 4)` will produce a maximum
of 4 parts. Because the length check will always fail, the code to extract the `keyword`
and `value` is never executed, rendering the entire `iTXt` detection feature
non-functional.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
| pkg = json.loads(pkg_json.read_text(encoding="utf-8", errors="replace")) | ||
| except (json.JSONDecodeError, OSError, ValueError): | ||
| continue | ||
| scripts = pkg.get("scripts") or {} |
There was a problem hiding this comment.
Non-dict JSON in package.json crashes the scanner
Medium Severity
If a package.json contains valid JSON that isn't an object (e.g., a JSON array like [1,2,3]), json.loads() succeeds but pkg.get("scripts") raises an AttributeError because lists and other non-dict types lack a .get() method. This call is outside the try/except block, so the exception propagates and crashes the entire scanner. A malicious skill could include such a file to prevent scanning.


Add detection for attack vectors discovered through systematic benchmarking
of 10 malicious skill patterns across 14 LLM models (https://github.com/gricha/dangerous-skills).
New detections in
scan_skill.py:examples/id_rsa.example→~/.ssh/id_rsa) — criticalPostToolUse, etc.) that execute shell commands automatically — critical!command`` pre-prompt injection that runs at template expansion before the model sees the prompt — highconftest.py,test_*.py,*.test.js) that execute as side effects ofpytest/npm test— highpostinstall) in bundledpackage.jsonfiles — criticalUpdated
SKILL.mdPhase 5 behavioral analysis with structural attack checklist.Updated reference files with new pattern documentation.
Tested against all 10 skills in the dangerous-skills repo — every attack vector is detected.
Agent transcript: https://claudescope.sentry.dev/share/KrNBlA9AEa20a0jWE4qxfVvJWVmp7ytU94Rs7fRFdfM