Skip to content

feat(claude-config): report the Claude Code permission state actually in effect - #2260

Merged
kyle-sexton merged 16 commits into
mainfrom
feat/audit-permission-state
Aug 12, 2026
Merged

feat(claude-config): report the Claude Code permission state actually in effect#2260
kyle-sexton merged 16 commits into
mainfrom
feat/audit-permission-state

Conversation

@kyle-sexton

@kyle-sexton kyle-sexton commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

Summary

Two new skills under claude-config, plus scope widening of two existing checks, that make the
Claude Code permission plane legible to a consumer.

/permissions lists your rules and the file each came from. It does not resolve which of two
conflicting rules wins, cannot tell a scope that was empty from one it could not read, and exists
only inside a live session. There is no claude permissions subcommand and no machine-readable
export. Meanwhile auto mode became the default for new sessions and silently drops broad allow
rules on entry, with no signal to the operator.

What ships

claude-config:audit-permission-state — report-only, six stages:

  1. Scope discovery across all five scopes — managed policy, user-global, project, local, and the
    pre-v2.1.211 start-directory copy whose rules stay in effect alongside the repo-root one. Every
    scope and managed surface emits a record on every OS, so a surface never attempted is never
    mistaken for one genuinely empty: absent means looked and found nothing, skipped means could
    not look.
  2. The merged effective set with per-rule provenance. Permission rules merge across scopes
    rather than override
    , so a rule at two scopes has no winner and is never reported as one. What a
    rule loses is its kind: deny before ask before allow, from any scope, in both directions — a
    user-level deny blocks a project-level allow just as the reverse. Beaten entries ship as inert
    records naming what beat them, which answers "why is my allow rule ignored".
  3. The auto-mode entry diff — every effective allow rule classified dropped (blanket, wildcarded
    interpreter, package-manager run, or Agent) or kept, using the same shared pattern vocabulary
    check P1 scans with. Reads autoMode.classifyAllShell, which inverts the answer wholesale.
  4. Permission-plane lints — eight checks for configuration written where nothing reads it. The
    highest-consequence one: disableAutoMode typed as a boolean instead of the string "disable" is
    valid JSON, is accepted, and does nothing — so the operator believes auto mode is locked out and
    it is not.
  5. The autoMode classifier block$defaults omissions (which discard the built-in list),
    contradictions, and entries an earlier hard_deny forecloses. claude auto-mode critique is
    surfaced and wrapped, never reimplemented.
  6. Managed conformance — which managed intents are enforced versus loosenable. A managed
    autoMode section is additive, not a policy boundary; a lower-scope deny beats a managed allow
    without overriding it.

claude-config:draft-auto-mode-rules — the authoring counterpart. Interviews and prints a
paste-ready block. The entry shape is critique's own recommendation applied at authoring time.
Writes nothing, in any scope, under any flag.

Two scope widenings — check P1 now sees user-global allow rules (where "Always allow" writes);
audit's structure check reports a start-directory settings.local.json and names the managed
surfaces it does not read.

Evidence

Every upstream fact is quoted from a page fetched during this work, and the citations live in the
skills' own reference/criteria.md — plugin-owned and shipped, not branch-local.

Measured rather than assumed, and one changed what ships:

  • A -p oracle session does not touch settings files (criterion 9 holds) but does rewrite
    ~/.claude.json
    and add project, session-env, security and subagent state. The cost notice
    originally said only "does not modify any settings file" — true, and incomplete for a flag whose
    job is honesty about cost. It now enumerates the measured writes.
  • A plain -p run emits drop lines with no mode flag — 216 of them.
  • An isolated probe cannot authenticate; copying credentials to fake isolation was rejected.
  • v2.1.142 and the useAutoModeDuringPlan scope restriction turned out documented, so criterion
    2 ships uncaveated.

Criterion 9 is proven by running all ten actions of both skills — oracle explicitly ON — and
diffing a fixture tree and fixture HOME either side.
12/12.

Tests: permission-state 42, permission-merge 49, automode-entry-diff 49, permission-plane-lint
43, automode-block-lint 44, managed-conformance 28, draft-automode-block 33, no-writes 12.
shellcheck -x clean, portability gate clean, check-skill PASS on both skills, listing budget
6737/8000 across nine skills.

Things found by building it

  • A whole-tool deny was ignored. deny: ["Bash"] with allow: ["Bash(git status)"] reported both
    as effective — but the bare name removes the tool from context entirely. Caught by fresh-context
    review after the merge had passed 40 checks and its author had signed off.
  • The raw-control-character fixture was silently valid JSON. Regenerated through json.dumps, it
    had escaped the line feed, so the headline regression case was testing nothing. Its own
    jq -e .-must-reject assertion caught it; the fixture is now pinned -text.
  • The reader took 65 seconds per run after Phase 4 added six jq spawns per settings file.
    Collapsed to one: 65s → 7s, identical output.
  • The criterion-9 harness reported "All 10 checks passed" over work it had not done — a helper
    returned a path on stdout, interleaving with its own PASS lines while four stages never ran. Now
    every stage asserts non-empty output and the action count is checked.
  • Criterion 10 was unmet. The permissions.ask discrepancy (#83766, #42797) was in the plan but
    nowhere the skills ship. Now a named caveat with a retirement condition.

Contract slice

docs/topics/permission-model/ is pruned, per the topic-docs convention. Durable outcomes graduated
first: ADR 0009 records the two decisions that outlive the branch (report-never-write; compute the
merge bounded by decidability, both with defensible opposites). The citation base moved into
reference/criteria.md. The allowed-tools trust measurement went into MIGRATION-PLAYBOOK.md's
plugin-acceptance security review — a marketplace skill's allowed-tools grant takes effect in a
never-trusted workspace, so the install-time prompt is the only gate in front of it.

Related

No linked issue — this change set originates from a research and planning thread rather than a filed
issue.

Filed during this work and deliberately not closed by this PR: #2338, the discovery:researcher
skill preload not reaching dispatched agents. It is a defect in a different plugin, and nothing here
fixes it.

🤖 Generated with Claude Code

https://claude.ai/code/session_01TgD5TknENb3ifk1a47eqUr

kyle-sexton and others added 10 commits August 10, 2026 01:12
Records the contract and implementation plan for two new claude-config
skills: audit-permission-state, which computes the effective merged
permission state across all five settings scopes with per-rule
provenance, and draft-auto-mode-rules, the non-writing authoring lane.

The plan is ten phases, sequential. Phase 0 discharges the fresh-docs
mandate before any version-gate constant ships. Phase 9 moved ahead of
the skill work because it owns two seams that do not exist yet: a
fixture-home override so user-global scope is testable without reading
the real home, and a side-effect-free extraction of the shared pattern
vocabulary, which today lives in a script that self-executes at load.

Two probes settled questions the Brief had parked. The debug channel
narrates the permission merge per destination and names every allow rule
auto mode drops, with its source path and reason; it becomes an opt-in,
explicitly priced oracle cross-checked against the computed prediction,
not a replacement for it. A non-strict JSON parser is genuinely required
for one optional lane: the malformed CLI output carries a raw line feed
inside a string value, so no line-oriented filter repairs it, which puts
Python in the optional-feature class and leaves the portable core intact.

A fresh-context reviewer returned nineteen findings against an earlier
draft. All nineteen were re-verified against the repository and all held.
Both critical ones were stale premises: PR #2089 had already merged with
its OFFICIAL-DOCS row, the frontmatter-name refactor had landed on main,
and this slice was untracked on a spent branch and absent from main.

Two items are left open rather than guessed: which second existing check
the Brief meant by widening two, and what a -p oracle session writes
outside its scratch path.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TgD5TknENb3ifk1a47eqUr
Phase 0 re-confirms every upstream fact the two planned skills would ship,
against the auto-mode-config, settings, permission-modes, and permissions
pages fetched today, rather than the single local capture the Brief was
resting on.

Eleven facts are confirmed with quoted wording, including the scope
precedence, the deny-then-ask-then-allow evaluation order, the four
classes of allow rule auto mode drops on entry, and that permission rules
from a pre-v2.1.211 start-directory settings.local.json stay in effect.
The absence of allowManagedAutoModeRulesOnly is now supported by the
governing page rather than by an unverified research slice.

Five corrections land against the plan. The largest: managed policy is
not two JSON files. It is a plist domain on macOS, a registry hive on
Windows, a JSON file, and a managed-settings.d drop-in directory, per OS,
so the managed read cannot be jq over a fixed pair of paths. The local
settings file also resolves through worktrees to the main checkout, which
this topic is itself being planned inside. And autoMode.classifyAllShell
suspends every shell allow rule when set, inverting the drop-set answer
that no criterion currently covers.

Three claims move to not-stated and must not ship as fact: the v2.1.142
gate on project-scope defaultMode, the scope restriction on
useAutoModeDuringPlan, and the Write(path) never-consulted mechanic,
which the page describes differently and with a startup warning as its
observable.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TgD5TknENb3ifk1a47eqUr
Phase 1's managed-policy reader is split into a portable core (the per-OS
managed-settings.json and its managed-settings.d/ drop-in directory, always
read) and two declared optional platform legs (the Windows registry policy
keys and the macOS managed-preferences domain) that warn visibly and skip
rather than failing the run. A Windows reader that checks only the Program
Files JSON reports "no managed policy deployed" while a registry-deployed
policy is in force, so dropping the registry buys a wrong finding rather
than a documented gap. An unelevated reg query of HKLM\SOFTWARE\Policies
was measured to succeed, so the leg costs no elevation.

The Brief's long-unassigned second scope-widening is named: claude-config's
audit skill gains the start-directory settings.local.json copy, whose
permission rules stay in effect and which nothing audits today.

The authoring lane drops its unnamed history input and drafts from the
interview plus the computed merge, removing an unreviewable read surface
and a second dependency on the priced debug-channel oracle.

Phase 9 absorbs the shared managed-scope path enumeration, since approving
the managed reader would otherwise make this repository's third copy of it.

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

Check P1 scanned project and local settings only, so an interpreter-wildcard
rule in the user-global settings file was invisible to it — and that is the
scope Claude Code's own "Always allow" path writes to, so it is where the
broad rules auto mode drops actually accumulate. The user scope resolves
CLAUDE_CONFIG_DIR before $HOME, matching the resolver claude-memory already
cites to the .claude-directory doc; that resolution is also the test seam, so
a fixture home replaces the operator's real one and no test reads it.

The audit skill's structure check gains the pre-v2.1.211 start-directory
settings.local.json, whose permission rules stay in effect alongside the
repository-root copy. The row appears only where the two directories genuinely
differ, so one file is never counted as two rule sources.

Two libraries come out of the widening. The auto-mode drop vocabulary was
inline in a detector that self-executes and cannot be sourced, leaving a
second consumer no way to reuse it; the per-OS managed-policy locations were
hand-kept in three places that had already disagreed about whether the
drop-in directory exists. claude-memory carries a byte-identical copy of the
managed-scope library, registered as a cross-plugin cluster so the two cannot
drift — verified by perturbing a copy and watching the check fail.

Both file-only readers now name the managed surfaces they do not read (the
Windows policy registry keys, the macOS preferences domain) so an absent
managed-settings.json cannot read as "no managed policy deployed".

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

With neither CLAUDE_CONFIG_DIR nor HOME set there is no user-global settings
file to resolve, and the detector scanned nothing without saying so — which
would let "No fragile permission grants found." rest on a scope that was
never opened. It now writes a named notice to stderr, with a regression test
covering it.

The plugin README carries the widened scope and the fact that a user-global
finding's remediation is the operator's, and the plan records the two review
findings this pass closed plus the P2 / standards-managed machine-path
overlap it deliberately left alone.

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

Claude Code exposes no way to see the permission rules actually in effect —
no `claude permissions` subcommand, no machine-readable export — so the
honest answer to "where is this rule coming from" has been to read five
files in five places and hope you knew all five. This reader finds them:
managed policy, user-global, project, local, and any pre-v2.1.211 copy left
in the session's start directory, each scope's allow/ask/deny rules
attributed to the surface they came from.

The status vocabulary is the substance. `absent` means looked and found
nothing; `skipped` means could not look. Every scope and every managed
surface emits a record on every OS, including the ones that do not apply
here, so a surface that was never attempted can never be read as one that is
genuinely empty. Server-managed settings arrive remotely and have no local
path at all, which the reader discloses rather than letting silence imply
completeness.

Managed policy is four surfaces per OS, not one file. The JSON file and its
drop-in directory are the portable core, read everywhere and merged in the
documented order; the Windows policy registry keys and the macOS
managed-preferences domain are optional platform integrations that announce
a skip and leave the core result intact.

Exercising it under a stub PATH caught two defects that each produced a
confident wrong answer. MSYS rewrites any argument containing backslashes as
a POSIX path, so every registry query died with "Invalid syntax" and scored
as no policy deployed. And with plugin-root resolution broken, the shared
library failed to source while the run still exited 0, reporting every
managed surface absent; that path now exits 2, in this reader and in the
audit skill's structure check, which had the same fall-through.

The registry surface is verified against the real registry via a scratch key
outside the policy tree, created and deleted within the check, so no policy
was ever deployed to the machine. The macOS domain and the Linux paths are
an honest manual-verification gap, stated in the skill.

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

A stray admin-level key could make user-level policy look like the managed
policy. The search stopped at the first key whose Settings value parsed, so
an HKLM key with a missing or malformed value fell through to HKCU — which
the settings doc calls lowest policy priority, used only when no admin-level
source exists. Reporting it while an admin-level key is in force is the
precise failure that rule exists to prevent.

`reg query <key> /v Settings` cannot distinguish a missing key from a
present key with no such value: both return exit 1 and the same message.
A bare `reg query <key>` does distinguish them, so key existence is now the
probe that ends the search, and an existing key that yields nothing readable
is reported unreadable with a note rather than as permission to consult the
next key.

The regression test uses HKCU\SOFTWARE, which exists on every Windows
install and carries no Settings value, so it covers the case read-only and
writes nothing to the registry.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TgD5TknENb3ifk1a47eqUr
…ule's mechanic named

audit-permission-state could say what every scope holds but not what any of it
means together. This adds the merge: one effective allow/ask/deny set, each rule
naming every scope that contributes it and the documented mechanic that put it
there.

The two governing doc sections were re-fetched before any code was written, and
they overturned the planned model. Permission rules "merge across scopes rather
than override", so a rule written at two scopes has no winner and electing one
would have been a precedence claim no page supports. What a rule can lose is its
kind: deny is evaluated before ask and ask before allow, from any scope, in both
directions -- a user-level deny blocks a project-level allow just as a
project-level deny blocks a user-level allow. That is the only election made
here, and a scope-ranked implementation would get the low-scope-deny case
exactly backwards. The beaten entry ships as an `inert` record naming what beat
it, which is the answer to "why is my allow rule ignored".

Every run states the two bounds on the claim. The command-line scope
(--settings, --allowedTools, --disallowedTools) outranks the files and has no
file to read. Rules are compared by exact text, so a narrow allow blocked only
by a broader deny pattern is still reported effective -- the docs state the
broad deny wins, so the error direction is known: it over-reports allow and
never over-reports blocking.

An empty merge cannot mean an empty machine. A pipeline that swallowed the
reader's exit 2 would report "nothing in effect" on a machine full of rules, so
zero scope records on input is exit 2 before any merge line is emitted.

The skill also stops overclaiming against `/permissions`, which the permissions
page documents as listing every rule and the file it came from. The Purpose now
points readers there for "where is this rule written" and claims only the
difference: outcome resolution, telling a scope that was empty from one that
could not be read, and an output another tool can consume.

reference/criteria.md finally has content that is not a restatement -- the
precedence_basis vocabulary, each token mapped to the sentence it follows from.

40 new checks, the reader's 42 still pass, shellcheck clean.

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

Exact-text comparison treated `deny: ["Bash"]` and `allow: ["Bash(git status)"]`
as two unrelated rules and reported both as effective. The permissions page says
the bare name "removes the tool from Claude's context entirely, so Claude never
sees it" -- so the report claimed prompt-free access to a tool the model cannot
see, in managed policy, which is exactly where whole-tool denies get written.

Not a case for the exact-text caveat. That caveat covers pattern subsumption,
which needs a matcher; the tool token is the text before the first `(` and
"contains no `(`" is the whole test, so this was decidable and therefore a
defect. A whole-tool deny now makes every rule naming that tool inert -- other
denies included, moot rather than weakened -- and a whole-tool ask outranks
every scoped allow for its tool, since it matches every call and ask is
evaluated first. `EndConversation` is exempt from removal per its documented
carve-out. Both cases print a NOTE naming the tool rather than leaving a reader
to infer it from a run of inert records.

Found by fresh-context review after the merge had already passed 40 checks and
its own author had signed off.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TgD5TknENb3ifk1a47eqUr
Version conflicts only, plus one comment hunk. main took claude-config to
0.31.0 and claude-memory to 0.8.0 while this branch held 0.30.0 and 0.7.1, so
this branch's entries move up to 0.32.0 and 0.8.1 above them -- still one entry
per plugin on this branch, not two.

check-structure.sh resolves to this branch: main only re-dated the comment above
the per-OS managed paths, and this branch had already replaced that block with a
source of lib/managed-scope.sh, which carries the 2026-08-10 verification date
itself.

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

cursor Bot commented Aug 12, 2026

Copy link
Copy Markdown

Bugbot is not enabled for your account, so this pull request was not reviewed.

Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs.

@claude

claude Bot commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

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


Security review complete

  • Gather context / diff (git diff origin/main...HEAD)
  • Review new lib scripts (managed-scope.sh, permission-patterns.sh)
  • Review audit-permission-state scripts (permission-state.sh, permission-merge.sh)
  • Review changes to audit-permission-grants and audit scripts
  • Review claude-memory scope-report.sh changes
  • Check for injection / unsafe eval / path traversal / secret handling
  • Post final findings

No security issues found. This PR is entirely shell scripts plus documentation (no GitHub Actions workflow changes, no CI permission changes), and its production code is a read-only reporting surface:

  • All JSON parsing goes through jq, never eval/source on rule text or settings content. Rule strings extracted from settings.json/frontmatter are only ever piped into grep -E/awk/printf '%s\n' as data — never interpolated into a command line or passed to a shell for re-execution — so a maliciously crafted .claude/settings.json (e.g. an "Bash($(rm -rf ~))" style allow-rule string) cannot achieve code execution through these readers.
  • Managed-scope reads (plugins/claude-config/lib/managed-scope.sh, used by permission-state.sh, check-structure.sh, scope-report.sh) resolve fixed, per-OS system paths and Windows registry keys hardcoded in the library; the only variable input is an explicit test-seam override (PERMISSION_STATE_MANAGED_PATH, etc.), not anything attacker-reachable in normal use.
  • reg_cmd() and defaults read in permission-state.sh invoke fixed argv arrays ("$@"), not a string passed through a shell, so there's no command-injection surface even though the registry key list is theoretically overridable via env var.
  • Secret-safety is a deliberate, well-executed design goal: check-structure.sh explicitly avoids holding file contents in shell variables (: <"$path" open-probe, piping instead of content=$(...)) specifically to keep credentials out of set -x trace output, and reports only counts/structure for settings.json/managed scope, never raw values.
  • New/expanded tests (permission-rule-check.test.sh, check-structure.test.sh) correctly isolate HOME/CLAUDE_CONFIG_DIR per run so the widened user-global scope scan can't read or leak the operator's real ~/.claude during CI.
  • No writes anywhere in the new/changed scripts — every script in this PR is documented and implemented as read-only, including managed policy which is explicitly never written.
  • Nothing in this PR touches .github/workflows/**, permissions: blocks, or dependency/action pins, so the supply-chain/workflow-trigger concerns this lane is meant to catch don't apply here — that's consistent with zizmor's advisory lane also having nothing relevant to flag on a docs+shell-script-only PR.

@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: d8f75f2565

ℹ️ 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 plugins/claude-config/skills/audit-permission-state/scripts/permission-state.sh Outdated
Comment thread plugins/claude-config/skills/audit-permission-state/scripts/permission-merge.sh Outdated
@claude

claude Bot commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

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


Review: feat(claude-config): report the Claude Code permission state actually in effect

  • Gather context (PR diff, CLAUDE.md, REVIEW.md)
  • Review new lib scripts (managed-scope.sh, permission-patterns.sh)
  • Review audit-permission-state skill scripts (permission-state.sh, permission-merge.sh)
  • Review widened checks (permission-rule-check.sh, check-structure.sh, scope-report.sh)
  • Review docs/skill files and tests
  • Post inline findings and summary

This repo's REVIEW.md routes security findings to the dedicated claude-security-review lane (the workflow exists in this repo), so this review is scoped to correctness, error handling, tests, and maintainability only.

Note on verification: the sandbox for this review does not permit running arbitrary shell commands (mkdir/redirection/script execution outside single simple commands were blocked), so I could not execute permission-state.test.sh / permission-merge.test.sh myself. The two findings below are based on static reading of the awk/jq logic plus cross-checking the existing test files for coverage gaps — I did not independently confirm them by running the scripts. The PR description's own test-run claims (42/42, 49/49, shellcheck clean) are author-reported and unverified by me.

Findings posted inline

  1. permission-state.sh (emit_file_rules, lines 192-202) — a .claude/settings.json whose permissions field is syntactically valid JSON but the wrong shape (e.g. a string instead of an object, or permissions.allow not an array) causes a swallowed jq runtime error and is silently reported as scope status present with zero rules — indistinguishable from a genuinely empty scope. This is exactly the ambiguity the skill's own design (and SKILL.md's "absent means looked and found nothing" / "invalid-json is not absent" sections) says must never happen; the existing invalid-json path only checks JSON syntax, not shape, and permission-state.test.sh has no case for this shape mismatch. → permalink

  2. permission-merge.sh (header/usage, lines 20 & 49-50) and SKILL.md (Phase 2 table) — all three document the inert record as always carrying outranked_by=<kind>, but the whole-tool-deny branch added in this PR (line 181) emits removed_by=deny@<tool> instead — a different field name that none of those three consumer-facing contracts mention. Only reference/criteria.md documents all three variants correctly. This looks like a doc-sync miss from commit 837d503 (it updated criteria.md and added SKILL.md prose but not the literal record-format strings). Low functional risk (behavior is correct and tested — permission-merge.test.sh exercises removed_by=deny@Bash at lines 143-144), but the record format is explicitly meant to be machine-consumed ("in a form another tool can consume"), so an external parser following only the documented contract would miss these lines. → permalink

What held up well

  • managed-scope.sh / permission-patterns.sh: clean, well-scoped define-only libraries with no side effects; test coverage (managed-scope.test.sh) explicitly checks sourcing produces no output/exit, which is the right thing to protect for a shared library.
  • permission-state.sh's scope/status vocabulary (absent vs skipped vs unreadable vs not-applicable) is carefully reasoned and consistently applied everywhere except the gap in finding 1.
  • The local/startdir-local worktree resolution logic (lines 128-157) and its accompanying tests in check-structure.test.sh (cases 9-11) correctly handle the "same directory reported twice" and "worktree vs main checkout" edge cases.
  • permission-rule-check.sh's new user-global scope widening is well-tested, including the CLAUDE_CONFIG_DIR relocation case and the unresolvable-scope announcement (cases 8c-8e).
  • The Windows MSYS2_ARG_CONV_EXCL fix and the registry key existence-vs-readable distinction (permission-state.sh lines 251-259) are documented with a dated, measured rationale rather than asserted — good practice for platform-specific behavior that's hard to test in CI.

@github-actions

Copy link
Copy Markdown

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

cursoragent and others added 2 commits August 12, 2026 01:37
Co-authored-by: Kyle Sexton <kyle-sexton@users.noreply.github.com>
…view threads

- Remove docs/topics/permission-model contract-tier slice (gate requires graduation before merge)
- Anchor Windows registry JSON extraction at the Settings type column (P1 review)
- Validate permissions object shape in classify_json_file (schema-wrong JSON)
- Skip duplicate user-global scan when it resolves to the project settings path
- Document removed_by=/outranked_by=ask@ inert record variants in permission-merge.sh
- Add portability-ok annotations for Windows registry path test fixtures

Co-authored-by: Kyle Sexton <kyle-sexton@users.noreply.github.com>
@github-actions

github-actions Bot commented Aug 12, 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-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

github-actions Bot commented Aug 12, 2026

Copy link
Copy Markdown

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

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.

cursoragent and others added 4 commits August 12, 2026 01:47
Co-authored-by: Kyle Sexton <kyle-sexton@users.noreply.github.com>
Co-authored-by: Kyle Sexton <kyle-sexton@users.noreply.github.com>
…arisons

Co-authored-by: Kyle Sexton <kyle-sexton@users.noreply.github.com>
Co-authored-by: Kyle Sexton <kyle-sexton@users.noreply.github.com>
kyle-sexton added a commit that referenced this pull request Aug 12, 2026
PR #2260 was merged as a squash while this branch kept moving, so main now
carries the walking skeleton and the merge stage while this branch carries those
plus Phases 3-8, the contract-slice prune, and thirteen defect fixes from three
independent verification rounds.

Every code conflict resolves to this branch: same lineage, strictly further
along. The squash landed the Phase 1-2 versions of permission-state.sh and
permission-merge.sh; this branch has both plus the whole-tool deny handling, the
conf-record passthrough, the newline and carriage-return reporting, and the
9x reader speedup.

Versions move above what main reached rather than re-using the numbers this
branch had claimed: claude-config 0.36.0 (main was at 0.35.2), claude-memory
0.9.1 (main was at 0.9.0). Both CHANGELOGs interleave so this branch's entry
sits above main's history in descending order.

Catalog and cheat sheet regenerated. validate-plugins and all four
changelog-parity modes pass.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TgD5TknENb3ifk1a47eqUr
kyle-sexton added a commit that referenced this pull request Aug 12, 2026
The changelog-parity gate caught this and it is a real defect, not a gate
quirk: resolving the merge relabelled the `## [0.8.1]` heading to `## [0.9.1]`
rather than adding a new one, so a released entry's heading vanished. The gate's
own message names that exact failure mode -- "RELABELLING that heading to the
new version; git leaves no conflict marker behind for either".

The right fix turned out to be smaller than restoring the heading. PR #2260
already shipped 0.8.1 with this body, and diffing the two entries shows they are
identical apart from the heading. This branch changes nothing in claude-memory:
no code, no docs, only a version number and a relabelled heading it had no
business touching. So both files revert to main exactly, and the plugin carries
no bump at all.

claude-config is unaffected -- 0.36.0 sits above main's history with every
heading main shipped still present.

All four parity modes and validate-plugins pass.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TgD5TknENb3ifk1a47eqUr
kyle-sexton added a commit that referenced this pull request Aug 12, 2026
)

Fixes #2282 (scoped rows A2, A3, A19).

> **Scope note:** this PR takes three of #2282's five rows. **A7b** and
**A12** both reproduce at HEAD and are untouched; they are now filed as
**#2397**, so the closing keyword above no longer drops them.

## Summary

Scoped fix for three of #2282's five rows in `audit-permission-grants`.

- **A2 — direction reversed after review.** As filed, this row implies
"make the `//` exemption real". The docs settle it the other way:
`permissions.md` gives `//path` = "Absolute path from filesystem root"
with `Read(//Users/alice/secrets/**)` → `/Users/alice/secrets/**`, and
"Use `//Users/alice/file` for absolute paths." So `//Users/<name>/…` is
the canonical *spelling* of a hardcoded user home, and exempting it
would have made an `error`-tier username-leak check blind to the
documentation's own example of the leak. **`criteria.md` moved; the
detector keeps flagging `//`.** The inconsistency the row reports was
real — the two shipped files disagreed — but the document was the wrong
one.
- **A3:** P2 findings report the full `Tool(…)` rule, not an
eight-character path fragment. Two regressions this introduced are also
fixed: the tool name was enumerated as
`(Read|Edit|Write|Bash|PowerShell)` (silently dropping
`WebFetch`/`Glob`/`NotebookEdit`/`mcp__*`/`Agent` rules — `Agent`
indefensible, this script ships `scan_agent()`), and the `//` skip was a
substring test that let `Read(//opt/data/../Users/kyle/secrets)` read
clean.
- **A19:** `Bash(npm view ctx7 version*)` is no longer treated as a bare
package-manager wildcard — `*` must be preceded by a separator.

Out of scope and **now tracked in #2397**: A7b (inert-grant check) and
A12 (`~user` username leak). Both reproduce at HEAD, with evidence
carried into that issue, so closing #2282 here drops nothing.

## Test plan

A19's change verified against the shipped library at this branch,
sourced verbatim — the false positive is gone and every true positive
still matches:

```
$ . plugins/claude-config/lib/permission-patterns.sh
$ for r in 'Bash(npm view ctx7 version*)' 'Bash(npm *)' 'Bash(npm:*)' 'Bash(npx *)' \
           'Bash(pnpm dlx *)' 'Bash(npm test)' 'Bash(npm run build *)'; do
    printf '%-32s -> ' "$r"
    out=$(printf '%s\n' "$r" | grep -oE "$CCPERM_P1_ERE"); [ -n "$out" ] && echo FLAGGED || echo clean
  done
Bash(npm view ctx7 version*)     -> clean      # the false positive A19 filed
Bash(npm *)                      -> FLAGGED
Bash(npm:*)                      -> FLAGGED
Bash(npx *)                      -> FLAGGED
Bash(pnpm dlx *)                 -> FLAGGED
Bash(npm test)                   -> clean
Bash(npm run build *)            -> FLAGGED
```

A2/A3's `P2_RULE_ERE` compared against the pattern it replaces, across
tool names — this is where the two regressions below were found:

```
rule                                   | old P2 | new P2 outcome
Read(//Users/alice/secrets/**)         | /Users/a | SKIPPED (//)
Bash(/c/Users/kyle/x.sh:*)             | /Users/k | FLAGGED  (full rule now — A3 works)
Read(/Users/kyle/.aws/credentials)      | /Users/k | FLAGGED
WebFetch(/Users/kyle/x)                | /Users/k | NOT MATCHED   <-- regression
Agent(/Users/kyle/x)                   | /Users/k | NOT MATCHED   <-- regression
mcp__srv__tool(/Users/kyle/x)          | /Users/k | NOT MATCHED   <-- regression
Read(//opt/data/../Users/kyle/secrets) | /Users/k | SKIPPED (//)  <-- over-broad exemption
```

`permission-rule-check.test.sh` passes on this branch (72 + 16 new
cases); none of the seven rows above is in it, which is why it stayed
green.

## Related

- **#2282** — the owning issue, closed here. Rows **A7b** and **A12**
were split to **#2397** before merge so the auto-close drops nothing.
- **#2397** — the follow-up carrying A7b + A12 with their HEAD
reproductions, the A12-vs-`%USERPROFILE%` split, and A7b's
branching-remedy constraint.
- **#2260** — extracted the P1 rule vocabulary into
`plugins/claude-config/lib/permission-patterns.sh`, which is why A19's
fix lands in `lib/` rather than at the
`scripts/permission-rule-check.sh:138,144` anchors #2282 names. That
library now has a **second consumer** (`audit-permission-state`), so
this change is no longer scoped to one detector.
- **#2248 / #2249** (closed) — the previous `permission-rule-check`
passes this builds on. #2248 rewrote P2's *rationale* without touching
`P2_ERE`; #2249 removed the `$PWD` fallback and added the exit-2
refusal.
- **#2283** — the sibling `audit-permission-grants` issue (clean bill
with no denominator, `vendor/`-only exclusion, unimplemented scope
filters). Not touched here.
- **#2284** — the `criteria.md` staleness cluster. **Directly
relevant:** the `//` exemption this PR implements comes from
`criteria.md:64-65`, and #2284 is the issue about that file's doctrine
being stale. See the review thread.
- **#1398** (open) — faults P1's bare-name-on-PATH `Recommend`, the same
remediation surface.
- **#2301, #2335** — handoff-inbox batch 4 lane CC, the other
`claude-config` waves. #2335 is open and bumps the same manifest, so
this PR and that one will collide on `plugin.json` / `CHANGELOG.md`;
whichever merges second needs a rebase.

Inbox item:
`20260811-024628-claude-config-audit-permission-grants-defects-and-fleet-grant-hygiene`.
Ledger:
`.work/handoff-inbox-batch-4/ledgers/I10-permission-grants-fleet.md` §
A2, A3, A19.

---------

Co-authored-by: Cursor Agent <cursoragent@cursor.com>
Co-authored-by: Kyle Sexton <kyle-sexton@users.noreply.github.com>
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
kyle-sexton added a commit that referenced this pull request Aug 12, 2026
…auto-mode authoring lane (#2396)

## Summary

PR #2260 landed the walking skeleton and the merge stage. This is the
rest: four more analysis
stages, the second skill, and thirteen defect fixes from three rounds of
independent verification.

## What this adds on top of #2260

- **The auto-mode entry diff** — every effective allow rule classified
as dropped (blanket,
wildcarded interpreter, package-manager run, or `Agent`) or kept, from
the same shared pattern
vocabulary `audit-permission-grants` check P1 scans with. Reads
`autoMode.classifyAllShell`, which
inverts the answer wholesale, and resolves it only from the scopes the
classifier actually reads.
- **Permission-plane lints** — nine checks for configuration written
where nothing reads it. The
highest-consequence one: `disableAutoMode` typed as a boolean instead of
the string `"disable"` is
valid JSON, accepted, and does nothing — so the operator believes auto
mode is locked out and it is
  not.
- **The `autoMode` classifier block** — `$defaults` omissions (which
**replace** the built-in list,
not extend it), contradictions, and entries an earlier `hard_deny`
forecloses. `claude auto-mode
  critique` is surfaced and wrapped, never reimplemented.
- **Managed conformance** — which managed intents are enforced versus
loosenable. A managed
`autoMode` section is **additive, not a policy boundary**; and a
lower-scope deny beats a managed
allow without overriding it, which the precedence table alone does not
suggest.
- **`claude-config:draft-auto-mode-rules`** — the authoring counterpart.
Interviews, prints a
paste-ready block, and **writes nothing, in any scope, under any flag**.
- **ADR 0009** and the contract-slice prune; the `allowed-tools` trust
measurement carried into
  `MIGRATION-PLAYBOOK.md`'s plugin-acceptance security review.

## Independent verification

Three rounds, fresh context, rationale withheld. Thirteen defects, all
reproduced before being fixed.
The ones worth naming:

- **A bare `Bash` allow was reported as surviving auto mode.** It is the
whole-tool form — broader
than `Bash(*)`, which the same run drops. `Agent`'s bare form was
handled; the shell tools' was not.
- **`--merge-only` silently inverted the entry diff.** Two documented
flags composing into a wrong
answer: `conf` records were dropped, so a suspended rule was reported
kept.
- **A carriage return inside a rule was silently deleted**, turning
`Bash(a\rb *)` into `Bash(ab *)` —
a rule in no settings file, flowing downstream as a real grant. The
newline fix from the previous
  round had covered LF only.
- **`C6-winPath` was dead in the real pipeline** — it tested the JSON
source spelling that `jq -r`
decodes away, and passed its own suite only because those fixtures
bypassed the reader.
- **A corrupt managed policy was reported as absence of policy** — the
worst answer that report can
  give an administrator.
- **Parameter matching is deny/ask only.** Found by re-reading the page
to settle a verifier
challenge: "allow rules continue to use each tool's own specifier
syntax", so
`allow: ["Agent(model:opus)"]` is not a narrowed grant. New check
`C6-allowParam`.

Two findings were about my own claims rather than the code: a comment
asserting as *measured*
something never measured, and a comment describing a design that had
been replaced.

## Verification

Suites: reader 50, merge 51, entry diff 63, plane lint 50, block lint
62, conformance 32, drafter 39,
grants 76, managed-scope 16, permission-patterns 12, **no-writes 12**.

Criterion 9 (writes nothing, any scope, any flag) is proven by **running
all ten actions of both
skills with the oracle explicitly ON** and diffing a fixture tree and
fixture HOME either side — not
by reading the code.

`shellcheck -x`, portability, drift, contract-slice prune, changelog
parity and `validate-plugins`
all clean. Listing budget 6737/8000.

## Note on the merge resolution

Resolving this branch against post-#2260 `main` produced eighteen
conflicts. Taking `--ours` was right
for the files this branch advanced and wrong for three it did not — most
importantly
`permission-rule-check.sh`, where main had **removed** a `$PWD`
scan-root fallback that could sweep
the operator's home directory. The drift gate caught one; checking the
rest of the `--ours` set found
the other two. All three restored from main; see `f8abbafa`.

## Related

No linked issue — this continues the research and planning thread that
produced #2260.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

https://claude.ai/code/session_01TgD5TknENb3ifk1a47eqUr

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-authored-by: Cursor Agent <cursoragent@cursor.com>
Co-authored-by: Kyle Sexton <kyle-sexton@users.noreply.github.com>
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.

2 participants