fix(devcontainer): repair bind mount points' daemon-created parents

[dlovell]

Defect

~/.claude/projects inside the container came up root:root 755, so the container user could not create anything in it — including the per-project memory directory Claude expects at ~/.claude/projects/<key>/memory/.

named_volume_targets filtered the merged compose config to type == "volume", so the transcript bind at ~/.claude/projects/<key> never reached the chown driver. Its parent is precisely a directory the daemon creates as root to host that mount point — the case lib/volume-perms.sh already documents for volumes, mechanically identical for a bind; only the type filter excluded it.

The compensating chown vscode:vscode /home/vscode/.claude in setup_claude() does not reach it either, and its "Top-level is enough — setup-claude creates the contents as vscode" comment was simply wrong: the directory in question is not created by setup-claude, it is created by the daemon. That comment is corrected here rather than left standing.

Fix

Binds cannot just be handed to the existing function: it does a guarded chown -R, which for a bind walks onto the host side and rewrites ownership of real files — the opposite of the invariant the lib states at lib/volume-perms.sh:64-65. So:

• the ancestor walk is split out of dev_chown_volume_targets into dev_chown_ancestors;
• a dev_chown_mount_points dispatcher routes volume → guarded recursion + ancestors, bind → ancestors only, anything else → nothing (the recursive branch is the destructive one, so a mount type the lib has not been taught about must not reach it by default);
• the compose query emits <kind>:<target>.

Loop variables move off the shared _vp_ prefix: these are POSIX sh globals, so a caller and callee sharing a prefix would let the callee clobber the caller's iterator mid-walk.

That repairs the directory after the daemon has created it. A second half — shipping it in the image so it is never created wrong — was added later; see Second half: the image ships the directory below.

Deviation from the patch proposed in the issue — please review this part closely

The issue's patch would have introduced a new host-side write, so this PR adds something that proposal did not have.

docker-compose.yml:26-27 mounts DEV_MAIN_TREE and DEV_MAIN_GIT at their host paths, and <tree>/.git nests inside <tree>. On a host whose checkout lives under the container home prefix — a host user named vscode, or Codespaces — the ancestor walk starting from the .git bind reaches the worktree bind and chowns the user's real repo directory. Ancestors-only is not sufficient protection on its own: a bind mount point can be reached as an ancestor.

So dev_chown_ancestors now refuses to chown any path that is itself a bind mount point, skipping it while continuing the walk past it (the dirs above a bind are still container-side and still need repair). Bind targets are collected in a first pass, so the protection cannot depend on the order compose emits mounts in.

This also makes #81's nested memory bind safe: its first ancestor is the transcript bind mount point.

Guards (ADR-0005)

tests/test-volume-chown-guard.sh: 21 → 47 assertions, three new sections, mutation record in the header per the Verified (ADR-0005 §2) format named in #111.

The compose query is guarded hermetically: its python snippet is extracted from dev/devcontainer at check time and run against a synthetic docker compose config document — derived from the source of truth, not restated. dc config itself needs docker; the snippet is the part that can run in this suite.

Three mutations, each reverted after watching it go red:

1. the filter reverted to the pre-~/.claude/projects left root-owned: bind mount targets are excluded from the volume chown, so the daemon-created parent is never repaired #106 type == "volume" → 1 red (only the compose-query assertion can catch this — everything else drives the lib directly)
2. the bind-mount-point protection dropped → 3 red
3. the bind) arm routed into the recursive branch → 1 red

Test plan

• bash tests/run-all → 28 suites, 0 failures, exit 0
• pre-commit run --all-files → all hooks pass
• The suite needs no docker daemon, so this is reproducible anywhere.

Review round (pr-reviewer, verdict needs-changes — addressed)

An independent review found that the PR overreached in its claims. dev_chown_volume_targets runs an unfiltered chown -R rooted at a volume, and chown has no mount awareness, so it still descends through a bind nested under that volume — the claude-home / transcript-bind topology this PR is about. _vp_never_chown is consulted by dev_chown_ancestors alone. I reproduced it: chown -Rv on the volume path visits the nested bind's contents.

The recursion predates this PR (origin/main:80, unchanged here). What this PR did was assert in four places that it could not happen. Fixed by scoping the claims rather than by widening the change:

• all four claims (lib header, dev/devcontainer, CLAUDE.md, and the setup_claude comment) now say ancestor walk, and say explicitly that the recursion is not covered
• CLAUDE.md records the residual as an accepted unguarded: invariant with the reason, per ADR-0005 §1
• chown -R on a named volume crosses into a bind nested under it — the recursive branch is not mount-aware #115 filed with the mechanism, the failure scenario, the repeat-offender history and three fix options
• the suite now pins the current, unsafe behaviour honestly ("a volume containing a bind is STILL recursed into") instead of leaving the impression both nesting directions were covered — item 7 previously tested only the safe direction. Verified that assertion flips red the moment the recursion becomes bind-aware, so it is chown -R on a named volume crosses into a bind nested under it — the recursive branch is not mount-aware #115's red-to-green target.
• nit: added the paired positive assertion to the argument-order case, which was negative-only and would have passed on an empty log.

Verified first-hand while addressing the above: the recursion really does descend into the nested bind (chown -Rv on the volume path visits the bind's contents), and docker compose config — which needs no daemon — emits type: bind for all six short-form mounts, volume for claude-home with the transcript bind nested inside it, and never puts tmpfs: entries in volumes at all. That closes the one item this PR originally listed as unverified, and confirms the dispatcher's tmpfs arm is defensive only.

Reported by the review but not re-verified here, so weigh them as its word rather than established: that the case-with-quoted-expansion membership test, the <kind>:<target> split against colon-bearing paths, the global-prefix discipline and the _vp_never_chown reset all hold under dash; that the item-8 awk extraction fails closed; and that Compose normalises trailing-slash targets (which would otherwise be a fail-open in the exact-string match).

Review round 2 (independent, verdict needs-changes — addressed)

A second pr-reviewer, given no knowledge of round 1's findings, found six. The serious one was mine, in the guard itself:

• Fail-open assertion. "a direct volume-targets call does not inherit protections" used assert_contains with a needle that is a prefix of the recursive log line, so it matched -R <owner> <tree>/.venv rather than any ancestor chown. Verified by mutation: deleting the _vp_never_chown reset leaves the suite at 41 passed, 0 failed while the leak is live. The assertion whose entire job was catching that reported PASS. Now line-anchored — and it is the same trap this file documents 150 lines earlier for the home-prefix case. A second, milder instance ("a volume target's ancestors are still chowned") fixed the same way.
• Over-claimed protection. _vp_is_protected was exact-match on mount points, so a path inside a bind but not itself a mount point was unprotected — reachable when one bind nests two levels under another, which host-mounts.txt permits (arbitrary <host>:<container>, no depth limit). Now protects under-a-bind paths too, with the deep-nesting case tested.
• False flat claim. "A bind is never recursed into" was unqualified and wrong, and the scoping bullet pointed at the wrong sentence — the ancestor walk does not recurse at all, so scoping that to it was incoherent. Reworded to the dispatcher.
• Mutation record misdirected: the header listed 8 items while the body had 9, so "Item 8" resolved to the wrong one.
• Nit: the chown -R on a named volume crosses into a bind nested under it — the recursive branch is not mount-aware #115 assertion's label implied it witnessed descent; under a stub it observes the unpruned invocation. Relabelled honestly.
• Nit: the new query-filter ↔ dispatcher coupling had no drift guard, against the repo's own convention. Added, derived — both kind sets are parsed from their own source.

All five mutations re-run and the recorded counts corrected to what the runs actually produced (they were not what I predicted). 21 → 47 assertions.

Review round 3 (independent, fable — addressed)

Five findings, none in the shipped lib (which survived every scenario the reviewer constructed under real dash). All in the guard and its record:

• The mutation record was wrong again, for the third time — and I found the cause rather than just the numbers. My scratch copies were made with cp -a from this worktree, whose .git is a file pointing at another worktree's gitdir, so git checkout between mutations silently failed to restore and mutations accumulated, inflating every hand-measured count. Re-measured in fresh copies: M3 and M4 are 1 red each, not 2. Record rewritten with a METHOD note so the next person doesn't repeat it.
• A second fail-open, same file: "the walk continues past a protected bind" passed with the walk mutated to break instead of skip, because the protected bind was itself an argument and its own walk chowned everything above it. Now drives dev_chown_ancestors directly with a hand-filled exclusion set; mutation 6 pins it.
• The "derived" kind-set guard derived over a hidden vocabulary — both extractions shared a hardcoded (volume|bind|tmpfs|npipe|cluster), so Compose's image type (v2.35+) was invisible to both and that drift would pass green. Both sides now parse generically; mutation 7 pins it.
• _vp_is_protected loops forever on a set missing its trailing newline (unreachable today, but this runs as root on every cold start) — termination guard added.
• Item 8's anti-vacuity assertions could not fire: set -e aborted at the assignment. || true added, and mutation 1 now reds that assertion, proving it works.

41 → 47 assertions, seven recorded mutations, all measured in isolation.

Second half: the image ships the directory (0f21a58)

Everything above repairs ~/.claude/projects after the daemon has created it root-owned. That repair runs in setup(), which only dev/devcontainer calls — so an entry path that never goes through it gets the #106 failure back untouched: VS Code "Reopen in Container" (#43), a bare docker compose up, anything driving the compose files directly.

Both image routes now pre-create the directory instead:

• Dockerfile — /home/vscode/.claude/projects added to the existing mkdir -p; the chown -R … /home/vscode/.claude on the next line already covers it.
• nix/base/Dockerfile.nix-default — a new RUN mkdir -p with an explicit chown. Explicit because the UID remap above it is guarded on USER_UID != 1000, so at the default UID it does not run and the dir would ship root-owned.

A fresh claude-home volume seeds its contents from the image path, so the volume comes up with projects/ already vscode-owned and the daemon has nothing left to create under it.

Why the nix half is in the tail build, not nix/base/flake.nix. The flake is where the sibling home dirs (.claude, .cache, .ssh) are pre-created, so that is the obvious home for it — but a fix there reaches nobody until a republish and a BASE_IMAGE repin (#83, the same trap that left #70 live on ghcr and unreachable). The tail build rebuilds for every project image, so the fix is live as soon as this merges. Noted in the code comment, since the asymmetry with its siblings is otherwise a puzzle.

This does not replace the ancestor walk — a container built before this change still needs it, and the walk is what covers any other nested bind a project overlay declares.

Guard

tests/test-volume-chown-guard.sh section 10, 47 → 54 assertions.

The directory is derived, not restated: it is read out of docker-compose.yml as the parent of the one mount target that interpolates DEV_CONTAINER_PROJECT_KEY (that variable is what makes the target a per-project subdirectory), so moving or renaming the transcript mount moves the guard with it.

The check is order-aware, which is the part worth reviewing. Both Dockerfiles run chown -R /home/vscode during the UID remap above these lines, where the directory does not yet exist. A guard that merely grepped for a mkdir and a chown would read that remap as ownership and stay green on an image shipping the dir root-owned — the exact state being guarded against. So precreated locates the mkdir and only accepts a covering chown at a later position.

Four mutations, fresh tree copy each (per the METHOD note now in that file's header):

1. /projects dropped from the root Dockerfile's mkdir, back to the pre-~/.claude/projects left root-owned: bind mount targets are excluded from the volume chown, so the daemon-created parent is never repaired #106 form → 2 red (once the mkdir is gone there is no creation for the chown probe to cover, so both assertions for that route fail)
2. the nix tail RUN deleted entirely → 2 red, the same pair for that route — the routes are checked independently, so neither mutation is masked by the other file being correct
3. that RUN's && chown dropped, mkdir left → 1 red, "chowns it AFTER creating it". This is the mutation that pays for the ordering logic.
4. the compose target moved to .claude/transcripts/<key> with both Dockerfiles left alone → 4 red, and the failure names print /home/vscode/.claude/transcripts — which is what shows the expectation is read out of compose rather than restated in the assertions.

bash tests/run-all → 0 failures, exit 0. pre-commit (shellcheck, hadolint) clean on the changed files.

CLAUDE.md records the invariant, typed test:.

Notes for the reviewer

• Existing containers are not repaired until their next recreate. setup() is cold-start only (gated on is_running), so a container already running keeps the root-owned directory. Recorded in the code comment.
• Not verified against a live container — no docker daemon in this environment. The dispatch, the protection and the compose query are covered hermetically. The one gap listed here originally — whether dc config --format=json really emits type: bind for the short-form mounts — was closed by the review, which verified it offline against the real compose file.
• This does not fix Transcript writes die silently and permanently when the host log dir is replaced under a running container; ensure_claude_logs() masks it with a fresh inode #105 (the transcript data loss). That one is about the bind source being unlinked on the host, and only a container recreate repairs a mount in that state.

Closes #106
Refs #115

🤖 Generated with Claude Code

https://claude.ai/code/session_01MMRkcVZwXpwBqjMY4bSehH