From 9580267fe180b277ba1dc1dff07b9a79ad7a2434 Mon Sep 17 00:00:00 2001 From: Austin Gregg-Smith Date: Wed, 9 Sep 2026 10:11:06 +0100 Subject: [PATCH 01/13] feat: share Claude and Codex skills in the container feature --- .devcontainer/claude-code/README.md | 71 +++++++--- .devcontainer/claude-code/TROUBLESHOOTING.md | 6 +- .../claude-code/devcontainer-feature.json | 8 +- .devcontainer/claude-code/init-host.sh | 14 +- CHANGELOG.md | 8 ++ docs/workspace-tools.md | 16 +++ test/e2e/test_claude_config_protection.py | 55 +++++++- test/unit/test_claude_code_feature_mounts.py | 48 ++++--- .../unit/test_init_host_heals_stale_mounts.py | 2 +- test/unit/test_shared_agent_skills.py | 123 ++++++++++++++++++ 10 files changed, 294 insertions(+), 57 deletions(-) create mode 100644 test/unit/test_shared_agent_skills.py diff --git a/.devcontainer/claude-code/README.md b/.devcontainer/claude-code/README.md index 48e1d2ac..bfebb37c 100644 --- a/.devcontainer/claude-code/README.md +++ b/.devcontainer/claude-code/README.md @@ -25,6 +25,8 @@ The following files and directories from your **host machine** are mounted into - `~/.claude/hooks/` → Event-driven shell hooks - `~/.claude/skills/` → Agent Skills, and the links a skill installer leaves there - `~/.claude/wf-skills/` → the skill bodies those links point at +- `~/.claude/shared-skills/` → Skill bodies shared by Claude and Codex +- `~/.agents/skills/` → Codex skill discovery and shared skill bodies These are **read-only** (`readonly` flag) to prevent: - Prompt injection attacks that could modify your Claude configuration @@ -36,15 +38,45 @@ coincidence — see "Why only directories are mounted" below. `CLAUDE.md` and `settings.json` are the two instruction files this list does not cover, because a read-only mount over either of them could not be kept. -Skills are two mounts because they are one thing. An installer that keeps the -prompt bodies in a sibling directory and leaves *relative* links behind — -`skills/wf -> ../wf-skills/wf`, which is what `wf skills install` from -blooop/wayfinder writes — needs both sides in here or every link arrives -dangling, and a container with dangling links has no skills at all rather than -stale ones. Read-only for the same reason `commands/` is: a skill is executable -instructions. That costs one thing, and it is a warning rather than a failure — -`wf` heals its own links on every launch, so a `wf` run *inside* a container -reports that it could not refresh them and carries on. +### Shared skills for Claude and Codex + +The feature mounts both discovery directories at the same paths relative to the +container home. The host username can differ from the container's `vscode` user. +Store a skill once and link it into the other agent's discovery directory: + +```text +~/.agents/skills/research/SKILL.md +~/.claude/skills/research -> ../../.agents/skills/research +``` + +A shared payload inside the Claude configuration also works: + +```text +~/.claude/shared-skills/sync/SKILL.md +~/.claude/skills/sync -> ../shared-skills/sync +~/.agents/skills/sync -> ../../.claude/shared-skills/sync +``` + +Both the links and their target directories are mounted read-only. Protecting +only the discovery directory would still allow writes through a link into the +writable `~/.claude` parent. Existing `skills/wf -> ../wf-skills/wf` links remain +supported by the `wf-skills` mount. + +The feature preserves your selection of skills. It creates missing mount roots +but does not add, rewrite, or remove individual links. Keep those in your dotfiles +or skill installer. Relative links to the mounted roots work across usernames; +absolute host paths and targets outside those roots need their own container +configuration. A pre-existing skill or symlink is never replaced by the host hook. + +Codex installation, authentication and `~/.codex/config.toml` remain the +workspace's responsibility. This feature shares skills and does not mount the +host's Codex credentials or session state. Recreate existing containers to pick +up the new mounts. A plain `dl` launch of a repo that does not use this feature +still gets its skills from that repo's configuration or dotfiles setup. + +Dotfiles installers must skip the mounted `~/.agents/skills` root, just as they +skip host-mounted Claude configuration. Detect ownership from the mount table; +a container profile alone cannot tell a local directory from a host bind. ### Read-Write Mounts (Authentication & State) - `~/.claude/.credentials.json` → OAuth access/refresh tokens @@ -110,7 +142,7 @@ container sees the host's copy of each, live, and can write to all of them. This is the part of the layout to weigh before using it. The read-only list is an allow-list of *protection*, not of visibility: a directory Claude starts writing to next month is visible and writable from the container the day it -appears, and only the five named directories are proof against a prompt +appears, and only the named directories are proof against a prompt injection that tries to edit its own instructions — and that only in an unprivileged container; see "The read-only mounts are not a container-escape boundary". @@ -248,7 +280,9 @@ container is created: ├── commands/ # Custom commands ├── hooks/ # Event hooks ├── skills/ # Agent Skills +├── shared-skills/ # Bodies shared with Codex └── wf-skills/ # Skill bodies +~/.agents/skills/ # Codex discovery and shared skill bodies ``` **None of them is optional.** A missing one aborts the container create rather @@ -256,7 +290,7 @@ than producing a warning — see "The host-side prerequisite" above, which is ho this is normally handled. By hand, it is: ```bash -mkdir -p ~/.claude/{agents,commands,hooks,skills,wf-skills} +mkdir -p ~/.claude/{agents,commands,hooks,skills,wf-skills,shared-skills} ~/.agents/skills ``` Only directories appear here, and that is the point of the layout rather than an @@ -379,7 +413,7 @@ devpod up . --recreate ## Modifying Configuration -The five instruction directories — `agents/`, `commands/`, `hooks/`, `skills/`, `wf-skills/` — are read-only, so an ordinary container process cannot add or change an agent, command, hook or skill. (A *privileged* container can remount them; see "The read-only mounts are not a container-escape boundary".) Everything else under `~/.claude` is writable and reaches the host, `settings.json` and `CLAUDE.md` included; see "Why only directories are mounted" for why those two could not be protected. +The instruction directories — `agents/`, `commands/`, `hooks/`, `skills/`, `wf-skills/`, `shared-skills/`, plus `~/.agents/skills/`, are read-only, so an ordinary container process cannot add or change an agent, command, hook or skill. (A *privileged* container can remount them; see "The read-only mounts are not a container-escape boundary".) Everything else under `~/.claude` is writable and reaches the host, `settings.json` and `CLAUDE.md` included; see "Why only directories are mounted" for why those two could not be protected. To change configuration: @@ -563,9 +597,10 @@ This implementation makes conscious security trade-offs to enable OAuth authenti ### What's Protected (Read-Only Mounts) -Five directories, and only these five: **`agents/`**, **`commands/`**, **`hooks/`**, -**`skills/`**, **`wf-skills/`**. They carry the code and instructions Claude -executes, which is why they are the ones singled out. +The directories listed under [Read-Only Mounts](#read-only-mounts-security-protected) +carry the code and instructions the agents execute. The list includes discovery +links and the shared directories they point to. The manifest-to-documentation +check in `test/unit/test_claude_code_feature_mounts.py` keeps that list current. `CLAUDE.md` and `settings.json` are **not** among them. They are files, and a file cannot be individually protected here — see "Why only directories are @@ -589,7 +624,7 @@ bind mount. That includes: - Only use this feature in **trusted repositories**, and treat the container as having the same access to your Claude account that you do -- The five read-only directories hold against an ordinary container process +- The read-only directories hold against an ordinary container process That is the honest list. In particular it is *not* true that "writable files are limited to authentication/state only", and it is *not* true that "all @@ -604,7 +639,7 @@ and **the devcontainer in this repository is privileged**, because the `docker-in-docker` feature it enables brings `"privileged": true` with it (`.devcontainer/devcontainer.json:76`). -So for this repo's own container, read the five directories as protection against +So for this repo's own container, read the protected directories as protection against a prompt injection that tries to edit its own instructions — a mistake, in other words — and not as protection against code that is actively trying to get out. @@ -615,7 +650,7 @@ words — and not as protection against code that is actively trying to get out. under `projects/` - A process in the container can write a hook command into the host's `settings.json`, which is host command execution -- A *privileged* container can additionally remount the five read-only +- A *privileged* container can additionally remount the read-only directories read-write and rewrite the host's agents, commands, hooks and skills - **Recommendation**: Only use in repositories you trust, as you would with any diff --git a/.devcontainer/claude-code/TROUBLESHOOTING.md b/.devcontainer/claude-code/TROUBLESHOOTING.md index 5173c28d..687bc821 100644 --- a/.devcontainer/claude-code/TROUBLESHOOTING.md +++ b/.devcontainer/claude-code/TROUBLESHOOTING.md @@ -14,11 +14,13 @@ ├── commands/ # Custom commands (read-only mount) ├── hooks/ # Event hooks (read-only mount) ├── skills/ # Agent Skills (read-only mount) +├── shared-skills/ # Shared skill bodies (read-only mount) └── wf-skills/ # Skill bodies (read-only mount) +~/.agents/skills/ # Codex skills (read-only mount) ``` Only the directories are mounted. `~/.claude` itself is one read-write bind, and -the five instruction directories are read-only binds on top of it; the files are +the instruction directories are read-only binds on top of it; the files are reached *through* the directory rather than bound one at a time. That is what keeps them live — a bind mount of a file does not survive the host replacing it by rename, which is what Claude does on every token refresh — and it is why @@ -171,7 +173,7 @@ reached through it are missing or unwritable. **Expected Behavior:** This is intentional! The instruction directories are mounted read-only: -- `agents/`, `commands/`, `hooks/`, `skills/`, `wf-skills/` → Read-only +- `agents/`, `commands/`, `hooks/`, `skills/`, `wf-skills/`, `shared-skills/` and `~/.agents/skills/` → Read-only `CLAUDE.md` and `settings.json` are **not** protected, and a write to either succeeds and reaches the host. See "Why only directories are mounted" in the diff --git a/.devcontainer/claude-code/devcontainer-feature.json b/.devcontainer/claude-code/devcontainer-feature.json index e7f5b62b..085c98c1 100644 --- a/.devcontainer/claude-code/devcontainer-feature.json +++ b/.devcontainer/claude-code/devcontainer-feature.json @@ -1,8 +1,8 @@ { "name": "Claude Code CLI", "id": "claude-code", - "version": "0.5.0", - "description": "Installs Claude Code CLI via pixi and sets up persistent configuration", + "version": "0.6.0", + "description": "Installs Claude Code and shares host agent skills with Claude and Codex", "options": {}, "documentationURL": "https://github.com/anthropics/devcontainer-features", "licenseURL": "https://github.com/anthropics/devcontainer-features/blob/main/LICENSE", @@ -22,6 +22,8 @@ "source=${localEnv:HOME}/.claude/commands,target=/home/vscode/.claude/commands,type=bind,readonly", "source=${localEnv:HOME}/.claude/hooks,target=/home/vscode/.claude/hooks,type=bind,readonly", "source=${localEnv:HOME}/.claude/skills,target=/home/vscode/.claude/skills,type=bind,readonly", - "source=${localEnv:HOME}/.claude/wf-skills,target=/home/vscode/.claude/wf-skills,type=bind,readonly" + "source=${localEnv:HOME}/.claude/wf-skills,target=/home/vscode/.claude/wf-skills,type=bind,readonly", + "source=${localEnv:HOME}/.claude/shared-skills,target=/home/vscode/.claude/shared-skills,type=bind,readonly", + "source=${localEnv:HOME}/.agents/skills,target=/home/vscode/.agents/skills,type=bind,readonly" ] } diff --git a/.devcontainer/claude-code/init-host.sh b/.devcontainer/claude-code/init-host.sh index 4968742a..37ecc690 100755 --- a/.devcontainer/claude-code/init-host.sh +++ b/.devcontainer/claude-code/init-host.sh @@ -6,9 +6,10 @@ # The claude-code feature mounts the developer's Claude configuration as the # directory itself, plus a read-only mount over each subdirectory holding -# *executable instructions* -- agents/, commands/, hooks/, skills/ and -# wf-skills/. Every one of those sources has to exist before the container is -# created, and the cost of a missing one is not a warning: the create is refused +# *executable instructions*, plus Codex's ~/.agents/skills. The mount list and +# these directory prerequisites are checked together by +# test/unit/test_claude_code_feature_mounts.py. Every source has to exist before +# the container is created. A missing one means the create is refused # outright with `bind mount source path does not exist`, measured on devpod # 0.26.1 -- and nothing is written to the host when it happens, which is why # creating them here is the whole fix. @@ -27,7 +28,7 @@ # so a token refresh fails # # A mount of a *directory* survives the same rename with its flags intact, which -# is why the read-only list is exactly the five instruction directories and why +# is why the read-only list is the instruction directories and why # CLAUDE.md and settings.json are no longer mounted at all: under a writable # parent their read-only mounts were enforceable only until the developer next # edited them, which is worse than not claiming the protection. @@ -139,7 +140,7 @@ done # # Every line is guarded on absence, and that is load-bearing rather than tidy. # Run from *inside* a container this repo built -- which is the point of giving -# it a Docker daemon -- the five instruction directories are the read-only +# it a Docker daemon -- the instruction directories are the read-only # mounts, and a write to one fails with EROFS. A non-zero initializeCommand # aborts `devpod up` outright. `mkdir -p` on a directory that already exists # writes nothing and is safe there. @@ -152,7 +153,8 @@ done # source, and on a host that has never run Claude it is indistinguishable from # a logged-out session. mkdir -p "$HOME/.claude" "$HOME/.claude/agents" "$HOME/.claude/commands" \ - "$HOME/.claude/hooks" "$HOME/.claude/skills" "$HOME/.claude/wf-skills" + "$HOME/.claude/hooks" "$HOME/.claude/skills" "$HOME/.claude/wf-skills" \ + "$HOME/.claude/shared-skills" "$HOME/.agents/skills" # known_hosts is mounted as a *file*, and Docker creates nothing for a file # source: if it is missing the container does not start degraded, it does not diff --git a/CHANGELOG.md b/CHANGELOG.md index 7d750973..f1ff7fbb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added + +- The local container feature shares `~/.agents/skills` with Codex and mounts + `~/.claude/shared-skills` read-only. Relative links between the shared roots + now resolve across host and container usernames, and writes through those + links cannot modify the host's shared skill bodies. Existing containers need + recreation to receive the mounts. + ## [0.34.0] - 2026-09-09 ### Fixed diff --git a/docs/workspace-tools.md b/docs/workspace-tools.md index 58c58728..71245b35 100644 --- a/docs/workspace-tools.md +++ b/docs/workspace-tools.md @@ -55,6 +55,22 @@ that is *already running* skips that step, and the token it was given at startup stays in place, including one it was given before you set `DEVLAUNCH_NO_GH_TOKEN`. Run `dl restart` to replace it. +## Shared agent skills + +The repo's [local container feature](../.devcontainer/claude-code/README.md#shared-skills-for-claude-and-codex) +mounts `~/.agents/skills` for Codex alongside Claude's discovery directory. +It also protects `~/.claude/shared-skills`, so a relative link can point into +that directory without making the skill body writable from the container. +Both storage layouts work across different host and container usernames. + +This is a feature opt-in, not a mount injected by `dl` into every workspace. +Repos with container-local configuration can provision the same directories +through dotfiles. Existing containers using the feature need recreation to +receive its new mounts. The feature preserves existing skills and links; it +does not install Codex or share its credentials and session state. Dotfiles +installers should skip host-mounted skill roots rather than reconcile their +contents through a read-only bind. + ## Claude authentication `claude` starts in every workspace `dl` opens without asking for a login. The diff --git a/test/e2e/test_claude_config_protection.py b/test/e2e/test_claude_config_protection.py index 99a4750b..886f389e 100644 --- a/test/e2e/test_claude_config_protection.py +++ b/test/e2e/test_claude_config_protection.py @@ -31,6 +31,7 @@ import json import os +import shlex import shutil import subprocess from pathlib import Path @@ -45,6 +46,7 @@ READ_ONLY_HEADING, READ_WRITE_HEADING, documented_paths, + documented_home_paths, ) WORKSPACE_ID = "e2e-test-claude-config-protection" @@ -190,17 +192,20 @@ def test_the_container_cannot_write_the_host_files_the_feature_protects( # its `readonly` would drop out of a list derived from it and stop being # probed, so the list comes from the document that promises the protection. config_dir = json.loads(FEATURE_JSON.read_text())["containerEnv"]["CLAUDE_CONFIG_DIR"] - protected = documented_paths(READ_ONLY_HEADING) + protected = documented_home_paths(READ_ONLY_HEADING) writable = documented_paths(READ_WRITE_HEADING) - host_config = home / CONFIG_DIRNAME - before = {name: host_config.joinpath(name).stat().st_mtime_ns for name in protected} + before = {name: home.joinpath(name).stat().st_mtime_ns for name in protected} for name in protected: # The README's trailing slash says whether this is a directory, and a # directory is probed with a new file: finding out costs nothing that # was already there. - probe = f"{config_dir}/{name}injected.md" if name.endswith("/") else f"{config_dir}/{name}" + probe = ( + f"{Path(config_dir).parent}/{name}injected.md" + if name.endswith("/") + else f"{Path(config_dir).parent}/{name}" + ) attempt = in_container( f'if echo injected >> "{probe}" 2>/dev/null; then echo accepted; else echo refused; fi' ) @@ -208,7 +213,7 @@ def test_the_container_cannot_write_the_host_files_the_feature_protects( assert "refused" in attempt.stdout for name, mtime in before.items(): - host_path = host_config / name + host_path = home / name assert host_path.stat().st_mtime_ns == mtime, f"the container changed the host's {name}" if host_path.is_dir(): assert not list(host_path.iterdir()), f"the container added a file to the host's {name}" @@ -229,6 +234,42 @@ def test_the_container_cannot_write_the_host_files_the_feature_protects( f'&& HOME="$(dirname "{config_dir}")" sh -e .devcontainer/{FEATURE_DIR.name}/init-host.sh' ) + # Host installers may keep bodies in either shared root. Both directions + # must resolve after crossing the mount boundary, with a different HOME. + bodies = (".agents/skills/native", ".claude/shared-skills/shared") + for relative in bodies: + folder = home / relative + folder.mkdir() + (folder / "SKILL.md").write_text("original\n") + (home / ".claude/skills/native").symlink_to("../../.agents/skills/native") + (home / ".claude/skills/shared").symlink_to("../shared-skills/shared") + (home / ".agents/skills/shared").symlink_to("../../.claude/shared-skills/shared") + + def check_skill_access(expected): + for agent in (".agents", ".claude"): + for name in ("native", "shared"): + body = shlex.quote( + str(Path(config_dir).parent / agent / "skills" / name / "SKILL.md") + ) + # Read first: a dangling link also refuses a write, but is not + # a protected, discoverable skill. + seen = in_container(f"cat {body}") + assert seen.stdout.strip() == expected + refused = in_container( + f"if echo injected >> {body} 2>/dev/null; " + "then echo accepted; else echo refused; fi" + ) + assert refused.stdout.strip() == "refused" + + check_skill_access("original") + for relative in bodies: + replacement = home / relative / "replacement" + replacement.write_text("updated\n") + replacement.replace(home / relative / "SKILL.md") + check_skill_access("updated") + for relative in bodies: + assert (home / relative / "SKILL.md").read_text() == "updated\n" + @pytest.mark.e2e @pytest.mark.creates_workspace @@ -287,8 +328,8 @@ def test_the_container_follows_the_host_replacing_a_file_by_rename(workspace_cle f"which is a host account switch reaching no running workspace" ) - for name in documented_paths(READ_ONLY_HEADING): - probe = f"{config_dir}/{name}injected.md" + for name in documented_home_paths(READ_ONLY_HEADING): + probe = f"{Path(config_dir).parent}/{name}injected.md" attempt = in_container( f'if echo injected >> "{probe}" 2>/dev/null; then echo accepted; else echo refused; fi', RENAME_WORKSPACE_ID, diff --git a/test/unit/test_claude_code_feature_mounts.py b/test/unit/test_claude_code_feature_mounts.py index e81c832b..a0579059 100644 --- a/test/unit/test_claude_code_feature_mounts.py +++ b/test/unit/test_claude_code_feature_mounts.py @@ -79,11 +79,11 @@ # character is the whole difference between a directory the pre-create hook has # to `mkdir` and a file it must not mount. The README's trailing slash is # therefore read as a declaration rather than as typography. -DOCUMENTED_PATH = re.compile(r"^- `~/\.claude/(?P[^`]+)`") +DOCUMENTED_PATH = re.compile(r"^- `~/(?P[^`]+)`") -def documented_paths(heading: str) -> set: - """The `~/.claude/...` paths the README lists under one mount heading. +def documented_home_paths(heading: str) -> set: + """The home-relative paths the README lists under one mount heading. Only the leading code span of a bullet counts. Prose under the heading mentions these files too, and a test that matched anywhere in the section @@ -100,6 +100,18 @@ def documented_paths(heading: str) -> set: return paths +def documented_paths(heading: str) -> set: + """The Claude-relative subset used by the authentication/state tests.""" + prefix = f"{CONFIG_DIRNAME}/" + paths = { + path.removeprefix(prefix) + for path in documented_home_paths(heading) + if path.startswith(prefix) + } + assert paths, f"the README lists no Claude paths under {heading!r}" + return paths + + @pytest.fixture(name="feature") def feature_fixture() -> dict: return json.loads(FEATURE_JSON.read_text()) @@ -126,20 +138,10 @@ def host_config_fixture(tmp_path) -> Path: def resolve(source: str, host_config: Path) -> Path: - """A manifest mount source as a path under the test's scratch home. - - The separator is part of the prefix, so `~/.claudeX` is outside the - configuration directory rather than a zero-length path inside it. Without - that, a mount of any sibling whose name merely starts with `.claude` - resolved to the configuration directory itself and was then checked in its - place -- passing whatever the real source would have failed, the missing - source that refuses the container create included. - """ - if source == HOST_CONFIG_DIR: - return host_config - prefix = f"{HOST_CONFIG_DIR}/" - assert source.startswith(prefix), f"{source} is outside the configuration directory" - return host_config / source[len(prefix) :] + """Resolve a host-home mount without assuming it belongs to Claude.""" + prefix = f"{LOCAL_HOME}/" + assert source.startswith(prefix), f"{source} is outside the host home" + return host_config.parent / source.removeprefix(prefix) def nested_sources(mounts: list) -> dict: @@ -226,8 +228,10 @@ def test_the_paths_documented_as_protected_are_exactly_the_read_only_mounts(moun defect this ticket reports, and an undocumented read-only mount is a file the container cannot write for reasons nobody wrote down. """ - read_only = {path for path, mount in nested_sources(mounts).items() if "readonly" in mount} - documented = {path.rstrip("/") for path in documented_paths(READ_ONLY_HEADING)} + read_only = { + mount["source"].removeprefix(f"{LOCAL_HOME}/") for mount in mounts if "readonly" in mount + } + documented = {path.rstrip("/") for path in documented_home_paths(READ_ONLY_HEADING)} assert read_only == documented @@ -333,7 +337,11 @@ def test_the_pre_create_hook_leaves_a_configuration_that_already_exists_alone(mo devcontainer = json.loads(strip_jsonc_comments(DEVCONTAINER_JSON.read_text())) run_initialize_command(devcontainer, tmp_path) - existing = [tmp_path / CONFIG_DIRNAME / relative for relative in nested_sources(mounts)] + existing = [ + resolve(mount["source"], tmp_path / CONFIG_DIRNAME) + for mount in mounts + if "readonly" in mount + ] assert existing, "no configuration was created, so this asserts nothing" for path in existing: os.utime(path, ns=(0, 0)) diff --git a/test/unit/test_init_host_heals_stale_mounts.py b/test/unit/test_init_host_heals_stale_mounts.py index b159477e..1bb94da2 100644 --- a/test/unit/test_init_host_heals_stale_mounts.py +++ b/test/unit/test_init_host_heals_stale_mounts.py @@ -118,7 +118,7 @@ # Everything the hook's creation guards would otherwise write, because the # directory is about to stop taking writes -- which is exactly a container # whose configuration is fully mounted, where those guards never fire. -mkdir -p "$dir/agents" "$dir/commands" "$dir/hooks" "$dir/skills" "$dir/wf-skills" +mkdir -p "$dir/agents" "$dir/commands" "$dir/hooks" "$dir/skills" "$dir/wf-skills" "$dir/shared-skills" touch "$dir/CLAUDE.md" "$dir/settings.json" "$dir/.credentials.json" "$dir/.claude.json" mount --bind "$dir" "$dir" mount -o remount,bind,ro "$dir" diff --git a/test/unit/test_shared_agent_skills.py b/test/unit/test_shared_agent_skills.py new file mode 100644 index 00000000..48fce431 --- /dev/null +++ b/test/unit/test_shared_agent_skills.py @@ -0,0 +1,123 @@ +"""Shared skill discovery and write protection across the feature's mounts.""" + +import json +import shlex +import subprocess +from pathlib import Path + +import pytest + +from unit.test_claude_code_feature_mounts import FEATURE_JSON +from unit.test_devcontainer_manifest import ( + DEVCONTAINER_JSON, + parse_mount, + run_initialize_command, + strip_jsonc_comments, +) +from unit.test_init_host_heals_stale_mounts import NAMESPACE + + +def skill_mounts(): + return [parse_mount(spec) for spec in json.loads(FEATURE_JSON.read_text())["mounts"]] + + +def test_codex_discovery_and_shared_bodies_are_mounted_read_only(): + mounts = {mount["source"]: mount for mount in skill_mounts()} + for relative in (".agents/skills", ".claude/shared-skills"): + mount = mounts.get(f"${{localEnv:HOME}}/{relative}") + assert mount is not None, f"the feature does not expose {relative}" + assert mount["type"] == "bind" + assert "readonly" in mount, f"the container can rewrite the host's {relative}" + config = json.loads(FEATURE_JSON.read_text())["containerEnv"]["CLAUDE_CONFIG_DIR"] + assert mount["target"] == f"{config.removesuffix('/.claude')}/{relative}" + + +def test_initialize_creates_missing_skill_roots_without_changing_existing_skills(tmp_path): + home = tmp_path / "host user" + home.mkdir() + shared = home / ".claude/shared-skills/example" + shared.mkdir(parents=True) + body = shared / "SKILL.md" + body.write_text("the existing instructions\n") + discovery = home / ".agents/skills" + discovery.mkdir(parents=True) + link = discovery / "example" + link.symlink_to("../../.claude/shared-skills/example") + config = json.loads(strip_jsonc_comments(DEVCONTAINER_JSON.read_text())) + before = (body.stat().st_mtime_ns, link.lstat().st_mtime_ns) + + for _ in range(2): + result = run_initialize_command(config, home) + assert result.returncode == 0, result.stderr + for mount in skill_mounts(): + source = mount["source"].replace("${localEnv:HOME}", str(home)) + assert Path(source).is_dir() + + assert body.read_text() == "the existing instructions\n" + assert link.readlink().as_posix() == "../../.claude/shared-skills/example" + assert before == (body.stat().st_mtime_ns, link.lstat().st_mtime_ns) + + +@pytest.mark.skipif(not NAMESPACE, reason="requires a private mount namespace") +def test_both_shared_layouts_resolve_and_stay_read_only_in_a_different_home(tmp_path): + """Exercise the actual mounts and writes, including through symlink targets.""" + host = tmp_path / "host user" + guest = tmp_path / "container user" + host.mkdir() + config = json.loads(strip_jsonc_comments(DEVCONTAINER_JSON.read_text())) + result = run_initialize_command(config, host) + assert result.returncode == 0, result.stderr + + # One skill stored in the standard shared root, another in the Claude + # config mount. Both agents see both skills through relative links. + for relative in (".agents/skills/native", ".claude/shared-skills/shared"): + folder = host / relative + folder.mkdir(parents=True, exist_ok=True) + (folder / "SKILL.md").write_text("original\n") + (host / ".claude/skills/native").symlink_to("../../.agents/skills/native") + (host / ".claude/skills/shared").symlink_to("../shared-skills/shared") + (host / ".agents/skills/shared").symlink_to("../../.claude/shared-skills/shared") + (host / ".claude/.credentials.json").write_text("original\n") + + feature_home = json.loads(FEATURE_JSON.read_text())["containerEnv"][ + "CLAUDE_CONFIG_DIR" + ].removesuffix("/.claude") + script = [] + for mount in skill_mounts(): + source = mount["source"].replace("${localEnv:HOME}", str(host)) + relative = mount["target"].removeprefix(feature_home + "/") + target = guest / relative + target.mkdir(parents=True, exist_ok=True) + script.append(f"mount --bind {shlex.quote(source)} {shlex.quote(str(target))}") + if "readonly" in mount: + script.append(f"mount -o remount,bind,ro {shlex.quote(str(target))}") + + for agent in (".agents", ".claude"): + for name in ("native", "shared"): + body = shlex.quote(str(guest / agent / "skills" / name / "SKILL.md")) + script.append(f'test "$(cat {body})" = original') + script.append(f"if echo injected >> {body}; then exit 1; fi") + # Replacing a body on the host must reach both agents while the directory + # mount keeps writes from the container refused. + for relative in (".agents/skills/native", ".claude/shared-skills/shared"): + folder = shlex.quote(str(host / relative)) + script.append(f"echo updated > {folder}/replacement") + script.append(f"mv {folder}/replacement {folder}/SKILL.md") + for agent in (".agents", ".claude"): + for name in ("native", "shared"): + body = shlex.quote(str(guest / agent / "skills" / name / "SKILL.md")) + script.append(f'test "$(cat {body})" = updated') + script.append(f"if echo injected >> {body}; then exit 1; fi") + credentials = shlex.quote(str(guest / ".claude/.credentials.json")) + script.append(f"echo refreshed > {credentials}") + result = subprocess.run( + [*NAMESPACE, "sh", "-eu", "-c", "\n".join(script)], + capture_output=True, + text=True, + check=False, + timeout=30, + ) + assert result.returncode == 0, result.stderr + assert (host / ".claude/.credentials.json").read_text() == "refreshed\n" + for relative in (".agents/skills/native", ".claude/shared-skills/shared"): + assert (host / relative / "SKILL.md").read_text() == "updated\n" From 1fb7138040f5792ef0fb21eaa7263918cc8d6655 Mon Sep 17 00:00:00 2001 From: Austin Gregg-Smith Date: Wed, 9 Sep 2026 09:24:38 +0000 Subject: [PATCH 02/13] fix: an undocumented writable bind of any host home directory passed every mount test Widening resolve() from ~/.claude to the whole host home, so ~/.agents/skills could be shared, also removed the only thing that refused a mount source outside ~/.claude. The read-only agreement test inspects mounts carrying `readonly` and nothing inspects the rest, so adding `source=${localEnv:HOME}/.codex,target=/home/vscode/.codex,type=bind` with a matching mkdir in the hook leaves all 53 tests green while shipping a writable bind of a host directory the README never mentions. Measured that way round before the guard was written, and the guard reports ['.codex'] on it. Claude-Session: https://claude.ai/code/session_01HWRhzmUMVpUo5RB6ZNScFH --- test/unit/test_claude_code_feature_mounts.py | 25 ++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/test/unit/test_claude_code_feature_mounts.py b/test/unit/test_claude_code_feature_mounts.py index a0579059..e14f76ae 100644 --- a/test/unit/test_claude_code_feature_mounts.py +++ b/test/unit/test_claude_code_feature_mounts.py @@ -235,6 +235,31 @@ def test_the_paths_documented_as_protected_are_exactly_the_read_only_mounts(moun assert read_only == documented +def test_no_mount_reaches_a_host_path_the_readme_does_not_list(mounts): + """Every mount is a documented read-only one, or the configuration bind itself. + + The test above only inspects mounts that carry `readonly`, so a mount that + carries no flag at all is invisible to it. That was harmless while `resolve` + refused any source outside `~/.claude`, because the only place an + undocumented mount could land was under a parent that was already writable. + Sharing `~/.agents/skills` widened `resolve` to the whole host home and took + that limit away with it: a writable bind of any home directory -- `~/.codex`, + `~/.ssh` -- now satisfies every other rule in this file, because it is a + bind, its source is a directory the hook creates, and it is nested inside + nothing. + """ + sources = { + mount["source"].removeprefix(f"{LOCAL_HOME}/") + for mount in mounts + if mount["source"] != HOST_CONFIG_DIR + } + documented = {path.rstrip("/") for path in documented_home_paths(READ_ONLY_HEADING)} + assert sources == documented, ( + f"{sorted(sources - documented)} are mounted into the container from the host " + f"home and appear under no mount heading in the README" + ) + + def test_nothing_nested_inside_the_configuration_directory_is_writable(mounts): """Every mount over the directory mount is read-only. From 9cdeacbf59620448b9f547a1ad35baa96c3d2bbf Mon Sep 17 00:00:00 2001 From: Austin Gregg-Smith Date: Wed, 9 Sep 2026 09:27:21 +0000 Subject: [PATCH 03/13] fix: the README accessor silently dropped every path outside ~/.claude Splitting documented_paths into a home-relative accessor and a `.claude/` subset left two functions returning the same set of the same str, told apart only by their names, and each call site joined one of them against a base the type could not carry: the e2e test imported both and joined them against `home` and `home/.claude` in the same function. The subset also filtered without asserting the filter was empty, so documented_paths(READ_ONLY_HEADING) returned six of the README's seven bullets and quietly discarded `.agents/skills/`. mounted_files() consumes it and promises "a file mount added to the feature is held to healing without anyone remembering to extend a list here" -- untrue after the split for anything outside ~/.claude, which is the half of the home the feature had just started mounting. One accessor now, home-relative, with the base joined where it is used. mounted_files() collects the same three paths as before, verified by collecting its parametrization. Claude-Session: https://claude.ai/code/session_01HWRhzmUMVpUo5RB6ZNScFH --- test/e2e/test_claude_config_protection.py | 33 +++++++++---------- test/unit/test_claude_code_feature_mounts.py | 23 +++++-------- .../unit/test_init_host_heals_stale_mounts.py | 12 +++++-- 3 files changed, 34 insertions(+), 34 deletions(-) diff --git a/test/e2e/test_claude_config_protection.py b/test/e2e/test_claude_config_protection.py index 886f389e..7be88815 100644 --- a/test/e2e/test_claude_config_protection.py +++ b/test/e2e/test_claude_config_protection.py @@ -40,12 +40,10 @@ from fixtures.e2e_helpers import create_e2e_workspace from unit.test_claude_code_feature_mounts import ( - CONFIG_DIRNAME, FEATURE_DIR, FEATURE_JSON, READ_ONLY_HEADING, READ_WRITE_HEADING, - documented_paths, documented_home_paths, ) @@ -192,8 +190,9 @@ def test_the_container_cannot_write_the_host_files_the_feature_protects( # its `readonly` would drop out of a list derived from it and stop being # probed, so the list comes from the document that promises the protection. config_dir = json.loads(FEATURE_JSON.read_text())["containerEnv"]["CLAUDE_CONFIG_DIR"] + container_home = Path(config_dir).parent protected = documented_home_paths(READ_ONLY_HEADING) - writable = documented_paths(READ_WRITE_HEADING) + writable = documented_home_paths(READ_WRITE_HEADING) before = {name: home.joinpath(name).stat().st_mtime_ns for name in protected} @@ -202,9 +201,9 @@ def test_the_container_cannot_write_the_host_files_the_feature_protects( # directory is probed with a new file: finding out costs nothing that # was already there. probe = ( - f"{Path(config_dir).parent}/{name}injected.md" + f"{container_home}/{name}injected.md" if name.endswith("/") - else f"{Path(config_dir).parent}/{name}" + else f"{container_home}/{name}" ) attempt = in_container( f'if echo injected >> "{probe}" 2>/dev/null; then echo accepted; else echo refused; fi' @@ -219,7 +218,7 @@ def test_the_container_cannot_write_the_host_files_the_feature_protects( assert not list(host_path.iterdir()), f"the container added a file to the host's {name}" for name in writable: - in_container(f'echo "{{}}" > "{config_dir}/{name}"') + in_container(f'echo "{{}}" > "{container_home}/{name}"') in_container(f'mkdir -p "{config_dir}/projects"') # Run under `sh -e`, which the hook is not run under in production, and that @@ -248,9 +247,7 @@ def test_the_container_cannot_write_the_host_files_the_feature_protects( def check_skill_access(expected): for agent in (".agents", ".claude"): for name in ("native", "shared"): - body = shlex.quote( - str(Path(config_dir).parent / agent / "skills" / name / "SKILL.md") - ) + body = shlex.quote(str(container_home / agent / "skills" / name / "SKILL.md")) # Read first: a dangling link also refuses a write, but is not # a protected, discoverable skill. seen = in_container(f"cat {body}") @@ -300,7 +297,6 @@ def test_the_container_follows_the_host_replacing_a_file_by_rename(workspace_cle home = tmp_path / "home" home.mkdir() project = consumer_project(tmp_path) - host_config = home / CONFIG_DIRNAME create_e2e_workspace( str(project), @@ -310,26 +306,29 @@ def test_the_container_follows_the_host_replacing_a_file_by_rename(workspace_cle ) config_dir = json.loads(FEATURE_JSON.read_text())["containerEnv"]["CLAUDE_CONFIG_DIR"] + container_home = Path(config_dir).parent - for state in sorted(documented_paths(READ_WRITE_HEADING)): - (host_config / state).write_text('{"account": "before"}') - assert "before" in in_container(f'cat "{config_dir}/{state}"', RENAME_WORKSPACE_ID).stdout + for state in sorted(documented_home_paths(READ_WRITE_HEADING)): + (home / state).write_text('{"account": "before"}') + assert ( + "before" in in_container(f'cat "{container_home}/{state}"', RENAME_WORKSPACE_ID).stdout + ) # Precisely what Claude does, and the reason a plain overwrite would not # do: the temporary file is a different inode, and the rename is what # moves the name onto it. - replacement = host_config / f"{state}.tmp" + replacement = home / f"{state}.tmp" replacement.write_text('{"account": "after"}') - replacement.replace(host_config / state) + replacement.replace(home / state) - seen = in_container(f'cat "{config_dir}/{state}"', RENAME_WORKSPACE_ID).stdout + seen = in_container(f'cat "{container_home}/{state}"', RENAME_WORKSPACE_ID).stdout assert "after" in seen, ( f"the container still reads the pre-rename {state}: it is pinned to a dead inode, " f"which is a host account switch reaching no running workspace" ) for name in documented_home_paths(READ_ONLY_HEADING): - probe = f"{Path(config_dir).parent}/{name}injected.md" + probe = f"{container_home}/{name}injected.md" attempt = in_container( f'if echo injected >> "{probe}" 2>/dev/null; then echo accepted; else echo refused; fi', RENAME_WORKSPACE_ID, diff --git a/test/unit/test_claude_code_feature_mounts.py b/test/unit/test_claude_code_feature_mounts.py index e14f76ae..fc081360 100644 --- a/test/unit/test_claude_code_feature_mounts.py +++ b/test/unit/test_claude_code_feature_mounts.py @@ -88,6 +88,13 @@ def documented_home_paths(heading: str) -> set: Only the leading code span of a bullet counts. Prose under the heading mentions these files too, and a test that matched anywhere in the section would be satisfied by a sentence *about* a mount that no longer exists. + + Home-relative and not `~/.claude`-relative, for every caller, because the + feature now mounts outside the configuration directory. A second accessor + returning the `.claude/` subset was the same `set` of the same `str` under a + different name, so the base each one was relative to lived only in that + name -- and it silently dropped `~/.agents/skills/`, which is how + `mounted_files` came to promise a derivation it no longer performed. """ paths = { match.group("path") @@ -100,18 +107,6 @@ def documented_home_paths(heading: str) -> set: return paths -def documented_paths(heading: str) -> set: - """The Claude-relative subset used by the authentication/state tests.""" - prefix = f"{CONFIG_DIRNAME}/" - paths = { - path.removeprefix(prefix) - for path in documented_home_paths(heading) - if path.startswith(prefix) - } - assert paths, f"the README lists no Claude paths under {heading!r}" - return paths - - @pytest.fixture(name="feature") def feature_fixture() -> dict: return json.loads(FEATURE_JSON.read_text()) @@ -288,9 +283,9 @@ def test_the_paths_documented_as_writable_have_no_mount_of_their_own(mounts): token refresh and onboarding state, and that argument is what a reviewer weighs -- so the check is that the argument survives while the mount does not. """ - documented = {path.rstrip("/") for path in documented_paths(READ_WRITE_HEADING)} + documented = {path.rstrip("/") for path in documented_home_paths(READ_WRITE_HEADING)} assert documented, "the README no longer says which files must stay writable" - mounted = set(nested_sources(mounts)) + mounted = {f"{CONFIG_DIRNAME}/{path}" for path in nested_sources(mounts)} assert not (documented & mounted), ( f"{sorted(documented & mounted)} are documented as writable and mounted individually; " f"a file mount pins the inode, so the container stops seeing host changes to them" diff --git a/test/unit/test_init_host_heals_stale_mounts.py b/test/unit/test_init_host_heals_stale_mounts.py index 1bb94da2..7dfc9941 100644 --- a/test/unit/test_init_host_heals_stale_mounts.py +++ b/test/unit/test_init_host_heals_stale_mounts.py @@ -48,7 +48,7 @@ FEATURE_DIR, READ_ONLY_HEADING, READ_WRITE_HEADING, - documented_paths, + documented_home_paths, ) INIT_HOST = FEATURE_DIR / "init-host.sh" @@ -204,9 +204,15 @@ def mounted_files() -> list: remembering to extend a list here. The trailing slash is the README's own declaration of directory-ness; directories cannot lose their inode to a rename and are not healed. + + Home-relative, which is what keeps that promise true now the feature mounts + outside `~/.claude`: taking the `.claude/` subset and prefixing it back on + read the same for today's list and silently covered nothing else. """ - documented = documented_paths(READ_ONLY_HEADING) | documented_paths(READ_WRITE_HEADING) - files = sorted(f"{CONFIG_DIRNAME}/{path}" for path in documented if not path.endswith("/")) + documented = documented_home_paths(READ_ONLY_HEADING) | documented_home_paths( + READ_WRITE_HEADING + ) + files = sorted(path for path in documented if not path.endswith("/")) assert files, "the README documents no file mounts" return files + [".ssh/known_hosts"] From 5291f90dcc23498f73569d29faf03a809c530159 Mon Sep 17 00:00:00 2001 From: Austin Gregg-Smith Date: Wed, 9 Sep 2026 09:28:41 +0000 Subject: [PATCH 04/13] fix: the namespace scenario could mkdir and bind-mount over the real home removeprefix returns its input unchanged when the prefix is absent, and Path discards its left side when the right is absolute, so the two composed into a silent escape. Measured: with CLAUDE_CONFIG_DIR at /home/vscode/.config/claude, removesuffix("/.claude") no-ops, every mount target falls through unchanged, and `scratch / "/home/vscode/.claude/skills"` is /home/vscode/.claude/skills. The scenario mkdir -p's and bind-mounts what that expression returns -- the mkdir outside the namespace, under sudo -- and still passes, because the binds satisfy every read and every EROFS probe it makes. Both joins now assert the prefix they strip. The same manifest edit fails with "CLAUDE_CONFIG_DIR /home/vscode/.config/claude does not end in /.claude" instead, and a target outside the container home has a test of its own. Claude-Session: https://claude.ai/code/session_01HWRhzmUMVpUo5RB6ZNScFH --- test/unit/test_shared_agent_skills.py | 53 ++++++++++++++++++++------- 1 file changed, 39 insertions(+), 14 deletions(-) diff --git a/test/unit/test_shared_agent_skills.py b/test/unit/test_shared_agent_skills.py index 48fce431..de209371 100644 --- a/test/unit/test_shared_agent_skills.py +++ b/test/unit/test_shared_agent_skills.py @@ -3,11 +3,10 @@ import json import shlex import subprocess -from pathlib import Path import pytest -from unit.test_claude_code_feature_mounts import FEATURE_JSON +from unit.test_claude_code_feature_mounts import CONFIG_DIRNAME, FEATURE_JSON, LOCAL_HOME from unit.test_devcontainer_manifest import ( DEVCONTAINER_JSON, parse_mount, @@ -21,15 +20,45 @@ def skill_mounts(): return [parse_mount(spec) for spec in json.loads(FEATURE_JSON.read_text())["mounts"]] +def container_home() -> str: + """The container home the feature puts its configuration directory in.""" + config = json.loads(FEATURE_JSON.read_text())["containerEnv"]["CLAUDE_CONFIG_DIR"] + suffix = f"/{CONFIG_DIRNAME}" + assert config.endswith(suffix), f"CLAUDE_CONFIG_DIR {config} does not end in {suffix}" + return config.removesuffix(suffix) + + +def under(base, path: str, prefix: str): + """`path` re-rooted from `prefix` onto `base`, refusing anything outside it. + + The assertion is the point. `removeprefix` returns its argument unchanged + when the prefix is absent and `Path` discards its left side when the right + is absolute, so the two compose into a silent escape: with a + `CLAUDE_CONFIG_DIR` that is not `/.claude`, every target falls through + unchanged and `scratch / "/home/vscode/.claude/skills"` is + `/home/vscode/.claude/skills`. The namespace scenario below `mkdir -p`s and + bind-mounts what this returns, so a no-op here reaches the real home rather + than the test's, and still passes -- the binds satisfy every read and every + EROFS probe it makes. + """ + assert path.startswith(f"{prefix}/"), f"{path} is not under {prefix}" + return base / path.removeprefix(f"{prefix}/") + + +def test_a_mount_target_outside_the_container_home_is_refused(tmp_path): + """Because the alternative is bind-mounting over the developer's own home.""" + with pytest.raises(AssertionError): + under(tmp_path, "/home/somebody-else/.claude/skills", container_home()) + + def test_codex_discovery_and_shared_bodies_are_mounted_read_only(): mounts = {mount["source"]: mount for mount in skill_mounts()} for relative in (".agents/skills", ".claude/shared-skills"): - mount = mounts.get(f"${{localEnv:HOME}}/{relative}") + mount = mounts.get(f"{LOCAL_HOME}/{relative}") assert mount is not None, f"the feature does not expose {relative}" assert mount["type"] == "bind" assert "readonly" in mount, f"the container can rewrite the host's {relative}" - config = json.loads(FEATURE_JSON.read_text())["containerEnv"]["CLAUDE_CONFIG_DIR"] - assert mount["target"] == f"{config.removesuffix('/.claude')}/{relative}" + assert mount["target"] == f"{container_home()}/{relative}" def test_initialize_creates_missing_skill_roots_without_changing_existing_skills(tmp_path): @@ -50,8 +79,7 @@ def test_initialize_creates_missing_skill_roots_without_changing_existing_skills result = run_initialize_command(config, home) assert result.returncode == 0, result.stderr for mount in skill_mounts(): - source = mount["source"].replace("${localEnv:HOME}", str(home)) - assert Path(source).is_dir() + assert under(home, mount["source"], LOCAL_HOME).is_dir() assert body.read_text() == "the existing instructions\n" assert link.readlink().as_posix() == "../../.claude/shared-skills/example" @@ -79,16 +107,13 @@ def test_both_shared_layouts_resolve_and_stay_read_only_in_a_different_home(tmp_ (host / ".agents/skills/shared").symlink_to("../../.claude/shared-skills/shared") (host / ".claude/.credentials.json").write_text("original\n") - feature_home = json.loads(FEATURE_JSON.read_text())["containerEnv"][ - "CLAUDE_CONFIG_DIR" - ].removesuffix("/.claude") + feature_home = container_home() script = [] for mount in skill_mounts(): - source = mount["source"].replace("${localEnv:HOME}", str(host)) - relative = mount["target"].removeprefix(feature_home + "/") - target = guest / relative + source = under(host, mount["source"], LOCAL_HOME) + target = under(guest, mount["target"], feature_home) target.mkdir(parents=True, exist_ok=True) - script.append(f"mount --bind {shlex.quote(source)} {shlex.quote(str(target))}") + script.append(f"mount --bind {shlex.quote(str(source))} {shlex.quote(str(target))}") if "readonly" in mount: script.append(f"mount -o remount,bind,ro {shlex.quote(str(target))}") From 7cf77d0001e13f6abffb73bb4bd17539124a8a10 Mon Sep 17 00:00:00 2001 From: Austin Gregg-Smith Date: Wed, 9 Sep 2026 09:29:53 +0000 Subject: [PATCH 05/13] fix: "readonly" in mount was True for readonly=false and False for ro Every protection assertion in the suite is that membership test, and this diff added three more call sites of it, including the one asserting the container cannot rewrite the host's shared skills. Measured on parse_mount as it stood: ...,type=bind,readonly=false -> "readonly" in mount is True ...,type=bind,ro -> "readonly" in mount is False So a writable mount spelled `readonly=false` is counted as protected -- a documented protection with nothing behind it, which is the failure test_claude_code_feature_mounts.py exists to catch -- and a genuinely read-only mount spelled with docker's own `ro` synonym is counted as writable. Both silent. parse_mount refuses all three valued spellings now, `readonly=true` included, so the question of whether the value is consulted cannot arise. Confirmed red on each of the three before the assertions were added. Claude-Session: https://claude.ai/code/session_01HWRhzmUMVpUo5RB6ZNScFH --- test/unit/test_devcontainer_manifest.py | 30 +++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/test/unit/test_devcontainer_manifest.py b/test/unit/test_devcontainer_manifest.py index 1bba9b73..5dea4911 100644 --- a/test/unit/test_devcontainer_manifest.py +++ b/test/unit/test_devcontainer_manifest.py @@ -74,11 +74,27 @@ def parse_mount(spec: str) -> dict: Valueless words like `readonly` become keys mapped to the empty string, so the question to ask of one is `"readonly" in mount` and never `mount.get("readonly")` -- the latter is falsy for a flag that is present. + + That question is only honest for the two spellings this refuses, and both + were silent. `readonly=false` is a key with a value, so `"readonly" in + mount` is True and a writable mount is counted as protected -- a documented + protection with nothing behind it, which is what these tests exist to + catch. Docker's own synonym `ro` parses to a different key, so a genuinely + read-only mount is counted as writable. Refused here rather than handled, + because a mount list has no reason to reach for either. """ fields = {} for part in spec.split(","): key, _, value = part.partition("=") fields[key.strip()] = value.strip() + assert fields.get("readonly", "") == "", ( + f"`readonly={fields['readonly']}` in {spec!r} is read as protected by " + f'`"readonly" in mount` whatever it says; write the bare flag or drop it' + ) + assert "ro" not in fields, ( + f"`ro` in {spec!r} is docker's synonym for `readonly` and is read as writable " + f'by `"readonly" in mount`; spell it `readonly`' + ) return fields @@ -172,6 +188,20 @@ def mounts_fixture(devcontainer) -> list: return [parse_mount(spec) for spec in devcontainer["mounts"]] +@pytest.mark.parametrize("flag", ["readonly=false", "readonly=true", "ro"]) +def test_a_readonly_spelling_the_membership_test_misreads_is_refused(flag): + """The two ways `"readonly" in mount` lies, and the one that only looks safe. + + Every protection assertion in this suite is that membership test, so a + spelling it misreads is a security claim that passes while being false in + whichever direction the spelling chose. `readonly=true` is refused with + them: it happens to read correctly, and keeping it out is what stops the + question of whether the value is consulted from arising at all. + """ + with pytest.raises(AssertionError): + parse_mount(f"source=/a,target=/b,type=bind,{flag}") + + def test_devcontainer_manifest_is_this_repos_and_parses(devcontainer): """Comments stripped, this file is JSON, and it is the manifest we mean. From 1f9651d5a996631fd13f6c047f79ec4335725205 Mon Sep 17 00:00:00 2001 From: Austin Gregg-Smith Date: Wed, 9 Sep 2026 09:32:10 +0000 Subject: [PATCH 06/13] fix: the documented remedy for a refused create left the create refused Four hand-written copies of the mount list, and only the README's Read-Only Mounts bullets had a test diffing them against the manifest, so the rest could drift and one had: the `bind mount source path does not exist` section still told a blocked developer to create three directories of seven, which leaves devpod refusing the create for the four it did not name. It also told them to run `echo '{}' > ~/.claude/settings.json`, which truncates the settings file of anyone who already had one, to satisfy a bind that has not existed since the feature stopped mounting files one at a time. The new guard holds all four copies -- both TROUBLESHOOTING.md lists and both by-hand mkdirs -- to set equality with the manifest's read-only sources. It reports the three that agreed and the one that did not: the README's '### `bind mount source path does not exist`' mkdir disagrees with the manifest: ['.agents/skills', '.claude/shared-skills', '.claude/skills', '.claude/wf-skills'] appears in one and not the other Claude-Session: https://claude.ai/code/session_01HWRhzmUMVpUo5RB6ZNScFH --- .devcontainer/claude-code/README.md | 9 ++- test/unit/test_claude_code_feature_mounts.py | 80 ++++++++++++++++++++ 2 files changed, 86 insertions(+), 3 deletions(-) diff --git a/.devcontainer/claude-code/README.md b/.devcontainer/claude-code/README.md index bfebb37c..de5e80ea 100644 --- a/.devcontainer/claude-code/README.md +++ b/.devcontainer/claude-code/README.md @@ -586,11 +586,14 @@ prerequisite" — that is what creates them, on every create, for everyone. To unblock one machine now: ```bash -mkdir -p ~/.claude/{agents,commands,hooks} -touch ~/.claude/CLAUDE.md -echo '{}' > ~/.claude/settings.json +mkdir -p ~/.claude/{agents,commands,hooks,skills,wf-skills,shared-skills} ~/.agents/skills ``` +Directories only. `CLAUDE.md` and `settings.json` have no mount of their own and +so cannot be what is missing — creating them here fixed nothing, and the +`echo '{}' >` that used to stand here truncated the settings file of anyone who +already had one. + ## Security Notes This implementation makes conscious security trade-offs to enable OAuth authentication and persistent setup state: diff --git a/test/unit/test_claude_code_feature_mounts.py b/test/unit/test_claude_code_feature_mounts.py index fc081360..c21472ff 100644 --- a/test/unit/test_claude_code_feature_mounts.py +++ b/test/unit/test_claude_code_feature_mounts.py @@ -255,6 +255,86 @@ def test_no_mount_reaches_a_host_path_the_readme_does_not_list(mounts): ) +TROUBLESHOOTING = FEATURE_DIR / "TROUBLESHOOTING.md" + + +# Both documents draw their trees *inside* `~/.claude`, so a leading `~/` is the +# only thing marking a path as home-relative: `skills/` is the configuration +# directory's, `~/.agents/skills/` is not. +def as_mount_source(path: str) -> str: + path = path.rstrip("/") + return path.removeprefix("~/") if path.startswith("~/") else f"{CONFIG_DIRNAME}/{path}" + + +TREE_MOUNT = re.compile(r"^(?:[├└]──\s+)?(?P[~\w./-]+/)\s+#.*\(read-only mount\)") +CODE_SPAN = re.compile(r"`([^`]+)`") +READ_ONLY_SYMPTOM = "→ Read-only" + +# The two places the README hands over a `mkdir` to run. Anchored per section +# because it offers others -- a dotfiles installer example, a three-directory +# fragment -- and only these two claim to create what the feature mounts. +MKDIR_HEADINGS = ("### Host Machine", "### `bind mount source path does not exist`") + + +def brace_expanded(word: str) -> set: + """`~/.claude/{agents,hooks}` as the two paths a shell would create.""" + head, brace, rest = word.partition("{") + if not brace: + return {word} + names, _, tail = rest.partition("}") + return {f"{head}{name}{tail}" for name in names.split(",")} + + +def documented_mkdir(heading: str) -> set: + for line in _section(FEATURE_README, heading).splitlines(): + if line.startswith("mkdir -p "): + return { + as_mount_source(path) + for word in line.removeprefix("mkdir -p ").split() + for path in brace_expanded(word) + } + raise AssertionError(f"the README section {heading!r} no longer offers a by-hand mkdir") + + +def test_every_hand_written_copy_of_the_mount_list_says_the_same_thing(mounts): + """The trees, the symptom list and the by-hand `mkdir`s agree with the manifest. + + Four hand-maintained copies of one list, and this repo's rule is that a + second copy is allowed only where a test diffs it against the first. Only + the README's Read-Only Mounts bullets had one, so the rest could and did + drift: the `bind mount source path does not exist` remedy still created + three directories of seven, which is a documented fix for a refused + container create that leaves the create refused, and told the developer to + truncate their own `settings.json` on the way past. + """ + read_only = { + mount["source"].removeprefix(f"{LOCAL_HOME}/") for mount in mounts if "readonly" in mount + } + troubleshooting = TROUBLESHOOTING.read_text().splitlines() + copies = { + "TROUBLESHOOTING.md's tree": { + as_mount_source(match.group("path")) + for match in (TREE_MOUNT.match(line) for line in troubleshooting) + if match + }, + "TROUBLESHOOTING.md's read-only symptom": { + as_mount_source(path) + for line in troubleshooting + if READ_ONLY_SYMPTOM in line + for path in CODE_SPAN.findall(line) + }, + **{ + f"the README's {heading!r} mkdir": documented_mkdir(heading) + for heading in MKDIR_HEADINGS + }, + } + for where, listed in copies.items(): + assert listed == read_only, ( + f"{where} disagrees with the manifest: {sorted(listed ^ read_only)} appears in one " + f"and not the other" + ) + + def test_nothing_nested_inside_the_configuration_directory_is_writable(mounts): """Every mount over the directory mount is read-only. From a9edcb068da3eb340cc7d51f068863b01d955da0 Mon Sep 17 00:00:00 2001 From: Austin Gregg-Smith Date: Wed, 9 Sep 2026 09:32:53 +0000 Subject: [PATCH 07/13] fix: resolve()'s argument changed meaning and kept its name The body became `host_config.parent / ...`, so what the parameter wanted was the host home passed as its `.claude` child, and one caller built `tmp_path / CONFIG_DIRNAME` for no reason but to be walked back up. A caller passing an actual home was well-typed and wrong. The fixture and the parameter are the home now. Restored with it is the record the rewrite dropped: the prefix assert is not tidiness, it is there because a mount of a sibling whose name merely started with the prefix used to resolve to the configuration directory and be checked in its place, passing what the real source would have failed. Claude-Session: https://claude.ai/code/session_01HWRhzmUMVpUo5RB6ZNScFH --- test/unit/test_claude_code_feature_mounts.py | 44 ++++++++++++-------- 1 file changed, 26 insertions(+), 18 deletions(-) diff --git a/test/unit/test_claude_code_feature_mounts.py b/test/unit/test_claude_code_feature_mounts.py index c21472ff..c0b1bbd3 100644 --- a/test/unit/test_claude_code_feature_mounts.py +++ b/test/unit/test_claude_code_feature_mounts.py @@ -117,26 +117,38 @@ def mounts_fixture(feature) -> list: return [parse_mount(spec) for spec in feature["mounts"]] -@pytest.fixture(name="host_config") -def host_config_fixture(tmp_path) -> Path: - """A host `~/.claude` as the pre-create hook leaves it, on a fresh machine. +@pytest.fixture(name="host_home") +def host_home_fixture(tmp_path) -> Path: + """A host home as the pre-create hook leaves it, on a fresh machine. Every test that asks what *kind* of thing a mount source is needs a host to look at, and this is the only honest one to use: the hook is what creates these paths on a machine that has never run Claude, so the answer it gives is the answer Docker will get. + + The home and not the `~/.claude` inside it, because the feature now mounts + `~/.agents/skills` too and a fixture returning the child left every caller + walking back up out of it. """ devcontainer = json.loads(strip_jsonc_comments(DEVCONTAINER_JSON.read_text())) result = run_initialize_command(devcontainer, tmp_path) assert result.returncode == 0, result.stderr - return tmp_path / CONFIG_DIRNAME + return tmp_path + +def resolve(source: str, host_home: Path) -> Path: + """A manifest mount source as a path under the test's scratch home. -def resolve(source: str, host_config: Path) -> Path: - """Resolve a host-home mount without assuming it belongs to Claude.""" + The separator is part of the prefix, so a mount of `~/.claudeX` resolves to + a sibling rather than to a zero-length path inside the configuration + directory. Without that, a mount of any sibling whose name merely started + with the prefix was checked in the configuration directory's place -- + passing whatever the real source would have failed, the missing source that + refuses the container create included. + """ prefix = f"{LOCAL_HOME}/" assert source.startswith(prefix), f"{source} is outside the host home" - return host_config.parent / source.removeprefix(prefix) + return host_home / source.removeprefix(prefix) def nested_sources(mounts: list) -> dict: @@ -180,7 +192,7 @@ def test_every_mount_is_a_bind_of_a_host_path(mounts): assert mount.get("type") == "bind", f"{mount.get('target')} is not a bind mount" -def test_every_mount_source_is_a_directory(mounts, host_config): +def test_every_mount_source_is_a_directory(mounts, host_home): """No mount names a file, whatever flags it would carry. This is the rule that keeps the read-only list honest, and it is stated over @@ -194,7 +206,7 @@ def test_every_mount_source_is_a_directory(mounts, host_config): directory" is measured rather than inferred from the path's spelling. """ for mount in mounts: - source = resolve(mount["source"], host_config) + source = resolve(mount["source"], host_home) assert source.is_dir(), ( f"{mount['source']} is mounted but is not a directory. A bind mount of a file " f"does not survive its source being replaced by rename: the mount leaves the " @@ -387,7 +399,7 @@ def test_each_mount_lands_where_the_feature_tells_claude_to_look(feature, mounts assert mount.get("target") == f"{config_dir}/{relative}" -def test_the_pre_create_hook_creates_every_host_path_the_feature_mounts(mounts, host_config): +def test_the_pre_create_hook_creates_every_host_path_the_feature_mounts(mounts, host_home): """The mounted paths exist on the host before the container is asked to start. A missing bind source is not a degraded container: the create is refused @@ -401,11 +413,11 @@ def test_the_pre_create_hook_creates_every_host_path_the_feature_mounts(mounts, this is where that stays true. """ for mount in mounts: - source = resolve(mount["source"], host_config) + source = resolve(mount["source"], host_home) assert source.exists(), f"{mount['source']} is mounted but the hook does not create it" -def test_the_pre_create_hook_creates_no_file_the_feature_does_not_mount(host_config): +def test_the_pre_create_hook_creates_no_file_the_feature_does_not_mount(host_home): """It seeds no `{}` placeholders for paths nothing binds any more. The empty `.credentials.json` and `.claude.json` this used to write existed @@ -414,7 +426,7 @@ def test_the_pre_create_hook_creates_no_file_the_feature_does_not_mount(host_con use -- while an empty credentials file on a host that has never run Claude is indistinguishable from a logged-out session. """ - stray = [path.name for path in host_config.iterdir() if path.is_file()] + stray = [path.name for path in (host_home / CONFIG_DIRNAME).iterdir() if path.is_file()] assert not stray, f"the hook creates {sorted(stray)}, which nothing mounts" @@ -437,11 +449,7 @@ def test_the_pre_create_hook_leaves_a_configuration_that_already_exists_alone(mo devcontainer = json.loads(strip_jsonc_comments(DEVCONTAINER_JSON.read_text())) run_initialize_command(devcontainer, tmp_path) - existing = [ - resolve(mount["source"], tmp_path / CONFIG_DIRNAME) - for mount in mounts - if "readonly" in mount - ] + existing = [resolve(mount["source"], tmp_path) for mount in mounts if "readonly" in mount] assert existing, "no configuration was created, so this asserts nothing" for path in existing: os.utime(path, ns=(0, 0)) From a9920428a7e8c9b70dffb95001d5f3dd5af45ab0 Mon Sep 17 00:00:00 2001 From: Austin Gregg-Smith Date: Wed, 9 Sep 2026 09:33:17 +0000 Subject: [PATCH 08/13] fix: two comments the diff falsified The mounts test's own module docstring still named the five configuration subdirectories as the whole read-only list, in the file whose job is holding that list to the manifest. The README paragraph the shared-skills section replaced carried an operational cost that has no other home: a read-only skills mount means an installer that heals its own links cannot, and `wf` run inside a container says so on every launch and continues. Dropping it left the cost undocumented while the mount it belongs to grew. Claude-Session: https://claude.ai/code/session_01HWRhzmUMVpUo5RB6ZNScFH --- .devcontainer/claude-code/README.md | 6 ++++++ test/unit/test_claude_code_feature_mounts.py | 8 ++++---- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/.devcontainer/claude-code/README.md b/.devcontainer/claude-code/README.md index de5e80ea..1d0ed1b7 100644 --- a/.devcontainer/claude-code/README.md +++ b/.devcontainer/claude-code/README.md @@ -62,6 +62,12 @@ only the discovery directory would still allow writes through a link into the writable `~/.claude` parent. Existing `skills/wf -> ../wf-skills/wf` links remain supported by the `wf-skills` mount. +Read-only for the same reason `commands/` is: a skill is executable +instructions. That costs one thing, and it is a warning rather than a failure — +`wf` heals its own links on every launch, so a `wf` run *inside* a container +reports that it could not refresh them and carries on. Any installer that +rewrites links on startup behaves the same way. + The feature preserves your selection of skills. It creates missing mount roots but does not add, rewrite, or remove individual links. Keep those in your dotfiles or skill installer. Relative links to the mounted roots work across usernames; diff --git a/test/unit/test_claude_code_feature_mounts.py b/test/unit/test_claude_code_feature_mounts.py index c0b1bbd3..e383e45f 100644 --- a/test/unit/test_claude_code_feature_mounts.py +++ b/test/unit/test_claude_code_feature_mounts.py @@ -1,10 +1,10 @@ """The protection the claude-code feature documents, held against what it mounts. The feature's README describes a read-write bind of `~/.claude` with the -subdirectories holding *executable instructions* -- `agents/`, `commands/`, -`hooks/`, `skills/` and `wf-skills/` -- mounted read-only on top of it, and gives -the reason: those are files a prompt injection that edits one of them is not -confined by. The edit is on the host, and it runs again in every later session, +subdirectories holding *executable instructions* mounted read-only on top of it, +plus `~/.agents/skills` outside it for the skills Claude and Codex share, and +gives the reason: those are files a prompt injection that edits one of them is +not confined by. The edit is on the host, and it runs again in every later session, in every other container, on the developer's own machine. Two failures have to be prevented here, and they pull in opposite directions. From 32d81584c61eb07b2eb8e8aee905c038eb5ecf54 Mon Sep 17 00:00:00 2001 From: Austin Gregg-Smith Date: Wed, 9 Sep 2026 10:07:17 +0000 Subject: [PATCH 09/13] fix: a home-relative path from the README could escape the scratch home Collapsing the two accessors dropped the `f".claude/{path}"` re-prefix that had been keeping these strings relative by accident. mounted_files() now feeds them straight into `scratch_home / relative`, and Path discards its left side for an absolute right -- the same escape 5291f90 asserted against one file over. Measured with the README's `.claude.json` bullet spelled `~//.claude/...`: mounted_files() -> ['.claude/.credentials.json', '/.claude/.claude.json', ...] scratch / '/.claude/.claude.json' -> /.claude/.claude.json unshare -m isolates mounts, not file contents, so the scenario's write-back would land on the real path. The assertion sits in the accessor rather than in mounted_files() because every consumer joins these onto a home. Claude-Session: https://claude.ai/code/session_01HWRhzmUMVpUo5RB6ZNScFH --- test/unit/test_claude_code_feature_mounts.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/test/unit/test_claude_code_feature_mounts.py b/test/unit/test_claude_code_feature_mounts.py index e383e45f..ef980c15 100644 --- a/test/unit/test_claude_code_feature_mounts.py +++ b/test/unit/test_claude_code_feature_mounts.py @@ -104,6 +104,12 @@ def documented_home_paths(heading: str) -> set: if match } assert paths, f"the README lists no mounts under {heading!r}" + escaping = sorted(path for path in paths if path.startswith("/")) + assert not escaping, ( + f"{escaping} under {heading!r} came out absolute, and every caller joins these onto a " + f"home: `scratch / '/.claude/x'` is '/.claude/x', so the path leaves the scratch home " + f"for the developer's real one" + ) return paths From efc7afe33ab1337d6df1b3436287cb80166e4452 Mon Sep 17 00:00:00 2001 From: Austin Gregg-Smith Date: Wed, 9 Sep 2026 10:08:52 +0000 Subject: [PATCH 10/13] fix: a second writable bind of a protected source passed every guard Every rule in the mounts test is set equality or a dict lookup keyed on the mount source, so a second mount of an already-protected source is absorbed by all of them at once: the set already contains it, the dict keeps whichever entry came last. Docker creates both. Measured -- this line added to the manifest leaves all 59 tests green: source=${localEnv:HOME}/.agents/skills,target=/home/vscode/.agents/skills-rw,type=bind That is write access to the host's shared skill bodies from inside the container, at a second path, which is the protection this branch exists to add. The guard I added in 1fb7138 was set-shaped too and absorbed it the same way. Two rules that iterate instead: every mount of a documented read-only source carries `readonly`, and no source is mounted twice. The second is what makes the set- and dict-shaped assertions here sound rather than accidentally sound, including nested_sources, which keys by source and had the same hole. Claude-Session: https://claude.ai/code/session_01HWRhzmUMVpUo5RB6ZNScFH --- test/unit/test_claude_code_feature_mounts.py | 40 ++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/test/unit/test_claude_code_feature_mounts.py b/test/unit/test_claude_code_feature_mounts.py index ef980c15..da99f39c 100644 --- a/test/unit/test_claude_code_feature_mounts.py +++ b/test/unit/test_claude_code_feature_mounts.py @@ -273,6 +273,46 @@ def test_no_mount_reaches_a_host_path_the_readme_does_not_list(mounts): ) +def test_every_mount_of_a_protected_source_carries_readonly(mounts): + """A protected source is read-only at *every* target it is mounted at. + + The rules above are set equality and dict lookup, and a source is what they + key on, so a *second* mount of an already-protected source is absorbed by + all of them: the set already contains it, and the dict keeps whichever entry + came last. Docker creates both. One line -- + `source=${localEnv:HOME}/.agents/skills,target=/home/vscode/.agents/skills-rw,type=bind` + -- hands the container write access to the host's shared skill bodies at a + second path while every assertion in this file stays green, which is the + protection this feature exists for, defeated by an entry that reads like a + typo. + """ + documented = {path.rstrip("/") for path in documented_home_paths(READ_ONLY_HEADING)} + writable = sorted( + mount["target"] + for mount in mounts + if mount["source"].removeprefix(f"{LOCAL_HOME}/") in documented and "readonly" not in mount + ) + assert not writable, ( + f"{writable} mount a documented read-only source without `readonly`, so the host " + f"path is writable from the container through those targets" + ) + + +def test_no_host_path_is_mounted_twice(mounts): + """One source, one target, so nothing can be absorbed by keying on it. + + The narrower rule above catches the case that costs a protection. This is + the general one, and it is what makes the set- and dict-shaped assertions in + this file sound rather than accidentally sound. + """ + sources = [mount["source"] for mount in mounts] + repeated = sorted({source for source in sources if sources.count(source) > 1}) + assert not repeated, ( + f"{repeated} are each mounted more than once, and every other rule here keys on the " + f"source, so the duplicate is invisible to them while Docker creates it" + ) + + TROUBLESHOOTING = FEATURE_DIR / "TROUBLESHOOTING.md" From 541efd7a47f2b626e639bc0b7a18d2b114fee44e Mon Sep 17 00:00:00 2001 From: Austin Gregg-Smith Date: Wed, 9 Sep 2026 10:10:02 +0000 Subject: [PATCH 11/13] fix: the guard named five copies of the mount list and diffed four The pattern required a trailing `(read-only mount)` comment, which only TROUBLESHOOTING.md's tree carries. The README draws the same seven paths under `### Host Machine` with comments that say `# Custom agents`, so that copy was read as empty and compared against nothing -- while the docstring told a reader "the trees" were covered. Renaming an entry there to `~/.somewhere-else/` left the suite green. The pattern no longer keys on the annotation, and both trees go through one helper that drops the `~/.claude/` root they hang from, since that bind is the writable one and is asserted elsewhere. The same edit now reports: the README's '### Host Machine' tree disagrees with the manifest: ['.agents/skills', '.somewhere-else'] appears in one and not the other Claude-Session: https://claude.ai/code/session_01HWRhzmUMVpUo5RB6ZNScFH --- test/unit/test_claude_code_feature_mounts.py | 41 ++++++++++++++++---- 1 file changed, 34 insertions(+), 7 deletions(-) diff --git a/test/unit/test_claude_code_feature_mounts.py b/test/unit/test_claude_code_feature_mounts.py index da99f39c..5442f707 100644 --- a/test/unit/test_claude_code_feature_mounts.py +++ b/test/unit/test_claude_code_feature_mounts.py @@ -324,10 +324,20 @@ def as_mount_source(path: str) -> str: return path.removeprefix("~/") if path.startswith("~/") else f"{CONFIG_DIRNAME}/{path}" -TREE_MOUNT = re.compile(r"^(?:[├└]──\s+)?(?P[~\w./-]+/)\s+#.*\(read-only mount\)") +# A drawn directory entry, with or without box-drawing lead-in and with or +# without a trailing comment. Deliberately not keyed on `(read-only mount)`: +# the README draws the same seven paths with comments that say `# Custom +# agents`, so a pattern requiring the annotation read that copy as empty and +# then compared it against nothing. +TREE_ENTRY = re.compile(r"^[│├└─ ]*(?P[~.\w][\w./-]*/)(?:\s+#.*)?$") CODE_SPAN = re.compile(r"`([^`]+)`") READ_ONLY_SYMPTOM = "→ Read-only" +# The tree under this heading is rooted at `~/.claude/` and names it, so the +# configuration bind is a legitimate entry there and the seven read-only mounts +# are the rest. +HOST_LAYOUT_HEADING = "### Host Machine" + # The two places the README hands over a `mkdir` to run. Anchored per section # because it offers others -- a dotfiles installer example, a three-directory # fragment -- and only these two claim to create what the feature mounts. @@ -354,10 +364,28 @@ def documented_mkdir(heading: str) -> set: raise AssertionError(f"the README section {heading!r} no longer offers a by-hand mkdir") +def drawn_tree(lines: list) -> set: + """The mount sources an ASCII directory tree draws, from its entry lines. + + Both trees mix roots: they are drawn inside `~/.claude` and then name + `~/.agents/skills/` at the same indent, so the tilde is what says which + base an entry is relative to. Both also draw `~/.claude/` itself as the + root they hang from, which is the one bind that is supposed to be writable + and is asserted elsewhere, so it is not part of what these copies promise. + """ + paths = { + as_mount_source(match.group("path")) + for match in (TREE_ENTRY.match(line) for line in lines) + if match + } + assert paths, "no tree entry was recognised, so this compares nothing" + return paths - {CONFIG_DIRNAME} + + def test_every_hand_written_copy_of_the_mount_list_says_the_same_thing(mounts): """The trees, the symptom list and the by-hand `mkdir`s agree with the manifest. - Four hand-maintained copies of one list, and this repo's rule is that a + Five hand-maintained copies of one list, and this repo's rule is that a second copy is allowed only where a test diffs it against the first. Only the README's Read-Only Mounts bullets had one, so the rest could and did drift: the `bind mount source path does not exist` remedy still created @@ -370,11 +398,10 @@ def test_every_hand_written_copy_of_the_mount_list_says_the_same_thing(mounts): } troubleshooting = TROUBLESHOOTING.read_text().splitlines() copies = { - "TROUBLESHOOTING.md's tree": { - as_mount_source(match.group("path")) - for match in (TREE_MOUNT.match(line) for line in troubleshooting) - if match - }, + "TROUBLESHOOTING.md's tree": drawn_tree(troubleshooting), + f"the README's {HOST_LAYOUT_HEADING!r} tree": drawn_tree( + _section(FEATURE_README, HOST_LAYOUT_HEADING).splitlines() + ), "TROUBLESHOOTING.md's read-only symptom": { as_mount_source(path) for line in troubleshooting From caf2c1b0b129b0377c9ef0acbcaec96683b6a637 Mon Sep 17 00:00:00 2001 From: Austin Gregg-Smith Date: Wed, 9 Sep 2026 10:11:20 +0000 Subject: [PATCH 12/13] fix: TROUBLESHOOTING.md promised read-only protection for two writable files `### What's Protected (Read-Only)` listed `CLAUDE.md` and `settings.json`, both of which are writable from the container and reach the host -- and a write to `settings.json` is host command execution, because it can name a hook command inline. It also listed three of the seven protected directories. The README's equivalent section was corrected in 9580267; this one is the copy that was missed, and it is the worse of the two to get wrong, because it is the page a developer opens when a write has just been refused. Issue 5's symptom bullet had the same inversion three lines above the sentence correcting it: it offered `~/.claude/CLAUDE.md` as the path that gives "Read-only file system", which is the one thing it cannot give. `test_no_heading_claims_protection_for_a_writable_file` is what should have caught both. It existed and was scoped to the README, so it is parametrized over both documents now, and reports all three offences on the text as it stood. Claude-Session: https://claude.ai/code/session_01HWRhzmUMVpUo5RB6ZNScFH --- .devcontainer/claude-code/TROUBLESHOOTING.md | 17 +++++++++---- test/unit/test_claude_code_feature_mounts.py | 25 ++++++++++++-------- 2 files changed, 28 insertions(+), 14 deletions(-) diff --git a/.devcontainer/claude-code/TROUBLESHOOTING.md b/.devcontainer/claude-code/TROUBLESHOOTING.md index 687bc821..f1183bc4 100644 --- a/.devcontainer/claude-code/TROUBLESHOOTING.md +++ b/.devcontainer/claude-code/TROUBLESHOOTING.md @@ -168,7 +168,7 @@ reached through it are missing or unwritable. ### Issue 5: "Read-only file system" Error **Symptoms:** -- Error when trying to write to `~/.claude/CLAUDE.md` or similar +- Error when trying to write to `~/.claude/skills/` or another instruction directory - Operations fail with "Read-only file system" **Expected Behavior:** @@ -376,13 +376,22 @@ watch -n 1 'stat ~/.claude/.claude.json | grep Modify' ## Security Considerations ### What's Protected (Read-Only) -- `CLAUDE.md` - Prevents prompt injection -- `settings.json` - Prevents config tampering -- `agents/`, `commands/`, `hooks/` - Prevents malicious modifications + +The instruction directories, and only those: see the tree at the top of this +page, which a test holds to the manifest. They carry the code and instructions +the agents execute, which is why they are the ones singled out. ### What's Writable (Necessary Risk) - `.credentials.json` - OAuth tokens (necessary for auth) - `.claude.json` - Setup state (necessary to skip wizard) +- `CLAUDE.md` - a prompt injection can edit it, and the edit reaches the host +- `settings.json` - can name a hook command inline, so a write here is host + command execution + +The last two were mounted read-only under the old file-mount layout and are +not protected any more. A read-only mount over a file does not survive the +host replacing that file by rename, so the protection ended at the developer's +next edit; see "Why only directories are mounted" in the README. ### Mitigation - Only use in trusted repositories diff --git a/test/unit/test_claude_code_feature_mounts.py b/test/unit/test_claude_code_feature_mounts.py index 5442f707..0b850d65 100644 --- a/test/unit/test_claude_code_feature_mounts.py +++ b/test/unit/test_claude_code_feature_mounts.py @@ -557,26 +557,31 @@ def _protection_headings(readme: str) -> list: ] -def test_no_heading_claims_protection_for_a_writable_file(): +@pytest.mark.parametrize("document", [FEATURE_README, TROUBLESHOOTING], ids=lambda p: p.name) +def test_no_heading_claims_protection_for_a_writable_file(document): """No section listing protected paths may name a file that is writable. The mount-agreement test binds one heading by its exact text, so a *second* - list of protected paths — which is what this README grew — is checked by + list of protected paths — which is what these documents grew — is checked by nothing. This asks the question of every heading that claims protection instead of one, because the failure was a heading nobody had registered. - Only bullets count, for `documented_paths`'s reason: the prose under these - headings discusses `settings.json` precisely to say it is *not* protected, - and a substring match anywhere in the section would fail on the sentence - that fixes the problem. + Both documents, because scoping it to the README is how TROUBLESHOOTING.md + went on promising that `CLAUDE.md` and `settings.json` were read-only after + they stopped being. That one is the worse of the two to get wrong: it is + the page a developer opens when a write has just been refused. + + Only bullets count, for `documented_home_paths`'s reason: the prose under + these headings discusses `settings.json` precisely to say it is *not* + protected, and a substring match anywhere in the section would fail on the + sentence that fixes the problem. """ - readme = FEATURE_README.read_text() - headings = _protection_headings(readme) - assert headings, "no heading in the README claims protection; the guard is guarding nothing" + headings = _protection_headings(document.read_text()) + assert headings, f"no heading in {document.name} claims protection; the guard guards nothing" offences = [] for heading in headings: - for line in _section(FEATURE_README, heading).splitlines(): + for line in _section(document, heading).splitlines(): stripped = line.lstrip() if not stripped.startswith(("-", "*")): continue From 5e98fbf5c1bb522306f788dd17b424d50338d51e Mon Sep 17 00:00:00 2001 From: Austin Gregg-Smith Date: Wed, 9 Sep 2026 10:11:48 +0000 Subject: [PATCH 13/13] fix: resolve()'s docstring described a defect it can no longer have a9edcb0 restored the paragraph about `~/.claudeX` collapsing to a zero-length path inside the configuration directory. That collapse needed the old `${localEnv:HOME}/.claude/` prefix; this branch resolves against the host home, so `~/.claudeX` lands at `home/.claudeX` with or without the separator -- measured both ways. The comment was true of the code it was written for and false of the code it was pasted into, which is worse than no comment. It now says what the assert does protect, and points at the guard that answers the question it used to be confused with. Claude-Session: https://claude.ai/code/session_01HWRhzmUMVpUo5RB6ZNScFH --- test/unit/test_claude_code_feature_mounts.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/test/unit/test_claude_code_feature_mounts.py b/test/unit/test_claude_code_feature_mounts.py index 0b850d65..0506f89c 100644 --- a/test/unit/test_claude_code_feature_mounts.py +++ b/test/unit/test_claude_code_feature_mounts.py @@ -145,12 +145,12 @@ def host_home_fixture(tmp_path) -> Path: def resolve(source: str, host_home: Path) -> Path: """A manifest mount source as a path under the test's scratch home. - The separator is part of the prefix, so a mount of `~/.claudeX` resolves to - a sibling rather than to a zero-length path inside the configuration - directory. Without that, a mount of any sibling whose name merely started - with the prefix was checked in the configuration directory's place -- - passing whatever the real source would have failed, the missing source that - refuses the container create included. + The assert is the whole of what this adds over a `removeprefix`, and it is + what keeps a wrong answer from being a plausible one: a source that is not + under `${localEnv:HOME}` has no place under the scratch home either, and + silently resolving it to one would check a path the real mount never names. + Which host paths may be mounted at all is a separate question, asked by + `test_no_mount_reaches_a_host_path_the_readme_does_not_list`. """ prefix = f"{LOCAL_HOME}/" assert source.startswith(prefix), f"{source} is outside the host home"