feat(config): move .claude to home directory with named volume and auth token support#3
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughMoves Claude config from Changes
sequenceDiagram
participant Container as Dev Container (postStart)
participant Migrator as setup-migrate-claude.sh
participant SetupAuth as setup-auth.sh
participant FS as Filesystem
participant Claude as Claude Client
Container->>Migrator: run migration
Migrator->>FS: check /workspaces/.claude
alt old config present and new absent
Migrator->>FS: copy -> $HOME/.claude (no-dereference, no-overwrite)
else
Migrator->>Container: skip
end
Container->>SetupAuth: run (if SETUP_AUTH=true)
alt CLAUDE_AUTH_TOKEN set
SetupAuth->>FS: check $HOME/.claude/.credentials.json
alt not exists
SetupAuth->>FS: mkdir + write .credentials.json (600 perm)
SetupAuth->>Claude: credentials available
else exists
SetupAuth->>SetupAuth: validate perms, skip injection
end
SetupAuth->>SetupAuth: unset CLAUDE_AUTH_TOKEN
else
SetupAuth->>Container: auth skipped
end
🎯 3 (Moderate) | ⏱️ ~25 minutes
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
docs/src/content/docs/customization/configuration.md (1)
10-13:⚠️ Potential issue | 🟡 MinorUpdate the settings.json deployment path to
~/.claude.The earlier description still says
.claude/settings.json, which is inconsistent with the new home-dir location.📝 Suggested fix
-The primary configuration file lives at `.devcontainer/config/defaults/settings.json`. It is deployed to `.claude/settings.json` on every container start and controls Claude Code's runtime behavior. +The primary configuration file lives at `.devcontainer/config/defaults/settings.json`. It is deployed to `~/.claude/settings.json` on every container start and controls Claude Code's runtime behavior.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/src/content/docs/customization/configuration.md` around lines 10 - 13, Update the deployment path text in the "## settings.json" section of configuration.md: replace the mention of ".claude/settings.json" with the new home-dir location "~/.claude/settings.json" so the docs reflect the current deployment target; edit the paragraph that currently reads "It is deployed to `.claude/settings.json` on every container start" to instead state it is deployed to "`~/.claude/settings.json`" to keep the description consistent..devcontainer/devcontainer.json (1)
16-22:⚠️ Potential issue | 🟡 MinorAdd
.devcontainer/CLAUDE.mddocumentation forCLAUDE_AUTH_TOKENsetup.The new config location (
CLAUDE_CONFIG_DIR) is already documented in the environment table, but the authentication token setup (.secretsplacement, auto-credentials flow) is missing from CLAUDE.md. It appears in CHANGELOG.md but not in the developer guide where users need it.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.devcontainer/devcontainer.json around lines 16 - 22, Add a new .devcontainer/CLAUDE.md documenting how to set up CLAUDE_AUTH_TOKEN and the .secrets placement and auto-credentials flow: explain that CLAUDE_CONFIG_DIR (referenced in devcontainer.json) is the directory where users should place a .secrets file containing CLAUDE_AUTH_TOKEN, include the expected key name (CLAUDE_AUTH_TOKEN), required file permissions/ownership, example file location relative to CLAUDE_CONFIG_DIR, and the automatic credential discovery steps (how the container reads CLAUDE_CONFIG_DIR and respects CLAUDECODE/null settings) so developers can follow the exact steps to provision tokens when spinning up the devcontainer..devcontainer/README.md (1)
309-318:⚠️ Potential issue | 🟡 MinorSection title mismatch: "Three methods" now lists four, and item 4 is for Claude auth, not GitHub/NPM.
Line 309 reads "Three methods for providing GitHub/NPM credentials, in order of precedence:" but the list now has four entries, and item 4 (
CLAUDE_AUTH_TOKEN) is about authenticating to Claude.ai — an orthogonal credential type that has no precedence relationship with items 1–3.📝 Suggested fix
-Three methods for providing GitHub/NPM credentials, in order of precedence: +GitHub/NPM credentials (in order of precedence):Then move item 4 into its own subsection, e.g.:
+### Claude Code Credentials + +4. **`.secrets` file with `CLAUDE_AUTH_TOKEN`** — Long-lived Claude auth token from `claude setup-token`. Auto-creates `~/.claude/.credentials.json` on container start.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.devcontainer/README.md around lines 309 - 318, The heading "Three methods for providing GitHub/NPM credentials" is wrong because the list includes a fourth item for Claude auth; update the docs by keeping items 1–3 under a corrected heading like "Methods for providing GitHub/NPM credentials" (or "Three methods" -> "Methods") and remove item 4 (the CLAUDE_AUTH_TOKEN / claude setup-token entry) from that list, then create a separate subsection titled e.g. "Claude auth token" that describes CLAUDE_AUTH_TOKEN, claude setup-token, and the auto-creation of ~/.claude/.credentials.json so credential types are not conflated and ordering/precedence only applies to GitHub/NPM entries.
🧹 Nitpick comments (1)
.devcontainer/features/ccstatusline/README.md (1)
106-109: UseCLAUDE_CONFIG_DIR-aware paths in examples.These examples hardcode
~/.claude, which is wrong for users overridingCLAUDE_CONFIG_DIR. Consider using a shell fallback to avoid misleading diagnostics.✏️ Suggested doc tweak
-cat ~/.claude/settings.json | jq '.statusLine' +cat "${CLAUDE_CONFIG_DIR:-$HOME/.claude}/settings.json" | jq '.statusLine'-cat ~/.claude/settings.json | jq '.statusLine' +cat "${CLAUDE_CONFIG_DIR:-$HOME/.claude}/settings.json" | jq '.statusLine'Also applies to: 205-208
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.devcontainer/features/ccstatusline/README.md around lines 106 - 109, The README examples hardcode the Claude config path (`~/.claude`) and should instead respect the `CLAUDE_CONFIG_DIR` environment variable; update the example commands that reference `~/.claude` (e.g., the `cat ~/.claude/settings.json | jq '.statusLine'` example and the similar block later) to use a shell-aware fallback like `${CLAUDE_CONFIG_DIR:-$HOME/.claude}` so users who set `CLAUDE_CONFIG_DIR` will get the correct path.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.devcontainer/CHANGELOG.md:
- Around line 5-26: The Unreleased changelog entries need proper subheadings:
add a "#### Documentation" entry under the existing "### Changed" section
describing the docs updates introduced by this PR (e.g., update devcontainer
README or migration docs), and create a "#### Scripts" (or other appropriate
area like "#### Configuration") subheading under "### Removed" and move the "-
`setup-symlink-claude.sh` — no longer needed with native home directory
location" line under that subheading so the Removed section follows the same
subheading structure as other sections; update the text to explicitly mention
the one-time migration script replacement (`setup-migrate-claude.sh`) if not
already noted.
- Around line 7-16: Update .devcontainer/CLAUDE.md to document the new
authentication and config behaviors: add a short section explaining the
CLAUDE_AUTH_TOKEN environment variable and how to place it into the .secrets
file (and that it must be declared in devcontainer.json), describe that the
container startup auto-generates ~/.claude/.credentials.json from
CLAUDE_AUTH_TOKEN (but will skip generation if ~/.claude/.credentials.json
already exists), and add a note about the Docker named volume used to persist
the ~/.claude directory across rebuilds (per-instance via devcontainerId) and
the default CLAUDE_CONFIG_DIR=/home/vscode/.claude.
In @.devcontainer/features/ccstatusline/install.sh:
- Around line 193-196: The SETTINGS_FILE fallback uses $HOME which is root in
post-start hooks; change the SETTINGS_FILE assignment so when CLAUDE_CONFIG_DIR
is unset it computes the target user's home (using USERNAME/SUDO_USER) instead
of $HOME (e.g., derive home via getent or using ~"$USERNAME") and keep
USERNAME="${SUDO_USER:-vscode}" as the user reference; also update
.devcontainer/CLAUDE.md to document that features read config from
CLAUDE_CONFIG_DIR and that the fallback uses the target user's home (not root),
and add the same configuration expectation and fallback behavior to
devcontainer-feature.json metadata for the ccstatusline feature.
In @.devcontainer/features/mcp-qdrant/install.sh:
- Around line 191-193: The post-start hook sets SETTINGS_FILE using
CLAUDE_CONFIG_DIR fallback to $HOME which is wrong because hooks run as root;
update the logic in poststart-hook.sh that defines SETTINGS_FILE so it derives
the fallback home from the invoking user (use SUDO_USER to determine that user's
HOME, e.g., lookup /etc/passwd or use getent to resolve SUDO_USER) instead of
$HOME when CLAUDE_CONFIG_DIR is not set, and keep the variable name
SETTINGS_FILE and CLAUDE_CONFIG_DIR unchanged; also update the mcp-qdrant
feature docs by documenting the CLAUDE_CONFIG_DIR dependency and add the note to
.devcontainer/CLAUDE.md under the mcp-qdrant section.
In @.devcontainer/features/mcp-qdrant/poststart-hook.sh:
- Around line 59-61: The poststart-hook.sh fallback uses $HOME which can resolve
to /root when run as root; change the SETTINGS_FILE resolution so it derives the
target user's home directory (e.g., prefer CLAUDE_CONFIG_DIR if set, otherwise
compute the real user's home from SUDO_USER or passwd lookup) and then append
/.claude/settings.json; update the SETTINGS_FILE assignment in poststart-hook.sh
to use that computed home (so SETTINGS_FILE no longer defaults to $HOME/.claude)
and ensure it still checks for the file as before.
In @.devcontainer/scripts/setup-auth.sh:
- Around line 78-90: The subshell that writes the Claude creds via ( umask 077;
cat > "$CLAUDE_CRED_FILE" <<CRED_EOF ... CRED_EOF ) can fail silently but the
script still prints "[setup-auth] Claude auth token configured" and sets
AUTH_CONFIGURED=true; modify that block to check the write succeeded before
reporting success: run the heredoc write and capture its exit status (or test
that "$CLAUDE_CRED_FILE" exists and is non-empty/readable), and only on success
echo the success message and set AUTH_CONFIGURED=true; on failure log an error
(processLogger-style or echo to stderr) and exit non-zero so failures are
propagated. Ensure you reference the subshell/heredoc that writes
"$CLAUDE_CRED_FILE" and the subsequent echo and AUTH_CONFIGURED assignment when
making the change.
In @.devcontainer/scripts/setup-migrate-claude.sh:
- Around line 19-20: The script currently runs cp -rn "$OLD_DIR/." "$NEW_DIR/"
2>/dev/null || true which silences failures and then unconditionally echoes
"[setup-migrate] Migration complete..."; change this to verify the copy
succeeded by checking for a known sentinel file (or another critical file
expected in $NEW_DIR) after the cp, and only print the success message and allow
removal if that sentinel exists; if it does not exist, print a clear error
including the cp exit status or run cp without redirecting stderr, and exit
non‑zero so users are not misled. Ensure you reference the cp invocation and the
$OLD_DIR/$NEW_DIR variables and add the sentinel existence check (test -e
"$NEW_DIR/<sentinel>") before printing the migration-complete line.
---
Outside diff comments:
In @.devcontainer/devcontainer.json:
- Around line 16-22: Add a new .devcontainer/CLAUDE.md documenting how to set up
CLAUDE_AUTH_TOKEN and the .secrets placement and auto-credentials flow: explain
that CLAUDE_CONFIG_DIR (referenced in devcontainer.json) is the directory where
users should place a .secrets file containing CLAUDE_AUTH_TOKEN, include the
expected key name (CLAUDE_AUTH_TOKEN), required file permissions/ownership,
example file location relative to CLAUDE_CONFIG_DIR, and the automatic
credential discovery steps (how the container reads CLAUDE_CONFIG_DIR and
respects CLAUDECODE/null settings) so developers can follow the exact steps to
provision tokens when spinning up the devcontainer.
In @.devcontainer/README.md:
- Around line 309-318: The heading "Three methods for providing GitHub/NPM
credentials" is wrong because the list includes a fourth item for Claude auth;
update the docs by keeping items 1–3 under a corrected heading like "Methods for
providing GitHub/NPM credentials" (or "Three methods" -> "Methods") and remove
item 4 (the CLAUDE_AUTH_TOKEN / claude setup-token entry) from that list, then
create a separate subsection titled e.g. "Claude auth token" that describes
CLAUDE_AUTH_TOKEN, claude setup-token, and the auto-creation of
~/.claude/.credentials.json so credential types are not conflated and
ordering/precedence only applies to GitHub/NPM entries.
In `@docs/src/content/docs/customization/configuration.md`:
- Around line 10-13: Update the deployment path text in the "## settings.json"
section of configuration.md: replace the mention of ".claude/settings.json" with
the new home-dir location "~/.claude/settings.json" so the docs reflect the
current deployment target; edit the paragraph that currently reads "It is
deployed to `.claude/settings.json` on every container start" to instead state
it is deployed to "`~/.claude/settings.json`" to keep the description
consistent.
---
Nitpick comments:
In @.devcontainer/features/ccstatusline/README.md:
- Around line 106-109: The README examples hardcode the Claude config path
(`~/.claude`) and should instead respect the `CLAUDE_CONFIG_DIR` environment
variable; update the example commands that reference `~/.claude` (e.g., the `cat
~/.claude/settings.json | jq '.statusLine'` example and the similar block later)
to use a shell-aware fallback like `${CLAUDE_CONFIG_DIR:-$HOME/.claude}` so
users who set `CLAUDE_CONFIG_DIR` will get the correct path.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (31)
.devcontainer/.env.example.devcontainer/.secrets.example.devcontainer/CHANGELOG.md.devcontainer/CLAUDE.md.devcontainer/README.md.devcontainer/devcontainer.json.devcontainer/docs/configuration-reference.md.devcontainer/docs/keybindings.md.devcontainer/docs/troubleshooting.md.devcontainer/features/ccstatusline/README.md.devcontainer/features/ccstatusline/install.sh.devcontainer/features/claude-session-dashboard/README.md.devcontainer/features/mcp-qdrant/CHANGES.md.devcontainer/features/mcp-qdrant/install.sh.devcontainer/features/mcp-qdrant/poststart-hook.sh.devcontainer/plugins/devs-marketplace/plugins/protected-files-guard/scripts/guard-protected-bash.py.devcontainer/plugins/devs-marketplace/plugins/protected-files-guard/scripts/guard-protected.py.devcontainer/plugins/devs-marketplace/plugins/workspace-scope-guard/README.md.devcontainer/plugins/devs-marketplace/plugins/workspace-scope-guard/scripts/guard-workspace-scope.py.devcontainer/scripts/check-setup.sh.devcontainer/scripts/setup-auth.sh.devcontainer/scripts/setup-migrate-claude.sh.devcontainer/scripts/setup-symlink-claude.sh.devcontainer/scripts/setup.shdocs/src/content/docs/customization/configuration.mddocs/src/content/docs/customization/rules.mddocs/src/content/docs/customization/system-prompts.mddocs/src/content/docs/plugins/workspace-scope-guard.mddocs/src/content/docs/reference/architecture.mddocs/src/content/docs/reference/changelog.mddocs/src/content/docs/reference/environment.md
💤 Files with no reviewable changes (1)
- .devcontainer/scripts/setup-symlink-claude.sh
| cp -rn "$OLD_DIR/." "$NEW_DIR/" 2>/dev/null || true | ||
| echo "[setup-migrate] Migration complete. You can safely remove /workspaces/.claude/" |
There was a problem hiding this comment.
Unconditional "Migration complete" masks a silent copy failure.
cp -rn ... 2>/dev/null || true swallows all errors and always exits 0, so the "Migration complete / you can safely remove..." message prints even if nothing was actually copied (e.g., disk full, permission denied on residual root-owned files). A user following that advice could permanently lose credentials.
Verify the sentinel file actually landed before declaring success:
🛡️ Proposed fix
-cp -rn "$OLD_DIR/." "$NEW_DIR/" 2>/dev/null || true
-echo "[setup-migrate] Migration complete. You can safely remove /workspaces/.claude/"
+cp -rn "$OLD_DIR/." "$NEW_DIR/" 2>/dev/null || true
+if [ -f "$NEW_DIR/.credentials.json" ]; then
+ echo "[setup-migrate] Migration complete. You can safely remove /workspaces/.claude/"
+else
+ echo "[setup-migrate] WARNING: .credentials.json not found in $NEW_DIR after copy — do not remove /workspaces/.claude/ yet"
+fi📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| cp -rn "$OLD_DIR/." "$NEW_DIR/" 2>/dev/null || true | |
| echo "[setup-migrate] Migration complete. You can safely remove /workspaces/.claude/" | |
| cp -rn "$OLD_DIR/." "$NEW_DIR/" 2>/dev/null || true | |
| if [ -f "$NEW_DIR/.credentials.json" ]; then | |
| echo "[setup-migrate] Migration complete. You can safely remove /workspaces/.claude/" | |
| else | |
| echo "[setup-migrate] WARNING: .credentials.json not found in $NEW_DIR after copy — do not remove /workspaces/.claude/ yet" | |
| fi |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.devcontainer/scripts/setup-migrate-claude.sh around lines 19 - 20, The
script currently runs cp -rn "$OLD_DIR/." "$NEW_DIR/" 2>/dev/null || true which
silences failures and then unconditionally echoes "[setup-migrate] Migration
complete..."; change this to verify the copy succeeded by checking for a known
sentinel file (or another critical file expected in $NEW_DIR) after the cp, and
only print the success message and allow removal if that sentinel exists; if it
does not exist, print a clear error including the cp exit status or run cp
without redirecting stderr, and exit non‑zero so users are not misled. Ensure
you reference the cp invocation and the $OLD_DIR/$NEW_DIR variables and add the
sentinel existence check (test -e "$NEW_DIR/<sentinel>") before printing the
migration-complete line.
AnExiledDev
left a comment
There was a problem hiding this comment.
Automated Review
Status: Requires human approval
Summary
Moves .claude config from /workspaces/.claude to ~/.claude with a Docker named volume, adds CLAUDE_AUTH_TOKEN secret support for headless auth, and replaces the symlink script with a one-time migration. 31 files changed across scripts, features, plugins, guards, and documentation. A second commit hardens the implementation based on security and architecture review findings.
Issues Found and Fixed (commit 69ca626)
All items below were identified during automated review and fixed before posting:
| ID | Severity | Fix Applied |
|---|---|---|
| H1 | High | setup-auth.sh: Replaced unquoted heredoc (<<CRED_EOF) with printf '%s' — prevents shell injection via metacharacters in CLAUDE_AUTH_TOKEN |
| H2 | High | setup-migrate-claude.sh: Added idempotency check — skips silently when destination already has content (no misleading "Migration complete" on every start) |
| M1 | Medium | setup-migrate-claude.sh: Added symlink protection (-L check on old dir) and --no-dereference flag on cp to prevent TOCTOU symlink attacks |
| M2 | Medium | setup-auth.sh: Documented /proc/<pid>/environ token visibility as a platform limitation (cannot be fixed in code) |
| M3 | Medium | setup-auth.sh: Changed CLAUDE_CRED_FILE to use ${CLAUDE_CONFIG_DIR:-$HOME/.claude} pattern, consistent with all other scripts |
| M4 | Medium | setup.sh: sudo chown failure now logs a warning instead of being silently suppressed |
| L1 | Low | setup-auth.sh: Added sk-ant-* format validation on CLAUDE_AUTH_TOKEN |
| L2 | Low | setup-auth.sh: Added permission check on existing .credentials.json — warns and fixes if not 600 |
| L3 | Low | setup-migrate-claude.sh: Broadened migration trigger — migrates all content from old dir, not just when .credentials.json exists |
| L4 | Low | setup-migrate-claude.sh: Uses ${CLAUDE_CONFIG_DIR} variable for destination consistency |
Items Not Fixed (by design)
| ID | Severity | Reason |
|---|---|---|
| M5 | Medium | expiresAt: 9999999999999 — intentional design; server-side enforcement is the real expiry mechanism |
| L5 | Low | Named volume accessible to other containers on same Docker daemon — inherent Docker limitation, mitigated by ${devcontainerId} scoping |
Security Considerations
- Auth token flow: Token enters via
.secretsfile or Codespaces secret → validated (sk-ant-*) → written viaprintf '%s'withumask 077(no shell expansion, 600 perms from creation) → env var unset after use - Migration safety: Old directory checked for symlink status →
cp --no-dereferenceprevents following symlinks inside →-nprevents overwriting existing files → idempotency check prevents repeated runs - Volume ownership: Named volumes created as
root:rootby Docker →sudo chownfixes on every start with logged warning on failure - Guard coverage: Scope guard allows
$HOME/.claude/(dynamic resolution). Protected-files guard blocks.credentials.json(regex\.?credentials\.json$) - Remaining risk: Token visible in
/proc/*/environfor process lifetime (documented, platform limitation)
Project Rules Compliance
| Rule | Status | Details |
|---|---|---|
| CHANGELOG entry required | ✅ OK | [Unreleased] section with Configuration, Authentication, Security, Scripts, Removed |
| Plugin changes → plugin README | ✅ OK | workspace-scope-guard README updated |
| Feature changes → feature README | ✅ OK | ccstatusline, dashboard, mcp-qdrant READMEs updated |
| Config system → CLAUDE.md | ✅ OK | Env table and deploy path updated |
| Docs site pages | ✅ OK | 7 docs pages updated (architecture, environment, configuration, rules, system-prompts, changelog, scope-guard) |
Test Coverage
Manual testing required (no automated test suite for shell scripts):
- Container build + start succeeds
- Volume created and owned by
vscode - Auth token →
.credentials.jsonwith 600 perms - Idempotent skip when credentials exist
- Migration from old location works
- Migration skips when destination has content
- Scope guard allows
~/.claude/writes - Protected guard blocks
.credentials.jsonwrites - All aliases work with new paths
- Stale path grep returns only expected hits
Threat Model Summary
| Feature | Primary Risks | Mitigations |
|---|---|---|
| Auth token injection | Shell injection, JSON breakout | printf '%s' (no expansion), umask 077, format validation |
| Named volume | Root ownership, cross-container access | sudo chown with warning, ${devcontainerId} isolation |
| Migration | Symlink TOCTOU, repeated runs | Symlink check, --no-dereference, idempotency guard |
| Scope guard | Incorrect allowlist | Dynamic $HOME with /home/vscode fallback |
| Protected guard | Incomplete regex | \.?credentials\.json$ covers both forms |
Automated review by Claude. Human approval required before merge.
There was a problem hiding this comment.
♻️ Duplicate comments (1)
.devcontainer/scripts/setup-auth.sh (1)
86-95: Subshell write failure still goes undetected.The
( umask 077; printf ... > "$CLAUDE_CRED_FILE" )subshell on line 92 can fail (e.g., permission denied on the directory), but lines 93–94 unconditionally report success and setAUTH_CONFIGURED=true. This was previously flagged — wrapping the subshell in anifor checking$?would close this gap.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.devcontainer/scripts/setup-auth.sh around lines 86 - 95, The subshell that writes the JSON to CLAUDE_CRED_FILE using ( umask 077; printf ... > "$CLAUDE_CRED_FILE" ) can fail silently, yet AUTH_CONFIGURED=true and a success message are always printed; change the script to check the subshell's exit status (or wrap the write in an if) so that on success you echo "[setup-auth] Claude auth token configured" and set AUTH_CONFIGURED=true, and on failure you log an error (including the path in CLAUDE_CRED_FILE/CLAUDE_CRED_DIR) and do not set AUTH_CONFIGURED (optionally exit non‑zero). Ensure you reference the existing subshell/printf write and only flip AUTH_CONFIGURED when that command succeeds.
🧹 Nitpick comments (1)
.devcontainer/scripts/setup-auth.sh (1)
92-92: Token value is not JSON-escaped before interpolation.
printf '%s'prevents shell expansion but does not escape JSON-special characters (",\, control chars). If a token ever contains a literal"or\, the resulting.credentials.jsonwill be malformed. Thesk-ant-prefix check makes this very unlikely today, but a defensivesedor similar escape pass would close the gap entirely.🛡️ Minimal defensive option
+ # Escape any JSON-special characters in the token value + ESCAPED_TOKEN=$(printf '%s' "$CLAUDE_AUTH_TOKEN" | sed 's/\\/\\\\/g; s/"/\\"/g') - ( umask 077; printf '{\n "claudeAiOauth": {\n "accessToken": "%s",\n "refreshToken": "%s",\n "expiresAt": 9999999999999,\n "scopes": ["user:inference", "user:profile"]\n }\n}\n' "$CLAUDE_AUTH_TOKEN" "$CLAUDE_AUTH_TOKEN" > "$CLAUDE_CRED_FILE" ) + ( umask 077; printf '{\n "claudeAiOauth": {\n "accessToken": "%s",\n "refreshToken": "%s",\n "expiresAt": 9999999999999,\n "scopes": ["user:inference", "user:profile"]\n }\n}\n' "$ESCAPED_TOKEN" "$ESCAPED_TOKEN" > "$CLAUDE_CRED_FILE" )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.devcontainer/scripts/setup-auth.sh at line 92, The printf invocation that writes CLAUDE_AUTH_TOKEN into CLAUDE_CRED_FILE does not JSON-escape token contents (the line using printf '{\n "claudeAiOauth": ... "%s" ...' "$CLAUDE_AUTH_TOKEN"'), so tokens containing quotes/backslashes can break the JSON; change the write to first JSON-escape the token (e.g., via jq -R -s / python -c 'import json,sys;print(json.dumps(sys.stdin.read().strip()))' or similar) and then insert the escaped string into the JSON payload when creating CLAUDE_CRED_FILE, ensuring both accessToken and refreshToken use the escaped value (CLAUDE_AUTH_TOKEN).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In @.devcontainer/scripts/setup-auth.sh:
- Around line 86-95: The subshell that writes the JSON to CLAUDE_CRED_FILE using
( umask 077; printf ... > "$CLAUDE_CRED_FILE" ) can fail silently, yet
AUTH_CONFIGURED=true and a success message are always printed; change the script
to check the subshell's exit status (or wrap the write in an if) so that on
success you echo "[setup-auth] Claude auth token configured" and set
AUTH_CONFIGURED=true, and on failure you log an error (including the path in
CLAUDE_CRED_FILE/CLAUDE_CRED_DIR) and do not set AUTH_CONFIGURED (optionally
exit non‑zero). Ensure you reference the existing subshell/printf write and only
flip AUTH_CONFIGURED when that command succeeds.
---
Nitpick comments:
In @.devcontainer/scripts/setup-auth.sh:
- Line 92: The printf invocation that writes CLAUDE_AUTH_TOKEN into
CLAUDE_CRED_FILE does not JSON-escape token contents (the line using printf '{\n
"claudeAiOauth": ... "%s" ...' "$CLAUDE_AUTH_TOKEN"'), so tokens containing
quotes/backslashes can break the JSON; change the write to first JSON-escape the
token (e.g., via jq -R -s / python -c 'import
json,sys;print(json.dumps(sys.stdin.read().strip()))' or similar) and then
insert the escaped string into the JSON payload when creating CLAUDE_CRED_FILE,
ensuring both accessToken and refreshToken use the escaped value
(CLAUDE_AUTH_TOKEN).
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.devcontainer/scripts/setup-auth.sh.devcontainer/scripts/setup-migrate-claude.sh.devcontainer/scripts/setup.sh
🚧 Files skipped from review as they are similar to previous changes (2)
- .devcontainer/scripts/setup-migrate-claude.sh
- .devcontainer/scripts/setup.sh
CodeRabbit Review ResponseAddressed all actionable findings in commit d5c2fe2. Fixed (4 findings)
Not applicable (3 findings — false positives)
|
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.devcontainer/CHANGELOG.md:
- Line 10: Update the changelog entry for CLAUDE_CONFIG_DIR to use the portable
home shorthand rather than a hard-coded vscode user path: replace the string
"/home/vscode/.claude" with "~/.claude" in the CLAUDE_CONFIG_DIR changelog line
so it matches the other entry and avoids leaking a vscode-specific path.
- Line 18: Remove the parenthetical "(leading dot)" from the changelog entry so
the user-facing note reads simply "Protected-files-guard now covers
`.credentials.json`"; keep the parenthetical only if it appears within the
Security section where implementation details belong. Locate the line containing
"Protected-files-guard now covers `.credentials.json` (leading dot)" and edit it
to remove " (leading dot)" while leaving the rest of the text and punctuation
unchanged.
In @.devcontainer/scripts/setup-auth.sh:
- Around line 78-85: The branch that handles an existing credentials file (elif
[ -f "$CLAUDE_CRED_FILE" ]) does not set AUTH_CONFIGURED when a token was
supplied via CLAUDE_AUTH_TOKEN, causing false “No tokens provided” summaries;
update that branch so when CLAUDE_AUTH_TOKEN is non-empty and the script detects
the existing $CLAUDE_CRED_FILE, set AUTH_CONFIGURED=true (after verifying/fixing
permissions) so the script treats the supplied token as configured; reference
the CLAUDE_CRED_FILE, CLAUDE_AUTH_TOKEN, and AUTH_CONFIGURED symbols when making
the change.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
.devcontainer/CHANGELOG.md.devcontainer/CLAUDE.md.devcontainer/scripts/setup-auth.sh.devcontainer/scripts/setup-migrate-claude.shdocs/src/content/docs/reference/changelog.md
🚧 Files skipped from review as they are similar to previous changes (2)
- .devcontainer/CLAUDE.md
- .devcontainer/scripts/setup-migrate-claude.sh
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
.devcontainer/scripts/setup-auth.sh (1)
80-100: Both prior review findings are now resolved — LGTM on this block.
AUTH_CONFIGURED=trueis correctly set on line 88 when credentials already exist (skipping the injection path).- The credential write is now wrapped in an
if/elseblock (line 95), so failures surface a warning instead of silently reporting success.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.devcontainer/scripts/setup-auth.sh around lines 80 - 100, No change required: the block correctly sets AUTH_CONFIGURED when $CLAUDE_CRED_FILE exists and safely writes the token using umask 077 with printf into $CLAUDE_CRED_FILE guarded by an if/else so failures emit a warning; keep the current logic around CLAUDE_CRED_FILE, AUTH_CONFIGURED, CLAUDE_CRED_DIR, and the printf that consumes CLAUDE_AUTH_TOKEN as-is.
🧹 Nitpick comments (3)
.devcontainer/features/ccstatusline/README.md (1)
109-109: Verification commands conflict with the newly advertisedCLAUDE_CONFIG_DIRawareness.Line 81 correctly documents that the feature respects
CLAUDE_CONFIG_DIR, but the shell commands on lines 109, 208, and 262 still hardcode~/.claude/settings.json. A user with a customCLAUDE_CONFIG_DIRwould run these commands against the wrong path and get no output, making troubleshooting confusing.📝 Proposed fix — use the env var with a fallback in all three locations
-cat ~/.claude/settings.json | jq '.statusLine' +cat "${CLAUDE_CONFIG_DIR:-$HOME/.claude}/settings.json" | jq '.statusLine'Apply the same substitution on line 208 (troubleshooting block) and line 262 ("Then update…" guidance text).
Also applies to: 208-208, 262-262
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.devcontainer/features/ccstatusline/README.md at line 109, The verification commands hardcode "~/.claude/settings.json" and conflict with the documented CLAUDE_CONFIG_DIR support; update all occurrences of the hardcoded path (the shell commands shown as cat ~/.claude/settings.json) to reference the environment variable with a fallback, e.g. use "${CLAUDE_CONFIG_DIR:-$HOME/.claude}/settings.json" (or equivalent POSIX shell expansion) in the three places mentioned so the commands honor CLAUDE_CONFIG_DIR if set and still fall back to the default..devcontainer/scripts/setup-auth.sh (1)
91-91: Directory is created with the default umask (typically755), while the credential file is600.Any user on the container can enumerate that
.credentials.jsonexists even though they cannot read it. Given that theumask 077subshell is already scoped to the write, the directory creation can be hardened at negligible cost:🔒 Proposed hardening
- mkdir -p "$CLAUDE_CRED_DIR" + mkdir -p "$CLAUDE_CRED_DIR" + chmod 700 "$CLAUDE_CRED_DIR"Or, move
mkdirinside the umask subshell to apply restrictive permissions atomically:- mkdir -p "$CLAUDE_CRED_DIR" - if ( umask 077; printf ... > "$CLAUDE_CRED_FILE" ); then + if ( umask 077; mkdir -p "$CLAUDE_CRED_DIR" && printf ... > "$CLAUDE_CRED_FILE" ); then🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.devcontainer/scripts/setup-auth.sh at line 91, The directory is created with default permissions allowing enumeration; make it restrictive by creating the directory with mode 700 or inside the existing umask 077 subshell so permissions are set atomically: change the mkdir "$CLAUDE_CRED_DIR" call to either mkdir -m 700 "$CLAUDE_CRED_DIR" or move that mkdir into the umask 077 block that already protects the .credentials.json file (referencing CLAUDE_CRED_DIR and the .credentials.json credential file)..devcontainer/features/mcp-qdrant/install.sh (1)
191-193: Prefergetent passwdovereval echofor home-directory resolution.
eval echo "~${_USERNAME}"works butevalis a shell antipattern; ifSUDO_USERever contains shell metacharacters it becomes a command-injection vector.getent passwdis the idiomatic, injection-safe alternative:♻️ Suggested refactor
-_USER_HOME=$(eval echo "~${_USERNAME}") +_USER_HOME=$(getent passwd "${_USERNAME}" | cut -d: -f6)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.devcontainer/features/mcp-qdrant/install.sh around lines 191 - 193, The current home-resolution uses unsafe eval echo via _USERNAME and _USER_HOME; replace that with an injection-safe getent lookup: query getent passwd for the chosen _USERNAME and extract the home field (e.g., parse field 6 with cut/awk), assign that to _USER_HOME, and add a robust fallback (e.g., /home/"$_USERNAME" or /root) if getent returns empty; ensure all expansions are quoted and check for an empty result before using _USER_HOME.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.devcontainer/features/mcp-qdrant/install.sh:
- Around line 191-196: _chown call still uses id -un:id -gn which can set
settings.json to root:root when the script runs as root; change the chown
invocation that currently uses $(id -un):$(id -gn) to use the resolved _USERNAME
(and its group) instead, e.g. chown "${_USERNAME}:$(id -gn "${_USERNAME}")"
"${SETTINGS_FILE}" (or any other targets), so ownership matches the user
resolved by _USERNAME/_USER_HOME/SETTINGS_FILE.
In @.devcontainer/scripts/setup-auth.sh:
- Around line 72-74: The tilde expansion via eval in _USER_HOME is unsafe
because _USERNAME can come from SUDO_USER/USER; replace the eval approach by
looking up the user's home from the system user database (use getent passwd
"$_USERNAME" and extract the home directory field) and fall back to a safe
default like /home/"$_USERNAME" if getent returns nothing; ensure you quote
_USERNAME and the resulting path, then use that safe _USER_HOME to compute
CLAUDE_CRED_DIR (referencing variables _USERNAME, _USER_HOME, and
CLAUDE_CRED_DIR).
---
Duplicate comments:
In @.devcontainer/scripts/setup-auth.sh:
- Around line 80-100: No change required: the block correctly sets
AUTH_CONFIGURED when $CLAUDE_CRED_FILE exists and safely writes the token using
umask 077 with printf into $CLAUDE_CRED_FILE guarded by an if/else so failures
emit a warning; keep the current logic around CLAUDE_CRED_FILE, AUTH_CONFIGURED,
CLAUDE_CRED_DIR, and the printf that consumes CLAUDE_AUTH_TOKEN as-is.
---
Nitpick comments:
In @.devcontainer/features/ccstatusline/README.md:
- Line 109: The verification commands hardcode "~/.claude/settings.json" and
conflict with the documented CLAUDE_CONFIG_DIR support; update all occurrences
of the hardcoded path (the shell commands shown as cat ~/.claude/settings.json)
to reference the environment variable with a fallback, e.g. use
"${CLAUDE_CONFIG_DIR:-$HOME/.claude}/settings.json" (or equivalent POSIX shell
expansion) in the three places mentioned so the commands honor CLAUDE_CONFIG_DIR
if set and still fall back to the default.
In @.devcontainer/features/mcp-qdrant/install.sh:
- Around line 191-193: The current home-resolution uses unsafe eval echo via
_USERNAME and _USER_HOME; replace that with an injection-safe getent lookup:
query getent passwd for the chosen _USERNAME and extract the home field (e.g.,
parse field 6 with cut/awk), assign that to _USER_HOME, and add a robust
fallback (e.g., /home/"$_USERNAME" or /root) if getent returns empty; ensure all
expansions are quoted and check for an empty result before using _USER_HOME.
In @.devcontainer/scripts/setup-auth.sh:
- Line 91: The directory is created with default permissions allowing
enumeration; make it restrictive by creating the directory with mode 700 or
inside the existing umask 077 subshell so permissions are set atomically: change
the mkdir "$CLAUDE_CRED_DIR" call to either mkdir -m 700 "$CLAUDE_CRED_DIR" or
move that mkdir into the umask 077 block that already protects the
.credentials.json file (referencing CLAUDE_CRED_DIR and the .credentials.json
credential file).
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
.devcontainer/CHANGELOG.md.devcontainer/features/ccstatusline/README.md.devcontainer/features/ccstatusline/install.sh.devcontainer/features/mcp-qdrant/README.md.devcontainer/features/mcp-qdrant/install.sh.devcontainer/features/mcp-qdrant/poststart-hook.sh.devcontainer/scripts/setup-auth.sh.devcontainer/scripts/setup-migrate-claude.shdocs/src/content/docs/customization/configuration.md
✅ Files skipped from review due to trivial changes (1)
- .devcontainer/features/mcp-qdrant/README.md
🚧 Files skipped from review as they are similar to previous changes (4)
- .devcontainer/features/mcp-qdrant/poststart-hook.sh
- .devcontainer/CHANGELOG.md
- .devcontainer/features/ccstatusline/install.sh
- .devcontainer/scripts/setup-migrate-claude.sh
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (4)
.devcontainer/features/ccstatusline/install.sh (1)
195-197: LGTM —$HOME/ root-resolution concern correctly addressed.
getent passwd "$USERNAME"is the right POSIX mechanism for home-directory lookup, properly quoted to handle any metacharacters. The/home/$USERNAMEfallback on line 196 safely covers the case wheregetentis unavailable or returns nothing, and line 197 now consistently uses${_USER_HOME}instead of the bare$HOMEthat would resolve to/rootin the post-start hook context.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.devcontainer/features/ccstatusline/install.sh around lines 195 - 197, This change correctly replaces use of $HOME with a POSIX-safe lookup using getent (getent passwd "$USERNAME") stored in _USER_HOME with a fallback to /home/$USERNAME and then uses ${_USER_HOME} when building SETTINGS_FILE; keep the quoted getent call, the fallback assignment to _USER_HOME, and the SETTINGS_FILE assignment that uses ${_USER_HOME} to avoid resolving to /root in post-start hooks (no code changes needed)..devcontainer/scripts/setup-auth.sh (3)
72-74: Shell-injection vector viaevalcorrectly replaced withgetent.The prior concern is fully addressed.
getent passwdqueries the system user database without shell interpretation, closing the injection path throughSUDO_USER/USER.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.devcontainer/scripts/setup-auth.sh around lines 72 - 74, The previous shell-injection issue is resolved by replacing eval with getent; keep the current lines that set _USERNAME and _USER_HOME using getent passwd (variables _USERNAME, _USER_HOME and the getent call) as-is—no further changes required.
81-89:AUTH_CONFIGURED=truecorrectly set for the existing-credentials branch.The prior concern is addressed. The summary will no longer report "No tokens provided" when a token is supplied and credentials already exist.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.devcontainer/scripts/setup-auth.sh around lines 81 - 89, Ensure the existing-credentials branch sets AUTH_CONFIGURED=true and retains the permission check: verify that the elif branch handling "$CLAUDE_CRED_FILE" still sets AUTH_CONFIGURED=true (and does not exit early), keeps the perms check using stat and chmod, and that downstream logic relies on AUTH_CONFIGURED (so avoid removing or renaming AUTH_CONFIGURED in functions that consume it).
100-105: Credential write failure is now correctly detected and reported.The prior concern is fully addressed. Wrapping the subshell write in
if/elseensures the success message andAUTH_CONFIGURED=trueare only emitted on a genuine successful write.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.devcontainer/scripts/setup-auth.sh around lines 100 - 105, Wrap the credential write command in a conditional so the success path (echo "[setup-auth] Claude auth token configured" and setting AUTH_CONFIGURED=true) only runs if the subshell that runs umask 077; printf ... > "$CLAUDE_CRED_FILE" exits successfully; ensure the subshell uses ESCAPED_TOKEN for both tokens and redirects to CLAUDE_CRED_FILE, and keep the else branch to echo a warning referencing CLAUDE_CRED_DIR when the write fails so failures are properly detected and reported.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.devcontainer/CHANGELOG.md:
- Around line 19-21: Update the three CHANGELOG bullets that expose
implementation details (mentions of `eval` tilde expansion`/`getent passwd`,
JSON-escaping before writing to `.credentials.json`, and setting `umask` to
create credential directory with `700`/file `600`) to user-facing phrasing:
describe the effect not the mechanism — e.g. "Safer username resolution to
prevent shell-injection via environment variables", "Auth tokens are now safely
escaped when stored in .credentials.json", and "Credential storage now uses
stricter access permissions so credentials are only readable by the user" —
replace the literal references to `eval`, `getent passwd`, `umask`, `_USERNAME`,
and `$(id -un)` with those impact-focused strings.
- Around line 5-15: The changelog incorrectly places new features under "###
Changed"; move the Docker named volume bullet ("Added Docker named volume for
persistence across rebuilds (per-instance isolation via ${devcontainerId})") and
the entire "#### Authentication" section (the bullets about CLAUDE_AUTH_TOKEN,
auto-creating .credentials.json, and adding CLAUDE_AUTH_TOKEN to
devcontainer.json secrets) into a new "### Added" section, leaving the existing
"#### Configuration" note about CLAUDE_CONFIG_DIR under "### Changed" if it
truly is a change; ensure headings reflect "### Added" for the new capabilities
and update any surrounding headings so the document structure remains valid.
In @.devcontainer/scripts/setup-auth.sh:
- Line 93: Detect and handle failure of the ( umask 077; mkdir -p
"$CLAUDE_CRED_DIR" ) call: after creating the directory in the setup-auth.sh
script, check the mkdir exit status and if it failed, log a clear error
mentioning that creating "$CLAUDE_CRED_DIR" failed (include $CLAUDE_CRED_DIR in
the message) and exit non‑zero so subsequent writes (e.g., the write that uses
$CLAUDE_CRED_DIR) do not produce misleading permission diagnostics; update the
error path that currently references "$CLAUDE_CRED_DIR" (the later check around
the write) to not assume the directory exists when mkdir failed.
- Around line 79-80: The summary message is misleading because when
CLAUDE_AUTH_TOKEN fails the ^sk-ant- prefix check AUTH_CONFIGURED remains false
and the final report prints "No tokens provided"; add a new flag (e.g.,
AUTH_ATTEMPTED) and set it when CLAUDE_AUTH_TOKEN is present but invalid, or
alternatively set AUTH_CONFIGURED to a distinct state; update the summary logic
to check AUTH_ATTEMPTED (and CLAUDE_AUTH_TOKEN presence) so the final message
differentiates "token provided but invalid format" from "no tokens provided" and
reference CLAUDE_AUTH_TOKEN, AUTH_CONFIGURED and the new AUTH_ATTEMPTED flag in
the change.
---
Duplicate comments:
In @.devcontainer/features/ccstatusline/install.sh:
- Around line 195-197: This change correctly replaces use of $HOME with a
POSIX-safe lookup using getent (getent passwd "$USERNAME") stored in _USER_HOME
with a fallback to /home/$USERNAME and then uses ${_USER_HOME} when building
SETTINGS_FILE; keep the quoted getent call, the fallback assignment to
_USER_HOME, and the SETTINGS_FILE assignment that uses ${_USER_HOME} to avoid
resolving to /root in post-start hooks (no code changes needed).
In @.devcontainer/scripts/setup-auth.sh:
- Around line 72-74: The previous shell-injection issue is resolved by replacing
eval with getent; keep the current lines that set _USERNAME and _USER_HOME using
getent passwd (variables _USERNAME, _USER_HOME and the getent call) as-is—no
further changes required.
- Around line 81-89: Ensure the existing-credentials branch sets
AUTH_CONFIGURED=true and retains the permission check: verify that the elif
branch handling "$CLAUDE_CRED_FILE" still sets AUTH_CONFIGURED=true (and does
not exit early), keeps the perms check using stat and chmod, and that downstream
logic relies on AUTH_CONFIGURED (so avoid removing or renaming AUTH_CONFIGURED
in functions that consume it).
- Around line 100-105: Wrap the credential write command in a conditional so the
success path (echo "[setup-auth] Claude auth token configured" and setting
AUTH_CONFIGURED=true) only runs if the subshell that runs umask 077; printf ...
> "$CLAUDE_CRED_FILE" exits successfully; ensure the subshell uses ESCAPED_TOKEN
for both tokens and redirects to CLAUDE_CRED_FILE, and keep the else branch to
echo a warning referencing CLAUDE_CRED_DIR when the write fails so failures are
properly detected and reported.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
.devcontainer/CHANGELOG.md.devcontainer/features/ccstatusline/README.md.devcontainer/features/ccstatusline/install.sh.devcontainer/features/mcp-qdrant/install.sh.devcontainer/features/mcp-qdrant/poststart-hook.sh.devcontainer/scripts/setup-auth.sh.devcontainer/scripts/setup-migrate-claude.shdocs/src/content/docs/reference/changelog.md
🚧 Files skipped from review as they are similar to previous changes (4)
- .devcontainer/scripts/setup-migrate-claude.sh
- docs/src/content/docs/reference/changelog.md
- .devcontainer/features/ccstatusline/README.md
- .devcontainer/features/mcp-qdrant/poststart-hook.sh
| ### Changed | ||
|
|
||
| #### Configuration | ||
| - Moved `.claude` directory from `/workspaces/.claude` to `~/.claude` (home directory) | ||
| - Added Docker named volume for persistence across rebuilds (per-instance isolation via `${devcontainerId}`) | ||
| - `CLAUDE_CONFIG_DIR` now defaults to `~/.claude` | ||
|
|
||
| #### Authentication | ||
| - Added `CLAUDE_AUTH_TOKEN` support in `.secrets` for long-lived tokens from `claude setup-token` | ||
| - Auto-creates `.credentials.json` from token on container start (skips if already exists) | ||
| - Added `CLAUDE_AUTH_TOKEN` to devcontainer.json secrets declaration |
There was a problem hiding this comment.
New features belong under ### Added, not ### Changed.
The Docker named volume (line 9) and the entire #### Authentication section (lines 12–15) introduce brand-new capabilities — they are not modifications to existing behavior. Placing them under ### Changed misclassifies them.
✍️ Suggested restructure
+### Added
+
+#### Authentication
+- `CLAUDE_AUTH_TOKEN` secret: set it in `.secrets` to automatically create `~/.claude/.credentials.json` on container start (skipped if already exists)
+- `CLAUDE_AUTH_TOKEN` declared in devcontainer.json secrets block
+
+#### Configuration
+- Docker named volume for `~/.claude` — persists credentials and settings across rebuilds, isolated per dev container instance
+
### Changed
#### Configuration
- Moved `.claude` directory from `/workspaces/.claude` to `~/.claude` (home directory)
-- Added Docker named volume for persistence across rebuilds (per-instance isolation via `${devcontainerId}`)
- `CLAUDE_CONFIG_DIR` now defaults to `~/.claude`
-
-#### Authentication
-- Added `CLAUDE_AUTH_TOKEN` support in `.secrets` for long-lived tokens from `claude setup-token`
-- Auto-creates `.credentials.json` from token on container start (skips if already exists)
-- Added `CLAUDE_AUTH_TOKEN` to devcontainer.json secrets declarationAs per coding guidelines: "Group changelog entries under the appropriate ### Added, ### Changed, ### Fixed, or ### Removed heading."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.devcontainer/CHANGELOG.md around lines 5 - 15, The changelog incorrectly
places new features under "### Changed"; move the Docker named volume bullet
("Added Docker named volume for persistence across rebuilds (per-instance
isolation via ${devcontainerId})") and the entire "#### Authentication" section
(the bullets about CLAUDE_AUTH_TOKEN, auto-creating .credentials.json, and
adding CLAUDE_AUTH_TOKEN to devcontainer.json secrets) into a new "### Added"
section, leaving the existing "#### Configuration" note about CLAUDE_CONFIG_DIR
under "### Changed" if it truly is a change; ensure headings reflect "### Added"
for the new capabilities and update any surrounding headings so the document
structure remains valid.
| - Replaced `eval` tilde expansion with `getent passwd` lookup across all scripts (prevents shell injection via `SUDO_USER`/`USER`) | ||
| - Auth token value is now JSON-escaped before writing to `.credentials.json` | ||
| - Credential directory created with restrictive umask (700) matching credential file permissions (600) |
There was a problem hiding this comment.
Rephrase Security/Scripts entries to focus on user-facing impact, not implementation mechanics.
Lines 19, 21, and 26 expose internal implementation details (eval, getent passwd, umask, _USERNAME, $(id -un)) rather than describing what changed for the user.
✍️ Suggested rewording
-Replaced `eval` tilde expansion with `getent passwd` lookup across all scripts (prevents shell injection via `SUDO_USER`/`USER`)
+Home directory is now resolved securely in all scripts, preventing potential shell injection when running under `sudo`
-Credential directory created with restrictive umask (700) matching credential file permissions (600)
+Credential directory is created with permissions restricted to the container user only
-`chown` in mcp-qdrant poststart hooks now uses resolved `_USERNAME` instead of hardcoded `vscode` or `$(id -un)`
+mcp-qdrant poststart hooks now correctly assign file ownership for non-default container usersAs per coding guidelines: "Write CHANGELOG entries from the user's perspective — what changed, not how it was implemented."
Also applies to: 26-26
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.devcontainer/CHANGELOG.md around lines 19 - 21, Update the three CHANGELOG
bullets that expose implementation details (mentions of `eval` tilde
expansion`/`getent passwd`, JSON-escaping before writing to `.credentials.json`,
and setting `umask` to create credential directory with `700`/file `600`) to
user-facing phrasing: describe the effect not the mechanism — e.g. "Safer
username resolution to prevent shell-injection via environment variables", "Auth
tokens are now safely escaped when stored in .credentials.json", and "Credential
storage now uses stricter access permissions so credentials are only readable by
the user" — replace the literal references to `eval`, `getent passwd`, `umask`,
`_USERNAME`, and `$(id -un)` with those impact-focused strings.
| if [[ ! "$CLAUDE_AUTH_TOKEN" =~ ^sk-ant- ]]; then | ||
| echo "[setup-auth] WARNING: CLAUDE_AUTH_TOKEN doesn't match expected format (sk-ant-*), skipping" |
There was a problem hiding this comment.
Summary misleadingly reports "No tokens provided" when an invalid-format token was supplied.
When the token fails the sk-ant- prefix check, AUTH_CONFIGURED stays false and the final summary prints "No tokens provided — auth configuration skipped", obscuring that a token was detected but rejected.
🛡️ Proposed fix
if [[ ! "$CLAUDE_AUTH_TOKEN" =~ ^sk-ant- ]]; then
echo "[setup-auth] WARNING: CLAUDE_AUTH_TOKEN doesn't match expected format (sk-ant-*), skipping"
+ AUTH_CONFIGURED=true # token was present; don't misreport as "no tokens"Alternatively, introduce a separate AUTH_ATTEMPTED flag to distinguish "token present but rejected" from "no token at all" in the summary.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if [[ ! "$CLAUDE_AUTH_TOKEN" =~ ^sk-ant- ]]; then | |
| echo "[setup-auth] WARNING: CLAUDE_AUTH_TOKEN doesn't match expected format (sk-ant-*), skipping" | |
| if [[ ! "$CLAUDE_AUTH_TOKEN" =~ ^sk-ant- ]]; then | |
| echo "[setup-auth] WARNING: CLAUDE_AUTH_TOKEN doesn't match expected format (sk-ant-*), skipping" | |
| AUTH_CONFIGURED=true # token was present; don't misreport as "no tokens" |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.devcontainer/scripts/setup-auth.sh around lines 79 - 80, The summary
message is misleading because when CLAUDE_AUTH_TOKEN fails the ^sk-ant- prefix
check AUTH_CONFIGURED remains false and the final report prints "No tokens
provided"; add a new flag (e.g., AUTH_ATTEMPTED) and set it when
CLAUDE_AUTH_TOKEN is present but invalid, or alternatively set AUTH_CONFIGURED
to a distinct state; update the summary logic to check AUTH_ATTEMPTED (and
CLAUDE_AUTH_TOKEN presence) so the final message differentiates "token provided
but invalid format" from "no tokens provided" and reference CLAUDE_AUTH_TOKEN,
AUTH_CONFIGURED and the new AUTH_ATTEMPTED flag in the change.
| else | ||
| echo "[setup-auth] Creating .credentials.json from CLAUDE_AUTH_TOKEN..." | ||
| # Create directory with restrictive permissions (matches credential file at 600) | ||
| ( umask 077; mkdir -p "$CLAUDE_CRED_DIR" ) |
There was a problem hiding this comment.
mkdir -p failure is not detected; error message at line 104 becomes misleading.
If mkdir -p "$CLAUDE_CRED_DIR" fails (e.g., parent owned by root), the subsequent write at line 100 also fails, and the diagnostic "check permissions on $CLAUDE_CRED_DIR" fires — but the directory doesn't exist, making it harder to diagnose the actual problem.
🛡️ Proposed fix
- ( umask 077; mkdir -p "$CLAUDE_CRED_DIR" )
+ if ! ( umask 077; mkdir -p "$CLAUDE_CRED_DIR" ); then
+ echo "[setup-auth] WARNING: Failed to create $CLAUDE_CRED_DIR — check parent directory permissions"
+ fi📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ( umask 077; mkdir -p "$CLAUDE_CRED_DIR" ) | |
| if ! ( umask 077; mkdir -p "$CLAUDE_CRED_DIR" ); then | |
| echo "[setup-auth] WARNING: Failed to create $CLAUDE_CRED_DIR — check parent directory permissions" | |
| fi |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.devcontainer/scripts/setup-auth.sh at line 93, Detect and handle failure of
the ( umask 077; mkdir -p "$CLAUDE_CRED_DIR" ) call: after creating the
directory in the setup-auth.sh script, check the mkdir exit status and if it
failed, log a clear error mentioning that creating "$CLAUDE_CRED_DIR" failed
(include $CLAUDE_CRED_DIR in the message) and exit non‑zero so subsequent writes
(e.g., the write that uses $CLAUDE_CRED_DIR) do not produce misleading
permission diagnostics; update the error path that currently references
"$CLAUDE_CRED_DIR" (the later check around the write) to not assume the
directory exists when mkdir failed.
Bug: Stale
|
| Script | Effect |
|---|---|
setup-migrate-claude.sh |
NEW_DIR=/workspaces/.claude — migrates from itself to itself (no-op) |
setup-config.sh |
Deploys settings.json, rules, prompts to /workspaces/.claude instead of ~/.claude |
setup-auth.sh |
Writes .credentials.json to /workspaces/.claude/ — Claude can't find it |
setup-aliases.sh |
Exports wrong CLAUDE_CONFIG_DIR to shell rc files |
40-ccstatusline.sh |
Writes statusLine config to /workspaces/.claude/settings.json |
Result: Docker volume at ~/.claude stays empty. No config, no credentials, no rules.
Fix
Add a deprecation guard in setup.sh after sourcing .env, before the defaults block:
# After: source "$ENV_FILE"
# Before: : "${CLAUDE_CONFIG_DIR:=$HOME/.claude}"
if [ "$CLAUDE_CONFIG_DIR" = "/workspaces/.claude" ]; then
echo "[setup] WARNING: CLAUDE_CONFIG_DIR=/workspaces/.claude is deprecated (moved to home dir in v2.0)"
echo "[setup] Using $HOME/.claude instead. Update .devcontainer/.env to remove CLAUDE_CONFIG_DIR."
CLAUDE_CONFIG_DIR="$HOME/.claude"
fiSingle fix — all child scripts inherit the corrected value.
Bug #2: Migration script — aggressive idempotency check + no .bak renameProblemTwo issues in
FixReplace the "has content" check with a marker-file check, use +MARKER="$NEW_DIR/.migrated-from-workspaces"
+
...
-# Idempotency: skip if destination already has content (migration already done)
-if [ -d "$NEW_DIR" ] && [ -n "$(ls -A "$NEW_DIR" 2>/dev/null)" ]; then
+# Idempotency: skip if migration already completed
+if [ -f "$MARKER" ]; then
exit 0
fi
...
-if cp -rn --no-dereference "$OLD_DIR/." "$NEW_DIR/" 2>/dev/null; then
- echo "[setup-migrate] Migration complete. You can safely remove /workspaces/.claude/"
+if cp -r --no-dereference "$OLD_DIR/." "$NEW_DIR/" 2>/dev/null; then
+ date -Iseconds > "$MARKER"
+ if mv "$OLD_DIR" "${OLD_DIR}.bak" 2>/dev/null; then
+ echo "[setup-migrate] Migration complete. Old directory moved to ${OLD_DIR}.bak"
+ else
+ echo "[setup-migrate] Migration complete. Could not rename old directory — remove /workspaces/.claude/ manually"
+ fiRationale
|
Bug #3: Migration copy fails silently —
|
Integration testing findings — two fixes neededDuring integration testing on 1. Onboarding bypass:
|
- Change CLAUDE_CONFIG_DIR from /workspaces/.claude to ~/.claude
- Add Docker named volume (per-instance via ${devcontainerId}) for persistence
- Add CLAUDE_AUTH_TOKEN secret support with auto .credentials.json creation
- Replace setup-symlink-claude.sh with setup-migrate-claude.sh (one-time migration)
- Fix named volume root ownership via sudo chown on startup
- Update scope guard allowlist to resolve $HOME dynamically
- Update protected-files guard regex to cover .credentials.json
- Update all feature heredocs, poststart hooks, and docs
Security and robustness fixes from PR review:
- setup-auth.sh: Replace unquoted heredoc with printf '%s' to prevent
shell injection via CLAUDE_AUTH_TOKEN metacharacters (H1)
- setup-auth.sh: Add sk-ant-* format validation on token (L1)
- setup-auth.sh: Check/fix 600 permissions on existing .credentials.json (L2)
- setup-auth.sh: Use ${CLAUDE_CONFIG_DIR:-$HOME/.claude} pattern
consistently with other scripts (M3)
- setup-auth.sh: Document /proc token visibility limitation (M2)
- setup-migrate-claude.sh: Add idempotency check — skip silently when
destination already has content (H2)
- setup-migrate-claude.sh: Broaden trigger — migrate all content, not
just when .credentials.json exists (L3)
- setup-migrate-claude.sh: Add symlink protection on old directory (M1)
- setup-migrate-claude.sh: Use --no-dereference with cp (M1)
- setup-migrate-claude.sh: Use ${CLAUDE_CONFIG_DIR} pattern (L4)
- setup.sh: Log warning on sudo chown failure instead of silent
suppression (M4)
- CHANGELOG.md: Add #### Documentation subsection under Changed, add #### Scripts subsection under Removed for consistent structure - CLAUDE.md: Document CLAUDE_AUTH_TOKEN, .credentials.json auto-creation, skip-if-exists behavior, sk-ant-* validation, and named volume persistence - setup-auth.sh: Detect printf subshell write failure — report warning instead of false success when .credentials.json write fails - setup-migrate-claude.sh: Verify cp exit status before printing success — warn if copy failed instead of unconditional "Migration complete" - docs/reference/changelog.md: Mirror CHANGELOG structure fixes Findings 3-5 (feature $HOME fallback) confirmed as false positives: postStartCommand runs as vscode user, CLAUDE_CONFIG_DIR is exported by setup.sh before hooks execute.
- Fix hardcoded /home/vscode/.claude in changelog, use portable ~/.claude - Remove implementation detail "(leading dot)" from changelog entry - Set AUTH_CONFIGURED=true when credentials already exist (fixes false "No tokens provided" summary) - Update docs site settings.json deployment path to ~/.claude - Harden $HOME fallback across all scripts: resolve target user's home via SUDO_USER/USER/vscode chain instead of relying on $HOME (guards against root context in feature installs and hooks) - Add CLAUDE_CONFIG_DIR documentation to ccstatusline and mcp-qdrant feature READMEs - Fix stale .claude/settings.json references in ccstatusline README
- Replace eval tilde expansion with getent passwd lookup in all PR-scoped scripts (setup-auth, setup-migrate, ccstatusline, mcp-qdrant install + poststart hook) to prevent shell injection via SUDO_USER/USER environment variables - JSON-escape auth token value before writing .credentials.json - Create credential directory with umask 077 (was default 755) - Fix mcp-qdrant chown to use resolved _USERNAME instead of hardcoded vscode or $(id -un) - Update ccstatusline README verification commands to respect CLAUDE_CONFIG_DIR environment variable - Sync docs site changelog with devcontainer CHANGELOG fixes (~/. claude path, remove "(leading dot)" aside)
Migration script: - Switch from cp -rn to cp -a (archive mode) for faithful copy - Marker-based idempotency instead of checking destination contents - Verify critical files (.claude.json, plugins/, .credentials.json) - Fix ownership after copy (source may have different uid) - Rename old directory to .bak on success Setup.sh: - Detect stale CLAUDE_CONFIG_DIR=/workspaces/.claude in .env - Override to $HOME/.claude with warning - Auto-comment the stale line on disk
9db7370 to
13f9354
Compare
Summary
.claudeconfig directory from/workspaces/.claudeto~/.claude(home directory) with a Docker named volume for persistenceCLAUDE_AUTH_TOKENsecret support — auto-creates.credentials.jsonfromclaude setup-tokenlong-lived tokenssetup-symlink-claude.shwithsetup-migrate-claude.shfor one-time migration from old locationChanges
codeforge-claude-config-${devcontainerId}), updatedCLAUDE_CONFIG_DIRto/home/vscode/.claude, addedCLAUDE_AUTH_TOKENto secrets blocksudo chownfor volume ownership fix, updatedCLAUDE_CONFIG_DIRdefault to$HOME/.claude, swapped symlink → migrate script.credentials.jsonwithumask 077subshell, idempotent skip, token unset after usecp -rnmigration from/workspaces/.claude/if.credentials.jsonpresent$HOME/.claude$HOMEdynamically instead of hardcoded/workspaces/.claude\.?credentials\.json$(covers.credentials.json)SETTINGS_FILEpaths updated to${CLAUDE_CONFIG_DIR:-$HOME/.claude}/settings.jsonCLAUDE_CONFIG_DIRcommented out with$HOME/.claudedefaultCLAUDE_AUTH_TOKENentry/workspaces/.claudereferences replaced, CHANGELOG [Unreleased] section added, auth docs added to README, Codespaces troubleshooting note addedTesting
devcontainer upsucceeds — container builds and startsdocker volume ls | grep codeforge-claude-configshows per-instance volumels -la $HOME/.claudeshowsvscode:vscodeownershipcheck-setuppasses — all config files at~/.claude/CLAUDE_AUTH_TOKENin.secrets→ verify~/.claude/.credentials.jsoncreated with 600 perms.credentials.json→ token injection skipped (idempotent).credentials.jsonat/workspaces/.claude/→ rebuild → verify migrated to~/.claude/~/.claude/settings.jsonpersists via volumecc/claude/ccwaliases work with new path~/.claude/from any project dir~/.claude/.credentials.jsongrep -r '/workspaces/\.claude' .devcontainer/ --include='*.sh' --include='*.py' --include='*.json'returns only migration scriptPR created by Claude. Awaiting human review.
Summary by CodeRabbit
New Features
Changed
Security
Documentation