Add persistent repo sandbox tools#4
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces a repository sandbox execution environment for running bounded commands in a persistent sandbox checkout of a PR's head ref, supporting both Cloudflare and Node.js environments. It configures Cloudflare Sandbox containers, adds new tools (run_repo_command and dependency_review), and integrates sandbox cleanup when a PR is closed. Feedback on these changes highlights a critical security vulnerability in command validation where find and sed can bypass safety checks, a portability issue with using the -l flag on /bin/sh in the Node.js runner, and a performance improvement for base64 conversion in the Cloudflare runner.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
Mimir review — 🔴 Changes requestedThe sandboxed command-execution design is mostly sound (command allowlisting, metachar rejection, per-PR lifecycle), but the Node runner — which remains the recommended production target per ADR 0001 — provides no real isolation: read-only commands inherit the server's full environment and can read arbitrary host files, so a prompt-injected PR can exfiltrate secrets. There is also a confirmed rg Findings: 1 critical · 1 major · 3 minor · 0 nit
💰 Review cost: $0.0912 — primary |
isSafeSandboxCommand treated find and sed as read-only, but find -exec/-ok/ -delete/-fprint* spawn arbitrary programs or write files, and sed's w/e commands and -i flag do the same — none require a blocked shell metacharacter. Deny find's exec/write primaries by flag, and move sed into the exec-gated command set alongside node/npm. Also fixes the Node sandbox runner: /bin/sh -lc fails on dash-based /bin/sh (Debian/Ubuntu/Alpine), tar extraction had no --no-same-owner guard, and the Cloudflare runner's toBase64 built an intermediate binary string instead of using Buffer.
rg's --pre/--pre-glob flags run an arbitrary command per searched file, bypassing the read-only guard the same way find's -exec family did. Deny it the same way. instruction.ts and run_repo_command's own description still advertised sed as a read-only example after it moved to the exec-gated set — the model would issue sed and get bounced. Dropped it from both. Pin @cloudflare/sandbox to an exact version; it's the isolation boundary for untrusted repo code so a floating patch range shouldn't be it.
execFile inherited the full process.env, so a read-only command like
awk 'BEGIN{print ENVIRON["GITHUB_APP_PRIVATE_KEY"]}' — no file read, no
blocked metacharacter — could exfiltrate the runner's GitHub App key,
model API keys, and DATABASE_URL into a review comment. Pass an explicit
allowlisted env (PATH/HOME/locale only) to both the command-v probe and
the command itself. The Cloudflare runner is unaffected; its commands run
in an isolated container that never sees the worker env.
Summary
Verification