diff --git a/docs/review-2026-06-20.md b/docs/review-2026-06-20.md new file mode 100644 index 0000000..24d0ff7 --- /dev/null +++ b/docs/review-2026-06-20.md @@ -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 `` 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.