fix: an empty seeded .credentials.json is a logged-out session that wins - #583
Open
JSmithRobotics wants to merge 1 commit into
Open
fix: an empty seeded .credentials.json is a logged-out session that wins#583JSmithRobotics wants to merge 1 commit into
JSmithRobotics wants to merge 1 commit into
Conversation
`dl --claude-profile bear <ws>` forwarded a valid token and `claude` asked the
operator to log in anyway.
`install.sh` seeded `{}` into `.credentials.json` when the file was missing.
Claude Code reads the credentials file before the environment, so an empty one
is not a placeholder: it is a logged-out session, and it beats the
`CLAUDE_CODE_OAUTH_TOKEN` that `dl` had just gone to the trouble of resolving
from the profile and forwarding over ssh.
Both halves of why this went unnoticed:
The stub existed to give a bind mount a source to cover, from the layout where
this feature mounted nine individual paths under `~/.claude`. The host-side
hook already retired its half of that on exactly this 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) -- and the container half was left
behind.
And while this feature mounts the host's real credentials file *over* the
stub, the stub is invisible. So the bug could only appear where the forwarded
token was the container's only login, which is the one configuration the mount
rules out: `forwarded_claude` refuses to forward at all into a `Foreign`
config. It surfaced on a container whose Claude mounts had been removed, which
is what a workspace wanting a profile has to be.
Measured both ways in one container, changing nothing else: with the stub,
`claude` prompts for a login; with it removed, `claude -p` answers on the
forwarded token.
`.claude.json` goes with it, seeded by the same block for the same retired
reason. Claude Code creates both itself on first use, so there is nothing to
replace either with.
`test_the_feature_seeds_no_empty_credential` asserts it over the whole
installer rather than about one line, so it cannot return under another name.
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Sorry @JSmithRobotics, you've used your own review budget of 250,000 diff characters for the last 7 days.
You can request another review in 6 days and 19 hours by commenting @sourcery-ai review. Upgrade to get a review now.
Reviewer's GuideRemoves the installer’s empty Claude config-file stubs, which could override a valid forwarded Sequence diagram for forwarded Claude authentication without seeded credentialssequenceDiagram
participant DL
participant Container
participant Claude
DL->>Container: Set CLAUDE_CODE_OAUTH_TOKEN
Container->>Claude: Start claude
Claude->>Claude: Read credentials configuration
alt Empty .credentials.json exists
Claude-->>Container: Prompt for login
else No seeded credentials file
Claude-->>Container: Authenticate with CLAUDE_CODE_OAUTH_TOKEN
end
Flow diagram for removing empty Claude config stubsflowchart LR
Installer[create_claude_directories]
Files[.credentials.json and .claude.json]
Test[test_the_feature_seeds_no_empty_credential]
Claude[Claude Code]
Installer -->|creates directories only| Claude
Installer -.->|does not seed| Files
Test -->|asserts both files are absent| Installer
Claude -->|creates files on first use| Files
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
dl --claude-profile bear <ws>resolved the profile, forwarded the token, andclaudeasked to log in anyway.The bug
.devcontainer/claude-code/install.shseeded{}into.credentials.jsonwhen the file was missing. Claude Code reads the credentials file before the environment, so an empty one is not a placeholder: it is a logged-out session, and it beats theCLAUDE_CODE_OAUTH_TOKENthatforwarded_claudehad just resolved from the profile and sent over ssh.Measured both ways in one container, changing nothing else:
The token was verified to arrive on both transports before suspecting the file, by fingerprint rather than by printing it:
(default),bearandbaseforward three differentsha256[0:10]values, overdevpod sshand over the OpenSSH hop alike.Why it went unnoticed, which is the interesting half
Two independent reasons, and both of them are about the mounts.
The stub is invisible while the feature mounts the host's credentials file over it. So the bug could only ever appear where the forwarded token is the container's only login, and that is the one configuration this feature's mounts rule out:
forwarded_claudedeclines to forward at all into aClaudeConfig::Foreignconfig, which is what any mount at, under or above the config directory produces. It surfaced on a container whose Claude mounts had been removed, which is what a workspace that wants to use a profile has to be.The reasoning was already written down, on the other side of the same feature.
init-host.shretired its half of this seeding and said why:The container half was left behind. The stub existed for the same reason the host one did: to give a bind mount a source to cover, from the layout where this feature mounted nine individual paths under
~/.claude.The change
Both seeded files go.
.claude.jsonwas written by the same block for the same retired reason, and Claude Code creates both itself on first use, so there is nothing to replace either with.test_the_feature_seeds_no_empty_credentialasserts it over the whole installer rather than about one line, so the stub cannot return under another name. It fails against the old installer, verified by reverting.devcontainer/with the test in place.Cost
This is a change under
.devcontainer/, so the prebuild tag moves and launches build locally untildevcontainer-prebuild.ymlrepublishes onmain. Same caveat as #582, which is open and green.Not in this PR
The mounts themselves. Removing them is what let the forwarded token be used at all, and it is not a fix worth shipping as written: a container with no
~/.claudemount loses the sharedagents/,commands/,hooks/,skills/andwf-skills/, which is the point of the feature. Worth noting that removing only the whole-directory mount would not help either, since a mount under the config directory convicts exactly as the directory mount does.The two shapes that keep both, for a separate discussion:
~/.clauderead-only at a neutral path and copy the instruction directories in frompostCreateCommand. Nothing is mounted at or under the config directory, so it readsOurs. Costs live propagation, which the directory mount was chosen to preserve..credentials.json. A read-onlyskills/mount does not own the login. Costs care: the descendants scan exists because this feature once mounted.credentials.jsonindividually.Happy to open that as an issue instead if you would rather keep it out of a PR thread.
Summary by Sourcery
Remove seeded Claude configuration stubs so forwarded profiles can authenticate correctly in containers without mounted credentials.
Bug Fixes:
Tests: