Conversation
…bridge - remote.py: fail-closed config resolver (HTTPS, bearer token ≥32 bytes, no redirects/credentials/query params) - host_validation.py: Host/Origin allowlist validation (no wildcards) - host_bridge.py: authenticated MCP host bridge (Starlette/ASGI, bearer auth, DNS rebinding protection, session idle timeout) - host_bridge_cli.py: CLI launcher (Linux Xvfb, macOS, Windows interactive) - host_bridge_standalone.py: standalone launcher with no Hermes deps - cua_backend_session.py: remote connection path in lifecycle coroutine - cua_backend.py: resolve remote config, skip local driver when remote - cua_backend_capture.py: skip X11 probe when capturing remote desktop - test_remote_config.py: 19 tests covering all fail-closed paths Verified E2E on Firecracker microVM (Ubuntu 24.04, Xvfb + openbox, cua-driver 0.23.2): auth, session, tools/list (60 tools), get_screen_size, get_cursor_position, check_permissions, health_report all pass.
…k_fn remote branch, CLI bridge subcommand, DEFAULT_CONFIG remote section
…, TLS gate for non-loopback binds, Xvfb lifecycle, platform guards
…middleware, no-store, control-char token rejection, host normalization, 1800s idle default
…ams, SDK httpx2 client, no env proxies, no local CLI fallback
…launchers The create_host_bridge_app default moved to 1800s (MCP interactive-session recommendation); the launchers still explicitly passed 300, so the production paths never saw the new default. Align all three.
…allowed-hosts help The live E2E surfaced that the SDK compares the raw Host header, so a portless allowlist entry (127.0.0.1) never matches Host: 127.0.0.1:8765 and legitimate clients get 421. State it in the help so operators do not trip it.
…st_bridge; update kwargs contract test
…zer, idle-timeout plumbing)
… child process envs
…er; pop bridge token from environ
…; bound startup with in-coroutine deadline
… reject explicit remote null config
…arity/arity/version tests
… help; narrow remote-config suppression to not-configured signals
andrexibiza
left a comment
There was a problem hiding this comment.
Reviewed exact head 4f307605ea89a4deaeea87b85bc1b04c1b655470 against exact current main 245e48008fa814b3251f50755eb656bd9fb86cb1. I went through the 22-file diff, the remote resolver/client, host bridge and standalone launcher, backend lifecycle, availability/config paths, secret propagation/child-env policy, the touched tests, all 17 surviving commits, exact-head Actions, and the related #103653 / #90380 / #90423 / #103932 / #71157 / #71160 graph.
There is a lot of real work here. In particular, the transport boundary is considerably harder than a naïve “HTTP wrapper around cua-driver”: transport-security middleware runs before auth, Host/Origin are explicit allowlists, non-loopback plaintext is refused unless deliberately acknowledged, redirects and ambient proxies are disabled client-side, bearer material is stripped from child-process paths, driver telemetry is forced off, remote selection has no local fallback, idle-expiry recovery is explicit, and the bridge/standalone parity tests are useful. The Linux/Windows E2E evidence in the PR body is also the right kind of evidence for this feature.
I do not think this exact head is landable yet, though. Three authority/liveness boundaries still need to move with the new remote capability.
1. Remote desktop ownership is selected per profile, but the live backend owner is still keyed only by session_id
tools/computer_use/tool.py::_get_backend() on current main caches _backends, _backend_call_locks, and _backend_permission_modes by sid = str(session_id or ""). Once a backend exists for that sid, it is returned without rebuilding it.
This PR then makes CuaDriverBackend.__init__() resolve and retain computer_use.remote + HERMES_CUA_REMOTE_TOKEN from the currently active profile when the backend is created. Those two facts do not compose safely under gateway.multiplex_profiles: profile A can create session S against A's remote URL/token; profile B can later reach the same session id S and receive A's already-live backend without B's config or secret scope ever being consulted. For this feature the cached object is not just a convenience object — it is physical mutation authority over a desktop.
This is the same underlying owner-qualification problem already exposed on #90380, and #104080's direct-main integration does not remove it. The resource owner needs to be qualified by the canonical profile/home identity as well as the session id (and the release/call-lock/permission-mode/empty-hook lifecycle needs to use the same identity). Please add a regression with two profile homes using the same session id but different remote URLs/tokens and prove that neither reuse nor release crosses profiles.
2. _resolve_token() re-opens the exact multiplex secret fallback that agent.secret_scope.get_secret() closes
Current-main agent.secret_scope.get_secret() is deliberately explicit: with an active scope, a missing credential returns the default when multiplexing is active and never falls through to another profile's os.environ value.
In this PR, tools/computer_use/remote.py::_resolve_token() does:
if current_secret_scope() is not None:
scoped = get_secret(_REMOTE_TOKEN_ENV)
if scoped is not None:
return scoped
return environ.get(_REMOTE_TOKEN_ENV, "")resolve_remote_cua_config() supplies os.environ when no test mapping is injected. So in production, multiplex_active=True + a valid profile scope that simply does not contain HERMES_CUA_REMOTE_TOKEN can still borrow the process-wide token. The inline comment says a multiplex scope miss must not borrow process state, but the next line does exactly that.
The existing test_scope_active_var_absent_falls_back_to_environ only establishes the single-profile overlay behavior because it does not enable multiplexing. Preserve that behavior for non-multiplex deployments, but add the missing production case: multiplex active + scope installed + remote token absent from scope + ambient token present must fail closed / resolve no token. The simplest repair is to make the post-get_secret() fallback conditional on multiplex being inactive rather than bypassing get_secret()'s policy.
This one is security-significant: the credential being crossed is the bearer authority that drives the remote desktop.
3. A process-global backend lock now contains a bounded network handshake
The PR body correctly calls this out as inherited behavior, but the new transport changes its consequence enough that I think it is a landing issue rather than a footnote. _get_backend() holds the process-global _backend_lock across backend.start(). For local CUA that was local process/driver startup. For this PR it can be MCP-over-HTTP initialization to another machine, with a deliberately bounded ~30–35s failure budget.
That means one dead/slow remote desktop can pin backend creation and lifecycle operations for unrelated sessions/profiles behind a network failure. The “other side of the shape” here is not only startup: release_computer_use_session() and other map ownership transitions share the same global lock, so an unrelated session can be prevented from relinquishing authority while another remote is connecting.
Please move expensive start out of the process-global ownership lock and give each qualified backend owner a single-flight/start state; then settle/install the winner atomically under the map lock. A concurrency regression should hold remote start for owner A and prove owner B can create/release without waiting for A's network deadline.
CI / surviving-commit acceptance
The current head itself is green on all three exact-SHA workflows I can see: CI 34014832785, Docker Build/Test/Publish 34014832496, and Nix flake check 34014832552, all at 4f307605....
I also checked every surviving commit SHA in the 17-commit train. GitHub currently exposes no PR-triggered workflow run for the first 16 surviving SHAs (c43a5ac0... through d0b4e27d...); only the current head has exact-object receipts. So the current head is green, but the surviving commit series is not yet proven green commit-by-commit. Please produce the missing exact implementation-commit evidence without proof-only/no-op CI commits.
Topology / attribution / merge order
- #103653 is the same contribution expressed through the #90380 provider seam, not an independent competing implementation. Preserve this work once: whichever integration route lands should absorb these authority/liveness fixes; the other should rebase/retire rather than creating two remote-CUA behavior owners.
- #90380 is the architectural seam owner, and its existing resource-owner/global-start findings remain relevant. #104080 avoids depending on that seam, but it does not make those underlying lifecycle invariants disappear.
- #90423 is complementary, not duplicate: it carries the Desktop→agent/client-mediated direction (and preserves #61507 / Dan Bennett provenance); this PR is agent→remote-host. The naming separation is good.
- #103932 is the fresh user-facing request for this exact remote-computer-use shape. This implementation should link/close that request when appropriate so the requirement/provenance chain is not lost.
- #71157 / #71160 (skyzea1) and upstream trycua/cua #2563 are adjacent architecture, not work to erase. They propose Cua-owned paired-host routing with Hermes as a thin consumer. Upstream #2563 is still an open docs/RFC PR with no driver implementation, so it is not a ready substitute for this work today; it is a future convergence/interoperability boundary worth preserving explicitly.
Once the profile-qualified resource owner, multiplex secret miss, global-start isolation, and surviving-commit proof are closed, the actual transport/bridge hardening here is strong. This is difficult cross-machine authority work, and the amount of adversarial cleanup already in the branch is visible. 🚀
…utside global lock; close multiplex token-scope miss Finding 1 (credential isolation): backend cache (_backends, _backend_call_locks, _backend_permission_modes) was keyed by session_id only. Under gateway.multiplex_profiles, profile B reaching the same session_id as profile A would reuse A's live backend — physical control of A's remote desktop. Keys are now profile-qualified via hermes_home_key():session_id so each profile's cache is independent. Finding 2 (token scope miss): _resolve_token returned environ fallback when a secret scope was active but the token var was absent. Under multiplexing this borrows another profile's process-wide token. Now returns empty string (fail closed) when multiplex is active, so the token-length validator rejects it instead of silently sending the wrong credential. Single-profile deployments (multiplex off) still fall through to environ. Finding 3 (global lock during startup): backend.start() ran inside _backend_lock, pinning all other sessions' lifecycle operations during a slow remote handshake (up to ~35s). start() now runs outside the cache lock under a per-owner single-flight lock; concurrent threads for the same owner wait, and unrelated owners proceed in parallel. Tests: updated upstream tests that directly access _backends with bare session IDs to use _backend_owner_key(); added test_multiplex_active_scope_present_var_absent_fails_closed covering the cross-profile credential-isolation guarantee. 255 passed, 1 skipped.
4f30760 to
7905af7
Compare
…ys and per-owner start isolation Covers the two dispatcher fixes from the NousResearch#104080 review: (1) the same session id under two profile homes must resolve to distinct cache entries — _backend_owner_key() embeds hermes_home_key(); (2) while one owner's backend.start() is blocked (slow remote handshake), another owner's cached-backend lookup must complete without waiting — start() runs outside _backend_lock under a per-owner single-flight lock.
…ys and per-owner start isolation Covers the two dispatcher fixes from the NousResearch#104080 review: (1) the same session id under two profile homes must resolve to distinct cache entries — _backend_owner_key() embeds hermes_home_key(); (2) while one owner's backend.start() is blocked (slow remote handshake), another owner's cached-backend lookup must complete without waiting — start() runs outside _backend_lock under a per-owner single-flight lock.
…ys and per-owner start isolation Covers the two dispatcher fixes from the NousResearch#104080 review: (1) the same session id under two profile homes must resolve to distinct cache entries — _backend_owner_key() embeds hermes_home_key(); (2) while one owner's backend.start() is blocked (slow remote handshake), another owner's cached-backend lookup must complete without waiting — start() runs outside _backend_lock under a per-owner single-flight lock.
|
The three blockers are on this head (
PR body now links #103932 (this implements that request) and notes #71157 / #71160 / trycua/cua#2563 as adjacent paired-host architecture, not a substitute. We are not rewriting the commit train. The stacked heads are green as a whole. |
andrexibiza
left a comment
There was a problem hiding this comment.
Re-review of current head 3bdc0f769f85bf88e86fb1c956896225e29ab065 against live main 693641aa8b4359c602283bdbbc14041e03bc47bc.
The previous three findings moved materially: the multiplex token read now fails closed on scope miss; backend cache identity is profile-qualified; and remote backend.start() no longer runs under the process-global cache lock. Exact-head CI, Docker, and Nix are all green on this head.
Two authority defects remain:
-
Same-owner release does not fence an in-flight start.
_detach_locked(owner)removes_backend_start_locks[owner]even while a caller can be blocked insidebackend.start(). A concurrentrelease_computer_use_session()therefore sees no cached backend, removes the single-flight lock, and returns; a new lookup can create a second start lock, while the original starter later unconditionally installs its backend. That can resurrect desktop authority after release and can leave two live backends for one owner. The start needs a generation/ownership record that release invalidates; completion may install only if that generation still owns the slot, otherwise it must stop/discard the newly started backend. Add a regression that blocks owner A instart(), releases owner A, reacquires owner A, then lets the original start finish and proves no stale backend is installed or leaked. -
Approval authority is still keyed by bare
session_idwhile backend authority is now profile-qualified. The PR's own isolation test establishes that the same session id can exist under two profile homes._session_auto_approveand_always_allowstill use that bare id, so analways_approve/session grant from profile A can suppress approval for profile B with the same id; releasing B also clears A's grant. The approval stores and their release path need the same profile-qualified owner identity (or an equivalent qualified approval key), with a same-session-id/two-profile regression.
Acceptance evidence is also not current-main evidence yet: the branch is currently 247 commits behind live main (merge base 245e48008fa814b3251f50755eb656bd9fb86cb1). The current head's three workflows are green, but the branch has 18 surviving commits and only 16 PR-triggered workflow runs total, so there is not an exact GitHub workflow receipt for every surviving commit. Restack onto current main, preserve the two fixes above, and regenerate CI/Docker/Nix on the resulting train before treating this as landable.
3bdc0f7 to
97dc752
Compare
…ys and per-owner start isolation Covers the two dispatcher fixes from the NousResearch#104080 review: (1) the same session id under two profile homes must resolve to distinct cache entries — _backend_owner_key() embeds hermes_home_key(); (2) while one owner's backend.start() is blocked (slow remote handshake), another owner's cached-backend lookup must complete without waiting — start() runs outside _backend_lock under a per-owner single-flight lock.
|
Both authority defects from the re-review are fixed on the new head
Both regressions were observed red on the previous head Exact-head receipts regenerated on |
Regression-first on 97dc752: 13 selected cases fail for owner collisions, trailing-colon release, empty-session sharing, mode-replacement resurrection and failed-start cleanup; the queued-waiter pin already passes. Production is unchanged in this test-only commit. Ownership model reference: salvage/provider-seam-blockers ace49c7; NousResearch#103653 lineage (bennybuoy-authored).
Use (home, session) keys for all backend and approval stores; detect empty sessions from the session component. Make the private empty-session injection hook home-keyed and migrate its callers and shutdown tests. Ported ownership and injection patterns from salvage/provider-seam-blockers ace49c7, NousResearch#103653 lineage (bennybuoy-authored). Defects 1-3 were RED on 97dc752. Focused computer-use suite excluding the pending mode-fence and failed-start regressions: 464 passed, 1 skipped, exit 0.
Keep the owner generation across mode-replacement teardown, recheck it after stop, and reread permission mode before constructing the replacement. Use one flat single-flight acquisition; never retry a revoked lookup into a new generation. Stop uninstalled candidates on BaseException and remove only the failing generation's bookkeeping. Release reports True when it revokes an in-flight generation, including mode teardown with no cached backend. Ported startup-generation pattern from salvage/provider-seam-blockers ace49c7, NousResearch#103653 lineage (bennybuoy-authored). Added failed-generation cleanup and release-result semantics required by this review. Mode-teardown and failed-start cases were RED on 97dc752; queued waiter was already GREEN. Full requested focused suite: 469 passed, 1 skipped, exit 0.
|
Follow-up: our own adversarial re-review of
The same head also closes two further items that review of the branch surfaced:
All regressions were red-first: 13 cases fail on Structural note for the route discussion: the ownership model here (tuple owner keys, home-keyed empty-session state, generation-fenced startup) is now the same shape as the provider-seam branch of this work (#103653 / #90380), so re-expressing this transport as a provider on the seam later is a small adapter rather than a redesign. Exact-head receipts on |
Capture the trusted home at agent construction and bind it only around CUA release. Never infer an unknown owner from the teardown thread. Add composed-close and blocked-start timer regressions; preserve the existing cleanup mock contracts with explicit owner homes.
e5649c4 to
0999d6f
Compare
|
Restack update: this branch now sits on current What changed since the last update:
Receipts on the exact head |
|
We prefer the repaired provider-factory seam in #103653 for extensibility. #104080 remains the alternative if maintainers want the narrower direct integration. Both target The direct version now carries the applicable shared safety fixes: release/dispatch fencing and both remote-to-local CLI guards, plus configuration-fallback/deleted-target protection. It also retains the constructed desktop for availability and diagnosis, restores each owner's home during exit teardown, and clears failed-generation approval grants without clearing a newer generation's grants. Incomplete remote selection fails closed; explicitly disabled remote remains a local choice. The common host-bridge and MCP-session sources match. Provider registration, registration-revision revocation and plugin-resource cleanup remain specific to the seam version. The direct version has no provider registry. Verified tips: seam Required checks pass on both published heads: seam CI and direct CI. The OSV annotation is neutral on both, so this is not a clean differential vulnerability-scan claim. Neither PR has been merged or deployed. |
|
@andrexibiza — flagging for your next pass when convenient: both findings from your latest review are closed on the current head, plus a teardown defect our own adversarial gate caught (red-first regressions in place). All required checks are green on the exact head. Routing-wise we now lean toward the seam twin #103653 — your call, reasoning in the comment above. |
andrexibiza
left a comment
There was a problem hiding this comment.
Re-review on exact head 9a7971626d1c10938d9e5d1adc5b8e862d1bcbca against live main b1f003e18633298d549668b8e186af84cca45b76.
The two code findings from my last pass are closed.
-
Release now fences in-flight startup authority. The start-lock object is the owner generation:
release_computer_use_session()removes that exact generation, and a delayed starter must still own the same object before it can install. A stale completion is stopped/discarded instead of resurrecting a released backend. The regression coverage exercises release/reacquire with a blocked startup, queued waiters, replacement teardown, and failed-start cleanup. -
Approval authority is now profile-qualified.
_session_auto_approveand_always_alloware keyed by the same structural(profile_home, session_id)owner used for backend authority, and reads, writes, and release cleanup use that owner consistently. The collision regression covers bothapprove_sessionandalways_approve, including same session IDs under different homes and release isolation.
I also checked the current exact-head GitHub receipts: the required CI, Docker, and Nix runs on 9a797162... are green. I do not have a new line-level code finding on this head, so I am not manufacturing inline comments.
This is not yet a landing receipt, for two separate reasons:
- The branch is currently 263 commits behind live
mainand the comparison is diverged. It needs a current-main restack, then fresh exact-head CI/Docker/Nix. The final surviving commit series also needs green exact-SHA evidence for every commit; this branch currently has 35 surviving PR commits but only 28pull_requestworkflow runs total, so the per-commit PR-triggered receipt gate cannot be satisfied by the present history. - On routing: I agree with moving the landing path to #103653. The two branches share the repaired direct transport/safety foundation through
0999d6f5..., but #103653 additionally carries the credited provider-factory seam, profile-scoped provider registration, registration-revision fencing, and plugin lifecycle coverage. That is the better architectural carrier. These are alternatives, not a stack: only one should land. Keep #104080 as the narrow fallback unless #103653 fails its own review/acceptance gate; do not merge both.
So: prior code blockers resolved; exact current head green; preferred carrier #103653; landing still blocked on current-main restack + final all-green commit evidence.
Summary
Adds remote transport to
computer_use: a headless agent connects over authenticated MCP streamable-HTTP to cua-driver on another machine. The desktop host runs a bridge, without needing Hermes; the agent needs neither a local display nor a local cua-driver binary.Implements #103932, linked without closing the issue here.
Two alternatives and our preference
We prefer the repaired provider-factory seam in #103653 for extensibility. It includes the credited seam and remote provider together, now targets
main, and no longer depends on the original seam branch in #90380.This PR remains the alternative if maintainers want the narrower integration. It selects remote transport through
computer_use.remote.enabled/urlin the existing backend; it does not add the provider registry or plugin-provider lifecycle. Both PRs carry the applicable shared safety fixes. Provider registration, registration-revision fencing and migration of provider selectors remain specific to the seam version. These are alternative merge routes, not a stack intended to land twice.Configuration and transport
Store
HERMES_CUA_REMOTE_TOKENin the profile's secret environment (at least 32 bytes). The client requires HTTPS for non-loopback destinations, rejects URL credentials/query/fragment and control characters, and disables ambient proxies and redirects. Remote sessions reject non-standard permission modes.Current computer-use configuration is checked against the normal loader's effective values, including environment expansion and managed precedence. An unrelated loader error cannot silently select local defaults or restore a removed target from last-known-good state. Boolean and integer values are distinguished. A written remote block requires an explicit boolean
enabled; incomplete target configuration cannot silently choose local. This direct version does not accept acomputer_use.providerselector; use the seam alternative for provider selection.Remote transport cannot substitute the gateway's local CLI, including on empty window/capture results. Idle-expired MCP sessions can rebuild, with replay restricted by action safety, and startup has bounded teardown.
Ownership and release
Backend and approval ownership uses structural
(profile_home, session_id)tuples. Startup is single-flight per owner, outside the global cache lock; teardown retains the original session's profile home. Release fences in-flight starts and queued dispatch. Already-admitted actions drain before the backend stops, and unrelated owners remain independent.Backend availability and empty-discovery diagnostics use the constructed transport, rather than borrowing a later configuration's target. Editing configuration does not hot-swap an already-running backend.
Host bridge
hermes computer-use host-bridgeexposes cua-driver behind bearer authentication, exact Host and Origin allowlists, transport-security validation before authentication,Cache-Control: no-store, and idle-session teardown. Host entries include the port. Non-loopback plaintext binds require explicit acknowledgement; protect exposed deployments with TLS. Child environments exclude bridge credentials and approval-bypass settings.The standalone launcher has no Hermes dependency. It needs three source files (
host_bridge_standalone.py,host_bridge.py,host_validation.py), Python 3.11+, MCP, Starlette, Uvicorn and cua-driver. On Linux it can manage Xvfb when needed. Both launchers have behavioural parity coverage, including idle-timeout forwarding.The published
cua-host-bridgepackage is a separate release artefact. This PR does not update it or claim that its released source is identical to this head.Verification
9a7971626d1c10938d9e5d1adc5b8e862d1bcbca: 43 repository test files, 577 passed, 0 failed, 1 skipped, using the canonical per-file runner with two workers and retries disabled. Includes remote transport, capture/CLI boundaries, real-loader configuration controls, ownership/approvals, startup/release races and launcher/child-environment checks.cua-host-bridge 0.1.0, not newly deployed host code.git diff --checkpassed. Source-pinned type checking introduced no diagnostics relative to0999d6f5dc; 36 inherited diagnostics remain in that checked surface.Earlier transport testing exercised a real Windows desktop over Tailscale with consented Notepad launch and typing, plus separate displays across three bridges in a Linux VM. Those are historical transport receipts, not fresh Windows or desktop-input tests of this head. The full repository suite was not run locally.
Related work and scope
host-bridgename leaves itsbridgesubcommand available.VM provisioning, fleet orchestration, Windows UIA integration, driver key-alias changes and live deployment are outside this PR.