docs(llm-security): SkillSpector gap analysis + reviewed execution plan
Brief + 8-step trekplan for 3 new dep-free scanners: TRG (trigger-abuse), SIG (signature engine over the decode pipeline), AST (Python AST taint). Adversarial review fixed 3 blockers + 7 majors on draft 1 (59/D -> 86/B). STATE.md prepped as a self-contained cold-start for /trekexecute next session. Un-gitignore STATE.md (global ~/.claude rule overrides old polyrepo convention); ignore generated plan annotation HTML. No scanner code yet. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01V3s6WnubSSrFjAQTLQdVbG
This commit is contained in:
parent
0ef780fd69
commit
8a772b0534
4 changed files with 780 additions and 2 deletions
8
.gitignore
vendored
8
.gitignore
vendored
|
|
@ -15,8 +15,9 @@ secrets.*
|
|||
.local/
|
||||
HANDOFF-FINDINGS.local.md
|
||||
|
||||
# --- session/local state (gitignored per ~/.claude polyrepo-konvensjon) ---
|
||||
STATE.md
|
||||
# --- session/local state ---
|
||||
# NOTE: STATE.md is intentionally TRACKED and committed (global ~/.claude rule
|
||||
# overrides the old polyrepo gitignore convention). Do NOT add STATE.md here.
|
||||
REMEMBER.md
|
||||
ROADMAP.md
|
||||
TODO.md
|
||||
|
|
@ -26,3 +27,6 @@ NEXT-SESSION-PROMPT*.local.md
|
|||
*.local.sh
|
||||
.DS_Store
|
||||
.claude/
|
||||
|
||||
# Voyage-generated plan annotation HTML (regenerable via annotate.mjs)
|
||||
docs/plans/*.html
|
||||
|
|
|
|||
43
STATE.md
Normal file
43
STATE.md
Normal file
|
|
@ -0,0 +1,43 @@
|
|||
# STATE — llm-security
|
||||
|
||||
**Active focus:** EXECUTE the SkillSpector-gap plan — 3 new scanners (TRG, SIG, AST). Plan is written,
|
||||
reviewed, validated. Next session = cold start straight into `/trekexecute`.
|
||||
|
||||
## COLD START — do this first
|
||||
1. `pwd` (should be repo root) and `git status` (expect clean; plan + brief committed).
|
||||
2. Read the plan: `docs/plans/trekplan-2026-06-20-skillspector-gap-analysis.md` (8 steps, validates clean).
|
||||
3. Run: **`/trekexecute docs/plans/trekplan-2026-06-20-skillspector-gap-analysis.md`**
|
||||
(optionally `/trekexecute --validate <plan>` or `--dry-run <plan>` first to sanity-check).
|
||||
4. TDD per step (failing test first), commit per step within its scope-fence. Check push window before push.
|
||||
|
||||
Build order: **TRG** (steps 1-2) → **SIG** (3-4) → **AST** (5-6) → **docs/packaging** (7) →
|
||||
**release v7.8.0** (8 — OPERATOR-GATED, confirm before doing; it's beyond the brief's scope).
|
||||
|
||||
## How we got here
|
||||
- Brief: `docs/plans/skillspector-gap-analysis.md` (gap analysis vs NVIDIA/SkillSpector; all claims verified).
|
||||
- Lean Voyage chosen (trekplan→execute→review, no research). trekplan done; adversarial review caught
|
||||
3 blockers + 7 majors on draft 1 (59/D) — all real — fixed → 86/B. The fixes are baked into the plan.
|
||||
|
||||
## Verified gotchas (DO NOT re-derive — these broke draft 1)
|
||||
- Orchestrator is **`scanners/scan-orchestrator.mjs`** (NOT repo root). Siblings import as `./x.mjs`.
|
||||
New scanner = export `scan(targetPath, discovery)`, add import + `{name,fn}` to SCANNERS (`:113-125`).
|
||||
- `severity.mjs` has **4 OWASP maps with ARRAY values** (`OWASP_MAP`/`_AGENTIC_`/`_SKILLS_`/`_MCP_`).
|
||||
Add array entries to all four per scanner; assert with `deepEqual`. (e.g. `WFL: ['LLM02','LLM06']`)
|
||||
- `parseFrontmatter` already exists: `scanners/lib/yaml-frontmatter.mjs:13` (TRG reuses it).
|
||||
- **Zero npm deps** (no `dependencies` key in package.json — keep it that way). python3 = optional,
|
||||
graceful `status:'skipped'` when absent. Emit via `finding()`/`scannerResult()` (`scanners/lib/output.mjs`).
|
||||
- Policy: add section to `DEFAULT_POLICY` (`scanners/lib/policy-loader.mjs`), read via `getPolicyValue`.
|
||||
- Distribution = Claude Code marketplace git-clone (whole repo), NOT npm — so `package.json` `files[]`
|
||||
is moot for runtime; the `knowledge/` add in Step 7 is correctness-only.
|
||||
- Template scanner to copy: `scanners/memory-poisoning-scanner.mjs:386`. Tests: `node --test`,
|
||||
committed `clean/`+`poisoned/` fixtures, `resetCounter()` in `beforeEach`. Full suite: `npm test`.
|
||||
|
||||
## Continuity
|
||||
- STATE.md is canonical + committed (now un-gitignored — global rule). Voyage `.session-state.local.json`
|
||||
is disposable per-plan scratch, gitignored (`*.local.json`).
|
||||
|
||||
## Committed this session
|
||||
- `docs/plans/skillspector-gap-analysis.md` (brief), `docs/plans/trekplan-2026-06-20-...md` (plan),
|
||||
`.gitignore` (un-ignore STATE.md; ignore `docs/plans/*.html`), this STATE.md.
|
||||
- Annotation HTML is gitignored (regenerable via Voyage `annotate.mjs`).
|
||||
- NO scanner code written yet — execution is next session.
|
||||
271
docs/plans/skillspector-gap-analysis.md
Normal file
271
docs/plans/skillspector-gap-analysis.md
Normal file
|
|
@ -0,0 +1,271 @@
|
|||
# SkillSpector Gap Analysis & Build Brief
|
||||
|
||||
**Date:** 2026-06-20
|
||||
**Plugin version at time of writing:** 7.7.2
|
||||
**Status:** Analysis complete, awaiting implementation go-ahead. No code written yet.
|
||||
**Reference:** NVIDIA/SkillSpector — https://github.com/nvidia/skillspector (Apache-2.0, Python, v2.2.3 as researched)
|
||||
|
||||
---
|
||||
|
||||
## 1. Framing
|
||||
|
||||
SkillSpector and llm-security solve partially different problems, and that determines what "missing"
|
||||
actually means:
|
||||
|
||||
- **SkillSpector** is a static **pre-install gate** for skill bundles (`SKILL.md` + code + deps).
|
||||
Python, `pip install`, runs in CI, emits pass/fail. Its depth is in *static code analysis of the
|
||||
bundle itself* — real Python AST, variable-level taint, YARA signatures.
|
||||
- **llm-security** is a broad **runtime + posture defense** for an entire Claude Code setup: 9 hooks,
|
||||
multi-session behavioral analysis, drift, baseline/diff, watch, IDE scanning, 7 package ecosystems,
|
||||
git forensics, MCP live-inspect.
|
||||
|
||||
We are **broader and own a runtime layer they lack entirely**. They are **deeper on a few static
|
||||
techniques** we either lack or do more coarsely. "Fill what's missing" therefore means closing a
|
||||
small number of sharp holes in our static analysis — not becoming SkillSpector.
|
||||
|
||||
The goal for each item below is an llm-security-native implementation (zero external npm
|
||||
dependencies, integrated with our decode/normalize pipeline and policy system) — not a port.
|
||||
|
||||
---
|
||||
|
||||
## 2. Verification log
|
||||
|
||||
Per the verification duty, every claim this brief depends on was checked against actual source —
|
||||
SkillSpector via raw source on `main`, our own plugin via direct file reads. Verdicts below are
|
||||
load-bearing; the priorities in §6 follow from them.
|
||||
|
||||
### SkillSpector (all VERIFIED against source)
|
||||
|
||||
| Claim | Verdict | Source file |
|
||||
|---|---|---|
|
||||
| 4 YARA rulesets (malware, webshells, cryptominers, hacktools) | VERIFIED | `src/skillspector/yara_rules/*.yar` |
|
||||
| Python `ast`-based behavioral analysis (exec/eval/subprocess) | VERIFIED | `nodes/analyzers/behavioral_ast.py` |
|
||||
| Variable-level taint tracking via AST (source→sink) | VERIFIED | `nodes/analyzers/behavioral_taint_tracking.py` |
|
||||
| Trigger-abuse rules TR1 (broad), TR2 (shadow), TR3 (baiting) | VERIFIED | `nodes/analyzers/static_patterns_supply_chain.py` |
|
||||
| 0–100 risk score + exit 0/1/2 at threshold 50 | VERIFIED | `cli.py` (`risk_score > 50 → Exit(1)`) |
|
||||
| Manifest-vs-code least privilege LP1–LP4 | VERIFIED | `nodes/analyzers/mcp_least_privilege.py` |
|
||||
| SARIF 2.1.0 output | VERIFIED | `cli.py` + `sarif_models.py` |
|
||||
|
||||
### llm-security (our actual status)
|
||||
|
||||
| Capability | Verdict | Evidence |
|
||||
|---|---|---|
|
||||
| #1 Known-malware signature engine | **MISSING** | No YARA/webshell/cryptominer/signature engine in `scanners/`, `scanners/lib/`, `knowledge/`. Entropy/taint detect *shape*, not known-bad *identity*. |
|
||||
| #2 Real AST analysis | **MISSING** | `taint-tracer.mjs:1–12` self-documents regex matching, ~70% recall, no scope/cross-file. No AST library imported anywhere (`package.json` has zero deps; no acorn/babel/espree/tree-sitter). |
|
||||
| #3 Trigger/activation-abuse detection | **MISSING** | `permission-mapper.mjs` has exactly 6 checks (purpose-vs-tools, dangerous combos, ghost hooks, haiku model, over-privilege, missing-doc). None inspect `name`/`description` for broad/baiting/shadowing triggers. |
|
||||
| #4 Per-scan 0–100 risk score | **HAVE** | `severity.mjs` `riskScore(counts)` → `output.aggregate.risk_score`; logged in orchestrator. |
|
||||
| #4 CI exit-code gating | **HAVE** | `scan-orchestrator.mjs:324–333`: `--fail-on <level>` exits 1 over threshold; default exits 2/1/0 on BLOCK/WARNING/ALLOW. Policy key `ci.failOn`. |
|
||||
|
||||
**Correction to the initial analysis:** #4 was provisionally listed as a gap. Verification REFUTES that
|
||||
— it is fully implemented. #4 is therefore reclassified from "build" to "already covered, optional
|
||||
polish" (§5). It is intentionally *not* a build item.
|
||||
|
||||
---
|
||||
|
||||
## 3. What we already beat them on (scope guard — do not rebuild)
|
||||
|
||||
Runtime hooks (9), multi-session lethal-trifecta + Jensen-Shannon behavioral drift, MCP description
|
||||
drift with immutable baseline, baseline/diff, watch mode, IDE-extension scanning with VSIX sandbox,
|
||||
7 package ecosystems (vs their PyPI+npm), git forensics, pre-compact transcript scan, evidence-package
|
||||
sanitization for remote repos, skill-registry fingerprinting. None of these exist in SkillSpector.
|
||||
No inspiration to draw there.
|
||||
|
||||
---
|
||||
|
||||
## 4. Build items
|
||||
|
||||
Three genuine gaps. Each lists the *our-own-way* differentiator, concrete plug-in points (verified
|
||||
from the orchestrator), framework mapping, risks, and per-item verification criteria.
|
||||
|
||||
Shared plug-in facts (verified):
|
||||
- New deterministic scanner = export `scan(targetPath, discovery)` (add `requiresPriorResults: true`
|
||||
for cross-scanner correlation), import it in `scan-orchestrator.mjs`, append to the `SCANNERS` array
|
||||
(`scan-orchestrator.mjs:101–125`).
|
||||
- Findings use `finding()` (`scanners/lib/output.mjs:30–44`): `scanner` (prefix), `severity`, `title`,
|
||||
`description`, `file`, `line`, `evidence`, `owasp`, `recommendation`. IDs auto-format as `DS-<PREFIX>-NNN`.
|
||||
- Configurable thresholds: add a section to `DEFAULT_POLICY` and read via
|
||||
`getPolicyValue('section','key',default)`. User policy at `<project>/.llm-security/policy.json`.
|
||||
|
||||
### 4.1 — Signature engine (#1) · proposed prefix `SIG`
|
||||
|
||||
**Gap:** We detect *shape* (entropy, obfuscation, dangerous flows) but never match against *known-bad
|
||||
identities* — reverse shells, PHP/JSP/ASPX webshells, cryptominer stratum strings, offensive-tool
|
||||
references. SkillSpector ships 4 YARA rulesets for exactly this.
|
||||
|
||||
**Our-own-way (better, not a copy):**
|
||||
- **Pure-Node signature engine, zero deps.** Rules live as data in `knowledge/signatures.json`
|
||||
(string/regex/codepoint matchers grouped by family: webshell, reverse-shell, cryptominer, hacktool,
|
||||
c2). No `yara-python` dependency.
|
||||
- **Match against the decode/normalize pipeline output, not just raw bytes.** This is the differentiator:
|
||||
YARA matches raw file content, so a base64- or hex-obfuscated webshell evades it. We already have
|
||||
`normalizeForScan()` (Unicode/hex/URL/base64/HTML-entity/zero-width/BIDI/homoglyph/rot13 decode).
|
||||
Running signatures against every decoded variant catches obfuscated known-malware SkillSpector misses.
|
||||
- **Optional acceleration, graceful degradation.** If a `yara` binary is present on PATH, optionally
|
||||
shell out to it for the heavy lifting (we already shell out to `npm audit`/`git`/`pip`); otherwise
|
||||
the pure-Node engine is authoritative. Never a hard dependency.
|
||||
|
||||
**Plug-in:** new `scanners/signature-scanner.mjs`, prefix `SIG`, registered in `SCANNERS`. Rule data in
|
||||
`knowledge/signatures.json`. Policy section `signatures` (enable/disable families, custom rule path).
|
||||
|
||||
**Frameworks:** LLM03 (supply chain) primary; webshell/C2 hits also LLM02 (exfiltration); ASI04.
|
||||
|
||||
**Risks:** signature corpus is maintenance surface and can go stale — keep it small, high-confidence,
|
||||
and family-scoped; document provenance of each rule. False positives on security tooling/education —
|
||||
mitigate with context down-weighting (§5, confidence) and path exclusions (`knowledge/`, tests).
|
||||
|
||||
**Verification criteria:**
|
||||
- Fixture containing a known PHP webshell pattern → `SIG` flags it (HIGH/CRITICAL).
|
||||
- Same payload base64-wrapped → still flagged (decode-pipeline integration; this is what beats raw YARA).
|
||||
- Benign file with the word "shell" in prose → not flagged.
|
||||
- `yara` binary absent → scanner still runs and flags via pure-Node path.
|
||||
|
||||
### 4.2 — AST analysis (#2) · augments `TNT`, proposed behavioral prefix `AST`
|
||||
|
||||
**Gap:** `taint-tracer.mjs` is regex/line-based: ~70% recall, no scope awareness, no cross-file, no
|
||||
flow through arrays/objects/closures (self-documented at `taint-tracer.mjs:1–12`). SkillSpector parses
|
||||
a real Python AST and tracks taint at variable level with high confidence. This is our deepest
|
||||
methodology gap.
|
||||
|
||||
**Constraint:** zero npm deps (`package.json` is empty of dependencies). A real JS AST would require a
|
||||
parser dependency. This shapes the recommendation.
|
||||
|
||||
**Our-own-way — staged, dep-free:**
|
||||
- **Phase A (recommended first): Python AST via a shipped helper.** Ship `scanners/lib/py-ast-taint.py`
|
||||
invoked as `python3 <helper> <file>` (same optional-python pattern as `dep-auditor`'s `pip audit`).
|
||||
The helper does `ast.parse` + `ast.walk`, runs source→sink taint at variable level, and emits JSON
|
||||
findings back to Node. Graceful degradation: if `python3` is absent, fall back to the existing regex
|
||||
tracer. *Rationale for going Python-first:* skill bundles' executable payloads are most often Python,
|
||||
it is exactly where SkillSpector's AST advantage is real and where our regex tracer is weakest, and
|
||||
it is achievable with no dependency.
|
||||
- **Phase B (smaller, optional): bracket-aware scope pass for JS/TS.** Improve the existing JS regex
|
||||
tracer with balanced-bracket/function-scope tokenization to cut the "variable named `input` taints
|
||||
the whole file" class of false positives — incremental, still dep-free, no full parser.
|
||||
- **Phase C (deferred): full JS/TS AST.** Requires either vendoring a micro-parser (maintenance cost)
|
||||
or accepting a dependency (breaks the zero-dep invariant). Defer until A+B are measured; decide then.
|
||||
|
||||
**Plug-in:** Python findings flow through the existing `taint` scanner result (or a new `AST` prefix if
|
||||
we want them separable in reports). Add policy section `ast` (enable, python path override). The helper
|
||||
emits the same `finding()` fields so the orchestrator/SARIF path is unchanged.
|
||||
|
||||
**Frameworks:** LLM01 (injection sinks), LLM02 (exfiltration sinks) — same mapping as `TNT`, higher
|
||||
precision and recall.
|
||||
|
||||
**Risks:** python3 availability varies (handled by fallback). Subprocess to an interpreter on untrusted
|
||||
code — the helper must only *parse* (`ast.parse` never executes), never `exec`/`eval`/import the target.
|
||||
State this as a hard invariant and test it.
|
||||
|
||||
**Verification criteria:**
|
||||
- Python fixture: `k = os.environ["AWS_SECRET"]; requests.post(url, data=k)` → flagged CRITICAL (creds→net),
|
||||
where the regex tracer's recall is shaky on the intermediate variable.
|
||||
- Python fixture with `input` reused in two functions, only one tainted → only the tainted use flagged
|
||||
(scope awareness the regex tracer lacks).
|
||||
- Helper invoked on a file that calls `os.system("rm -rf /")` → parses without executing it (parse-only
|
||||
invariant; assert no side effects).
|
||||
- `python3` removed from PATH → orchestrator still completes via regex fallback (no crash).
|
||||
|
||||
### 4.3 — Trigger / activation-abuse scanner (#3) · proposed prefix `TRG`
|
||||
|
||||
**Gap:** Cleanest uncovered hole and the most skill-native. Claude skills/commands auto-activate on
|
||||
their `description`. A skill can be engineered to hijack activation — a single common-word trigger, a
|
||||
maximally-activating description ("use this for anything", "always", "all messages"), or a `name` that
|
||||
shadows a built-in command. SkillSpector covers this as TR1–TR3; we have nothing. `permission-mapper`'s
|
||||
6 checks never read `name`/`description` for activation-surface abuse.
|
||||
|
||||
**Our-own-way:** dedicated `scanners/trigger-abuse.mjs`, reusing our existing frontmatter parsing across
|
||||
`commands/`, `agents/`, `skills/`:
|
||||
- **TRG-broad:** single common-word or ≤2-char trigger names; description that claims universal
|
||||
applicability.
|
||||
- **TRG-baiting:** maximally-activating phrasing ("anything", "everything", "always", "all
|
||||
files/messages", "whenever") — word lists in policy, extensible.
|
||||
- **TRG-shadow:** `name` colliding with built-in command/tool names (configurable built-in list).
|
||||
- Differentiator vs SkillSpector: run the description through `normalizeForScan()` first, so a baiting
|
||||
description hidden behind homoglyphs/zero-width still trips — consistent with the rest of our pipeline.
|
||||
|
||||
**Plug-in:** new scanner registered in `SCANNERS`, prefix `TRG`, policy section `triggers`
|
||||
(word lists, built-in names, severity per rule).
|
||||
|
||||
**Frameworks:** AST04 (Scope Creep) primary; LLM06 (Excessive Agency). Cross-reference
|
||||
`knowledge/owasp-skills-top10.md`.
|
||||
|
||||
**Risks:** legitimate broad utilities exist (e.g. a general formatter) — keep severity advisory
|
||||
(MEDIUM) by default, allowlist via policy, and require the *combination* of broad-name + universal-claim
|
||||
for the higher tier to cut false positives.
|
||||
|
||||
**Verification criteria:**
|
||||
- Skill with `name: run` and `description: "use for anything"` → TRG-shadow + TRG-baiting fire.
|
||||
- Skill with a specific, scoped description → no finding.
|
||||
- Baiting phrase obfuscated with zero-width chars → still flagged (normalize-first differentiator).
|
||||
- New scanner appears in `scan-orchestrator` JSON and SARIF output with correct `owasp` tag.
|
||||
|
||||
---
|
||||
|
||||
## 5. #4 — already covered (no build)
|
||||
|
||||
Per §2, the 0–100 risk score and CI exit-code gate exist. Optional, low-cost polish only:
|
||||
|
||||
- **Per-finding `confidence`.** The `finding()` schema has no `confidence` field; SkillSpector uses
|
||||
confidence pervasively and adjusts it by context (e.g. P5 down-weights safety/education contexts).
|
||||
Adding an optional `confidence` field and feeding it into `riskScore()` would (a) let the new
|
||||
signature/AST/trigger scanners express uncertainty and (b) reduce false-positive weight on
|
||||
docs/test/fixture contexts. This is the one cross-cutting enhancement that connects #1/#2/#3 to the
|
||||
existing score. Small, optional, do-after.
|
||||
- **Document the CI story.** `docs/ci-cd-guide.md` exists; confirm `--fail-on` + `risk_score` +
|
||||
SARIF upload are documented end-to-end. Doc-only.
|
||||
|
||||
---
|
||||
|
||||
## 6. Priority & sequencing
|
||||
|
||||
Recommended order (value × independence × risk):
|
||||
|
||||
1. **#3 Trigger-abuse (`TRG`)** — smallest, lowest-risk, fully dep-free, most skill-native, reuses
|
||||
existing frontmatter + normalize machinery. Best first win; establishes the new-scanner pattern.
|
||||
2. **#1 Signature engine (`SIG`)** — high value, dep-free, the decode-pipeline integration is a clear
|
||||
"better than YARA" story. Main cost is curating a small high-confidence rule corpus.
|
||||
3. **#2 AST, Phase A (Python helper)** — deepest methodology lift but most design surface (subprocess,
|
||||
parse-only invariant, fallback). Do after the new-scanner pattern is proven by #3/#1. Phases B/C
|
||||
gated on measuring A.
|
||||
|
||||
Each item is independent and shippable on its own; no item blocks another. TDD per the Iron Law —
|
||||
failing fixture test first for every rule.
|
||||
|
||||
---
|
||||
|
||||
## 7. Out of scope / deliberate non-goals
|
||||
|
||||
- **Harmful-content moderation (SkillSpector P5: toxic substances / dangerous actions).** Content
|
||||
safety is not our mission — we are a code/config security scanner, not a content moderator. Excluded.
|
||||
- **LLM semantic analyzers (SkillSpector SSD/SDI/SQP).** We already cover semantic analysis through our
|
||||
agents (`skill-scanner-agent`, etc.); not reimplementing as inline LLM nodes.
|
||||
- **Standalone `pip install` distribution.** Our identity is the runtime Claude Code plugin;
|
||||
`scan-orchestrator.mjs` already works as a CLI. Repackaging for non-Claude CI is deprioritized.
|
||||
- **Manifest-vs-code least privilege (SkillSpector LP1–LP4).** Partially covered by `permission-mapper`
|
||||
(intent-vs-tools, over-privilege). The under-declared dimension (code reaches network but tool not
|
||||
declared) is a *possible later extension* of `permission-mapper`, not part of this brief's three items.
|
||||
|
||||
---
|
||||
|
||||
## 8. Consolidated verification criteria
|
||||
|
||||
Acceptance for the build phase (each maps to a fixture test):
|
||||
|
||||
- [ ] `SIG` flags a known webshell fixture, including a base64-obfuscated variant; ignores benign prose;
|
||||
runs without a `yara` binary.
|
||||
- [ ] `AST` (Python helper) flags creds→network with an intermediate variable; respects function scope;
|
||||
never executes target code; orchestrator survives missing `python3`.
|
||||
- [ ] `TRG` flags shadow+baiting skills, including obfuscated baiting; passes scoped skills; appears in
|
||||
JSON + SARIF with correct OWASP tags.
|
||||
- [ ] All three appear in `scan-orchestrator.mjs` `SCANNERS`, emit standard `finding()` objects, and are
|
||||
policy-configurable via `.llm-security/policy.json`.
|
||||
- [ ] No new npm dependency added (`package.json` dependencies remain empty).
|
||||
- [ ] Existing test suite (1820+) stays green; new rules covered by failing-first tests.
|
||||
|
||||
---
|
||||
|
||||
## Sources
|
||||
|
||||
- NVIDIA/SkillSpector source (`main`), verified file-by-file 2026-06-20 — paths in §2.
|
||||
- NVIDIA Technical Blog: developer.nvidia.com/blog (verified-agent-skills capability governance).
|
||||
- OWASP Agentic Skills Top 10 (AST) — `knowledge/owasp-skills-top10.md`.
|
||||
- llm-security internals verified by direct read: `scan-orchestrator.mjs`, `scanners/taint-tracer.mjs`,
|
||||
`scanners/permission-mapper.mjs`, `scanners/lib/{output,severity,policy-loader}.mjs`.
|
||||
460
docs/plans/trekplan-2026-06-20-skillspector-gap-analysis.md
Normal file
460
docs/plans/trekplan-2026-06-20-skillspector-gap-analysis.md
Normal file
|
|
@ -0,0 +1,460 @@
|
|||
# SkillSpector-Inspired Scanners: TRG, SIG, AST
|
||||
|
||||
> **Plan quality: B** (86/100) — APPROVE_WITH_NOTES (revised after adversarial review)
|
||||
>
|
||||
> Generated by trekplan v2.0 on 2026-06-20 — `plan_version: 1.7`
|
||||
|
||||
## Context
|
||||
|
||||
llm-security is a broad runtime + posture defense for Claude Code. A gap analysis against
|
||||
NVIDIA/SkillSpector (`docs/plans/skillspector-gap-analysis.md`) confirmed three genuine holes in our
|
||||
static analysis, each to be closed our own dep-free way:
|
||||
|
||||
1. **#3 TRG — trigger/activation-abuse**: no scanner inspects command/agent/skill `name`+`description`
|
||||
frontmatter for broad triggers, keyword-baiting, or built-in shadowing. Skills auto-activate on
|
||||
description, so this is a real, uncovered, skill-native attack surface.
|
||||
2. **#1 SIG — signature engine**: we detect *shape* (entropy/obfuscation/taint) but never *known-bad
|
||||
identity* (webshells, reverse shells, cryptominers, hacktools). Our differentiator: run signatures
|
||||
against the existing `normalizeForScan()` decode pipeline so obfuscated known-malware is caught —
|
||||
something raw YARA (which SkillSpector ships) misses.
|
||||
3. **#2 AST — Python AST taint**: `taint-tracer.mjs` is regex (~70% recall, no scope/cross-file). A
|
||||
shipped `python3` AST helper raises recall/precision for Python skill code, with graceful fallback
|
||||
to the regex tracer when python3 is absent.
|
||||
|
||||
Hard invariant: **no new npm dependency** — `package.json` has no `dependencies` key today and must
|
||||
keep none. The 1820+ test suite must stay green.
|
||||
|
||||
## Architecture Diagram
|
||||
|
||||
```mermaid
|
||||
graph TD
|
||||
subgraph "Changes in this plan"
|
||||
ORCH["scanners/scan-orchestrator.mjs<br/>(SCANNERS array + imports)"]
|
||||
TRG["scanners/trigger-scanner.mjs (NEW)<br/>prefix TRG"]
|
||||
SIG["scanners/signature-scanner.mjs (NEW)<br/>prefix SIG"]
|
||||
AST["scanners/ast-taint-scanner.mjs (NEW)<br/>prefix AST"]
|
||||
PYH["scanners/lib/py-ast-taint.py (NEW)<br/>parse-only helper"]
|
||||
SIGDATA["knowledge/signatures.json (NEW)"]
|
||||
POL["scanners/lib/policy-loader.mjs<br/>+trg/+sig/+ast sections"]
|
||||
SEV["scanners/lib/severity.mjs<br/>4 OWASP maps +TRG/+SIG/+AST"]
|
||||
OUT["scanners/lib/output.mjs<br/>finding() / scannerResult() (reused)"]
|
||||
NORM["scanners/lib/string-utils.mjs<br/>normalizeForScan() (reused)"]
|
||||
FM["scanners/lib/yaml-frontmatter.mjs<br/>parseFrontmatter() (reused)"]
|
||||
|
||||
ORCH --> TRG & SIG & AST
|
||||
AST --> PYH
|
||||
SIG --> SIGDATA
|
||||
SIG --> NORM
|
||||
TRG --> NORM & FM
|
||||
TRG & SIG & AST --> OUT
|
||||
ORCH --> POL
|
||||
TRG & SIG & AST --> SEV
|
||||
end
|
||||
```
|
||||
|
||||
## Codebase Analysis
|
||||
|
||||
- **Tech stack:** Node.js ESM (`.mjs`), zero npm dependencies (`package.json` has no `dependencies` key). Test runner: `node --test` + `node:assert/strict`. Optional `python3` shell-outs already used (`dep-auditor` → pip).
|
||||
- **Key patterns:** every deterministic scanner exports `async function scan(targetPath, discovery)`, loops `discovery.files` (`FileInfo {absPath, relPath, ext, size}`), emits `finding(...)`, returns `scannerResult(...)`. The orchestrator resets the finding-ID counter per scanner.
|
||||
- **Relevant files (verified against source):**
|
||||
- Orchestrator: **`scanners/scan-orchestrator.mjs`** (NOT repo root). Imports at `:101-111`, `SCANNERS` array at `:113-125`. Sibling scanners imported as `./unicode-scanner.mjs` (so a new scanner in `scanners/` imports as `./trigger-scanner.mjs`).
|
||||
- Template scanner: `scanners/memory-poisoning-scanner.mjs:386` (scan signature + `discovery.files` loop + dual-return).
|
||||
- Output: `scanners/lib/output.mjs:30` (`finding()`), `:57` (`scannerResult()`), `:12` (`resetCounter()`).
|
||||
- Policy: `scanners/lib/policy-loader.mjs:13-68` (`DEFAULT_POLICY`), `:150` (`getPolicyValue(section, key, default, projectRoot)` — 4th arg is the project root, resolved to `<root>/.llm-security/policy.json`).
|
||||
- OWASP maps: `scanners/lib/severity.mjs` — **four** frozen maps with **array** values: `OWASP_MAP` (`:132`, LLM), `OWASP_AGENTIC_MAP` (`:151`, ASI), `OWASP_SKILLS_MAP` (`:170`, AST), `OWASP_MCP_MAP` (`:189`, MCP). Every scanner prefix has a key in all four (empty array where N/A, e.g. `WFL: []` in the skills map). `owaspCategorize` spreads `OWASP_MAP[prefix]` (`:227`).
|
||||
- Frontmatter: `scanners/lib/yaml-frontmatter.mjs:13` exports `parseFrontmatter(content)` (already used by `permission-mapper.mjs:10`). TRG reuses it directly — no inline parser needed.
|
||||
- Knowledge loader model: `scanners/dep-auditor.mjs:20-42` (cached `__dirname`-relative JSON loader, graceful fallback).
|
||||
- Decode API: `scanners/lib/string-utils.mjs:512` (`normalizeForScan()→string`), `:486` (`foldHomoglyphs`), `:474` (`rot13`).
|
||||
- File discovery: `scanners/lib/file-discovery.mjs:52` (`discoverFiles`), `readTextFile`; `.md`/`.py`/`.php` are in `TEXT_EXTENSIONS` so TRG/SIG/AST fixtures are discoverable.
|
||||
- `package.json` `files`: `["bin/","scanners/","LICENSE","README.md","CONTRIBUTING.md","SECURITY.md","CHANGELOG.md"]` — `knowledge/` absent. Moot for runtime (marketplace = git-clone of whole repo; existing scanners already load `knowledge/*.json`), but Step 7 adds `knowledge/` for correctness.
|
||||
- **Reusable code:** `finding`/`scannerResult`/`resetCounter` (`lib/output.mjs`); `discoverFiles`/`readTextFile` (`lib/file-discovery.mjs`); `normalizeForScan`/`foldHomoglyphs`/`rot13` (`lib/string-utils.mjs`); `parseFrontmatter` (`lib/yaml-frontmatter.mjs`); `getPolicyValue` (`lib/policy-loader.mjs`); the four OWASP maps + `SEVERITY` (`lib/severity.mjs`).
|
||||
- **Test infra:** tests in `tests/scanners/*.test.mjs`; committed fixtures under `tests/fixtures/<name>-scan/{clean,poisoned}/` (model: `tests/fixtures/memory-scan/`). `resetCounter()` in `beforeEach`. Single-file run: `node --test tests/scanners/<name>.test.mjs`. No lint/coverage gate. Registration-test model asserts `deepEqual` on all four OWASP maps (`tests/scanners/workflow-scanner.test.mjs:196-216`). Binary-absent skip: `which python3` guard + early `return` (`tests/scanners/vsix-sandbox.test.mjs:27-30`) and graceful `status:'skipped'` envelope (`tests/scanners/dep.test.mjs:117-124`).
|
||||
|
||||
## Implementation Plan
|
||||
|
||||
Each scanner's **core** (new files only — revert-safe) lands before its **wiring** (small edits to
|
||||
shared `scanners/scan-orchestrator.mjs` / `scanners/lib/policy-loader.mjs` / `scanners/lib/severity.mjs`).
|
||||
Build order follows the brief: TRG → SIG → AST. Step 8 (release) is operator-gated.
|
||||
|
||||
### Step 1: Add TRG trigger-abuse scanner with tests and fixtures
|
||||
|
||||
- **Files:** `scanners/trigger-scanner.mjs` (new), `tests/scanners/trigger-scanner.test.mjs` (new), `tests/fixtures/trigger-scan/clean/` (new), `tests/fixtures/trigger-scan/poisoned/` (new)
|
||||
- **Changes:** Create `scan(targetPath, discovery)` mirroring `memory-poisoning-scanner.mjs:386`. Filter `discovery.files` to `commands/*.md`, `agents/*.md`, `skills/**/SKILL.md`. Parse frontmatter via `parseFrontmatter` from `scanners/lib/yaml-frontmatter.mjs`. Emit three rule classes: **TRG-broad** (single common-word/≤2-char `name`; description claiming universal applicability), **TRG-baiting** (maximally-activating phrases: "anything","everything","always","all files/messages","whenever" — list from policy), **TRG-shadow** (`name` colliding with a configurable built-in command/tool list). Run the description through `normalizeForScan()` before phrase matching so obfuscated baiting (zero-width/homoglyph) still trips. Default severity MEDIUM; broad-name + universal-claim combination → HIGH. Emit via `finding({scanner:'TRG', owasp:'LLM06', ...})`; return `scannerResult('trigger-scanner', ...)`. Fixtures: `poisoned/` = a skill named `run` with "use for anything" + an obfuscated-baiting case; `clean/` = a scoped, specifically-described skill. (new files)
|
||||
- **Reuses:** `scan(targetPath, discovery)` + `discovery.files` loop (`memory-poisoning-scanner.mjs:386`); `finding`/`scannerResult`/`resetCounter` (`lib/output.mjs`); `readTextFile`/`discoverFiles` (`lib/file-discovery.mjs`); `normalizeForScan` (`lib/string-utils.mjs`); `parseFrontmatter` (`lib/yaml-frontmatter.mjs`).
|
||||
- **Test first:**
|
||||
- File: `tests/scanners/trigger-scanner.test.mjs` (new)
|
||||
- Verifies: `clean/` → `status:'ok'`, `findings.length === 0`; `poisoned/` → TRG-shadow + TRG-baiting fire, obfuscated baiting still flagged, all `id` start `DS-TRG-`, `scanner==='TRG'`, required fields present.
|
||||
- Pattern: `tests/scanners/memory-poisoning.test.mjs` (`resetCounter()` in `beforeEach`)
|
||||
- **Verify:** `node --test tests/scanners/trigger-scanner.test.mjs` → expected: `fail 0`
|
||||
- **On failure:** revert — `git checkout -- scanners/trigger-scanner.mjs tests/scanners/trigger-scanner.test.mjs && git clean -fd tests/fixtures/trigger-scan`
|
||||
- **Checkpoint:** `git commit -m "feat(llm-security): add TRG trigger-abuse scanner"`
|
||||
- **Manifest:**
|
||||
```yaml
|
||||
manifest:
|
||||
expected_paths:
|
||||
- scanners/trigger-scanner.mjs
|
||||
- tests/scanners/trigger-scanner.test.mjs
|
||||
min_file_count: 2
|
||||
commit_message_pattern: "^feat\\(llm-security\\): add TRG trigger-abuse scanner$"
|
||||
bash_syntax_check: []
|
||||
forbidden_paths:
|
||||
- scanners/scan-orchestrator.mjs
|
||||
must_contain:
|
||||
- path: scanners/trigger-scanner.mjs
|
||||
pattern: "'TRG'"
|
||||
- path: scanners/trigger-scanner.mjs
|
||||
pattern: "normalizeForScan"
|
||||
- path: scanners/trigger-scanner.mjs
|
||||
pattern: "parseFrontmatter"
|
||||
```
|
||||
|
||||
### Step 2: Wire TRG into orchestrator, policy, and the four OWASP maps
|
||||
|
||||
- **Files:** `scanners/scan-orchestrator.mjs`, `scanners/lib/policy-loader.mjs`, `scanners/lib/severity.mjs`
|
||||
- **Changes:** Add `import { scan as trgScan } from './trigger-scanner.mjs';` to the import block (`scan-orchestrator.mjs:101-111`) and `{ name: 'trg', fn: trgScan }` to the `SCANNERS` array (`:113-125`). Add a frozen `trg` section to `DEFAULT_POLICY` (`policy-loader.mjs:13-68`): `{ mode:'warn', baiting_phrases:[...], builtin_names:[...], broad_single_words:[...] }`; the scanner reads these via `getPolicyValue('trg', key, default, targetPath)` (targetPath is the scan root where `.llm-security/policy.json` lives). Add **array** entries to all four maps in `severity.mjs`: `OWASP_MAP.TRG=['LLM06']`, `OWASP_AGENTIC_MAP.TRG=[]`, `OWASP_SKILLS_MAP.TRG=['AST04']` (brief's primary framework), `OWASP_MCP_MAP.TRG=[]`. Add registration assertions to the TRG test (registered in `SCANNERS`; `assert.deepEqual(OWASP_SKILLS_MAP.TRG, ['AST04'])`; policy override of `baiting_phrases` changes detection).
|
||||
- **Reuses:** `SCANNERS` registration (`scan-orchestrator.mjs:113`); `DEFAULT_POLICY`/`getPolicyValue` (`policy-loader.mjs:13,150`); the four OWASP maps (`severity.mjs:132-189`).
|
||||
- **Test first:**
|
||||
- File: `tests/scanners/trigger-scanner.test.mjs` (existing — extend)
|
||||
- Verifies: TRG in orchestrator `SCANNERS`; `deepEqual(OWASP_SKILLS_MAP.TRG, ['AST04'])` and `deepEqual(OWASP_MAP.TRG, ['LLM06'])`; policy override changes detection.
|
||||
- Pattern: `tests/scanners/workflow-scanner.test.mjs:196-216`
|
||||
- **Verify:** `npm test` → expected: `fail 0`
|
||||
- **On failure:** revert — `git checkout -- scanners/scan-orchestrator.mjs scanners/lib/policy-loader.mjs scanners/lib/severity.mjs tests/scanners/trigger-scanner.test.mjs`
|
||||
- **Checkpoint:** `git commit -m "feat(llm-security): wire TRG scanner into orchestrator and policy"`
|
||||
- **Manifest:**
|
||||
```yaml
|
||||
manifest:
|
||||
expected_paths:
|
||||
- scanners/scan-orchestrator.mjs
|
||||
- scanners/lib/policy-loader.mjs
|
||||
- scanners/lib/severity.mjs
|
||||
min_file_count: 3
|
||||
commit_message_pattern: "^feat\\(llm-security\\): wire TRG scanner into orchestrator and policy$"
|
||||
bash_syntax_check: []
|
||||
forbidden_paths: []
|
||||
must_contain:
|
||||
- path: scanners/scan-orchestrator.mjs
|
||||
pattern: "trgScan"
|
||||
- path: scanners/lib/severity.mjs
|
||||
pattern: "TRG"
|
||||
```
|
||||
|
||||
### Step 3: Add SIG signature scanner with rule data, tests and fixtures
|
||||
|
||||
- **Files:** `scanners/signature-scanner.mjs` (new), `knowledge/signatures.json` (new), `tests/scanners/signature-scanner.test.mjs` (new), `tests/fixtures/signature-scan/clean/` (new), `tests/fixtures/signature-scan/poisoned/` (new)
|
||||
- **Changes:** Create `knowledge/signatures.json` with a small, high-confidence, family-grouped ruleset (`webshell`,`reverse_shell`,`cryptominer`,`hacktool`) — each rule `{ id, family, severity, pattern, description, provenance }`. Create `scan(targetPath, discovery)` that, for each file in `discovery.files`, builds the decoded variant set (`content`, `normalizeForScan(content)`, `foldHomoglyphs(content)`, `rot13(content)`) and tests every signature regex against all variants — a match on a decoded-only variant is the differentiator vs raw YARA. De-dup per (file, rule). Emit `finding({scanner:'SIG', owasp:'LLM03', ...})`. Load the ruleset via the cached `__dirname`-relative loader copied from `dep-auditor.mjs:20-42`; graceful empty-ruleset fallback. Path-exclude `knowledge/`, `tests/`, `docs/`. Fixtures: `poisoned/` = a PHP webshell sample AND a base64-wrapped variant of it; `clean/` = benign prose mentioning "shell". (new files)
|
||||
- **Reuses:** scan/loop/return (`memory-poisoning-scanner.mjs:386`); JSON loader (`dep-auditor.mjs:20-42`); `normalizeForScan`/`foldHomoglyphs`/`rot13` (`string-utils.mjs`); `finding`/`scannerResult` (`output.mjs`).
|
||||
- **Test first:**
|
||||
- File: `tests/scanners/signature-scanner.test.mjs` (new)
|
||||
- Verifies: `poisoned/` → webshell flagged AND its base64-wrapped variant flagged (decode-pipeline integration); `clean/` → 0 findings; `scanner==='SIG'`, ids start `DS-SIG-`.
|
||||
- Pattern: `tests/scanners/memory-poisoning.test.mjs`
|
||||
- **Verify:** `node --test tests/scanners/signature-scanner.test.mjs` → expected: `fail 0`
|
||||
- **On failure:** revert — `git checkout -- scanners/signature-scanner.mjs knowledge/signatures.json tests/scanners/signature-scanner.test.mjs && git clean -fd tests/fixtures/signature-scan`
|
||||
- **Checkpoint:** `git commit -m "feat(llm-security): add SIG signature scanner with decode-pipeline matching"`
|
||||
- **Manifest:**
|
||||
```yaml
|
||||
manifest:
|
||||
expected_paths:
|
||||
- scanners/signature-scanner.mjs
|
||||
- knowledge/signatures.json
|
||||
- tests/scanners/signature-scanner.test.mjs
|
||||
min_file_count: 3
|
||||
commit_message_pattern: "^feat\\(llm-security\\): add SIG signature scanner with decode-pipeline matching$"
|
||||
bash_syntax_check: []
|
||||
forbidden_paths:
|
||||
- scanners/scan-orchestrator.mjs
|
||||
must_contain:
|
||||
- path: scanners/signature-scanner.mjs
|
||||
pattern: "'SIG'"
|
||||
- path: scanners/signature-scanner.mjs
|
||||
pattern: "normalizeForScan"
|
||||
```
|
||||
|
||||
### Step 4: Wire SIG into orchestrator, policy, and the four OWASP maps
|
||||
|
||||
- **Files:** `scanners/scan-orchestrator.mjs`, `scanners/lib/policy-loader.mjs`, `scanners/lib/severity.mjs`
|
||||
- **Changes:** Add `import { scan as sigScan } from './signature-scanner.mjs';` and `{ name: 'sig', fn: sigScan }` to `SCANNERS`. Add a frozen `sig` section to `DEFAULT_POLICY`: `{ enabled_families:['webshell','reverse_shell','cryptominer','hacktool'], custom_rules_path:null }`, read via `getPolicyValue('sig', ...)`. Add array entries to all four maps: `OWASP_MAP.SIG=['LLM03','LLM02']`, `OWASP_AGENTIC_MAP.SIG=['ASI04']`, `OWASP_SKILLS_MAP.SIG=[]`, `OWASP_MCP_MAP.SIG=[]`. Add registration assertions (incl. `deepEqual(OWASP_MAP.SIG, ['LLM03','LLM02'])` and a family-disable policy test) to the SIG test.
|
||||
- **Reuses:** same wiring sites as Step 2.
|
||||
- **Test first:**
|
||||
- File: `tests/scanners/signature-scanner.test.mjs` (existing — extend)
|
||||
- Verifies: SIG in `SCANNERS`; `deepEqual(OWASP_MAP.SIG, ['LLM03','LLM02'])`; disabling a family via policy suppresses its findings.
|
||||
- Pattern: `tests/scanners/workflow-scanner.test.mjs:196-216`
|
||||
- **Verify:** `npm test` → expected: `fail 0`
|
||||
- **On failure:** revert — `git checkout -- scanners/scan-orchestrator.mjs scanners/lib/policy-loader.mjs scanners/lib/severity.mjs tests/scanners/signature-scanner.test.mjs`
|
||||
- **Checkpoint:** `git commit -m "feat(llm-security): wire SIG scanner into orchestrator and policy"`
|
||||
- **Manifest:**
|
||||
```yaml
|
||||
manifest:
|
||||
expected_paths:
|
||||
- scanners/scan-orchestrator.mjs
|
||||
- scanners/lib/policy-loader.mjs
|
||||
- scanners/lib/severity.mjs
|
||||
min_file_count: 3
|
||||
commit_message_pattern: "^feat\\(llm-security\\): wire SIG scanner into orchestrator and policy$"
|
||||
bash_syntax_check: []
|
||||
forbidden_paths: []
|
||||
must_contain:
|
||||
- path: scanners/scan-orchestrator.mjs
|
||||
pattern: "sigScan"
|
||||
- path: scanners/lib/severity.mjs
|
||||
pattern: "SIG"
|
||||
```
|
||||
|
||||
### Step 5: Add AST Python-taint helper and scanner with graceful fallback
|
||||
|
||||
- **Files:** `scanners/lib/py-ast-taint.py` (new), `scanners/ast-taint-scanner.mjs` (new), `tests/scanners/ast-taint-scanner.test.mjs` (new), `tests/fixtures/ast-scan/` (new)
|
||||
- **Changes:** Create `scanners/lib/py-ast-taint.py`: reads a target `.py` path, runs `ast.parse` (PARSE-ONLY — never `exec`/`eval`/`compile`/import the target), walks the tree for variable-level taint (sources: `os.environ`/`os.getenv`/`open`/`requests`/`sys.stdin`/`input`; sinks: `requests.post/put`, `exec`/`eval`/`subprocess`/`os.system`, file writes), and prints to stdout a single JSON object with this **contract**: `{ "status": "ok", "findings": [ { "rule": "TT3", "severity": "critical", "line": <int>, "source": <str>, "sink": <str>, "message": <str> } ] }`; on a parse error it prints `{ "status": "error", "message": <str> }` and exits non-zero. Create `scanners/ast-taint-scanner.mjs`: for each `.py` in `discovery.files`, call `spawnSync('python3', [helperPath, file], { encoding:'utf8', timeout: 5000 })`. Handle every branch: ENOENT / `python3` absent → return `scannerResult('ast-taint-scanner', 'skipped', ...)`; non-zero exit, timeout, or unparseable stdout → record that file as an `info`-level scan note and continue (never throw); valid `{status:'ok'}` → map each helper finding to `finding({scanner:'AST', owasp:'LLM01', ...})`. Fixtures: `creds-net.py` (`k=os.environ["AWS_SECRET"]; requests.post(url,data=k)`), `scope.py` (`input` reused across two functions, only one tainted), and `sentinel.py` (contains `os.system("touch SENTINEL")` to prove parse-only safety — the file must never run).
|
||||
- **Reuses:** scan/return shape (`memory-poisoning-scanner.mjs:386`); `spawnSync` graceful-skip precedent (`dep-auditor.mjs`/`git-forensics.mjs`); `finding`/`scannerResult` (`output.mjs`).
|
||||
- **Test first:**
|
||||
- File: `tests/scanners/ast-taint-scanner.test.mjs` (new)
|
||||
- Verifies: (a) `which python3` guard, early `return` if absent; with python3 → creds→net flagged through the intermediate variable, scoped `input` only flags the tainted use, ids start `DS-AST-`; (b) python3-absent path → `status:'skipped'`, no crash; (c) parse-only safety: after scanning `sentinel.py`, assert no `SENTINEL` file exists; (d) a deliberately malformed `.py` → scan completes without throwing.
|
||||
- Pattern: `tests/scanners/vsix-sandbox.test.mjs:27-30` (which-guard) + `tests/scanners/dep.test.mjs:117-124` (skipped envelope)
|
||||
- **Verify:** `node --test tests/scanners/ast-taint-scanner.test.mjs` → expected: `fail 0`
|
||||
- **On failure:** revert — `git checkout -- scanners/lib/py-ast-taint.py scanners/ast-taint-scanner.mjs tests/scanners/ast-taint-scanner.test.mjs && git clean -fd tests/fixtures/ast-scan`
|
||||
- **Checkpoint:** `git commit -m "feat(llm-security): add AST Python-taint scanner with python3 fallback"`
|
||||
- **Manifest:**
|
||||
```yaml
|
||||
manifest:
|
||||
expected_paths:
|
||||
- scanners/lib/py-ast-taint.py
|
||||
- scanners/ast-taint-scanner.mjs
|
||||
- tests/scanners/ast-taint-scanner.test.mjs
|
||||
min_file_count: 3
|
||||
commit_message_pattern: "^feat\\(llm-security\\): add AST Python-taint scanner with python3 fallback$"
|
||||
bash_syntax_check: []
|
||||
forbidden_paths:
|
||||
- scanners/scan-orchestrator.mjs
|
||||
must_contain:
|
||||
- path: scanners/lib/py-ast-taint.py
|
||||
pattern: "ast.parse"
|
||||
- path: scanners/ast-taint-scanner.mjs
|
||||
pattern: "'AST'"
|
||||
- path: scanners/ast-taint-scanner.mjs
|
||||
pattern: "timeout"
|
||||
```
|
||||
|
||||
### Step 6: Wire AST into orchestrator, policy, and the four OWASP maps
|
||||
|
||||
- **Files:** `scanners/scan-orchestrator.mjs`, `scanners/lib/policy-loader.mjs`, `scanners/lib/severity.mjs`
|
||||
- **Changes:** Add `import { scan as astScan } from './ast-taint-scanner.mjs';` and `{ name: 'ast', fn: astScan }` to `SCANNERS`. Add a frozen `ast` section to `DEFAULT_POLICY`: `{ enabled:true, python_path:'python3', timeout_ms:5000 }`, read via `getPolicyValue('ast', ...)`. Add array entries to all four maps: `OWASP_MAP.AST=['LLM01','LLM02']`, `OWASP_AGENTIC_MAP.AST=[]`, `OWASP_SKILLS_MAP.AST=['AST02']`, `OWASP_MCP_MAP.AST=[]`. Add registration assertions (incl. `deepEqual(OWASP_MAP.AST, ['LLM01','LLM02'])` and `enabled:false` → `skipped`) to the AST test.
|
||||
- **Reuses:** same wiring sites as Step 2.
|
||||
- **Test first:**
|
||||
- File: `tests/scanners/ast-taint-scanner.test.mjs` (existing — extend)
|
||||
- Verifies: AST in `SCANNERS`; `deepEqual(OWASP_MAP.AST, ['LLM01','LLM02'])`; `enabled:false` policy short-circuits to `skipped`.
|
||||
- Pattern: `tests/scanners/workflow-scanner.test.mjs:196-216`
|
||||
- **Verify:** `npm test` → expected: `fail 0`
|
||||
- **On failure:** revert — `git checkout -- scanners/scan-orchestrator.mjs scanners/lib/policy-loader.mjs scanners/lib/severity.mjs tests/scanners/ast-taint-scanner.test.mjs`
|
||||
- **Checkpoint:** `git commit -m "feat(llm-security): wire AST scanner into orchestrator and policy"`
|
||||
- **Manifest:**
|
||||
```yaml
|
||||
manifest:
|
||||
expected_paths:
|
||||
- scanners/scan-orchestrator.mjs
|
||||
- scanners/lib/policy-loader.mjs
|
||||
- scanners/lib/severity.mjs
|
||||
min_file_count: 3
|
||||
commit_message_pattern: "^feat\\(llm-security\\): wire AST scanner into orchestrator and policy$"
|
||||
bash_syntax_check: []
|
||||
forbidden_paths: []
|
||||
must_contain:
|
||||
- path: scanners/scan-orchestrator.mjs
|
||||
pattern: "astScan"
|
||||
- path: scanners/lib/severity.mjs
|
||||
pattern: "AST"
|
||||
```
|
||||
|
||||
### Step 7: Docs, scanner inventory, packaging, and full-suite gate
|
||||
|
||||
- **Files:** `CLAUDE.md`, `docs/scanner-reference.md`, `scanners/lib/output.mjs`, `package.json`
|
||||
- **Changes:** Add TRG/SIG/AST rows + update the deterministic-scanner counts in `CLAUDE.md` and `docs/scanner-reference.md` (grep current numeric counts and bump each to reflect 3 new scanners). Add `TRG`/`SIG`/`AST` to the valid-prefix list in the `finding()` JSDoc (`output.mjs:19`). Fix the stale header comment in `scanners/scan-orchestrator.mjs` ("all 7 scanners" → actual count). Add `"knowledge/"` to the `files` array in `package.json` (correctness; moot for git-clone distribution). Run the full suite as the integration gate. No version bump here — that is Step 8.
|
||||
- **Reuses:** existing doc structure in `docs/scanner-reference.md`; JSDoc prefix list (`output.mjs:19`).
|
||||
- **Test first:** n/a (docs/comment/packaging). Verification = full suite + presence greps.
|
||||
- **Verify:** `npm test` → expected: `fail 0`; then `grep -q '"knowledge/"' package.json && grep -q 'TRG' docs/scanner-reference.md` → expected: both match
|
||||
- **On failure:** revert — `git checkout -- CLAUDE.md docs/scanner-reference.md scanners/lib/output.mjs scanners/scan-orchestrator.mjs package.json`
|
||||
- **Checkpoint:** `git commit -m "docs(llm-security): document TRG/SIG/AST scanners and package knowledge/"`
|
||||
- **Manifest:**
|
||||
```yaml
|
||||
manifest:
|
||||
expected_paths:
|
||||
- CLAUDE.md
|
||||
- docs/scanner-reference.md
|
||||
- scanners/lib/output.mjs
|
||||
- package.json
|
||||
min_file_count: 4
|
||||
commit_message_pattern: "^docs\\(llm-security\\): document TRG/SIG/AST scanners and package knowledge/$"
|
||||
bash_syntax_check: []
|
||||
forbidden_paths: []
|
||||
must_contain:
|
||||
- path: package.json
|
||||
pattern: "knowledge/"
|
||||
- path: docs/scanner-reference.md
|
||||
pattern: "TRG"
|
||||
```
|
||||
|
||||
### Step 8: Release v7.8.0 — version sync (OPERATOR-GATED, beyond brief scope)
|
||||
|
||||
- **Files:** `.claude-plugin/plugin.json`, `package.json`, `README.md`, `CHANGELOG.md`, `docs/version-history.md`, `CLAUDE.md`
|
||||
- **Changes:** Bump `7.7.2 → 7.8.0` in `.claude-plugin/plugin.json` and `package.json`; update the README badge (`README.md:9`); add a `[7.8.0]` CHANGELOG entry and a `docs/version-history.md` v7.8.0 section describing the three scanners; bump the `CLAUDE.md` header version. The brief's success criteria do **not** require a release — this step exists only because the global version-sync convention demands that *if* we bump, we bump everywhere. Skip this step if the operator prefers to defer the release.
|
||||
- **Reuses:** prior version-bump commit as the model (e.g. `3db6b65`, `b626ba0`).
|
||||
- **Test first:** n/a. Verification = version-consistency grep.
|
||||
- **Verify:** `npm test` then `grep -rl "7\\.7\\.2" .claude-plugin/plugin.json package.json README.md CHANGELOG.md CLAUDE.md docs/version-history.md` → expected: tests `fail 0`, grep prints nothing (exit 1 = no stale version left)
|
||||
- **On failure:** revert — `git checkout -- .claude-plugin/plugin.json package.json README.md CHANGELOG.md docs/version-history.md CLAUDE.md`
|
||||
- **Checkpoint:** `git commit -m "chore(llm-security): v7.8.0 — TRG/SIG/AST scanners"`
|
||||
- **Manifest:**
|
||||
```yaml
|
||||
manifest:
|
||||
expected_paths:
|
||||
- .claude-plugin/plugin.json
|
||||
- package.json
|
||||
- README.md
|
||||
- CHANGELOG.md
|
||||
min_file_count: 4
|
||||
commit_message_pattern: "^chore\\(llm-security\\): v7\\.8\\.0"
|
||||
bash_syntax_check: []
|
||||
forbidden_paths: []
|
||||
must_contain:
|
||||
- path: .claude-plugin/plugin.json
|
||||
pattern: "7.8.0"
|
||||
- path: README.md
|
||||
pattern: "7.8.0"
|
||||
```
|
||||
|
||||
## Alternatives Considered
|
||||
|
||||
| Approach | Pros | Cons | Why rejected |
|
||||
|----------|------|------|--------------|
|
||||
| Fold AST findings into existing `taint-tracer.mjs` (no new scanner) | Fewer files | Mixes regex + AST provenance; harder to toggle/skip; muddier reporting | Separate `AST` prefix keeps provenance and graceful-skip clean |
|
||||
| Ship a real JS/TS AST parser for #2 | Higher JS recall | Requires an npm dep or vendoring a parser — breaks the zero-dep invariant | Python-first (dep-free via `python3`); JS AST phased/deferred per brief |
|
||||
| Use YARA binary as the SIG engine | Mature rulesets | Hard dependency; matches raw bytes only (misses obfuscation) | Pure-Node engine over the decode pipeline is dep-free and catches obfuscated malware; optional `yara` shell-out can be added later |
|
||||
| Extend `permission-mapper.mjs` for trigger-abuse instead of a new scanner | One fewer file | Overloads a permission scanner with activation-surface concerns; harder to policy-gate independently | Dedicated `TRG` scanner is clearer and independently configurable |
|
||||
|
||||
## Test Strategy
|
||||
|
||||
- **Framework:** `node --test` + `node:assert/strict`. No lint/coverage gate.
|
||||
- **Existing patterns:** committed `clean/`+`poisoned/` fixture dirs (model `tests/fixtures/memory-scan/`); `resetCounter()` in `beforeEach`; assert on `result.status`, `findings.length`, `finding.scanner/severity/id/owasp/evidence`; registration tests use `assert.deepEqual` on the four OWASP maps.
|
||||
- **New tests:** 3 scanner test files (TRG, SIG, AST), each covering true-positive, false-positive (clean), obfuscation/scope edge cases, parse-only safety (AST), and orchestrator+OWASP registration.
|
||||
|
||||
### Tests to write
|
||||
|
||||
| Type | File | Verifies | Model test |
|
||||
|------|------|----------|------------|
|
||||
| Unit | `tests/scanners/trigger-scanner.test.mjs` | broad/baiting/shadow + obfuscated baiting + 4-map registration | `tests/scanners/memory-poisoning.test.mjs` + `workflow-scanner.test.mjs:196` |
|
||||
| Unit | `tests/scanners/signature-scanner.test.mjs` | webshell + base64 variant + clean + family-disable + registration | `tests/scanners/memory-poisoning.test.mjs` |
|
||||
| Unit | `tests/scanners/ast-taint-scanner.test.mjs` | creds→net via var, scope, python3-absent skip, parse-only safety, malformed-input resilience | `tests/scanners/vsix-sandbox.test.mjs` + `tests/scanners/dep.test.mjs` |
|
||||
|
||||
## Risks and Mitigations
|
||||
|
||||
| Priority | Risk | Location | Impact | Mitigation |
|
||||
|----------|------|----------|--------|------------|
|
||||
| High | AST helper executes untrusted target code | `scanners/lib/py-ast-taint.py` | RCE on scan | PARSE-ONLY invariant (`ast.parse`, never exec/eval/import); `sentinel.py` fixture + test assert no side-effect file created; 5s `spawnSync` timeout |
|
||||
| Medium | AST helper hangs or emits malformed JSON | `scanners/ast-taint-scanner.mjs` | Scan stalls/crashes | `timeout: 5000`; non-zero/timeout/unparseable → `info` note, never throw; tested with malformed `.py` |
|
||||
| Medium | SIG false positives on security tooling / docs | `scanners/signature-scanner.mjs` | Noise, eroded trust | Small high-confidence ruleset; path-exclude `knowledge/`/`tests/`/`docs/`; family toggles in policy |
|
||||
| Medium | TRG flags legitimate broad utilities | `scanners/trigger-scanner.mjs` | False positives | Default MEDIUM advisory; HIGH only on broad-name + universal-claim combo; policy allowlist |
|
||||
| Medium | python3 absent in CI/user env | `scanners/ast-taint-scanner.mjs` | AST coverage lost | Graceful `status:'skipped'`; regex `taint-tracer` still runs; both paths tested |
|
||||
| Low | Wiring edits to shared files conflict if steps reordered | `scanners/scan-orchestrator.mjs` etc. | Merge friction | Sequential execution; each wiring step isolated; `forbidden_paths` fences core steps off shared files |
|
||||
|
||||
## Assumptions
|
||||
|
||||
| # | Assumption | Why unverifiable | Impact if wrong |
|
||||
|---|-----------|-----------------|-----------------|
|
||||
| 1 | `node --test` per-file output reports `fail 0` on success | Standard node:test behavior | Verify string adjusted to the actual output marker |
|
||||
| 2 | Brief framework codes for the skills map (TRG→AST04, AST→AST02) match `OWASP_SKILLS_MAP`'s code set | Skills-map code list not fully enumerated in exploration | Adjust the specific code in Step 2/6 to a valid entry; localized, no structural impact |
|
||||
|
||||
## Verification
|
||||
|
||||
End-to-end checks crossing step boundaries (per-step manifests are checked automatically during execution):
|
||||
|
||||
- [ ] `npm test` → expected: all tests pass (`fail 0`), no regression in the existing 1820+ suite
|
||||
- [ ] `node scanners/scan-orchestrator.mjs <a sample project> --format json` → expected: JSON `results` include `trg`, `sig`, `ast` keys
|
||||
- [ ] `! grep -q '"dependencies"' package.json` → expected: exit 0 (no `dependencies` key was introduced — zero-dep invariant holds)
|
||||
- [ ] `node --test tests/scanners/ast-taint-scanner.test.mjs` with `python3` present and (simulated) absent → expected: both pass; absent path yields `status:'skipped'`
|
||||
|
||||
## Estimated Scope
|
||||
|
||||
- **Files to modify:** ~7 (`scanners/scan-orchestrator.mjs`, `scanners/lib/policy-loader.mjs`, `scanners/lib/severity.mjs`, `scanners/lib/output.mjs`, `CLAUDE.md`, `docs/scanner-reference.md`, `package.json`; +6 more in optional Step 8)
|
||||
- **Files to create:** ~11 (3 scanners, 1 python helper, 1 knowledge JSON, 3 test files, fixture dirs)
|
||||
- **Complexity:** medium
|
||||
|
||||
## Execution Strategy
|
||||
|
||||
Three independent scanner tracks, a docs/packaging wrap-up, and an operator-gated release. Core steps
|
||||
touch only new files (parallel-safe); wiring steps touch shared `scanners/scan-orchestrator.mjs` /
|
||||
`policy-loader.mjs` / `severity.mjs` and must serialize. Default execution is sequential in brief
|
||||
order (TRG → SIG → AST → wrap-up → release).
|
||||
|
||||
### Session 1: TRG scanner
|
||||
- **Steps:** 1, 2
|
||||
- **Wave:** 1
|
||||
- **Depends on:** none
|
||||
- **Scope fence:** Touch `scanners/trigger-scanner.mjs`, its test+fixtures, shared wiring files (Step 2). Never touch SIG/AST scanner files.
|
||||
|
||||
### Session 2: SIG scanner
|
||||
- **Steps:** 3, 4
|
||||
- **Wave:** 1
|
||||
- **Depends on:** none (core); wiring serializes after Session 1's wiring
|
||||
- **Scope fence:** Touch `scanners/signature-scanner.mjs`, `knowledge/signatures.json`, its test+fixtures, shared wiring files (Step 4). Never touch TRG/AST scanner files.
|
||||
|
||||
### Session 3: AST scanner
|
||||
- **Steps:** 5, 6
|
||||
- **Wave:** 1
|
||||
- **Depends on:** none (core); wiring serializes after Session 2's wiring
|
||||
- **Scope fence:** Touch `scanners/lib/py-ast-taint.py`, `scanners/ast-taint-scanner.mjs`, its test+fixtures, shared wiring files (Step 6). Never touch TRG/SIG scanner files.
|
||||
|
||||
### Session 4: Docs + packaging
|
||||
- **Steps:** 7
|
||||
- **Wave:** 2
|
||||
- **Depends on:** Sessions 1–3
|
||||
- **Scope fence:** Touch docs + `output.mjs` JSDoc + orchestrator comment + `package.json` files. Never touch scanner logic.
|
||||
|
||||
### Session 5: Release (operator-gated)
|
||||
- **Steps:** 8
|
||||
- **Wave:** 3
|
||||
- **Depends on:** Session 4
|
||||
- **Scope fence:** Version files only.
|
||||
|
||||
### Execution Order
|
||||
|
||||
- **Wave 1:** Sessions 1, 2, 3 — core steps parallel-safe; wiring steps serialize on shared files
|
||||
- **Wave 2:** Session 4 (after Wave 1)
|
||||
- **Wave 3:** Session 5 (operator-gated; optional)
|
||||
|
||||
### Grouping rules applied
|
||||
|
||||
- Steps sharing files → same session (each scanner's core + wiring)
|
||||
- Independent scanners → separate sessions (parallelizable cores)
|
||||
- Wrap-up and release depend on all scanners → later waves
|
||||
|
||||
## Plan Quality Score
|
||||
|
||||
| Dimension | Weight | Score | Notes |
|
||||
|-----------|--------|-------|-------|
|
||||
| Structural integrity | 0.15 | 88 | core-before-wiring TDD ordering; parallel-safe waves; correct file paths |
|
||||
| Step quality | 0.20 | 85 | concrete reuse citations; correct imports/arg types post-revision |
|
||||
| Coverage completeness | 0.20 | 88 | all 3 build items; four OWASP maps; packaging; brief success criteria mapped |
|
||||
| Specification quality | 0.15 | 85 | no placeholders; AST JSON contract + timeout specified |
|
||||
| Risk & pre-mortem | 0.15 | 85 | parse-only RCE, timeout, malformed-JSON, python3-absent all handled |
|
||||
| Headless readiness | 0.10 | 85 | On failure + Checkpoint per step; revert paths corrected |
|
||||
| Manifest quality | 0.05 | 85 | correct expected/forbidden paths; lenient must_contain |
|
||||
| **Weighted total** | **1.00** | **86** | **Grade: B** |
|
||||
|
||||
**Adversarial review:**
|
||||
- **Plan critic:** 3 blockers + 7 majors + 5 minors found on the first draft (score 59/D); all addressed — see Revisions.
|
||||
- **Scope guardian:** MIXED — three build items correctly scoped, non-goals respected; OWASP multi-map gap and path/import errors addressed; version bump isolated to operator-gated Step 8.
|
||||
|
||||
## Revisions
|
||||
|
||||
| # | Finding | Severity | Resolution |
|
||||
|---|---------|----------|------------|
|
||||
| 1 | Orchestrator path is `scanners/scan-orchestrator.mjs`, not repo root | blocker | All references, expected/forbidden paths, and revert commands corrected |
|
||||
| 2 | Wiring imports used `./scanners/…`; siblings import as `./…` | blocker | Imports changed to `./trigger-scanner.mjs` etc. |
|
||||
| 3 | `OWASP_MAP` values are arrays, asserts must use `deepEqual` | blocker | All entries are arrays; tests use `assert.deepEqual` |
|
||||
| 4 | Three sibling OWASP maps (AGENTIC/SKILLS/MCP) never updated | major | Steps 2/4/6 wire all four maps; brief's primary frameworks (TRG→AST04, SIG→ASI04) included |
|
||||
| 5 | `knowledge/` not in `package.json` `files` | major | Step 7 adds `knowledge/` (noted moot for git-clone distribution) |
|
||||
| 6 | Zero-dep verify checked a non-existent `dependencies` object | major | Verify changed to `! grep -q '"dependencies"' package.json` |
|
||||
| 7 | `getPolicyValue` 4th arg is `projectRoot`, not `targetPath` | major | Clarified: pass scan-root `targetPath` (where `.llm-security/policy.json` lives) |
|
||||
| 8 | README/CHANGELOG omitted from version bump | major | Folded into operator-gated Step 8 with a full version-consistency grep |
|
||||
| 9 | Step 7 manifest orphan path + ambiguous "update count" | major | Step 7 rescoped; expected_paths aligned; counts bumped by grep |
|
||||
| 10 | AST helper JSON contract / timeout / failure path unspecified | major | Step 5 defines the JSON contract, 5s timeout, and non-zero/malformed handling |
|
||||
| 11 | `parseFrontmatter` already exported (Assumption #1 pessimistic) | minor | TRG reuses `scanners/lib/yaml-frontmatter.mjs`; assumption removed |
|
||||
| 12 | finding() JSDoc prefix list, orchestrator header comment drift | minor | Updated in Step 7 |
|
||||
| 13 | `must_contain` whitespace fragility; verify orchestrator path | minor | Patterns loosened to tokens; verify uses `scanners/scan-orchestrator.mjs` |
|
||||
| 14 | Version bump is beyond brief scope | minor (scope) | Isolated into operator-gated Step 8 |
|
||||
Loading…
Add table
Add a link
Reference in a new issue