From 1f4fa66b502bcd9da4a87aa41f8078fff4093e49 Mon Sep 17 00:00:00 2001 From: Koji Wakayama Date: Wed, 5 Aug 2026 14:58:50 +0200 Subject: [PATCH 1/2] fix(security): honor the host-execution capability on every renderer surface MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes veryfront-issue-inbox#366. Fixes veryfront-issue-inbox#376. Markdown preview, component snippets and the legacy module endpoints still gated on bare `isSharedProjectRuntime(ctx)`, so they denied tenant rendering to hosts that had been granted `allowHostProjectCodeExecution`. On staging that surfaced as an authenticated markdown preview returning 503 project-execution-unavailable — a runtime it told the operator to route to a dedicated isolated runtime that does not exist. All three now ask what SSR and the API surfaces ask: refuse only when the runtime is shared *and* the host did not grant execution. Module and snippet carried code comments citing renderer architecture — the legacy loader importing page code without a prepared module graph. Per the product-owner decision recorded on #366, that is an architecture concern and not a policy one: a host that grants the capability is asserting it is a suitable executor. `rsc/endpoints/endpoint-router.ts` already resolved the identical tension the same way. The comments are updated rather than deleted so the reasoning stays visible. SSR was already correct but spelled the predicate out inline as `isSharedProjectRuntime(ctx) && !isHostProjectCodeExecutionAllowed(ctx)`. That is the helper's exact body, and an inline copy is how surfaces drift apart, so it now calls the helper. Why this went unnoticed, and what now catches it: - Every test on these gates asserted only the denial direction, which a handler hardcoded to deny passes. #3364 shipped exactly such a hardcoded `true` on a sibling surface. Each capability-gated surface now has a paired granted-path test; the module and SSR ones fail against a literal denial, and the snippet denial test had to be corrected too — its context claimed `isLocalProject: true`, which grants the capability, so it had stopped describing a denial at all. - The gate was inert in production for the population it governs. The adapter Proxy fixed in #3378 forwarded `getOwnPropertyDescriptor` to the host adapter, so `isSharedProjectRuntime` answered `false` for remote- filesystem projects. Unit tests pass plain object stubs, never a Proxy, so tests saw the truthful value and production did not. Correcting the Proxy is what exposed the drift. - #3364 said these surfaces "cannot drift apart again", but enforced that by convention — collapsing call sites onto one helper — with nothing checking it. `execution-surface-policy.test.ts` is that check: it inventories every handler's predicate against a registry of capability-gated surfaces and documented non-gate uses, so a wrong predicate or an unlisted new surface fails by construction. Reverting any one surface makes it fail and names the file. It reads source rather than behaviour on purpose — a behavioural sweep only covers surfaces someone remembered to add. `cors.ts` also branches on the narrow predicate but degrades rather than denies, so it is recorded as a documented non-gate use instead of converted. --- .../handlers/execution-surface-policy.test.ts | 143 ++++++++++++++++++ .../preview/markdown-preview.handler.test.ts | 39 ++++- .../preview/markdown-preview.handler.ts | 4 +- .../request/module/module.handler.test.ts | 35 +++++ .../handlers/request/module/module.handler.ts | 15 +- .../handlers/request/snippet.handler.test.ts | 59 +++++++- .../handlers/request/snippet.handler.ts | 4 +- .../handlers/request/ssr/ssr.handler.test.ts | 31 ++++ .../handlers/request/ssr/ssr.handler.ts | 7 +- 9 files changed, 320 insertions(+), 17 deletions(-) create mode 100644 src/server/handlers/execution-surface-policy.test.ts diff --git a/src/server/handlers/execution-surface-policy.test.ts b/src/server/handlers/execution-surface-policy.test.ts new file mode 100644 index 0000000000..77f7982860 --- /dev/null +++ b/src/server/handlers/execution-surface-policy.test.ts @@ -0,0 +1,143 @@ +import "#veryfront/schemas/_test-setup.ts"; +import { assertEquals } from "#veryfront/testing/assert.ts"; +import { describe, it } from "#veryfront/testing/bdd.ts"; +import { fromFileUrl } from "#veryfront/compat/path"; + +/** + * Guards the shared-runtime execution boundary against silent drift. + * + * Every surface that refuses to run tenant project code must ask the same + * question: `requiresIsolatedProjectRuntime(ctx)` — refuse only when the + * runtime is shared *and* the host did not grant execution. Asking the + * narrower `isSharedProjectRuntime(ctx)` instead denies hosts that were + * explicitly granted the capability. + * + * That drift has now happened twice. veryfront-code#3364 converted three + * surfaces and left three behind; the survivors were invisible because the + * adapter Proxy fixed in #3378 made the predicate answer `false` for exactly + * the remote-filesystem projects it governs, so the gate never fired anywhere + * it was wrong. Once the Proxy was corrected, markdown preview started + * returning 503 on staging (veryfront-issue-inbox#376, #366). + * + * Per-handler tests cannot catch this: each one is individually consistent. + * Only an inventory across surfaces can, so this test is the inventory. It + * reads source rather than behaviour deliberately — a behavioural sweep can + * only cover the surfaces someone remembered to add to it, whereas an + * unlisted file here is a failure by construction. + */ + +const HANDLERS_DIR = fromFileUrl(new URL(".", import.meta.url)); + +/** Surfaces that gate tenant code execution. These must honour the capability. */ +const CAPABILITY_GATED_SURFACES = [ + "preview/markdown-preview.handler.ts", + "request/api/api-handler-wrapper.ts", + "request/api/app-router-handler.ts", + "request/api/project-discovery.ts", + "request/module/module.handler.ts", + "request/snippet.handler.ts", + "request/ssr/ssr.handler.ts", +].toSorted(); + +/** + * Files that legitimately read the narrower predicate because they are not + * execution gates. Each needs a reason, because "it compiles" is how the + * original drift got in. + */ +const NON_GATE_USES: Record = { + "response/cors.ts": + "Chooses which CORS methods to advertise. Degrades to defaults on a shared runtime rather than denying, so the capability does not apply.", +}; + +async function readHandlerSources(): Promise> { + const sources = new Map(); + + async function walk(relativeDir: string): Promise { + for await (const entry of Deno.readDir(`${HANDLERS_DIR}${relativeDir}`)) { + const relativePath = `${relativeDir}${entry.name}`; + if (entry.isDirectory) { + await walk(`${relativePath}/`); + continue; + } + if (!entry.name.endsWith(".ts") || entry.name.endsWith(".test.ts")) continue; + if (entry.name.endsWith(".test-helpers.ts")) continue; + sources.set(relativePath, await Deno.readTextFile(`${HANDLERS_DIR}${relativePath}`)); + } + } + + await walk(""); + return sources; +} + +/** Ignore the import statement so only real call sites count. */ +function callsPredicate(source: string, predicate: string): boolean { + return new RegExp(`(? !line.trim().startsWith("import ")).join("\n"), + ); +} + +describe("server/handlers shared-runtime execution boundary", () => { + it("gates every execution surface on the capability, not on sharedness alone", async () => { + const sources = await readHandlerSources(); + const drifted: string[] = []; + + for (const [path, source] of sources) { + if (!callsPredicate(source, "isSharedProjectRuntime")) continue; + if (path in NON_GATE_USES) continue; + drifted.push(path); + } + + assertEquals( + drifted.toSorted(), + [], + `These files call isSharedProjectRuntime() directly. If a file gates tenant code ` + + `execution it must call requiresIsolatedProjectRuntime() instead, so a host that ` + + `was granted allowHostProjectCodeExecution is served. If it is not an execution ` + + `gate, add it to NON_GATE_USES with a reason.`, + ); + }); + + it("keeps the capability-gated inventory accurate", async () => { + const sources = await readHandlerSources(); + + const missing = CAPABILITY_GATED_SURFACES.filter((path) => { + const source = sources.get(path); + return !source || !callsPredicate(source, "requiresIsolatedProjectRuntime"); + }); + + assertEquals( + missing, + [], + `These surfaces are listed as capability-gated but no longer call ` + + `requiresIsolatedProjectRuntime(). Either restore the call or remove the entry ` + + `deliberately — silently dropping the gate is how a surface stops being enforced.`, + ); + + const unlisted = [...sources.keys()] + .filter((path) => callsPredicate(sources.get(path)!, "requiresIsolatedProjectRuntime")) + .filter((path) => !CAPABILITY_GATED_SURFACES.includes(path)) + .toSorted(); + + assertEquals( + unlisted, + [], + `New execution surfaces found. Add them to CAPABILITY_GATED_SURFACES and give each ` + + `a paired fail-closed and granted-path test — a fail-closed test alone cannot ` + + `distinguish a correct predicate from a hardcoded denial.`, + ); + }); + + it("documents why each non-gate use of the narrow predicate is safe", async () => { + const sources = await readHandlerSources(); + const stale = Object.keys(NON_GATE_USES).filter((path) => { + const source = sources.get(path); + return !source || !callsPredicate(source, "isSharedProjectRuntime"); + }); + + assertEquals( + stale, + [], + "NON_GATE_USES lists files that no longer call isSharedProjectRuntime(). Remove them.", + ); + }); +}); diff --git a/src/server/handlers/preview/markdown-preview.handler.test.ts b/src/server/handlers/preview/markdown-preview.handler.test.ts index 39a9e55775..2bc49b0f64 100644 --- a/src/server/handlers/preview/markdown-preview.handler.test.ts +++ b/src/server/handlers/preview/markdown-preview.handler.test.ts @@ -1,5 +1,5 @@ import "#veryfront/schemas/_test-setup.ts"; -import { assertEquals } from "#veryfront/testing/assert.ts"; +import { assertEquals, assertNotEquals } from "#veryfront/testing/assert.ts"; import type { HandlerContext } from "../types.ts"; import { describe, it } from "#veryfront/testing/bdd.ts"; import { MarkdownPreviewHandler } from "./markdown-preview.handler.ts"; @@ -99,6 +99,43 @@ Deno.test("MarkdownPreviewHandler fails closed before shared source reads", asyn assertEquals(reads, 0); }); +Deno.test("MarkdownPreviewHandler renders once the host grants execution", async () => { + // The granted counterpart of the shared-runtime denial above. #3364 collapsed + // the execution surfaces onto requiresIsolatedProjectRuntime so they could not + // drift apart, but markdown preview kept a bare isSharedProjectRuntime check + // and denied unconditionally. Without this case, an unconditional denial here + // is indistinguishable from a correct fail-closed guard. + let reads = 0; + const ctx = { + projectDir: "/remote/project", + isLocalProject: false, + requestContext: { mode: "preview" }, + adapter: { + fs: { + isMultiProjectMode: () => true, + readFile: () => { + reads++; + throw new Error("not found"); + }, + }, + }, + securityConfig: null, + cspUserHeader: null, + allowHostProjectCodeExecution: true, + } as unknown as HandlerContext; + + const result = await new MarkdownPreviewHandler().handle( + new Request("https://tenant.example/README.md"), + ctx, + ); + + assertNotEquals( + result.response?.status, + 503, + "a granted shared executor must not return project-execution-unavailable", + ); +}); + Deno.test("MarkdownPreviewHandler admits and reads through a real wrapped GitHub adapter", async () => { const originalFetch = globalThis.fetch; let contentReads = 0; diff --git a/src/server/handlers/preview/markdown-preview.handler.ts b/src/server/handlers/preview/markdown-preview.handler.ts index 2336b9a7d7..96794c6225 100644 --- a/src/server/handlers/preview/markdown-preview.handler.ts +++ b/src/server/handlers/preview/markdown-preview.handler.ts @@ -16,7 +16,7 @@ import { extract } from "#std/front-matter/yaml.ts"; import { tryNotFoundFallback } from "../request/ssr/not-found-fallback.ts"; import { generateMarkdownHtml } from "./markdown-html-generator.ts"; import { validateLexicalPath, validatePath, ValidationPresets } from "#veryfront/security"; -import { isSharedProjectRuntime } from "#veryfront/security/project-locality.ts"; +import { requiresIsolatedProjectRuntime } from "#veryfront/security/project-locality.ts"; import { createErrorResponseFromDefinition, PROJECT_EXECUTION_UNAVAILABLE, @@ -48,7 +48,7 @@ export class MarkdownPreviewHandler extends BaseHandler { return this.continue(); } - if (isSharedProjectRuntime(ctx)) { + if (requiresIsolatedProjectRuntime(ctx)) { const problem = createErrorResponseFromDefinition( PROJECT_EXECUTION_UNAVAILABLE, { diff --git a/src/server/handlers/request/module/module.handler.test.ts b/src/server/handlers/request/module/module.handler.test.ts index 1406d94b0e..b943c67bdd 100644 --- a/src/server/handlers/request/module/module.handler.test.ts +++ b/src/server/handlers/request/module/module.handler.test.ts @@ -257,6 +257,41 @@ describe("server/handlers/request/module/module.handler", () => { assertEquals(rendererCalls, 0); }); + it("serves the endpoints once the host grants execution", async () => { + // The granted counterpart to the fail-closed test above. A handler that + // denies every shared runtime unconditionally — which is what this + // surface did before veryfront-issue-inbox#366 — passes that test and + // fails this one. Without the pair, the two are indistinguishable. + const handler = new ModuleHandler(); + for ( + const pathname of [ + "/_veryfront/modules/runtime.js", + "/_veryfront/pages/page.js", + "/_veryfront/data/page.json", + "/_veryfront/page-data/page.json", + ] + ) { + const result = await handler.handle( + new Request(`https://tenant.example${pathname}`), + makeCtx({ + isLocalProject: false, + allowHostProjectCodeExecution: true, + } as Partial), + ); + const type = result.response + ? await result.response.clone().json().then( + (body: { type?: string }) => body.type, + () => undefined, + ) + : undefined; + assertEquals( + type === "https://veryfront.com/docs/errors/project-execution-unavailable", + false, + `${pathname} denied execution to a granted host`, + ); + } + }); + it("returns an empty fail-closed response for HEAD", async () => { const result = await new ModuleHandler().handle( new Request("https://tenant.example/_veryfront/page-data/page.json", { diff --git a/src/server/handlers/request/module/module.handler.ts b/src/server/handlers/request/module/module.handler.ts index 8ffbacc1af..7ef48be0d8 100644 --- a/src/server/handlers/request/module/module.handler.ts +++ b/src/server/handlers/request/module/module.handler.ts @@ -16,7 +16,7 @@ import { createErrorResponseFromDefinition, PROJECT_EXECUTION_UNAVAILABLE, } from "#veryfront/errors"; -import { isSharedProjectRuntime } from "#veryfront/security/project-locality.ts"; +import { requiresIsolatedProjectRuntime } from "#veryfront/security/project-locality.ts"; const MODULE_ENDPOINT_PREFIXES = [ "/_vf_modules/", @@ -69,11 +69,16 @@ export class ModuleHandler extends BaseHandler { } // These endpoints delegate to the legacy renderer, whose module loader - // imports page and layout code in the host process. Remote source must not - // reach that path until rendering has a generation-owned prepared module - // graph equivalent to isolated API routes. + // imports page and layout code in the host process rather than through a + // generation-owned prepared module graph. + // + // That is a renderer-architecture concern, not a policy one, so it does not + // decide who may execute tenant code: the host-execution capability does. + // A host that grants the capability is asserting it is a suitable executor, + // and this surface honours that like every other. `rsc/endpoints/ + // endpoint-router.ts` already resolved the identical tension the same way. if ( - isSharedProjectRuntime(ctx) && + requiresIsolatedProjectRuntime(ctx) && HOST_RENDERER_ENDPOINT_PREFIXES.some((prefix) => pathname.startsWith(prefix)) ) { const problem = createErrorResponseFromDefinition( diff --git a/src/server/handlers/request/snippet.handler.test.ts b/src/server/handlers/request/snippet.handler.test.ts index 7b48344a47..cc04f34ff4 100644 --- a/src/server/handlers/request/snippet.handler.test.ts +++ b/src/server/handlers/request/snippet.handler.test.ts @@ -1,5 +1,5 @@ import "#veryfront/schemas/_test-setup.ts"; -import { assertEquals } from "#veryfront/testing/assert.ts"; +import { assertEquals, assertNotEquals } from "#veryfront/testing/assert.ts"; import { describe, it } from "#veryfront/testing/bdd.ts"; import { validateLexicalPath } from "#veryfront/security"; import { SnippetHandler } from "./snippet.handler.ts"; @@ -90,11 +90,16 @@ Deno.test("SnippetHandler rejects shared rendering before proxy context or sourc return Promise.resolve(""); }, }; + // `isLocalProject: false` matters. This context used to say `true`, which + // reads as a denial test but no longer is one: an explicitly local project + // carries the host-execution capability, so the surface is now supposed to + // serve it. Only a shared runtime that was never granted execution belongs + // here. const ctx = { projectDir: "/project", projectSlug: "project", proxyToken: "token", - isLocalProject: true, + isLocalProject: false, adapter: { fs }, } as unknown as HandlerContext; @@ -108,6 +113,56 @@ Deno.test("SnippetHandler rejects shared rendering before proxy context or sourc assertEquals(readPath, undefined); }); +Deno.test("SnippetHandler serves a shared runtime the host granted execution", async () => { + // The granted counterpart to the test above. Without it, a handler that + // simply denies every shared runtime — the pre-#366 behaviour — passes the + // whole suite. + let readPath: string | undefined; + const fs = { + symlinkSemantics: "none" as const, + isMultiProjectMode: () => true, + isContextualMode: () => true, + runWithContext: async ( + _slug: string, + _token: string, + fn: () => Promise, + ) => await fn(), + exists: () => Promise.resolve(true), + stat: () => + Promise.resolve({ + isFile: true, + isDirectory: false, + isSymlink: false, + size: 0, + mtime: new Date(), + }), + readFile: (path: string) => { + readPath = path; + return Promise.resolve("export default function Button() {}\n"); + }, + }; + const ctx = { + projectDir: "/project", + projectSlug: "project", + proxyToken: "token", + isLocalProject: false, + allowHostProjectCodeExecution: true, + adapter: { fs }, + } as unknown as HandlerContext; + + const result = await new SnippetHandler().handle( + new Request("http://localhost/@components/button"), + ctx, + ); + + assertNotEquals( + result.response?.status, + 503, + "a granted shared executor must not return project-execution-unavailable", + ); + assertNotEquals(readPath, undefined, "the granted path must reach the source read"); +}); + Deno.test("SnippetHandler preserves dedicated local rendering", async () => { let readPath: string | undefined; const fs = { diff --git a/src/server/handlers/request/snippet.handler.ts b/src/server/handlers/request/snippet.handler.ts index 946576aff5..a888863d27 100644 --- a/src/server/handlers/request/snippet.handler.ts +++ b/src/server/handlers/request/snippet.handler.ts @@ -12,7 +12,7 @@ import { VeryfrontError, } from "#veryfront/errors"; import { validatePath, ValidationPresets } from "#veryfront/security"; -import { isSharedProjectRuntime } from "#veryfront/security/project-locality.ts"; +import { requiresIsolatedProjectRuntime } from "#veryfront/security/project-locality.ts"; import { createHandlerDependencyPinningSource, getHandlerDependencyPinningIdentity, @@ -37,7 +37,7 @@ export class SnippetHandler extends BaseHandler { return this.continue(); } - if (isSharedProjectRuntime(ctx)) { + if (requiresIsolatedProjectRuntime(ctx)) { const problem = createErrorResponseFromDefinition( PROJECT_EXECUTION_UNAVAILABLE, { diff --git a/src/server/handlers/request/ssr/ssr.handler.test.ts b/src/server/handlers/request/ssr/ssr.handler.test.ts index 162e970fac..e0d5e9ae55 100644 --- a/src/server/handlers/request/ssr/ssr.handler.test.ts +++ b/src/server/handlers/request/ssr/ssr.handler.test.ts @@ -163,6 +163,37 @@ describe("server/handlers/request/ssr/ssr.handler", () => { assertEquals(renderCalls, 0); }); + it("renders once the host grants execution", async () => { + // The granted counterpart to the fail-closed test above. veryfront-code + // #3364 shipped a hardcoded `true` on a sibling surface that survived + // review because a fail-closed test cannot tell a correct predicate from + // a literal denial. Only this direction can. + let renderCalls = 0; + const handler = new SSRHandler(createMockSSRService({ + renderPage: () => { + renderCalls++; + return Promise.resolve({ + status: 200, + html: "granted", + isStreaming: false, + cacheStrategy: "short" as const, + slug: "private-page", + }); + }, + })); + const result = await handler.handle( + new Request("https://tenant.example/private-page"), + makeCtx({ + isLocalProject: false, + allowHostProjectCodeExecution: true, + prepareHostedConfigContext: (() => {}) as HandlerContext["prepareHostedConfigContext"], + } as Partial), + ); + + assertEquals(result.response?.status, 200); + assertEquals(renderCalls, 1); + }); + it("returns response from renderPage result", async () => { const mockService = createMockSSRService({ renderPage: () => diff --git a/src/server/handlers/request/ssr/ssr.handler.ts b/src/server/handlers/request/ssr/ssr.handler.ts index c67683cbff..71b2dbaba8 100644 --- a/src/server/handlers/request/ssr/ssr.handler.ts +++ b/src/server/handlers/request/ssr/ssr.handler.ts @@ -47,10 +47,7 @@ import { createErrorResponseFromDefinition, PROJECT_EXECUTION_UNAVAILABLE, } from "#veryfront/errors"; -import { - isHostProjectCodeExecutionAllowed, - isSharedProjectRuntime, -} from "#veryfront/security/project-locality.ts"; +import { requiresIsolatedProjectRuntime } from "#veryfront/security/project-locality.ts"; const logger = serverLogger.component("ssr"); @@ -104,7 +101,7 @@ export class SSRHandler extends BaseHandler { return Promise.resolve(this.continue()); } - if (isSharedProjectRuntime(ctx) && !isHostProjectCodeExecutionAllowed(ctx)) { + if (requiresIsolatedProjectRuntime(ctx)) { const problem = createErrorResponseFromDefinition( PROJECT_EXECUTION_UNAVAILABLE, { From 30ebae397721cff3778078550c838ac18c170299 Mon Sep 17 00:00:00 2001 From: Koji Wakayama Date: Wed, 5 Aug 2026 15:18:55 +0200 Subject: [PATCH 2/2] fix(server): address review on the execution-surface policy tests - Assert the granted markdown request reaches the source read. Adding the assertion proved the test was weaker than intended: it never got past `withProxyContext`, because the stub filesystem had no `runWithContext` and the context carried no slug or token, so it was passing on "did not 503" alone while the read never happened. The stub now mirrors the snippet one and the read is asserted. - Assert `continue === false` on the granted module endpoints, so a handler that fell through emitting no response cannot pass. - Drop the negative lookbehind from the predicate matcher. It compiles (V8 supports variable-length lookbehind, and the suite ran), but import lines are already stripped before matching, so it was dead weight. - Read the handler tree once, so all three assertions inspect one snapshot. - Use `describe`/`it` for the new tests, and remove em dashes from `.ts` sources, per the coding guidelines. Also stop leaking a perf-timer entry in the SSR handler. `startRequest` registered a timings entry before the hidden-route and fail-closed guards, neither of which calls `endRequest`. Moving the call below the guards fixes it; adding `endRequest` to those paths would not, because it returns early without deleting when no timer ever ran. --- .../handlers/execution-surface-policy.test.ts | 39 +++++--- .../preview/markdown-preview.handler.test.ts | 86 +++++++++++------ .../request/module/module.handler.test.ts | 14 ++- .../handlers/request/snippet.handler.test.ts | 94 ++++++++++--------- .../handlers/request/ssr/ssr.handler.ts | 7 +- 5 files changed, 144 insertions(+), 96 deletions(-) diff --git a/src/server/handlers/execution-surface-policy.test.ts b/src/server/handlers/execution-surface-policy.test.ts index 77f7982860..4afd69de55 100644 --- a/src/server/handlers/execution-surface-policy.test.ts +++ b/src/server/handlers/execution-surface-policy.test.ts @@ -7,7 +7,7 @@ import { fromFileUrl } from "#veryfront/compat/path"; * Guards the shared-runtime execution boundary against silent drift. * * Every surface that refuses to run tenant project code must ask the same - * question: `requiresIsolatedProjectRuntime(ctx)` — refuse only when the + * question: `requiresIsolatedProjectRuntime(ctx)`, which refuses only when the * runtime is shared *and* the host did not grant execution. Asking the * narrower `isSharedProjectRuntime(ctx)` instead denies hosts that were * explicitly granted the capability. @@ -19,11 +19,11 @@ import { fromFileUrl } from "#veryfront/compat/path"; * it was wrong. Once the Proxy was corrected, markdown preview started * returning 503 on staging (veryfront-issue-inbox#376, #366). * - * Per-handler tests cannot catch this: each one is individually consistent. - * Only an inventory across surfaces can, so this test is the inventory. It - * reads source rather than behaviour deliberately — a behavioural sweep can - * only cover the surfaces someone remembered to add to it, whereas an - * unlisted file here is a failure by construction. + * Per-handler tests cannot catch this, because each one is individually + * consistent. Only an inventory across surfaces can, so this test is the + * inventory. It reads source rather than behaviour deliberately: a behavioural + * sweep can only cover the surfaces someone remembered to add to it, whereas + * an unlisted file here is a failure by construction. */ const HANDLERS_DIR = fromFileUrl(new URL(".", import.meta.url)); @@ -69,16 +69,25 @@ async function readHandlerSources(): Promise> { return sources; } -/** Ignore the import statement so only real call sites count. */ +/** Read the tree once so every assertion inspects the same snapshot. */ +let cachedSources: Promise> | undefined; +function handlerSources(): Promise> { + cachedSources ??= readHandlerSources(); + return cachedSources; +} + +/** Strip imports first, so only real call sites count. */ function callsPredicate(source: string, predicate: string): boolean { - return new RegExp(`(? !line.trim().startsWith("import ")).join("\n"), - ); + const body = source + .split("\n") + .filter((line) => !line.trim().startsWith("import ")) + .join("\n"); + return new RegExp(`\\b${predicate}\\s*\\(`).test(body); } describe("server/handlers shared-runtime execution boundary", () => { it("gates every execution surface on the capability, not on sharedness alone", async () => { - const sources = await readHandlerSources(); + const sources = await handlerSources(); const drifted: string[] = []; for (const [path, source] of sources) { @@ -98,7 +107,7 @@ describe("server/handlers shared-runtime execution boundary", () => { }); it("keeps the capability-gated inventory accurate", async () => { - const sources = await readHandlerSources(); + const sources = await handlerSources(); const missing = CAPABILITY_GATED_SURFACES.filter((path) => { const source = sources.get(path); @@ -110,7 +119,7 @@ describe("server/handlers shared-runtime execution boundary", () => { [], `These surfaces are listed as capability-gated but no longer call ` + `requiresIsolatedProjectRuntime(). Either restore the call or remove the entry ` + - `deliberately — silently dropping the gate is how a surface stops being enforced.`, + `deliberately. Silently dropping the gate is how a surface stops being enforced.`, ); const unlisted = [...sources.keys()] @@ -122,13 +131,13 @@ describe("server/handlers shared-runtime execution boundary", () => { unlisted, [], `New execution surfaces found. Add them to CAPABILITY_GATED_SURFACES and give each ` + - `a paired fail-closed and granted-path test — a fail-closed test alone cannot ` + + `a paired fail-closed and granted-path test. A fail-closed test alone cannot ` + `distinguish a correct predicate from a hardcoded denial.`, ); }); it("documents why each non-gate use of the narrow predicate is safe", async () => { - const sources = await readHandlerSources(); + const sources = await handlerSources(); const stale = Object.keys(NON_GATE_USES).filter((path) => { const source = sources.get(path); return !source || !callsPredicate(source, "isSharedProjectRuntime"); diff --git a/src/server/handlers/preview/markdown-preview.handler.test.ts b/src/server/handlers/preview/markdown-preview.handler.test.ts index 2bc49b0f64..0eb900be91 100644 --- a/src/server/handlers/preview/markdown-preview.handler.test.ts +++ b/src/server/handlers/preview/markdown-preview.handler.test.ts @@ -99,41 +99,65 @@ Deno.test("MarkdownPreviewHandler fails closed before shared source reads", asyn assertEquals(reads, 0); }); -Deno.test("MarkdownPreviewHandler renders once the host grants execution", async () => { - // The granted counterpart of the shared-runtime denial above. #3364 collapsed - // the execution surfaces onto requiresIsolatedProjectRuntime so they could not - // drift apart, but markdown preview kept a bare isSharedProjectRuntime check - // and denied unconditionally. Without this case, an unconditional denial here - // is indistinguishable from a correct fail-closed guard. - let reads = 0; - const ctx = { - projectDir: "/remote/project", - isLocalProject: false, - requestContext: { mode: "preview" }, - adapter: { - fs: { - isMultiProjectMode: () => true, - readFile: () => { - reads++; - throw new Error("not found"); +describe("MarkdownPreviewHandler host-execution capability", () => { + it("renders once the host grants execution", async () => { + // The granted counterpart of the shared-runtime denial above. #3364 + // collapsed the execution surfaces onto requiresIsolatedProjectRuntime so + // they could not drift apart, but markdown preview kept a bare + // isSharedProjectRuntime check and denied unconditionally. Without this + // case, an unconditional denial here is indistinguishable from a correct + // fail-closed guard. + let reads = 0; + const ctx = { + projectDir: "/remote/project", + projectSlug: "project", + proxyToken: "token", + isLocalProject: false, + requestContext: { mode: "preview" }, + adapter: { + fs: { + symlinkSemantics: "none" as const, + isMultiProjectMode: () => true, + isContextualMode: () => true, + runWithContext: async ( + _slug: string, + _token: string, + fn: () => Promise, + ) => await fn(), + exists: () => Promise.resolve(true), + stat: () => + Promise.resolve({ + isFile: true, + isDirectory: false, + isSymlink: false, + size: 0, + mtime: new Date(), + }), + readFile: () => { + reads++; + return Promise.resolve("# Readme\n"); + }, }, }, - }, - securityConfig: null, - cspUserHeader: null, - allowHostProjectCodeExecution: true, - } as unknown as HandlerContext; + securityConfig: null, + cspUserHeader: null, + allowHostProjectCodeExecution: true, + } as unknown as HandlerContext; - const result = await new MarkdownPreviewHandler().handle( - new Request("https://tenant.example/README.md"), - ctx, - ); + const result = await new MarkdownPreviewHandler().handle( + new Request("https://tenant.example/README.md"), + ctx, + ); - assertNotEquals( - result.response?.status, - 503, - "a granted shared executor must not return project-execution-unavailable", - ); + assertNotEquals( + result.response?.status, + 503, + "a granted shared executor must not return project-execution-unavailable", + ); + // Not merely "did not 503": the granted request has to actually reach the + // shared filesystem, otherwise a fallthrough returning no response passes. + assertNotEquals(reads, 0, "the granted path must reach the project source read"); + }); }); Deno.test("MarkdownPreviewHandler admits and reads through a real wrapped GitHub adapter", async () => { diff --git a/src/server/handlers/request/module/module.handler.test.ts b/src/server/handlers/request/module/module.handler.test.ts index b943c67bdd..e3c944fb29 100644 --- a/src/server/handlers/request/module/module.handler.test.ts +++ b/src/server/handlers/request/module/module.handler.test.ts @@ -259,9 +259,9 @@ describe("server/handlers/request/module/module.handler", () => { it("serves the endpoints once the host grants execution", async () => { // The granted counterpart to the fail-closed test above. A handler that - // denies every shared runtime unconditionally — which is what this - // surface did before veryfront-issue-inbox#366 — passes that test and - // fails this one. Without the pair, the two are indistinguishable. + // denies every shared runtime unconditionally, which is what this surface + // did before veryfront-issue-inbox#366, passes that test and fails this + // one. Without the pair, the two are indistinguishable. const handler = new ModuleHandler(); for ( const pathname of [ @@ -278,6 +278,14 @@ describe("server/handlers/request/module/module.handler", () => { allowHostProjectCodeExecution: true, } as Partial), ); + // `continue: false` matters as much as the absent 503. Without it a + // handler that fell through entirely, emitting no response at all, + // would satisfy "did not return project-execution-unavailable". + assertEquals( + result.continue, + false, + `${pathname} fell through instead of serving a granted host`, + ); const type = result.response ? await result.response.clone().json().then( (body: { type?: string }) => body.type, diff --git a/src/server/handlers/request/snippet.handler.test.ts b/src/server/handlers/request/snippet.handler.test.ts index cc04f34ff4..036700ed2d 100644 --- a/src/server/handlers/request/snippet.handler.test.ts +++ b/src/server/handlers/request/snippet.handler.test.ts @@ -113,54 +113,56 @@ Deno.test("SnippetHandler rejects shared rendering before proxy context or sourc assertEquals(readPath, undefined); }); -Deno.test("SnippetHandler serves a shared runtime the host granted execution", async () => { - // The granted counterpart to the test above. Without it, a handler that - // simply denies every shared runtime — the pre-#366 behaviour — passes the - // whole suite. - let readPath: string | undefined; - const fs = { - symlinkSemantics: "none" as const, - isMultiProjectMode: () => true, - isContextualMode: () => true, - runWithContext: async ( - _slug: string, - _token: string, - fn: () => Promise, - ) => await fn(), - exists: () => Promise.resolve(true), - stat: () => - Promise.resolve({ - isFile: true, - isDirectory: false, - isSymlink: false, - size: 0, - mtime: new Date(), - }), - readFile: (path: string) => { - readPath = path; - return Promise.resolve("export default function Button() {}\n"); - }, - }; - const ctx = { - projectDir: "/project", - projectSlug: "project", - proxyToken: "token", - isLocalProject: false, - allowHostProjectCodeExecution: true, - adapter: { fs }, - } as unknown as HandlerContext; +describe("SnippetHandler host-execution capability", () => { + it("serves a shared runtime the host granted execution", async () => { + // The granted counterpart to the test above. Without it, a handler that + // simply denies every shared runtime, which is the pre-#366 behaviour, + // passes the whole suite. + let readPath: string | undefined; + const fs = { + symlinkSemantics: "none" as const, + isMultiProjectMode: () => true, + isContextualMode: () => true, + runWithContext: async ( + _slug: string, + _token: string, + fn: () => Promise, + ) => await fn(), + exists: () => Promise.resolve(true), + stat: () => + Promise.resolve({ + isFile: true, + isDirectory: false, + isSymlink: false, + size: 0, + mtime: new Date(), + }), + readFile: (path: string) => { + readPath = path; + return Promise.resolve("export default function Button() {}\n"); + }, + }; + const ctx = { + projectDir: "/project", + projectSlug: "project", + proxyToken: "token", + isLocalProject: false, + allowHostProjectCodeExecution: true, + adapter: { fs }, + } as unknown as HandlerContext; - const result = await new SnippetHandler().handle( - new Request("http://localhost/@components/button"), - ctx, - ); + const result = await new SnippetHandler().handle( + new Request("http://localhost/@components/button"), + ctx, + ); - assertNotEquals( - result.response?.status, - 503, - "a granted shared executor must not return project-execution-unavailable", - ); - assertNotEquals(readPath, undefined, "the granted path must reach the source read"); + assertNotEquals( + result.response?.status, + 503, + "a granted shared executor must not return project-execution-unavailable", + ); + assertNotEquals(readPath, undefined, "the granted path must reach the source read"); + }); }); Deno.test("SnippetHandler preserves dedicated local rendering", async () => { diff --git a/src/server/handlers/request/ssr/ssr.handler.ts b/src/server/handlers/request/ssr/ssr.handler.ts index 71b2dbaba8..34406f2e48 100644 --- a/src/server/handlers/request/ssr/ssr.handler.ts +++ b/src/server/handlers/request/ssr/ssr.handler.ts @@ -94,7 +94,6 @@ export class SSRHandler extends BaseHandler { const slug = pathname === "/" ? "" : pathname.replace(/^\//, "").replace(/\/$/, ""); const requestId = `${slug || "index"}-${Date.now()}`; - startRequest(requestId); if (shouldHideRouteInProduction(ctx, slug)) { this.logDebug("Dot path blocked in production", { slug }, ctx); @@ -121,6 +120,12 @@ export class SSRHandler extends BaseHandler { this.logDebug("SSR attempt", { pathname, slug }, ctx); + // Allocated only once the request is certain to be rendered. `startRequest` + // registers a timings entry that `endRequest` removes, but `endRequest` + // returns early when no timer ever ran, so an entry allocated before the + // guards above would survive on every hidden-route or fail-closed request. + startRequest(requestId); + return this.setupContextAndRender(req, ctx, slug, requestId, url); }