docs: add full-depth plugin review (2026-06-20)

Grade B — strong git path; one MEDIUM indirect-injection surface in the SessionStart hook. 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
This commit is contained in:
Kjell Tore Guttormsen 2026-06-20 09:14:08 +02:00
commit a6f3ad4e93

35
docs/review-2026-06-20.md Normal file
View file

@ -0,0 +1,35 @@
# Plugin review — graceful-handoff (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 — well-engineered, one real injection surface.** The git commit/push path is the
*strongest* part (no shell-string interpolation, `execFileSync` arrays, `--` pathspec guards,
hardcoded `origin`, no force-push, explicit staging allowlist, push user-gated). One indirect
prompt-injection surface in the SessionStart hook caps the grade.
## Results by dimension
| Dimension | Result |
|-----------|--------|
| config-audit posture | **A** (Hooks B, Feature Coverage F 33 — Feat-Cov expected for a focused plugin) |
| config-audit plugin-health | 1 finding: "CLAUDE.md missing hooks section" — **legitimate** (it ships a hook; document it). |
| llm-security posture | **B** — see findings. Supply-chain: zero third-party deps (Node built-ins only). |
| structure / hygiene | README ✓, CHANGELOG ✓, CLAUDE.md ✓, LICENSE ✓ |
| version consistency | **OK** (gate) |
## Findings
| ID | Severity | Location | Finding |
|----|----------|----------|---------|
| F1 | **Medium** | `hooks/scripts/session-start-load-handoff.mjs:61-80` | On resume/compact the hook walks cwd + 3 ancestors, reads the first `NEXT-SESSION-*.local.md`, and injects its **raw, uncapped** contents into `additionalContext` (wrapped in `<session-handoff>` tags but unsanitized). In a shared/multi-repo tree a planted file becomes trusted continuation instructions — indirect prompt injection. Mitigated: `.local.md` is gitignored (doesn't travel via repo); walk is bounded. **Fix:** cap injected size + document that the handoff file is treated as instructions. Closing this lifts the grade to A. |
| F2 | Low | `stop-context-monitor.mjs:55-63,127-137` | Context-threshold is a size heuristic; inflating the transcript can trip an early auto-commit, but the worst outcome is an extra artifact + a **reversible local commit** (push withheld). Lock file is unauthenticated (feature-DoS only). Completeness only. |
| F3 | Info (false-positive-prone) | `handoff-pipeline.mjs:57-77` | `execSync` with shell in read-only `gitOk()` helpers — **safe**: all command strings are static literals; the only interpolation (`${upstream}`, :72) comes from git's own ref-name output, not user input. Called out so a scanner doesn't double-count it. |
**Positive (no finding):** the git mutation path uses `execFileSync('git', [...])` (no shell), `--`
pathspec separators, hardcoded `origin` + current branch (no arbitrary remote/`--force`/`--no-verify`),
an explicit staging allowlist (never `git add -A`), and array-arg commit messages. Push never
auto-executes (Stop hook spawns `--no-push`). Correct agency boundary.