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..4afd69de55 --- /dev/null +++ b/src/server/handlers/execution-surface-policy.test.ts @@ -0,0 +1,152 @@ +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)`, 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. + * + * 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, 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)); + +/** 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; +} + +/** 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 { + 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 handlerSources(); + 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 handlerSources(); + + 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 handlerSources(); + 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..0eb900be91 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,67 @@ Deno.test("MarkdownPreviewHandler fails closed before shared source reads", asyn assertEquals(reads, 0); }); +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; + + 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", + ); + // 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 () => { 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..e3c944fb29 100644 --- a/src/server/handlers/request/module/module.handler.test.ts +++ b/src/server/handlers/request/module/module.handler.test.ts @@ -257,6 +257,49 @@ 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), + ); + // `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, + () => 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..036700ed2d 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,58 @@ Deno.test("SnippetHandler rejects shared rendering before proxy context or sourc assertEquals(readPath, undefined); }); +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, + ); + + 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..34406f2e48 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"); @@ -97,14 +94,13 @@ 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); return Promise.resolve(this.continue()); } - if (isSharedProjectRuntime(ctx) && !isHostProjectCodeExecutionAllowed(ctx)) { + if (requiresIsolatedProjectRuntime(ctx)) { const problem = createErrorResponseFromDefinition( PROJECT_EXECUTION_UNAVAILABLE, { @@ -124,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); }