Introduce Devcontainers-based Claude Code sandbox#2194
Introduce Devcontainers-based Claude Code sandbox#2194Stephan202 wants to merge 2 commits intomasterfrom
Conversation
Stephan202
added
the
chore
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
Suggested commit message: See above. |
📝 WalkthroughWalkthroughAdded a development container setup comprising a Dockerfile with Node, build tools, and SDKMAN!, a corresponding devcontainer.json configuration file, and a Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 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/Dockerfile:
- Around line 50-58: Avoid piping remote installers directly to bash: for the
Claude Code install (the RUN curl ... | bash line) and the SDKMAN! install (the
RUN curl ... | bash line), change to a two-step approach that downloads a
pinned/versioned installer to a local file, verify its checksum or signature
against a known good value, and only then execute the saved installer with bash;
update the Dockerfile to reference explicit installer versions/URLs and include
checksum verification commands before running the installer, and preserve the
existing ARG TARGET_JDK and .sdkmanrc usage while replacing the direct curl |
bash invocations.
- Around line 55-61: The current RUN step appends "java=${TARGET_JDK}" to
.sdkmanrc which creates duplicate java= entries; modify the RUN command that
currently echoes into .sdkmanrc so it replaces the existing java= line instead
(e.g., use a sed/awk replacement or remove any existing ^java= line then write
the new one) before running "sdk env install", targeting the .sdkmanrc file and
using the TARGET_JDK variable to ensure the java entry is updated rather than
appended.
In `@claude-sandbox.sh`:
- Around line 74-80: The shared lock (LOCK_FILE / LOCK_FD with flock -s) only
tracks active sessions and does not prevent two processes racing through
devcontainer up; fix by adding a separate exclusive startup lock around the
devcontainer up call: create a new lock file (e.g.,
STARTUP_LOCK="/tmp/claude-sandbox-${WORKSPACE_ID}.startup.lock"), open a new FD
(e.g., STARTUP_FD), acquire flock -x on that FD before running devcontainer up,
run devcontainer up, then release/close STARTUP_FD so other processes can
proceed; keep the original shared LOCK_FILE/LOCK_FD and flock -s for lifetime
tracking/teardown as before (apply same change to the other startup block
referenced near the second flock usage).
- Around line 24-27: Add upfront checks similar to the existing devcontainer
check to verify required host commands: git, docker, and flock; for SHA-256
support check for sha256sum and if missing check for shasum and fail with an
actionable message listing install steps. Implement a portable SHA-256 helper
function (e.g., compute_sha256) used elsewhere that calls sha256sum when
available or falls back to shasum -a 256, and update any call sites to use this
helper; keep error messages clear and reference the missing command so users
know how to install it.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: afe13e48-0836-4260-bc65-564b4e110517
📒 Files selected for processing (3)
.devcontainer/Dockerfile.devcontainer/devcontainer.jsonclaude-sandbox.sh
|
Looks good. No mutations were possible for these changes. |
| "remoteEnv": { | ||
| "GITHUB_ACTOR": "${localEnv:GITHUB_ACTOR}", | ||
| "GITHUB_TOKEN": "${localEnv:GITHUB_TOKEN}", | ||
| "MAVEN_OPTS": "-Daether.enhancedLocalRepository.split=true -Daether.enhancedLocalRepository.splitRemoteRepository=true -Daether.enhancedLocalRepository.localPrefix=${localEnv:WORKSPACE_ID}" |
There was a problem hiding this comment.
This setup works with mvn, but not with mvnd. I didn't investigate that further right now. This could in theory cause some issues i.c.w. apply-error-prone-suggestions.sh, which is the only script that uses mvnd if available.
Stephan202
force-pushed
the
sschroevers/introduce-devcontainer-sandbox
branch
from
7156d0d to
2a639f1
Compare
Stephan202
left a comment
Stephan202
left a comment
There was a problem hiding this comment.
I rebased and squashed the commits, because I want to cherry-pick this on another branch I'm working on.
| # XXX: Drop `pandoc` installation if `./generate-review-checklist.sh` is | ||
| # migrated to Java. |
There was a problem hiding this comment.
This comment makes sense only in the context of #2187; ideally we merge that PR first.
|
Looks good. No mutations were possible for these changes. |
rickie
left a comment
rickie
left a comment
There was a problem hiding this comment.
Adding what I encountered and mentioned offline. Didn't dive into it further yet.
| zip \ | ||
| && apt-get clean \ | ||
| && rm -rf /var/lib/apt/lists/* | ||
|
|
There was a problem hiding this comment.
Not sure where, but it doesn't fully work for me to run this locally.
[+] Building 0.0s (6/6) FINISHED docker:default
=> [internal] load build definition from updateUID.Dockerfile-0.85.0 0.0s
=> => transferring dockerfile: 1.52kB 0.0s
=> WARN: InvalidDefaultArgInFrom: Default value for ARG $BASE_IMAGE results in empty or invalid base image name (line 4) 0.0s
=> [internal] load metadata for docker.io/library/vsc-branchname-36c33d6251eaf99073cf5c101dd4b4ad73668df3de509ffb2ca536ec1b113c1d:latest 0.0s
=> [internal] load .dockerignore 0.0s
=> => transferring context: 2B 0.0s
=> [1/2] FROM docker.io/library/vsc-branchname-36c33d6251eaf99073cf5c101dd4b4ad73668df3de509ffb2ca536ec1b113c1d:latest 0.0s
=> CACHED [2/2] RUN eval $(sed -n "s/rick:[^:]:([^:]):([^:]):[^:]:([^:])./OLD_UID=\1;OLD_GID=\2;HOME_FOLDER=\3/p" /etc/passwd); eval $(sed -n "s/([^:]):[^:]:10 0.0s
=> exporting to image 0.0s
=> => exporting layers 0.0s
=> => writing image sha256:34977a3a11e44759cecc0d9e0252bdfd375e90b7812f5598eda1b080b1226f70 0.0s
=> => naming to docker.io/library/vsc-branchname-36c33d6251eaf99073cf5c101dd4b4ad73668df3de509ffb2ca536ec1b113c1d-uid 0.0s
1 warning found (use docker --debug to expand):
InvalidDefaultArgInFrom: Default value for ARG $BASE_IMAGE results in empty or invalid base image name (line 4)
There was a problem hiding this comment.
There were some things that didn't exist yet and adding this would fix all that:
93 +# Ensure all bind-mount source paths exist. Docker bind mounts fail if the
94 +# source does not exist on the host.
95 +mkdir -p "${HOME}/.claude" \
96 + "${HOME}/.config/ccstatusline" \
97 + "${HOME}/.config/gh" \
98 + "${HOME}/.m2/repository" \
99 + "${HOME}/.ssh"
100 +touch "${HOME}/.claude.json" \
101 + "${HOME}/.gitconfig" \
102 + "${HOME}/.ssh/known_hosts"
103 +
2a639f1 to
d56c536
Compare
|
Looks good. No mutations were possible for these changes. |
Stephan202
force-pushed
the
sschroevers/introduce-devcontainer-sandbox
branch
from
d56c536 to
77e5fec
Compare
The new `claude-sandbox.sh` script executes Claude Code with `--dangerously-skip-permissions` in a Devcontainers-managed Docker container, with limited access to the host filesystem. The container's working directory is either an explicitly specified directory, or a Git worktree (which will be created if it does not yet exist). Maven SNAPSHOTs installed by the container have a custom prefix, so that these artifacts are ignored by other containers and processes on the host system. This enables concurrent work on multiple branches. This setup is not fully secure: the host system access provides plenty of opportunity to wreak havoc, and network access is not restricted. The primary aim here is to reduce friction and facilitate concurrent development.
Stephan202
force-pushed
the
sschroevers/introduce-devcontainer-sandbox
branch
from
77e5fec to
c6c46bd
Compare
|
Looks good. No mutations were possible for these changes. |
|
3 participants
Suggested commit message: