Skip to content

Route browser fs and logs endpoints directly to the VM - #174

Open
tnsardesai wants to merge 5 commits into
mainfrom
hypeship/direct-vm-fs-logs
Open

Route browser fs and logs endpoints directly to the VM#174
tnsardesai wants to merge 5 commits into
mainfrom
hypeship/direct-vm-fs-logs

Conversation

@tnsardesai

@tnsardesai tnsardesai commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Summary

Adds fs and logs/stream to the default KERNEL_BROWSER_ROUTING_SUBRESOURCES prefixes, so every /browsers/{id}/fs/* operation (JSON, binary read/write, multipart upload, watch SSE) and /browsers/{id}/logs/stream is rewritten by the direct-VM middleware to the browser's cached base_url with ?jwt= and no API-key Authorization.

Browser lifecycle/metadata, extensions, replays, telemetry/events, and anything else under logs/ stay on the control plane. KERNEL_BROWSER_ROUTING_SUBRESOURCES still overrides the list, and an empty value still disables routing.

Also fixes the ordering of the stale-JWT path in DirectVMRoutingMiddleware: the route is now evicted before deciding whether the control-plane fallback can run. The GetBody guard still refuses to replay a non-rewindable body (an io.Reader passed to Fs.WriteFile) and surfaces the original 401/403, but the stale route no longer survives that case, so the caller's next call goes to the control plane instead of retrying the same dead JWT.

Tests

browser_routing_test.go:

  • default prefix list includes fs and logs/stream
  • fs JSON (ListFiles), binary ReadFile, streamed WriteFile, multipart Upload (dot-indexed files.0.… names, which the VM accepts), fs/watch/{id}/events SSE and logs/stream SSE route to the VM with the query preserved and no Authorization header
  • logs/stream context cancellation reaches the server's request context
  • stale-JWT fallback on Fs.Upload replays the multipart body byte for byte on the control plane with bearer auth and evicts the route
  • env override keeps fs/logs on the control plane
  • control-plane subresources (telemetry/events, replays) keep the API origin

lib/browserrouting/route_cache_test.go:

  • allowlist matching table extended with fs/read_file, fs/watch/{id}/events, logs/stream, logs/stream/x routing to the VM and fsx/..., logs, logs/history, logstream, extensions, replays/... staying on the control plane
  • the two "keeps auth response when the body cannot rewind" tests now also assert the stale route is evicted

Ran locally: go test ./... (all packages pass), ./scripts/lint (go build ./... plus test compile), gofmt -s -l . clean. The integration test in lib/browserrouting still skips without KERNEL_API_KEY/KERNEL_BASE_URL.

Live validation

Ran against staging with real headless browsers: Fs.WriteFile, Fs.ReadFile, Fs.ListFiles, Fs.Upload (two entries), Fs.Watch.Start/EventsStreaming/Stop and Logs.StreamStreaming all hit https://<browser-host>/browser/kernel/...?jwt=... with no Authorization header and returned the expected data, with uploaded files reading back with the correct per-entry contents. telemetry/events and the browser delete stayed on the control plane. Re-validated after switching the default to logs/stream.


Note

Medium Risk
Changes default HTTP routing for filesystem and log streaming plus auth-fallback ordering; misconfiguration could send traffic to the wrong host or briefly surface 401s, though env override and tests mitigate this.

Overview
Default direct-to-VM routing now includes fs (all filesystem APIs) and logs/stream (live log SSE only). Requests are rewritten to the browser base_url with ?jwt= and without the API Authorization header. telemetry/events, other logs/* paths, replays, and extensions still hit the control plane. KERNEL_BROWSER_ROUTING_SUBRESOURCES still overrides or disables routing when set.

Stale JWT handling in DirectVMRoutingMiddleware evicts the cached route before attempting control-plane fallback. When the body cannot be replayed, the original 401/403 is still returned, but the dead route is cleared so the next call goes to the API instead of retrying the same JWT.

Tests cover VM routing for fs (list/read/write/upload/watch) and logs streaming (including context cancel), control-plane-only endpoints, env overrides, multipart fallback on 401, and expanded allowlist matching in route_cache_test.go.

Reviewed by Cursor Bugbot for commit 507abd2. Bugbot is set up for automated code reviews on this repo. Configure here.

Add fs and logs to the default direct-to-VM subresource prefixes so
filesystem operations and log streaming use the cached browser base_url
and JWT instead of the control plane.

Evict a stale direct-to-VM route before deciding whether the control-plane
fallback can run. A request without GetBody still cannot be replayed, but
the caller's next call now goes to the control plane instead of retrying
the same stale JWT.
Narrows the default prefix so a future logs read served by the control
plane is not swept onto the browser VM.
@tnsardesai
tnsardesai marked this pull request as ready for review September 2, 2026 20:57
@tnsardesai
tnsardesai requested review from Sayan- and rgarcia September 2, 2026 21:19
rgarcia
rgarcia previously approved these changes Sep 3, 2026

@rgarcia rgarcia left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

reviewed alongside the node (#178) and python (#164) siblings, including a control-plane vs kernel-images surface comparison. all 16 fs/* and logs/stream endpoints match on params, body fields, 2xx codes and content types, the control-plane handlers were pure pass-throughs, and metro-api's /browser/kernel/* handler already wakes standby VMs and records session activity for direct requests, so the routing change itself looks safe. the eviction reorder is correct and DeleteIfJWT keeps it safe against a concurrent refresh. tests pass locally, including 10 runs of the SSE cancellation test under -race.

Nits

  • browser_routing.go:89 — the comment above only explains the telemetry split; add a line for logs/stream vs other logs/* and note fs is intentionally a bare prefix
  • browser_routing_test.go:665 — fallback server returns 204 for fs/upload; production returns 201

Sayan-
Sayan- previously approved these changes Sep 3, 2026

@Sayan- Sayan- left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving. Verified against a mock control plane and VM over real HTTP: fs/* and logs/stream route to the VM with ?jwt= and no Authorization, telemetry/events and replays stay on the control plane with bearer auth, query strings and binary bodies survive intact, and the env override pushes everything back. Dot-indexed files.0.dest_path / files.0.file is accepted by the in-VM handler as claimed. The eviction reorder holds under test: a stale-JWT upload with a rewindable body replays byte for byte on the control plane with bearer auth, and a non-rewindable io.Reader surfaces the original failure with the route evicted so the next call recovers.

  • p2: one asymmetry that only shows up when the three SDKs are tested side by side. This fallback is independent of retry config, while the Python SDK's runs through its retry machinery and therefore does not fall back at all when max_retries=0. Nothing to change here; it matters if the three are ever documented as behaving identically.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants