feat(plugin-quality): post-use component audit skill, auditor agent + config surface

[kyle-sexton]

Summary

Lane B of the plugin-audit-port topic (contract: docs/topics/plugin-audit-port/PLAN.md; lane A merged as #1241 — this lane consumes its frozen reader contract). Adds the plugin-quality plugin — the machine-local plugin-audit skill ported, generalized, and improved:

• /plugin-quality:audit <plugin>[:<component>] — six-step post-use behavioral audit: main-thread evidence capture into a compaction-proof packet (${CLAUDE_PLUGIN_DATA}/evidence/<session_id>/<target-slug>/<run-nonce>/, deterministic resume rule, 30-day retention) → map+ground in the fresh named auditor subagent with per-topic fresh-docs verification → blindspot + candidates → interactive contract lock (written INTO the packet) → presence-gated review seams with stated fallbacks → sink emit.
• Context-gate over context-guard snapshots (soft dependency): per-zone decision table (steps 2–3 always subagent; dumb row = summary+pointer, mandatory dispatch, immediate flush artifacts; unknown = dumb row + visible notice), re-evaluated at steps 2 and 5; zones.json read directly via jq; inlined floor values byte-identical to the merged reader contract (drift-grepped post-rebase).
• Sink ladder (tracked config → registration inference → ask+persist → local markdown) with the unconditional draft+confirm egress gate surfacing the ACTING gh account; verb-contract deviation recorded at the coupling site.
• Auditor agent — fresh-context, tools named honestly (Bash for validate/config probes; Write scoped to the evidence packet — gap surfaced by the dumb-zone smoke); standing untrusted-content posture in both hub and agent, backed by a prompt-injection anti-pattern eval.
• Reference corpus ported + generalized (recurring-concerns + 5 component-type lenses); config surface .claude/plugin-quality.md per the config-cascade convention (per-key override declared, implementers row added); setup skill (check with per-layer provenance / apply scoped to the tracked config).

Sanity-check evidence (per PLAN phases B1–B6)

• Drift check (post-rebase, vs MERGED main): snapshot path pattern, 10-minute staleness value, and 50/75 band line grep-match plugins/context-guard/reference/reader-contract.md byte-identically.
• grep 'context: fork' in the audit SKILL.md: no hits. gh issue create appears with the confirm gate in the same section.
• Forbidden-strings grep (machine paths, consumer names, dead inbox refs) across the plugin tree: empty (re-run post-rebase).
• claude plugin validate plugins/plugin-quality exit 0; claude plugin validate --strict . exit 0 (post-rebase).
• skill-quality:check PASS (0 errors) on both skills; evals validate against the bundled schema.
• Compat reconciliation recorded in reference/config.md: markdown item schema byte-matches the live handoff inbox contract fixture; work-items local-markdown adapter storage documented as divergent-by-design (seam-CLI-only emission).
• Interactive smokes (headless, recorded observables):
  • Zero-config clean repo: conservative notice printed verbatim once; unknown at steps 2 AND 5; packet with all five flush artifacts; rung 1 miss → rung 2 HIT (registration inference proposed melodic-software/claude-code-plugins) → confirm declined → rung 4 local item; no gh issue create.
  • Rung-3/4 (unregistered --plugin-dir target): rungs 1–2 fail correctly, rung 4 local item written; ${CLAUDE_PLUGIN_DATA} resolves to …/data/plugin-quality-inline/ under --plugin-dir (identity difference vs installed form noted per stress-test fix(comment-hygiene): exclude '.' from the owner/repo#N leading boundary #12).
  • Falsifiable dumb-zone: hand-crafted fresh snapshot (used_percentage 90) for the smoke session id → step 2 resolved dumb with the conservative notice ABSENT (gate did not fall through), flush artifacts present at every step boundary (evidence/findings/contract/review/item), steps 3–4 ran summary+pointer; when the snapshot honestly aged past 10 minutes mid-run, step 5 re-evaluated to unknown with the notice — correct re-evaluation observed live.

Related

• feat(context-guard): per-session context snapshots, zone resolver + setup skill #1241 — lane A (context-guard), merged; this lane inlines its frozen reader-contract floor.
• plugin-quality: operator cutover — sink override, full-item diff, retire local skill #1233 — operator cutover (post-merge, HITL; deliberately left open).
• Topic contract: docs/topics/plugin-audit-port/PLAN.md.

Closes #1237
Closes #1231
Closes #1232
Closes #1238
Closes #1239

🤖 Generated with Claude Code

[claude]

Claude finished @kyle-sexton's task in 2m 48s —— View job

---

PR Review — feat(plugin-quality): post-use component audit skill, auditor agent + config surface

Tasks

• Gather context and understand request
• Read plugin.json and manifest structure
• Review audit skill (SKILL.md)
• Review auditor agent definition
• Review reference corpus and config surface
• Review evals
• Review setup skill
• Review marketplace and doc changes
• Synthesize findings and post review

---

Review

This is a well-structured port and generalization. The hub-thin / progressive-disclosure model is cleanly applied, the security posture is thoughtful and honestly documented, and the interoperability design (context-guard soft dependency, work-items seam boundary, config-cascade conformance) is careful. The items below are correctness concerns and design gaps, roughly ordered by severity.

---

Issues

1. Run-nonce second-level collision (correctness)

skills/audit/SKILL.md:76

<run-nonce> = this run's start timestamp (YYYYMMDDTHHMMSSZ)

The stated invariant is "a same-target re-audit in one session gets its own directory instead of clobbering the first," but the nonce has second-level granularity. Two audits of the same target within the same second (e.g. a user that aborts and immediately restarts) produce the same nonce and the second overwrites the first's evidence. A sub-second suffix (milliseconds from date +%s%3N, or a short UUID from uuidgen | head -c8) would make the guarantee hold.

---

2. ${CLAUDE_SESSION_ID} unavailability — path for step 1 unspecified (correctness)

skills/audit/SKILL.md:41-43

The context-gate explicitly handles the unexpanded ${CLAUDE_SESSION_ID} case (zone = unknown). But the evidence packet path created in step 1 also uses this variable:

${CLAUDE_PLUGIN_DATA}/evidence/<session_id>/<target-slug>/<run-nonce>/

If the substitution is unavailable, the literal string ${CLAUDE_SESSION_ID} becomes part of the directory name — not a traversal risk, but: (a) the resume rule cannot reconstruct it deterministically (there's no session id to re-derive), and (b) multiple sessions with unexpanded ids would collide. The zone-gate fallback is specified; step 1's fallback for this case is not. A simple mitigation: if the substitution is unexpanded, state a fallback strategy (e.g. use a fixed sentinel like unknown-session, accept the collision risk, and note in evidence.md).

---

3. Missing dumb-zone eval (test coverage gap)

skills/audit/evals/evals.json

The 7 evals cover triggering, routing boundaries, emit shape, the conservative-dispatch anti-pattern, and prompt injection. No eval covers the dumb-zone path: the summary + packet pointer only behavior for steps 3–4, the immediate flush at every step boundary contract, or the prohibition on bulk re-reads in the main thread. This is a core differentiating feature of the skill (the "falsifiable dumb-zone" headless smoke exists, but that's manual evidence, not a repeatable eval). Suggested eval ID 8: provide a synthetic scenario with a high-used_percentage snapshot and assert the skill emits a summary-and-pointer rather than inlining findings.

---

4. command.md stub's "merged into skills" claim lacks a fresh-docs citation (minor, growable stub)

skills/audit/references/component-types/command.md:3

Note: custom commands have been merged into skills — a `.claude/commands/x.md` and a skill both produce `/x`.

The stub is explicitly "growable" but this claim is a harness-behavior assertion that the fresh-docs mandate requires a citation for (CLAUDE.md: "cite the URL"). The file contains no fetch date or doc link. Since auditors reading this lens during an audit session would be instructed to verify harness behavior claims from current docs, the claim itself may already model that correctly, but the reference file's own provenance should be consistent with the discipline it's teaching. Minor — add a comment pointing to the commands/skills doc and a fetch date, or remove the claim and leave it for the auditor to verify at use time.

---

Design Observations (no fix needed, worth noting)

repo_map merge semantics are non-standard. reference/config.md:18 — per-plugin-name merge inside a YAML mapping is documented clearly ("Repo-map entries merge per plugin name"), but a consumer who reads YAML semantics as full-mapping-replacement will be silently surprised when a user-global repo_map entry survives alongside a team-layer entry for a different plugin. The documentation is correct; this is a "expect it to confuse implementers" note for the next audit cycle.

Write scope in the auditor agent is model-enforced only. agents/auditor.md:13-16 — The security review in MIGRATION-PLAYBOOK explicitly accepts this as "under standing instructions." That's the right framing — just confirming the review's characterization aligns with what the agent actually says. It does. The honesty of naming ("neither is read-only") is exactly the right disclosure posture.

---

Strengths

• Compaction override in the context-gate (SKILL.md:53–54) is a non-obvious edge case handled precisely: a compacted session's used_percentage resets while its evidence is already gone, so a green zone post-compaction would be a false signal. Forcing dumb row here is the correct call.
• Acting-account surface at both the emit gate and setup check is excellent. A machine with multiple GitHub identity domains silently posting to the wrong repo would be a painful incident; surfacing it at both setup and emit time catches it early and late.
• Eval docs(playbook): document the local plugin development loop #7 (prompt-injection anti-pattern) covers a real, specific failure mode with concrete audited-content fixture. This is the right kind of eval — falsifiable and adversarial.
• Compat reconciliation block in reference/config.md:98-111 prevents silent format divergence between three adjacent storage shapes. Future maintainers have a documented decision rather than a mystery.
• Producer/consumer split is enforced in the skill's routing boundaries, in the agent's output contract, in eval ci: onboard to the CI platform (ci-workflows + standards) #4, and in the README's security section — four independent points of enforcement for one rule.

---

[claude]

Claude finished @kyle-sexton's task in 2m 50s —— View job

---

Security Review — PR #1243

• Read PR diff and changed files
• Analyze for injection surfaces (command, path, template)
• Check secrets/credentials handling
• Review agent trust boundaries and prompt injection posture
• Check egress gates and data exfiltration risk
• Check GitHub Actions patterns
• Report findings

---

Findings

IMPORTANT — High confidence

Auditor agent's Bash grant + "piping a fixture into a hook script" creates a pre-confirm-gate arbitrary code execution path

plugins/plugin-quality/agents/auditor.md lines 12–16:

Bash is for claude plugin validate, config-resolution probes (checking which settings scope a value comes from), and harmless empirical reproductions (piping a fixture into a hook script).

When the audit target is a malicious plugin, its hook scripts are adversarial code. The auditor's explicit permission to pipe fixtures into them means executing those scripts on the user's machine as part of step 2 ("Map + ground") — well before the step 6 draft+confirm egress gate. A hook script that receives any piped input can run arbitrary shell commands: exfiltrate environment variables, write files, or reach the network, all without user confirmation.

The MIGRATION-PLAYBOOK.md security review (added in this PR) asserts the blast radius is "bounded to the findings text itself, which the draft+confirm gate puts in front of the user before it leaves the machine." Line 857–858. This claim is false for hook script execution: harm from an adversarial hook occurs in step 2, not step 6. The confirm gate cannot retroactively prevent code that already ran.

The qualifiers "harmless" and "safe fixture" in the standing instruction are model-honored advisories, not technical enforcement. There is no mechanism preventing the agent from invoking an adversarial hook script under a well-crafted audit prompt or after a prompt injection.

Recommended fix: Remove or strictly scope the "piping a fixture into a hook script" Bash permission. Empirical hook testing belongs to a separate, explicitly consented action, not a default in the auditor's tool grant. If hook execution is needed for coverage, it should require an explicit user approval step before each invocation of an untrusted script, and the MIGRATION-PLAYBOOK blast radius assessment should be corrected.

---

IMPORTANT — High confidence

Prompt injection from audited content + Bash grant escalates from "corrupt findings" to "arbitrary code execution"

The PR correctly identifies prompt injection as an attack surface and acknowledges it is defended only by a model-honored standing instruction — "an advisory, model-honored defense, stated honestly as such" (MIGRATION-PLAYBOOK.md). That honest framing is appropriate for a skill that is read-only.

However, because the auditor agent carries a Bash grant, a successful prompt injection does not merely corrupt the findings text: it can invoke the agent's Bash tool to execute arbitrary commands. The auditor reads the full contents of the audited plugin's SKILL.md, manifests, README, and reference files into its context (auditor.md step 2), any of which can embed injection payloads.

The combination — broad untrusted input into context + Bash execution capability — means the effective blast radius is the user's entire local environment, not just the findings. The standing instruction in auditor.md lines 23–27 is the only barrier.

Eval #7 (anti-pattern-injection-in-audited-source) tests redirect of the sink, not Bash command injection through the agent — there is no eval covering the scenario where injected content causes the auditor to run a crafted shell command.

Recommended fix: Consider whether the auditor agent needs both untrusted-content ingestion AND a Bash grant simultaneously. If Bash is required, a separate confined "execution agent" that receives only a minimal, sanitized task (not the full plugin source) is a stronger containment model. Alternatively, note explicitly in the review record that the advisory-only defense applies to an agent with execution capability, not a read-only agent.

---

SUGGESTION — High confidence

Sink rung 2 — marketplace source field from an adversarial plugin is an unvalidated open redirect to an attacker-controlled issue destination

skills/audit/SKILL.md step 6, rung 2:

Infer — the audited plugin's marketplace registration names its source repo; propose it.

A malicious plugin can set its marketplace registration source to any attacker-controlled GitHub repository (e.g. attacker-org/plausible-name). When rung 2 fires, the confirm gate shows the user this repo name and asks for confirmation. A plausibly named repo (e.g. melodic-software-forks/context-guard) could pass casual review, causing the user to file findings — potentially containing sensitive behavioral observations or environment details — to an attacker's issue tracker.

There is no validation that the inferred repo is the plugin's legitimate canonical source. The repo_map override in config mitigates this only for explicitly listed plugins.

Recommended fix: When proposing an inferred repo, note its source in the confirm surface — e.g. "inferred from marketplace metadata" — so the user has a clear signal that this repo comes from the plugin's own registration, not a canonical allowlist. Optionally, cross-check the inferred repo against the marketplace's own source field in this repo's marketplace.json for plugins that originated here.

---

SUGGESTION — Medium confidence

30-day packet cleanup Bash operation on model-sanitized <target-slug> — no technical path-containment guarantee

skills/audit/SKILL.md evidence packet section:

on every new audit run, delete packet directories older than 30 days.

The <target-slug> segment in the packet path is described as sanitized to [A-Za-z0-9_-] by the model following a standing instruction. Sanitization is model-executed, not technically validated before the Bash deletion command is constructed. Edge cases (Unicode characters that normalize to ASCII sequences, very long slugs that interact with shell expansion, null bytes in some contexts) could in principle produce a path segment that escapes ${CLAUDE_PLUGIN_DATA}/evidence/ in the cleanup invocation.

The risk is bounded by ${CLAUDE_PLUGIN_DATA} being a harness-resolved variable, so drift would be limited to paths within the user's Claude data directory. Nonetheless, Bash deletion (rm -rf) on paths containing user-controlled segments that are only model-sanitized is a fragile pattern.

Recommended fix: Implement the slug sanitization as a shell sed or tr invocation that is technically enforced at the point of use, and validate that the resulting cleanup path starts with the ${CLAUDE_PLUGIN_DATA}/evidence/ prefix before issuing the delete.

---

Summary

No GitHub Actions files were changed in this PR. The two IMPORTANT findings both stem from the same root cause: the auditor agent holding a Bash grant while also ingesting arbitrary third-party plugin content. The MIGRATION-PLAYBOOK security review is thorough on the egress side (confirm gate, acting account surfacing, producer/consumer split) but the blast radius assessment for the code-execution path is incorrect. The two SUGGESTION findings are lower-severity social-engineering and path-safety improvements.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 5b3f16d66d

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[claude]

Claude finished @kyle-sexton's task in 5m 1s —— View job

---

PR Review — feat(plugin-quality): post-use component audit skill, auditor agent + config surface

Tasks

• Gather context and understand request
• Diff current branch vs origin/main
• Review key plugin files (SKILL.md, auditor.md, plugin.json, evals, config)
• Check prior review findings — addressed vs still open
• Evaluate Codex inline comments
• Synthesize and post review

---

Review

This pass reviews the full PR diff (7 commits, 943 additions) focusing on correctness, security, and alignment with the repo's design rules. The latest commit (b9856ab) correctly addresses the leaf-name collision — plugin-quality is now registered in the audit entry of skill-leaf-name-registry.txt with a sound rationale. That specific item is resolved. The items below are open concerns.

---

Issues

1. Config keys zone_behavior and repo_map are documented but not wired (correctness)

reference/config.md:43-48 documents two keys the skill never consumes:

• zone_behavior: always-conservative — The context-gate in SKILL.md:33-67 reads ${CLAUDE_SESSION_ID}, the snapshot file, and the zones.json bands. It never reads the merged config to check this key. A user who sets zone_behavior: always-conservative sees no behavioral change — the context-gate can still resolve smart or acceptable and allow inline handling, directly contradicting the documented contract ("forces the unknown/dumb row regardless of a fresh smart snapshot").

• repo_map — Step 6 rung 2 at SKILL.md:141 says "the audited plugin's marketplace registration names its source repo; propose it." The repo_map override is documented in reference/config.md as the correction mechanism for monorepo mirrors, but the SKILL.md's rung-2 description never mentions checking repo_map first. A user who configures repo_map: some-plugin: my-org/correct-repo would still see the unoverridden registration repo proposed.

These are advertised consumer-facing config promises with no enforcement path. Fix this →

---

2. ${CLAUDE_PLUGIN_DATA} resolution mechanism not documented (correctness, Codex P1)

The architecture plugin (plugins/architecture/skills/improve/SKILL.md) explicitly records: "That token does not substitute in skill markdown content (it is a hook/monitor/MCP path substitution only)."

SKILL.md:70 uses ${CLAUDE_PLUGIN_DATA} for the evidence packet path. Bash commands CAN expand this variable if the harness exports it as a shell env var (hook scripts confirm this pattern works in hook context), but:

1. The skill never instructs the model to resolve ${CLAUDE_PLUGIN_DATA} via Bash before constructing the path. If the model interprets it as a literal string in a non-Bash context, the packet silently lands somewhere wrong.
2. Whether the harness exports CLAUDE_PLUGIN_DATA during model-invoked Bash (vs. only during hook execution) requires fresh-docs verification per the CLAUDE.md mandate — this fact isn't confirmed from a fetched page this session.

The ai-briefing plugin uses the same pattern in its reference files, so precedent exists, but plugin-quality is the first audit skill here to write to ${CLAUDE_PLUGIN_DATA}. A one-line note in step 1 — "create the path via data_dir=$(echo "${CLAUDE_PLUGIN_DATA}") to ensure Bash expansion, not literal use" — would make the resolution explicit and catch a wrong-env case early. Fix this →

---

3. Run-nonce second-level collision (from prior review — still open)

SKILL.md:75: <run-nonce> = this run's start timestamp (YYYYMMDDTHHMMSSZ). Two same-target audits within one second collide. A sub-second suffix (date +%s%3N for milliseconds, or uuidgen | cut -c1-8) makes the "separate directory" invariant hold.

---

4. ${CLAUDE_SESSION_ID} unexpanded — step 1 path fallback unspecified (from prior review — still open)

SKILL.md:40-43: the context-gate handles the unexpanded-substitution case (zone = unknown). But step 1 creates the packet at ${CLAUDE_PLUGIN_DATA}/evidence/<session_id>/.... If the substitution is unavailable, the literal ${CLAUDE_SESSION_ID} string becomes part of the directory name, causing multi-session collision and breaking the resume rule. A documented fallback (e.g. unknown-session sentinel, accepted collision note) closes this gap.

---

5. Missing dumb-zone eval (from prior review — still open)

skills/audit/evals/evals.json: 7 evals, none covering the dumb-zone path. The "falsifiable dumb-zone" smoke is in the PR description as manual evidence, but the summary + packet pointer only contract, the immediate flush at every step boundary invariant, and the prohibition on bulk re-reads have no repeatable eval. This is the skill's core differentiating behavior and warrants at least one machine-checkable case.

---

6. Bash grant + hook script execution path in the auditor (security — from prior security review — still open)

agents/auditor.md:14-15: "harmless empirical reproductions (piping a fixture into a hook script)". The MIGRATION-PLAYBOOK review records the blast-radius as "bounded to the findings text, which the draft+confirm gate puts in front of the user before it leaves the machine." This assessment holds for text findings but not for hook execution: an adversarial hook script that receives piped input runs at step 2, before the step-6 egress gate. Harm (env var exfiltration, file writes, network calls) occurs locally and is not gated by the confirm surface. The qualifier "harmless" is model-honored, not technically enforced. The security review's conclusion on this path remains unaddressed. Fix this →

---

Minor

7. command.md "merged into skills" claim lacks a citation (from prior review — still open)

skills/audit/references/component-types/command.md:3: the "merged into skills" harness-behavior assertion has no fetch date or doc URL. Per the repo's fresh-docs mandate, a one-line comment with a link to the commands/skills doc and a fetch date (or removal of the unverified claim) would make this reference file consistent with the discipline it teaches.

---

What the latest commit fixed

• audit leaf-name collision properly registered in skill-leaf-name-registry.txt with a clear rationale (verb-contract conformance, deviation recorded at the coupling site). Well-argued.

Strengths

• Compaction override in the context-gate (SKILL.md:51-53) is precise: a compacted session's percentage resets while evidence is gone, so a post-compaction green zone would be a false signal. Forcing dumb row here is correct.
• Acting-account surface at both emit gate and setup check is excellent — catches identity cross-pollination at setup time and at every gh issue create.
• Compat reconciliation block (reference/config.md:98-111) prevents silent format divergence between three adjacent storage shapes.
• disable-model-invocation: true on the setup skill is correct — prevents noise triggers without explicit invocation.
• Producer/consumer split enforced at four independent points (hub, agent, eval ci: onboard to the CI platform (ci-workflows + standards) #4, README security section).

---

Summary

6 issues remain open (2 from this pass, 4 carried from prior reviews): the two unwired config keys are the highest-priority correctness gap because they silently break documented consumer-facing contracts. The security finding (#6) about hook script execution before the egress gate is the highest-priority security gap. Items #3–5 are lower-severity but straightforward to address before 1.0.

[claude]

Claude finished @kyle-sexton's task in 3m 9s —— View job

---

Security Review — PR #1243

Tasks

• Gather PR context and changed files
• Read plugin manifest and agent definitions
• Analyze Bash grant + untrusted-content ingestion surfaces
• Review egress gates and sink ladder
• Check evidence packet path construction and cleanup
• Analyze prompt injection posture and eval coverage
• Report findings

---

Findings

Security issues only; no style or coverage commentary. Findings ordered by severity.

---

CRITICAL — High Confidence

Auditor agent holds Bash + WebFetch simultaneously while ingesting arbitrary untrusted plugin content — creates an egress-gate-bypassing exfiltration path

plugins/plugin-quality/agents/auditor.md line 4:

tools: "Read, Grep, Glob, WebFetch, Bash, Write"

The auditor agent holds both Bash (shell execution) and WebFetch (arbitrary URL fetch) while ingesting the full source of the audited plugin — its SKILL.md, manifests, reference files, and README — as in-context data. The only defense against weaponizing this combination is the model-honored standing instruction at lines 21–26.

A successful prompt injection into any audited-content field could:

1. Use Bash to read local credentials (~/.ssh/, env-exported AWS_* / GITHUB_TOKEN, gh auth token, .netrc, etc.)
2. Use WebFetch to exfiltrate them to an attacker-controlled URL (e.g. as query parameters or a POST body)

This entire path runs inside the auditor agent — before and entirely outside the main skill's step-6 egress gate. The MIGRATION-PLAYBOOK.md blast-radius assessment at lines 874–876 states:

"bounds the blast radius of a hostile audited plugin to the findings text itself, which the draft+confirm gate puts in front of the user before it leaves the machine"

And at lines 871–872:

"WebFetch in the auditor agent reaches official docs pages for claim grounding — read-only GETs"

Both characterizations are incorrect as technical enforcement claims. The auditor's WebFetch is not restricted to any URL allowlist — the "official docs pages only" constraint is model-honored, the same advisory-only defense the playbook honestly identifies for Bash. An adversarial WebFetch call is not the gh issue create that the egress gate controls; it bypasses the confirm surface entirely. The actual blast radius is the user's full local environment reachable by the Bash shell.

Eval #7 (anti-pattern-injection-in-audited-source) tests sink redirection, not Bash-to-WebFetch exfiltration — there is no eval covering the scenario where injected content uses the auditor's own tools for outbound data.

Recommended fix: Bash and WebFetch must not be held simultaneously by an agent that ingests arbitrary untrusted content. Either:

• Remove WebFetch from the auditor (docs grounding moves to a separate, Bash-free read-only agent), or
• Remove Bash from the auditor (validation/config probes move to a separate agent that does not see untrusted source), or
• Split into two agents: one that runs Bash against first-party tooling without reading plugin source, and one that reads plugin source without holding Bash.

The MIGRATION-PLAYBOOK blast-radius assessment and playbook verdict must be corrected to reflect that the current arrangement places the user's local environment inside the blast radius, not just findings text.

---

IMPORTANT — High Confidence

Auditor agent explicitly permits hook-script execution before the egress gate

plugins/plugin-quality/agents/auditor.md lines 12–14:

Bash is for `claude plugin validate`, config-resolution probes (checking which settings scope a
value comes from), and harmless empirical reproductions (piping a fixture into a hook script).

When auditing a malicious plugin, its hook scripts are adversarial code. The standing instruction explicitly names "piping a fixture into a hook script" as a permitted Bash use case. This means arbitrary shell commands from a third-party hook script execute on the user's machine during step 2 (Map + ground) — before the step-6 egress gate's confirm surface ever appears. The MIGRATION-PLAYBOOK's containment claim (findings text bounded by the confirm gate) does not apply here: harm from an adversarial hook occurs at step 2, retroactively beyond what step 6 can prevent.

The qualifier "harmless" is advisory and model-enforced. No mechanism prevents the auditor from executing an adversarial hook script under a well-formed dispatch prompt or after a prompt injection that overrides the "harmless" qualifier.

Recommended fix: Remove "piping a fixture into a hook script" from the auditor's stated Bash permission. Hook-script reproduction is not a default audit operation — it requires explicit per-invocation user consent before each untrusted script execution. If empirical hook testing is required, it belongs behind a separate, explicit user approval step in the main workflow, not a standing tool grant in the fresh-context agent.

---

IMPORTANT — Medium Confidence

30-day cleanup uses rm -rf on a path containing a model-sanitized user-supplied segment with no shell-level path-prefix check

plugins/plugin-quality/skills/audit/SKILL.md lines 72–77:

<target-slug> = the <plugin>[:<component>] argument sanitized to [A-Za-z0-9_-] (every
other character → -) — the same character class the context-guard tee applies; path containment.
…
on every new audit run, delete packet directories older than 30 days.

The <target-slug> segment travels from user-supplied argument → model sanitization instruction → Bash command construction. This is model-only sanitization — technically equivalent to the advisory defenses elsewhere. No shell-level tr/sed invocation enforces the character class before the path is used in a deletion command, and no realpath / prefix-check verifies the resulting path begins with ${CLAUDE_PLUGIN_DATA}/evidence/ before the rm -rf (or equivalent) fires. Edge cases (Unicode that survives partial sanitization, adversarially long slugs causing unexpected shell tokenization, a future change to how the harness resolves ${CLAUDE_PLUGIN_DATA}) could cause deletion outside the intended subtree.

The risk is bounded by ${CLAUDE_PLUGIN_DATA} being a harness-resolved variable, so drift stays within the user's Claude data directory — but that directory contains config and other plugin data.

Recommended fix: Technically enforce slug sanitization at the Bash layer (target_slug=$(printf '%s' "$raw" | tr -cd 'A-Za-z0-9_-')), then validate the final cleanup path with an explicit prefix check before issuing the deletion ([[ "$cleanup_path" == "${CLAUDE_PLUGIN_DATA}/evidence/"* ]]).

---

SUGGESTION — High Confidence

Sink rung 2 infers the issue target from an audited plugin's own marketplace registration — an attacker-controlled field

plugins/plugin-quality/skills/audit/SKILL.md step 6, rung 2:

Infer — the audited plugin's marketplace registration names its source repo; propose it.

A malicious plugin can set its marketplace source field to any attacker-controlled GitHub repository (attacker-org/plausible-name). When rung 2 fires, the confirm surface shows this repo. A plausibly named repository could pass casual user review, causing audit findings — which include transcript path, platform details, hook failures, and sensitive behavioral observations — to be filed as an issue on the attacker's tracker. The repo_map override mitigates this only for explicitly listed plugins.

The confirm surface does not indicate how the proposed repo was derived, so the user has no signal that it comes from the audited plugin's own self-reported metadata rather than a canonical allowlist.

Recommended fix: When proposing an inferred repo, include its provenance in the confirm surface — e.g. "Target repo inferred from audited plugin's marketplace \source` field: — verify this is the plugin's canonical upstream before confirming."Optionally, cross-check the inferred repo againstmarketplace.json` for plugins registered in this marketplace.

---

Summary

No GitHub Actions files changed. All four findings stem from the same root: the auditor agent holds execution capability (Bash, Write) and network capability (WebFetch) simultaneously with arbitrary third-party content in context, defended by advisory-only standing instructions. The CRITICAL finding is that the MIGRATION-PLAYBOOK's blast-radius assessment mischaracterizes the containment boundary — the egress gate covers gh issue create, not the auditor's own Bash and WebFetch calls, which run before and independent of step 6.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b9856aba66

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[claude]

Claude finished @kyle-sexton's task in 3m 28s —— View job

---

PR Review — feat(plugin-quality): post-use component audit skill, auditor agent + config surface

Tasks

• Gather context and understand request
• Diff full branch vs origin/main
• Read latest-commit changed files (eb1e642)
• Check prior review issues — resolved vs still open
• Review new changes for correctness and security
• Synthesize and post review

---

What eb1e642 resolves

The latest commit cleanly addresses all four Codex P1/P2 findings and the work-items seam gap from the prior Claude review. Each fix lands correctly:

• ${CLAUDE_PLUGIN_DATA} substitution — SKILL.md:88-95: the "token does not substitute in skill markdown" caveat is now explicit and prominently placed, with the correct deterministic derivation (~/.claude/plugins/data/<plugin-id>/, list-then-match). Matches the architecture plugin's documented gotcha.
• Config keys wired — SKILL.md:33-45: "Config resolution (once, at invocation)" section binds all three documented keys — sink/markdown_dir to ladder rung 1, zone_behavior: always-conservative to the context-gate short-circuit, repo_map to rung-2 inference override.
• Egress gate covers work-items seam — SKILL.md:175-181: the confirm surface now explicitly covers work-items seam emission alongside gh issue create.
• YAML fence escaping — reference/config.md:29: outer four-backtick fence renders the inner ```yaml fences as real literals. Fixed.

---

Issues still open

1. MIGRATION-PLAYBOOK blast-radius assessment still incorrect (security, from prior security review)

docs/MIGRATION-PLAYBOOK.md:875-876:

"bounds the blast radius of a hostile audited plugin to the findings text itself, which the draft+confirm gate puts in front of the user before it leaves the machine"

And line 872:

"WebFetch in the auditor agent reaches official docs pages for claim grounding — read-only GETs"

Both remain unedited. The auditor agent holds Bash, WebFetch, and Write simultaneously while ingesting arbitrary third-party plugin source. A successful prompt injection (acknowledged as advisory-only defense) could use Bash to read local credentials and WebFetch to exfiltrate them to an attacker URL — both actions occur inside the agent before step 6's egress gate. The confirm gate governs gh issue create; it does not and cannot retroactively prevent agent-internal Bash/WebFetch invocations. The blast radius characterization is incorrect as a technical enforcement claim.

Additionally, auditor.md:14-15 still explicitly permits "piping a fixture into a hook script" — executing adversarial third-party shell code before the egress gate.

If this is accepted as a known risk (advisory-only posture acknowledged, not a technical control), the playbook's security review narrative should say so accurately rather than describing a stronger containment than exists. Fix this →

---

2. Run-nonce second-level collision (correctness, from prior review — still open)

SKILL.md:99:

<run-nonce> = this run's start timestamp (YYYYMMDDTHHMMSSZ)

Two same-target audits within one second produce the same nonce and the second overwrites the first's evidence packet, defeating the stated "gets its own directory instead of clobbering the first" invariant. A millisecond suffix (date +%s%3N, or uuidgen | cut -c1-8) closes this gap. Fix this →

---

3. ${CLAUDE_SESSION_ID} unavailability — step 1 path fallback still unspecified (correctness, from prior review — still open)

SKILL.md:56-57: the context-gate documents the unexpanded case (→ zone = unknown). But step 1 writes the packet to <plugin-data-dir>/evidence/<session_id>/… using this same variable. If ${CLAUDE_SESSION_ID} is unavailable, the literal string becomes part of the directory name — multiple sessions with unexpanded ids collide, and the resume rule at line 103 ("session id (${CLAUDE_SESSION_ID})") cannot re-derive the path deterministically. A one-line fallback (e.g. use a fixed sentinel like unknown-session, accept the collision risk, note it in evidence.md) closes this gap. Fix this →

---

4. Missing dumb-zone eval (test coverage, from prior review — still open)

skills/audit/evals/evals.json: 7 evals, none cover the dumb-zone path. The summary + packet pointer only contract, the immediate flush at every step boundary invariant, and the "no bulk re-read in main thread" prohibition are the skill's core differentiating behavior and have no machine-checkable case. The headless smoke in the PR description is manual evidence. A synthetic scenario with a high-used_percentage snapshot asserting pointer-only output would make this repeatable. Fix this →

---

5. rm -rf on model-sanitized <target-slug> — no shell-level enforcement (security, from prior review — still open)

SKILL.md:96-101: the <target-slug> sanitization to [A-Za-z0-9_-] is a model-executed standing instruction, not a shell tr/sed invocation. The 30-day cleanup then issues a deletion command on a path containing this user-supplied segment. No prefix-check against <plugin-data-dir>/evidence/ is documented before the rm -rf fires. Technically enforcing the sanitization at the Bash layer (target_slug=$(printf '%s' "$raw" | tr -cd 'A-Za-z0-9_-')) and validating the resulting path prefix before deletion would close this. Fix this →

---

6. command.md "merged into skills" claim lacks a citation (minor, from prior review — still open)

skills/audit/references/component-types/command.md:3: the "merged into skills" assertion is a harness-behavior claim with no fetch date or doc URL. Per the repo's fresh-docs mandate, this reference file should carry a citation for its own claim (or remove the assertion and leave verification to the auditor at use time). Fix this →

---

7. Minor: MIGRATION-PLAYBOOK still references ${CLAUDE_PLUGIN_DATA} for the evidence packet path (consistency)

docs/MIGRATION-PLAYBOOK.md:866: "Writes: the evidence packet (session-derived data…) under ${CLAUDE_PLUGIN_DATA}/evidence/…". The SKILL.md was updated in this commit to explicitly document that ${CLAUDE_PLUGIN_DATA} does NOT substitute in skill markdown — yet the playbook's security review still uses the token as if it resolves cleanly. A reader of the playbook's cache isolation entry gets the wrong picture of how the path is derived.

---

What the prior reviews marked resolved

For completeness: the four Codex findings (P1 packet path, P2 config keys consumed, P2 egress gate seam, P2 YAML fences) and the "config keys not wired" correctness finding from the second PR review are all cleanly resolved in this commit.

---

Strengths (unchanged from prior passes)

• Compaction override in the context-gate is precise and correct.
• Acting-account surface at both emit gate and setup check is excellent.
• Compat reconciliation block prevents silent format divergence across three adjacent shapes.
• Sink provenance labeling (rung 2 now explicitly says "unless the resolved repo_map carries an entry" — the override path is wired correctly).
• disable-model-invocation: true on the setup skill is correct.
• Producer/consumer split enforced at four independent points.

---

The two highest-priority open items are the MIGRATION-PLAYBOOK blast-radius correction (#1) and the ${CLAUDE_SESSION_ID} fallback (#3). Items #2, #4, #5 are each a one-line or small addition to SKILL.md. Item #6 is a doc nit. Item #7 is a consistency fix in the playbook's own review record.

[claude]

Claude finished @kyle-sexton's task in 3m 7s —— View job

---

Security Review — PR #1243

Tasks

• Gather context and understand request
• Read PR diff (current HEAD eb1e642)
• Read auditor agent definition
• Read audit SKILL.md
• Read MIGRATION-PLAYBOOK.md security section
• Analyze Bash/WebFetch grant vs untrusted content ingestion
• Check hook-script execution path
• Check evidence packet path construction and cleanup
• Check sink rung 2 open redirect surface
• Synthesize and post findings

---

Findings

Security issues only. Reviewing the current HEAD (eb1e6423). Two prior security reviews raised the same root concern each time; the response across the five commits in this PR addressed config wiring, path resolution, and seam gating but left the agent trust surface unchanged. This review verifies the current state of each finding.

---

CRITICAL — High Confidence

Auditor agent holds Bash + WebFetch simultaneously with untrusted plugin source in context; MIGRATION-PLAYBOOK blast-radius characterization remains incorrect

plugins/plugin-quality/agents/auditor.md line 4:

tools: "Read, Grep, Glob, WebFetch, Bash, Write"

plugins/plugin-quality/agents/auditor.md lines 36–41 — step 2 (Map the component) reads the audited plugin's manifest, SKILL.md, config surfaces, and README into the agent's context. The untrusted-content posture at lines 21–26 is a model-honored standing instruction — the only barrier between in-context adversarial content and tool calls.

docs/MIGRATION-PLAYBOOK.md lines 874–876:

"bounds the blast radius of a hostile audited plugin to the findings text itself, which the draft+confirm gate puts in front of the user before it leaves the machine"

docs/MIGRATION-PLAYBOOK.md lines 871–872:

"WebFetch in the auditor agent reaches official docs pages for claim grounding — read-only GETs"

Both characterizations are incorrect as technical enforcement claims. The step-6 egress gate (now correctly covering both gh issue create and the work-items seam, per eb1e642) controls what leaves the machine via the skill's explicit emit actions. It does not gate the auditor agent's own tool calls — those run during step 2 dispatch, before and independent of step 6.

A successful prompt injection into any audited-content field could use the auditor's Bash tool to read local credentials (gh auth token, ~/.ssh/, AWS_* env vars, .netrc) and its WebFetch tool to exfiltrate them to an attacker-controlled URL — entirely outside the confirm surface. The WebFetch tool accepts any URL; the "official docs pages only" constraint is the same advisory-model-honored defense named for Bash, not a URL allowlist.

Eval #7 (anti-pattern-injection-in-audited-source) tests sink redirection — whether the model obeys the instruction to redirect gh issue create. It does not test Bash-to-WebFetch exfiltration, where the injected content uses the agent's own granted tools to send data outbound, bypassing the step-6 gate entirely.

The MIGRATION-PLAYBOOK verdict at line 879 states "1 bounded to an honestly-named agent Bash grant under standing instructions" — this framing accepts the advisory defense for Bash alone, but the combination with WebFetch was not the condition evaluated. The playbook's own standard for data egress (surface 5) is "confirm-gated egress plus docs-only WebFetch," but docs-only WebFetch is advisory.

Recommended fix: Bash and WebFetch must not be held simultaneously by an agent that ingests arbitrary untrusted content. Structural options (pick one):

• Remove WebFetch from the auditor; docs-grounding moves to a separate read-only, Bash-free claim-verification step.
• Remove Bash from the auditor; claude plugin validate and config probes move to a separate agent that does not read plugin source.
• Split into two agents: one for first-party tooling (Bash, no plugin source in context), one for reading and grounding plugin source (no Bash, no WebFetch, or WebFetch with no local credential access).

The MIGRATION-PLAYBOOK blast-radius assessment and the egress-gating claim (lines 871–876) must be corrected regardless: the confirm gate does not bound the auditor's own tool execution.

---

IMPORTANT — High Confidence

Auditor Bash grant explicitly permits hook-script execution; adversarial hooks run before the step-6 egress gate

plugins/plugin-quality/agents/auditor.md lines 12–15:

Bash is for `claude plugin validate`, config-resolution probes (checking which settings scope a
value comes from), and harmless empirical reproductions (piping a fixture into a hook script).

When the audited plugin is malicious, its hook scripts are adversarial code. The standing instruction names "piping a fixture into a hook script" as a permitted Bash use case. Hook script execution runs during step 2 (Map + ground), before the step-6 egress gate. The word "harmless" is a model-honored qualifier — no mechanism prevents the auditor from executing an adversarial hook under a well-formed dispatch prompt or after a prompt injection that overrides the qualifier.

An adversarial hook script receiving piped input can run arbitrary shell commands — exfiltrate environment variables, write files, reach the network — all before the confirm surface appears and beyond what it can retroactively prevent.

This is distinct from and compounds the CRITICAL finding: even without a prompt injection, the skill's standing instruction creates a designed-in code-execution path for untrusted code.

Recommended fix: Remove "piping a fixture into a hook script" from the auditor's stated Bash permission. Empirical hook-script reproduction is not a default audit operation; it requires explicit per-invocation user consent and should live behind an explicit approval step in the main workflow, not a standing tool grant in the fresh-context agent.

---

IMPORTANT — Medium Confidence

30-day packet cleanup uses model-sanitized <target-slug> in a deletion path with no shell-level prefix check

plugins/plugin-quality/skills/audit/SKILL.md lines 96–101:

<target-slug> = the <plugin>[:<component>] argument sanitized to [A-Za-z0-9_-] (every
other character → -) — the same character class the context-guard tee applies; path containment.
…
Retention: on every new audit run, delete packet directories older than 30 days.

The <target-slug> segment travels from user input → model-executed sanitization instruction → path construction → deletion. No shell-level tr/sed call enforces the character class at the point of use, and no realpath / prefix assertion verifies the cleanup path stays inside ~/.claude/plugins/data/<plugin-id>/evidence/ before deletion fires.

eb1e642 correctly replaced ${CLAUDE_PLUGIN_DATA} (non-substituting in skill markdown) with a deterministically derived ~/.claude/plugins/data/<plugin-id>/ root, which tightens the base. But the slug-containment claim ("path containment") is still model-only. Edge cases — Unicode input that survives partial normalization, adversarially long slugs that interact with shell expansion — could in principle cause deletion of adjacent directories within the Claude data directory (config, other plugin data).

Recommended fix: Technically enforce slug sanitization at the Bash layer (target_slug=$(printf '%s' "$raw" | tr -cd 'A-Za-z0-9_-')), then validate the cleanup path with an explicit prefix check before deletion ([[ "$cleanup_path" == "$HOME/.claude/plugins/data/"* ]]).

---

SUGGESTION — High Confidence

Sink rung 2 confirm surface does not disclose that the proposed repo originates from the audited plugin's own marketplace registration

plugins/plugin-quality/skills/audit/SKILL.md lines 167–169:

Infer — the audited plugin's marketplace registration names its source repo, unless the
resolved repo_map carries an entry for this plugin — the mapped owner/repo wins; propose
the result.

eb1e642 correctly adds the repo_map override to rung 2. However, the egress gate description at lines 175–183 describes showing "the destination (target repo, tracker, or directory)" — it does not specify disclosing how the destination was derived (config vs. inference vs. asked). A rung-2 inferred repo that came from the audited plugin's own self-reported source field in marketplace metadata can be set to any attacker-controlled repository. A plausibly named repo (melodic-software-forks/context-guard, acme-corp/triage) could pass casual review; the user has no signal from the confirm surface that the destination originated from the plugin's own registration rather than an operator-controlled allowlist.

Recommended fix: When the rung-2 path fires, the confirm surface should name the derivation source — e.g. Destination inferred from audited plugin's marketplace \source` field: ` — so the user can weigh its trustworthiness. A one-line addition to the egress gate description for rung-2 emits is sufficient.

---

What has been addressed since the initial commit

• ${CLAUDE_PLUGIN_DATA} substitution (eb1e642): The evidence packet path now derives the plugin-data dir as ~/.claude/plugins/data/<plugin-id>/ rather than relying on the non-substituting ${CLAUDE_PLUGIN_DATA} token. Correctly fixed.
• Unwired config keys (eb1e642): zone_behavior: always-conservative, repo_map, sink, and markdown_dir are now explicitly consumed in the skill. Correctly fixed.
• work-items seam not behind the gate (eb1e642): The presence-gated work-items:create-item emit now sits behind the same full-draft + destination + acting-identity confirm as gh issue create. Correctly fixed.
• Zero-width-prefixed YAML fences (eb1e642): Config example now uses outer four-backtick fencing. Correctly fixed.

---

Summary

The two CRITICAL+IMPORTANT findings share a root: the auditor agent holds execution capability (Bash) and network capability (WebFetch) simultaneously with arbitrary third-party plugin content in context, defended by advisory-only standing instructions. The MIGRATION-PLAYBOOK's security review verdict (ACCEPT) rests on a blast-radius characterization that is incorrect for the agent's own tool calls, which run outside and before the step-6 egress gate. Three rounds of review have raised this without a structural resolution. The SUGGESTION is a social-engineering vector through the rung-2 confirm surface. The IMPORTANT cleanup finding is lower-severity but represents the same pattern (model-only sanitization before a destructive shell operation).