diff --git a/client/service_worker/proxy_router.test.ts b/client/service_worker/proxy_router.test.ts new file mode 100644 index 00000000..fbb7bc3c --- /dev/null +++ b/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); +}); diff --git a/client/service_worker/proxy_router.ts b/client/service_worker/proxy_router.ts index 5e5e0367..a3f952d1 100644 --- a/client/service_worker/proxy_router.ts +++ b/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 { 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 { // 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) ); diff --git a/e2e/multi-space-service-worker.test.ts b/e2e/multi-space-service-worker.test.ts new file mode 100644 index 00000000..13a65f69 --- /dev/null +++ b/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 ``, 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 { + 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("