docs(claude-config): state the scope of a Read deny in the audit permission baseline - #1599
Conversation
…ission baseline `required-permissions.md` recommended `Read(./.env)` / `Read(./secrets/**)` / `Read(./.claude/settings.local.json)` as secret protection without saying what a `Read` deny reaches, so a reader came away believing the file was protected. Add a "Scope of a Read deny" subsection, verified against current official docs: the rule covers Read/Grep/Glob/LSP, `@file` mentions, IDE selection context, Edit on the same path, and the file commands Claude Code recognizes inside a Bash command (`cat`, `head`, `tail`, `sed`) — but not an arbitrary subprocess that opens the path itself, which is how an interpreter one-liner reads a denied file with no deny firing. Remedies are ranked rather than listed: the sandbox is the documented OS-level enforcement path and does not run on native Windows; a PreToolUse hook on Bash|PowerShell is a speed bump, not a boundary, because it inspects the same evadable command string; where no OS-level boundary exists, the durable control is keeping the secret out of the session's reach. Enumerating shell readers as `Bash(cat *)` deny globs is named a non-remedy. PowerShell-tool read coverage and the full recognized-command set are flagged unverified rather than asserted. Category B now reports the baseline with that scope in both directions, and `context/procedures.md` stops implying its own `settings.local.json` recipes escape the recommended deny: the safety is in what gets emitted, not what gets opened, and routing around a deny with an interpreter one-liner is prohibited. The pattern table is unchanged — it is the Category B presence check. Refs #1598 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Claude encountered an error after 0s —— View job I'll analyze this and get back to you. |
|
Claude encountered an error after 0s —— View job I'll analyze this and get back to you. |
|
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."
Re-running the job, or pushing a new commit, will retry the review. |
|
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. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6dc6393922
ℹ️ 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".
…ection P1 — directory location is not a boundary. The residual-risk paragraph offered "outside the working directory and additionalDirectories" as a place to put a secret, which is the same false confidence this change exists to remove: a subprocess opens absolute paths, so relocation changes nothing about who can read the file. The boundary is the OS principal. Reworded to say so and to give controls that actually hold — credential store or secrets manager with use-time injection, a short-lived credential, or a different principal / container. P2 — reconcile the hook ordering. The new text said a hook exiting 2 blocks before permission rules are evaluated, while "Interaction with hook-based gates" said a deny fires before any PreToolUse hook. Both are true in one direction each: a hook cannot loosen a deny or ask, and a hook exiting 2 can tighten past an allow rule. Both passages now state which direction they mean. Refs #1598 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Claude encountered an error after 1s —— View job I'll analyze this and get back to you. |
|
Claude encountered an error after 1s —— View job I'll analyze this and get back to you. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 51a4c079a0
ℹ️ 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".
… decisions The reconciled ordering paragraph claimed ask rules are evaluated regardless of the hook result and then said exit 2 stops the call before permission rules are evaluated, which gives two answers for a hook that exits 2 while an ask rule also matches. Split the two cases: a returned decision cannot loosen a deny or ask, and exit 2 short-circuits instead of feeding a decision in — it stops the call before permission evaluation, so nothing downstream runs, an otherwise-matching ask rule included. Refs #1598 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Claude encountered an error after 0s —— View job I'll analyze this and get back to you. |
|
Claude encountered an error after 0s —— View job I'll analyze this and get back to you. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 76c4931ba3
ℹ️ 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".
…locked read P1 — the sandbox was ranked as the enforcement path without its escape surfaces, which recreates the assurance this change removes. All four are open at their defaults: allowUnsandboxedCommands lets a failing command be retried outside the sandbox, failIfUnavailable warns and runs unsandboxed, excludedCommands runs listed commands outside and can always be appended to, and filesystem.disabled lifts the denyRead and credentials.files read protections. Tabled, with an enabled-but-default sandbox reported as partial rather than as protection. P2 — procedures.md asserted check-structure.sh still supplies counts, but with the sandbox enabled the baseline Read deny merges into the filesystem boundary and the OS blocks the script and its tr/jq children too. The script previously surfaced that as `Valid JSON: no` and failed the run — a false malformed-config finding. It now separates the two: `Present: yes` / `Readable: no` with a `not inspectable` note, exiting cleanly. SKILL.md Phase 1 and procedures.md both say that is a correct result to record, not a reason to find another reader. New test case, announcing a skip where the platform does not enforce chmod 000. Refs #1598 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Claude encountered an error after 0s —— View job I'll analyze this and get back to you. |
|
Claude encountered an error after 1s —— View job I'll analyze this and get back to you. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 812b963ecb
ℹ️ 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".
…riables The readability check introduced in 812b963 assigned the whole file to a shell variable, so `bash -x` or an inherited SHELLOPTS=xtrace would print every credential in settings.local.json — a regression in a script whose contract is never to emit secret values. Replace it with `: <"$path"`, an open() probe that reads no bytes, and restore the original `tr | jq` pipelines. File contents stay inside pipelines, where tracing prints the command and not the data. Refs #1598 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Claude encountered an error after 1s —— View job I'll analyze this and get back to you. |
|
Claude encountered an error after 0s —— View job I'll analyze this and get back to you. |
Fixes #1607 The `check-structure.sh` unreadable-file note no longer lists a bare `Read(...)` deny as a cause. A Read deny alone cannot make `open()` fail inside this script (the arbitrary-subprocess carve-out from #1599); sandbox `denyRead` — including a Read deny merged into the sandbox boundary — or filesystem permissions can. ## Related - #1599 Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: Kyle Sexton <kyle-sexton@users.noreply.github.com>
Closes #1598.
What was wrong
plugins/claude-config/skills/audit/reference/required-permissions.mdrecommended asensitive-file-denybaseline (Read(./.env),Read(./secrets/**),Read(./.claude/settings.local.json), key/PEM/SSH patterns) as the remedy for protecting secrets,without ever saying what a
Readdeny reaches. A reader who followed it believed the file wasprotected. This is a security-documentation defect: the doc created confidence the mechanism does
not deliver.
What I verified — and what I refuted
Fetched this session:
permissions,
sandboxing,
tools reference.
Refuted, in part. The originating report framed this as
Read(...)andBash(...)beingseparate matcher namespaces, so that a
Readdeny leavescat/grep/head/sedopen. Upstreamsays otherwise: Read and Edit deny rules "apply to Claude's built-in file tools and to file
commands Claude Code recognizes in Bash, such as
cat,head,tail, andsed." The obviouscatfallback is blocked.Confirmed. They "don't apply to arbitrary subprocesses that read or write files indirectly, like
a Python or Node script that opens files itself." That is the real gap and it matches the reported
observation (an interpreter one-liner) exactly.
Also refuted: the report's own suggested fix. It proposed a
PreToolUsehook onBash|PowerShellinspecting the command string as "the durable remedy". Upstream documentsargument-constraining Bash patterns as fragile, and a hook reading the same command string inherits
the same evasion surface. Shipping that as the answer would have relocated the false confidence
rather than removing it, so the hook is ranked below the sandbox and explicitly called a speed bump,
not a boundary.
What landed
required-permissions.md: covered vs not covered;Bash(cat *)-style enumeration named a non-remedy; the sandbox(
sandbox.filesystem.denyRead,sandbox.credentials.files"mode": "deny") as the documentedOS-level path with its platform limit — it does not run on native Windows, so it is unavailable
in exactly the environment that produced the observation; a
PreToolUsehook as best-effort; andthe residual risk stated plainly — a deny glob cannot keep a secret from a session that has shell
execution, so the durable control is keeping the secret out of the session's reach.
(
Get-Content,type) are covered at all — upstream scopes the coverage to commands "in Bash" andthe tools reference lists
Read(...)as applying to "Read, Grep, Glob, LSP", neither addressingPowerShell; and the full membership of the recognized-command set, which upstream gives with "such
as", leaving
grep,jq,strings, and redirects unconfirmed in both directions.SKILL.md: Category B reports the baseline with that scope in both directions — a presentbaseline is not reported as proof the file is unreachable.
context/procedures.md: the skill's ownsettings.local.jsonrecipes no longer imply theyescape the recommended deny. The safety is in what gets emitted, not what gets opened —
check-structure.shreads from a subprocess and is safe because it emits counts only, while thesupplemental
cat … | jqrecipes are blocked in a compliant project, correctly so. Routing aroundthat with an interpreter one-liner is prohibited; the audit reports the file as not inspectable
under the project's own rule.
read-deny-scope-not-overstated) covering the new reporting behavior.0.13.0→0.14.0with a matching CHANGELOG entry.The pattern table is unchanged. It is the Category B presence check; changing rows would change
what the audit flags.
Deliberately not done
claude-config.guardrailsownsPreToolUseonBash|PowerShell; itssecret-pattern-detection.shmatchesWrite|Edit|NotebookEdit(it blocks writing secrets), andits shell-matched hooks are git/commit-scoped, so nothing there covers credential reads today.
Whether
guardrailsshould grow a credential-read matcher is a product call for that plugin, andper the analysis above a command-string hook would be a speed bump regardless — it does not belong
in
claude-config.required-permissions.mdstates this recommendation; no other pluginand no
docs/**page does.Related
docs.claude.com→code.claude.comURL migration whose destinationdomain every citation added here uses.
guardrailsshould grow a credential-readPreToolUsematcher.That is a product call for
guardrails, argued against as a boundary in this PR's analysis; noissue filed.
audit-checklist.mdB.4 flags:*permission-rule syntax as "deprecated",but the current permissions page documents
Bash(ls:*)as an equivalent trailing-wildcard form.Noticed while verifying this item, out of its scope, left for a separate change.
Verification
check-changed-skills.shPASS (0 errors),check-changelog-parity.sh --check-bumppass,generate-catalog.mjs --checkin sync,validate-plugin-contracts.mjspass,validate-plugins.shpass, markdownlint 0 issues, evals JSON parses.
🤖 Generated with Claude Code