diff --git a/.devcontainer/claude-code/README.md b/.devcontainer/claude-code/README.md index 48e1d2ac..1d0ed1b7 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,51 @@ 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 +### 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. + +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. +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; +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 +148,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 +286,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 +296,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 +419,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: @@ -552,20 +592,24 @@ 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: ### 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 +633,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 +648,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 +659,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..f1183bc4 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 @@ -166,12 +168,12 @@ 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:** 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 @@ -374,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/.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..7be88815 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 @@ -39,12 +40,11 @@ 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, ) WORKSPACE_ID = "e2e-test-claude-config-protection" @@ -190,17 +190,21 @@ 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) - writable = documented_paths(READ_WRITE_HEADING) - host_config = home / CONFIG_DIRNAME + container_home = Path(config_dir).parent + protected = documented_home_paths(READ_ONLY_HEADING) + writable = documented_home_paths(READ_WRITE_HEADING) - 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"{container_home}/{name}injected.md" + if name.endswith("/") + else f"{container_home}/{name}" + ) attempt = in_container( f'if echo injected >> "{probe}" 2>/dev/null; then echo accepted; else echo refused; fi' ) @@ -208,13 +212,13 @@ 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}" 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 @@ -229,6 +233,40 @@ 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(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}") + 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 @@ -259,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), @@ -269,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_paths(READ_ONLY_HEADING): - probe = f"{config_dir}/{name}injected.md" + for name in documented_home_paths(READ_ONLY_HEADING): + 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 e81c832b..0506f89c 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. @@ -79,15 +79,22 @@ # 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 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") @@ -97,6 +104,12 @@ def documented_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 @@ -110,36 +123,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_config: 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 `~/.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. + 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`. """ - 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) :] + prefix = f"{LOCAL_HOME}/" + assert source.startswith(prefix), f"{source} is outside the host home" + return host_home / source.removeprefix(prefix) def nested_sources(mounts: list) -> dict: @@ -183,7 +198,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 @@ -197,7 +212,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 " @@ -226,11 +241,185 @@ 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 +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_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" + + +# 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}" + + +# 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. +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 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. + + 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 + 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": 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 + 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. @@ -259,9 +448,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" @@ -283,7 +472,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 @@ -297,11 +486,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 @@ -310,7 +499,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" @@ -333,7 +522,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 = [tmp_path / CONFIG_DIRNAME / relative for relative in nested_sources(mounts)] + 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)) @@ -368,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 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. diff --git a/test/unit/test_init_host_heals_stale_mounts.py b/test/unit/test_init_host_heals_stale_mounts.py index b159477e..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" @@ -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" @@ -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"] diff --git a/test/unit/test_shared_agent_skills.py b/test/unit/test_shared_agent_skills.py new file mode 100644 index 00000000..de209371 --- /dev/null +++ b/test/unit/test_shared_agent_skills.py @@ -0,0 +1,148 @@ +"""Shared skill discovery and write protection across the feature's mounts.""" + +import json +import shlex +import subprocess + +import pytest + +from unit.test_claude_code_feature_mounts import CONFIG_DIRNAME, FEATURE_JSON, LOCAL_HOME +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 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"{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}" + assert mount["target"] == f"{container_home()}/{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(): + 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" + 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 = container_home() + script = [] + for mount in skill_mounts(): + 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(str(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"