SW proxy router changes

client/service_worker/proxy_router.test.ts

@@ -0,0 +1,46 @@
+import { expect, test } from "vitest";
+import { belongsToAnotherSpace } from "./proxy_router.ts";
+
+// A space bound at "/" registers its service worker at scope "/", so it
+// receives requests for every *other* space on the origin too. Answering those
+// from its own precache or local data is how `/notes/.client/auth.js` came
+// back as the SPA shell — HTML where a JavaScript module was expected, leaving
+// the login page blank.
+//
+// Paths here are already space-relative (basePathName stripped), so for the
+// root worker they are the full path.
+
+test.each([
+  "/notes/.client/auth.js",
+  "/notes/.client/client.js",
+  "/notes/.auth",
+  "/notes/.fs/index.md",
+  "/notes/.config",
+  "/deeply/nested/prefix/.client/app.css",
+])("%s belongs to another space", (path) => {
+  expect(belongsToAnotherSpace(path)).toBe(true);
+});
+
+test.each([
+  // Our own surfaces sit directly under our base.
+  "/.client/client.js",
+  "/.auth",
+  "/.fs/index.md",
+  "/.config",
+  // Ordinary pages, including ones that merely look like a prefix.
+  "/",
+  "/index",
+  "/some/page",
+  "/notes/subpage",
+  // A page whose name starts with a dot but is not a known surface.
+  "/notes/.hidden",
+  // A dotted segment at the root that is not a server surface either.
+  "/.something-else",
+])("%s is ours to handle", (path) => {
+  expect(belongsToAnotherSpace(path)).toBe(false);
+});
+
+test("a page named like a surface, one level down, is still another space", () => {
+  // `/x/.fs` is unambiguous: no space serves a page called ".fs".
+  expect(belongsToAnotherSpace("/x/.fs")).toBe(true);
+});

client/service_worker/proxy_router.ts

@@ -11,15 +11,51 @@ import {
 import type { SyncEngine } from "./sync_engine.ts";
 import { EventEmitter } from "../plugos/event.ts";
 
-const alwaysProxy = [
-  "/.auth",
-  "/.shell",
-  "/.logout",
-  "/.config",
-  "/.logs",
-  "/.proxy",
-  "/.client/manifest.json",
+// The server surfaces every space carries under its own base path. This worker
+// can answer exactly two of them for its OWN space — `.client` from the
+// install-time precache and `.fs` from locally synced files — and proxies the
+// rest. Both lists below derive from this one, so a new surface is declared
+// once instead of being kept in sync by hand.
+const spaceSurfaces = [
+  "client",
+  "fs",
+  "auth",
+  "config",
+  "logout",
+  "shell",
+  "proxy",
+  "logs",
 ];
+const locallyServed = ["client", "fs"];
+
+// Always straight to the server: the space surfaces we have no local answer
+// for, plus the server-wide ones, which only ever exist at the origin root.
+// Matched as a prefix of the *space-relative* path.
+const alwaysProxy = [
+  ...spaceSurfaces
+    .filter((surface) => !locallyServed.includes(surface))
+    .map((surface) => `/.${surface}`),
+  "/.spaces",
+  "/.setup",
+  "/.instance",
+];
+
+const anotherSpaceSurface = new RegExp(
+  `/[^/]+/\\.(${spaceSurfaces.join("|")})(/|$)`,
+);
+
+/**
+ * Whether a space-relative path belongs to a *different* space on this origin.
+ *
+ * A space bound at "/" registers its worker at scope "/", so it is handed every
+ * sibling space's requests too. A space surface one or more levels down —
+ * "/notes/.client/auth.js" — is by definition not ours, since our own sit
+ * directly under our base. Answering those from our precache is how a sibling's
+ * login page came back as this space's app shell.
+ */
+export function belongsToAnotherSpace(pathname: string): boolean {
+  return anotherSpaceSurface.test(pathname);
+}
 
 export type ProxyRouterEvents = {
   // Use case: the user likely has this file open in the editor, so it's good to prioritize syncing it
@@ -179,6 +215,12 @@ export class ProxyRouter extends EventEmitter<ProxyRouterEvents> {
             return await fetch(request);
           }
 
+          // Another space's surface, reachable only because this worker is
+          // scoped at the origin root. Never ours to answer.
+          if (belongsToAnotherSpace(pathname)) {
+            return await fetch(request);
+          }
+
           // Not yet configured (no sync engine / local storage) — must proxy.
           // No local data exists to fall back to.
           if (!this.localSpacePrimitives || !this.syncEngine) {
@@ -227,7 +269,17 @@ export class ProxyRouter extends EventEmitter<ProxyRouterEvents> {
             // Handle /.fs file system APIs
             return this.handleRequest(pathname, request);
           } else {
-            // Fallback to the app shell for all other requests (SPA)
+            // Fallback to the app shell for all other requests (SPA).
+            if (request.mode === "navigate" && this.online) {
+              try {
+                return await fetch(request);
+              } catch (e: any) {
+                if (e.message !== "Offline" && !isNetworkError(e)) {
+                  throw e;
+                }
+                this.online = false;
+              }
+            }
             return (
               (await caches.match(this.precacheFiles["/"])) || fetch(request)
             );

e2e/multi-space-service-worker.test.ts

@@ -0,0 +1,178 @@
+import { type ChildProcess, execFileSync, spawn } from "node:child_process";
+import { mkdtemp, rm } from "node:fs/promises";
+import { tmpdir } from "node:os";
+import { join } from "node:path";
+import { expect, type Page, test } from "@playwright/test";
+import {
+  ADMIN_PASSWORD,
+  ADMIN_USER,
+  getFreePort,
+  waitForServer,
+} from "./fixtures";
+
+/**
+ * A space bound at "/" registers its service worker at scope "/", which covers
+ * every path on the origin — including the prefixes of *other* spaces. These
+ * tests pin down that such a worker must not answer navigations belonging to a
+ * different space with its own cached app shell: that shell carries the root
+ * space's `<base href="/">`, so the client would boot resolving `.config` and
+ * `/.fs/*` against the server root and silently load the wrong space.
+ *
+ * Unlike the rest of the multi-space e2e suite, these run with the service
+ * worker ENABLED — the bug is unreachable without it.
+ */
+
+const BIN = "./target/debug/silverbullet";
+const CWD = join(import.meta.dirname, "..");
+
+let proc: ChildProcess;
+let rootDir: string;
+let base: string;
+let cookie: string;
+
+/** Call the admin API (nested at `/.spaces/api/admin`) as the logged-in admin. */
+async function admin(method: string, path: string, body?: unknown) {
+  const resp = await fetch(`${base}/.spaces/api/admin/${path}`, {
+    method,
+    headers: { "Content-Type": "application/json", Cookie: cookie },
+    body: body === undefined ? undefined : JSON.stringify(body),
+  });
+  if (!resp.ok) {
+    throw new Error(`${method} ${path} -> ${resp.status} ${await resp.text()}`);
+  }
+  return resp.json();
+}
+
+/** Create a public space so the browser side needs no login. */
+async function createSpace(name: string, prefix: string): Promise<string> {
+  const { id } = await admin("POST", "spaces", {
+    name,
+    folder: join(rootDir, `${name}-space`),
+    binding: { prefix },
+    public: true,
+    seedIndex: true,
+  });
+  return id;
+}
+
+/**
+ * Load the root space and wait until its service worker controls the page —
+ * that worker is what the assertions below guard. Each test gets a fresh
+ * browser context, so this re-registers per test.
+ */
+async function registerRootServiceWorker(page: Page) {
+  await page.goto(`${base}/`);
+  await page.waitForFunction(
+    () => !!navigator.serviceWorker.controller,
+    undefined,
+    {
+      timeout: 30_000,
+    },
+  );
+  // Let the worker finish precaching the app shell before we navigate away.
+  await page.waitForTimeout(2000);
+}
+
+test.beforeAll(async () => {
+  rootDir = await mkdtemp(join(tmpdir(), "sb-sw-scope-e2e-"));
+  execFileSync(
+    BIN,
+    ["setup", rootDir, "--admin", `${ADMIN_USER}:${ADMIN_PASSWORD}`],
+    {
+      cwd: CWD,
+      stdio: "pipe",
+    },
+  );
+
+  const port = await getFreePort();
+  proc = spawn(BIN, [rootDir, "-p", String(port), "-L", "127.0.0.1"], {
+    cwd: CWD,
+    stdio: ["ignore", "pipe", "pipe"],
+    // Note: SB_DISABLE_SERVICE_WORKER is deliberately NOT set here.
+    env: { ...process.env, SB_RUNTIME_API: "0" },
+  });
+  base = `http://127.0.0.1:${port}`;
+  await waitForServer(`${base}/.spaces`);
+
+  const login = await fetch(`${base}/.spaces/api/login`, {
+    method: "POST",
+    headers: { "Content-Type": "application/json" },
+    body: JSON.stringify({ username: ADMIN_USER, password: ADMIN_PASSWORD }),
+  });
+  cookie = login.headers
+    .getSetCookie()
+    .map((c) => c.split(";")[0])
+    .join("; ");
+
+  // The root-bound space whose service worker shadows the whole origin. Both
+  // tests depend on it, and only one space can hold "/" — so it is created
+  // once here rather than per test.
+  await createSpace("root", "/");
+});
+
+test.afterAll(async () => {
+  proc?.kill();
+  await rm(rootDir, { recursive: true, force: true });
+});
+
+test("a sibling space is not shadowed by the root space's service worker", async ({
+  page,
+}) => {
+  await registerRootServiceWorker(page);
+  await createSpace("sibling", "/sibling");
+
+  await page.goto(`${base}/sibling/`);
+
+  // The decisive assertion: the client must boot against the sibling space's
+  // base URL. When the root worker serves its own cached shell instead, this
+  // is `${base}/` and the client silently loads the root space.
+  expect(await page.evaluate(() => document.baseURI)).toBe(`${base}/sibling/`);
+});
+
+test("a space moved to a different prefix still boots", async ({ page }) => {
+  const id = await createSpace("moved", "/before");
+  await registerRootServiceWorker(page);
+
+  await admin("PATCH", `spaces/${id}`, { binding: { prefix: "/after" } });
+
+  await page.goto(`${base}/after/`);
+
+  expect(await page.evaluate(() => document.baseURI)).toBe(`${base}/after/`);
+});
+
+test("a sibling space's client assets are not answered with the app shell", async ({
+  page,
+}) => {
+  // The root worker's scope covers `/private/.client/*`, but those files are
+  // not in its precache (whose keys are its own space's) and not in its local
+  // data — so it used to fall through to the SPA-shell fallback and answer a
+  // JavaScript module request with HTML. The browser refuses to execute that,
+  // and the login page it belongs to renders blank.
+  //
+  // A private space, because its login page is the one asset request a visitor
+  // makes before authenticating anywhere — the case a user actually hits.
+  await registerRootServiceWorker(page);
+  await admin("POST", "spaces", {
+    name: "private",
+    folder: join(rootDir, "private-space"),
+    binding: { prefix: "/private" },
+    public: false,
+    seedIndex: true,
+  });
+
+  const asset = await page.evaluate(async (url) => {
+    const response = await fetch(url);
+    return {
+      status: response.status,
+      body: (await response.text()).slice(0, 40),
+    };
+  }, `${base}/private/.client/auth.js`);
+
+  expect(asset.status).toBe(200);
+  expect(asset.body).not.toContain("<!doctype");
+
+  // And the page that needs it actually renders its form.
+  await page.goto(`${base}/private/`);
+  await expect(page.locator("#login")).toBeVisible({ timeout: 30_000 });
+  await expect(page.locator("#username")).toBeVisible();
+});