Improve rust image upgrades and built-in browser tooling#253
Improve rust image upgrades and built-in browser tooling#253
Conversation
- pass resolved tool versions into rust image builds and cover it in CI - normalize .agents mount targets so overrides and container reuse work
|
Claude finished @lroolle's task in 2m 59s —— View job 🔴 CRITICAL SECURITY AUDIT FINDINGS - BLOCKING MERGEParanoid security audit of PR #253 reveals CRITICAL security vulnerabilities requiring immediate remediation. Security Model: Container IS the sandbox. Real risks are host bridges: mounted paths, auth files, docker socket, host-net, tmux bridge. 🚨 BLOCKING ISSUES (MUST FIX BEFORE MERGE):1. BROWSER ATTACK SURFACE EXPLOSION 🔴🔴🔴
2. POTENTIAL SHELL INJECTION IN BROWSER INSTALL 🔴File: "$DEVA_HOME/.npm-global/bin/playwright" install --with-deps chromium firefox webkit
3. MOUNT HANDLING PATH TRAVERSAL RISK 🔴File: normalize_container_bind_target() {
local path="$1"
while [ "$path" != "/" ] && [[ "$path" == */ ]]; do
path="${path%/}"
done
printf '%s' "$path"
}
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 946efa83de
ℹ️ 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".
| [ "$QUICK_MODE" = false ] || return 0 | ||
| [ -d "$HOME/.agents" ] || return 0 | ||
|
|
||
| if ! user_volume_mounts_target "$target"; then | ||
| USER_VOLUMES+=("$HOME/.agents:$target") |
There was a problem hiding this comment.
This mount is added whenever -Q is not set, but it does not check whether the user passed an explicit --config-home. In that mode the script elsewhere explicitly avoids host config/cache mounts to preserve isolation (see the CONFIG_HOME_FROM_CLI gate later), so auto-mounting $HOME/.agents reintroduces host state and can leak credentials into what users expect to be an isolated auth context. Please gate this mount behind CONFIG_HOME_FROM_CLI=false (or equivalent) so explicit config homes stay isolated.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
This PR updates the deva image build pipeline to introduce a stable core image layer, improve Rust image rebuild efficiency, and bake in browser tooling (Playwright + browsers, plus Chrome on amd64), while updating CI/workflows and documentation to match.
Changes:
- Add a
coreimage build path and updateversions-up/Make targets to buildcorefirst and base Rust on it. - Rebuild images with updated runtimes/tooling (Node 24, Go version arg, bubblewrap, Playwright + MCP + in-image browsers, Chrome on amd64).
- Update CI, release/nightly workflows, and docs; add a test validating
version-upgrade.shdocker build args.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
tests/version-upgrade.sh |
Adds CI test harness to validate scripts/version-upgrade.sh docker build invocations/args. |
scripts/version-upgrade.sh |
Builds core, then main, then rust; adds args for Go/Playwright versions and uses core as Rust base. |
scripts/release-utils.sh |
Expands changelog printing behavior (removes prior truncation limits). |
scripts/install-browser-tooling.sh |
Installs Playwright CLI + MCP and installs browsers in-image. |
scripts/install-agent-tooling.sh |
Minor change to run atlas install from $DEVA_HOME. |
Makefile |
Adds Go/Playwright variables and new build targets (buildx-multi-core); bases Rust builds on core. |
docs/troubleshooting.md |
Documents bubblewrap requirement for newer Claude Code on Linux. |
docs/custom-images.md |
Updates custom build instructions for new core + Rust layering and browser tooling. |
Dockerfile.rust |
Adds Playwright versions/labels, sets browsers path, installs Playwright + browsers, and installs Chrome on amd64 only. |
Dockerfile |
Adds bubblewrap, bumps Node major to 24, and parameterizes Go install via GO_VERSION. |
deva.sh |
Hardens .agents shared mount behavior and normalizes bind targets for mount comparisons. |
.github/workflows/release.yml |
Adds/publishes core image, updates Node version, and wires Rust builds to use *-core base. |
.github/workflows/nightly-images.yml |
Adds nightly core image publishing and updates Rust to use nightly *-core base; updates Node version. |
.github/workflows/ci.yml |
Adds version-upgrade build-args test and rust/browser tooling smoke checks; updates Node version. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # Defaults | ||
| CHECK_IMAGE=${MAIN_IMAGE:-ghcr.io/thevibeworks/deva:latest} | ||
| BUILD_IMAGE=${BUILD_IMAGE:-ghcr.io/thevibeworks/deva:latest} | ||
| CORE_IMAGE=${CORE_IMAGE:-ghcr.io/thevibeworks/deva:core} | ||
| RUST_IMAGE=${RUST_IMAGE:-ghcr.io/thevibeworks/deva:rust} |
There was a problem hiding this comment.
CHECK_IMAGE is initialized from MAIN_IMAGE, so there is no way to override the inspected image via a CHECK_IMAGE env var even though the internal variable is named CHECK_IMAGE. Consider either renaming the env var in the script/usage to CHECK_IMAGE, or adding CHECK_IMAGE as an accepted override (e.g., CHECK_IMAGE=${CHECK_IMAGE:-${MAIN_IMAGE:-...}}) to avoid confusion and keep tests/CI consistent.
| for v, ver, body in sorted(changes, key=lambda x: x[0], reverse=True): | ||
| print(f"{ver}") | ||
| if body: | ||
| for line in [l.rstrip() for l in body.split("\n") if l.strip()][:15]: | ||
| for line in [l.rstrip() for l in body.split("\n") if l.strip()]: | ||
| print(f" {line}") |
There was a problem hiding this comment.
This loop now prints all GitHub release bodies between current and latest, without any line/entry limits. For repos with long release notes, this can bloat output and increase runtime. Consider reintroducing a default limit (e.g., top N releases and first M lines), with an env var to opt into full output when needed.
| lines = section.strip().split("\n") | ||
| print(lines[0].replace("## ", "")) | ||
| for line in [l for l in lines[1:] if l.strip()][:8]: | ||
| for line in [l for l in lines[1:] if l.strip()]: | ||
| print(line) | ||
| print() |
There was a problem hiding this comment.
fetch_recent_markdown_changelog is still limited by count, but each section now prints all content lines. Some changelog sections can be extremely long and overwhelm make versions/CI logs. Consider keeping a per-section line cap (or making it configurable) to keep the "recent" view concise.
| body = (rel.get("body") or "").replace("\r\n", "\n").strip() | ||
| print(ver) | ||
| if body: | ||
| for line in [l.rstrip() for l in body.split("\n") if l.strip()][:12]: | ||
| for line in [l.rstrip() for l in body.split("\n") if l.strip()]: | ||
| print(f" {line}") |
There was a problem hiding this comment.
fetch_recent_github_releases now prints the full release body (all non-empty lines). Even with per_page=$count, individual release notes can be large and dominate output. Consider restoring the previous line cap (or making it configurable) so the "recent" view stays readable.
| PATH="$FAKE_BIN:$PATH" \ | ||
| DOCKER_BUILD_LOG="$DOCKER_BUILD_LOG" \ | ||
| AUTO_YES=1 \ | ||
| CHECK_IMAGE="ghcr.io/thevibeworks/deva:rust" \ |
There was a problem hiding this comment.
The test sets CHECK_IMAGE, but scripts/version-upgrade.sh does not read CHECK_IMAGE from the environment (it derives CHECK_IMAGE from MAIN_IMAGE). This makes the test misleading and could mask regressions around which image is inspected. Set MAIN_IMAGE here instead, or update version-upgrade.sh to accept CHECK_IMAGE as an explicit override.
| CHECK_IMAGE="ghcr.io/thevibeworks/deva:rust" \ | |
| MAIN_IMAGE="ghcr.io/thevibeworks/deva:rust" \ |
| for change in reversed(changes): | ||
| print(change) | ||
| print() |
There was a problem hiding this comment.
This now prints all matching changelog sections between current and latest versions (previously capped to the most recent few). That can produce very large CI logs and slow runs when upgrades span many versions. Consider restoring a default cap (with an env override for full output) to keep output bounded.
Summary
.agentsmount handling and theversions-upupgrade pathbubblewrap, Node 24 LTS, Go 1.26.2, Playwright CLI/MCP, in-image Playwright browsers, andgoogle-chrome-stableonamd64coreimage path so Rust, nightly, and release builds reuse stable layers and keep agent upgrades cheapVerification
bash tests/version-upgrade.shdeva-local:core-testanddeva-local:rust-testbubblewrap, Rust, Go1.26.2, Playwright1.59.1, Playwright MCP, installed browser inventory, and agent CLIs insidedeva-local:rust-testgoogle-chromeis intentionally absent onarm64and present only onamd64