Skip to content

refactor(claude-permissions): align allow floor with auto-mode built-ins - #267

Merged
kyle-sexton merged 8 commits into
mainfrom
feat/claude-permissions-auto-mode-floor-trim
Jul 26, 2026
Merged

refactor(claude-permissions): align allow floor with auto-mode built-ins#267
kyle-sexton merged 8 commits into
mainfrom
feat/claude-permissions-auto-mode-floor-trim

Conversation

@kyle-sexton

@kyle-sexton kyle-sexton commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

What

Re-derives the claude-permissions allow floor against Claude Code auto mode's
built-in coverage (fleet default is now permissions.defaultMode: "auto";
migration tracked in melodic-software/dotfiles#309, this PR is its P1b phase).
Allow floor 90 → 81; deny grows by 14 pull-spelling rows (--no-verify and forced-fetch families mirrored onto git pull, both shells — the review round that also dropped the rerun grant); the component carries no ask array.

An earlier shape of this PR trimmed 45 rows on the premise "auto mode covers
read-only commands promptless." That premise was then MEASURED and found false
for everything except read-only git (empirical record: the operator-local
migration file .work/auto-mode-defaults/RESEARCH.md, "Addendum — built-in
read-only set, MEASURED" — not repo-reachable, so the decisive figures are
inlined here; claude -p --setting-sources "" --permission-mode default,
v2.1.219): 9/9 git inspection spellings ran promptless (status, diff,
log, show, branch, branch --list, ls-files, merge-base,
rev-parse); 16/16 non-git commands were denied — every gh read verb, every third-party linter, even
bare --version. Membership is command identity, not flag safety, and it is
tool-agnostic (git ran promptless through PowerShell() too). The trim was
re-derived accordingly:

  • TRIM 16 — read-only git inspection only (10 Bash() + 6
    PowerShell() spellings). The built-in read-only command set runs these
    promptless in every mode, so floor entries for them are dead weight
    everywhere, not just under auto.
  • RESTORE 29 — the read-only gh verbs, third-party linters, and
    claude plugin inspection rows the earlier shape wrongly trimmed. Measured
    denied without them; they are load-bearing in every non-auto lane (workflow
    subagents always run acceptEdits; headless -p/SDK runs fail with nobody
    to answer a prompt).
  • REPLACE 2 dead rows with 4 live spellings — the bare-wrapper rows
    Bash(source-control-babysit-merge *) /
    Bash(source-control-babysit-resolve-thread *) match nothing today: the
    plugin bin/ directory is not on the shell's PATH and the skill invokes
    each wrapper as bash "${CLAUDE_PLUGIN_ROOT}/bin/<wrapper>"
    (claude-code-plugins 3fc72d351c). They are dropped, and the wrappers
    instead carry the same interim path-form convention as the five plugin
    scripts already on the floor — quoted and unquoted
    Bash(bash …${CLAUDE_PLUGIN_ROOT}/bin/<wrapper>…*) spellings, 4 rows.
    Bare-name rows return when claude-code-plugins#843 makes the bare name
    resolve.
  • DO NOT PROMOTE the 4 pytest rows the earlier shape added. A test
    runner executes whatever test files are on disk, and a session that can
    write files plus a blanket pytest grant compose into a general
    code-execution grant wearing a narrow name — the same refusal
    melodic-software/dotfiles#315's git entry records three times. Test runs
    are judged per session.
  • PROMOTE 6 — additions with observed-usage evidence or a reviewed
    unattended-lane need, per the component's change discipline: git pull
    (bare + starred) and four gh write verbs (rationale below). No PowerShell
    parity rows: the PS-mirror convention covers read-only inspection only, and
    write verbs have never carried mirrors.

Promote — issue/PR metadata verbs and CI re-run

  • gh issue close *, gh issue reopen *, gh pr edit * — the same class as
    floor incumbents already present (gh issue comment/create/edit,
    gh pr comment/create/ready); excluding them was an arbitrary gap in an
    otherwise-consistent issue/PR metadata-write posture.

Considered, not promoted (decision record)

Scope limit under auto mode

These grants are deterministic only outside auto mode's classifyAllShell.
That key is NOT yet set fleet-wide: G3a split it out of
melodic-software/dotfiles#315 into its own gated follow-up draft PR in
dotfiles, which lands only after the operator's prose-grant test
(dotfiles#309 P3 test 3) passes. Until then the
narrow pre-classifier short-circuit these rows provide still operates in
auto-mode sessions for matching spellings. Once the key lands, every shell
allow rule is suspended in auto-mode sessions and this floor is the fallback
posture for non-auto lanes — which the measurement above shows are exactly the
lanes that need it.

deny is unaffected in every mode and every regime — classifyAllShell
suspends allow rules only; the 262 deny entries stay pre-classifier and
non-overridable, which is exactly why G1 keeps all of them.

Why

Per-entry disposition (reconciles to 90)

Baseline 90 = TRIM 16 + DROP 2 + KEEP 72; final 81 = KEEP 72 + PROMOTE 5 +
REPLACE 4 (the wrappers' path-form spellings). A review round later dropped the
sixth PROMOTE candidate (gh run rerun * — a bare run id names any accessible
run, so no floor glob holds it to merged workflow code; judged per session
until a guarded wrapper exists).

TRIM — 16 entries (read-only git, built-in-covered, measured)
Entry
Bash(git branch --list *)
Bash(git branch)
Bash(git diff *)
Bash(git log *)
Bash(git ls-files *)
Bash(git merge-base *)
Bash(git rev-parse *)
Bash(git show *)
Bash(git status *)
Bash(git status)
PowerShell(git diff *)
PowerShell(git log *)
PowerShell(git merge-base *)
PowerShell(git rev-parse *)
PowerShell(git show *)
PowerShell(git status *)
DROP 2 / REPLACE 4 — bare wrappers out, path-form spellings in

Dropped (match nothing until ccp#843):

Entry
Bash(source-control-babysit-merge *)
Bash(source-control-babysit-resolve-thread *)

Added in their place (the real invocation shape, parity with the five
script rows):

Entry
Bash(bash "${CLAUDE_PLUGIN_ROOT}/bin/source-control-babysit-merge"*)
Bash(bash ${CLAUDE_PLUGIN_ROOT}/bin/source-control-babysit-merge*)
Bash(bash "${CLAUDE_PLUGIN_ROOT}/bin/source-control-babysit-resolve-thread"*)
Bash(bash ${CLAUDE_PLUGIN_ROOT}/bin/source-control-babysit-resolve-thread*)
KEEP — 72 entries

Everything else in the baseline: the non-destructive working verbs, the gh
write incumbents, the RESTORED read-only gh/linter/claude plugin rows
(measured load-bearing), the 10 interim ${CLAUDE_PLUGIN_ROOT} script-path
rules (README carries the interim note; end state is bare wrappers on the
plugin bin/ PATH, trigger claude-code-plugins#843), and the PowerShell
read-only mirrors outside the built-in git set.

PROMOTE — 6 entries
Entry Evidence
Bash(gh issue close *) metadata-write parity with floor incumbents
Bash(gh issue reopen *) metadata-write parity with floor incumbents
Bash(gh pr edit *) metadata-write parity with floor incumbents
Bash(git pull *) operator runtime keeper (dotfiles#309 P2 keep set)
Bash(git pull) operator runtime keeper (dotfiles#309 P2 keep set)

Status

DRAFT — stays draft pending operator review of this amended body; merges AFTER
melodic-software/dotfiles#315 (merging this first would trigger the
standards→dotfiles sync bot to rewrite .chezmoidata/claude-permissions.json
mid-flight under #315).

Related

No linked issue. This is the P1b phase of a cross-repo migration; it closes no
issue in this repository.

🤖 Generated with Claude Code

Trim the 45 read-only allow entries auto mode already handles promptless
(read-only git/gh inspection, read-only lint tooling, claude plugin
inspection), keep the 45 deterministic grants it does not (non-destructive
working verbs, write-capable gh verbs, babysit-lane gate tooling incl. the
10 interim interpreter+script-path rules), and promote 6 operator runtime
keepers with observed-usage evidence (git pull, pytest forms). Allow floor
90 -> 51; deny/ask untouched per dotfiles#309 G1.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TJn7ZVyCCHQfVDmVNjJxBj
@claude

claude Bot commented Jul 24, 2026

Copy link
Copy Markdown

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


PR Review

  • Gather context (REVIEW.md, changed files, git diff)
  • Verify entry counts and math
  • Review README.md changes
  • Review claude-permissions.json changes
  • Check alignment with CLAUDE.md / REVIEW.md guidelines
  • Post findings

Scope: components/claude-permissions/README.md (+14/-4) and claude-permissions.json (+6/-45). No security-blocking criteria from REVIEW.md apply here (no auth paths, no tenant data, no secrets, no injection surface, no multi-record atomicity). All findings are Suggestions.


Math and disposition audit

Entry count reconciles cleanly:

Bucket Count
KEEP (working verbs + babysit lane) 35
KEEP interim (script-path entries) 10
PROMOTE (new) 6
Total allow (post-PR) 51

90 − 45 (trim) + 6 (promote) = 51. ✓ Consistent with the PR body's stated "Allow floor 90 → 51."

The TRIM set (45 entries) is all read-only git/gh inspection, read-only lint tooling, and claude plugin wildcard forms — each falls into categories the PR correctly identifies as covered by auto mode's built-in read-only handling or dropped on entry by the interpreter-wildcard classifier.

The PROMOTE justification (observed-usage evidence from dotfiles#309 P2 keep set) meets the change discipline stated in the README.


🟡 Nit — bare python -m pytest forms missing

The promote adds pytest / pytest * as a matched pair, but only the starred variants for the python -m invocations:

"Bash(python -m pytest *)",    // line 44
"Bash(python3 -m pytest *)",   // line 45

No bare Bash(python -m pytest) or Bash(python3 -m pytest) counterparts are present. An agent running python -m pytest with no arguments (e.g. to run the full suite from a project root) won't match either rule and will be prompted in an auto-mode session where the test runner is invoked without positional args.

claude-permissions.json lines 44–45


🟡 Nit — gh pr diff * and gh pr checks * classification warrants a note

Both are kept as bash-gh-mixed while gh pr view *, gh pr list *, gh run view *, and gh run list * are trimmed as bash-gh-readonly. The PR body's KEEP rationale is "External System Writes soft_deny would otherwise consent-gate them," but gh pr diff and gh pr checks are read operations — the same character as gh pr view.

If there is observed classifier evidence that these two specifically trigger the soft_deny gate (and gh pr view does not), a one-line comment in the README's allow description or a table column in the PR body would make the distinction load-bearing rather than implicit. If they were kept conservatively, that's fine too, but documenting the reason prevents a future trimming pass from removing them on the assumption that they're the same as gh pr view.

claude-permissions.json lines 11, 13


README changes

The updated allow section description accurately reflects the new posture — stating what's absent and why (auto mode covers it) rather than listing what's present. The ${CLAUDE_PLUGIN_ROOT} interim-shapes note with the #843 trigger is the right way to signal technical debt without embedding a comment in the JSON. Both changes are clear and consistent with the component's composition model documentation.


Overall: The trim is well-reasoned and the disposition table in the PR body reconciles correctly. The two nits above are the only issues; neither is blocking. Pending operator opt-in per dotfiles#309 as noted in the draft status.

Add gh issue close/reopen and gh pr edit — the same metadata-write class
as the floor's incumbent issue/PR verbs, closing an arbitrary gap in an
otherwise-consistent posture — and gh run rerun, which re-executes
already-merged workflow code and is needed by the loop lanes for flaky
CI. No PowerShell parity rows: write verbs carry no mirrors. Allow floor
90 -> 55.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01H6gcbtrPYdtMMwx2M8PfEu
@claude

claude Bot commented Jul 24, 2026

Copy link
Copy Markdown

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


PR Review

  • Gather context (REVIEW.md, changed files, git diff)
  • Review new commit: promote four gh write verbs to allow floor
  • Verify entry counts and math against updated JSON
  • Check README alignment with new additions
  • Post findings

Scope: components/claude-permissions/README.md and claude-permissions.json, across two commits. No blocking REVIEW.md criteria apply (no auth paths, no tenant data, no secrets, no injection surface, no multi-record atomicity). All findings are Suggestions → 🟡 Nit.


Entry count audit

The second commit (b32338a) adds 4 entries beyond the first commit's 51:

Bucket Count
KEEP (working verbs + babysit lane) — from first commit 35
KEEP interim (script-path) — from first commit 10
PROMOTE (first commit) 6
PROMOTE (second commit — gh issue close/reopen, gh pr edit, gh run rerun) 4
Total allow (current HEAD) 55

The second commit message correctly says "Allow floor 90 → 55." ✓ The JSON (55 entries) and commit message are consistent.


🟡 Nit — PR body count is stale

The PR body's "Allow floor 90 → 51" and the per-entry disposition tables predate the second commit; they're now off by 4. The commit message for b32338a already says "90 → 55," so the source of truth is correct, but a reader comparing the PR description to the final JSON will see a mismatch. The PROMOTE tables in the description are also missing the four new entries.

Not blocking — the commit message is accurate — but worth updating before this comes out of draft.


🟡 Nit — gh run rerun * permits cross-repo CI triggers

The README qualifies this as "CI re-runs of already-merged workflow code," but the rule pattern Bash(gh run rerun *) also matches gh run rerun <id> --repo org/other-repo, enabling an agent to trigger workflow runs in repositories it doesn't own. The deny list has no corresponding gh run rerun * --repo * entry.

If cross-repo rerun is not intended coverage, adding a deny entry for Bash(gh run rerun * --repo *) / Bash(gh run rerun *--repo*) would narrow the scope to the current working repo.

claude-permissions.json line 24


🟡 Nit — gh pr edit * incidentally covers base-branch changes

gh pr edit --base <branch> redirects a PR's target branch — a structural PR operation (the target of the merge) distinct from metadata edits (title/body/labels/assignees). The * wildcard covers this incidentally. A mis-directed agent could reassign a PR's base from main to an arbitrary branch, which is harder to notice than a label change.

If base-branch redirection is not intended, Bash(gh pr edit * --base *) could be added to deny, or the allow entry narrowed to exclude --base.

claude-permissions.json line 18


🟡 Nit — bare python -m pytest forms missing (carry-forward)

Bash(pytest) has a bare counterpart alongside Bash(pytest *), but Bash(python -m pytest) and Bash(python3 -m pytest) do not — only the starred forms exist. An agent running the full suite with python -m pytest (no positional args) will be prompted. This was flagged in the previous review cycle and remains unresolved.

claude-permissions.json lines 48–49 Fix this →


README changes

The updated allow description now accurately reflects the two-commit outcome: non-destructive working verbs, PR/issue CRUD, gh run rerun for CI, pytest forms, and the babysit lane's tooling. The explicit absence statement for read-only inspection is clear and the #843 interim note is unchanged. Both changes are well-aligned with the new JSON posture.


Overall: The trim rationale is sound, the new promotions fit the "observed usage + write-verb" pattern, and the second commit's math (55) is internally consistent. The four nits above are the only issues; none are blocking. The draft status and dotfiles#309 gating remain appropriate.

Every allow entry on this floor is a shell rule, and a consumer setting
autoMode.classifyAllShell: true suspends all shell allow rules. The
component's own README described these grants as deterministic without
that qualifier, which is false for the fleet's own configuration once
dotfiles#315 lands. Record the scope limit, note that deny is unaffected
because the setting suspends allow rules only, and tell contributors to
check whether a prompting session routes shell through the classifier
before proposing a floor addition that cannot fix it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01H6gcbtrPYdtMMwx2M8PfEu
@claude

claude Bot commented Jul 24, 2026

Copy link
Copy Markdown

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


PR Review

  • Gather context (REVIEW.md, changed files, git diff)
  • Review new commit: scope allow floor to non-auto sessions (README only)
  • Verify carry-forward nits from prior cycles
  • Check README alignment with JSON state
  • Post findings

Scope: Commit 4fc50c0 is README-only (+30/-9). No changes to claude-permissions.json in this push. No blocking REVIEW.md criteria apply (no auth paths, no tenant data, no secrets, no injection surface, no multi-record atomicity). All findings are Suggestions.


Latest commit — README assessment

The scope-limit block is accurate and well-placed: it correctly states that classifyAllShell: true suspends allow rules only and that deny entries are unaffected, references the authoritative docs URL, and gives contributors actionable guidance (check whether the prompting session routes through the classifier before proposing a floor addition). The rewrite of the allow bullet description now accurately conveys the posture and the explicit absence of read-only entries. No inaccuracies found; no new issues introduced.


Carry-forward nits (three unresolved from prior cycles)

🟡 Nit — bare python -m pytest forms still missing (flagged twice)

Bash(pytest) / Bash(pytest *) are a matched pair; Bash(python -m pytest *) and Bash(python3 -m pytest *) (lines 48–49) have no bare counterparts. An agent invoking python -m pytest or python3 -m pytest with no positional arguments will prompt in a non-auto session.

claude-permissions.json lines 48–49 Fix this →


🟡 Nit — gh run rerun * permits cross-repo CI triggers (flagged once)

The pattern Bash(gh run rerun *) matches gh run rerun <id> --repo org/other-repo, allowing an agent to trigger workflow runs outside the current repository. No deny entry narrows this to the working repo. If cross-repo rerun is not intended, adding Bash(gh run rerun * --repo *) to deny would restrict coverage to same-repo runs.

claude-permissions.json line 24


🟡 Nit — gh pr edit * incidentally covers base-branch redirection (flagged once)

gh pr edit --base <branch> changes a PR's merge target, which is a structural operation distinct from metadata edits (title/body/labels/assignees). A mis-directed agent could silently retarget a PR away from main. If base-branch redirection is not intended, Bash(gh pr edit * --base *) in deny would close that gap.

claude-permissions.json line 18


🟡 Nit — PR body math is stale (carry-forward)

The PR description still reads "Allow floor 90 → 51" in multiple places; the current JSON has 55 entries (90 − 45 + 10 = 55), matching the b32338a commit message. Worth updating before this comes out of draft so the description reconciles with the JSON.


Summary

The latest commit adds accurate, needed documentation about auto-mode scope; no new issues. Four carry-forward nits remain open from prior review cycles — none are blocking. Math (55 entries) and JSON are consistent; the README now correctly characterises both what the floor grants and where those grants are inert.

kyle-sexton and others added 2 commits July 24, 2026 22:51
…verage

The trim premise was false. "Auto mode covers these read-only entries"
does not reach the lanes that never enter auto mode -- workflow
subagents always run acceptEdits regardless of session mode, headless
-p/SDK runs cannot answer a prompt, and a non-auto session is the
posture this floor exists to define. Under classifyAllShell: true the
premise buys nothing even inside auto mode, since no shell allow rule
short-circuits the classifier there.

The correct criterion is Claude Code's built-in read-only command set,
which is promptless in EVERY mode and is not an allow rule, so
classifyAllShell does not suspend it. Membership is command identity and
much narrower than assumed. Verified on 2.1.219 in default mode with
--setting-sources "": read-only git ran promptless through both the Bash
and PowerShell tools, while every gh verb, every third-party linter, and
claude plugin list were denied -- bare --version forms included.

Restore the 29 entries that fail the corrected test (read-only gh
inspection in both spellings, read-only lint tooling, claude plugin
inspection) and keep trimmed only the 16 read-only git rows the built-in
set actually covers. Allow floor 90 -> 84; deny (262) and ask untouched
per dotfiles#309 G1.

Replace the rationale in README.md rather than only the list: cite
permissions "Read-only commands" instead of auto-mode-config, reconcile
the classifyAllShell scope limit against the built-in set with the
timing evidence, and give removals an explicit empirical test so the
next contributor cannot re-derive the same mistake.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01H6gcbtrPYdtMMwx2M8PfEu
…pper rows

A test runner executes whatever test files are on disk, and a session
that can write files plus a blanket pytest grant compose into a general
code-execution grant wearing a narrow name — the same refusal
melodic-software/dotfiles#315's git entry records three times. The two
bare-wrapper rows never match anything today: the plugin bin/ directory
is not on PATH and the skill invokes each wrapper as
bash "${CLAUDE_PLUGIN_ROOT}/bin/<wrapper>" (claude-code-plugins
3fc72d351c); they return when claude-code-plugins#843 makes the bare
name resolve. Allow floor 84 -> 78. README updated for both grounds and
the bare-wrapper interim framing reconciled.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@claude

claude Bot commented Jul 25, 2026

Copy link
Copy Markdown

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


PR Review

  • Gather context (REVIEW.md, changed files, git diff)
  • Review latest commit: drop pytest grants and dead bare-wrapper rows
  • Verify entry counts and math against updated JSON
  • Check carry-forward nit resolution status
  • Post findings

Scope: Commit d59552c removes 6 entries from claude-permissions.json (pytest/python -m pytest forms ×4, bare-wrapper rows ×2) and updates README.md to document both removals. No blocking REVIEW.md criteria apply (no auth paths, no tenant data, no secrets, no injection surface, no multi-record atomicity). All findings are Suggestions.


Latest commit — assessment

Pytest removal (Bash(pytest *), Bash(pytest), Bash(python -m pytest *), Bash(python3 -m pytest *))

The rationale is sound: a session that can write files plus a blanket test-runner grant compose into a general code-execution grant wearing a narrow name — the rule grammar cannot distinguish a test file the agent wrote this session from one already in the repository. The README's new "Test-suite invocations are deliberately absent" block states this cleanly and is the right place for it. This also resolves the bare python -m pytest nit flagged in the two prior review cycles.

Bare-wrapper removal (Bash(source-control-babysit-merge *), Bash(source-control-babysit-resolve-thread *))

Removal is correct. The plugin bin/ directory is not on PATH; the skill invokes each wrapper via bash "${CLAUDE_PLUGIN_ROOT}/bin/<wrapper>" (claude-code-plugins 3fc72d351c), so a rule naming the bare command matches nothing today. The README's updated ${CLAUDE_PLUGIN_ROOT} framing correctly explains why bare-wrapper rules stay OUT and what the end state is (#843).


Entry count audit

Change Count
Pre-commit (after 93be920) 84
Removed (pytest ×4, bare wrappers ×2) −6
Current HEAD 78

Commit message "84 → 78" is accurate. Cumulative from main's 90-entry floor: 90 → 78 (12 net removals). I count 78 entries in the current JSON. ✓


Carry-forward nit resolution

Finding Status
Bare python -m pytest forms missing ✅ Resolved — all pytest forms removed
gh run rerun * cross-repo scope 🟡 Still open
gh pr edit * base-branch redirection 🟡 Still open
PR body disposition tables stale 🟡 Now worse — see below

🟡 Nit — PR body disposition tables are now significantly stale (carry-forward, escalated)

The PR body's TRIM-45 table lists Bash(actionlint *), Bash(claude plugin details/list/validate *), Bash(gh issue list *), Bash(gh pr list *), Bash(gh run list/view *), all linter entries, and the PowerShell gh read-only set as trimmed. Commit 93be920 re-added all of those after empirical testing showed they ARE NOT covered by the built-in set. The net change from main is 90 → 78, not 90 → 51 or 90 → 55.

Additionally, the PROMOTE table still lists the four pytest forms, and the KEEP-35 table still lists source-control-babysit-merge * and source-control-babysit-resolve-thread * — all six are now removed. The README accurately reflects the current posture; the PR body does not. Worth a squash or a description update before this comes out of draft.


🟡 Nit — gh run rerun * permits cross-repo CI triggers (flagged twice, carry-forward)

Bash(gh run rerun *) matches gh run rerun <id> --repo org/other-repo. The deny list has no Bash(gh run rerun * --repo *) entry to narrow scope to the working repo. If cross-repo rerun is not intended, adding that deny entry would close the gap.

claude-permissions.json line 37


🟡 Nit — gh pr edit * incidentally covers base-branch redirection (flagged twice, carry-forward)

gh pr edit --base <branch> changes a PR's merge target — a structural operation distinct from metadata edits (title, body, labels, assignees). The * wildcard covers it incidentally. If base-branch redirection is not intended, adding Bash(gh pr edit * --base *) to deny would close that gap.

claude-permissions.json line 27


Overall: The pytest removal and bare-wrapper cleanup are both well-reasoned and clean. The README accurately captures the current posture including the new deliberately-absent sections. Three nits remain open from prior cycles; none are blocking. The draft status and dotfiles#309 gating remain appropriate.

…shape

Round-2 verifier findings. The two guarded bin/ wrappers now follow the
same interim path-form convention as the five plugin scripts — quoted
and unquoted bash "${CLAUDE_PLUGIN_ROOT}/bin/<wrapper>" spellings — so
dropping the dead bare rows no longer leaves the babysit gate tooling
with no floor coverage at all (allow 78 -> 82). README enumerates all
nine measured promptless git spellings (git branch --list was measured
separately), names the fleet default the trim rationale leans on, and
the interim paragraph now covers scripts and wrappers alike.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@claude

claude Bot commented Jul 25, 2026

Copy link
Copy Markdown

Claude encountered an error after 19s —— View job


I'll analyze this and get back to you.

@kyle-sexton

Copy link
Copy Markdown
Contributor Author

Content complete at 99b4648 and re-verified by a fresh-context auditor (counts reconcile: baseline 90 = TRIM 16 + DROP 2 + KEEP 72; final 82 = 72 + PROMOTE 6 + REPLACE 4 path-form wrapper spellings; JSON/README/body consistent). Body amended to match. Two gates remain before merge, in order: (1) operator reads this amended body, then un-drafts; (2) merges only AFTER melodic-software/dotfiles#315 lands, so the standards-sync bot's dotfiles PR does not rewrite claude-permissions.json mid-flight under #315.

@kyle-sexton
kyle-sexton marked this pull request as ready for review July 25, 2026 22:22
@claude

claude Bot commented Jul 25, 2026

Copy link
Copy Markdown

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


PR Review

  • Gather context (REVIEW.md, changed files, git diff)
  • Verify entry counts and math (baseline 90 → final 82)
  • Review README.md changes
  • Review claude-permissions.json changes
  • Verify carry-forward nit resolution status
  • Post findings

Scope: components/claude-permissions/README.md (+102/-4) and claude-permissions.json (+10/-18) across six commits. No blocking REVIEW.md criteria apply (no auth paths, no tenant data, no secrets, no injection surface, no multi-record atomicity). All findings are Suggestions.


Entry count audit

Bucket Count
KEEP (working verbs, gh write incumbents, RESTORED gh/linter/claude rows) 62
KEEP — interim ${CLAUDE_PLUGIN_ROOT} script/wrapper path-form rules 10
PROMOTE (6 new allow entries) 6
REPLACE — path-form wrapper spellings (4 in, 2 bare dead rows out) 4
Total allow (current HEAD) 82
deny 262

Manually verified: 82 entries in allow, 262 in deny. Arithmetic: 90 − 16 (TRIM) − 2 (DROP) + 4 (REPLACE) + 6 (PROMOTE) = 82 ✓. PR body's stated "Allow floor 90 → 82" is accurate. Deny is unchanged at 262 ✓.


Latest commit (99b4648) assessment

The four new wrapper path-form entries follow the exact same quoted + unquoted convention as the five existing script rows:

"Bash(bash \"${CLAUDE_PLUGIN_ROOT}/bin/source-control-babysit-merge\"*)",
"Bash(bash \"${CLAUDE_PLUGIN_ROOT}/bin/source-control-babysit-resolve-thread\"*)",
"Bash(bash ${CLAUDE_PLUGIN_ROOT}/bin/source-control-babysit-merge*)",
"Bash(bash ${CLAUDE_PLUGIN_ROOT}/bin/source-control-babysit-resolve-thread*)"

These are correctly interleaved in alphabetical order within the allow array. The README update adds git branch --list to the measured nine-spelling enumeration (commit message confirms it was measured separately from bare git branch) and names permissions.defaultMode: "auto" as the fleet default explicitly — both accurate. The interim paragraph now correctly covers both plugin scripts and bin/ wrappers in a single description. No inaccuracies found in the latest commit.


Carry-forward nit resolution

Finding Status
Bare python -m pytest forms missing (flagged ×2) ✅ Resolved — all pytest forms removed in d59552c
PR body disposition tables stale (flagged ×3) ✅ Resolved — operator amended body at 99b4648; body now reconciles to 90 → 82
gh run rerun * cross-repo scope (flagged ×3) 🟡 Still open
gh pr edit * base-branch redirection (flagged ×3) 🟡 Still open

🟡 Nit — gh run rerun * permits cross-repo CI triggers (carry-forward ×3)

Bash(gh run rerun *) matches gh run rerun <id> --repo org/other-repo, enabling an agent to trigger workflow runs outside the current working repository. The deny list carries no Bash(gh run rerun * --repo *) entry to narrow scope. If cross-repo rerun is not intended coverage, adding that deny entry would close the gap.

claude-permissions.json line 41


🟡 Nit — gh pr edit * incidentally covers base-branch redirection (carry-forward ×3)

gh pr edit --base <branch> changes a PR's merge target — a structural operation distinct from metadata edits (title, body, labels, assignees). The * wildcard covers this incidentally. A mis-directed agent could silently retarget a PR away from main. If base-branch redirection is not intended, Bash(gh pr edit * --base *) in deny would close the gap.

claude-permissions.json line 31


README assessment

The final README accurately represents the component's current posture in full: the deliberately-absent test-suite invocations block (with rationale), the deliberately-absent read-only git inspection block (with the nine measured spellings and the carve-out warning), the auto-mode scope limit block (with the two-collision note), and the interim ${CLAUDE_PLUGIN_ROOT} block updated to cover both scripts and wrappers. No inaccuracies found. The change discipline section's removal criterion (must test, never infer) and the test recipe are unchanged and correct.


Overall: The trim is well-reasoned and every entry in the per-entry disposition section reconciles to the JSON. The README is the most complete and accurate description this component has carried. Two carry-forward nits remain open after three flaggings each — both are Suggestions, neither is blocking. The draft → ready-for-review transition, merge-order dependency on dotfiles#315, and operator review gate noted in the PR body remain the appropriate process controls.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 99b464882a

ℹ️ 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".

Comment thread components/claude-permissions/claude-permissions.json
Comment thread components/claude-permissions/claude-permissions.json Outdated
kyle-sexton added a commit to melodic-software/claude-code-plugins that referenced this pull request Jul 25, 2026
…d value (#1275)

No linked issue

## Summary

`block-dangerous-git` treated every `--force-with-lease` spelling as
safe force. Two of them are not, by git's own account, and the guard let
them through while blocking `--force` for the same underlying hazard.

## Fix

`--force-with-lease` and `--force-with-lease=<refname>` state no
expected value, so git leases against the **remote-tracking ref**. From
[git-push(1)](https://git-scm.com/docs/git-push), "A general note on
safety":

> supplying this option without an expected value, i.e. as
`--force-with-lease` or `--force-with-lease=<refname>` interacts very
badly with anything that implicitly runs `git fetch` on the remote to be
pushed to in the background, e.g. `git fetch origin` on your repository
in a cronjob.
>
> The protection it offers over `--force` is ensuring that subsequent
changes your work wasn't based on aren't clobbered, but this is
**trivially defeated if some background process is updating refs in the
background**.

So the lease can be satisfied by a ref *some other process* fetched, and
the push clobbers work the pusher never saw — the failure mode `--force`
has, reached by a slower road. Git also marks every form other than
`=<refname>:<expect>` experimental.

Those two no-expected-value forms are now blocked under a new
**`push-lease-unsafe`** token, unless `--force-if-includes` (git 2.30+)
is present — git's documented mitigation for exactly these forms, which
it declares a no-op alongside an explicit `:<expect>`.

### What still passes

- `--force-with-lease=<refname>:<expect>`, including an empty `<expect>`
(asserts the ref must not exist — still explicit).
- Any lease form paired with `--force-if-includes`.
- A push dry-run, which disarms the whole check as before.

### Detection detail

Unique-prefix abbreviations are handled. `--force`, `--force-with-lease`
and `--force-if-includes` share the `--force` prefix, so `--force-w` and
`--force-i` are the shortest spellings git accepts, and both match. A
shorter `--forc` is ambiguous and git rejects it outright, which is why
the exact `--force` arm needs no abbreviation handling. After `--`,
words are operands rather than flags, so a literal `--force-if-includes`
refspec does not disarm the check.

## Why the hook, and not the permission deny-list

This started from the opposite direction: a `--force-with-lease` push
was denied by the `claude-permissions` floor, and the obvious fix looked
like removing that deny.

Research against [the permissions
docs](https://code.claude.com/docs/en/permissions.md) killed that:

> Rules are evaluated in order: deny, then ask, then allow. The first
match in that order determines the outcome, and rule specificity doesn't
change the order.

> A broad deny rule like `Bash(aws *)` blocks every matching call,
including calls that also match a narrower allow rule like `Bash(aws s3
ls)`, so a deny rule can't carry allowlist exceptions.

Bash rules are whole-string globs with `*` as the only metacharacter —
no negation. So "deny the unsafe lease forms, allow `=<ref>:<expect>`"
is **not expressible** in the permission language, and the docs name a
PreToolUse hook as the mechanism for what globs cannot express. This is
that hook.

The blunt deny in `melodic-software/standards` is therefore doing a job
the permission layer cannot do precisely. Removing it before this landed
would have been a net widening — it would have exposed the unsafe bare
form, which `guardrails` permitted. With this merged, that deny can be
dropped and the policy becomes: safe form allowed, unsafe forms blocked,
both enforced where the distinction is actually expressible.

## Testing

`block-dangerous-git.test.sh` — **261 pass, 0 fail**. 14 new cases:
bare, `=<refname>`, `=<refname>:<expect>`, empty `<expect>`, both
abbreviations, `--force-if-includes` alone and paired, dry-run, and the
`--` operand boundary. Three existing cases asserted the old permissive
behavior and were updated to the new contract; one PowerShell case
likewise, plus a new PowerShell case for the passing form.

`shellcheck -x` clean at the repo ruleset. `markdownlint-cli2` clean.
`plugin.json` validates.

## Related

- `melodic-software/standards#267` — in flight on the same
`claude-permissions` component (it trims the **allow** floor; this
affects **deny** policy). Its README states "Force/destructive spellings
stay covered by `deny`, which always wins" — the follow-up that drops
the lease deny will need to update that sentence.
- Follow-up, not in this PR: remove the four `--force-with-lease` deny
patterns (`Bash` and `PowerShell` mirrors) from the `claude-permissions`
component now that the precise check exists here.

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
kyle-sexton added a commit that referenced this pull request Jul 25, 2026
… in the hook (#272)

## Summary

The permission floor denied every `--force-with-lease` spelling —
including the forms that are actually safe. This removes those four
patterns and moves enforcement to the one place the distinction can be
expressed.

## Why `deny` cannot do this job

Claude Code's Bash rules are whole-string globs with `*` as the only
metacharacter, and precedence is fixed
([permissions](https://code.claude.com/docs/en/permissions)):

> Rules are evaluated in order: deny, then ask, then allow. The first
match in that order determines the outcome, and rule specificity doesn't
change the order.

> A broad deny rule like `Bash(aws *)` blocks every matching call,
including calls that also match a narrower allow rule like `Bash(aws s3
ls)`, so a deny rule can't carry allowlist exceptions.

No negation, no exceptions. So a `deny` here is all-or-nothing.

## Why that matters — the forms are not equivalent

[git-push(1)](https://git-scm.com/docs/git-push), "A general note on
safety":

> supplying this option without an expected value, i.e. as
`--force-with-lease` or `--force-with-lease=<refname>` interacts very
badly with anything that implicitly runs `git fetch` … this is
**trivially defeated if some background process is updating refs in the
background**.

Only `--force-with-lease=<refname>:<expect>` states the expectation, and
it is the one form git does not mark experimental. A single glob cannot
deny the unsafe spellings and permit the safe one — so the floor denied
all of them, which is why a correctly-formed safe push was blocked in
practice.

## What replaces it

The `guardrails` plugin's `block-dangerous-git` PreToolUse hook, which
parses the argv and can therefore make the distinction:

- Blocks any lease that resolves against something **movable at push
time** — bare, `=<refname>`, or an `=<refname>:<expect>` whose
`<expect>` is a name like `origin/main`, `HEAD`, or `refs/remotes/...`.
- Permits an expectation git cannot resolve to something newer: an
object id, or the empty string (asserting the ref must not exist).
- Honors `--force-if-includes` (git 2.30+) as git's documented
mitigation for the unpinned forms.
- Tracks the last-wins negations git documents as
`--[no-]force-with-lease` and `--[no-]force-if-includes`.

Claude Code's own docs name a PreToolUse hook as the mechanism for
exactly what globs cannot express.

## Ordering — this lands second, deliberately

Removing the deny on its own would have been a **net widening**. The
hook permitted every lease form until
melodic-software/claude-code-plugins#1275, so dropping the blunt rule
first would have exposed the unsafe spellings with nothing catching
them. The hook hardens first; the blunt rule comes out after.

## README

"Force/destructive spellings stay covered by `deny`, which always wins"
is corrected rather than deleted — it still holds for every other force
spelling. The carve-out is recorded beneath it with the reason and both
upstream citations, so the next reader does not re-add the patterns.

## Verification

- `claude-permissions.json` validates; the four removed entries are the
only change to it (2 Bash, 2 PowerShell mirrors).
- Pre-commit gates green: biome, editorconfig, gitleaks, typos,
markdownlint.

## Related

- melodic-software/claude-code-plugins#1275 — the hook that makes this
safe. **Merge that first.**
- #267 — in flight on the same component; it trims the **allow** floor
while this touches **deny**, so they should auto-merge. Its README
rewrite touches the same paragraph, so whichever lands second should
confirm the carve-out survived.

No linked issue: this is the second half of the
melodic-software/claude-code-plugins#1275 change, not a separately
tracked defect.

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
@kyle-sexton
kyle-sexton enabled auto-merge (squash) July 26, 2026 00:36
@claude

claude Bot commented Jul 26, 2026

Copy link
Copy Markdown

Claude encountered an error after 2s —— View job


I'll analyze this and get back to you.

@github-actions

github-actions Bot commented Jul 26, 2026

Copy link
Copy Markdown

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."

Re-running the job, or pushing a new commit, will retry the review.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: aecd7b8250

ℹ️ 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".

Comment thread components/claude-permissions/claude-permissions.json
…grant

The pull wildcard pre-approved git pull --no-verify (documented as
bypassing the pre-merge and commit-msg hooks) and the --force/-f/
+<refspec> forced-overwrite forms the fetch family already denies —
deny now carries the pull spellings of both families in both shells,
args-before-flag forms included. And gh run rerun takes a bare run id
naming any accessible run, so no floor glob can hold it to flaky CI on
merged workflow code — the allow row is dropped (81 rows) and reruns
are judged per session, recorded beside the test-suite absence with
the guarded-wrapper end state named.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Ha8DkNT4nSnVjKDNWVpj3w
@claude

claude Bot commented Jul 26, 2026

Copy link
Copy Markdown

Claude encountered an error after 2s —— View job


I'll analyze this and get back to you.

@kyle-sexton
kyle-sexton merged commit a8ad326 into main Jul 26, 2026
45 checks passed
@kyle-sexton
kyle-sexton deleted the feat/claude-permissions-auto-mode-floor-trim branch July 26, 2026 01:24

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 318114b476

ℹ️ 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".

"Bash(git pull * -f *)",
"Bash(git pull * -f)",
"Bash(git pull --force*)",
"Bash(git pull -f *)",

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Deny the exact git pull -f spelling

In a repository whose configured fetch refspec maps a remote ref directly to another local branch, bare git pull -f force-updates that branch and can discard unpushed commits; git pull -h describes -f as “force overwrite of local branch” (git-pull documentation). The fresh deny rows added after the earlier review still only match git pull -f with a trailing space, while the new wildcard allow pre-approves the bare spelling. Add exact Bash(git pull -f) and corresponding PowerShell denies.

Useful? React with 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant