fix(plugins): pair seven allowed-tools grants with the invocations they must match - #2225
Conversation
… it must match
Both skills granted `Bash(bash ${CLAUDE_PLUGIN_ROOT}/scripts/detect-ecosystems.sh:*)`.
`${CLAUDE_PLUGIN_ROOT}` is not substituted in `allowed-tools` — only
`${CLAUDE_SKILL_DIR}` and `${CLAUDE_PROJECT_DIR}` are — so the rule stayed a
literal string, never matched, and the ecosystem pre-compute fell through to a
prompt or the classifier on every invocation.
Dropping `bash` from the rule, the repair that suggests itself, would have made
the grant dead rather than working: `bash` is not among the wrappers Claude Code
strips before matching. The change is paired — the body invokes the script
directly and unquoted, and the rule names that same string.
`${CLAUDE_SKILL_DIR}` resolves to the skill's own subdirectory, so each skill
gets a self-locating entry point under `skills/<skill>/scripts/` that execs the
still-single-sourced detector at the plugin root.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ions
`tidy` granted `Bash(bash ${CLAUDE_PLUGIN_ROOT}/skills/tidy/scripts/open-pr-count.sh:*)`,
which never matched — `${CLAUDE_PLUGIN_ROOT}` is not substituted in
`allowed-tools`. `audit-comment-residue`'s `Bash(bash *audit-comment-residue/scripts/detect.sh*)`
did match, but only because its wildcards absorbed the `bash` wrapper and the
quotes around the body's path; that is the wildcarded-interpreter shape auto mode
drops, and it matches the wrapper name at any path.
Dropping `bash` from the rules would have made the first grant dead and
regressed the second from working to broken: `bash` is not among the wrappers
Claude Code strips before matching. Both changes are paired — the bodies invoke
their scripts directly and unquoted, and the rules name those same strings.
Both skills also now grant the read-only commands their pre-computes pipe
through; a rule must match each subcommand of a compound command independently,
so a script grant alone left the pipeline uncovered.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`Bash(bash *audit-noise/scripts/detect.sh*)` matched only because its leading and trailing wildcards absorbed the `bash` wrapper and the quotes around the body's path. That is the wildcarded-interpreter shape auto mode drops outright, and a rule anchored on a bare wrapper name matches it at any path. Dropping `bash` from the rule would have been a straight regression from a working grant to a broken one — `bash` is not among the wrappers Claude Code strips before matching, and removing the wildcards without unquoting the body breaks the match a second way. The change is paired: the body invokes the script directly and unquoted, and the rule names that same string, narrow and anchored to this skill's own directory. The skill also now grants the read-only commands its pre-compute pipes through. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… read-only rules
`Bash(bash ${CLAUDE_PLUGIN_ROOT}/skills/clean/scripts/*)` never matched:
`${CLAUDE_PLUGIN_ROOT}` is not substituted in `allowed-tools`. Had it matched it
would have pre-approved every script in the directory behind one
wildcarded-interpreter rule — including `git-tree-reset.sh` and `remove-path.sh`.
Dropping `bash` from the rule would have produced a dead grant rather than a
working one, since `bash` is not among the wrappers Claude Code strips before
matching. The change is paired: `SKILL.md` and every bundled `context/*.md` now
invoke their scripts directly through `${CLAUDE_SKILL_DIR}/scripts/…`, and the
rules name those same strings.
The grant is now five narrow rules covering the read-only scripts only. The
mutating scripts stay unapproved and keep routing through the PreToolUse
destructive guard and the permission flow. Nothing regresses — the old rule
matched nothing. The guard hook's own command still resolves
`${CLAUDE_PLUGIN_ROOT}` and is unchanged; hook commands are a different
substitution context where that variable is documented to work.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…t grant dead #1798 corrected this rule's variable half and explicitly parked quoting as "Unverified, not asserted." The shipped rule wrote the path unquoted while the body ran `bash "${CLAUDE_SKILL_DIR}/scripts/audit-fleet.sh"` with it quoted. A Bash rule matches the literal command string, so the character after the wrapper name is a closing quote where the rule expects a path: the grant has never matched since that fix landed. #1798's remaining advice — drop the `bash` prefix only — would not have fixed it either. `bash` is not among the wrappers Claude Code strips before matching, so a rule without it stops matching a body that still says `bash <path>`, and dropping the prefix addresses nothing about the quoting. The change is paired on both axes: the body invokes the script directly and unquoted, and the rule names that exact string. Passed arguments stay individually quoted; it is a prefix rule, so their quoting does not affect the match. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
|
Claude finished @kyle-sexton's task in 3m 56s —— View job Security review — PR #2225Reviewed the 30 changed files ( Verified safe / as-claimed:
Finding (worth a second look, not blocking): [IMPORTANT / plausible confidence] Three skills gain a brand-new, fully unscoped
Before this PR these skills had no grant covering This is a real widening (not present pre-PR) and, unlike the No other issues found. No command/path injection, no credential handling, no unsafe |
|
Claude finished @kyle-sexton's task in 4m 11s —— View job Review complete
This lane covers correctness, design, conventions, error handling, tests, and maintainability; security findings are left to the dedicated 🔴 Important — new wrapper scripts committed without the executable bitPosted as inline comments on I independently confirmed via This also appears to contradict the plugin's own new Fix is mechanical: Everything else checked out
|
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
|
Claude has reviewed this PR 4 times. The lane skips further automatic reviews after 5; deleting this comment resets the count. |
`core.filemode` is false in this checkout, so `chmod +x` never reached the
index and the two new prototype wrappers plus the five pairing gates went in as
100644. That is not cosmetic here: the whole point of the paired rewrite is that
the body invokes the bundled script directly, which requires the exec bit to
survive the plugin-cache install. CI's exec-bit gate caught it, and so did the
new pairing gate's own "granted script exists and is executable" assertion —
which is what that assertion is for.
Also silences SC2016 file-wide in the pairing gate. Every single-quoted `${…}`
in it is a fixed string searched for verbatim in markdown and frontmatter, where
those placeholders are substituted by Claude Code at load time; letting the
shell expand any of them would make the gate silently match nothing.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Claude finished @kyle-sexton's task in 3m 21s —— View job Security review — PR #2225 (re-review at latest HEAD
|
…ired grant
Surface 4 asserted that bundled assets are addressed through
`${CLAUDE_PLUGIN_ROOT}`, which the pairing fix falsified — the grant and the
documented invocation now both resolve through `${CLAUDE_SKILL_DIR}`. Adds the
2026-08-11 re-check line recording that the change admits no new execution,
network, or config surface: the previous rule never matched, so this makes an
already user-invoked script prompt-free rather than granting anything new.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Claude finished @kyle-sexton's task in 43s —— View job Security review — PR #2225 (re-review at HEAD
|
|
Claude finished @kyle-sexton's task in 5m 52s —— View job Code review — PR #2225 (re-review at latest HEAD
|
…lowlist
The pairing checks validate whatever IS granted; they could not catch the grant
set being re-widened. Every mutating script under `clean` is bundled,
executable, and invoked in the skill's markdown, so a rule added for
`clean-caches.sh` or `git-tree-reset.sh` would "pair" correctly and pass every
other assertion green — silently undoing the read-only-only boundary this
change just carved out and that the PR's security note stakes posture on.
Adds an optional per-skill allowlist; `clean` names its five read-only scripts
and the granted set must equal it exactly. The other four plugins have no
narrowing decision, so their allowlist is empty and the check is skipped.
Verified by injecting `Bash(${CLAUDE_SKILL_DIR}/scripts/clean-caches.sh:*)` into
the frontmatter: every other check still passed, and the allowlist failed with
the expected/actual diff.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Claude finished @kyle-sexton's task in 45s —— View job Security review — PR #2225 (re-review at latest HEAD
|
|
Claude finished @kyle-sexton's task in 1m 39s —— View job Code review — PR #2225 (re-review at latest HEAD
|
Converting the bundled context/*.md to ${CLAUDE_SKILL_DIR} assumed that
substitution reaches a bundled non-SKILL.md file. The skills docs scope it to
"the skill's markdown content" and do not resolve whether that includes a
context file loaded on demand; ${CLAUDE_PLUGIN_ROOT}'s documented scope is
broader ("anywhere the placeholder appears"), so the two are not interchangeable
here just because both work in SKILL.md.
If the assumption is wrong the body emits a literal the substituted rule cannot
match. That fails safe — a prompt, never a wrong action — but silently, which is
the defect class this PR exists to remove. Not a trade worth making inside a
change whose thesis is "make the grant actually match the invocation."
The grant fix is unaffected: all five granted scripts are invoked from SKILL.md,
so the pairing is complete for everything the grant covers, and the pairing gate
still passes. CHANGELOG corrected to describe the SKILL.md-only scope and the
reason. #2237 tracks the rest with the empirical test that settles it, and
cross-links #1824 as the neighbouring open question on the same variable.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Claude finished @kyle-sexton's task in 1m 44s —— View job Security review — PR #2225 (re-review at latest HEAD
|
|
Claude finished @kyle-sexton's task in 2m 9s —— View job Code review — PR #2225 (re-review at latest HEAD
|
…uence resume before discard, and un-invert the research hub (#2311) ## Summary Six filed issues against `discovery`, one plugin, one release: **0.14.0 → 0.15.0**. Every one is the same shape — an obligation or an explanation carried in several files that drifted apart, or a harness behavior asserted without a source. **#2270 — the `$ARGUMENTS`-on-preload claim, retired rather than inverted.** Verified at HEAD before touching it: the empty-string rendering was asserted at the 5 core sites the issue names, weakly at `explore/SKILL.md:161`, **and at four more the issue did not count** — `skills/explore/evals/evals.json:69,73` and `skills/research/evals/evals.json:124,128`, where it had become a *grading criterion*. 12 grep hits in total. Re-checked against raw markdown (not a summarized fetch) on 2026-08-11: the skills page scopes the placeholder to "All arguments passed **when invoking** the skill" and says preload "work[s] differently: the full skill content is injected at startup"; the sub-agents page says only "The full content of each listed skill is injected into the subagent's context at startup". **Neither covers argument substitution on that path in either direction.** So this ships the rule that holds whichever way the harness renders it — *do not rely on seeing an unfilled slot; a topic that did not arrive in the dispatch prompt is a parent-envelope failure the agent reports rather than repairs* — and records the doc status once. Recorded as **unsupported, not false**; the opposite is not asserted either. The `${CLAUDE_…}`-caller caveat #2222 separated from this is kept separate. Same issue's `F6`: three files stated three different write boundaries. Reconciled to one statement in `reference/topic-docs.md` with a `scratch-` naming prefix and a cleanup owner. The researcher's **session** scratch dir is kept as a distinct, harness-owned place rather than merged — they were never the same location. `reference/artifact-protocol.md` is byte-identical across four plugins and is not touched. **#2272 — a contradiction, resolved by sequencing.** 5 sites, not 3: the three discard statements plus `explore/reference/dispatch.md:133`, which back-referenced the discard rule to justify its own. **Resume first; decide about the slice from what the resume returns**, sourced against the sub-agents page ("Resumed subagents retain their full conversation history … picks up exactly where it stopped"). The discard is not removed — it is sequenced, and stays mandatory once the resume is refused. `truncated` still means the turn-budget stop, so #2203's `persistence:` axis is not reopened. **#2268 — envelope delivery.** `Memory root` becomes a row in the parent-obligation table and the envelope becomes one labelled template, reproduced from `research-deep`'s existing literal block so the two cannot drift. Memory root is the one *degradable* field (derive + flag, not halt). Per the issue's constraint, **write capability is not asserted as a flag**: it is not probeable pre-dispatch, the parent's own `mkdir`/`touch` proves only the parent, and an agent-side probe would corrupt the freshness baseline — the question is routed to `persistence: by-value`, which already answers it. **#2269 — portability.** The baseline command now has one home carrying POSIX **and** PowerShell forms; the two monorepo pointers become `${CLAUDE_PLUGIN_ROOT}` form. **#2267 — the gate's grants and step 1.** `allowed-tools` pairing **does not apply here**, on three sourced legs from the skills page (raw markdown, 2026-08-11): `${CLAUDE_PLUGIN_ROOT}` is not substituted in `allowed-tools` rules (inert grant); `${CLAUDE_SKILL_DIR}` is "the skill's subdirectory within the plugin, **not the plugin root**", where these scripts live because one gate serves both families; and the grant "clears when you send your next message" while the parent runs the gate a turn later. `bash` is not in the docs' stripped-wrapper list, so a covering rule would be interpreter-led — this repo's `permission-rule-hygiene` anti-pattern 1. Following #2225's precedent means shipping the **rationale plus a test**, not an inert grant: both skills now state *a gate that could not run is a FAIL, never a skip*, and the operator-setup path the docs prescribe is recorded. `explorer` `maxTurns` 30 → 40 **on parity grounds only** — explicitly not offered as the cause of any past bare-prose return, since `evidence-2.md` supersedes that reading. "Budget a turn for the payload" is replaced by emitting the payload early, because an agent cannot observe its own remaining budget. **#2271 — the inversion, re-measured at HEAD and reduced.** `research/SKILL.md` was **6,629 words vs `context/discipline.md`'s 5,087** (confirmed unchanged since `9b34a82a`). This PR **shrinks** it: **6,629 → 5,026 words, 242 → 232 lines**, now 210 words *below* the spoke. The two densest lines moved rather than compressed — the fetch-log spec to `context/artifact-shape.md`, criterion 9's elaboration into `context/discipline.md`'s existing artifact-ladder section. The description gains its missing `research-deep` boundary clause. ### New surface `reference/parent-contract.md` — the parent's **cross-family** contract. The plugin had two family-specific parent-side spokes and no home for what is identical across both; that absence is why five statements existed in 2–6 copies each. Each existing spoke keeps its family-specific half and points here. ### Not fixed, deliberately **#2267 `B-F8`** (three consecutive releases fixed assertions that could not fail) is left unchecked. It is a process observation about past releases, not a defect at HEAD, and inventing a mechanism for it would be the silently-checked row this batch is named for. The repo already owns this class in `scripts/check-discriminating-test-skips.sh`. It did, however, catch this PR's own test. A first revision asserted that each agent *"points at"* the write boundary — which passed at the merge-base too, because both agents already linked `topic-docs.md` for an unrelated reason. Two vacuous `ok`s, inside a test written to pin non-vacuity. Those assertions now key on the restatements being **gone**. ### Two corrections made to this PR's own claims Recorded because the rest of the batch is about exactly this. 1. **A false mechanism in the first CHANGELOG draft.** It said criterion 9's elaboration was "folded into `context/discipline.md`'s existing artifact-ladder section". `git diff --numstat` says `discipline.md` gained **2 lines** — the Tier-3 exception, nothing else. What actually happened is that criterion 9's cell was **compressed to a pointer at text `discipline.md` already carried** ("A probe locates a rung; it does not grade one", the exhaustive-surface rule, the `unresolved` default, all in "Primary-source-first protocol" before this PR): the hub was restating a spoke rather than owning anything. Three distinct operations — one move to `artifact-shape.md` (+644 words), one move to `discipline.md` (+149), one compress-to-existing — are now stated as three. 2. **A count that was not executed.** "12 grep hits" was arithmetic, not output. The union over all five patterns at `a0abaf81` is **10 lines across 7 files**, and one of those patterns (`reaches a preloaded skill`, in both `evals.json` grading criteria) had no assertion pinning it. It has one now. Two pointers created in the trimming pass were also grep-verified against their targets rather than assumed: `discipline.md`'s "Recency gate" does carry the stable-project carve-out and the 30/14/90 windows, and "Corpus enumeration" does carry both the exhaustive-surface table and the criterion-drift reasoning. ### Explore/research parity The trimming pass initially hit `research/SKILL.md` only, which left three verbatim-shared gate sentences compressed on one side and not the other. This plugin's 0.12.0 rationale treats that parity as a value (#2268 cites it), so the same compressions were applied to `explore/SKILL.md`: **4,154 → 3,977 words.** `explore/SKILL.md` is *also* hub-inverted against its own 2,007-word `reference/dispatch.md`; that is unfiled and out of this PR's scope, and is reported rather than fixed. ## Test plan New: `plugins/discovery/scripts/contract.test.sh` — 25 assertions, discovered automatically by `scripts/run-plugin-tests.sh`, which globs `plugins/**/*.test.sh`. **Fail-before — the shipped test run against a detached worktree at the merge-base `a0abaf81`. 23 of 25 fail:** ```console $ bash plugins/discovery/scripts/contract.test.sh FAIL - no file asserts $ARGUMENTS substitutes to the empty string — 3 hit(s) FAIL - no file asserts $ARGUMENTS reaches a preloaded body as the empty string — 2 hit(s) FAIL - no evals entry grades against "empty under preload" — 2 hit(s) FAIL - no file asserts $ARGUMENTS is empty under dispatch — 1 hit(s) FAIL - no evals criterion asserts what does or does not reach a preloaded skill — 4 hit(s) FAIL - no monorepo-path pointer to the agent definitions — 2 hit(s) FAIL - the pre-dispatch baseline command has exactly one home — found in: <6 files> FAIL - the baseline home states a PowerShell form — no match for /New-Item/ in reference/parent-contract.md FAIL - no file prescribes discard-instead-of-resume — 3 hit(s) FAIL - no file back-references a discard-rather-than-resume rule — 1 hit(s) FAIL - the ordering is stated once, in the parent contract — no match for /Resume first/ in reference/parent-contract.md FAIL - the research parent-obligation table carries a Memory root row — no match for /^\| Memory root \|/ in skills/research/context/dispatch.md FAIL - the parent contract ships a literal envelope template — no match for /Memory root:/ in reference/parent-contract.md ok - no Bash permission rule is written with the non-substituting ${CLAUDE_PLUGIN_ROOT} ok - neither skill declares allowed-tools (the un-run case is stated instead) FAIL - the un-run case is stated — no match for /could not run/ in reference/parent-contract.md FAIL - explorer maxTurns (30) >= researcher maxTurns (40) FAIL - research/SKILL.md (6629 words) is NOT smaller than context/discipline.md (5087 words) FAIL - the research description carries a boundary against research-deep — no match for /^description:.*research-deep/ in skills/research/SKILL.md FAIL - the write boundary names a scratch prefix — no match for /scratch-/ in reference/topic-docs.md FAIL - the write boundary assigns a cleanup owner — no match for /[Cc]leanup/ in reference/topic-docs.md FAIL - no agent restates the write boundary as a closed two-destination list — 1 hit(s) FAIL - no agent restates the write boundary as a single destination — 1 hit(s) FAIL - agents/explorer.md defers to the single write boundary — no match for /single write boundary/ in agents/explorer.md FAIL - agents/researcher.md defers to the single write boundary — no match for /single write boundary/ in agents/researcher.md 23 contract assertion(s) failed. ``` `<6 files>` is elided for width; the real output names `skills/explore/evals/evals.json`, `skills/explore/reference/dispatch.md`, `skills/explore/SKILL.md`, `skills/research/evals/evals.json`, `skills/research/SKILL.md`, `skills/research-deep/SKILL.md`. The two `ok`s at the merge-base are the deliberate **no-grant guards** — they pin a decision (this plugin ships no `allowed-tools` rule, and no rule is written with the non-substituting `${CLAUDE_PLUGIN_ROOT}`) rather than a fix, so they correctly hold on both sides. **Pass-after, at the tip — all 25:** ```console $ bash plugins/discovery/scripts/contract.test.sh ok - no file asserts $ARGUMENTS substitutes to the empty string ok - no file asserts $ARGUMENTS reaches a preloaded body as the empty string ok - no evals entry grades against "empty under preload" ok - no file asserts $ARGUMENTS is empty under dispatch ok - no evals criterion asserts what does or does not reach a preloaded skill ok - no monorepo-path pointer to the agent definitions ok - the pre-dispatch baseline command has exactly one home ok - the baseline home states a PowerShell form ok - no file prescribes discard-instead-of-resume ok - no file back-references a discard-rather-than-resume rule ok - the ordering is stated once, in the parent contract ok - the research parent-obligation table carries a Memory root row ok - the parent contract ships a literal envelope template ok - no Bash permission rule is written with the non-substituting ${CLAUDE_PLUGIN_ROOT} ok - neither skill declares allowed-tools (the un-run case is stated instead) ok - the un-run case is stated ok - explorer maxTurns (40) >= researcher maxTurns (40) ok - research/SKILL.md (5026 words) is smaller than context/discipline.md (5236 words) ok - the research description carries a boundary against research-deep ok - the write boundary names a scratch prefix ok - the write boundary assigns a cleanup owner ok - no agent restates the write boundary as a closed two-destination list ok - no agent restates the write boundary as a single destination ok - agents/explorer.md defers to the single write boundary ok - agents/researcher.md defers to the single write boundary All contract assertions passed. EXIT=0 ``` **No regression in the two existing suites:** ```console $ bash plugins/discovery/scripts/check-dispatch-artifact.test.sh # exit 0 all tests passed $ bash plugins/discovery/scripts/check-coverage-complete.test.sh # exit 0 All checks passed. ``` **Repo gates run locally:** ```console $ bash scripts/check-changelog-parity.sh --check Every versioned plugin has a CHANGELOG.md (or a stale-guarded baseline entry), and none documents a version above its manifest. $ bash scripts/check-changelog-parity.sh --check-order All 75 changelog(s) read newest-first with no duplicate versions. $ bash scripts/check-changelog-parity.sh --check-bump origin/main Every plugin whose version changed vs origin/main has a '## [<version>]' CHANGELOG.md entry. $ npx markdownlint-cli2 --config .markdownlint-cli2.jsonc "plugins/discovery/**/*.md" Linting: 19 files Summary: 0 issues in 0 files $ bash plugins/skill-quality/scripts/check-listing-budget.sh plugins/discovery/skills CHECK-LISTING-BUDGET: OK — aggregate 2739/8000 chars within budget. ``` Both `evals.json` files re-parsed with `json.load` after the wording sweep — valid. ## Related Closes #2267 Closes #2268 Closes #2269 Closes #2270 Closes #2271 Closes #2272 Inbox items: `20260810-225904-discovery-dispatch-persistence-contract`, `2026-08-10-plugin-quality-audit-four-components` (023241Z), `20260811-021645-plugin-audit-four-components-and-guard-deadlock-ownership`. Ledgers: `.work/handoff-inbox-batch-4/ledgers/I1-discovery-dispatch.md`, `ledgers/I7-four-components-023241Z.md`, `ledgers/I9-021645-four-components.md`. Adjacent, not reopened: #2203 / #2222 (the `persistence:` axis), #2225 (the paired body+rule precedent this follows), #2237 / #1824 (`${CLAUDE_SKILL_DIR}` substitution scope), #1414 (no Windows runner, which is why #2269's class survives green checks). --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: Kyle Sexton <kyle-sexton@users.noreply.github.com>
1 participant
Summary
Seven skill
allowed-toolsgrants across five plugins were interpreter-led (Bash(bash <path>…)),and four of them named
${CLAUDE_PLUGIN_ROOT}— which is not substituted inallowed-tools, sothose rules stayed literal strings that never matched. One more (
repo-fleet-hygiene) was dead froman unfiled quote mismatch, and two (
code-tidying/audit-comment-residue,docs-hygiene/audit-noise)worked only because leading/trailing wildcards absorbed both the
bashwrapper and the body'squotes — the wildcarded-interpreter shape auto mode drops.
The corrected mechanism (the filed rationale was falsified)
The originating item prescribed "drop
bashfrom the rule." That is wrong.bashis not one ofthe wrappers Claude Code strips before matching a Bash rule — the stripped set is
timeout,time,nice,nohup,stdbuf,command,builtin,noglob— and this repo's own convention alreadyrecords that at
docs/conventions/permission-rule-hygiene/README.md:218-220. Every one of the sevenskill bodies invoked its script through
bash "<path>", so a rule withoutbashstops matchingthe command the body actually runs. Rule-only edits would have produced dead grants, and for the
two that work today it would have been a straight regression.
Per the operator ruling (
RECONCILE.mdOR-5 / DQ-3), this is a paired body+rule rewrite appliedidentically across all five plugins: the bodies invoke their bundled scripts directly and
unquoted, and the rules name that same string via
${CLAUDE_SKILL_DIR}, the token that issubstituted in
allowed-tools. Quoting is part of the pairing — an unquoted rule does not match aquoted body path, which is exactly how
repo-fleet-hygiene's grant shipped dead after #1798 fixedonly the variable half and explicitly parked quoting as "Unverified, not asserted."
Two empirical checks were run before committing to this shape, because the whole policy depends on
them: the scripts' exec bits survive plugin-cache install (
-rwxr-xr-xin~/.claude/plugins/cache/...), and direct invocation by bare unquoted path works there.Per-plugin
prototype${CLAUDE_SKILL_DIR}resolves to the skill's subdirectory, so each skill gains a self-locating wrapper underskills/<skill>/scripts/thatexecs the still-single-sourced detector at the plugin root — no duplicated logic, no novel rule form.code-tidyingtidy(inert) andaudit-comment-residue(works-by-accident) both paired.repo-hygieneBash(bash …/scripts/*)wildcard becomes five narrow rules covering the read-only scripts only. See the security note below.repo-fleet-hygienedocs-hygieneaudit-noisepaired.Three skills also gained grants for the read-only commands their pre-computes pipe through (
grep,head,echo): a rule must match each subcommand of a compound command independently, so a scriptgrant alone left the pipeline uncovered and the pre-compute prompted regardless.
Security review note
This widens a trust surface — dead grants become live ones — so it is called out deliberately:
${CLAUDE_SKILL_DIR}. Thetwo leading-wildcard rules that matched a bare wrapper name at any path (including an unvetted
copy) are gone; so is the one wildcarded-target directory glob.
repo-hygiene/cleanis the one with real blast radius, and it is narrowed, not widened. Onlythe five read-only scripts (
resolve-clean-action,scan,preflight,git-branch-audit,git-stash-audit) are pre-approved. The mutating scripts —clean-caches,clean-build,git-prune,git-tree-reset,git-tree-reset-batch,remove-path,clean-batch— aredeliberately not granted and keep routing through the PreToolUse destructive guard and the
permission flow, where the dry-run-then-confirm contract is actually enforced. Since the old rule
matched nothing, nothing regresses.
cleanPreToolUse guard's owncommandstill resolves${CLAUDE_PLUGIN_ROOT}and isunchanged — hook commands are a different substitution context where that variable is
documented to work. Breaking that guard would have been the worst outcome available here.
context7is not in this PRLedger row
B8(Bash(npm view ctx7 version*)) is NOT_REPRODUCED — a fully-pinned rule theconvention explicitly exempts at
README.md:94-100, flagged only by aP1_EREregex over-reach(verifier row
A19, owned byclaude-config). Checked independently:context7'sallowed-toolscarry no
bashwrapper and no${CLAUDE_PLUGIN_ROOT}, so the paired policy has nothing to pairthere. No change, no bump.
Test plan
1. The shipped
claude-configdetector suite still passes, unmodified (that file belongs tolane A, which is editing line 135 of it this batch — see "Not done" below):
2. The detector's own findings against these six plugins: 8 → 1.
Before (at
685dd381) — seven real defects plus the knowncontext7false positive:After — only the row the ledger classified
NOT_REPRODUCEDremains, which this PR is instructed notto touch:
3. New pairing contract, one
*.test.shper changed plugin — asserts no interpreter-led grantand no
${CLAUDE_PLUGIN_ROOT}inallowed-tools, every${CLAUDE_SKILL_DIR}invocation in theskill's markdown unquoted and free of a
bashwrapper, and every granted script present,executable, and actually invoked by a body.
Passes on this branch:
Fails on the pre-fix tree (
git archive HEAD | tar -x, tests copied in) — including the two trapsthis PR had to avoid:
That third assertion is the mechanized form of the
B5quote mismatch #1798 could not assert.3b.
repo-hygieneadditionally guards its grant narrowing with an explicit allowlist (added inreview). The pairing checks validate whatever is granted; they could not catch the set being
re-widened, because every mutating script under
cleanis bundled, executable, and invoked in theskill's markdown — so a grant added for one of them would "pair" correctly. Verified by injecting
Bash(${CLAUDE_SKILL_DIR}/scripts/clean-caches.sh:*)into the frontmatter:The first two lines are the point — it would have gone green without the allowlist.
4. Direct invocation verified against the installed plugin cache, not just the worktree:
5. Repo gates:
6. Existing tests in the five changed plugins all pass (
detect.test.sh,open-pr-count.test.sh,audit-fleet.test.sh, and the fullrepo-hygiene/cleansuite).Lane judgment calls (flagged for review)
OR-5 authorized a body+rule rewrite. These three go slightly beyond that wording. Each is defensible
and each is independently revertible without touching the grant fix:
Three skills gained
Bash(grep:*)/Bash(head:*)/Bash(echo:*). A rule must match eachsubcommand of a compound command independently, so a script grant alone left the surrounding
pipeline uncovered and the pre-compute prompted anyway — the fix would not have taken effect
without them. Bare-name shape, read-only commands, precedented by
prototype's pre-existinghead/echogrants. Back these out and the grants still match; they just don't help yet.Open question, flagged rather than assumed. A bare-name grant like
Bash(echo:*)reads as"this command with any arguments," and the permissions doc's compound-command separators
(
&&,||,;,|,|&,&, newline) do not include redirection — so whether a redirectoperand rides along inside such a grant is undocumented, and I did not verify it. That deserves a
deliberate call rather than a guess from me. Surrounding facts:
prototypealready shippedBash(head:*)andBash(echo:*)onmainbefore this batch, so the shape is existing repoposture rather than something introduced here; the net-new instances are
code-tidying's twoskills and
docs-hygiene:audit-noise. If the answer is "yes, redirects ride along," narrowing isa separate mechanical change (exact-match rules for the three fixed fallback strings) that does
not disturb the grant pairing.
repo-hygienegrants five of twelve scripts, not twelve. Choosing which scripts topre-approve is a scope decision, not a mechanical rewrite. Rationale in the security note above.
Nothing regresses either way, since the old rule matched nothing.
repo-hygiene'scontext/*.mdwere converted too.0fba87c8atorchestrator direction — this PR now touches
SKILL.mdonly. Converting them assumed${CLAUDE_SKILL_DIR}is substituted in a bundled non-SKILL.mdfile, which I flagged asunverified and which the docs resolve neither way: the skills page scopes substitution to "the
skill's markdown content", while
${CLAUDE_PLUGIN_ROOT}'s documented scope is broader ("anywherethe placeholder appears"), so the two are not interchangeable here just because both work in
SKILL.md. If the assumption were wrong, the body would emit a literal the substituted rulecannot match — failing safe (a prompt, never a wrong action) but silently, which is the exact
defect class this PR removes.
The grant fix is unaffected: all five granted scripts are invoked from
SKILL.md, so thepairing is complete for everything the grant covers, and the pairing gate still passes (output
below). The
context/*.mdhalf is tracked in fix(repo-hygiene): finish clean's invocation pairing in context/*.md, gated on ${CLAUDE_SKILL_DIR} substitution scope #2237, gated on settling the substitution scope,with the empirical test that would settle it and a cross-link to skills: ${CLAUDE_SKILL_DIR} used in pre-compute but not documented as harness-substituted — three skills may silently report fake pre-compute data #1824 — the open issue on the
same variable's substitution behaviour in pre-compute.
The five per-plugin pairing gates are near-identical by design: plugins must be independently
installable, so each owns its copy rather than sourcing a shared file. They are not byte-identical
(each carries its own
SKILLS=(…)), socross-plugin-source-driftcorrectly does not claim them —but equally, no gate protects them from diverging.
Not done, deliberately
The lane brief asked to extend
plugins/claude-config/.../permission-rule-check.test.sh. That filebelongs to lane A, whose
A1fix sketch names line 135 of it — editing it here would produceexactly the merge conflict the plugin partition exists to prevent. The suite was run unmodified
instead (50/50, above), the detector's before/after finding counts are recorded, and the new
per-plugin pairing tests carry the contract inside this lane's own fence.
Related
Closes #2221
Follow-up split out of this PR: #2237 — finish
clean's pairing incontext/*.md, gated onsettling
${CLAUDE_SKILL_DIR}'s substitution scope in bundled non-SKILL.mdfiles. Cross-links#1824 (open), the neighbouring uncertainty about the same variable in pre-compute.
Inbox item:
20260811-024628-claude-config-audit-permission-grants-defects-and-fleet-grant-hygiene(ledger
I10-permission-grants-fleet.md, half B rowsB1–B7;B8NOT_REPRODUCED and excluded).Prior art: #1798 (closed — fixed
repo-fleet-hygiene's variable half only and parked quoting);#843 (plugin
bin/delivery gap, which is why relocation rather thanbin/is the remedy here).