Conversation
Reviewer's GuideAdds a local Dev Container Feature for the Claude Code CLI, including installation logic, configuration directory setup, and extensive docs/troubleshooting for running Claude inside devcontainers, plus minor devcontainer/task wiring changes. Sequence diagram for OAuth authentication flow with Claude in devcontainersequenceDiagram
actor DevUser
participant HostVSCode
participant DevContainer
participant ClaudeCLI
participant OAuthServer as OAuth_provider
participant HostBrowser
participant HostFS
DevUser ->> HostVSCode: Open_repo_and_start_devcontainer
HostVSCode ->> DevContainer: Create_container_with_runArgs_network_host
DevContainer ->> ClaudeCLI: Feature_install.sh_installs_CLI
DevUser ->> ClaudeCLI: Run_claude
ClaudeCLI ->> ClaudeCLI: Start_local_OAuth_callback_server
ClaudeCLI ->> HostBrowser: Print_OAuth_URL
DevUser ->> HostBrowser: Open_URL
HostBrowser ->> OAuthServer: Authorize_request
OAuthServer -->> HostBrowser: Redirect_to_http_localhost_random_port_callback
Note over DevContainer,HostBrowser: With_network_host,_container_port_equals_host_port
HostBrowser ->> ClaudeCLI: HTTP_callback_to_localhost_port
ClaudeCLI ->> HostFS: Write_tokens_to_~/.claude/.credentials.json
ClaudeCLI ->> HostFS: Update_~/.claude/.claude.json_state
ClaudeCLI -->> DevUser: Authenticated_interactive_session
Flow diagram for install.sh Claude Code feature installation logicflowchart TD
A["Start_install.sh"] --> B["Detect_package_manager"]
B --> C["Check_node_and_npm_present"]
C -->|"Missing"| D["install_nodejs(pkg_manager)"]
C -->|"Present"| E["Skip_Node.js_install"]
D --> F{"node_and_npm_installed"}
F -->|"No"| G["print_nodejs_requirement_and_exit"]
F -->|"Yes"| H["Proceed_to_CLI_install"]
E --> H
H --> I{"claude_command_exists"}
I -->|"Yes"| J["Print_existing_version"]
I -->|"No"| K["npm_install_-g_@anthropic-ai/claude-code"]
J --> L["create_claude_directories"]
K --> M{"claude_installed"}
M -->|"No"| N["Exit_with_error"]
M -->|"Yes"| L
L --> O["Determine_target_home_and_user(_REMOTE_USER_or_vscode)"]
O --> P["mkdir_-p_.claude,_agents,_commands,_hooks"]
P --> Q["Ensure_.credentials.json_and_.claude.json_exist_with_600_perms"]
Q --> R["chown_-R_target_user_if_running_as_root"]
R --> S["Print_summary_and_success"]
S --> T["End_install.sh"]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've found 4 issues, and left some high level feedback:
- In
install.sh, theinstall_nodejsfunction assumescurlis available for theyumanddnfpaths but only explicitly installs it forapt; consider installingcurl(and any other prerequisites likeca-certificates) before using it in those branches to avoid failures on minimal images. - When
create_claude_directoriescreates placeholder.credentials.jsonand.claude.jsonfiles with{}content, it does so silently; adding an explicit log message or warning in this code path would make it easier to distinguish between real credentials/state and auto-generated placeholders during debugging.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `install.sh`, the `install_nodejs` function assumes `curl` is available for the `yum` and `dnf` paths but only explicitly installs it for `apt`; consider installing `curl` (and any other prerequisites like `ca-certificates`) before using it in those branches to avoid failures on minimal images.
- When `create_claude_directories` creates placeholder `.credentials.json` and `.claude.json` files with `{}` content, it does so silently; adding an explicit log message or warning in this code path would make it easier to distinguish between real credentials/state and auto-generated placeholders during debugging.
## Individual Comments
### Comment 1
<location> `.devcontainer/claude-code/install.sh:23-47` </location>
<code_context>
+}
+
+# Function to install packages using the appropriate package manager
+install_packages() {
+ local pkg_manager="$1"
+ shift
+ local packages="$@"
+
+ case "$pkg_manager" in
+ apt)
+ apt-get update
+ apt-get install -y $packages
+ ;;
+ apk)
+ apk add --no-cache $packages
+ ;;
+ dnf|yum)
+ $pkg_manager install -y $packages
+ ;;
+ *)
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Use "$@" instead of collecting packages into a single unquoted string to preserve arguments correctly.
`install_packages` currently does `local packages="$@"` and later uses `$packages` unquoted, which collapses all arguments into one string and relies on word-splitting. Since you already `shift` the package manager off, you can drop the extra variable and pass `"$@"` directly to the package manager, e.g.:
```sh
a pt-get install -y "$@"
apk add --no-cache "$@"
"$pkg_manager" install -y "$@"
```
This avoids incorrect splitting if arguments contain spaces or glob characters and follows common shell practices.
```suggestion
# Function to install packages using the appropriate package manager
install_packages() {
local pkg_manager="$1"
shift
case "$pkg_manager" in
apt)
apt-get update
apt-get install -y "$@"
;;
apk)
apk add --no-cache "$@"
;;
dnf|yum)
"$pkg_manager" install -y "$@"
;;
*)
echo "WARNING: Unsupported package manager. Cannot install packages:" "$@"
return 1
;;
esac
return 0
}
```
</issue_to_address>
### Comment 2
<location> `.devcontainer/claude-code/install.sh:1-4` </location>
<code_context>
+#!/bin/sh
+set -eu
+
+# Claude Code CLI Local Feature Install Script
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Consider enabling pipefail to avoid masking errors in piped commands (e.g., curl | bash).
Because this script uses pipelines like `curl -sL https://rpm.nodesource.com/setup_18.x | bash -`, `set -eu` alone won’t fail the script if `curl` fails, which can hide errors and leave partial setup. If `/bin/sh` is actually bash (or if you switch to `#!/bin/bash`), consider adding `set -o pipefail` to ensure the script exits on failure in any part of the pipeline. Alternatively, download the script with `curl` to a file, check `curl`’s exit code, and only then execute it instead of using `curl | bash` directly.
```suggestion
#!/bin/bash
set -euo pipefail
# Claude Code CLI Local Feature Install Script
```
</issue_to_address>
### Comment 3
<location> `.devcontainer/claude-code/install.sh:99` </location>
<code_context>
+ echo "Installing Claude Code CLI globally..."
+
+ # Install with npm
+ npm install -g @anthropic-ai/claude-code
+
+ # Verify installation
</code_context>
<issue_to_address>
**suggestion:** Consider pinning the Claude Code CLI version or allowing it to be configured for reproducible devcontainer builds.
Installing `@anthropic-ai/claude-code` without a version makes devcontainer builds non-deterministic across time and team members. To keep environments reproducible and upgrades controlled, either pin a specific version (e.g. `npm install -g @anthropic-ai/claude-code@X.Y.Z`) or parameterize it via an environment variable:
```sh
CLAUDE_CODE_VERSION="${CLAUDE_CODE_VERSION:-latest}"
npm install -g "@anthropic-ai/claude-code@${CLAUDE_CODE_VERSION}"
```
Suggested implementation:
```
# Function to install Claude Code CLI
install_claude_code() {
# Allow version to be configured, defaulting to "latest" for flexibility.
# Example: export CLAUDE_CODE_VERSION="1.2.3" to pin a specific version.
CLAUDE_CODE_VERSION="${CLAUDE_CODE_VERSION:-latest}"
echo "Installing Claude Code CLI globally (version: ${CLAUDE_CODE_VERSION})..."
# Ensure Node.js and npm are available
if ! command -v node >/dev/null || ! command -v npm >/dev/null; then
echo "Node.js and npm are required to install Claude Code CLI but were not found in PATH."
return 1
fi
# Install with npm, using configurable version for reproducible builds
if npm install -g "@anthropic-ai/claude-code@${CLAUDE_CODE_VERSION}"; then
echo "Successfully installed Claude Code CLI (version: ${CLAUDE_CODE_VERSION})"
return 0
else
echo "Failed to install Claude Code CLI (version: ${CLAUDE_CODE_VERSION})"
return 1
fi
}
```
If your devcontainer uses environment files or `devcontainer.json` to set build-time environment variables, you may also want to:
1. Add `CLAUDE_CODE_VERSION` to those configurations to pin a specific version for the team.
2. Document the variable in your project README or contributing guide so developers know how to override it.
</issue_to_address>
### Comment 4
<location> `.devcontainer/claude-code/README.md:446` </location>
<code_context>
+Based on [PR #25](https://github.com/anthropics/devcontainer-features/pull/25):
+
+- **Read-Only Mounts**: Prevents prompt injection attacks that could modify CLAUDE.md or hooks
+- **No Credential Mounts**: `.credentials.json` is NOT mounted to prevent key exfiltration
+- **Isolated Configuration**: Each container uses host config but cannot modify it
+
</code_context>
<issue_to_address>
**🚨 issue (security):** Security notes contradict earlier sections about mounting `.credentials.json` and `.claude.json`.
This contradicts the earlier description (and TROUBLESHOOTING) where `.credentials.json` and `.claude.json` are described as read-write mounts that must be writable. Please align this bullet with the actual behavior—either clarify under what conditions credentials are mounted or update/remove the “No Credential Mounts” claim so the security notes accurately reflect the real mount configuration.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| # Function to install packages using the appropriate package manager | ||
| install_packages() { | ||
| local pkg_manager="$1" | ||
| shift | ||
| local packages="$@" | ||
|
|
||
| case "$pkg_manager" in | ||
| apt) | ||
| apt-get update | ||
| apt-get install -y $packages | ||
| ;; | ||
| apk) | ||
| apk add --no-cache $packages | ||
| ;; | ||
| dnf|yum) | ||
| $pkg_manager install -y $packages | ||
| ;; | ||
| *) | ||
| echo "WARNING: Unsupported package manager. Cannot install packages: $packages" | ||
| return 1 | ||
| ;; | ||
| esac | ||
|
|
||
| return 0 | ||
| } |
There was a problem hiding this comment.
suggestion (bug_risk): Use "$@" instead of collecting packages into a single unquoted string to preserve arguments correctly.
install_packages currently does local packages="$@" and later uses $packages unquoted, which collapses all arguments into one string and relies on word-splitting. Since you already shift the package manager off, you can drop the extra variable and pass "$@" directly to the package manager, e.g.:
a pt-get install -y "$@"
apk add --no-cache "$@"
"$pkg_manager" install -y "$@"This avoids incorrect splitting if arguments contain spaces or glob characters and follows common shell practices.
| # Function to install packages using the appropriate package manager | |
| install_packages() { | |
| local pkg_manager="$1" | |
| shift | |
| local packages="$@" | |
| case "$pkg_manager" in | |
| apt) | |
| apt-get update | |
| apt-get install -y $packages | |
| ;; | |
| apk) | |
| apk add --no-cache $packages | |
| ;; | |
| dnf|yum) | |
| $pkg_manager install -y $packages | |
| ;; | |
| *) | |
| echo "WARNING: Unsupported package manager. Cannot install packages: $packages" | |
| return 1 | |
| ;; | |
| esac | |
| return 0 | |
| } | |
| # Function to install packages using the appropriate package manager | |
| install_packages() { | |
| local pkg_manager="$1" | |
| shift | |
| case "$pkg_manager" in | |
| apt) | |
| apt-get update | |
| apt-get install -y "$@" | |
| ;; | |
| apk) | |
| apk add --no-cache "$@" | |
| ;; | |
| dnf|yum) | |
| "$pkg_manager" install -y "$@" | |
| ;; | |
| *) | |
| echo "WARNING: Unsupported package manager. Cannot install packages:" "$@" | |
| return 1 | |
| ;; | |
| esac | |
| return 0 | |
| } |
| echo "Installing Claude Code CLI globally..." | ||
|
|
||
| # Install with npm | ||
| npm install -g @anthropic-ai/claude-code |
There was a problem hiding this comment.
suggestion: Consider pinning the Claude Code CLI version or allowing it to be configured for reproducible devcontainer builds.
Installing @anthropic-ai/claude-code without a version makes devcontainer builds non-deterministic across time and team members. To keep environments reproducible and upgrades controlled, either pin a specific version (e.g. npm install -g @anthropic-ai/claude-code@X.Y.Z) or parameterize it via an environment variable:
CLAUDE_CODE_VERSION="${CLAUDE_CODE_VERSION:-latest}"
npm install -g "@anthropic-ai/claude-code@${CLAUDE_CODE_VERSION}"Suggested implementation:
# Function to install Claude Code CLI
install_claude_code() {
# Allow version to be configured, defaulting to "latest" for flexibility.
# Example: export CLAUDE_CODE_VERSION="1.2.3" to pin a specific version.
CLAUDE_CODE_VERSION="${CLAUDE_CODE_VERSION:-latest}"
echo "Installing Claude Code CLI globally (version: ${CLAUDE_CODE_VERSION})..."
# Ensure Node.js and npm are available
if ! command -v node >/dev/null || ! command -v npm >/dev/null; then
echo "Node.js and npm are required to install Claude Code CLI but were not found in PATH."
return 1
fi
# Install with npm, using configurable version for reproducible builds
if npm install -g "@anthropic-ai/claude-code@${CLAUDE_CODE_VERSION}"; then
echo "Successfully installed Claude Code CLI (version: ${CLAUDE_CODE_VERSION})"
return 0
else
echo "Failed to install Claude Code CLI (version: ${CLAUDE_CODE_VERSION})"
return 1
fi
}
If your devcontainer uses environment files or devcontainer.json to set build-time environment variables, you may also want to:
- Add
CLAUDE_CODE_VERSIONto those configurations to pin a specific version for the team. - Document the variable in your project README or contributing guide so developers know how to override it.
Summary by Sourcery
Add a local devcontainer feature for installing and configuring the Claude Code CLI with mounted host configuration.
New Features:
Enhancements: