feat: add secure browser human handoff - #92350
Conversation
andrexibiza
left a comment
There was a problem hiding this comment.
Blocking review (GitHub will not permit this identity to set REQUEST_CHANGES without explicit repository review access).
The core handoff machinery is thoughtfully built: the bearer is 256-bit and digest-only at rest, terminal states stop rendering the live URL, browser retention is tied to the durable handoff/wake state, claim_wake() is an atomic single-delivery boundary, and the turn loop now has an explicit wait boundary rather than asking the model to keep acting while the human owns the browser. I also verified this exact head against current main (999703fd43ab6d75c4a5c7bc8b610dd73ecece76): CI 32584694775, Docker 32584694468, and Nix 32584694449 are all green.
There is one blocking routing defect in the claimed raw-API + multiplex path.
Blocker — a /p/<profile> API-origin handoff loses the selected profile before the URL and wake identities are persisted.
The production chain on this head is:
APIServerAdapter's profile-prefix middleware correctly resolves/p/worker/..., sets_api_request_profile, and enters_profile_scope("worker"). That makesget_hermes_home()/ secrets / profile-local state point at the worker profile for the request._bind_api_server_session()is documented as the single structural chokepoint for every API-server agent entry, but itsset_session_vars(...)call bindsplatform="api_server", session/chat identity, browser-control identity, andasync_delivery=Falsewithout bindingprofile.gateway/browser_handoff.py::_source_from_context()derives durable handoff routing identity only from theHERMES_SESSION_*context and addssource["profile"]only whenHERMES_SESSION_PROFILEis non-empty. For the API path above it therefore persists an API source with no profile.create_browser_handoff()is still executing inside the worker profile's runtime scope, soBrowserHandoffStore()writes the token row to the worker profile'sstate/browser-handoffs.db. But the public link is constructed fromsource.get("profile"); because that field is absent, the DM contains/browser-handoff/<token>instead of/p/worker/browser-handoff/<token>. The recipient lands on the default-profile route/database, which cannot find the worker-profile token.- Even if the operator manually repairs the URL, the row itself still has no profile.
_deliver_browser_handoff_wake()later callsdeliver_wake(..., profile=str(record.source.get("profile") or "")), so the raw API resume path is also told to target the default profile rather than the profile that owns the paused session.
That breaks two explicit claims of this PR at the same boundary: that handoff tokens are profile-scoped, and that Done/expiry resumes the exact pinned raw API session under multiplexing.
The current tests miss this because they manufacture the identity that production fails to bind: test_secondary_profile_handoff_link_uses_profile_prefix explicitly sets HERMES_SESSION_PROFILE=worker, and test_api_origin_wake_uses_raw_session_without_gateway_lookup constructs a source dict containing "profile": "worker". Neither test traverses the real /p/worker middleware → _bind_api_server_session() → browser_exec(action="handoff") path.
Required repair: make the URL-selected profile a server-owned part of API session context at the admission chokepoint (or carry an equivalent explicit profile capability through the handoff source), rather than relying on ambient runtime-home scoping while leaving HERMES_SESSION_PROFILE blank. Then add a production-path multiplex regression proving all four identities agree: a /p/worker raw API turn creates the row only in worker state, emits a /p/worker/browser-handoff/... URL, Done/expiry resumes under worker, and the default profile cannot consume or wake that capability. Keep the existing single/default-profile behavior unchanged.
Architecture/provenance: #90435 by @ctaylor86 is complementary Desktop Viewer / human-agent ownership work, not a duplicate of this Browser Use cloud lane. #84000 by @SolshineCode is likewise the complementary visible-Chrome human-assist lane. The already-merged extension-controller authority is #91535 by @kshitijk4poor, which explicitly preserves @abundantbeing's #85351 implementation provenance; this PR should remain a distinct cloud/Discord handoff mechanism rather than being treated as superseding that lineage.
Once the profile identity is bound at API-session admission and the integrated multiplex regression closes this path, the rest of the handoff mechanism I checked looks coherent.
Reviewed the 16-file diff (+1951/-113) with focus on What's good
Suggestions
Impressively thorough security posture for a feature whose whole job is handing credentials to a browser. |
What changed
browser_execConfiguration
The HTTPS reverse proxy must route
/browser-handoff/and multiplex/p/<profile>/browser-handoff/paths to the gateway API server. Browser handoff is available only for Browser Use cloud sessions that provideliveUrl; cloud mode is what provides full per-bot Chrome-process isolation and background concurrency.Risk / blast radius
This adds two intentionally API-key-free, bearer-token-authenticated routes. Tokens have 256 bits of randomness, are stored only by SHA-256 digest, expire in at most 30 minutes, are one-shot, profile-scoped, hard-rate-limited, no-store/no-referrer, redacted from access logs, and pinned to one browser and one Hermes session. All other API server routes retain their existing authentication.
Validation
git diff --checkpassedgpt-5.6-solhigh-reasoning review completed; all seven substantive findings were fixed and regression-testedMerge
Owner approval to merge was given in the implementation thread.