diff --git a/.devcontainer/claude-code/install.sh b/.devcontainer/claude-code/install.sh index d726f06e..6c66e725 100755 --- a/.devcontainer/claude-code/install.sh +++ b/.devcontainer/claude-code/install.sh @@ -175,16 +175,38 @@ create_claude_directories() { mkdir -p "$TARGET_HOME/.claude/commands" mkdir -p "$TARGET_HOME/.claude/hooks" - # Create empty config files if they don't exist - if [ ! -f "$TARGET_HOME/.claude/.credentials.json" ]; then - echo "{}" > "$TARGET_HOME/.claude/.credentials.json" - chmod 600 "$TARGET_HOME/.claude/.credentials.json" - fi - - if [ ! -f "$TARGET_HOME/.claude/.claude.json" ]; then - echo "{}" > "$TARGET_HOME/.claude/.claude.json" - chmod 600 "$TARGET_HOME/.claude/.claude.json" - fi + # No empty config files are seeded, and `.credentials.json` is the reason. + # + # This used to write `{}` into `.credentials.json` and `.claude.json` when they were + # missing. Both existed only to give a bind mount a source to cover, from the layout + # where this feature mounted nine individual paths under `~/.claude` -- and the + # host-side hook already retired its half of that on the same reasoning: "Seeding an + # empty {} over a credentials file was never anything but a way to satisfy a bind + # source, and on a host that has never run Claude it is indistinguishable from a + # logged-out session" (init-host.sh). + # + # In the container that stub is worse than useless, because it wins. `dl` forwards a + # profile's login as CLAUDE_CODE_OAUTH_TOKEN, Claude Code reads the credentials file + # first, and an empty one is a logged-out session: the agent asks the operator to log + # in while a valid token sits in its environment. It stayed hidden for as long as this + # feature mounted the host's real credentials file *over* the stub -- so the bug was + # invisible in exactly the configuration that could not use the token anyway, and + # surfaced the moment a container was given the forwarded login as its only one. + # + # Measured both ways in one container: with the stub, `claude` prompts for a login; + # with it removed and nothing else changed, `claude -p` answers on the forwarded + # token. Claude Code creates `.credentials.json` itself on first use, so there is + # nothing to replace it with. + # + # `.claude.json` had a second victim, and it was devlaunch. The provisioner seeds + # {"hasCompletedOnboarding":true} into $CLAUDE_CONFIG_DIR/.claude.json and exits + # early when that file already exists -- and this feature points CLAUDE_CONFIG_DIR + # at the directory being created right here, so the stub satisfied that guard. + # Every container built from this feature skipped the onboarding seed and met its + # operator with the trust prompt, which is the opposite of what seeding it was for. + # + # Guarded by test_the_feature_seeds_no_empty_credential and + # test_the_installer_may_not_create_the_file_that_marks_claude_onboarded. # Set proper ownership if [ "$(id -u)" -eq 0 ]; then diff --git a/CHANGELOG.md b/CHANGELOG.md index 2cf8202d..f4edf9a9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,41 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Fixed + +- **A forwarded Claude login no longer loses to an empty credentials file.** + `dl --claude-profile bear ` resolved the profile, forwarded the token as + `CLAUDE_CODE_OAUTH_TOKEN`, and `claude` asked the operator to log in anyway. + The devcontainer feature seeded `{}` into `.credentials.json` when the file was + missing, and Claude Code reads that file before it reads the environment: an + empty one is not a placeholder, it is a logged-out session, and it wins. + + The stub was invisible for as long as the feature mounted the host's real + credentials file *over* it, so the bug could only ever appear where the + forwarded token was the container's only login -- which is the one + configuration those mounts rule out, since `forwarded_claude` declines to + forward into a config directory it does not own. It surfaced the moment a + workspace had its Claude mounts removed, which is what wanting a profile + requires. + + `.claude.json` went with it, and that half fixed something else. The + provisioner seeds `{"hasCompletedOnboarding":true}` there and exits early when + the file exists, so the stub had been suppressing devlaunch's own onboarding + seed: every container built from this feature met its operator with the trust + prompt. Claude Code creates the credentials file itself on first use, so + neither stub had anything to replace it. + +- **The ssh agent socket is bound from `$SSH_AUTH_SOCK` rather than from a guess + at where it lives.** The mount source was `${localEnv:HOME}/.ssh/agent.sock`, + which is not a path any agent picks by itself: gpg-agent listens on + `$XDG_RUNTIME_DIR/gnupg/S.gpg-agent.ssh` and `ssh-agent` on a `/tmp/ssh-XXXX` + mktemp path. On such a host `devpod up` refused the create outright with + `bind mount source path does not exist`, before the container existed, which + reads as a broken tool rather than as a manifest naming a path the host never + had. `init-host.sh` cannot paper over this the way it does for `known_hosts`: + that one is touched into existence, and there is no touching a socket into + being an agent. + ## [0.33.0] - 2026-09-07 ### Fixed diff --git a/test/unit/test_devcontainer_manifest.py b/test/unit/test_devcontainer_manifest.py index f252c3bb..1bba9b73 100644 --- a/test/unit/test_devcontainer_manifest.py +++ b/test/unit/test_devcontainer_manifest.py @@ -683,6 +683,119 @@ def test_the_devcontainer_installs_the_committed_lock_rather_than_solving_its_ow ) +SEEDED_NAMES = (".credentials.json", ".claude.json") + + +def _seeding_lines(installer, name): + """Lines of `installer` that would put `name` on disk. + + A comment naming the file is the one exemption. Nothing else is: any other + mention counts, including one that only assigns the path to a variable. + + Keying on a list of write verbs was the first attempt and it is the reason + this is a function with tests of its own. `>`, `tee` and `cp` miss `touch` + -- which is what `.devcontainer/claude-code/README.md` prescribes for this + exact file, so it is the likeliest way the stub returns -- they miss + `install -m 600 /dev/null`, and they miss any spelling that names the path + on one line and redirects into the variable on the next. + """ + return [ + line + for line in installer.splitlines() + if name in line and not line.lstrip().startswith("#") + ] + + +@pytest.mark.parametrize( + "write", + [ + 'touch "$TARGET_HOME/.claude/.credentials.json"', + 'install -m 600 /dev/null "$TARGET_HOME/.claude/.credentials.json"', + # Names the path here and redirects into `$cred` on the next line, so + # neither line carries both the filename and a redirection. + 'cred="$TARGET_HOME/.claude/.credentials.json"', + ': > "$TARGET_HOME/.claude/.credentials.json"', + 'echo "{}" > "$TARGET_HOME/.claude/.credentials.json"', + ], +) +def test_the_seeding_guard_catches_a_write_that_is_not_a_redirection(write): + """The guard has to hold against the spellings, not against one idiom. + + Every line here re-seeds the credentials file and every one of them passed + the guard as first written, which claimed to hold "over the whole installer + ... so it cannot come back under another name" while matching only `>`, + `tee` and `cp`. + """ + installer = f' mkdir -p "$TARGET_HOME/.claude"\n {write}\n' + assert _seeding_lines(installer, ".credentials.json") == [f" {write}"] + + +def test_the_seeding_guard_reads_a_comment_as_a_comment(): + """The exemption the guard does grant, since the fix is 22 lines of comment + explaining why the file is not seeded, and every one of them names it.""" + assert _seeding_lines(" # writes .credentials.json\n", ".credentials.json") == [] + + +def test_the_feature_seeds_no_empty_credential(): + """The feature must not write a `.credentials.json` into the container. + + An empty ``{}`` there is not a harmless placeholder, it is a logged-out + session that wins: ``dl`` forwards a profile's login as + ``CLAUDE_CODE_OAUTH_TOKEN``, Claude Code reads the credentials file first, + and the agent then asks the operator to log in while a valid token sits in + its environment. + + It stayed hidden for as long as the feature mounted the host's real + credentials file *over* the stub, so the bug was invisible in exactly the + configuration that could not use a forwarded token anyway. + + ``.claude.json`` is held to the same rule because it was seeded by the same + block for the same retired reason. + + The installer names both files only in comments today, so the guard forbids + every other mention rather than guessing at write syntax. A future line that + genuinely needs to name one is meant to be a conversation, not a match the + filter happens to let through. + """ + installer = FEATURE_INSTALLER.read_text() + for name in SEEDED_NAMES: + seeding = _seeding_lines(installer, name) + assert not seeding, f"{name} is named outside a comment in install.sh: {seeding}" + + +def test_the_installer_may_not_create_the_file_that_marks_claude_onboarded(devcontainer): + """`.claude.json` is forbidden for a second reason, and it is devlaunch's own. + + The provisioner seeds ``{"hasCompletedOnboarding":true}`` into + ``$CLAUDE_CONFIG_DIR/.claude.json`` and exits early if that file is already + there. This feature points ``CLAUDE_CONFIG_DIR`` at the very directory the + installer sets up, so the ``{}`` stub was satisfying that guard: every + container built from this feature skipped the onboarding seed, which is the + opposite of what seeding it was for. + + Two files hold that one fact, so this diffs them rather than restating it: + the name the provisioner exits on has to be a name the installer is + forbidden to write. + """ + guarded = re.findall(r'\[ -e \\"\$dir/([^\\]+)\\" \]', SHIPPING_PROVISIONER.read_text()) + assert guarded, ( + f"{SHIPPING_PROVISIONER.name} no longer guards the onboarding seed on an " + "existing file; this test diffs that name against the installer and has " + "nothing left to diff" + ) + config_dir = devcontainer["containerEnv"]["CLAUDE_CONFIG_DIR"] + assert config_dir.endswith("/.claude"), ( + f"CLAUDE_CONFIG_DIR is {config_dir}, which is no longer the directory " + "install.sh populates, so the two no longer collide and this test is moot" + ) + for name in guarded: + assert name in SEEDED_NAMES, ( + f"the provisioner skips the onboarding seed when {name} exists, but " + f"install.sh is only forbidden to create {SEEDED_NAMES}, so the feature " + f"is free to suppress it again" + ) + + def test_the_agent_socket_is_bound_from_the_variable_that_names_it(devcontainer, mounts): """The agent socket mount reads $SSH_AUTH_SOCK, not a guess at where it is.