feat: Claude --chrome support, build stage refactor, bridge tmpdir fix#227
feat: Claude --chrome support, build stage refactor, bridge tmpdir fix#227
Conversation
- normalize embedded tags from env and config for image resolution - split persistent containers by resolved image and preserve installer fallback tags
- Extract agent-base stage for stable core image layer - Add shared install-agent-tooling.sh for main and rust images - Mount Chrome profile Extensions/ dirs for extension detection - Setup MCP browser bridge symlink in container entrypoint - Add CI smoke tests for Chrome mount and bridge setup - Update custom-images and troubleshooting docs
cf16070 to
4047ddd
Compare
|
Claude finished @lroolle's task in 2m 46s —— View job Security Audit of PR #227 - CRITICAL VULNERABILITIES FOUNDStatus: ⛔ DO NOT DEPLOY TO PRODUCTION - Multiple container escape vectors identified 🚨 BLOCKING SECURITY ISSUES1. Directory Traversal via Chrome Profile Name (deva.sh:449-450, 469) profile_name="$(basename "$profile_path")" # Insufficient validation
profile_target="/home/deva/.config/google-chrome/$profile_name/Extensions"
2. Host User Injection (deva.sh:552-555) host_user="$(configured_env_value DEVA_CHROME_HOST_USER || true)"
# Used in: bridge_dir="$host_tmpdir/claude-mcp-browser-bridge-$host_user"
3. Bridge Directory Race Condition (docker-entrypoint.sh:279-282) rm -f "$container_socket_path" 2>/dev/null || true
ln -snf "$host_bridge_dir" "$container_socket_path"
4. Supply Chain Attack Vector (install-agent-tooling.sh:25-26) curl -fsSL "https://raw.githubusercontent.com/lroolle/atlas-cli/${ATLAS_CLI_VERSION}/install.sh" | bash
🔥 CONTAINER ESCAPE PATHCritical combination enabling full host compromise:
Attack scenario: export DEVA_HOST_CHROME_BRIDGE_DIR='/etc'
export DEVA_CHROME_PROFILE_PATH='/host/secrets'
deva.sh claude -- --chrome
# Inside container: sudo mount --bind /deva-host-chrome-bridge/shadow /etc/shadowResult: Full host privilege escalation 🛡️ IMMEDIATE FIXES REQUIRED1. Add strict path validation: validate_chrome_profile_name() {
case "$1" in
Default|Profile\ [0-9]*) return 0 ;;
*) echo "Invalid profile name: $1" >&2; return 1 ;;
esac
}2. Atomic symlink creation: temp_link="$container_socket_path.tmp.$$"
ln -snf "$host_bridge_dir" "$temp_link"
mv "$temp_link" "$container_socket_path"3. Supply chain integrity: expected_sha256="abc123..."
curl -fsSL "https://raw.githubusercontent.com/lroolle/atlas-cli/${ATLAS_CLI_VERSION}/install.sh" | \
tee >(sha256sum -c <(echo "$expected_sha256 -")) | bash4. Remove passwordless sudo or isolate Chrome functionality in separate unprivileged container 5. Implement mount allowlist validation: validate_mount_path() {
local normalized="$1"
case "$normalized" in
/home/*/.*|/tmp/*|/var/tmp/*) return 0 ;;
*) echo "Mount path not allowed: $normalized" >&2; return 1 ;;
esac
}🧪 MISSING SECURITY TESTSCritical test gaps:
🔍 ADDITIONAL FINDINGS6. Unsafe basename usage (deva.sh:500):
|
There was a problem hiding this comment.
Pull request overview
Adds Claude --chrome host integration and refactors the Docker build into reusable stages so downstream images can avoid rebuilding volatile agent layers, while also tightening Docker image ref/tag handling in the launcher and installer.
Changes:
- Add Claude
--chromesupport by mounting host ChromeExtensions/trees plus a host “bridge” dir and creating the expected tmpdir symlink inside the container. - Refactor Dockerfiles into a stable
agent-basestage + final stage, extracting shared npm/atlas installs intoscripts/install-agent-tooling.sh. - Improve Docker image ref handling (tag/digest, embedded tags) and add build targets + CI smoke tests for the new behavior.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| scripts/install-agent-tooling.sh | New shared script to install agent CLIs + atlas-cli in a single place for multiple images. |
| install.sh | Refactors installer image reference resolution (tag/digest handling, fallback behavior). |
| docs/troubleshooting.md | Adds troubleshooting guidance for Claude --chrome integration and bridge behavior. |
| docs/custom-images.md | Updates recommended build workflow to use new Makefile targets and staged builds. |
| docker-entrypoint.sh | Adds tmpdir detection + bridge symlink creation in container startup for --chrome. |
| deva.sh | Adds chrome mount/bridge assembly, image ref normalization, and container shape hashing including image. |
| Makefile | Adds build-core/build-rust-image targets and updates default tool version fallbacks. |
| Dockerfile | Splits into agent-base and final, moving volatile agent installs into the final stage. |
| Dockerfile.rust | Uses shared tooling install script and new build-arg defaults/labels for agent tools. |
| .github/workflows/ci.yml | Adds CI smoke tests and Makefile-based build steps for core/rust + chrome integration. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| mkdir -p "$container_tmpdir" | ||
|
|
||
| local container_socket_path="$container_tmpdir/claude-mcp-browser-bridge-$DEVA_USER" | ||
| rm -f "$container_socket_path" 2>/dev/null || true |
There was a problem hiding this comment.
rm -f "$container_socket_path" will not remove an existing directory. If Claude (or a previous run) created $container_tmpdir/claude-mcp-browser-bridge-$DEVA_USER as a directory, the subsequent ln -snf will create the symlink inside that directory instead of replacing it, and Chrome bridge discovery will fail. Consider removing/replacing the path robustly (e.g., handle the directory case explicitly, or use a removal that covers directories) before creating the symlink.
| rm -f "$container_socket_path" 2>/dev/null || true | |
| rm -rf "$container_socket_path" 2>/dev/null || true |
| .PHONY: build-core | ||
| build-core: | ||
| @echo "🔨 Building stable core image..." | ||
| docker build -f $(DOCKERFILE) --target agent-base --build-arg COPILOT_API_VERSION=$(COPILOT_API_VERSION) -t $(CORE_IMAGE) . | ||
| @echo "✅ Core build completed: $(CORE_IMAGE)" |
There was a problem hiding this comment.
Makefile passes COPILOT_API_VERSION as a build arg (including in build-core), but the Dockerfile checks out the Copilot API repo via COPILOT_API_COMMIT (which defaults to master). As a result, builds are not actually pinned to the SHA in COPILOT_API_VERSION, and the label can misrepresent what was built. Consider passing COPILOT_API_COMMIT=$(COPILOT_API_VERSION) (or renaming/aligning the args) so the checked-out commit matches the version you record.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cf1607044f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| ARG CLAUDE_CODE_VERSION=2.1.81 | ||
| ARG CODEX_VERSION=0.116.0 | ||
| ARG GEMINI_CLI_VERSION=0.35.0 | ||
| ARG ATLAS_CLI_VERSION=v0.1.4 |
There was a problem hiding this comment.
Thread rust CLI version args through Dockerfile.rust builds
This stage now reinstalls Claude/Codex/Gemini from hardcoded defaults, so any build path that does not pass explicit CLAUDE_CODE_VERSION/CODEX_VERSION/GEMINI_CLI_VERSION will silently diverge from the resolved release versions. I checked .github/workflows/release.yml: the build-and-push-rust job only passes BASE_IMAGE, so rust releases can lag or downgrade tool versions relative to the base image/job metadata. Please wire the resolved version args into every rust build path (or stop reinstalling these tools in the rust stage).
Useful? React with 👍 / 👎.
| rm -f "$container_socket_path" 2>/dev/null || true | ||
| ln -snf "$host_bridge_dir" "$container_socket_path" |
There was a problem hiding this comment.
Replace existing bridge directory before creating symlink
Using rm -f here does not remove directories, so if /tmp/claude-mcp-browser-bridge-<user> already exists as a directory, ln -snf will place a link inside that directory instead of replacing the path Claude probes. In that state, --chrome still fails to find the mounted host bridge. Use a directory-safe replacement (rm -rf or ln -sfnT) so the path is guaranteed to become the symlink target.
Useful? React with 👍 / 👎.
Summary
--chromeintegration: mount host Chrome profile Extensions and bridge socket into containeragent-base+finalstages; extract sharedinstall-agent-tooling.shos.tmpdir()order (macOS/var/folders/vs/tmp)DEVA_CHROME_HOST_SOCKET_PATH,DEVA_CHROME_HOST_TMPDIR)build-core/build-rust-imageMakefile targets