fix(studio-server): reject cross-origin state-changing requests - #6128

Thanks for picking this up, the threat is real and the tests go through the actual API object, which is the right way in. One gap: the guard only asks whether the Origin host equals the Host header, so a page whose DNS points its own name at 127.0.0.1 sends a matching pair and gets through. The server already has the missing half in isLoopbackHost and identityAllowed, and sameOriginPost in the CLI server applies both for the open-in-app route, including a stricter Sec-Fetch-Site rule. Could this reuse that helper, lifted somewhere both can import, so there is one answer to "is this write same origin" instead of two that disagree?

Also worth knowing: no test or lint workflow has run on this PR yet, only the mergeability check.

Reviewed at 110a300aef. The tightening is real and I could not find a way around it, but as written it also blocks every write in setups we document as supported, so I am requesting changes on that.

What I verified works. Applied to current main (it merges cleanly): the 11 guard tests pass, the full studio-server suite passes (1381), the CLI server suites pass (229), and the Studio tests that drive createStudioApi through a real fetch pass. I also probed sameOriginPost directly. On a default loopback preview, a cross-site POST, a form or beacon POST with Origin: null, a same-site request from another localhost port, and a rebinding page whose Origin and Host agree on a non-loopback name are all refused, while the Studio page itself is allowed. Every GET route in createStudioApi is a read, so skipping GET, HEAD and OPTIONS does not leave a write open. The CLI forwards /api/* with new Request(..., { headers: c.req.raw.headers }), and I checked that Node keeps Host, Origin and Sec-Fetch-Site through that, so the guard sees the real headers. Miguel's rebinding point from the first revision is resolved by reusing sameOriginPost.

The blocker: remote and proxied setups lose all writes. docs/packages/studio-server.mdx says Studio is meant to work on non-loopback origins, naming Codespaces, tunnels and access proxies, and the package is documented for hosts mounting createStudioApi in their own server. sameOriginPost was written for the telemetry identity and the desktop button, where failing closed only drops an id or hides a button. As a write guard it fails closed on all of those setups:

• identityAllowed refuses any non-loopback Host unless the CLI's HYPERFRAMES_PREVIEW_HOST names it, so a Codespaces or tunnel hostname, or a third-party host's own domain, is refused.
• The last line compares against http://${host}, so any HTTPS origin fails even when the host matches. I probed https://localhost:3002 with Host localhost:3002, a tunnel that rewrites Host to localhost, and a Codespaces-style public name. All three return false, and with HYPERFRAMES_PREVIEW_HOST=0.0.0.0 the Codespaces case is still false.

So after this lands, editing in Studio behind any HTTPS proxy returns 403 on every save, and a library user has to set a CLI env var just to make writes work.

A shape that keeps the protection without that breakage: compare Origin to the request's own origin in a scheme-aware way (accept https:// when the request arrived over TLS or via a trusted forwarded proto), and keep the Host allowlist where the bind is loopback, which is exactly where DNS rebinding is the threat. For the library, either make the allowed hosts an option on createStudioApi with this as the default for loopback, or apply the guard in the CLI's /api/* forwarder, where HYPERFRAMES_PREVIEW_HOST already means something. Either way, a test for an HTTPS same-origin write would pin it.

Also needed: the comment ratchet fails. node scripts/comment-ratchet.mjs on the merge result reports 5 problems: the 22-line block in createStudioApi.ts (share 3.5% to 19.2%, plus a new block over 12 lines), the new requestIdentity.ts at 39% against the package's 7% with two long blocks, and one added line in desktopRoutes.ts. Most of the requestIdentity.ts prose is moved, but the ratchet grades it as a new file. Cutting the middleware comment to the why in four lines, and trimming the moved docs, should clear it.

Need: yes, unauthenticated writes from any website are a real hole · Size: ok · Reuse: good, one shared sameOriginPost instead of two answers · Simpler: the guard is already small; the re-export shims in desktopRoutes.ts and telemetryIdentity.ts could become direct imports in their tests

The probe I used is plain sameOriginPost({ host, origin, fetchSite }) calls on the built dist/helpers/requestIdentity.js, if you want to reproduce.

— Rames

Thanks for working through both rounds. The HTTPS case and the library case from the last review are fixed: originMatches now follows the request scheme and X-Forwarded-Proto, allowedHosts lets a host mounting the API on its own domain accept writes, the docs explain it, and the comment check passes now.

One part of the earlier blocker is still open: the CLI itself. studioServer.ts calls createStudioApi(adapter) with no options, and nothing lets a hyperframes preview user pass allowedHosts. Behind Codespaces port forwarding or a tunnel the browser's origin is the public name, and hostAllowed refuses it whether the proxy keeps it in Host or moves it to X-Forwarded-Host. I called sameOriginPost with both header shapes and got false each time without allowedHosts, which is what your own test "rejects a write on that host without the allowedHosts option" pins. Main accepts those writes today, so every save in those setups would start returning 403. The Studio dev server in packages/studio/vite.config.ts has the same gap. A CLI flag or env var that feeds allowedHosts, documented next to HYPERFRAMES_PREVIEW_HOST, plus a test through the CLI's /api/* forwarder, would close it.

Smaller things:

• X-Forwarded-Host is trusted from any client, so a rebinding page can set it to localhost:<port> on its own same-origin request and get past the Host check; the Origin comparison is what stops it, since browsers always send Origin on writes. A test that a forged X-Forwarded-Host with a non-matching Origin is refused would keep that from regressing.
• The PR description still describes the first version (Origin vs Host, no docs change, 8 tests). Worth updating to cover allowedHosts, the forwarded headers and the new ./request-identity subpath.

No test workflows have run on this branch yet; they are waiting for a maintainer to start them.

Verdict: REQUEST_CHANGES
Reasoning: The CLI and Studio dev server still have no way to accept writes from a proxied origin, so hyperframes preview behind Codespaces or a tunnel would 403 on every save where main works.

Reviewed at 27193bb105900a293f571087df3d466827e5e3ca.

The latest head resolves the earlier HTTPS and tunnel regressions. Both CLI and Studio dev hosts pass the documented allowed-host list into createStudioApi, while the shared guard still refuses cross-site and rebinding writes.

5 actual sameOriginPost helper controls pass: loopback success, cross-site refusal, rebinding refusal, allowed HTTPS tunnel success, mismatched forwarded Origin refusal. Full originGuard suite blocked at collection by borrowed core missing ./safe-path. Security hardening, not authentication: browser Host checks and same-origin checks protect local writes, local clients remain trusted. No live tunnel test. Six unsigned commits remain; root independent security pass required.

Need: yes · Size: ok · Reuse: existing implementation seam · Simpler: already minimal · Structure: fine
Risk: medium - local write-origin trust boundary.

Verdict: APPROVE
Reasoning: The latest head resolves the earlier HTTPS and tunnel regressions. Both CLI and Studio dev hosts pass the documented allowed-host list into createStudioApi, while the shared guard still refuses cross-site and rebinding writes.

Reasoning: The latest head resolves the earlier HTTPS and tunnel regressions. Both CLI and Studio dev hosts pass the documented allowed-host list into createStudioApi, while the shared guard still refuses cross-site and rebinding writes.