diff --git a/.claude/skills/package-skill/SKILL.md b/.claude/skills/package-skill/SKILL.md index 0198e3cc..a71304e7 100644 --- a/.claude/skills/package-skill/SKILL.md +++ b/.claude/skills/package-skill/SKILL.md @@ -147,10 +147,10 @@ Add each as a `security.allowed_issues` entry, matching by `rule_id` (exact — ```yaml security: allowed_issues: - - rule_id: ATR_2026_00066 - reason: "FP: matched shell variable expansion (\`${TOKEN}\`) in a - documented setup command (SKILL.md:45) — standard shell syntax, - not injected secrets." + - rule_id: PG_PII_CREDENTIAL_HARVESTING + reason: "FP: matched advice to store credentials in a dedicated + \`~/.mcp-env\` file (SKILL.md:213). The skill tells users where to keep + their own credentials; it never asks the user for them." ``` When packaging many skills from the same repo, findings cluster heavily by `rule_id` — collect all blocking findings across the batch first (group by skill + rule_id), write one templated-but-specific reason per rule_id, then customize per skill using that skill's actual matched text. Don't reuse a reason verbatim across skills without checking the cited location actually matches what's in *that* skill — genuinely different constructs can share a rule_id (e.g. an `iex (...)` PowerShell bootstrap vs. a `sudo apt-get install` line both trip the same "documented install command" rule but need different citations). diff --git a/docs/adding-skills.md b/docs/adding-skills.md index a50d2a52..46953e49 100644 --- a/docs/adding-skills.md +++ b/docs/adding-skills.md @@ -176,10 +176,10 @@ When `task scan-skill` reports an unallowlisted finding: ```yaml security: allowed_issues: - - rule_id: ATR_2026_00066 - reason: "FP: matched shell variable expansion (`${TOKEN}`) in a - documented setup command (SKILL.md:45) — standard shell syntax, - not injected secrets." + - rule_id: PG_PII_CREDENTIAL_HARVESTING + reason: "FP: matched advice to store credentials in a dedicated + `~/.mcp-env` file (SKILL.md:213). The skill tells users where to keep + their own credentials; it never asks the user for them." ``` 4. Re-run `task scan-skill -- skills/{skill-name}` until it passes. diff --git a/docs/security.md b/docs/security.md index ffc99171..a2499ae3 100644 --- a/docs/security.md +++ b/docs/security.md @@ -68,7 +68,7 @@ All agent skills are scanned using [Cisco AI Defense skill-scanner](https://gith ### What We Scan For -Skill-scanner runs pattern-based (YARA/ATR) and prompt-injection rule packs, plus optional LLM-based semantic and behavioral (AST/taint) analysis, looking for the same broad categories as mcp-scanner (prompt injection, tool/agent poisoning, credential harvesting, PII exposure) applied to a skill's `SKILL.md` and reference files instead of MCP tool descriptions. +Skill-scanner runs its core pattern-based (static and YARA) rules plus the PromptGuard rule pack, behavioral (AST/taint) analysis, and LLM-based semantic analysis, under the scanner's `quiet` policy preset. It looks for the same broad categories as mcp-scanner (prompt injection, tool/agent poisoning, credential harvesting, PII exposure) applied to a skill's `SKILL.md` and reference files instead of MCP tool descriptions. ### Allowing Known Issues @@ -77,10 +77,10 @@ Skill documentation is prose- and example-heavy, so pattern rules produce many f ```yaml security: allowed_issues: - - rule_id: ATR_2026_00066 - reason: "FP: matched shell variable expansion (`${TOKEN}`) in a - documented setup command (SKILL.md:45) — standard shell syntax, - not injected secrets." + - rule_id: PG_PII_CREDENTIAL_HARVESTING + reason: "FP: matched advice to store credentials in a dedicated + `~/.mcp-env` file (SKILL.md:213). The skill tells users where to keep + their own credentials; it never asks the user for them." ``` Each allowed issue must include: diff --git a/scripts/skill-scan/README.md b/scripts/skill-scan/README.md index 5fad3794..c76bd1ac 100644 --- a/scripts/skill-scan/README.md +++ b/scripts/skill-scan/README.md @@ -11,10 +11,14 @@ Invokes `skill-scanner scan --format json` and writes the JSON report. Exits `0` regardless of findings — allowlist filtering happens in `process_scan_results.py`. -The wrapper hard-codes the free/in-tree analyzers we always want: - -- `--rule-packs atr promptguard` — 340+ extra signatures (MCP tool poisoning, - agent attacks, Anthropic/OpenAI key detection, markdown exfil). +The wrapper hard-codes the free/in-tree analyzers and scan policy we always want: + +- `--rule-packs promptguard` - Anthropic/OpenAI key detection and markdown + exfiltration signatures. The ATR pack is intentionally not enabled: its + regex rules produced almost all HIGH+ false positives across the catalog, + and upstream ATR marks its skill-targeted rules as not ready for gating. +- `--policy quiet` - upstream's lowest-FPR gating preset. Demotes noisy rules + to LOW and caps low-confidence and contextual-risk LLM findings at LOW. - `--use-trigger` — vague-description / capability-inflation detector. - `--use-behavioral` — AST + dataflow taint analysis (Python and Bash). @@ -28,7 +32,7 @@ Optional environment variables: | Variable | Purpose | |---|---| -| `SKILL_SCANNER_USE_LLM` | `true` enables `--use-llm` + `--enable-meta`. Requires `SKILL_SCANNER_LLM_API_KEY`. | +| `SKILL_SCANNER_USE_LLM` | `true` enables `--use-llm`. Requires `SKILL_SCANNER_LLM_API_KEY`. The meta-analyzer (`--enable-meta`) is intentionally off because it made the blocking decision nondeterministic. | | `SKILL_SCANNER_LLM_API_KEY` | API key for the LLM analyzer (works for OpenAI/Anthropic/Azure/Bedrock/Vertex/Gemini/OpenRouter via LiteLLM). | | `SKILL_SCANNER_LLM_MODEL` | Model id (e.g. `anthropic/claude-sonnet-4-20250514`, `ollama/llama3`). Defaults to the scanner's built-in model. | | `SKILL_SCANNER_LLM_CONSENSUS_RUNS` | Integer >1 enables majority-vote consensus across N LLM runs. Multiplies LLM cost; off by default. | diff --git a/scripts/skill-scan/run_scan.py b/scripts/skill-scan/run_scan.py index a44b0d43..dbbf0ee1 100755 --- a/scripts/skill-scan/run_scan.py +++ b/scripts/skill-scan/run_scan.py @@ -43,9 +43,15 @@ def main() -> None: "--format", "json", "--output-json", args.output, # Always-on analyzers — free, in-tree, no network, no LLM key. - # ATR pack (314 rules) targets agent/MCP-style attacks; PromptGuard - # adds Anthropic/OpenAI key detection and markdown exfiltration rules. - "--rule-packs", "atr", "promptguard", + # PromptGuard adds Anthropic/OpenAI key detection and markdown + # exfiltration rules. The ATR pack is deliberately off: its regexes + # produced ~98% of HIGH+ hits across the catalog, and upstream ATR + # ships every skill-targeted rule as maturity "test", not for gating. + "--rule-packs", "promptguard", + # Upstream's lowest-FPR gating preset: demotes noisy rules to LOW and + # caps low-confidence and contextual-risk LLM findings at LOW, so they + # can't cross the HIGH block threshold. + "--policy", "quiet", # Vague-description / capability-inflation detector (skill discovery abuse). "--use-trigger", # AST + dataflow analyzer (Python/Bash). No execution, no key. @@ -55,10 +61,11 @@ def main() -> None: if os.environ.get("SKILL_SCANNER_USE_LLM", "").lower() == "true": if os.environ.get("SKILL_SCANNER_LLM_API_KEY"): scanner_args.extend([ + # No --enable-meta: the meta-analyzer can drop deterministic + # rule findings, which made the blocking decision flip between + # runs on identical content. Upstream also measured it costing + # 16.4 points of recall and turned it off by default. "--use-llm", - # Second-pass LLM correlator + false-positive filter. - # Costs one extra LLM call per scan but materially cuts noise. - "--enable-meta", # Match the scanner's own default; pin so we can tune from CI. "--llm-max-tokens", "8192", ])