Repository navigation
browser and computer tools in the sandbox - #36
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
…er in the sandbox Agents drive a web browser and a virtual screen inside their own sandbox (spec/2026-10-07-browser-computer-use). The sandbox keeps no network: the browser reaches loopback and files only, and isolation defaults are unchanged. - deploy/images/display builds swarmeval/display: Xvfb, matchbox, xdotool, Playwright 1.63's Chromium, and `swarm-display`, a daemon and CLI run as the user `swarmdisplay`. Agents' own users cannot reach the screen or the browser except through the tools. - sandboxd: CreateSandboxRequest.display starts the display before the first manifest; the display user's processes are left out of every listing, so renderers are neither reported nor make changes ambiguous. ExecRequest.collect reads and removes files outside the key paths as the call's user and returns them as blobs. Exec moves to exec.go. - runtime: `computer` (screen pixels) and `browser` (accessibility snapshot refs) tools; Exec.user, Exec.collect, SandboxTool.output. Tool results carry ImageRefs; RequestOptions.max_images (default 3) keeps a request to the latest images. - model-gateway: tool messages may carry inline PNG image parts, moved into a user message after the tool messages, after reasoning passback. - Event schema 10 (`images`, `max_images`, `collected`); older runs read as before. The .eval export embeds the images; the transcript check rebuilds requests with them. - env.yaml schema 2 adds a profile's `display`; `browser` and `computer` need one. A fork after a run's first display call is refused. - console: the case flow shows a profile's display. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
ca06d11 to
073c739
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 073c7398f8
ℹ️ 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".
| for plan in sandboxes.values(): | ||
| if env.sandbox_profiles[plan.profile].display is not None: | ||
| continue |
There was a problem hiding this comment.
Reject agents configured as the display user
When a profile has a display, this early continue skips every agent-level check, including os_user. Because the display image already defines swarmdisplay and sandboxd preserves image users, a case can set os_user: swarmdisplay; the agent's shell calls then run with access to the X cookie, daemon socket, screenshots, and browser profile under /run/swarm-display. This lets it manipulate or inspect the browser without the browser/computer tool and image-event path, so agents assigned to display sandboxes must be forbidden from using this reserved account.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 75ab13d. The loader refuses os_user: swarmdisplay. With a display, sandboxd also refuses that user among the sandbox users. After it starts the display, it checks the uid a command run without a user gets, so an image built FROM swarmeval/display whose USER is swarmdisplay fails the sandbox too. I tested that against a real image with USER swarmdisplay.
| if req.Display != nil { | ||
| uid, err := s.startDisplay(ctx, sb, req.Display) | ||
| if err != nil { | ||
| return errors.Join(fmt.Errorf("sandbox %s of run %s: %w", req.SandboxID, req.RunID, err), s.drv.Remove(context.WithoutCancel(ctx), id)) | ||
| } | ||
| sb.displayUID = uid |
There was a problem hiding this comment.
Start forked displays after restoring sandbox files
For the explicitly allowed case of a fork before the first display-tool call, sandbox creation reaches this block before worker/run.py restores the source files. Consequently the start.d applications and initial browser URL consume the case's seed files, and only afterward are the mounted key paths replaced with fork state; an application that loads state at startup or the already-loaded page can therefore expose stale seed-time data on the fork's first browser call. Display startup needs to occur after RestoreFiles, or the display stack must be restarted after restoration.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Keeping this order on purpose. When the source sandbox was created, its display (start.d apps and first page) started from the seed files, and the agent's file changes came after. A fork before the first display call replays that: start from the seed files, then RestoreFiles brings the files to the source's state at the fork point. If the display started after the restore, an app that loads state at startup would begin from fork-point files the source app never started from, which is less faithful. In-memory state a process built from later file changes is not restored for any process. That is the existing fork rule ("background processes never come back"). The ordering is now documented in docs/case-format.md#display (75ab13d).
An agent with `os_user: swarmdisplay` ran its shell as the display's user, with the X cookie, the daemon's socket, and the browser profile in reach, around the `browser` and `computer` tools. The loader now refuses that os_user. sandboxd, with a display, refuses it among the sandbox's users, and after starting the display checks the uid a command without a user gets: an image whose `USER` is `swarmdisplay` fails the sandbox. The case format says why a fork before the first display call starts the display before restoring files: in the source, too, the display started from the seed files. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Summary
Agents can drive a web browser and a virtual screen inside their own sandbox (plan A in spec/2026-10-07-browser-computer-use). The sandbox still has no network: the browser reaches only loopback and files. The isolation defaults do not change.
deploy/images/display,mise run display:build): Xvfb, matchbox, xdotool, and Playwright 1.63's Chromium, plusswarm-display, a daemon and CLI that run as the userswarmdisplay. The screen needs an X cookie only that user can read, and the browser is driven through a pipe, with no debugging port open. An agent's own user cannot reach either one except through the tools.CreateSandboxRequest.displaystarts the display before the first manifest.ExecRequest.collectreads and removes files outside the key paths, as the call's user, and returns them as blobs.Execmoves toexec.go, soservice.gostays under 700 lines.computertool: mouse, keyboard, and screenshots, in screen pixels.browsertool: actions on refs from Playwright'saria_snapshot(mode="ai"), with an optional screenshot.Exec.user/Exec.collect, andSandboxTool.output.ImageRefs, which are blob hashes.RequestOptions.max_images(default 3) limits a request to the context's latest images.image_urlparts. Before the request goes upstream, the gateway moves them into a user message after the tool messages. It does this after reasoning passback, so thewithin_turnboundary does not move.images,max_images, andcollected. Older runs read as before..evalexport embeds the images. Tool results carry them inline; contexts reference them as attachments, with[image omitted]for images a request left out.env.yamlschema 2 adds a profile'sdisplay; version 1 still loads.browserandcomputerrequire a profile with a display.FAILED_PRECONDITION, because a fork restores files but not the browser.Test plan
mise run checkmise run go:test-integration:TestLiveDisplayDrivesTheBrowserAndKeepsItsProcessesOutruns against the real display image, and the rest of the sandboxd suite passes.mise run test:docker:tests/worker/test_display_e2e.pyruns the whole platform:.evalresolves them;/run/swarm-display.runsc). Only runc on arm64 was available here, so Chromium under runsc still needs checking on the Linux host.Notes
docs/services/orchestrator.mdwas already over 700 lines (794); this PR adds two..eval.test:dockerrun,test_takeover.py::test_a_live_worker_keeps_its_run_past_many_leasesfailed once. It uses 0.3 s leases and failed while the Go integration tests were running at the same time; it passes on its own.🤖 Generated with Claude Code