feat(claude-config): give audit-permission-grants a denominator, and make audit-prompting-postures' contract agree with itself

[kyle-sexton]

Closes #2281
Closes #2283

Scope note. #2281's eight rows are all taken, with CC-F11 partial and said so below.
#2283 takes four of five — A8 is declined on its rationale and is filed as #2406 with the
corrected mechanism, so neither closing keyword drops a reproducing defect. Two follow-ups filed:
#2406 (A8) and #2415 (audit-pass's missing disallowed-tools, blocked on #2403).

Summary

Two claude-config skills whose contracts contradicted themselves. Released together as 0.37.0
(minor: new output on every detector run, new env-var surface). #2382 (0.35.3), #2403 (0.35.4) and
#2396 (0.36.0) all merged while this was in flight; 0.37.0 sits above all three and the CHANGELOG
order gate is green.

Review round: 7 P2 threads, all real, all fixed, replied to individually and resolved (verified
unresolved=0 via GraphQL, not inferred). Four were this PR's own defect class turned on this PR,
which is itself the finding — so they are named rather than folded in silently:

#	Finding	Instance of
1	P3 axis missing from audited	denominator counting only successes (2nd) — already fixed at HEAD; thread was outdated
2, 5	A candidate find can list but the process cannot read is counted nowhere	same shape (3rd) — two reviewers converged independently
6	audited counts "produced a finding" on two axes, "examined" on the third	same shape (4th)
3	disallowed-tools claimed to make read-only "a property of the tool set"	a false assurance claim, mine
7	Phase D "dropped or demoted" vs the new schema's "kept for the record"	contract disagreeing with itself, introduced by its own fix
4	P7 searched rules + hooks but not the script gate its own catalog blesses	same, introduced by its own fix

After three instances of one shape, the completeness property stopped being asserted per-site and
is derived once: four exhaustive buckets, reconciled against the enumeration on every run, with
a negative test that deletes a bucket increment and asserts the check fires. The denominator's unit
is stated once for all three axes — an input successfully read and examined, never one that
produced something — and printed on every run.

audit-permission-grants — reports a clean bill with no denominator (#2283)

A5 — the headline. No fragile permission grants found. printed identically whether the run
parsed forty allowed-tools blocks and found them healthy or parsed none at all. Every run now ends
with a coverage block, and a run whose denominator is zero prints NOTHING TO AUDIT instead.

The denominator counts what was not read, because one built only from successes is the same
defect in a new spelling. Establishing that turned up two fail-open paths that were not in the
issue, both folded in:

• A settings file present but not valid JSON was skipped by a silent || return 0. Its rules
  were never read and the run still printed a clean bill — and an unparsable rules file is exactly
  where a fragile grant would sit unexamined. Now reported per scope as
  NOT VALID JSON — its rules were not read.
• Both find walks discarded stderr. This script's own header already argues against that: "a
  swallowed permission error was indistinguishable from a clean bill." Unreadable paths are now
  captured and counted.

The vendor/ exclusion moved out of the find predicate into the loop so the run can report how
many files it removed. Same predicate, same result set — but an exclusion whose count is printed
cannot suppress silently. --count keeps the bare integer on stdout (the machine contract) and puts
the block on stderr.

A11. $PERMISSION_HYGIENE_SCAN_ROOT is now the sanctioned name for the one scoping lever, with
$PERMISSION_HYGIENE_FIXTURE_DIR kept as a back-compatible alias (new name wins when both are set).
#2249 made that variable the documented operator remedy for the exit-2 refusal while its name still
told them it was a test seam; reference/criteria.md, which never mentioned it at all, now sanctions
it explicitly.

A15. Consumer-declared exemptions must disclose themselves, may widen the fragile set but
never delete a finding, and a run where every finding is exempted says so instead of printing a clean
bill. The audited repo authors those declarations — the threat model the docs name directly ("Review
project skills before trusting a repository, since a skill can grant itself broad tool access",
fetched 2026-08-12). The report schema grows an Exempt? column to hold it.

A16 — the filed remedy declined, with the measurement. The row says the four scope filters are
"advertised but not implemented". They are implemented, as a presentation filter, and SKILL.md
says so at :70-71. The real defect is that the argument hint reads like a scan-scope. The filed fix
(detector flags) rests on a cost claim that no longer holds: since #2249 the root is a git toplevel,
$CLAUDE_PROJECT_DIR, or an explicitly named directory — never an unbounded sweep — and I measured
the two walks over this repository at 0.49 s and 0.41 s. Flags to skip half a second would
buy nothing and add a second place for scope to be defined. Fixed the wording instead, adopting the
formulation both sibling audit skills already ship.

A8 — declined, filed as #2406. It reasons from the vendor/ exclusion's own justification ("not
loadable, so the grant never takes effect") to node_modules/, worktrees and marketplace mirrors.
That step is false. https://code.claude.com/docs/en/skills, fetched 2026-08-12:

Skills also load from nested .claude/skills/ directories below your working directory. When
Claude reads or edits a file in a subdirectory, skills from that subdirectory's .claude/skills/
become available.

So node_modules/<pkg>/.claude/skills/<name>/SKILL.md is loadable, and the exclusion would make
P2 — an error-tier check whose whole subject is a leaked username — silently blind to a live grant.
Same failure shape as the // path exemption on #2382. The defensible half (every exclusion reports
its own count) ships here; the rest needs a loadability model, which #2406 specifies.

audit-prompting-postures — contract disagrees with itself in eight places (#2281)

All eight rows taken (CC-F11 partial — see its bullet). Prose-only, so there is no behavioral test to
write and I am not inventing one — same posture #2403 took on the sibling skill.

• CC-F3. P7 blesses a deny-by-default hook or script gate as presence evidence "without any
  prose", while Phase B inventories instruction text — so the one evidence form P7 names was the
  one form Phase B could not see, on the posture whose false MISSING is most expensive. Split the
  two: the inventory bounds what may produce a finding, not what counts as evidence, and Phase C now
  looks for the gate before judging P7 — in all three places the catalog blesses: settings rules,
  hook configuration, and, after review caught the procedure searching only the first two, the
  script the component delegates the destructive step to, followed and read. A component whose
  destructive action runs through a gating script is gated and nothing in its own text says so.
  Tightened destructive-capable from "can delete, reset, force-push" — which matches every
  component with a shell — to what the body has the model DO, per the classification section's own
  opening line.
• CC-F5 + CC-F10, written as one edit because both move the same seam and landing them
  separately would ship a Phase A that disagreed with itself. The best-practices page is fetched
  every run and its failure aborts (single non-negotiable input; ten wording-unverified
  postures is a report shaped like an audit that audited nothing). Model subpages are fetched lazily
  in Phase C per applicable row and fail locally — which is what the observed run already did and the
  wording forbade. The verdict schema was closed at three tokens while the body mandated two more; it
  now carries four verdicts including info, with wording-unverified / (unverified) named as
  markers that ride alongside a verdict rather than replacing one.
• CC-F6. The surface set is named in this skill instead of inherited by reference from a sibling
  that versions independently — the coupling that let output-styles become inventoried here and
  unnameable by this skill's own filter. output-styles is now a scope token.
• CC-F7. P8 carries the model condition the skill's own gotcha mandates. Leg the issue marked
  unverified, re-fetched by me 2026-08-12: the pointed-at section scopes context awareness to
  "Claude Sonnet 5, Claude Sonnet 4.6, Claude Sonnet 4.5, and Claude Haiku 4.5".
• CC-F4 — and the false assurance claim review caught in it. disallowed-tools: Edit, NotebookEdit is declared, but an earlier draft of this PR claimed it made "never edits a
  component" a property of the tool set. That was false and is removed rather than softened.
  Write is retained for the mandated persist and Phase B has already read every audited component,
  so it can overwrite one; Bash is retained for the state key, and a shell mutates files too. The
  declaration narrows the accident surface, it does not enforce the contract — both skills now say
  so, and both forbid telling an operator the skill cannot edit their files. A skill whose subject
  is auditing assurance must not ship a false assurance claim about itself; the CHANGELOG records
  that the earlier claim was wrong instead of quietly shipping the corrected text. Second
  unverified leg, re-fetched 2026-08-12 — the frontmatter reference's semantics, including "The
  restriction clears when you send your next message", which is the right lifetime: whoever accepts
  a proposal can apply it.
• CC-F8 — the issue's stated mechanism is wrong at HEAD, and I fixed the real one. The issue
  says grep -c "audit-prompting-postures" .../audit-instructions/SKILL.md → 0, "the token
  appears nowhere in the sibling". At HEAD it returns 1, at audit-instructions/SKILL.md:392, in
  a state-key aside. The grep claim is false; the substantive claim survives, because a mention in an
  aside is not a route-out — audit-instructions' Scope boundary section still never named this
  skill. Added that route-out line. Also added two evals: one whose prompt carries no slash
  invocation, so description-driven selection is exercised for the first time (all five existing
  cases invoke explicitly), and one pinning CC-F3's mechanical-gate rule. The
  description-drives-discovery claim was itself an unverified leg — third one re-fetched
  2026-08-12 (skills.md:259, :425-426).
• CC-F11 — partial, and the partial goes one way I should name. The uninstall half is fixed: the
  state key stops overwrites, not reaping, with the sentence quoted and --keep-data named
  (re-fetched 2026-08-12). The row's other two observations are recorded, not fixed — and one
  moved the wrong way. when_to_use is still unused, and the description grew from 1,290 to
  1,305 of its 1,536-char cap to carry output-styles for CC-F6, which is the opposite direction
  from the row's headroom note. That trade is deliberate (a scope token that is unnameable is the
  actual defect; 231 chars of headroom is not), but "all eight rows taken" would have been the wrong
  sentence to leave standing.

CC-F9 is correctly not touched: the issue records it as falsified, and it is.

One divergence this PR would otherwise have created

CC-F4 declares disallowed-tools: Edit, NotebookEdit on audit-prompting-postures.
audit-instructions states the identical report-only contract ("There is no --fix… never by
this skill") and names neither Edit nor Write anywhere in its body — so declaring it on one of
the pair and not the other would have opened a fresh instance of exactly the sibling divergence
CC-F6 is about, in the release that fixes CC-F6. It is declared on both, in the file this PR was
already editing. audit-pass states the contract too and is not touched: PR #2403 owns that
file right now, so it is filed as #2415 rather than collided with.

Test plan

Fail-before / pass-after for A5. Three roots that origin/main (5ea4f87f) describes with one
identical string, run against the old and new detectors:

==================== BEFORE — origin/main (5ea4f87f) ====================
--- root with 0 grants ---
No fragile permission grants found.
--- root with 2 healthy grants ---
No fragile permission grants found.
--- root whose only rules file is invalid JSON ---
No fragile permission grants found.
--- --count ---
  empty=0   healthy=0   badjson=0

==================== AFTER — this branch ====================
--- root with 0 grants ---
NOTHING TO AUDIT: 0 allowed-tools block(s) and 0 allow rule(s) were read under this root, so this
run has no denominator. That is a scan of nothing, not a clean bill — do not report it as one.

Scan coverage (the denominator — what this run actually read):
  root: /tmp/tmp.EbQtjPHKzq/empty-root (resolved from $PERMISSION_HYGIENE_FIXTURE_DIR (alias of $PERMISSION_HYGIENE_SCAN_ROOT))
  frontmatter: 0 allowed-tools block(s) parsed from 0 candidate file(s); 0 excluded under a vendor/ path segment as non-loadable
  settings: 0 allow rule(s) from 0 scope(s) read — project: absent; local: absent; user-global (...): absent
  plugins: 0 manifest(s); 0 settings.json parsed
  NOT read: 0 path(s) the walk could not open; 0 settings file(s) and 0 plugin settings.json present but not valid JSON
  never in scope here: managed-policy and enterprise settings, a --settings flag file, and the pre-v2.1.211 start-directory copy...

--- root with 2 healthy grants ---
No fragile permission grants found.

Scan coverage (the denominator — what this run actually read):
  ...
  settings: 2 allow rule(s) from 1 scope(s) read — project: 2 rule(s); local: absent; user-global (...): absent
  ...

--- root whose only rules file is invalid JSON ---
NOTHING TO AUDIT: 0 allowed-tools block(s) and 0 allow rule(s) were read under this root...

Scan coverage (the denominator — what this run actually read):
  ...
  settings: 0 allow rule(s) from 0 scope(s) read — project: NOT VALID JSON — its rules were not read; local: absent; ...
  NOT read: 0 path(s) the walk could not open; 1 settings file(s) and 0 plugin settings.json present but not valid JSON
  ...
--- --count (stdout only) ---
  empty=0   healthy=0

The --count line is the point restated: stdout is still 0 in both cases (the machine contract is
unchanged), and stderr now separates them.

The denominator got the same defect wrong four times, and the fourth is why it is now structural.
Recorded in full because the pattern is more useful than any one instance:

1. audited omitted the P3 axis, so a root of clean plugin settings.json printed NOTHING TO AUDIT two lines above plugins: 2 manifest(s); 2 settings.json parsed. Caught in self-review.
2. A candidate find can list but the process cannot read — find needs only directory
   traversal to report -type f — reached awk, failed, wrote to the real stderr rather than
   WALK_ERR, and was counted in no bucket, while the coverage block promised to disclose exactly
   that input. Caught by two reviewers independently.
3. audited counted "produced a finding" on the frontmatter and settings axes but "examined
   successfully" on P3's, so a SKILL.md with no allowed-tools and a settings.json with an empty
   allow array contributed nothing despite being read and found to grant nothing.

Three instances means the invariant was maintained by convention at each continue, so it is now
derived once. Every enumerated candidate lands in exactly one of four buckets — vendor-excluded,
unreadable, no allowed-tools block, parsed — and reconcile_frontmatter checks they sum to the
enumeration on every run, printing DENOMINATOR BUG and naming itself as the defect when they do
not. A check that cannot fail is not a check, so a negative test deletes a bucket increment from a
copy of the script and asserts the reconciliation fires. And the unit is stated once for all three
axes — an input successfully read and examined, never one that produced something — and printed:

  DENOMINATOR = 3 input(s) successfully examined: 2 frontmatter file(s) + 1 settings scope(s) + 0
  plugin settings.json. The unit on every axis is "read and examined", never "produced a finding".
  reconciled: 3 candidate(s) = 1 vendor-excluded + 0 unreadable + 1 without an allowed-tools block + 1 parsed

Extraction stderr now joins the walk's rather than escaping to the terminal, and a run that audited
nothing and could not open its own inputs says so distinctly instead of reporting an empty tree.

Suite: 76 → 102 on this branch's own base, then 121 after the review round, all passing.
26 of those are this branch's, across the denominator (including the P3 axis), the
unparsable-settings skip, the --count stdout/stderr split, the exclusion disclosure, and the
scan-root rename; the rest are #2382's, which landed mid-flight.

Merge note — and the trap it walked into. #2382 (0.35.3), #2403 (0.35.4) and #2396 (0.36.0) all
merged into main while this was in flight, twice leaving the PR CONFLICTING. A conflicted merge
ref suppresses the pull_request-triggered runs entirely, and the PR then displayed 3 checks,
all passing instead of 33 — nothing distinguishes that from a real green except counting the rows.
Caught by comparing against #2382's 34, not by reading the failure count.

Resolved with merge commits rather than repeated rebases, and verified marker-free across the whole
tree before each commit:

• plugin.json — 0.37.0, because feat(claude-config): complete the permission-state skill and add the auto-mode authoring lane #2396 took 0.36.0 (the version this PR originally claimed) and
  also rewrote the description to "Nine configuration-health skills". Theirs kept, version raised.
• CHANGELOG.md — 0.37.0 / 0.36.0 / 0.35.4 / 0.35.3 / 0.35.2; --check-order green.
• permission-rule-check.test.sh — both sides pure additions, both kept.
• permission-rule-check.sh and criteria.md auto-merged with no conflict: fix(claude-config): tighten permission-rule-check P1/P2 precision #2382's hunks are
  the P2 pattern block and P2's criteria section, neither of which this PR touches.
• feat(claude-config): complete the permission-state skill and add the auto-mode authoring lane #2396 also changed lib/permission-patterns.sh, which this detector sources — so the suite was
  re-run against the merged library, not just against this branch's own base. Still passing.

$ bash plugins/claude-config/skills/audit-permission-grants/scripts/permission-rule-check.test.sh
PASS: empty root reports NOTHING TO AUDIT
PASS: empty root does NOT print a clean bill
PASS: empty root still prints the coverage block
PASS: empty root denominator names zero blocks
PASS: healthy root prints the clean bill
PASS: healthy root is not NOTHING TO AUDIT
PASS: clean bill carries a non-zero rule count
PASS: a clean P3-only root is a clean bill
PASS: a clean P3-only root is NOT a scan of nothing
PASS: coverage counts the plugin settings it parsed
PASS: coverage names the project scope it read
PASS: coverage names an absent scope as absent
PASS: unparsable settings file is named, not skipped in silence
PASS: unparsable file is counted under NOT read
PASS: a run whose only rules file will not parse is not a clean bill
PASS: --count stdout is still the bare integer
PASS: --count writes the coverage block to stderr
PASS: --count coverage carries the denominator
PASS: vendor exclusion discloses its count
PASS: coverage names the candidate file total
PASS: PERMISSION_HYGIENE_SCAN_ROOT resolves a root
PASS: coverage names the rung that resolved the root
PASS: the legacy alias still resolves a root
PASS: the sanctioned name wins over the alias
PASS: the alias did not win
PASS: refusal names the sanctioned variable as the fix
...
All 121 checks passed.

The A16 measurement, since it is what declines the row:

$ time (find . -type f \( -name 'SKILL.md' -o \( -name '*.md' -path '*/agents/*' \) \
        -o \( -name '*.md' -path '*/commands/*' \) \) ! -path '*/vendor/*' | sort -u | wc -l)
214
real    0m0.491s
$ time (find . -type f -path '*/.claude-plugin/plugin.json' | wc -l)
65
real    0m0.412s

Repo gates:

$ CHECK_SKILL_SKILLS_ROOT=plugins/claude-config/skills bash plugins/skill-quality/scripts/check-skill.sh audit-prompting-postures
INFO: description length 1305/1536 chars
INFO: all 6 base-ref trigger phrase(s) preserved
INFO: SKILL.md 200/500 lines
INFO: markdownlint clean
CHECK-SKILL audit-prompting-postures: PASS — 0 errors, 0 warning(s)

$ ... check-skill.sh audit-permission-grants
INFO: script test passed: scripts/permission-rule-check.test.sh
CHECK-SKILL audit-permission-grants: PASS — 0 errors, 1 warning(s)   # no-Gotchas warning is pre-existing

$ npx markdownlint-cli2 <the 6 changed markdown files>
Summary: 0 issues in 0 files

$ shellcheck -S warning .../permission-rule-check.sh .../permission-rule-check.test.sh
(clean)

$ bash scripts/check-shell-portability.sh <merge-base>
No unexcused GNU-only constructs in 2 shell file(s).
$ bash scripts/check-skill-portability.sh <merge-base>
No unexcused coupling tokens in 6 skill file(s).
$ bash scripts/check-changelog-parity.sh --check-order
All 76 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.
$ bash scripts/validate-plugins.sh
All plugin manifests and the catalog validated.
$ bash scripts/check-silent-skips.sh
No silent prerequisite skips found in hook entry scripts.
$ bash plugins/skill-quality/scripts/check-evals-quality.sh .../audit-prompting-postures/evals/evals.json
check-evals-quality: PASS (0 warning(s) across 1 file(s))
$ npx ajv-cli@5 validate --spec=draft2020 -s plugins/skill-quality/reference/evals.schema.json -d .../evals.json
.../evals.json valid

check-listing-budget was run and reports the aggregate already over budget on main
(97537/8000) — advisory-only, and this PR's contribution is +15 chars (description 1290 → 1305
for output-styles). Not introduced here and not resolvable here.

Related

• fix(claude-config): audit-prompting-postures' contract disagrees with itself in eight places #2281 — closed here; all eight rows (CC-F3 … CC-F11) taken. CC-F9 deliberately untouched, as
  the issue's own "Not in this issue, on purpose" section requires.
• fix(claude-config): audit-permission-grants reports a clean bill with no denominator, and four amplifiers around it #2283 — closed here for A5, A11, A15, A16. A8 split to fix(claude-config): permission-rule-check's loadability filter needs a model, not more path exclusions #2406 before merge so the auto-close
  drops nothing.
• fix(claude-config): permission-rule-check's loadability filter needs a model, not more path exclusions #2406 — the A8 follow-up, with the falsified rationale and what a real loadability model must
  distinguish.
• fix(claude-config): tighten permission-rule-check P1/P2 precision #2382 (0.35.3), fix(claude-config): stop audit-pass asking the model to test a substituted placeholder #2403 (0.35.4) and feat(claude-config): complete the permission-state skill and add the auto-mode authoring lane #2396 (0.36.0) — all merged during this PR's life;
  see the merge note in the test plan. feat(claude-config): complete the permission-state skill and add the auto-mode authoring lane #2396 took 0.36.0, so this PR is 0.37.0.
• fix(claude-config): permission-rule-check falls through to $PWD and sweeps the user profile, exiting 0 #2249 (closed) — removed the $PWD fallback and added the exit-2 refusal. This is the residue
  it named: a resolved root with nothing in it still reported clean.
• fix(claude-config): audit-prompting-postures report path has no project dimension, so any other root overwrites it #2250 (closed) — keyed the report path per project; CC-F11 is the residual amplifier
  (uninstall still reaps the directory).
• fix(claude-config): drop P2's unsupported no-expansion claim, refuse an unresolvable scan root, and key the postures report per project #2259 — added the two evals and 67 lines to audit-prompting-postures/SKILL.md; every anchor
  in fix(claude-config): audit-prompting-postures' contract disagrees with itself in eight places #2281 past :78 was re-derived at this HEAD before editing.

Inbox items: 20260811-021645-plugin-audit-four-components-and-guard-deadlock-ownership (#2281),
20260811-024628-claude-config-audit-permission-grants-defects-and-fleet-grant-hygiene (#2283).
Ledgers: .work/handoff-inbox-batch-4/ledgers/I9-021645-four-components.md § Lane A;
.work/handoff-inbox-batch-4/ledgers/I10-permission-grants-fleet.md § A5, A11, A15, A16.

[cursor]

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.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 726aa9e569

ℹ️ 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 2m 11s —— View job

---

Security Review — PR #2408

Reviewed all 10 files changed in this PR against origin/main (base) at HEAD 7d59df2. Scope: the permission-hygiene detector script (permission-rule-check.sh / .test.sh) and the doc/skill-contract changes to audit-permission-grants, audit-prompting-postures, and audit-instructions.

Findings

IMPORTANT (confidence: medium) — disallowed-tools: Edit, NotebookEdit is described as making the read-only contract "mechanical" but does not actually close the write surface

• audit-prompting-postures/SKILL.md#L27-L30
• audit-instructions/SKILL.md#L35-L36

Both skills now declare disallowed-tools: Edit, NotebookEdit (line 4 of each frontmatter) and describe this as turning "never edits a component" into "a property of the tool set, not of model obedience." Write is deliberately kept enabled (for the Phase D persist step), and Bash is not restricted at all. Since Phase B has already read every audited component's file path by the time this runs, both the Write tool and an available Bash tool (cat > file, sed -i, etc.) can still overwrite the very files the skill claims it mechanically cannot edit. The frontmatter restriction only closes two of the several tool paths capable of mutating those files, so the "mechanical, not obedience-based" framing overstates the actual guarantee. This matters because these are audit/report-only skills whose entire value proposition is a trustworthy read-only contract — an operator who takes the new "mechanical" language at face value could reasonably (but incorrectly) conclude the skill is incapable of modifying repository files it audits, when in fact it still can via Write/Bash. This does not introduce a new capability (the skills could already write/edit before this PR), but the PR adds a false enforcement claim, which is itself the access-control-adjacent risk worth flagging. Recommend either also disallowing Write/Bash for the audit phases (routing the Phase D persist through a narrower path or a separate invocation) or rewording the claim to something like "reduces the editing surface" rather than "mechanical" enforcement of a no-edit contract. (This overlaps with a Codex review comment already left on this PR making a related point about Write/Bash remaining available.)

Reviewed and found no issues

• permission-rule-check.sh (the ~170-line diff adding the coverage/denominator block, the PERMISSION_HYGIENE_SCAN_ROOT env-var rename/alias, and the unparsable-JSON/vendor/-exclusion accounting): all new code paths are read-only (parses frontmatter and settings.json for reporting only, never executes discovered content), uses jq with static filters (no filter strings built from file content), quotes all variable expansions, uses mktemp+trap correctly for the new WALK_ERR scratch file, and does not introduce eval, unsafe globbing, or command construction from untrusted input. The env-var rename (PERMISSION_HYGIENE_FIXTURE_DIR → PERMISSION_HYGIENE_SCAN_ROOT) is operator-controlled, not attacker-controlled, and the precedence (new name wins) is intentional and tested.
• permission-rule-check.test.sh: test-only harness changes (new -u PERMISSION_HYGIENE_SCAN_ROOT isolation, new fixtures), no injection surface.
• reference/criteria.md, reference/postures.md, audit-instructions/SKILL.md, audit-prompting-postures/evals/evals.json, CHANGELOG.md, plugin.json: documentation/prompt-contract and metadata only; no code execution, no secrets, no permission-widening in any actual settings/workflow file. The A8 discussion in criteria.md about node_modules//nested-skill loadability is itself a security-hardening discussion (declining a fix that would have made a leaked-username check go blind) rather than a new risk.
• No GitHub Actions workflow files, CI config, or settings.json/permission-grant files are touched by this PR — no pull_request_target/workflow_run trust-boundary changes, no permissions: widening, and no unpinned/loosened dependency or action pins to assess (out of scope for this PR's diff regardless, per zizmor's lane).
• settings.local.json continues to be parsed only for its permissions.allow array and never echoed wholesale — the existing token-leak guard noted in the script's own comments is preserved, not weakened, by this diff.

[claude]

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

---

Security Review — PR #2408

• Gather context (PR diff, changed files)
• Review permission-rule-check.sh / .test.sh for injection, unsafe file handling, secret exposure
• Review skill/reference doc changes for trust-boundary or access-control claims
• Review CHANGELOG.md / plugin.json
• Check for GitHub Actions / workflow permission changes (none in this diff)
• Post findings

Reviewed the full diff (git diff origin/main...HEAD / gh pr diff 2408) against the 10 changed files, focused on the detector script and the skill-contract docs it ships alongside.

Findings

IMPORTANT (confidence: medium) — disallowed-tools: Edit, NotebookEdit is documented as making the no-edit contract "mechanical," but the write surface stays open

• audit-prompting-postures/SKILL.md#L27-L31
• audit-instructions/SKILL.md#L35-L40

Both skills now declare disallowed-tools: Edit, NotebookEdit and describe this as turning "never edits a component" into "a property of the tool set, not of model obedience" (audit-prompting-postures/SKILL.md#L27). Write is deliberately left enabled for the Phase D persist step, and Bash isn't restricted at all. Since Phase B has already resolved every audited component's file path by that point, Write (or Bash via cat >/sed -i) can still overwrite the very files the skill claims it mechanically cannot edit. So the frontmatter closes two of several mutation paths, and the "mechanical" framing overstates the actual guarantee for a pair of skills whose entire value proposition is a trustworthy read-only audit. Recommend either narrowing the guarantee's wording (e.g. "reduces the editing surface" rather than "mechanical … not obedience") or routing the Phase D persist through a scope where Write/Bash aren't available during the audit phases proper. Note this overlaps with both Codex's inline comment on this PR and this PR's own earlier automated Claude review — flagging again here because it's a genuine access-control-adjacent overstatement, not because it's new.

Reviewed, no issues found

• permission-rule-check.sh (the coverage/denominator block, the PERMISSION_HYGIENE_SCAN_ROOT rename/alias, the unparsable-JSON and vendor/-exclusion accounting): every new code path is read-only — file contents are only ever parsed (via jq -e/awk/grep -oE with static patterns) for reporting, never executed or fed into eval. Variable expansions are quoted throughout, the new WALK_ERR scratch file uses mktemp + trap ... EXIT correctly, and no rule/file content is interpolated into a shell command or regex built from untrusted input. PERMISSION_HYGIENE_SCAN_ROOT/_FIXTURE_DIR are operator-set env vars, not attacker-controlled, and precedence (new name wins) is intentional and tested.
• permission-rule-check.test.sh: test-only harness changes (env isolation additions, new fixtures); no injection surface, nothing that touches a real ~/.claude.
• reference/criteria.md, SKILL.md (both audit skills), postures.md, evals.json, CHANGELOG.md, plugin.json: prompt-contract/doc/metadata only. The new "consumer-declared exemptions" contract (A15) is worth naming as a residual trust question — an audited repo's own CLAUDE.md/rules can supply an "exemption" the auditing model reads — but the PR explicitly constrains it (must be disclosed with source, may only widen/downgrade, can never delete a finding, and an all-exempted run can't print a clean bill), and this behavior already existed pre-PR ("Read those when present"); the PR only tightens it. Not a new gap.
• No GitHub Actions workflow files, CI config, permissions: blocks, or dependency/action pins are touched by this PR — nothing in zizmor's lane to add to.
• settings.local.json continues to be parsed only for its permissions.allow array and never echoed wholesale — that existing token-leak guard is preserved, not weakened.

One non-security note for completeness, already raised by Codex inline and not re-litigated as a finding here: an unreadable (but find-enumerable) frontmatter file's awk permission error isn't captured into WALK_ERR, so the coverage block's "NOT read" count can undercount in that specific case. That's a reporting-completeness gap in a detector script, not an injection/authz/secrets issue, so it's out of scope for this security-focused pass.

[claude]

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

---

Code Review — PR #2408

• Gather context (PR description, diff, repo CLAUDE.md / REVIEW.md)
• Review permission-rule-check.sh / .test.sh changes
• Review audit-permission-grants SKILL.md / criteria.md changes
• Review audit-prompting-postures SKILL.md / postures.md / evals.json changes
• Review audit-instructions SKILL.md, CHANGELOG.md, plugin.json
• Post inline findings and summary

Reviewed per this repo's REVIEW.md code-review lane (correctness, design, error handling, tests, documentation). Security-shaped findings are left to the security lane, which already ran on this PR — I did not duplicate its disallowed-tools/Write/Bash finding.

Finding posted inline

One correctness finding on permission-rule-check.sh:333: the new frontmatter walk (lines 321–344) counts find-level traversal failures into WALK_ERR, but a file find can list (needs only directory-execute permission) that the process then can't read (e.g. chmod 000, a restrictive ACL) is silently dropped by the awk-based extract_allowed_tools — its failure goes to real stderr, not WALK_ERR, and the resulting empty at just hits continue. That file never lands in fm_candidates's "not read" complement or anywhere in the coverage block, even though both reference/criteria.md's new denominator section and SKILL.md's "Say what was not read" bullet assert unopenable paths are tracked. scan_settings_allow and the plugin-settings loop both handle their analogous case (unreadable/unparsable JSON) explicitly and count it either way — the frontmatter loop is the one path that doesn't. Concretely, a root whose only auditable file is one unreadable SKILL.md still prints NOTHING TO AUDIT with NOT read: 0 path(s), which is the exact "swallowed permission error is indistinguishable from a clean bill" failure this PR is otherwise closing, just relocated to the per-file parse. Suggested a [[ -r "$file" ]] check that counts into the same not-read bucket as walk_errors.

I independently verified this by reading the code paths (not executing the script — this sandbox's Bash tool declined to run the test suite or a direct invocation of permission-rule-check.sh/.test.sh in this session, so I could not empirically reproduce the unreadable-file case or independently confirm the author's Test-plan output; that section of the PR body is author-claimed, not verified by me). I did independently confirm: plugin.json is 0.36.0 and CHANGELOG.md's newest entry is ## [0.36.0] (ordering consistent), and evals/evals.json is valid JSON with 7 eval entries (jq empty + jq '.evals | length').

Reviewed, no other issues found

• Doc/contract consistency — audit-prompting-postures/SKILL.md's four-token verdict schema (MISSING/PRESENT/NOT-APPLICABLE/info) and its two orthogonal markers (wording-unverified, (unverified)) are used consistently across Phase C, Phase D, and the report-table section; the P7 mechanical-gate carve-out in Phase C matches reference/postures.md's P7 row; audit-instructions/SKILL.md's new route-out line to audit-prompting-postures and vice versa are reciprocal and don't create a cycle or contradiction.
• permission-rule-check.sh denominator logic — the audited sum folding in all three axes (frontmatter blocks, allow rules, plugin settings parsed) correctly avoids the "P3-only clean root reports zero denominator" bug the PR describes catching in its own review (confirmed by reading the code, matching test case 10bb); set -u-safe empty-array handling in coverage_block; PERMISSION_HYGIENE_SCAN_ROOT vs PERMISSION_HYGIENE_FIXTURE_DIR precedence is implemented as documented (new name wins).
• Version/changelog gate — plugin.json version and CHANGELOG.md ordering are consistent (checked directly, not just via the author's stated check-changelog-parity.sh output).
• Test additions — new cases (10a/10b/10bb/10c/10d/10e, case 11) reuse existing fixtures (e.g. $D6B) correctly and assert on strings that match what the script actually emits.

[github-actions]

Claude has reviewed this PR 2 times. The lane skips further automatic reviews after 5; deleting this comment resets the count.

[claude]

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

---

Security Review — PR #2408

• Gather context (diff at latest HEAD 22fdd55, post-merge)
• Review permission-rule-check.sh / .test.sh for injection, unsafe file handling, secret exposure
• Review skill/reference doc changes for trust-boundary or access-control claims
• Review CHANGELOG.md / plugin.json
• Check for GitHub Actions / workflow permission changes
• Post findings

Reviewed the PR's actual diff (gh pr diff 2408, confirmed against the 10-file change list) at HEAD 22fdd553f7ecc34c7b36577a927a2bf3f6f0e8c8. This HEAD is a merge of origin/main into the branch; I diffed 22fdd55 against the previously-reviewed commit 4c4afb0 and confirmed all 8 substantive PR files (both SKILL.mds, criteria.md, postures.md, the script, both test/evals files) are byte-identical to what was already reviewed twice by this security lane and once by the code-review lane. Only plugin.json (version bump, now 0.37.0) and CHANGELOG.md (a rephrased 0.37.0 entry, required by the merge with #2382/#2403 landing in between) changed — both non-functional, metadata-only. No .github/workflows/ files are touched by this PR.

Findings

IMPORTANT (confidence: medium) — disallowed-tools: Edit, NotebookEdit is still described as making the no-edit contract "mechanical," but Write/Bash remain available

• audit-prompting-postures/SKILL.md#L27-L31
• audit-instructions/SKILL.md#L35-L40

Unchanged from the prior two security passes on this PR, so restating rather than re-discovering: both skills declare disallowed-tools: Edit, NotebookEdit and describe this as making "never edits a component... a property of the tool set, not of model obedience." Write is deliberately left enabled for the Phase D persist step, and Bash isn't restricted at all. By the time this runs, Phase B has already resolved every audited component's file path, so Write (or Bash via cat >/sed -i) can still overwrite the very files the skill claims it mechanically cannot edit. The frontmatter closes two of several mutation paths, so "mechanical" overstates the actual guarantee for a pair of skills whose value proposition is a trustworthy read-only audit. This doesn't grant a new capability (both skills could already write/edit pre-PR) — the risk is the PR adds a false enforcement claim an operator could reasonably rely on. Recommend narrowing the wording (e.g. "reduces the editing surface") or routing the Phase D persist through a path where Write/Bash aren't available during the audit phases proper. Also flagged by Codex's inline review on this PR and both prior automated Claude passes — noting again only because it remains unresolved at current HEAD, not because it's new.

Reviewed, no security issues found

• permission-rule-check.sh (coverage/denominator block, PERMISSION_HYGIENE_SCAN_ROOT rename/alias, unparsable-JSON and vendor/-exclusion accounting): every new/changed path is read-only — file contents are parsed via jq -e/awk/grep -oE with static patterns for reporting only, never eval'd or fed into a constructed shell command. Variable expansions are quoted throughout; the new WALK_ERR scratch file uses mktemp + trap ... EXIT correctly; P2_RULE_ERE/_p2_path are built from fixed character classes, not input-derived. PERMISSION_HYGIENE_SCAN_ROOT/_FIXTURE_DIR are operator-set env vars, not attacker-controlled, and the new-name-wins precedence is implemented as documented.
• permission-rule-check.test.sh: test-only harness changes; no injection surface, no interaction with a real ~/.claude.
• criteria.md, postures.md, both SKILL.mds (contract text beyond the finding above), evals.json, CHANGELOG.md, plugin.json: prompt-contract/doc/metadata only. The A15 "consumer-declared exemptions" contract is a residual trust surface worth naming (an audited repo's own declarations can widen/downgrade a finding) but the PR only tightens pre-existing behavior — every declaration must disclose its source, may never delete a finding, and an all-exempted run can't print a clean bill — so it's a hardening, not a new gap.
• No GitHub Actions workflow files, CI config, permissions: blocks, or dependency/action pins are touched by this PR — nothing in zizmor's lane to add to.
• settings.local.json continues to be parsed only for its permissions.allow array and never echoed wholesale — the existing token-leak guard is preserved, not weakened.

Everything above is a repeat confirmation of the prior two security passes' conclusions — this push introduced no new source-visible changes to review, only a version/changelog reconciliation from the origin/main merge.
· Branch: fix/2281-2283-claude-config-audit-contracts

[claude]

Claude finished @kyle-sexton's task in 6m 17s —— View job

---

Code Review — PR #2408

• Gather context (PR diff via gh pr diff 2408, REVIEW.md, CLAUDE.md)
• Review permission-rule-check.sh / .test.sh
• Review audit-permission-grants SKILL.md / criteria.md
• Review audit-prompting-postures SKILL.md / postures.md / evals.json
• Review audit-instructions SKILL.md, CHANGELOG.md, plugin.json
• Post inline findings and summary

Reviewed per this repo's REVIEW.md code-review lane (correctness, design, error handling, tests, documentation). This repo has a claude-security-review lane, so security-shaped findings (the disallowed-tools/Write/Bash enforcement-strength question, already raised by two prior Claude reviews and Codex on this PR) are out of scope here and not repeated. I also did not re-report the unreadable-frontmatter-file gap in extract_allowed_tools (around line 331) — already flagged inline by Codex and by a prior Claude review comment on this PR, unchanged since.

Findings posted inline

1. permission-rule-check.sh:447 — the audited denominator is asymmetric across its three axes. plugin_settings_parsed counts every successfully-parsed settings.json regardless of whether it declares permissions (examined-and-clean counts). But fm_with_block only counts frontmatter files that carried a non-empty allowed-tools block, and allow_rules_read only counts settings scopes that yielded at least one rule — fm_candidates and scopes_read (both tracked, neither folded into audited) are the "examined successfully" counterparts and are left out. A root with real SKILL.md files that simply don't restrict tools, plus a settings.json that parses but declares no allow rules, would print NOTHING TO AUDIT right above a coverage block showing non-zero candidate files and scopes read — the exact defect class the PR's own test plan describes catching and fixing for the P3/plugin_settings_parsed axis, left open on the other two. I read this from the code; I could not execute the script in this sandboxed session to empirically confirm the exact output string, so treat the reasoning (not a captured run) as the evidence.
2. audit-prompting-postures/SKILL.md:171-173 — this PR's own new verdict-schema text disagrees with pre-existing Phase D wording eleven lines above it. The new line says info is "kept for the record" for every verifier-refuted finding; the untouched line 109 says refuted findings are "dropped or demoted to info" — a disjunction with no stated criterion, and "dropped" directly conflicts with "kept for the record." Given the whole PR is closing eight instances of exactly this shape of self-disagreement, this reads as a residual (or freshly introduced) instance of the same pattern.

Reviewed, no other issues found

• Denominator/coverage mechanics otherwise — scope_status accumulation, the set -u-safe empty-array guard in coverage_block, the WALK_ERR mktemp/trap cleanup, the vendor/ exclusion's move from find predicate to loop (verified the case pattern */vendor/* is equivalent to the old -path predicate), and the PERMISSION_HYGIENE_SCAN_ROOT/_FIXTURE_DIR precedence are all implemented as documented.
• audit-prompting-postures contract consistency (the other 7 of 8 CC-F rows) — P7's mechanical-gate carve-out in Phase C matches its reference/postures.md row; the P8 model-condition addition matches the Gotchas' "Model-conditional postures stay conditional" line; the CC-F6 output-styles scope token is threaded consistently through the description, argument-hint, and Phase B surface-set enumeration; the CC-F8 route-out addition to audit-instructions/SKILL.md is reciprocal and doesn't create a routing cycle.
• Version/changelog — independently confirmed plugin.json is 0.37.0 and CHANGELOG.md's newest entry is ## [0.37.0], above 0.36.0/0.35.4/0.35.3 in order (checked directly, not just via the author's stated gate output).
• Test fixtures — case 10bb (P3-only denominator) and case 10e (vendor exclusion count) reuse fixtures correctly and assert on strings that match what the script emits for the paths I traced by hand.

Everything else in the Test plan and repo-gate output in the PR body (the 112-case suite run, check-skill.sh, shellcheck, check-changelog-parity.sh, etc.) is author-claimed; I did not re-run the suite or the gates myself in this session (bash script execution required interactive approval this session didn't have), so I'm not counting that as independently verified.

[claude]

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

---

Security Review — PR #2408

• Gather context (diff at current HEAD 3d40b9d, vs. last-reviewed commit 22fdd55)
• Review permission-rule-check.sh / .test.sh for injection, unsafe file handling, secret exposure
• Review skill/reference doc changes for trust-boundary or access-control claims
• Review CHANGELOG.md / plugin.json
• Check for GitHub Actions / workflow permission changes
• Post findings

This is the fifth automated pass on this PR (fourth security-lane pass). Rather than re-walk the whole diff, I diffed the current HEAD (3d40b9d) against the last commit this lane reviewed (22fdd55) to isolate what actually changed since the last pass: CHANGELOG.md, audit-instructions/SKILL.md, permission-rule-check.sh, permission-rule-check.test.sh, audit-prompting-postures/SKILL.md, reference/postures.md. plugin.json, criteria.md, and the audit-permission-grants SKILL.md are unchanged since 22fdd55 and were already covered.

Prior finding — now resolved

The disallowed-tools: Edit, NotebookEdit "mechanical" overstatement flagged by every prior security pass on this PR (and by Codex) is fixed at this HEAD. Both skills now state the honest posture instead of the false one:

• audit-prompting-postures/SKILL.md#L27-L32 — "Instruction-held, not tool-enforced — never tell an operator this skill cannot edit their files." Write/Bash are named as retained and capable of mutating an already-read file.
• audit-instructions/SKILL.md#L35-L41 — same correction, same caveat.
• The "What this skill does NOT do" section (audit-prompting-postures/SKILL.md#L198-L199) and the CHANGELOG entry both now say the same thing consistently — no remaining place in the diff still claims mechanical enforcement.

This closes the access-control-adjacent risk: an operator reading the current text can no longer be misled into believing Write/Bash are unavailable during these audits.

New in this diff, reviewed — no security issues

• permission-rule-check.sh — the new readability gate ([[ ! -f "$file" || ! -r "$file" ]]) and reconcile_frontmatter invariant check are read-only additions: they classify already-enumerated paths into disjoint counters and printf static-format diagnostics (%d/%s with no user-controlled format strings). extract_allowed_tools's stderr is now folded into the existing mktemp-backed WALK_ERR file rather than escaping to the terminal — no new file is created outside the existing trap-cleaned scratch file, and no path or file content is echoed back into a shell context. No new variable expansion is left unquoted; no new eval, command substitution built from file content, or jq filter built from untrusted input.
• permission-rule-check.test.sh — the new negative test constructs a broken copy of the script via sed on a known literal line and runs it in an isolated HOME/CLAUDE_CONFIG_DIR/PERMISSION_HYGIENE_FIXTURE_DIR sandbox — a legitimate self-test pattern operating only on the repo's own trusted script text, not on external input.
• audit-prompting-postures/SKILL.md / reference/postures.md — the P7 fix (Phase C now also follows and reads a delegated destructive-action gate script, not just settings/hook config) tightens the posture-detection contract; it doesn't grant the auditing skill any new tool capability or bypass. No trust-boundary change.
• CHANGELOG.md — prose only, documents the same corrections above.
• No .github/workflows/ files, permissions: blocks, or dependency/action pins are touched anywhere in this diff.

Not re-reported (out of scope for this lane)

A documentation self-consistency question (whether refuted findings are always demoted to info or sometimes silently dropped, SKILL.md:112 vs :175) was already raised inline by a prior review on this PR. It's a doc/correctness matter, not a security finding, so it's left to the code-review lane per this task's scope.

Conclusion: no security issues found in this PR's current state. The one finding this lane raised on every prior pass is fixed as of 3d40b9d.
· Branch: fix/2281-2283-claude-config-audit-contracts

[github-actions]

Warning

Automated review did not complete — this is an infrastructure failure, not a review verdict.

Treat any Claude comment on this PR (including a placeholder like "I'll analyze this and get back to you") as incomplete, not "no findings."

• Run: https://github.com/melodic-software/claude-code-plugins/actions/runs/31590778673
• Failure class: other
• Last SDK result: (no execution file was produced)

Re-run the job to retry the review. A new push re-triggers this lane only if the caller's pull_request triggers include synchronize (the canonical caller omits it).
An automatic retry may already have run — it is skipped when a partial review could duplicate comments, or when the failure class needs an operator (auth).

[github-actions]

Warning

Automated security review did not complete — this is an infrastructure failure, not a review verdict.

Treat any Claude comment on this PR (including a placeholder like "I'll analyze this and get back to you") as incomplete, not "no findings."

• Run: https://github.com/melodic-software/claude-code-plugins/actions/runs/31590778677
• Failure class: other
• Last SDK result: (no execution file was produced)

The check is green on purpose, and it is not evidence. It certifies that a security pass ran, and this one did not complete — but the cause is outside this PR's control, so merging is deliberately left unblocked rather than locking every merge for the length of the outage. Nothing was reviewed at this head. Where this check is required, it is satisfied without that evidence; a human should review security-sensitive changes here before merging.

Re-run the job to retry the review; a new push also retries it only if the caller's pull_request triggers include synchronize (the canonical security caller keeps it). An automatic retry may already have run — it is skipped when a partial review could duplicate comments, or when the failure class needs an operator.

Re-running does NOT help for every class:

• rate-limit that persists across re-runs, or auth — the credential or usage budget needs an operator; retrying will not clear it.
• a run that exhausted its turn budget ("subtype":"error_max_turns" above) will exhaust it again. As the PR author, split the change into smaller PRs; raising --max-turns is a change to the caller workflow, not something you can set on this PR.