The feature README's Security Notes describe a layout it stopped using - #381
Merged
Conversation
Two sections of one file disagreed. "Why only directories are mounted" says CLAUDE.md and settings.json are writable and that settings.json naming a hook inline makes it a real hole. "What's Protected" 450 lines below still said both were read-only, that writable files were limited to authentication and state, and that all code-execution files stayed read-only. None of that survived #362's move to a directory mount. The mount-agreement test binds one heading by its exact text, so the second protection list was checked by nothing. The new guard asks every heading that claims protection, which is what the missed one was. Also qualifies the five-directory guarantee: it holds against an unprivileged container, and docker-in-docker makes this repo's own container privileged. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
Reviewer's GuideThis PR reconciles the feature README’s security notes with the current ~/.claude read-write directory mount and privileged devcontainer model, then adds a heading-independent regression test to prevent writable files from being documented as protected again. Flow diagram for heading-independent security documentation guardflowchart TD
Manifest[Feature mount manifest]
Readme[README security headings and bullets]
Test[test_no_heading_claims_protection_for_a_writable_file]
Writable[Writable paths from manifest]
Protected[Headings containing read-only or protected]
Failure[Fail with offending heading and path]
Pass[Pass when no writable file is claimed protected]
Manifest --> Writable
Readme --> Protected
Writable --> Test
Protected --> Test
Test -->|overlap found| Failure
Test -->|no overlap| Pass
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
This was referenced Aug 24, 2026
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Found by a retrospective
wf-reviewof #335/#362/#374 — none of the three was reviewed (Sourcery out of quota).Two sections of one file, disagreeing
#362 moved
~/.claudefrom individual file mounts to a read-write directory mount, and wrote an honest account of the cost atREADME.md:98-115:450 lines below,
### What's Protected (Read-Only Mounts)still said:Every one of those is false under the current manifest, which mounts
~/.clauderead-write and marks exactly five directories readonly:agents/,commands/,hooks/,skills/,wf-skills/. A reader who scrolls to a heading called "Security Notes" — which is what a reader assessing risk does — gets the pre-#362 answer.What is actually reachable from the container, and now said so: the host's OAuth tokens in
.credentials.json, the host's session transcripts for every project underprojects/, and a hook command written into the host'ssettings.json, which is host command execution.The five read-only mounts are not an escape boundary
README.md:113and:380stated the guarantee unqualified — "only the five named directories are proof against a prompt injection", "you cannot add or change an agent, command, hook or skill from within the container".True of an unprivileged container. A container with
CAP_SYS_ADMINcanmount -o remount,rwits own read-only bind mounts — and this repo's devcontainer is privileged: thedocker-in-dockerfeature brings"privileged": true, as.devcontainer/devcontainer.json:76notes in passing.Both sentences are now qualified, and there is a short section saying what the five directories are and are not: protection against a prompt injection editing its own instructions, not against code actively trying to get out.
Why it drifted, and the guard
test_claude_code_feature_mounts.pybinds the README to the manifest throughREAD_ONLY_HEADING = "### Read-Only Mounts (Security-Protected)"— one heading, by exact text. The stale claims lived under a different heading, so they were checked by nothing. That is the actual defect: the guard was addressed to a heading rather than to a property.test_no_heading_claims_protection_for_a_writable_fileasks every heading whose text contains "read-only" or "protected" whether it bullets a file that is writable, and asserts the set of such headings is non-empty first, so it cannot pass by matching nothing. Only bullets count, fordocumented_paths's existing reason: the prose under these headings discussessettings.jsonprecisely to say it is not protected, and a substring match would fail on the sentence that fixes the problem.Proven against the real defect. Restoring the two original bullets turns it red with the offending lines named:
Verified
pixi run -q python -m pytest test/ --ignore=test/e2e: 284 passed (was 283 — the new test), 20 failed, byte-identical failures to pristinemainin a scratch clone (they need a builtdl).pixi run style10.00/10.Left for follow-ups
From the same review, not in this PR:
install.sh:172-224's banner repeats the stale claims and still seeds{}into.credentials.json(the behaviour #362 removed frominit-host.shand pins against);init-host.sh:143creates the OAuth-token directory with the ambient umask, measured775, two lines above amkdir -m 700for~/.ssh;TROUBLESHOOTING.md:166-185answers a symptom that can no longer occur; andtest_init_host_heals_stale_mounts.py:208derives its parametrization from the current README when the heal exists for legacy mounts, which has silently droppedCLAUDE.mdandsettings.jsonfrom coverage. Full report on #374.Also worth knowing: all six of #335's heal tests skip where no private mount namespace is available. If that guard fires on CI runners too, that fix has never been executed by a test.
🤖 Generated with Claude Code
Summary by Sourcery
Align the Claude Code feature's security documentation and regression coverage with its current mount behavior.
Bug Fixes:
Enhancements:
Documentation:
Tests: