llm-security/docs/review-2026-06-20.md
Kjell Tore Guttormsen ff9bd13c6f docs: add full-depth plugin review (2026-06-20)
Grade B− — exceptionally disciplined design, but one CRITICAL zero-interaction RCE (F-1) plus two HIGH shell/path sinks. PUSH PARKED pending responsible-disclosure decision. Part of the marketplace-wide review (config-audit v5.4.0 + llm-security + structure + version). Read-only; this file is the only artifact.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Ter3E2JSi1Khgmuf2kady8
2026-06-20 09:14:11 +02:00

4.2 KiB
Raw Permalink Blame History

Plugin review — llm-security v7.7.2 (2026-06-20)

Full-depth review (part of the marketplace-wide sweep; pilot was okr). Tooling: config-audit v5.4.0 scanners (from source) + llm-security posture assessor + structure/version checks. Read-only; this file is the only artifact.

Verdict

Grade B — exceptionally disciplined design undermined by one true zero-interaction RCE in a default scanner. At the design level this is one of the most security-conscious plugins in the marketplace (untrusted targets treated as data behind an evidence-package boundary, least-privilege agents, backup/dry-run-gated mutations, inert stdin-isolated fixtures, no eval/unsafe-deser). But the scanner code does not hold itself to the standard it enforces on others: three shell/path sinks trust untrusted strings, one of them a critical RCE.

⚠️ F-1 is a CRITICAL, zero-interaction RCE, reproduced end-to-end. Treat as fix-before-anything.

Results by dimension

Dimension Result
config-audit posture A (Conflicts B 75, Feature Coverage D 57)
config-audit plugin-health 0 findings
llm-security posture (self-meta) B — see findings. Meta-note: the reviewer distinguished deliberate research fixtures/blocklists from real defects and disproved 4 false positives (no prototype-pollution, no billion-laughs, no ReDoS, fixtures inert).
structure / hygiene README ✓, CHANGELOG ✓, CLAUDE.md ✓, LICENSE ✓
version consistency OK (gate)

Findings

ID Severity Location Finding
F-1 CRITICAL scanners/git-forensics.mjs:59-66 (sinks :211,:223,:231,:564) git() runs execSync(\git ${cmd}`)(shell string) with attacker-controlled filenames from the scanned repo interpolated in.gitScanis in the **default** SCANNERS array, and/security scan clones a user-supplied repo then scans it. A hostile repo with a file namedcommands/$(touch INJECTED).mdruns **arbitrary code on the analyst's machine with no install and no confirmation** (forensics runs outside the clone sandbox). Reproduced end-to-end. **Fix:** convertgit()tospawnSync('git', [...argArray], {cwd})` — every call site already has discrete tokens.
F-2 High scanners/auto-cleaner.mjs:776,878-881 resolve(targetPath, f.file) with no containment check; f.file from findings JSON (untrusted repo filenames, or an attacker-chosen --findings file). file: "../../../.claude/settings.json" writes outside the scanned tree. Fix: assert `absPath === targetPath
F-3 High hooks/scripts/pre-install-supply-chain.mjs:254supply-chain-data.mjs:221-227 execSafe(\npm view ${spec} --json`)isexecSync(shell). A spec likefoo;touch /tmp/Xsurvives parsing and reaches the shell, firing on **PreToolUse(Bash) before** the user's npm install — so it executes pre-confirmation even if the user then denies. **Fix:**spawnSync('npm', ['view', spec, '--json'])or validate spec^[@/A-Za-z0-9._-]+$`.
F-4 Low mcp-live-inspect.mjs:296 Spawns the scanned target's declared MCP command (array-arg, no metachar injection). By-design for /security mcp-inspect, but scanning an untrusted target launches attacker-declared processes. Suggest a confirm before spawning servers from a non-home target.
F-5/F-6 Low (hygiene) repo root Two stray committed artifacts: --json (0-byte redirect husk) and .orphaned_at. Inert; git rm.

Cleared (so they aren't re-flagged): malicious fixtures (stdin-isolated to subprocess, never shell-executed), zip-extract (zip-slip + bomb guards), git-clone (hooksPath=/dev/null, protocol.file.allow=never, array args, sandbox), vsix-fetch (HTTPS host allowlist), dep-auditor (fixed strings), all 9 hooks (fail-open, advisory-only). Agents least-privilege.

Priority

The three shell/path sinks (F-1/F-2/F-3) share one root cause — trusting untrusted strings at a subprocess/filesystem sink — and all have small, localized execSync→spawnSync(array) / containment fixes. Fixing them lifts the plugin from B to a solid A. F-1 should be fixed before the next /security scan of any untrusted repo URL.