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

packages/cli/src/server/desktopRoutes.ts

@@ -5,21 +5,13 @@ import {
   desktopInstalled,
   openInDesktop,
 } from "../utils/desktopApp.js";
-import { identityAllowed } from "./telemetryIdentity.js";
+import { sameOriginPost } from "@hyperframes/studio-server/request-identity";
 
-const CTA_ID = /^[A-Za-z0-9-]{1,64}$/;
+// Re-exported for this module's tests; the definition is shared with
+// the studio API's write guard.
+export { sameOriginPost };
 
-/** A bodiless POST is a simple request, so any page can send one to localhost: it has to come from this Studio. */
-export function sameOriginPost(headers: {
-  host?: string;
-  origin?: string;
-  fetchSite?: string;
-}): boolean {
-  const { host, origin, fetchSite } = headers;
-  if (!host || !identityAllowed(host)) return false;
-  if (fetchSite && fetchSite !== "same-origin") return false;
-  return origin === undefined || origin === `http://${host}`;
-}
+const CTA_ID = /^[A-Za-z0-9-]{1,64}$/;
 
 /** The desktop-app button's route: show it or not, the download, and whether the app here takes the project. */
 export function mountDesktopRoutes(

packages/cli/src/server/telemetryIdentity.ts

@@ -16,7 +16,7 @@
 // server's heavy render dependencies (@hyperframes/producer, engine, …).
 // ---------------------------------------------------------------------------
 
-import { hostname, networkInterfaces } from "node:os";
+import { identityAllowed, isLoopbackHost } from "@hyperframes/studio-server/request-identity";
 import { readConfig, readConfigFresh } from "../telemetry/config.js";
 import {
   resetTelemetryPostureCache,
@@ -25,6 +25,12 @@ import {
 import { canaryDecisionsForStudio, type CliCanaryDecision } from "../telemetry/canary.js";
 import { detectAgentRuntime } from "../telemetry/agent_runtime.js";
 
+// The host-identity half of this module moved to
+// @hyperframes/studio-server/request-identity, where the studio API's
+// write guard shares the same definition. Re-exported so this module's
+// existing importers keep one source for it.
+export { identityAllowed, isLoopbackHost };
+
 /**
  * The CLI's anonymous distinct id to hand to Studio, or null when CLI telemetry
  * is disabled or no id is available. Fail-silent — telemetry must never break
@@ -58,33 +64,6 @@ function resolveCliBucketSeed(): string | null {
   }
 }
 
-/**
- * Is this request's `Host` a loopback name the studio server could have been
- * reached on directly?
- *
- * A bare `[::1]`/`localhost`/dotted-quad check rather than a full parse: the
- * port is irrelevant (any port on loopback is us), and anything exotic enough
- * to miss here should be refused rather than guessed at.
- *
- * Scope, stated precisely: this is a **DNS-rebinding mitigation for browsers**,
- * not access control. A browser sets `Host` from the URL it was given, so a
- * page that rebinds its own hostname to 127.0.0.1 arrives carrying that
- * hostname and is refused. A non-browser client sets `Host` to whatever it
- * likes, so this stops nothing there — but on a loopback-bound server such a
- * client is already local, and on a LAN-bound one it can read the project
- * files through the unauthenticated studio API anyway. See `identityAllowed`.
- */
-export function isLoopbackHost(host: string | undefined): boolean {
-  if (!host) return false;
-  // Strip the port. IPv6 literals are bracketed (`[::1]:1234`), so take the
-  // bracketed part when present and only split on ":" otherwise.
-  const bracketed = /^\[([^\]]+)\]/.exec(host);
-  const hostname = (bracketed ? bracketed[1] : host.split(":")[0])?.toLowerCase() ?? "";
-  if (hostname === "localhost" || hostname === "::1" || hostname === "0:0:0:0:0:0:0:1") return true;
-  // 127.0.0.0/8 — the whole loopback block, not just 127.0.0.1.
-  return /^127\.\d{1,3}\.\d{1,3}\.\d{1,3}$/.test(hostname);
-}
-
 // JSON.stringify does not escape "<" or "/". Escaping both means no
 // "</script>" (or "</…") sequence can form in the emitted value, so it can
 // never terminate the inline <script> or open a new tag. (The values are
@@ -197,96 +176,6 @@ export function buildStudioHeadScripts(
   return `${buildCliIdentityScript(options)}${envScript}`;
 }
 
-/**
- * The `<head>` scripts for a request, given its `Host`.
- *
- * The Host split lives here rather than in the route so it is testable
- * without a Studio bundle on disk — the route's own test can only reach the
- * injection branch when `packages/studio/dist` happens to be built, which is
- * true locally and false in the CI test lane.
- */
-/** Hostname of a `Host` header, port and IPv6 brackets removed. */
-function hostnameOf(host: string | undefined): string {
-  if (!host) return "";
-  const bracketed = /^\[([^\]]+)\]/.exec(host);
-  return (bracketed ? bracketed[1] : host.split(":")[0])?.toLowerCase() ?? "";
-}
-
-/**
- * Names this machine legitimately answers to on a wildcard bind: every
- * interface address, plus its own hostname and the `.local` mDNS form people
- * actually type. Fail closed — a throw yields an empty set, which denies.
- */
-function localNames(): Set<string> {
-  const names = new Set<string>();
-  try {
-    for (const entries of Object.values(networkInterfaces())) {
-      for (const entry of entries ?? []) names.add(entry.address.toLowerCase());
-    }
-    const self = hostname().toLowerCase();
-    if (self !== "") {
-      names.add(self);
-      // `my-box` is reachable as `my-box.local`, and a `my-box.lan.example`
-      // FQDN is reachable by its short form. Register both directions.
-      names.add(`${self}.local`);
-      const short = self.split(".")[0];
-      if (short !== undefined && short !== "") {
-        names.add(short);
-        names.add(`${short}.local`);
-      }
-    }
-  } catch {
-    /* fail closed */
-  }
-  return names;
-}
-
-/**
- * Wildcard binds answer on every local name; a specific bind on itself only.
- *
- * This is the part that was missing: the previous rule accepted ANY Host once
- * the env var was set, so a rebinding page could name itself `evil.example.com`
- * and still be handed the CLI identity.
- */
-function hostMatchesBind(host: string | undefined, bind: string): boolean {
-  const requested = hostnameOf(host);
-  if (requested === "") return false;
-  const bound = bind.toLowerCase();
-  if (requested === bound) return true;
-  return bound === "0.0.0.0" || bound === "::" || bound === "*"
-    ? localNames().has(requested)
-    : false;
-}
-
-/**
- * May this request receive the CLI's identity (distinct id + bucket seed)?
- *
- * The server binds loopback by DEFAULT and exposes the LAN only when an
- * operator sets `HYPERFRAMES_PREVIEW_HOST` (portUtils.ts, F-001). A loopback
- * `Host` is always fine: whatever reached us came via loopback, and the only
- * interesting attacker is a rebinding browser page — which the Host check
- * catches, because a browser cannot forge `Host`.
- *
- * For anything else the bind decides:
- *
- *  - **Unset, or bound to loopback.** No LAN exposure was requested, so the
- *    Host check is a live rebinding mitigation and a non-loopback Host is
- *    refused. Previously ANY non-empty value disabled the check, so even
- *    `HYPERFRAMES_PREVIEW_HOST=127.0.0.1` — which exposes nothing — turned the
- *    loopback service from hostile-Host refusal into accept-everything.
- *  - **Bound to a LAN address.** The operator opted into exposure, so identity
- *    has to reach the LAN name the user browses or the CLI-to-Studio stitch
- *    breaks and Studio mints a second person for the same human. But it is
- *    still checked: the Host must name an address this machine actually
- *    answers on, not any attacker-chosen name a rebinding page supplies.
- */
-export function identityAllowed(host: string | undefined): boolean {
-  if (isLoopbackHost(host)) return true;
-  const bind = (process.env["HYPERFRAMES_PREVIEW_HOST"] ?? "").trim();
-  if (bind === "" || isLoopbackHost(bind)) return false;
-  return hostMatchesBind(host, bind);
-}
-
 /**
  * Re-read the persisted telemetry preference for this request.
  *
@@ -309,6 +198,14 @@ export function refreshTelemetryPosture(): void {
   }
 }
 
+/**
+ * The `<head>` scripts for a request, given its `Host`.
+ *
+ * The Host split lives here rather than in the route so it is testable
+ * without a Studio bundle on disk — the route's own test can only reach the
+ * injection branch when `packages/studio/dist` happens to be built, which is
+ * true locally and false in the CI test lane.
+ */
 export function buildStudioHeadScriptsForHost(envScript: string, host: string | undefined): string {
   refreshTelemetryPosture();
   return buildStudioHeadScripts(envScript, { includeIdentity: identityAllowed(host) });

packages/studio-server/package-subpaths.json

@@ -26,6 +26,12 @@
       "types": "./dist/helpers/safePath.d.ts",
       "environments": ["bun", "node"]
     },
+    "./request-identity": {
+      "source": "./src/helpers/requestIdentity.ts",
+      "runtime": "./dist/helpers/requestIdentity.js",
+      "types": "./dist/helpers/requestIdentity.d.ts",
+      "environments": ["bun", "node"]
+    },
     "./hf-ids": {
       "source": "./src/helpers/hfIdPersist.ts",
       "runtime": "./dist/helpers/hfIdPersist.js",

packages/studio-server/package.json

@@ -33,6 +33,12 @@
       "import": "./src/helpers/safePath.ts",
       "types": "./src/helpers/safePath.ts"
     },
+    "./request-identity": {
+      "bun": "./src/helpers/requestIdentity.ts",
+      "node": "./dist/helpers/requestIdentity.js",
+      "import": "./src/helpers/requestIdentity.ts",
+      "types": "./src/helpers/requestIdentity.ts"
+    },
     "./hf-ids": {
       "bun": "./src/helpers/hfIdPersist.ts",
       "node": "./dist/helpers/hfIdPersist.js",
@@ -122,6 +128,10 @@
         "import": "./dist/helpers/safePath.js",
         "types": "./dist/helpers/safePath.d.ts"
       },
+      "./request-identity": {
+        "import": "./dist/helpers/requestIdentity.js",
+        "types": "./dist/helpers/requestIdentity.d.ts"
+      },
       "./hf-ids": {
         "import": "./dist/helpers/hfIdPersist.js",
         "types": "./dist/helpers/hfIdPersist.d.ts"

packages/studio-server/src/createStudioApi.originGuard.test.ts

@@ -0,0 +1,147 @@
+// @vitest-environment node
+// Node environment on purpose: the package default (happy-dom) implements
+// the browser fetch spec, which drops forbidden request headers such as
+// Origin and Sec-Fetch-Site when a Request is constructed. A test that
+// simulates cross-origin browser traffic has to set exactly those headers,
+// so it needs Node's fetch classes, which keep them.
+import { afterEach, describe, expect, it } from "vitest";
+import { existsSync, mkdtempSync, rmSync } from "node:fs";
+import { tmpdir } from "node:os";
+import { join } from "node:path";
+import { createStudioApi } from "./createStudioApi";
+import { stubAdapter } from "./routes/stubAdapter.test-helpers";
+
+const tempDirs: string[] = [];
+
+afterEach(() => {
+  for (const dir of tempDirs.splice(0)) {
+    rmSync(dir, { recursive: true, force: true });
+  }
+});
+
+function createApp(): { app: ReturnType<typeof createStudioApi>; projectDir: string } {
+  const projectDir = mkdtempSync(join(tmpdir(), "hf-origin-guard-"));
+  tempDirs.push(projectDir);
+  return { app: createStudioApi(stubAdapter(projectDir)), projectDir };
+}
+
+const EVIL_ORIGIN = "http://evil.example:9999";
+
+function createFile(
+  app: ReturnType<typeof createStudioApi>,
+  headers: Record<string, string>,
+  name = "planted.html",
+): Promise<Response> {
+  // A real browser request always carries a Host header; app.request does
+  // not derive one in this environment, so send it explicitly. That keeps
+  // every case exercising the Origin comparison itself.
+  return app.request(`http://localhost/projects/demo/files/${name}`, {
+    method: "POST",
+    headers: { Host: "localhost", ...headers },
+    body: "<html>planted</html>",
+  });
+}
+
+describe("createStudioApi cross-origin write guard", () => {
+  it("rejects a cross-origin POST file create and writes nothing", async () => {
+    const { app, projectDir } = createApp();
+    const res = await createFile(app, { Origin: EVIL_ORIGIN, "Content-Type": "text/plain" });
+    expect(res.status).toBe(403);
+    expect(existsSync(join(projectDir, "planted.html"))).toBe(false);
+  });
+
+  it("rejects a cross-origin multipart upload", async () => {
+    const { app, projectDir } = createApp();
+    const form = new FormData();
+    form.append("files", new File(["planted"], "planted.html", { type: "text/html" }));
+    const res = await app.request("http://localhost/projects/demo/upload", {
+      method: "POST",
+      headers: { Host: "localhost", Origin: EVIL_ORIGIN },
+      body: form,
+    });
+    expect(res.status).toBe(403);
+    expect(existsSync(join(projectDir, "planted.html"))).toBe(false);
+  });
+
+  it("rejects a write whose Origin is the opaque literal null", async () => {
+    const { app } = createApp();
+    const res = await createFile(app, { Origin: "null", "Content-Type": "text/plain" });
+    expect(res.status).toBe(403);
+  });
+
+  it("rejects a write marked cross-site by Fetch Metadata even without Origin", async () => {
+    const { app } = createApp();
+    const res = await createFile(app, { "Sec-Fetch-Site": "cross-site" });
+    expect(res.status).toBe(403);
+  });
+
+  it("rejects a cross-origin PUT overwrite", async () => {
+    const { app } = createApp();
+    const res = await app.request("http://localhost/projects/demo/files/planted.html", {
+      method: "PUT",
+      headers: { Host: "localhost", Origin: EVIL_ORIGIN, "If-None-Match": "*" },
+      body: "overwritten",
+    });
+    expect(res.status).toBe(403);
+  });
+
+  it("rejects a write from a DNS name pointing at loopback, even when Origin and Host match", async () => {
+    // The attacker's page is served from a name that resolves to 127.0.0.1,
+    // so its Origin and Host agree with each other and a guard that only
+    // compares the pair lets it through. The shared predicate also requires
+    // the Host to be an identity this server answers to, and
+    // "rebind.example" is not one, whatever it resolves to.
+    const { app, projectDir } = createApp();
+    const res = await createFile(app, {
+      Host: "rebind.example:9999",
+      Origin: "http://rebind.example:9999",
+      "Sec-Fetch-Site": "same-origin",
+      "Content-Type": "text/plain",
+    });
+    expect(res.status).toBe(403);
+    expect(existsSync(join(projectDir, "planted.html"))).toBe(false);
+  });
+
+  it("rejects a write with no Origin when the Host is a DNS name, not a server identity", async () => {
+    const { app, projectDir } = createApp();
+    const res = await createFile(app, { Host: "rebind.example:9999" });
+    expect(res.status).toBe(403);
+    expect(existsSync(join(projectDir, "planted.html"))).toBe(false);
+  });
+
+  it("rejects a same-site write even when Origin and Host match", async () => {
+    const { app } = createApp();
+    const res = await createFile(app, {
+      Origin: "http://localhost",
+      "Sec-Fetch-Site": "same-site",
+      "Content-Type": "text/plain",
+    });
+    expect(res.status).toBe(403);
+  });
+
+  it("allows a same-origin write (Origin host matches Host)", async () => {
+    const { app, projectDir } = createApp();
+    const res = await createFile(app, {
+      Origin: "http://localhost",
+      "Content-Type": "text/plain",
+    });
+    expect(res.status).toBe(201);
+    expect(existsSync(join(projectDir, "planted.html"))).toBe(true);
+  });
+
+  it("allows a write with no Origin header (non-browser callers)", async () => {
+    const { app, projectDir } = createApp();
+    const res = await createFile(app, { "Content-Type": "text/plain" });
+    expect(res.status).toBe(201);
+    expect(existsSync(join(projectDir, "planted.html"))).toBe(true);
+  });
+
+  it("does not block reads from any origin", async () => {
+    const { app } = createApp();
+    await createFile(app, { "Content-Type": "text/plain" });
+    const res = await app.request("http://localhost/projects/demo/files/planted.html", {
+      headers: { Origin: EVIL_ORIGIN },
+    });
+    expect(res.status).toBe(200);
+  });
+});