Add an on-demand browser editor for running VMs by thomas-chauchefoin-tob · Pull Request #599 · trailofbits/coop

thomas-chauchefoin-tob · 2026-10-26T23:29:53Z

Add coop web-editor to install and run code-server (Visual Studio Code) inside an existing VM, and print a local URL and session password for the user to open in their browser. Editing, extensions, and terminals run in the guest so this brings a stronger isolation than coop editor. This was only tested on a macOS host.

CI failure should be fixed after #598 lands.

Add `coop web-editor` to install and run code-server inside an existing VM. Print a local URL and session password for the user to open in their browser. Editing, extensions, and terminals run in the guest so this brings a stronger isolation than `coop editor`.

hbrodin · 2026-10-28T11:36:56Z

Review summary: I left two inline suggestions: reject a conflicting --port when joining an existing editor session, and assert guest service cleanup in the VM integration test. The password-output documentation wording is a minor follow-up, so I did not leave an inline finding for it. No material diff noise found.

I checked correctness, design, conventions, security, API usage, tests, documentation, and comments against the PR head and discussion. The existing CodeQL password-output thread is resolved; interactive display is intentional. The check job passed. deny fails on the pre-existing Tokio 1.53.2 advisory; the license check passed. Lima and Firecracker VM integration were not run in the visible CI checks or in this review.

Co-authored-by: Henrik Brodin <90325907+hbrodin@users.noreply.github.com>

hbrodin · 2026-10-30T08:43:22Z

+    </dev/null >"$base/editor.log" 2>&1 9>&- &
+child=$!
+ready=0
+while kill -0 "$child" 2>/dev/null; do


I reproduced a healthy code-server that never reached the host's ready state: the guest /healthz returned 200 and the transient unit was active, but this wrapper emitted no ready marker and was blocked in wait. When the 600-second host startup timeout fired, coop removed the tunnel, host password, metadata, and host temp directory, while the guest unit, descendants, sockets, password config, and /tmp/coop-we-* directory remained.

Could we avoid treating the long-lived sudo systemd-run --wait --pipe client PID as both service liveness and lease state? A systemd-native shape would be:

start a detached transient unit with systemd-run --collect --service-type=exec (without --wait/--pipe);

route unit output with StandardOutput=append:... and StandardError=append:...;

keep this SSH shell in the heartbeat loop, checking systemctl is-active and /healthz;

have the EXIT/HUP trap synchronously systemctl stop the unit before removing the sockets/config.

Please also add a regression for timeout or owner death before readiness; the existing cleanup assertions only run after a URL has been published.

hbrodin · 2026-10-30T08:43:24Z

CVE-2026-111497

+[[ $password =~ ^[a-f0-9]{64}$ ]]
+config="${socket%/*}/config.yaml"
+
+version=4.141.0


This pins and executes code-server 4.141.0, but 4.141.1 was released before this PR and explicitly updates bundled Node.js for the Brotli decoder RCE (CVE-2026-111497) and rejects compressed request bodies. Please update the version and both architecture hashes to at least 4.141.1, preferably a currently reviewed release.

Could we also prevent this hard-coded pin from silently aging again? Dependabot only discovers supported manifests, so it will not see a shell literal (supported ecosystems). A small scheduled checker would fit the current repository better than introducing Renovate just for this pin: compare against the latest non-draft/non-prerelease GitHub release, download the exact amd64/arm64 assets, compute both SHA-256 values, and open a human-reviewed PR updating these three values. It should not auto-merge or install an unreviewed release.

hbrodin · 2026-10-30T08:43:26Z

+    rm -rf -- "$staging"
+    mkdir -- "$staging"
+    curl --fail --silent --show-error --location --proto '=https' --proto-redir '=https' \
+        --connect-timeout 15 --max-time 300 --max-filesize 536870912 \


The fixed five-minute total timeout makes first use deterministically fail on slower links. The ARM64 archive was 234,587,826 bytes; in my test it reached about 132 MB after 230 seconds, then hit this timeout. The EXIT cleanup and the next invocation's rm -rf "$staging" discard the partial archive, so every retry starts from zero.

Could we retain the private, install-locked partial file and resume safely (with the final pinned SHA-256 still mandatory), or otherwise choose a timeout appropriate to the artifact size? This should also emit bounded phase/download progress so a multi-minute first run does not look hung.

hbrodin · 2026-10-30T08:43:28Z

+        .process_group(0)
+        .stdin(Stdio::piped())
+        .stdout(Stdio::piped())
+        .stderr(Stdio::null());


Discarding SSH stderr loses the only concrete startup failure. When the first-use curl timed out, coop reported “Editor SSH session exited unexpectedly” and directed me to guest editor.log, but that file did not exist because code-server had never started.

Rather than forwarding arbitrary guest stderr, could stdout carry a bounded structured startup protocol such as installing, download progress, starting, ready, and failed {stage, reason}? That would preserve the distinction between download, SSH-forward, systemd-launch, and code-server runtime failures and make the host diagnostic actionable.

hbrodin · 2026-10-30T08:43:30Z

        #[arg(long)]
        clean: bool,
    },
+    /// Serve a browser editor inside a running VM (installs code-server on first use)


After testing this, I don't think browser editing warrants another top-level command alongside the existing editor/vscode commands. Could this be exposed as coop editor [NAME] --web [--port PORT] instead? --web would conflict with --editor and --clean, --port would require --web, and --project could remain meaningful for both modes. This keeps the user goal and instance-resolution behavior under one discoverable command.

Also, we should probably retire vscode at some point.

hbrodin · 2026-10-30T08:43:32Z

 administration aliases, and pull does not sanitize repositories created by an
 older vulnerable release.

+### `web-editor`


If this remains a top-level command, please also update the exhaustive command mappings: docs/multi-instance.md omits web-editor from the commands using optional instance-name resolution, and docs/shell-completion.md omits it from the commands completing running instances even though the clap argument uses running_instance_candidates.

hbrodin · 2026-10-30T08:43:34Z

+reducing exposure through the native host-editor integrations used by `coop editor`.
+Browser vulnerabilities remain part of the attack surface.
+
+Code-server's password login restricts access to the editor, and the host 


This added line has trailing whitespace, and git diff --check 34a24da...463aab7 fails on it. Please remove the trailing space after “host”.

hbrodin · 2026-10-30T08:43:43Z

Hands-on follow-up review at 463aab7: I left seven inline comments covering the reproduced pre-readiness/systemd lifecycle failure, the vulnerable code-server pin plus release monitoring, non-resumable first-use download timeout, lost startup diagnostics, CLI placement under editor, missing command-map documentation, and the whitespace gate.

Reproduction: built the release binary, created a disposable Lima VM, cloned a public project, and exercised the browser editor end to end. One first-use download timed out after five minutes and discarded its partial archive. After disabling the VPN, code-server installed and /healthz returned 200, but coop never published readiness; its ten-minute timeout removed the host tunnel/state while leaving the guest transient unit, descendants, sockets, config/password, and temp directory alive. Focused web-editor unit tests passed, demonstrating that they do not cover this real sudo/systemd lifecycle.

I reviewed correctness, lifecycle, design, conventions, security, external API/tool usage, tests, documentation, and comments. Existing review threads about a conflicting attached --port and post-ready cleanup were not duplicated. The visible PR checks are green but do not include a real VM integration job. This run exercised macOS/Lima only; Linux/Firecracker integration was not run.

The new outbound GitHub release URL and loopback listener are explicit stop-and-confirm surfaces under the repository trust model. Their boundaries otherwise looked appropriate: loopback-only forwarding, a private guest Unix socket, password delivery over stdin, private host/guest storage, HTTPS-only redirects, and a pinned SHA-256.

github-advanced-security Bot found potential problems Oct 26, 2026

View reviewed changes

Comment thread src/web_editor.rs Dismissed

thomas-chauchefoin-tob force-pushed the feat/web-editor branch from 0994a1d to 33c4b02 Compare October 26, 2026 23:43

thomas-chauchefoin-tob added 2 commits October 27, 2026 00:48

Remove trailing whitespace in CLI tests

10d1265

Update VM and SSH types

ff15a69

thomas-chauchefoin-tob marked this pull request as ready for review October 27, 2026 00:04

hbrodin reviewed Oct 28, 2026

View reviewed changes

Comment thread src/web_editor.rs Resolved

hbrodin reviewed Oct 28, 2026

View reviewed changes

Comment thread tests/integration-web-editor.py Resolved

thomas-chauchefoin-tob and others added 2 commits October 28, 2026 17:33

Update src/web_editor.rs

41c042d

Co-authored-by: Henrik Brodin <90325907+hbrodin@users.noreply.github.com>

test: verify guest editor cleanup after Ctrl+C and owner death

463aab7

hbrodin reviewed Oct 30, 2026

View reviewed changes

Navigation Menu

Repository navigation

Add an on-demand browser editor for running VMs - #599

Add an on-demand browser editor for running VMs#599
thomas-chauchefoin-tob wants to merge 5 commits into
mainfrom
feat/web-editor

thomas-chauchefoin-tob commented Oct 26, 2026 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Uh oh!

hbrodin commented Oct 28, 2026

Uh oh!

hbrodin Oct 30, 2026

Uh oh!

hbrodin Oct 30, 2026

Uh oh!

hbrodin Oct 30, 2026

Uh oh!

hbrodin Oct 30, 2026

Uh oh!

hbrodin Oct 30, 2026

Uh oh!

hbrodin Oct 30, 2026 •

edited

Loading

Uh oh!

hbrodin Oct 30, 2026

Uh oh!

hbrodin Oct 30, 2026

Uh oh!

hbrodin commented Oct 30, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

thomas-chauchefoin-tob commented Oct 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

hbrodin commented Oct 28, 2026

Uh oh!

hbrodin Oct 30, 2026

Choose a reason for hiding this comment

Uh oh!

hbrodin Oct 30, 2026

Choose a reason for hiding this comment

Uh oh!

hbrodin Oct 30, 2026

Choose a reason for hiding this comment

Uh oh!

hbrodin Oct 30, 2026

Choose a reason for hiding this comment

Uh oh!

hbrodin Oct 30, 2026

Choose a reason for hiding this comment

Uh oh!

hbrodin Oct 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hbrodin Oct 30, 2026

Choose a reason for hiding this comment

Uh oh!

hbrodin Oct 30, 2026

Choose a reason for hiding this comment

Uh oh!

hbrodin commented Oct 30, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

thomas-chauchefoin-tob commented Oct 26, 2026 •

edited

Loading

hbrodin Oct 30, 2026 •

edited

Loading