From e296e083ec59440d60fae8333f55d1936e70f47a Mon Sep 17 00:00:00 2001 From: ekklesia Date: Fri, 24 Jul 2026 18:36:23 -0500 Subject: [PATCH 1/5] feat: define module HTTP routes Phase 5B, pass 1: framework-neutral module HTTP route contracts and runtime registration. Adds @ekklesia/contracts' module-http.ts (methods, canonical path grammar, route definition/context/handler types, and validation) and Core's module runtime staging/publication of a module's declared routes alongside its capabilities, gated on the module's own manifest permissions. Routes are collected into StartedModuleRuntime as an immutable snapshot but are not yet mounted into apps/api; that is pass 2. Co-Authored-By: Claude Sonnet 5 --- core/src/modules/runtime.ts | 168 +++++- core/tests/module-http-import.test.ts | 26 + core/tests/module-runtime.test.ts | 491 +++++++++++++++++- docs/module-development/README.md | 79 ++- packages/contracts/src/index.ts | 1 + packages/contracts/src/module-http.ts | 256 +++++++++ packages/contracts/src/module-runtime.ts | 5 + packages/contracts/tests/module-http.test.ts | 255 +++++++++ .../tests/define-module-runtime.test.ts | 46 +- templates/module/tests/manifest.test.ts | 5 + 10 files changed, 1304 insertions(+), 28 deletions(-) create mode 100644 core/tests/module-http-import.test.ts create mode 100644 packages/contracts/src/module-http.ts create mode 100644 packages/contracts/tests/module-http.test.ts diff --git a/core/src/modules/runtime.ts b/core/src/modules/runtime.ts index 111005d..42ab1de 100644 --- a/core/src/modules/runtime.ts +++ b/core/src/modules/runtime.ts @@ -4,12 +4,17 @@ import { type CapabilityRegistrar, type CapabilityResolver, capabilityIdSchema, + type ModuleHttpRouteRegistrar, type ModuleId, type ModuleRegistrationContext, + type ModuleRouteDefinitionInput, type ModuleRuntimeContext, type ModuleRuntimeDefinition, + moduleRouteShapeKey, parseModuleRuntimeDefinition, type RegisteredCapability, + type RegisteredModuleHttpRoute, + safeParseModuleRouteDefinition, } from "@ekklesia/contracts"; import { type DiscoveredModule, @@ -27,6 +32,12 @@ export type ModuleRuntimeIssueCode = | "MODULE_CAPABILITY_UNREGISTERED" | "MODULE_REGISTER_FAILED" | "MODULE_ROLLBACK_FAILED" + | "MODULE_ROUTE_DEFINITION_INVALID" + | "MODULE_ROUTE_DUPLICATE" + | "MODULE_ROUTE_METHOD_INVALID" + | "MODULE_ROUTE_PATH_INVALID" + | "MODULE_ROUTE_PERMISSION_INVALID" + | "MODULE_ROUTE_PERMISSION_UNDECLARED" | "MODULE_RUNTIME_EXPORT_INVALID" | "MODULE_RUNTIME_ID_MISMATCH" | "MODULE_RUNTIME_IMPORT_FAILED" @@ -53,6 +64,9 @@ export interface RunningModule { export interface StartedModuleRuntime { readonly capabilities: CapabilityRegistry; readonly modules: readonly RunningModule[]; + /** One immutable startup snapshot: discovered-module order, then registration order within each + * module. Never mutated after startup — there is no dynamic route addition or removal. */ + readonly routes: readonly RegisteredModuleHttpRoute[]; stop(): Promise; } @@ -63,11 +77,16 @@ interface LoadedModule { readonly discovered: DiscoveredModule; } -interface RegistrationStage { - readonly context: ModuleRegistrationContext; +interface CapabilityRegistrationStage { + readonly capabilities: CapabilityRegistrar; commit(): readonly ModuleRuntimeIssue[]; } +interface HttpRouteRegistrationStage { + readonly http: ModuleHttpRouteRegistrar; + commit(): void; +} + class ModuleRuntimeSignal extends Error { readonly issue: ModuleRuntimeIssue; @@ -109,7 +128,7 @@ function defaultExport(moduleNamespace: unknown): unknown { class StagedCapabilityRegistry { readonly #registered = new Map(); - createRegistrationStage(module: DiscoveredModule): RegistrationStage { + createRegistrationStage(module: DiscoveredModule): CapabilityRegistrationStage { const local = new Map(); const declarations = new Map( module.manifest.capabilities.map((declaration) => [declaration.id, declaration] as const), @@ -154,13 +173,9 @@ class StagedCapabilityRegistry { }, resolve, }); - const context: ModuleRegistrationContext = Object.freeze({ - capabilities, - moduleId: module.manifest.id, - }); return { - context, + capabilities, commit: (): readonly ModuleRuntimeIssue[] => { const missing = [...declarations.keys()] .filter((capabilityId) => !local.has(capabilityId)) @@ -252,6 +267,88 @@ class StagedCapabilityRegistry { } } +/** + * Stages a module's HTTP route registrations during `register`, exactly like + * `StagedCapabilityRegistry` stages capability provision: routes accumulate in a per-module local + * list, validated and rejected immediately (`ModuleRuntimeSignal`) at the `http.route(...)` call + * site, and are only merged into the shared, eventually-published list by `commit()` — called only + * after the module's `register` hook returns without throwing. Unlike capabilities, there is no + * "declared but never provided" concept for routes (a manifest never declares routes), so `commit` + * has nothing to validate; it only publishes what already passed validation. + */ +class StagedModuleHttpRoutes { + readonly #routes: RegisteredModuleHttpRoute[] = []; + + createRegistrationStage(module: DiscoveredModule): HttpRouteRegistrationStage { + const local: RegisteredModuleHttpRoute[] = []; + const shapes = new Set(); + const ownedPermissionIds = new Set( + module.manifest.permissions.map((declaration) => declaration.id), + ); + + const http: ModuleHttpRouteRegistrar = Object.freeze({ + route: (definition: ModuleRouteDefinitionInput): void => { + const parsed = safeParseModuleRouteDefinition(definition); + if (!parsed.success) { + throw new ModuleRuntimeSignal( + runtimeIssue( + parsed.code, + "register", + module.manifest.id, + `Module ${module.manifest.id} registered an invalid HTTP route.`, + ), + ); + } + // Ownership is checked against this module's own manifest-declared permissions only, never + // the composed platform registry — a module can therefore never gate a route with Core's or + // another module's permission, structurally, regardless of what that other permission is. + if (!ownedPermissionIds.has(parsed.data.permission)) { + throw new ModuleRuntimeSignal( + runtimeIssue( + "MODULE_ROUTE_PERMISSION_UNDECLARED", + "register", + module.manifest.id, + `Module ${module.manifest.id} registered an HTTP route with a permission its own manifest does not declare.`, + ), + ); + } + const shapeKey = moduleRouteShapeKey(parsed.data.method, parsed.data.path); + if (shapes.has(shapeKey)) { + throw new ModuleRuntimeSignal( + runtimeIssue( + "MODULE_ROUTE_DUPLICATE", + "register", + module.manifest.id, + `Module ${module.manifest.id} registered a duplicate HTTP route.`, + ), + ); + } + shapes.add(shapeKey); + local.push( + Object.freeze({ + handler: parsed.data.handler, + method: parsed.data.method, + moduleId: module.manifest.id, + path: parsed.data.path, + permission: parsed.data.permission, + }), + ); + }, + }); + + return { + http, + commit: (): void => { + this.#routes.push(...local); + }, + }; + } + + publish(): readonly RegisteredModuleHttpRoute[] { + return Object.freeze([...this.#routes]); + } +} + async function loadRuntimeDefinitions( discoveredModules: readonly DiscoveredModule[], ): Promise { @@ -327,12 +424,19 @@ async function loadRuntimeDefinitions( async function registerModules( modules: readonly LoadedModule[], - registry: StagedCapabilityRegistry, + capabilityRegistry: StagedCapabilityRegistry, + httpRoutes: StagedModuleHttpRoutes, ): Promise { for (const module of modules) { - const stage = registry.createRegistrationStage(module.discovered); + const capabilityStage = capabilityRegistry.createRegistrationStage(module.discovered); + const routeStage = httpRoutes.createRegistrationStage(module.discovered); + const context: ModuleRegistrationContext = Object.freeze({ + capabilities: capabilityStage.capabilities, + http: routeStage.http, + moduleId: module.discovered.manifest.id, + }); try { - await module.definition.register?.(stage.context); + await module.definition.register?.(context); } catch (error) { if (error instanceof ModuleRuntimeSignal) { throw new ModuleRuntimeError([error.issue]); @@ -346,10 +450,15 @@ async function registerModules( ), ]); } - const issues = stage.commit(); + const issues = capabilityStage.commit(); if (issues.length > 0) { throw new ModuleRuntimeError(issues); } + // Routes are only merged into the shared published list once this module's registration has + // fully succeeded (register hook did not throw, and every declared capability was provided) — + // a failed registration therefore publishes no routes for this module, matching the same + // all-or-nothing guarantee capabilities already have. + routeStage.commit(); } } @@ -405,16 +514,25 @@ async function startModules( } } -export async function startModuleRuntime( - options: StartModuleRuntimeOptions, +/** + * Starts the module runtime from a module list the caller already discovered — performs no + * module-directory scan or manifest read of its own, and preserves the given list's order exactly. + * `startModuleRuntime` below is the convenient discovery-plus-start wrapper most callers want; + * `apps/api`'s production startup (pass 2) will call this form directly so permission composition + * and module HTTP/runtime startup share Phase 5A's exact same one-time discovery rather than + * discovering modules twice. + */ +export async function startDiscoveredModuleRuntime( + discoveredModules: readonly DiscoveredModule[], ): Promise { - const discovered = await discoverModules(options); - const modules = await loadRuntimeDefinitions(discovered); - const stagedRegistry = new StagedCapabilityRegistry(); - await registerModules(modules, stagedRegistry); - await startModules(modules, stagedRegistry); + const modules = await loadRuntimeDefinitions(discoveredModules); + const capabilityRegistry = new StagedCapabilityRegistry(); + const httpRoutes = new StagedModuleHttpRoutes(); + await registerModules(modules, capabilityRegistry, httpRoutes); + await startModules(modules, capabilityRegistry); - const capabilities = stagedRegistry.publish(); + const capabilities = capabilityRegistry.publish(); + const routes = httpRoutes.publish(); const runningModules = Object.freeze( modules.map((module) => Object.freeze({ @@ -428,9 +546,10 @@ export async function startModuleRuntime( return Object.freeze({ capabilities, modules: runningModules, + routes, stop: (): Promise => { stopPromise ??= (async () => { - const issues = await stopModules(modules, stagedRegistry, "stop"); + const issues = await stopModules(modules, capabilityRegistry, "stop"); if (issues.length > 0) { throw new ModuleRuntimeError(issues); } @@ -439,3 +558,10 @@ export async function startModuleRuntime( }, }); } + +export async function startModuleRuntime( + options: StartModuleRuntimeOptions, +): Promise { + const discovered = await discoverModules(options); + return startDiscoveredModuleRuntime(discovered); +} diff --git a/core/tests/module-http-import.test.ts b/core/tests/module-http-import.test.ts new file mode 100644 index 0000000..1c425e9 --- /dev/null +++ b/core/tests/module-http-import.test.ts @@ -0,0 +1,26 @@ +import { describe, expect, it, vi } from "vitest"; + +const readdir = vi.fn(); +const readFile = vi.fn(); +const realpath = vi.fn(); +const stat = vi.fn(); + +vi.mock("node:fs/promises", async (importOriginal) => ({ + ...(await importOriginal()), + readdir, + readFile, + realpath, + stat, +})); + +describe("module HTTP contract and runtime import safety", () => { + it("performs no filesystem I/O merely by importing the module runtime or the module-http contract", async () => { + await import("../src/modules/index.js"); + await import("@ekklesia/contracts"); + + expect(readdir).not.toHaveBeenCalled(); + expect(readFile).not.toHaveBeenCalled(); + expect(realpath).not.toHaveBeenCalled(); + expect(stat).not.toHaveBeenCalled(); + }); +}); diff --git a/core/tests/module-runtime.test.ts b/core/tests/module-runtime.test.ts index 28b02d9..aefc443 100644 --- a/core/tests/module-runtime.test.ts +++ b/core/tests/module-runtime.test.ts @@ -1,14 +1,21 @@ import { mkdir, mkdtemp, rm, writeFile } from "node:fs/promises"; import { tmpdir } from "node:os"; import path from "node:path"; -import { afterEach, describe, expect, it } from "vitest"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import { discoverModules } from "../src/modules/discovery.js"; import { ModuleDiscoveryError, ModuleRuntimeError, type ModuleRuntimeIssueCode, + startDiscoveredModuleRuntime, startModuleRuntime, } from "../src/modules/index.js"; +vi.mock("../src/modules/discovery.js", async (importOriginal) => { + const actual = await importOriginal(); + return { ...actual, discoverModules: vi.fn(actual.discoverModules) }; +}); + interface RuntimeModuleFixture { readonly directory?: string; readonly manifest?: Record; @@ -40,6 +47,7 @@ function manifestFor( options: { capabilities?: Array<{ id: string; version: string }>; dependencies?: Record; + permissions?: Array<{ id: string; baseline: string; description: string }>; } = {}, ): Record { return { @@ -52,7 +60,7 @@ function manifestFor( moduleSdk: "^0.1.0", }, capabilities: options.capabilities ?? [], - permissions: [], + permissions: options.permissions ?? [], ...(options.dependencies ? { dependencies: options.dependencies } : {}), entrypoints: { server: "./dist/server/index.js", @@ -370,3 +378,482 @@ describe("module runtime", () => { expect(JSON.stringify(error.issues)).not.toContain(modulesDirectory); }); }); + +function noopHandlerSource(): string { + return "() => new Response(null, { status: 204 })"; +} + +function permissionDeclaration( + id: string, + baseline: "authenticated" | "public" | "restricted" = "restricted", +): { id: string; baseline: string; description: string } { + return { id, baseline, description: `Test permission ${id}.` }; +} + +describe("module HTTP routes", () => { + it("registers one route and publishes it on the started runtime", async () => { + const modulesDirectory = await createModulesDirectory(); + await createRuntimeModule(modulesDirectory, "single", { + manifest: manifestFor("single", { + permissions: [permissionDeclaration("single.items.read")], + }), + serverSource: `export default { + id: "single", version: "0.1.0", + register({ http }) { + http.route({ + method: "GET", + path: "/items", + permission: "single.items.read", + handler: ${noopHandlerSource()}, + }); + } + };`, + }); + + const runtime = await startModuleRuntime(runtimeOptions(modulesDirectory)); + expect(runtime.routes).toHaveLength(1); + expect(runtime.routes[0]).toMatchObject({ + moduleId: "single", + method: "GET", + path: "/items", + permission: "single.items.read", + }); + await runtime.stop(); + }); + + it("retains registration order within a module and discovery order across modules", async () => { + const modulesDirectory = await createModulesDirectory(); + await createRuntimeModule(modulesDirectory, "alpha", { + manifest: manifestFor("alpha", { + permissions: [ + permissionDeclaration("alpha.first.read"), + permissionDeclaration("alpha.second.read"), + ], + }), + serverSource: `export default { + id: "alpha", version: "0.1.0", + register({ http }) { + http.route({ method: "GET", path: "/first", permission: "alpha.first.read", handler: ${noopHandlerSource()} }); + http.route({ method: "GET", path: "/second", permission: "alpha.second.read", handler: ${noopHandlerSource()} }); + } + };`, + }); + await createRuntimeModule(modulesDirectory, "beta", { + manifest: manifestFor("beta", { + permissions: [permissionDeclaration("beta.third.read")], + }), + serverSource: `export default { + id: "beta", version: "0.1.0", + register({ http }) { + http.route({ method: "GET", path: "/third", permission: "beta.third.read", handler: ${noopHandlerSource()} }); + } + };`, + }); + + const runtime = await startModuleRuntime(runtimeOptions(modulesDirectory)); + expect( + runtime.routes.map((route) => `${route.moduleId}:${route.method}:${route.path}`), + ).toEqual(["alpha:GET:/first", "alpha:GET:/second", "beta:GET:/third"]); + await runtime.stop(); + }); + + it("allows the same path under different methods", async () => { + const modulesDirectory = await createModulesDirectory(); + await createRuntimeModule(modulesDirectory, "same-path", { + manifest: manifestFor("same-path", { + permissions: [ + permissionDeclaration("same-path.items.read"), + permissionDeclaration("same-path.items.write"), + ], + }), + serverSource: `export default { + id: "same-path", version: "0.1.0", + register({ http }) { + http.route({ method: "GET", path: "/items", permission: "same-path.items.read", handler: ${noopHandlerSource()} }); + http.route({ method: "POST", path: "/items", permission: "same-path.items.write", handler: ${noopHandlerSource()} }); + } + };`, + }); + + const runtime = await startModuleRuntime(runtimeOptions(modulesDirectory)); + expect(runtime.routes.map((route) => `${route.method} ${route.path}`)).toEqual([ + "GET /items", + "POST /items", + ]); + await runtime.stop(); + }); + + it("rejects a duplicate method/path route registered by the same module", async () => { + const modulesDirectory = await createModulesDirectory(); + await createRuntimeModule(modulesDirectory, "dup", { + manifest: manifestFor("dup", { + permissions: [permissionDeclaration("dup.items.read")], + }), + serverSource: `export default { + id: "dup", version: "0.1.0", + register({ http }) { + http.route({ method: "GET", path: "/items", permission: "dup.items.read", handler: ${noopHandlerSource()} }); + http.route({ method: "GET", path: "/items", permission: "dup.items.read", handler: ${noopHandlerSource()} }); + } + };`, + }); + + const error = await captureRuntimeError(modulesDirectory); + expect(runtimeIssueCodes(error)).toEqual(["MODULE_ROUTE_DUPLICATE"]); + }); + + it("collides parameter-name-only path variants under the same method", async () => { + const modulesDirectory = await createModulesDirectory(); + await createRuntimeModule(modulesDirectory, "param-shape", { + manifest: manifestFor("param-shape", { + permissions: [permissionDeclaration("param-shape.events.read")], + }), + serverSource: `export default { + id: "param-shape", version: "0.1.0", + register({ http }) { + http.route({ method: "GET", path: "/events/:eventId", permission: "param-shape.events.read", handler: ${noopHandlerSource()} }); + http.route({ method: "GET", path: "/events/:id", permission: "param-shape.events.read", handler: ${noopHandlerSource()} }); + } + };`, + }); + + const error = await captureRuntimeError(modulesDirectory); + expect(runtimeIssueCodes(error)).toEqual(["MODULE_ROUTE_DUPLICATE"]); + }); + + it("fails a malformed route with its specific structured issue code", async () => { + const badMethodDirectory = await createModulesDirectory(); + await createRuntimeModule(badMethodDirectory, "bad-method", { + manifest: manifestFor("bad-method", { + permissions: [permissionDeclaration("bad-method.items.read")], + }), + serverSource: `export default { + id: "bad-method", version: "0.1.0", + register({ http }) { + http.route({ method: "get", path: "/items", permission: "bad-method.items.read", handler: ${noopHandlerSource()} }); + } + };`, + }); + expect(runtimeIssueCodes(await captureRuntimeError(badMethodDirectory))).toEqual([ + "MODULE_ROUTE_METHOD_INVALID", + ]); + + const badPathDirectory = await createModulesDirectory(); + await createRuntimeModule(badPathDirectory, "bad-path", { + manifest: manifestFor("bad-path", { + permissions: [permissionDeclaration("bad-path.items.read")], + }), + serverSource: `export default { + id: "bad-path", version: "0.1.0", + register({ http }) { + http.route({ method: "GET", path: "items", permission: "bad-path.items.read", handler: ${noopHandlerSource()} }); + } + };`, + }); + expect(runtimeIssueCodes(await captureRuntimeError(badPathDirectory))).toEqual([ + "MODULE_ROUTE_PATH_INVALID", + ]); + + const badDefinitionDirectory = await createModulesDirectory(); + await createRuntimeModule(badDefinitionDirectory, "bad-shape", { + manifest: manifestFor("bad-shape", { + permissions: [permissionDeclaration("bad-shape.items.read")], + }), + serverSource: `export default { + id: "bad-shape", version: "0.1.0", + register({ http }) { + http.route({ + method: "GET", + path: "/items", + permission: "bad-shape.items.read", + baseline: "public", + handler: ${noopHandlerSource()}, + }); + } + };`, + }); + expect(runtimeIssueCodes(await captureRuntimeError(badDefinitionDirectory))).toEqual([ + "MODULE_ROUTE_DEFINITION_INVALID", + ]); + }); + + it("rejects a route permission the registering module does not itself declare", async () => { + const modulesDirectory = await createModulesDirectory(); + await createRuntimeModule(modulesDirectory, "undeclared", { + manifest: manifestFor("undeclared"), + serverSource: `export default { + id: "undeclared", version: "0.1.0", + register({ http }) { + http.route({ method: "GET", path: "/items", permission: "undeclared.items.read", handler: ${noopHandlerSource()} }); + } + };`, + }); + + const error = await captureRuntimeError(modulesDirectory); + expect(runtimeIssueCodes(error)).toEqual(["MODULE_ROUTE_PERMISSION_UNDECLARED"]); + }); + + it("rejects a module gating a route with a Core-owned permission it cannot declare", async () => { + const modulesDirectory = await createModulesDirectory(); + await createRuntimeModule(modulesDirectory, "coreattempt", { + manifest: manifestFor("coreattempt"), + serverSource: `export default { + id: "coreattempt", version: "0.1.0", + register({ http }) { + http.route({ + method: "GET", + path: "/items", + permission: "core.authorization.grants.manage", + handler: ${noopHandlerSource()}, + }); + } + };`, + }); + + const error = await captureRuntimeError(modulesDirectory); + expect(runtimeIssueCodes(error)).toEqual(["MODULE_ROUTE_PERMISSION_UNDECLARED"]); + }); + + it("rejects a module gating a route with another module's permission", async () => { + const modulesDirectory = await createModulesDirectory(); + await createRuntimeModule(modulesDirectory, "alpha", { + manifest: manifestFor("alpha", { + permissions: [permissionDeclaration("alpha.items.read")], + }), + serverSource: `export default { id: "alpha", version: "0.1.0" };`, + }); + await createRuntimeModule(modulesDirectory, "beta", { + manifest: manifestFor("beta"), + serverSource: `export default { + id: "beta", version: "0.1.0", + register({ http }) { + http.route({ method: "GET", path: "/items", permission: "alpha.items.read", handler: ${noopHandlerSource()} }); + } + };`, + }); + + const error = await captureRuntimeError(modulesDirectory); + expect(runtimeIssueCodes(error)).toEqual(["MODULE_ROUTE_PERMISSION_UNDECLARED"]); + }); + + it("accepts a declared public, authenticated, or restricted permission without interpreting its baseline", async () => { + const modulesDirectory = await createModulesDirectory(); + await createRuntimeModule(modulesDirectory, "baselines", { + manifest: manifestFor("baselines", { + permissions: [ + permissionDeclaration("baselines.public.read", "public"), + permissionDeclaration("baselines.authenticated.read", "authenticated"), + permissionDeclaration("baselines.restricted.read", "restricted"), + ], + }), + serverSource: `export default { + id: "baselines", version: "0.1.0", + register({ http }) { + http.route({ method: "GET", path: "/public", permission: "baselines.public.read", handler: ${noopHandlerSource()} }); + http.route({ method: "GET", path: "/authenticated", permission: "baselines.authenticated.read", handler: ${noopHandlerSource()} }); + http.route({ method: "GET", path: "/restricted", permission: "baselines.restricted.read", handler: ${noopHandlerSource()} }); + } + };`, + }); + + const runtime = await startModuleRuntime(runtimeOptions(modulesDirectory)); + expect(runtime.routes).toHaveLength(3); + for (const route of runtime.routes) { + expect(Object.keys(route).sort()).toEqual([ + "handler", + "method", + "moduleId", + "path", + "permission", + ]); + } + await runtime.stop(); + }); + + it("never invokes a route handler while registering or starting", async () => { + const modulesDirectory = await createModulesDirectory(); + const recorder = createEventRecorder(); + await createRuntimeModule(modulesDirectory, "never-invoked", { + manifest: manifestFor("never-invoked", { + permissions: [permissionDeclaration("never-invoked.items.read")], + }), + serverSource: `export default { + id: "never-invoked", version: "0.1.0", + register({ http }) { + http.route({ + method: "GET", + path: "/items", + permission: "never-invoked.items.read", + handler: () => { ${recorder.expression}.push("handler:called"); return new Response(null, { status: 204 }); }, + }); + } + };`, + }); + + const runtime = await startModuleRuntime(runtimeOptions(modulesDirectory)); + expect(recorder.events).toEqual([]); + await runtime.stop(); + expect(recorder.events).toEqual([]); + }); + + it("publishes frozen, immutable routes as a frozen route list", async () => { + const modulesDirectory = await createModulesDirectory(); + await createRuntimeModule(modulesDirectory, "frozen", { + manifest: manifestFor("frozen", { + permissions: [permissionDeclaration("frozen.items.read")], + }), + serverSource: `export default { + id: "frozen", version: "0.1.0", + register({ http }) { + http.route({ method: "GET", path: "/items", permission: "frozen.items.read", handler: ${noopHandlerSource()} }); + } + };`, + }); + + const runtime = await startModuleRuntime(runtimeOptions(modulesDirectory)); + expect(Object.isFrozen(runtime.routes)).toBe(true); + expect(Object.isFrozen(runtime.routes[0])).toBe(true); + await runtime.stop(); + }); + + it("does not let a module mutate the published route by mutating its own definition afterward", async () => { + const modulesDirectory = await createModulesDirectory(); + await createRuntimeModule(modulesDirectory, "mutate", { + manifest: manifestFor("mutate", { + permissions: [ + permissionDeclaration("mutate.items.read"), + permissionDeclaration("mutate.other.read"), + ], + }), + serverSource: `export default { + id: "mutate", version: "0.1.0", + register({ http }) { + const definition = { + method: "GET", + path: "/items", + permission: "mutate.items.read", + handler: ${noopHandlerSource()}, + }; + http.route(definition); + definition.method = "POST"; + definition.path = "/mutated"; + definition.permission = "mutate.other.read"; + } + };`, + }); + + const runtime = await startModuleRuntime(runtimeOptions(modulesDirectory)); + expect(runtime.routes[0]).toMatchObject({ + method: "GET", + path: "/items", + permission: "mutate.items.read", + }); + await runtime.stop(); + }); + + it("publishes no runtime when a later module's route registration fails", async () => { + const modulesDirectory = await createModulesDirectory(); + await createRuntimeModule(modulesDirectory, "first-ok", { + manifest: manifestFor("first-ok", { + permissions: [permissionDeclaration("first-ok.items.read")], + }), + serverSource: `export default { + id: "first-ok", version: "0.1.0", + register({ http }) { + http.route({ method: "GET", path: "/items", permission: "first-ok.items.read", handler: ${noopHandlerSource()} }); + } + };`, + }); + await createRuntimeModule(modulesDirectory, "second-bad", { + manifest: manifestFor("second-bad"), + serverSource: `export default { + id: "second-bad", version: "0.1.0", + register({ http }) { + http.route({ method: "GET", path: "/items", permission: "second-bad.items.read", handler: ${noopHandlerSource()} }); + } + };`, + }); + + const error = await captureRuntimeError(modulesDirectory); + expect(runtimeIssueCodes(error)).toEqual(["MODULE_ROUTE_PERMISSION_UNDECLARED"]); + expect(error.issues[0]?.moduleId).toBe("second-bad"); + }); + + it("preserves existing rollback behavior when a registered route's module later fails to start", async () => { + const modulesDirectory = await createModulesDirectory(); + const recorder = createEventRecorder(); + await createRuntimeModule(modulesDirectory, "routed", { + manifest: manifestFor("routed", { + permissions: [permissionDeclaration("routed.items.read")], + }), + serverSource: `export default { + id: "routed", version: "0.1.0", + register({ http }) { + http.route({ method: "GET", path: "/items", permission: "routed.items.read", handler: ${noopHandlerSource()} }); + }, + start() { ${recorder.expression}.push("routed:start"); }, + stop() { ${recorder.expression}.push("routed:stop"); } + };`, + }); + await createRuntimeModule(modulesDirectory, "failing", { + manifest: manifestFor("failing", { + dependencies: { modules: { required: { routed: "^0.1.0" } } }, + }), + serverSource: `export default { + id: "failing", version: "0.1.0", + start() { throw new Error("boom"); } + };`, + }); + + const error = await captureRuntimeError(modulesDirectory); + expect(runtimeIssueCodes(error)).toEqual(["MODULE_START_FAILED"]); + expect(recorder.events).toEqual(["routed:start", "routed:stop"]); + }); +}); + +describe("already-discovered module runtime", () => { + it("starts from an already-discovered inventory without a second directory scan", async () => { + const modulesDirectory = await createModulesDirectory(); + await createRuntimeModule(modulesDirectory, "already-discovered", { + serverSource: `export default { id: "already-discovered", version: "0.1.0" };`, + }); + + vi.mocked(discoverModules).mockClear(); + const discovered = await discoverModules(runtimeOptions(modulesDirectory)); + expect(discoverModules).toHaveBeenCalledTimes(1); + + const runtime = await startDiscoveredModuleRuntime(discovered); + expect(discoverModules).toHaveBeenCalledTimes(1); + expect(runtime.modules.map((module) => module.id)).toEqual(["already-discovered"]); + await runtime.stop(); + }); + + it("preserves the given discovered-module list order exactly, without re-sorting", async () => { + const modulesDirectory = await createModulesDirectory(); + await createRuntimeModule(modulesDirectory, "first", { + serverSource: `export default { id: "first", version: "0.1.0" };`, + }); + await createRuntimeModule(modulesDirectory, "second", { + serverSource: `export default { id: "second", version: "0.1.0" };`, + }); + + const discovered = await discoverModules(runtimeOptions(modulesDirectory)); + expect(discovered.map((module) => module.manifest.id)).toEqual(["first", "second"]); + + const runtime = await startDiscoveredModuleRuntime([...discovered].reverse()); + expect(runtime.modules.map((module) => module.id)).toEqual(["second", "first"]); + await runtime.stop(); + }); + + it("startModuleRuntime remains backward-compatible and discovers exactly once", async () => { + const modulesDirectory = await createModulesDirectory(); + await createRuntimeModule(modulesDirectory, "compat", { + serverSource: `export default { id: "compat", version: "0.1.0" };`, + }); + + vi.mocked(discoverModules).mockClear(); + const runtime = await startModuleRuntime(runtimeOptions(modulesDirectory)); + expect(discoverModules).toHaveBeenCalledTimes(1); + await runtime.stop(); + }); +}); diff --git a/docs/module-development/README.md b/docs/module-development/README.md index b11449f..429158c 100644 --- a/docs/module-development/README.md +++ b/docs/module-development/README.md @@ -177,6 +177,75 @@ Entrypoint imports must have no top-level side effects. Modules are trusted back evaluation cannot be rolled back. Keep registration declarative; open resources in `start` and close them in `stop`. +## HTTP routes + +A module registers plain Fetch (`Request` in, `Response` out) route handlers during `register`, +through the `http` registrar `ModuleRegistrationContext` provides alongside `capabilities`: + +```ts +import { defineModuleRuntime } from "@ekklesia/module-sdk"; + +export default defineModuleRuntime({ + id: "calendar", + version: "0.1.0", + + register({ capabilities, http }) { + capabilities.provide("calendar.events", calendarEvents); + + http.route({ + method: "GET", + path: "/events/:eventId", + permission: "calendar.events.read", + async handler(request, context) { + return Response.json({ + eventId: context.params.eventId, + principal: context.principal.kind, + }); + }, + }); + }, +}); +``` + +A route definition has exactly four fields — `method`, `path`, `permission`, `handler` — and nothing +else; an unknown field (a baseline, a mount prefix, CORS/origin policy, middleware, audit metadata, +authentication behavior, or another module's namespace) fails registration outright rather than +being ignored. `method` is one of `GET`, `POST`, `PUT`, `PATCH`, `DELETE`, always uppercase; there is +no `OPTIONS` (the platform owns CORS preflight) and no wildcard/all-method registration. + +`path` is local to the module, never the path a caller actually sends. The platform alone owns the +production mount prefix, `/api/v1/modules/`: `"/"` is the module's own root, `"/events"` +means `/api/v1/modules/calendar/events`, and `"/events/:eventId"` is a parameterized route using the +same `:name` syntax a caller's request is matched against. A canonical path begins with `/`, a +non-root path never ends with `/`, and every segment is either a lowercase kebab-case static segment +or one `:parameterName` segment — no empty or repeated slashes, no query string or fragment, no +backslash or percent-encoding, no dot segments, and no wildcard/catch-all segment. A malformed path +is rejected outright, never silently normalized. + +Every route — including a route meant to be publicly reachable — names exactly one `permission` the +owning module's own manifest declares; there is no way to register an unprotected, no-permission +route. A module cannot supply a baseline directly: the permission's trusted `public`, `authenticated`, +or `restricted` baseline always comes from the composed platform registry, exactly like any other +enforcement decision. Public access is therefore expressed by referencing a permission whose manifest +declaration has `baseline: "public"`, never by omitting a permission. A module may only reference a +permission its own manifest declares — never a Core-owned permission, and never another module's — +so it can never gate a route behind authority it does not itself own. The same local path may be +registered under more than one method, but registering the same module twice under the identical +method and path (or two paths that are identical once every parameter is treated as an anonymous +placeholder, such as `/events/:eventId` and `/events/:id` under `GET`) fails as a duplicate route. + +The handler receives the standard `Request` plus a context of exactly `moduleId`, the caller's +already-authorized `EkklesiaPrincipal`, and the immutable decoded path parameters — never a database +connection, authorization store, grant/platform-owner state, storage, jobs, configuration, secrets, +a framework-specific request context, or a raw Better Auth session. The full query string remains +available through `Request.url`; there is no separate query or body abstraction. + +This pass only defines, validates, and collects routes into `StartedModuleRuntime`'s immutable route +snapshot — Core never invokes a handler here. Mounting routes into the production `apps/api` Hono +application, enforcing each route's permission through `requirePermission`, applying +`requireTrustedOrigin`, mapping handler errors to sanitized responses, and wiring this into +`startApi`'s production lifecycle are later work. + ## Drizzle schema definition A module with tables keeps the generator-facing declarations in `src/schema/tables.ts`: @@ -327,10 +396,12 @@ not misreported as a successful check. Core completes discovery before importing any server entrypoint. It then loads and verifies every runtime definition before running hooks, registers all modules dependency-first into an unpublished -staged capability registry, and starts them in the same order. Shutdown is reverse-order. If a later -module fails to start, Core stops already-started modules in reverse order and reports any cleanup -failures without leaking caught exceptions or machine paths. A failing `start` hook must clean up -its own partial work before rejecting. +staged capability registry and an unpublished staged HTTP route list side by side, and starts them in +the same order. Neither the capability registry nor the route list is published — visible on +`StartedModuleRuntime` — unless every module registers and starts successfully. Shutdown is +reverse-order. If a later module fails to start, Core stops already-started modules in reverse order +and reports any cleanup failures without leaking caught exceptions or machine paths. A failing +`start` hook must clean up its own partial work before rejecting. ## Start from the template diff --git a/packages/contracts/src/index.ts b/packages/contracts/src/index.ts index 3fd62c0..8577526 100644 --- a/packages/contracts/src/index.ts +++ b/packages/contracts/src/index.ts @@ -1,6 +1,7 @@ export * from "./authorization.js"; export * from "./identifiers.js"; export * from "./identity.js"; +export * from "./module-http.js"; export * from "./module-lifecycle.js"; export * from "./module-manifest.js"; export * from "./module-migration.js"; diff --git a/packages/contracts/src/module-http.ts b/packages/contracts/src/module-http.ts new file mode 100644 index 0000000..ccbbdd6 --- /dev/null +++ b/packages/contracts/src/module-http.ts @@ -0,0 +1,256 @@ +import { z } from "zod"; +import type { EkklesiaPrincipal } from "./identity.js"; +import { type ModuleId, type PermissionId, permissionIdSchema } from "./identifiers.js"; + +// Not imported from `./module-runtime.js`'s own `Awaitable` (structurally identical) — importing +// it would create a source dependency cycle with that module, which also imports from this one for +// `ModuleHttpRouteRegistrar`. `scripts/check-boundaries.ts` rejects any source cycle, including one +// formed only by `import type`, so this tiny, fixed-shape utility type is kept private here instead. +type Awaitable = Value | PromiseLike; + +/** The exact, canonical HTTP methods a module route may declare. Uppercase only — a module never + * gets `OPTIONS` (the platform owns CORS preflight) or a wildcard/all-methods registration. */ +export const MODULE_HTTP_METHODS = ["DELETE", "GET", "PATCH", "POST", "PUT"] as const; + +export type ModuleHttpMethod = (typeof MODULE_HTTP_METHODS)[number]; + +const moduleHttpMethods: ReadonlySet = new Set(MODULE_HTTP_METHODS); + +export function isModuleHttpMethod(value: unknown): value is ModuleHttpMethod { + return typeof value === "string" && moduleHttpMethods.has(value); +} + +const MODULE_ROUTE_PATH_MAX_LENGTH = 256; +const staticSegmentPattern = /^[a-z0-9]+(?:-[a-z0-9]+)*$/u; +const parameterSegmentPattern = /^:[a-z][a-zA-Z0-9]*$/u; + +/** + * A module-local route path — never the production-mounted `/api/v1/modules/` path a + * caller actually sends (pass 2). Not branded (unlike `PermissionId`): like `PackageRelativePath`, + * this is a validated location, not an opaque cross-system identifier. `"/"` is the module root; + * any other canonical path begins with `/`, never ends with `/`, and is composed only of lowercase + * kebab-case static segments or single `:parameterName` segments — no empty or repeated slashes, no + * query string or fragment, no backslash or percent-encoding, no dot segments, and no wildcard or + * catch-all segment, since none of those can ever match either segment grammar. Malformed input is + * rejected outright rather than normalized, so a module can never register a path Core silently + * reinterpreted. + */ +export const moduleRoutePathSchema = z + .string() + .max(MODULE_ROUTE_PATH_MAX_LENGTH) + .superRefine((value, context) => { + if ( + !value.startsWith("/") || + value.includes("\\") || + value.includes("%") || + value.includes("?") || + value.includes("#") + ) { + context.addIssue({ + code: "custom", + message: + "Module route paths must begin with / and must not contain backslashes, " + + "percent-encoding, query strings, or fragments.", + }); + return; + } + if (value === "/") { + return; + } + if (value.endsWith("/")) { + context.addIssue({ + code: "custom", + message: "Non-root module route paths must not end with /.", + }); + return; + } + + const segments = value.slice(1).split("/"); + const parameterNames = new Set(); + for (const segment of segments) { + const isParameter = segment.startsWith(":"); + const matchesGrammar = isParameter + ? parameterSegmentPattern.test(segment) + : staticSegmentPattern.test(segment); + if (!matchesGrammar) { + context.addIssue({ + code: "custom", + message: + "Module route path segments must be lowercase kebab-case static segments or a " + + "single :parameterName segment.", + }); + return; + } + if (isParameter) { + const parameterName = segment.slice(1); + if (parameterNames.has(parameterName)) { + context.addIssue({ + code: "custom", + message: "Module route path parameter names must be unique within the path.", + }); + return; + } + parameterNames.add(parameterName); + } + } + }); + +export type ModuleRoutePath = z.output; + +export function parseModuleRoutePath(input: unknown): ModuleRoutePath { + return moduleRoutePathSchema.parse(input); +} + +export function safeParseModuleRoutePath(input: unknown): z.ZodSafeParseResult { + return moduleRoutePathSchema.safeParse(input); +} + +/** + * Produces a deterministic key identifying a route's shape — method plus path with every parameter + * segment collapsed to one placeholder — so `/events/:eventId` and `/events/:id` collide under the + * same method exactly like the identical route they are, regardless of the parameter's chosen name. + * Takes an already-validated `ModuleRoutePath`; never re-validates or echoes the original path. + */ +export function moduleRouteShapeKey(method: ModuleHttpMethod, path: ModuleRoutePath): string { + if (path === "/") { + return `${method} /`; + } + const shapeSegments = path + .slice(1) + .split("/") + .map((segment) => (segment.startsWith(":") ? ":param" : segment)); + return `${method} /${shapeSegments.join("/")}`; +} + +/** + * What a module's route handler receives about the caller and the match — never a database + * connection, authorization-store, grant-management, storage, jobs, configuration, secrets, the + * Hono `Context`, or a raw Better Auth session. `principal` is already authorized by the time a + * handler runs (pass 2 mounts and enforces); this pass never invokes a handler at all. + */ +export interface ModuleRouteHandlerContext { + readonly moduleId: ModuleId; + readonly principal: EkklesiaPrincipal; + readonly params: Readonly>; +} + +/** A framework-neutral Fetch handler: standard `Request` in, standard `Response` out. The full + * query string remains available through `Request.url`; there is no competing query or body + * abstraction. */ +export type ModuleRouteHandler = ( + request: Request, + context: ModuleRouteHandlerContext, +) => Awaitable; + +/** + * What a module passes to `http.route(...)` during `register`. `permission` is a plain string here + * (never the branded `PermissionId`) so a module can write a literal permission id without an + * explicit parse call first; `http.route` validates and brands it. The type has exactly these four + * fields — a module cannot specify a baseline, mount prefix, absolute path, CORS/origin policy, + * authentication behavior, grant/platform-owner state, middleware, audit metadata, another module's + * namespace, or an unprotected/no-permission route. + */ +export interface ModuleRouteDefinitionInput { + readonly method: ModuleHttpMethod; + readonly path: string; + readonly permission: string; + readonly handler: ModuleRouteHandler; +} + +/** The validated, canonical form of a `ModuleRouteDefinitionInput` — what `http.route` produces + * only after every field passes its own grammar. */ +export interface ModuleRouteDefinition { + readonly method: ModuleHttpMethod; + readonly path: ModuleRoutePath; + readonly permission: PermissionId; + readonly handler: ModuleRouteHandler; +} + +/** The Fetch-based registrar exposed as `context.http` during `register` only — never on + * `ModuleRuntimeContext`, so `start`/`stop` cannot register a route after the fact. */ +export interface ModuleHttpRouteRegistrar { + route(definition: ModuleRouteDefinitionInput): void; +} + +/** One published route in `StartedModuleRuntime`'s immutable route snapshot — the same validated + * fields as `ModuleRouteDefinition` plus the owning module's ID, which the production mount prefix + * (pass 2) uses to keep two modules' routes from ever colliding regardless of their own local + * paths. */ +export interface RegisteredModuleHttpRoute { + readonly moduleId: ModuleId; + readonly method: ModuleHttpMethod; + readonly path: ModuleRoutePath; + readonly permission: PermissionId; + readonly handler: ModuleRouteHandler; +} + +const moduleRouteDefinitionKeys = new Set(["method", "path", "permission", "handler"]); + +function isRecord(value: unknown): value is Record { + return typeof value === "object" && value !== null && !Array.isArray(value); +} + +function hasModuleRouteDefinitionShape( + value: unknown, +): value is { method: unknown; path: unknown; permission: unknown; handler: unknown } { + if (!isRecord(value)) { + return false; + } + const keys = Object.keys(value); + return ( + keys.length === moduleRouteDefinitionKeys.size && + keys.every((key) => moduleRouteDefinitionKeys.has(key)) && + typeof value.handler === "function" + ); +} + +export type ModuleRouteDefinitionIssueCode = + | "MODULE_ROUTE_DEFINITION_INVALID" + | "MODULE_ROUTE_METHOD_INVALID" + | "MODULE_ROUTE_PATH_INVALID" + | "MODULE_ROUTE_PERMISSION_INVALID"; + +export type ModuleRouteDefinitionParseResult = + | { readonly success: true; readonly data: ModuleRouteDefinition } + | { readonly success: false; readonly code: ModuleRouteDefinitionIssueCode }; + +/** + * Validates every field-level rule a route definition must satisfy on its own — exact shape, + * canonical method, canonical path, well-formed permission id — but never whether the permission is + * actually declared by the registering module. That ownership check needs the module's own manifest + * and belongs to the caller (Core's module runtime), not this framework-neutral contract. Never + * echoes the rejected input in the returned code. + */ +export function safeParseModuleRouteDefinition(value: unknown): ModuleRouteDefinitionParseResult { + if (!hasModuleRouteDefinitionShape(value)) { + return { success: false, code: "MODULE_ROUTE_DEFINITION_INVALID" }; + } + if (!isModuleHttpMethod(value.method)) { + return { success: false, code: "MODULE_ROUTE_METHOD_INVALID" }; + } + const parsedPath = moduleRoutePathSchema.safeParse(value.path); + if (!parsedPath.success) { + return { success: false, code: "MODULE_ROUTE_PATH_INVALID" }; + } + const parsedPermission = permissionIdSchema.safeParse(value.permission); + if (!parsedPermission.success) { + return { success: false, code: "MODULE_ROUTE_PERMISSION_INVALID" }; + } + return { + success: true, + data: Object.freeze({ + method: value.method, + path: parsedPath.data, + permission: parsedPermission.data, + handler: value.handler as ModuleRouteHandler, + }), + }; +} + +export function parseModuleRouteDefinition(value: unknown): ModuleRouteDefinition { + const result = safeParseModuleRouteDefinition(value); + if (!result.success) { + throw new TypeError("Invalid Ekklesia module route definition."); + } + return result.data; +} diff --git a/packages/contracts/src/module-runtime.ts b/packages/contracts/src/module-runtime.ts index 7bb16e7..e61d5c7 100644 --- a/packages/contracts/src/module-runtime.ts +++ b/packages/contracts/src/module-runtime.ts @@ -1,5 +1,6 @@ import type { CapabilityId, ModuleId } from "./identifiers.js"; import { moduleIdSchema } from "./identifiers.js"; +import type { ModuleHttpRouteRegistrar } from "./module-http.js"; import { semanticVersionSchema } from "./versions.js"; export type Awaitable = Value | PromiseLike; @@ -21,6 +22,10 @@ export interface CapabilityRegistrar extends CapabilityResolver { export interface ModuleRegistrationContext { readonly capabilities: CapabilityRegistrar; + /** Available only during `register` — never on `ModuleRuntimeContext` — so a route can only ever + * be declared while Core is still building the immutable startup snapshot, never added or removed + * afterward from `start`, `stop`, or any other post-registration code path. */ + readonly http: ModuleHttpRouteRegistrar; readonly moduleId: ModuleId; } diff --git a/packages/contracts/tests/module-http.test.ts b/packages/contracts/tests/module-http.test.ts new file mode 100644 index 0000000..ee281b1 --- /dev/null +++ b/packages/contracts/tests/module-http.test.ts @@ -0,0 +1,255 @@ +import { describe, expect, it } from "vitest"; +import { + isModuleHttpMethod, + MODULE_HTTP_METHODS, + type ModuleRouteDefinitionInput, + moduleRoutePathSchema, + moduleRouteShapeKey, + parseModuleRouteDefinition, + parseModuleRoutePath, + safeParseModuleRouteDefinition, + safeParseModuleRoutePath, +} from "../src/index.js"; + +function noopHandler(): Response { + return new Response(null, { status: 204 }); +} + +function validDefinition( + overrides: Partial = {}, +): ModuleRouteDefinitionInput { + return { + method: "GET", + path: "/events/:eventId", + permission: "calendar.events.read", + handler: noopHandler, + ...overrides, + }; +} + +describe("ModuleHttpMethod", () => { + it("accepts every supported method", () => { + for (const method of MODULE_HTTP_METHODS) { + expect(isModuleHttpMethod(method)).toBe(true); + } + expect(MODULE_HTTP_METHODS).toEqual(["DELETE", "GET", "PATCH", "POST", "PUT"]); + }); + + it("rejects lowercase, unknown, OPTIONS, HEAD, and wildcard methods", () => { + for (const method of ["get", "post", "TRACE", "OPTIONS", "HEAD", "*", "ALL", ""]) { + expect(isModuleHttpMethod(method)).toBe(false); + } + expect(isModuleHttpMethod(42)).toBe(false); + expect(isModuleHttpMethod(undefined)).toBe(false); + }); +}); + +describe("moduleRoutePathSchema", () => { + it("accepts the root, static, nested, and parameterized canonical paths", () => { + for (const path of [ + "/", + "/events", + "/events/upcoming", + "/events/:eventId", + "/events/:eventId/attendees/:attendeeId", + ]) { + expect(parseModuleRoutePath(path)).toBe(path); + } + }); + + it("rejects paths that do not begin with /", () => { + expect(safeParseModuleRoutePath("events").success).toBe(false); + expect(safeParseModuleRoutePath("").success).toBe(false); + }); + + it("rejects a non-root path ending with /", () => { + expect(safeParseModuleRoutePath("/events/").success).toBe(false); + }); + + it("rejects empty and repeated segments", () => { + expect(safeParseModuleRoutePath("/events//upcoming").success).toBe(false); + expect(safeParseModuleRoutePath("//events").success).toBe(false); + }); + + it("rejects query strings and fragments", () => { + expect(safeParseModuleRoutePath("/events?limit=10").success).toBe(false); + expect(safeParseModuleRoutePath("/events#upcoming").success).toBe(false); + }); + + it("rejects backslashes and percent-encoded path syntax", () => { + expect(safeParseModuleRoutePath("/events\\upcoming").success).toBe(false); + expect(safeParseModuleRoutePath("/events%2Fupcoming").success).toBe(false); + expect(safeParseModuleRoutePath("/%2e%2e/events").success).toBe(false); + }); + + it("rejects dot segments and traversal syntax", () => { + expect(safeParseModuleRoutePath("/events/..").success).toBe(false); + expect(safeParseModuleRoutePath("/events/.").success).toBe(false); + expect(safeParseModuleRoutePath("/../events").success).toBe(false); + }); + + it("rejects wildcard or catch-all segments", () => { + expect(safeParseModuleRoutePath("/events/*").success).toBe(false); + expect(safeParseModuleRoutePath("/events/**").success).toBe(false); + expect(safeParseModuleRoutePath("/*").success).toBe(false); + }); + + it("rejects non-kebab-case static segments", () => { + expect(safeParseModuleRoutePath("/Events").success).toBe(false); + expect(safeParseModuleRoutePath("/events_list").success).toBe(false); + expect(safeParseModuleRoutePath("/events--upcoming").success).toBe(false); + expect(safeParseModuleRoutePath("/-events").success).toBe(false); + expect(safeParseModuleRoutePath("/events-").success).toBe(false); + }); + + it("rejects a malformed parameter segment", () => { + expect(safeParseModuleRoutePath("/events/:").success).toBe(false); + expect(safeParseModuleRoutePath("/events/:Event-Id").success).toBe(false); + expect(safeParseModuleRoutePath("/events/:1id").success).toBe(false); + }); + + it("rejects duplicate parameter names within one path", () => { + expect(safeParseModuleRoutePath("/events/:id/attendees/:id").success).toBe(false); + }); + + it("rejects an unreasonably long path", () => { + expect(safeParseModuleRoutePath(`/${"a".repeat(300)}`).success).toBe(false); + }); + + it("never echoes the rejected raw path in its issue messages", () => { + const secret = "/machine/private/path/traversal-attempt-%2e%2e"; + const result = moduleRoutePathSchema.safeParse(secret); + expect(result.success).toBe(false); + expect(JSON.stringify(result.error?.issues)).not.toContain(secret); + }); +}); + +describe("moduleRouteShapeKey", () => { + it("collapses different parameter names into the same shape for the same method", () => { + const eventId = parseModuleRoutePath("/events/:eventId"); + const id = parseModuleRoutePath("/events/:id"); + expect(moduleRouteShapeKey("GET", eventId)).toBe(moduleRouteShapeKey("GET", id)); + }); + + it("distinguishes shapes by method and by static path structure", () => { + const path = parseModuleRoutePath("/events/:eventId"); + expect(moduleRouteShapeKey("GET", path)).not.toBe(moduleRouteShapeKey("POST", path)); + expect(moduleRouteShapeKey("GET", parseModuleRoutePath("/events"))).not.toBe( + moduleRouteShapeKey("GET", parseModuleRoutePath("/events/:eventId")), + ); + }); + + it("treats the root path as its own shape", () => { + expect(moduleRouteShapeKey("GET", parseModuleRoutePath("/"))).toBe("GET /"); + }); +}); + +describe("safeParseModuleRouteDefinition", () => { + it("accepts a well-formed definition and brands its path and permission", () => { + const result = safeParseModuleRouteDefinition(validDefinition()); + expect(result.success).toBe(true); + if (result.success) { + expect(result.data).toEqual({ + method: "GET", + path: "/events/:eventId", + permission: "calendar.events.read", + handler: noopHandler, + }); + expect(Object.isFrozen(result.data)).toBe(true); + } + }); + + it("rejects a non-object, null, or array definition", () => { + for (const value of [null, undefined, "route", 42, []]) { + expect(safeParseModuleRouteDefinition(value)).toEqual({ + success: false, + code: "MODULE_ROUTE_DEFINITION_INVALID", + }); + } + }); + + it("rejects an unknown route-definition field", () => { + expect(safeParseModuleRouteDefinition({ ...validDefinition(), baseline: "public" })).toEqual({ + success: false, + code: "MODULE_ROUTE_DEFINITION_INVALID", + }); + expect( + safeParseModuleRouteDefinition({ ...validDefinition(), mountPrefix: "/custom" }), + ).toEqual({ success: false, code: "MODULE_ROUTE_DEFINITION_INVALID" }); + }); + + it("rejects a missing required field", () => { + const { handler: _handler, ...withoutHandler } = validDefinition(); + expect(safeParseModuleRouteDefinition(withoutHandler)).toEqual({ + success: false, + code: "MODULE_ROUTE_DEFINITION_INVALID", + }); + }); + + it("rejects a non-function handler", () => { + expect( + safeParseModuleRouteDefinition({ ...validDefinition(), handler: "not-a-function" }), + ).toEqual({ success: false, code: "MODULE_ROUTE_DEFINITION_INVALID" }); + }); + + it("rejects an invalid method", () => { + expect(safeParseModuleRouteDefinition({ ...validDefinition(), method: "get" })).toEqual({ + success: false, + code: "MODULE_ROUTE_METHOD_INVALID", + }); + }); + + it("rejects an invalid path", () => { + expect(safeParseModuleRouteDefinition({ ...validDefinition(), path: "events" })).toEqual({ + success: false, + code: "MODULE_ROUTE_PATH_INVALID", + }); + }); + + it("rejects an invalid permission id", () => { + expect( + safeParseModuleRouteDefinition({ ...validDefinition(), permission: "not namespaced" }), + ).toEqual({ success: false, code: "MODULE_ROUTE_PERMISSION_INVALID" }); + expect(safeParseModuleRouteDefinition({ ...validDefinition(), permission: "*" })).toEqual({ + success: false, + code: "MODULE_ROUTE_PERMISSION_INVALID", + }); + }); + + it("does not retain the caller's mutable definition object: mutating it afterward cannot change the parsed result", () => { + const source = { ...validDefinition() }; + const result = safeParseModuleRouteDefinition(source); + expect(result.success).toBe(true); + source.method = "POST"; + source.path = "/mutated"; + source.permission = "mutated.permission.id"; + if (result.success) { + expect(result.data.method).toBe("GET"); + expect(result.data.path).toBe("/events/:eventId"); + expect(result.data.permission).toBe("calendar.events.read"); + } + }); +}); + +describe("parseModuleRouteDefinition", () => { + it("returns the parsed definition for valid input", () => { + expect(parseModuleRouteDefinition(validDefinition())).toMatchObject({ + method: "GET", + path: "/events/:eventId", + permission: "calendar.events.read", + }); + }); + + it("throws without echoing the rejected input", () => { + const secret = "secret-token-value"; + expect(() => parseModuleRouteDefinition({ ...validDefinition(), permission: secret })).toThrow( + "Invalid Ekklesia module route definition.", + ); + try { + parseModuleRouteDefinition({ ...validDefinition(), permission: secret }); + throw new Error("expected parseModuleRouteDefinition to throw"); + } catch (error) { + expect(String(error)).not.toContain(secret); + } + }); +}); diff --git a/packages/module-sdk/tests/define-module-runtime.test.ts b/packages/module-sdk/tests/define-module-runtime.test.ts index e53213f..4fd4106 100644 --- a/packages/module-sdk/tests/define-module-runtime.test.ts +++ b/packages/module-sdk/tests/define-module-runtime.test.ts @@ -1,5 +1,11 @@ import { describe, expect, it, vi } from "vitest"; -import { defineModuleRuntime } from "../src/index.js"; +import { + defineModuleRuntime, + isModuleHttpMethod, + MODULE_HTTP_METHODS, + type ModuleHttpRouteRegistrar, + type ModuleRegistrationContext, +} from "../src/index.js"; describe("defineModuleRuntime", () => { it("defines a frozen server runtime without duplicating the static manifest", async () => { @@ -22,4 +28,42 @@ describe("defineModuleRuntime", () => { }), ).toThrow("Invalid Ekklesia module runtime definition."); }); + + it("re-exports the module-http contract so a module's register hook can declare routes", async () => { + expect(MODULE_HTTP_METHODS).toContain("GET"); + expect(isModuleHttpMethod("GET")).toBe(true); + expect(isModuleHttpMethod("get")).toBe(false); + + const registeredRoutes: unknown[] = []; + const http: ModuleHttpRouteRegistrar = { + route: (definition) => { + registeredRoutes.push(definition); + }, + }; + const runtime = defineModuleRuntime({ + id: "example", + version: "0.1.0", + register({ http: registrar }) { + registrar.route({ + method: "GET", + path: "/records", + permission: "example.records.read", + handler: () => new Response(null, { status: 204 }), + }); + }, + }); + + const context: ModuleRegistrationContext = { + capabilities: { + provide: () => undefined, + resolve: () => undefined, + }, + http, + moduleId: "example", + }; + await runtime.register?.(context); + + expect(registeredRoutes).toHaveLength(1); + expect(registeredRoutes[0]).toMatchObject({ method: "GET", path: "/records" }); + }); }); diff --git a/templates/module/tests/manifest.test.ts b/templates/module/tests/manifest.test.ts index 0b3b1e3..32b429f 100644 --- a/templates/module/tests/manifest.test.ts +++ b/templates/module/tests/manifest.test.ts @@ -70,6 +70,11 @@ describe("module template", () => { return undefined; }, }, + http: { + route() { + throw new Error("The template does not register any HTTP route."); + }, + }, moduleId: "example", }); -- 2.54.0 From dfaf324a653eeed69820fdaf999dddf01c1efb9e Mon Sep 17 00:00:00 2001 From: ekklesia Date: Fri, 24 Jul 2026 19:16:20 -0500 Subject: [PATCH 2/5] feat: mount module HTTP routes Phase 5B, pass 2: production API mounting, authorization enforcement, and module-runtime lifecycle integration. apps/api now discovers the installed module inventory exactly once (prepareStartupModuleInventory), composes the platform permission registry from it, starts the module runtime from that same discovered list (startDiscoveredModuleRuntime, never rediscovering), and mounts StartedModuleRuntime's published routes under /api/v1/modules/ via a new apps/api-owned Hono adapter (module-http-routes.ts). Every module route is protected by the existing requirePermission middleware against the composed registry, state-changing methods additionally require a trusted Origin, OPTIONS remains entirely platform-owned, and a handler that throws, rejects, or returns a non-Response becomes a fixed, sanitized 500 MODULE_HANDLER_FAILED. Module-supplied CORS headers are stripped and every module response is forced to Cache-Control: no-store with Vary: Origin, Cookie. RunningApi shutdown now stops the module runtime between draining the HTTP server and closing the shared pool, and every startup failure path stops the runtime and/or closes the pool before propagating, without masking the original failure. Co-Authored-By: Claude Sonnet 5 --- README.md | 17 +- apps/api/src/app.ts | 35 +- apps/api/src/module-http-routes.ts | 153 ++++ apps/api/src/platform-permissions.ts | 61 +- apps/api/src/server.ts | 115 ++- apps/api/tests/api-http.integration.ts | 141 +++- apps/api/tests/app.test.ts | 15 + apps/api/tests/lifecycle.test.ts | 327 ++++++++ apps/api/tests/module-http-routes.test.ts | 936 ++++++++++++++++++++++ docs/development/api.md | 108 ++- docs/development/authorization.md | 126 ++- docs/module-development/README.md | 19 +- 12 files changed, 1930 insertions(+), 123 deletions(-) create mode 100644 apps/api/src/module-http-routes.ts create mode 100644 apps/api/tests/module-http-routes.test.ts diff --git a/README.md b/README.md index fab23bb..2256c2b 100644 --- a/README.md +++ b/README.md @@ -61,12 +61,25 @@ enforceable and grantable through the exact same `requirePermission` middleware claimable by a module. `pnpm doctor` and `pnpm modules:validate` both validate permission composition. Installing, removing, or editing a module currently requires an API restart; removing one makes its permission unavailable to new checks and new grants without deleting or revoking any -historical grant or audit row. Module HTTP route registration and relationship-derived grant -creation remain later work. See +historical grant or audit row. See [Authorization](docs/development/authorization.md#module-permission-composition), [Module development](docs/module-development/README.md), and [ADR 0014](docs/architecture/decisions/0014-module-permission-declarations-and-platform-registry.md). +Phase 5B mounts a module's own HTTP routes in production. A module registers framework-neutral +Fetch (`Request`/`Response`) route handlers during its existing `register` hook; Core collects them +into an immutable snapshot alongside its capabilities, and `apps/api` discovers modules exactly +once, starts the module runtime from that same discovered inventory, and mounts its published +routes under `/api/v1/modules/` — after every module has registered and started +successfully, so no request can reach a module before startup completes. Every route is enforced +through the identical `requirePermission` middleware and composed permission registry Core's own +routes use (a module can never supply its own baseline), state-changing methods require a trusted +`Origin`, and a handler failure or non-`Response` return becomes a fixed, sanitized `500` — never a +raw error, stack, path, or permission id. Relationship-derived grant creation remains later work. +See [API](docs/development/api.md#module-http-routes), +[Module development](docs/module-development/README.md#http-routes), and +[ADR 0014](docs/architecture/decisions/0014-module-permission-declarations-and-platform-registry.md#deferred). + ## Quick start Prerequisites: Node.js 24 LTS, pnpm 11.12, and Docker with Compose. diff --git a/apps/api/src/app.ts b/apps/api/src/app.ts index fce51be..3207bd0 100644 --- a/apps/api/src/app.ts +++ b/apps/api/src/app.ts @@ -1,4 +1,4 @@ -import { isAuthenticatedPrincipal } from "@ekklesia/contracts"; +import { isAuthenticatedPrincipal, type RegisteredModuleHttpRoute } from "@ekklesia/contracts"; import type { AuthorizationEvaluator, AuthorizationStore, @@ -9,23 +9,30 @@ import { Hono } from "hono"; import type { Context } from "hono"; import { cors } from "hono/cors"; import { createGrantsRoutes } from "./grants-routes.js"; +import { mountModuleHttpRoutes } from "./module-http-routes.js"; export interface CreateApiAppAuthorizationOptions { readonly evaluator: AuthorizationEvaluator; /** The complete platform permission registry (Core's own declarations plus every installed * module's) — always the caller-composed registry `startApi` produces via - * `composeStartupPermissionRegistry`, never a Core-only fallback constructed here. */ + * `prepareStartupModuleInventory`, never a Core-only fallback constructed here. */ readonly registry: PermissionRegistry; readonly store: AuthorizationStore; } export interface CreateApiAppOptions { readonly authorization: CreateApiAppAuthorizationOptions; + /** The started module runtime's published, immutable route snapshot — always the caller-started + * runtime's exact `routes` (`startDiscoveredModuleRuntime`, `apps/api/src/server.ts`), mounted in + * publication order under `/api/v1/modules/`. Required rather than defaulted so a + * caller not exercising modules must say so explicitly (an empty frozen array), never rely on an + * implicit fallback. */ + readonly moduleRoutes: readonly RegisteredModuleHttpRoute[]; readonly runtime: EkklesiaIdentityRuntime; /** Exact allowed browser origins. Never a wildcard; combined with `credentials: true`, only an * exact, reflected match is ever returned in `Access-Control-Allow-Origin`. Also the trusted set - * `requireTrustedOrigin` checks for grant-mutation routes — there is no second, competing origin - * configuration. */ + * `requireTrustedOrigin` checks for grant- and module-route-mutation routes — there is no second, + * competing origin configuration. */ readonly trustedOrigins: readonly string[]; } @@ -36,9 +43,14 @@ function sanitizedErrorResponse(c: Context, error: unknown, status: 500 | 503) { } /** - * Pure Hono application factory: no I/O, no environment reads, no process side effects. Suitable - * for unit and integration tests via `app.fetch` directly, with a fully injected identity runtime - * and authorization evaluator/store standing in for real Better Auth/PostgreSQL where appropriate. + * Pure Hono application factory: no I/O, no environment reads, no process side effects, and it + * never invokes a module route handler while composing routes. Suitable for unit and integration + * tests via `app.fetch` directly, with a fully injected identity runtime and authorization + * evaluator/store standing in for real Better Auth/PostgreSQL where appropriate. Synchronously + * throws `ModuleRouteConfigurationError` (`./module-http-routes.js`) if `moduleRoutes` and + * `authorization.registry` are inconsistent — a route naming a permission the registry does not + * register, which should never happen since both derive from the same discovered inventory, but is + * checked explicitly rather than silently mounting an unenforceable route. */ export function createApiApp(options: CreateApiAppOptions): Hono { const { runtime } = options; @@ -48,7 +60,7 @@ export function createApiApp(options: CreateApiAppOptions): Hono { const corsMiddleware = cors({ allowHeaders: ["Content-Type"], - allowMethods: ["GET", "POST", "OPTIONS"], + allowMethods: ["GET", "POST", "PUT", "PATCH", "DELETE", "OPTIONS"], credentials: true, maxAge: 600, origin: (origin) => (origin && allowedOrigins.has(origin) ? origin : undefined), @@ -91,6 +103,13 @@ export function createApiApp(options: CreateApiAppOptions): Hono { }), ); + mountModuleHttpRoutes(app, options.moduleRoutes, { + evaluator: options.authorization.evaluator, + registry: options.authorization.registry, + runtime, + trustedOrigins: allowedOrigins, + }); + app.notFound((c) => { c.header("Cache-Control", "no-store"); return c.json({ code: "NOT_FOUND" }, 404); diff --git a/apps/api/src/module-http-routes.ts b/apps/api/src/module-http-routes.ts new file mode 100644 index 0000000..a73a165 --- /dev/null +++ b/apps/api/src/module-http-routes.ts @@ -0,0 +1,153 @@ +import type { + ModuleId, + ModuleRouteHandlerContext, + RegisteredModuleHttpRoute, +} from "@ekklesia/contracts"; +import type { Context, Hono } from "hono"; +import { + type AuthorizationMiddlewareOptions, + getAuthorizedPrincipal, + requirePermission, + requireTrustedOrigin, +} from "./authorization-middleware.js"; + +const MODULE_ROUTE_PREFIX = "/api/v1/modules"; +const STATE_CHANGING_METHODS: ReadonlySet = new Set(["DELETE", "PATCH", "POST", "PUT"]); +const STRIPPED_CORS_RESPONSE_HEADERS = [ + "access-control-allow-credentials", + "access-control-allow-headers", + "access-control-allow-methods", + "access-control-allow-origin", + "access-control-expose-headers", + "access-control-max-age", +]; + +export interface ModuleHttpRoutesOptions extends AuthorizationMiddlewareOptions { + readonly trustedOrigins: ReadonlySet; +} + +/** Never carries the offending permission id, route path, module id, or any other configuration + * detail — a forged or inconsistent route snapshot is a startup-time composition bug, not + * something to describe in detail to whatever eventually logs it. */ +export class ModuleRouteConfigurationError extends Error { + constructor() { + super("A published module HTTP route references a permission that is not registered."); + this.name = "ModuleRouteConfigurationError"; + this.stack = `${this.name}: ${this.message}`; + Object.freeze(this); + } +} + +/** "/" is the module's own root and maps to the bare module prefix, never a required trailing + * slash; every other local path appends directly. Root and non-root therefore never collide, and + * `/api/v1/modules//` (the root path with a trailing slash) is never produced here. */ +function moduleRoutePath(moduleId: ModuleId, routePath: string): string { + return routePath === "/" + ? `${MODULE_ROUTE_PREFIX}/${moduleId}` + : `${MODULE_ROUTE_PREFIX}/${moduleId}${routePath}`; +} + +function sanitizedModuleHandlerFailure(c: Context): Response { + c.header("Cache-Control", "no-store"); + return c.json({ code: "MODULE_HANDLER_FAILED" }, 500); +} + +/** + * The platform owns CORS and caching for every module route: strips any CORS-control header a + * handler set (defense in depth — the outer `cors()` middleware already computes the authoritative + * values for the actual response) and forces `Cache-Control: no-store` plus a `Vary` that always + * includes `Origin` and `Cookie`, since even a public-baseline handler receives an authorized + * principal and may personalize its response. A separate cache-policy contract is future work; + * this is the conservative default until then. A module's own legitimate `Vary` values (if any) + * are preserved alongside the forced ones. + */ +function sanitizeModuleResponse(response: Response): Response { + const headers = new Headers(response.headers); + for (const headerName of STRIPPED_CORS_RESPONSE_HEADERS) { + headers.delete(headerName); + } + headers.set("Cache-Control", "no-store"); + const varyValues = new Set( + (headers.get("Vary") ?? "") + .split(",") + .map((value) => value.trim()) + .filter((value) => value.length > 0), + ); + varyValues.add("Origin"); + varyValues.add("Cookie"); + headers.set("Vary", [...varyValues].join(", ")); + return new Response(response.body, { + headers, + status: response.status, + statusText: response.statusText, + }); +} + +/** + * Mounts `StartedModuleRuntime`'s published, immutable route snapshot into `app`, in publication + * order — never sorted or mutated, and never invoking a handler while mounting. Every route is + * protected by the exact same `requirePermission` middleware Core's own routes use, with + * `route.permission` as the only permission id ever consulted and its baseline coming exclusively + * from `options.registry`; a state-changing method (`POST`/`PUT`/`PATCH`/`DELETE`) additionally + * requires `requireTrustedOrigin` before authorization ever runs. `GET` inherits Hono's own + * implicit `HEAD` handling, which re-enters this exact same middleware chain and strips the + * response body afterward — there is no separate `HEAD` registration or permission path. + * `route.handler` is invoked only after authorization succeeds, with the raw Fetch `Request` and a + * frozen context — never a Hono `Context`, database, store, or configuration. + * + * Because the routes and registry both derive from the same discovered inventory + * (`prepareStartupModuleInventory`), every registered route permission should already be + * registered; this loop still checks that invariant explicitly and throws + * `ModuleRouteConfigurationError` before mounting anything further if it ever does not hold, so a + * forged or hand-built route snapshot fails app construction instead of silently mounting an + * unenforceable route. `requirePermission`'s own per-request registry lookup remains the live, + * defense-in-depth enforcement regardless. + */ +export function mountModuleHttpRoutes( + app: Hono, + routes: readonly RegisteredModuleHttpRoute[], + options: ModuleHttpRoutesOptions, +): void { + const middlewareOptions: AuthorizationMiddlewareOptions = { + evaluator: options.evaluator, + registry: options.registry, + runtime: options.runtime, + }; + const requireOrigin = requireTrustedOrigin(options.trustedOrigins); + + for (const route of routes) { + if (!options.registry.has(route.permission)) { + throw new ModuleRouteConfigurationError(); + } + + const requireRoutePermission = requirePermission(middlewareOptions, route.permission); + const path = moduleRoutePath(route.moduleId, route.path); + const finalHandler = async (c: Context): Promise => { + const context: ModuleRouteHandlerContext = Object.freeze({ + moduleId: route.moduleId, + params: Object.freeze({ ...c.req.param() }), + principal: getAuthorizedPrincipal(c), + }); + + let response: Response; + try { + response = await route.handler(c.req.raw, context); + } catch { + return sanitizedModuleHandlerFailure(c); + } + if (!(response instanceof Response)) { + return sanitizedModuleHandlerFailure(c); + } + return sanitizeModuleResponse(response); + }; + + // Hono's `on()` overloads resolve by literal call arity, not by the runtime length of a spread + // array — each branch below is therefore a distinct, fixed-arity call rather than one call + // spreading a variable-length handlers array. + if (STATE_CHANGING_METHODS.has(route.method)) { + app.on(route.method, path, requireOrigin, requireRoutePermission, finalHandler); + } else { + app.on(route.method, path, requireRoutePermission, finalHandler); + } + } +} diff --git a/apps/api/src/platform-permissions.ts b/apps/api/src/platform-permissions.ts index 707a38e..dd57ca8 100644 --- a/apps/api/src/platform-permissions.ts +++ b/apps/api/src/platform-permissions.ts @@ -54,7 +54,7 @@ async function readPackageVersion(packagePath: string): Promise { return packageData.version; } -export interface ComposeStartupPermissionRegistryOptions { +export interface PrepareStartupModuleInventoryOptions { /** Test-only escape hatch: overrides the directory module discovery scans for installed * modules. Never set by the executable entry point — production always discovers the real * repository's `modules/` directory. Platform and Module SDK versions are always read from the @@ -62,7 +62,8 @@ export interface ComposeStartupPermissionRegistryOptions { readonly modulesDirectoryOverride?: string; /** Test-only escape hatch: supplies an already-discovered module list directly, skipping the * filesystem discovery step entirely, while still running the real `composePlatformPermissionRegistry` - * on it. Exists because a genuine exact permission-id collision between two modules is + * on it — and, if startup progresses that far, driving module runtime startup with this exact + * list too. Exists because a genuine exact permission-id collision between two modules is * unreachable through real discovery: module ids are unique (discovery itself rejects a * duplicate) and every permission id is namespaced by its own module's id, so no two * really-discovered modules can ever contribute the same permission id. Proving @@ -76,18 +77,35 @@ export interface ComposeStartupPermissionRegistryOptions { readonly discoveredModulesOverride?: readonly DiscoveredModule[]; } +export type ComposeStartupPermissionRegistryOptions = PrepareStartupModuleInventoryOptions; + +export interface StartupModuleInventory { + /** The exact discovered-module list — in discovery order — permission composition ran against. + * Production startup (`apps/api/src/server.ts`) passes this same list to + * `startDiscoveredModuleRuntime` rather than letting the module runtime rediscover modules a + * second time. */ + readonly modules: readonly DiscoveredModule[]; + readonly permissionRegistry: PermissionRegistry; +} + /** - * Discovers every installed module's manifest and composes the one immutable platform permission - * registry (Core's own declarations plus every module's) — the production source of truth - * `apps/api` enforces every request against. Only manifests are read; no module server entrypoint - * is imported or executed, and no module lifecycle hook runs. Read the platform and Module SDK - * versions only when this function actually executes, never at import time. + * Discovers every installed module's manifest exactly once and composes the one immutable platform + * permission registry (Core's own declarations plus every module's) from that exact discovered + * list — the production source of truth `apps/api` enforces every request against, and the exact + * module inventory `startDiscoveredModuleRuntime` starts from. Only manifests are read here; no + * module server entrypoint is imported or executed, and no module lifecycle hook runs. Read the + * platform and Module SDK versions only when this function actually executes, never at import + * time. */ -export async function composeStartupPermissionRegistry( - options: ComposeStartupPermissionRegistryOptions = {}, -): Promise { +export async function prepareStartupModuleInventory( + options: PrepareStartupModuleInventoryOptions = {}, +): Promise { if (options.discoveredModulesOverride) { - return composePlatformPermissionRegistry(options.discoveredModulesOverride); + const modules = options.discoveredModulesOverride; + return Object.freeze({ + modules, + permissionRegistry: composePlatformPermissionRegistry(modules), + }); } const repositoryRoot = resolveRepositoryRoot(); @@ -102,5 +120,24 @@ export async function composeStartupPermissionRegistry( platformVersion, }); - return composePlatformPermissionRegistry(modules); + return Object.freeze({ + modules, + permissionRegistry: composePlatformPermissionRegistry(modules), + }); +} + +/** + * Compatible wrapper for callers — currently only existing tests — that need this package's + * composed registry without the discovered module list itself. (Doctor and `modules:validate` are + * Core-owned and never call into `apps/api` at all; they call Core's own + * `composePlatformPermissionRegistry` directly over their own already-discovered module list.) + * Production `startApi` does not call this wrapper: it calls `prepareStartupModuleInventory` + * directly so it can hand the exact same discovered list to `startDiscoveredModuleRuntime`, rather + * than discarding it here and rediscovering modules a second time for runtime startup. + */ +export async function composeStartupPermissionRegistry( + options: ComposeStartupPermissionRegistryOptions = {}, +): Promise { + const inventory = await prepareStartupModuleInventory(options); + return inventory.permissionRegistry; } diff --git a/apps/api/src/server.ts b/apps/api/src/server.ts index 754df1b..59ef64e 100644 --- a/apps/api/src/server.ts +++ b/apps/api/src/server.ts @@ -5,13 +5,17 @@ import { createAuthorizationStore, } from "@ekklesia/core/authorization"; import { createIdentityRuntime, type IdentityRateLimitOverride } from "@ekklesia/core/identity"; -import type { DiscoveredModule } from "@ekklesia/core/modules"; +import { + type DiscoveredModule, + type StartedModuleRuntime, + startDiscoveredModuleRuntime, +} from "@ekklesia/core/modules"; import { serve, type ServerType } from "@hono/node-server"; import type { Hono } from "hono"; import { createApiApp } from "./app.js"; import { type ApiRuntimeConfig, createApiRuntimeConfig } from "./config.js"; import { type ApiDatabaseHandle, createApiDatabase } from "./database.js"; -import { composeStartupPermissionRegistry } from "./platform-permissions.js"; +import { prepareStartupModuleInventory } from "./platform-permissions.js"; const connectionTimeoutMs = 10_000; const operationTimeoutMs = 10_000; @@ -26,12 +30,13 @@ export interface StartApiOptions { /** Test-only escape hatch: never set by the executable entry point. See * `IdentityRateLimitOverride` for why production must leave this unset. */ readonly rateLimitOverride?: IdentityRateLimitOverride; - /** Test-only escape hatch: overrides the directory permission-composition module discovery - * scans. Never set by the executable entry point. See `composeStartupPermissionRegistry`. */ + /** Test-only escape hatch: overrides the directory module discovery scans. Never set by the + * executable entry point. See `prepareStartupModuleInventory`. */ readonly modulesDirectoryOverride?: string; - /** Test-only escape hatch: supplies an already-discovered module list directly to permission - * composition, skipping filesystem discovery entirely. Never set by the executable entry - * point. See `composeStartupPermissionRegistry`. */ + /** Test-only escape hatch: supplies an already-discovered module list directly, skipping + * filesystem discovery entirely — this exact list drives both permission composition and module + * runtime startup. Never set by the executable entry point. See + * `prepareStartupModuleInventory`. */ readonly discoveredModulesOverride?: readonly DiscoveredModule[]; } @@ -44,6 +49,17 @@ async function safeCloseDatabase(handle: ApiDatabaseHandle): Promise { } } +/** Same swallow-on-cleanup-path rule as `safeCloseDatabase`, used only while unwinding a startup + * failure that already occurred after the module runtime started — never during normal shutdown, + * where a module stop failure remains an observable result instead (see `startApi`'s `shutdown`). */ +async function safeStopModuleRuntime(moduleRuntime: StartedModuleRuntime): Promise { + try { + await moduleRuntime.stop(); + } catch { + // See safeCloseDatabase: never mask the startup failure already propagating. + } +} + function bindServer(app: Hono, host: string, port: number): Promise { return new Promise((resolve, reject) => { let settled = false; @@ -63,25 +79,36 @@ function bindServer(app: Hono, host: string, port: number): Promise } /** - * Explicit runtime composition, in a fixed order: discover every installed module's manifest and - * compose the one immutable platform permission registry from Core's own declarations plus every - * module's (`./platform-permissions.js`); only then construct the one shared PostgreSQL pool - * (identity and authorization each get a typed Drizzle view over it — see `./database.js`), the - * Core identity runtime, the Core authorization store and evaluator, and the Hono application; - * finally start the Node HTTP server. Permission composition happens before any pool, Better - * Auth instance, or HTTP server exists, so a discovery or composition failure — an invalid + * Explicit runtime composition, in a fixed order: + * + * 1. Discover every installed module's manifest exactly once and compose the one immutable + * platform permission registry from Core's own declarations plus every module's + * (`prepareStartupModuleInventory`, `./platform-permissions.js`). + * 2. Construct the one shared PostgreSQL pool (identity and authorization each get a typed + * Drizzle view over it — see `./database.js`). + * 3. Construct the Core identity runtime, authorization store, and evaluator. + * 4. Start the module runtime from that exact same discovered inventory + * (`startDiscoveredModuleRuntime`) — never rediscovering modules a second time. + * 5. Create the Hono application, mounting the started module runtime's published routes. + * 6. Bind the Node HTTP server. + * + * Discovery and permission composition happen before any pool, Better Auth instance, module + * entrypoint import, or HTTP server exists, so a discovery or composition failure — an invalid * manifest, a dependency-graph problem, a module claiming the reserved `core.*` permission * namespace, or a duplicate permission declaration — leaves nothing to close: it propagates - * directly, and startup never falls back to a Core-only registry. Returns a handle with an - * idempotent `shutdown()` so both the executable entry point and tests can stop the server and - * close the shared pool deterministically. Never applies migrations or generates schema — the - * database must already be migrated through `pnpm migrations:apply`. + * directly, and startup never falls back to a Core-only registry. Core infrastructure (the pool, + * identity, authorization) exists before any executable module lifecycle code runs, and every + * module route is published only after the entire module runtime has registered and started + * successfully, so no request can ever reach a module before every module has started. Returns a + * handle with an idempotent `shutdown()` so both the executable entry point and tests can stop the + * server, the module runtime, and the shared pool deterministically. Never applies migrations or + * generates schema — the database must already be migrated through `pnpm migrations:apply`. */ export async function startApi( config: ApiRuntimeConfig, options: StartApiOptions = {}, ): Promise { - const permissionRegistry = await composeStartupPermissionRegistry({ + const inventory = await prepareStartupModuleInventory({ ...(options.modulesDirectoryOverride === undefined ? {} : { modulesDirectoryOverride: options.modulesDirectoryOverride }), @@ -114,16 +141,40 @@ export async function startApi( platformOwners: authorizationStore, }); - const app = createApiApp({ - authorization: { evaluator, registry: permissionRegistry, store: authorizationStore }, - runtime, - trustedOrigins: config.identity.trustedOrigins, - }); + // Module load/register/start failures use Core's own existing rollback (already-started + // modules are stopped, in reverse order, before this rejects) — there is no separate runtime + // handle to stop here, only the pool this failure leaves open. + let moduleRuntime: StartedModuleRuntime; + try { + moduleRuntime = await startDiscoveredModuleRuntime(inventory.modules); + } catch (error) { + await safeCloseDatabase(databaseHandle); + throw error; + } + + let app: Hono; + try { + app = createApiApp({ + authorization: { + evaluator, + registry: inventory.permissionRegistry, + store: authorizationStore, + }, + moduleRoutes: moduleRuntime.routes, + runtime, + trustedOrigins: config.identity.trustedOrigins, + }); + } catch (error) { + await safeStopModuleRuntime(moduleRuntime); + await safeCloseDatabase(databaseHandle); + throw error; + } let server: ServerType; try { server = await bindServer(app, config.host, config.port); } catch (error) { + await safeStopModuleRuntime(moduleRuntime); await safeCloseDatabase(databaseHandle); throw error; } @@ -133,7 +184,12 @@ export async function startApi( // would previously get an already-resolved `undefined` immediately, before the listener had // actually drained or the pool had actually closed. Storing the promise itself means every // caller — concurrent or sequential — awaits the exact same drain-then-close operation and - // observes it complete (or fail) exactly once. + // observes it complete (or fail) exactly once. Order matters: the HTTP listener is drained and + // closed first so no new or in-flight request can reach a module handler while the module + // runtime is stopping, then the module runtime stops, then the shared pool closes. Unlike the + // startup-cleanup paths above, a module-stop failure here is not swallowed — it is a genuine + // shutdown result a caller should observe — but database cleanup still always runs first, so a + // stop failure never leaves the pool open. let shutdownPromise: Promise | undefined; const shutdown = (): Promise => { if (!shutdownPromise) { @@ -141,7 +197,16 @@ export async function startApi( await new Promise((resolve) => { server.close(() => resolve()); }); + let moduleStopError: unknown; + try { + await moduleRuntime.stop(); + } catch (error) { + moduleStopError = error; + } await safeCloseDatabase(databaseHandle); + if (moduleStopError) { + throw moduleStopError; + } })(); } return shutdownPromise; diff --git a/apps/api/tests/api-http.integration.ts b/apps/api/tests/api-http.integration.ts index 902ae1b..c25af5a 100644 --- a/apps/api/tests/api-http.integration.ts +++ b/apps/api/tests/api-http.integration.ts @@ -200,6 +200,7 @@ async function applyCoreMigrations(url: string): Promise { } const trackedModulesDirectories: string[] = []; +const eventSymbols: symbol[] = []; async function createFixtureModulesDirectory(): Promise { const directory = await mkdtemp(path.join(tmpdir(), "ekklesia-api-installed-modules-")); @@ -207,13 +208,31 @@ async function createFixtureModulesDirectory(): Promise { return directory; } -/** Writes a minimal, structurally valid module fixture straight to disk — real discovery and - * composition run against it, nothing hand-built. The server entrypoint throws immediately if - * ever imported/executed, so a scenario that completes without that throw firing is direct - * evidence permission composition and Doctor never ran module lifecycle code. */ +/** A dynamically imported module fixture cannot close over this test file's local variables, so + * lifecycle markers cross that boundary through a `globalThis` symbol instead (same technique as + * `core/tests/module-runtime.test.ts`). */ +function createEventRecorder(): { events: string[]; expression: string } { + const key = `ekklesia.api.integration.test.${eventSymbols.length}.${Date.now()}.${Math.random()}`; + const symbol = Symbol.for(key); + const events: string[] = []; + (globalThis as unknown as Record)[symbol] = events; + eventSymbols.push(symbol); + return { events, expression: `globalThis[Symbol.for(${JSON.stringify(key)})]` }; +} + +/** Writes a minimal, structurally valid module fixture straight to disk — real discovery, + * composition, and (where a scenario actually starts the API) real module runtime execution all + * run against it, nothing hand-built. Register/start/stop push markers onto `recorder.events` + * instead of throwing, so both "Doctor/composition never executed this" (an empty recorder) and + * "startApi actually imported, registered, and started this" (a populated recorder) can be proven + * against the exact same fixture. When declared with the `calendar.events.read` permission, the + * module also registers a restricted `GET /events/:eventId` route and a restricted, state-changing + * `POST /events` route against that same permission, so the full HTTP mounting/enforcement chain + * can be exercised live. */ async function createFixtureModule( modulesDirectory: string, id: string, + recorder: { expression: string }, manifestOverrides: Record = {}, ): Promise { const moduleDirectory = path.join(modulesDirectory, id); @@ -227,6 +246,9 @@ async function createFixtureModule( entrypoints: { server: "./dist/server/index.js" }, ...manifestOverrides, }; + const declaresCalendarRead = (manifest.permissions as ReadonlyArray<{ id: string }>).some( + (declaration) => declaration.id === "calendar.events.read", + ); await mkdir(path.join(moduleDirectory, "dist/server"), { recursive: true }); await Promise.all([ writeFile( @@ -241,7 +263,39 @@ async function createFixtureModule( writeFile(path.join(moduleDirectory, "ekklesia.module.json"), JSON.stringify(manifest)), writeFile( path.join(moduleDirectory, "dist/server/index.js"), - 'throw new Error("permission composition and Doctor must never execute a module server entrypoint");', + `export default { + id: ${JSON.stringify(id)}, + version: "0.1.0", + register({ http }) { + ${recorder.expression}.push(${JSON.stringify(`${id}:register`)}); + ${ + declaresCalendarRead + ? `http.route({ + method: "GET", + path: "/events/:eventId", + permission: "calendar.events.read", + async handler(request, context) { + return Response.json({ eventId: context.params.eventId, principal: context.principal }); + }, + }); + http.route({ + method: "POST", + path: "/events", + permission: "calendar.events.read", + async handler(request, context) { + return Response.json({ created: true, principal: context.principal }, { status: 201 }); + }, + });` + : "" + } + }, + start() { + ${recorder.expression}.push(${JSON.stringify(`${id}:start`)}); + }, + stop() { + ${recorder.expression}.push(${JSON.stringify(`${id}:stop`)}); + }, + };`, ), ]); } @@ -253,6 +307,10 @@ afterAll(async () => { for (const directory of trackedModulesDirectories.splice(0)) { await rm(directory, { recursive: true, force: true }).catch(() => undefined); } + const globalValues = globalThis as unknown as Record; + for (const symbol of eventSymbols.splice(0)) { + delete globalValues[symbol]; + } }); const identityBaseUrl = "https://identity.example.test"; @@ -1222,10 +1280,11 @@ describe.sequential("live API — installed module permission end-to-end", () => const password = "correct horse battery staple"; const modulePermission = "calendar.events.read"; const absentPermission = "scheduler.jobs.read"; + const recorder = createEventRecorder(); it("recognizes a real installed module and passes permission composition in Doctor, without executing its server entrypoint", async () => { modulesDirectory = await createFixtureModulesDirectory(); - await createFixtureModule(modulesDirectory, "calendar", { + await createFixtureModule(modulesDirectory, "calendar", recorder, { permissions: [ { baseline: "restricted", @@ -1257,14 +1316,21 @@ describe.sequential("live API — installed module permission end-to-end", () => details: ["2 permissions registered"], status: "pass", }); + // Doctor and permission composition read only the manifest — the fixture's register/start + // markers must remain entirely unset. + expect(recorder.events).toEqual([]); }); - it("starts a freshly migrated API that discovers the fixture module and signs up its eventual owner", async () => { + it("starts a freshly migrated API that discovers the fixture module, imports and starts its runtime exactly once, and mounts its route", async () => { database = await createDisposableDatabase("authz_module"); await applyCoreMigrations(database.url); running = await startMigratedApi(database.url, modulesDirectory); origin = apiOrigin(running); + // Unlike Doctor above, a successful production startup does deliberately import, register, + // and start the module's runtime — exactly once each. + expect(recorder.events).toEqual(["calendar:register", "calendar:start"]); + const ownerSignUp = await signUp(origin, ownerEmail, password); ownerUserId = ownerSignUp.body.user.id; const ownerSignIn = await signIn(origin, ownerEmail, password); @@ -1316,6 +1382,56 @@ describe.sequential("live API — installed module permission end-to-end", () => } }); + it("denies the module's own mounted route to an anonymous caller, then serves it to the newly granted owner with the decoded path parameter and authenticated principal", async () => { + const anonymousResponse = await fetch(`${origin}/api/v1/modules/calendar/events/evt-42`); + expect(anonymousResponse.status).toBe(403); + expect(await anonymousResponse.json()).toEqual({ code: "AUTHORIZATION_DENIED" }); + + const grantedResponse = await fetch(`${origin}/api/v1/modules/calendar/events/evt-42`, { + headers: { cookie: ownerCookie }, + }); + expect(grantedResponse.status).toBe(200); + const body = (await grantedResponse.json()) as { + eventId: string; + principal: { kind: string; userId: string }; + }; + expect(body.eventId).toBe("evt-42"); + expect(body.principal).toEqual({ kind: "authenticated", userId: ownerUserId }); + }); + + it("requires a trusted Origin for the module's own state-changing route, exactly like the platform's own mutation routes", async () => { + const noOriginResponse = await fetch(`${origin}/api/v1/modules/calendar/events`, { + body: JSON.stringify({}), + headers: { "content-type": "application/json", cookie: ownerCookie }, + method: "POST", + }); + expect(noOriginResponse.status).toBe(403); + expect(await noOriginResponse.json()).toEqual({ code: "UNTRUSTED_ORIGIN" }); + + const untrustedResponse = await fetch(`${origin}/api/v1/modules/calendar/events`, { + body: JSON.stringify({}), + headers: { + "content-type": "application/json", + cookie: ownerCookie, + origin: untrustedOrigin, + }, + method: "POST", + }); + expect(untrustedResponse.status).toBe(403); + + const trustedResponse = await fetch(`${origin}/api/v1/modules/calendar/events`, { + body: JSON.stringify({}), + headers: { + "content-type": "application/json", + cookie: ownerCookie, + origin: dashboardOrigin, + }, + method: "POST", + }); + expect(trustedResponse.status).toBe(201); + expect(await trustedResponse.json()).toMatchObject({ created: true }); + }); + it("rejects a well-formed permission absent from the composed registry, writing no grant or audit row", async () => { const admin = await openAdministrativeClient(database.url); try { @@ -1355,15 +1471,24 @@ describe.sequential("live API — installed module permission end-to-end", () => } }); - it("shuts down cleanly, then restarting with the module absent rejects new grants for its former permission while the historical grant remains listable", async () => { + it("shuts down cleanly, then restarting with the module absent rejects new grants for its former permission, returns NOT_FOUND for its former route, while the historical grant remains listable", async () => { await running.shutdown(); const remainingConnections = await activeConnectionCount(database.name); expect(remainingConnections).toBe(0); + // Exactly one stop, matching the exactly-one register/start already asserted above. + expect(recorder.events.filter((event) => event === "calendar:stop")).toHaveLength(1); const emptyModulesDirectory = await createFixtureModulesDirectory(); running = await startMigratedApi(database.url, emptyModulesDirectory); origin = apiOrigin(running); + // The module's own route is unmounted along with the module itself. + const removedRouteResponse = await fetch(`${origin}/api/v1/modules/calendar/events/evt-42`, { + headers: { cookie: ownerCookie }, + }); + expect(removedRouteResponse.status).toBe(404); + expect(await removedRouteResponse.json()).toEqual({ code: "NOT_FOUND" }); + // The owner's existing session survives the restart (sessions live in the database, not the // process), so the same cookie proves ordinary identity behavior is unaffected here too. const listResponse = await fetch(`${origin}/api/v1/grants?userId=${ownerUserId}`, { diff --git a/apps/api/tests/app.test.ts b/apps/api/tests/app.test.ts index a7d9e27..a2bbe26 100644 --- a/apps/api/tests/app.test.ts +++ b/apps/api/tests/app.test.ts @@ -52,6 +52,7 @@ describe("Ekklesia API Hono application", () => { it("mounts Better Auth exactly under /api/auth/*", async () => { const runtime = fakeRuntime(); const app = createApiApp({ + moduleRoutes: Object.freeze([]), authorization: fakeAuthorization({}), runtime, trustedOrigins: [trustedOrigin], @@ -67,6 +68,7 @@ describe("Ekklesia API Hono application", () => { it("does not route requests outside /api/auth/* to Better Auth", async () => { const runtime = fakeRuntime(); const app = createApiApp({ + moduleRoutes: Object.freeze([]), authorization: fakeAuthorization({}), runtime, trustedOrigins: [trustedOrigin], @@ -83,6 +85,7 @@ describe("Ekklesia API Hono application", () => { resolvePrincipal: vi.fn().mockResolvedValue(ANONYMOUS_PRINCIPAL), }); const app = createApiApp({ + moduleRoutes: Object.freeze([]), authorization: fakeAuthorization({}), runtime, trustedOrigins: [trustedOrigin], @@ -101,6 +104,7 @@ describe("Ekklesia API Hono application", () => { resolvePrincipal: vi.fn().mockResolvedValue(createAuthenticatedPrincipal("user_123")), }); const app = createApiApp({ + moduleRoutes: Object.freeze([]), authorization: fakeAuthorization({}), runtime, trustedOrigins: [trustedOrigin], @@ -121,6 +125,7 @@ describe("Ekklesia API Hono application", () => { .mockRejectedValue(new IdentityRuntimeError("PRINCIPAL_RESOLUTION_FAILED")), }); const app = createApiApp({ + moduleRoutes: Object.freeze([]), authorization: fakeAuthorization({}), runtime, trustedOrigins: [trustedOrigin], @@ -138,6 +143,7 @@ describe("Ekklesia API Hono application", () => { handleAuthRequest: vi.fn().mockRejectedValue(new Error("postgresql://admin:secret@db/x")), }); const app = createApiApp({ + moduleRoutes: Object.freeze([]), authorization: fakeAuthorization({}), runtime, trustedOrigins: [trustedOrigin], @@ -154,6 +160,7 @@ describe("Ekklesia API Hono application", () => { it("has deterministic 404 behavior for an unknown route", async () => { const app = createApiApp({ + moduleRoutes: Object.freeze([]), authorization: fakeAuthorization({}), runtime: fakeRuntime(), trustedOrigins: [trustedOrigin], @@ -167,6 +174,7 @@ describe("Ekklesia API Hono application", () => { it("reflects an allowed origin in Access-Control-Allow-Origin with credentials support", async () => { const app = createApiApp({ + moduleRoutes: Object.freeze([]), authorization: fakeAuthorization({}), runtime: fakeRuntime(), trustedOrigins: [trustedOrigin], @@ -182,6 +190,7 @@ describe("Ekklesia API Hono application", () => { it("never returns Access-Control-Allow-Origin for a disallowed origin", async () => { const app = createApiApp({ + moduleRoutes: Object.freeze([]), authorization: fakeAuthorization({}), runtime: fakeRuntime(), trustedOrigins: [trustedOrigin], @@ -196,6 +205,7 @@ describe("Ekklesia API Hono application", () => { it("supports requests with no Origin header at all (native/server clients)", async () => { const app = createApiApp({ + moduleRoutes: Object.freeze([]), authorization: fakeAuthorization({}), runtime: fakeRuntime(), trustedOrigins: [trustedOrigin], @@ -208,6 +218,7 @@ describe("Ekklesia API Hono application", () => { it("handles preflight OPTIONS requests deterministically for an allowed origin", async () => { const app = createApiApp({ + moduleRoutes: Object.freeze([]), authorization: fakeAuthorization({}), runtime: fakeRuntime(), trustedOrigins: [trustedOrigin], @@ -227,6 +238,7 @@ describe("Ekklesia API Hono application", () => { it("never wildcards Access-Control-Allow-Origin", async () => { const app = createApiApp({ + moduleRoutes: Object.freeze([]), authorization: fakeAuthorization({}), runtime: fakeRuntime(), trustedOrigins: [trustedOrigin], @@ -249,6 +261,7 @@ describe("Ekklesia API Hono application", () => { it("mounts the v1 grants routes under /api/v1/grants", async () => { const evaluatorAuthorize = vi.fn().mockResolvedValue({ allowed: false }); const app = createApiApp({ + moduleRoutes: Object.freeze([]), authorization: fakeAuthorization({ evaluator: { authorize: evaluatorAuthorize } }), runtime: fakeRuntime(), trustedOrigins: [trustedOrigin], @@ -274,6 +287,7 @@ describe("Ekklesia API Hono application", () => { has: () => false, }; const app = createApiApp({ + moduleRoutes: Object.freeze([]), authorization: fakeAuthorization({ evaluator: { authorize: evaluatorAuthorize }, registry: emptyRegistry, @@ -290,6 +304,7 @@ describe("Ekklesia API Hono application", () => { it("applies the same CORS policy to /api/v1/grants as the rest of /api/v1/*", async () => { const app = createApiApp({ + moduleRoutes: Object.freeze([]), authorization: fakeAuthorization({}), runtime: fakeRuntime(), trustedOrigins: [trustedOrigin], diff --git a/apps/api/tests/lifecycle.test.ts b/apps/api/tests/lifecycle.test.ts index c58cdae..b404456 100644 --- a/apps/api/tests/lifecycle.test.ts +++ b/apps/api/tests/lifecycle.test.ts @@ -13,6 +13,13 @@ vi.mock("pg", () => ({ Pool: poolConstructor })); vi.mock("better-auth", () => ({ betterAuth: betterAuthMock })); vi.mock("better-auth/adapters/drizzle", () => ({ drizzleAdapter: drizzleAdapterMock })); vi.mock("@hono/node-server", () => ({ serve: serveMock })); +// Wraps the real `createApiApp` in a spy rather than replacing it, so every existing test still +// gets the real Hono application; only the one test that deliberately forces an app-construction +// failure overrides this with `mockImplementationOnce`. +vi.mock("../src/app.js", async (importOriginal) => { + const actual = await importOriginal(); + return { ...actual, createApiApp: vi.fn(actual.createApiApp) }; +}); import { parseModuleManifest } from "@ekklesia/contracts"; import { @@ -21,6 +28,7 @@ import { ModulePermissionCompositionError, } from "@ekklesia/core/modules"; import { PermissionRegistryError } from "@ekklesia/core/authorization"; +import { createApiApp } from "../src/app.js"; import { createApiRuntimeConfig } from "../src/config.js"; import { installShutdownSignalHandlers, startApi } from "../src/server.js"; @@ -87,6 +95,76 @@ async function createFixtureModule( ]); } +const eventSymbols: symbol[] = []; + +/** Same technique as `core/tests/module-runtime.test.ts`: a dynamically imported module fixture + * cannot close over this test file's local variables, so lifecycle events cross that boundary + * through a `globalThis` symbol instead. Returns the same `events` array the fixture's generated + * source pushes into, so test code can both read it directly and push its own markers (pool + * close, server close) onto the identical array to observe cross-cutting shutdown order. */ +function createEventRecorder(): { events: string[]; expression: string } { + const key = `ekklesia.api.lifecycle.test.${eventSymbols.length}.${Date.now()}.${Math.random()}`; + const symbol = Symbol.for(key); + const events: string[] = []; + (globalThis as unknown as Record)[symbol] = events; + eventSymbols.push(symbol); + return { events, expression: `globalThis[Symbol.for(${JSON.stringify(key)})]` }; +} + +interface WorkingFixtureModuleOptions { + /** Declares one `GET /ping` route against `.ping.read` when true. */ + readonly withRoute?: boolean; + readonly registerThrows?: boolean; + readonly startThrows?: boolean; + readonly stopThrows?: boolean; + readonly dependencies?: Record; +} + +/** Writes a real, working module fixture — unlike `createFixtureModule`, this entrypoint does not + * throw on import; it pushes lifecycle markers onto `recorder.events` and can be told to fail at a + * specific hook, so tests can prove exactly what a successful (or a specifically-failing) module + * runtime startup actually executes and in what order, relative to pool/server construction. */ +async function createWorkingFixtureModule( + modulesDirectory: string, + id: string, + recorder: { expression: string }, + options: WorkingFixtureModuleOptions = {}, +): Promise { + const permissionId = `${id}.ping.read`; + await createFixtureModule(modulesDirectory, id, { + permissions: options.withRoute + ? [{ baseline: "public", description: "Ping.", id: permissionId }] + : [], + ...(options.dependencies ? { dependencies: options.dependencies } : {}), + }); + const routeSource = options.withRoute + ? `http.route({ + method: "GET", + path: "/ping", + permission: ${JSON.stringify(permissionId)}, + handler: async () => new Response("pong", { status: 200 }), + });` + : ""; + const serverSource = `export default { + id: ${JSON.stringify(id)}, + version: "0.1.0", + register({ http }) { + ${recorder.expression}.push(${JSON.stringify(`${id}:register`)}); + ${routeSource} + ${options.registerThrows ? 'throw new Error("register failed");' : ""} + }, + start() { + ${recorder.expression}.push(${JSON.stringify(`${id}:start`)}); + ${options.startThrows ? 'throw new Error("start failed");' : ""} + }, + stop() { + ${recorder.expression}.push(${JSON.stringify(`${id}:stop`)}); + ${options.stopThrows ? 'throw new Error("stop failed");' : ""} + }, + };`; + await writeFile(path.join(modulesDirectory, id, "dist/server/index.js"), serverSource); +} + /** A minimal, in-memory `DiscoveredModule` double built from a real, validated manifest — used * only to reach `composePlatformPermissionRegistry`'s own duplicate-detection failure, which is * unreachable through real filesystem discovery (module ids are unique and every permission id is @@ -518,3 +596,252 @@ describe("Ekklesia API module permission composition at startup", () => { expect(poolConstructor).not.toHaveBeenCalled(); }); }); + +describe("Ekklesia API module runtime startup and shutdown", () => { + afterEach(async () => { + const globalValues = globalThis as unknown as Record; + for (const symbol of eventSymbols.splice(0)) { + delete globalValues[symbol]; + } + await Promise.all( + temporaryDirectories.splice(0).map((directory) => rm(directory, { recursive: true })), + ); + }); + + beforeEach(() => { + poolConstructor.mockReset(); + betterAuthMock.mockReset(); + drizzleAdapterMock.mockClear(); + serveMock.mockReset(); + vi.mocked(createApiApp).mockClear(); + }); + + function mockPoolAndAuth(): { poolEnd: ReturnType } { + const poolEnd = vi.fn().mockResolvedValue(undefined); + poolConstructor.mockImplementation(function fakePool() { + return { end: poolEnd, on: vi.fn() }; + }); + betterAuthMock.mockReturnValue({ api: { getSession: vi.fn() }, handler: vi.fn() }); + return { poolEnd }; + } + + it("starts the module runtime from the exact discovered inventory permission composition used, mounting routes in discovery order", async () => { + const modulesDirectory = await createTemporaryModulesDirectory(); + const recorder = createEventRecorder(); + await createWorkingFixtureModule(modulesDirectory, "alpha", recorder, { withRoute: true }); + await createWorkingFixtureModule(modulesDirectory, "beta", recorder, { withRoute: true }); + mockPoolAndAuth(); + mockSuccessfulServe(); + + const running = await startApi(validConfig(), { modulesDirectoryOverride: modulesDirectory }); + + expect(recorder.events).toEqual([ + "alpha:register", + "beta:register", + "alpha:start", + "beta:start", + ]); + // Hono records one internal route entry per handler in a route's middleware chain, not one + // per logical route, so entries for the same method/path are deduplicated (preserving first- + // seen order) before asserting mounting order. + const moduleRoutePaths = [ + ...new Set( + running.app.routes + .filter((route) => route.path.startsWith("/api/v1/modules/")) + .map((route) => `${route.method} ${route.path}`), + ), + ]; + expect(moduleRoutePaths).toEqual([ + "GET /api/v1/modules/alpha/ping", + "GET /api/v1/modules/beta/ping", + ]); + + await running.shutdown(); + }); + + it("closes the pool exactly once and binds no server when a module's entrypoint fails to load", async () => { + const modulesDirectory = await createTemporaryModulesDirectory(); + await createFixtureModule(modulesDirectory, "broken-export"); + await writeFile( + path.join(modulesDirectory, "broken-export", "dist/server/index.js"), + "export default { thisIsNotAValidRuntimeDefinition: true };", + ); + const { poolEnd } = mockPoolAndAuth(); + + await expect( + startApi(validConfig(), { modulesDirectoryOverride: modulesDirectory }), + ).rejects.toMatchObject({ + issues: [expect.objectContaining({ code: "MODULE_RUNTIME_EXPORT_INVALID" })], + }); + + expect(poolConstructor).toHaveBeenCalledTimes(1); + expect(poolEnd).toHaveBeenCalledTimes(1); + expect(serveMock).not.toHaveBeenCalled(); + }); + + it("closes the pool exactly once and binds no server when a module's register hook fails", async () => { + const modulesDirectory = await createTemporaryModulesDirectory(); + const recorder = createEventRecorder(); + await createWorkingFixtureModule(modulesDirectory, "register-fails", recorder, { + registerThrows: true, + }); + const { poolEnd } = mockPoolAndAuth(); + + await expect( + startApi(validConfig(), { modulesDirectoryOverride: modulesDirectory }), + ).rejects.toMatchObject({ + issues: [expect.objectContaining({ code: "MODULE_REGISTER_FAILED" })], + }); + + expect(recorder.events).toEqual(["register-fails:register"]); + expect(poolConstructor).toHaveBeenCalledTimes(1); + expect(poolEnd).toHaveBeenCalledTimes(1); + expect(serveMock).not.toHaveBeenCalled(); + }); + + it("closes the pool exactly once and binds no server when a module's start hook fails, preserving Core's rollback of already-started modules", async () => { + const modulesDirectory = await createTemporaryModulesDirectory(); + const recorder = createEventRecorder(); + await createWorkingFixtureModule(modulesDirectory, "one", recorder); + await createWorkingFixtureModule(modulesDirectory, "two", recorder, { + dependencies: { modules: { required: { one: "^0.1.0" } } }, + startThrows: true, + }); + const { poolEnd } = mockPoolAndAuth(); + + await expect( + startApi(validConfig(), { modulesDirectoryOverride: modulesDirectory }), + ).rejects.toMatchObject({ + issues: [expect.objectContaining({ code: "MODULE_START_FAILED" })], + }); + + expect(recorder.events).toEqual([ + "one:register", + "two:register", + "one:start", + "two:start", + "one:stop", + ]); + expect(poolConstructor).toHaveBeenCalledTimes(1); + expect(poolEnd).toHaveBeenCalledTimes(1); + expect(serveMock).not.toHaveBeenCalled(); + }); + + it("stops the module runtime and closes the pool, binding no server, when app construction fails", async () => { + const modulesDirectory = await createTemporaryModulesDirectory(); + const recorder = createEventRecorder(); + await createWorkingFixtureModule(modulesDirectory, "mounted", recorder); + const { poolEnd } = mockPoolAndAuth(); + vi.mocked(createApiApp).mockImplementationOnce(() => { + throw new Error("forced app construction failure"); + }); + + await expect( + startApi(validConfig(), { modulesDirectoryOverride: modulesDirectory }), + ).rejects.toThrow("forced app construction failure"); + + expect(recorder.events).toEqual(["mounted:register", "mounted:start", "mounted:stop"]); + expect(poolConstructor).toHaveBeenCalledTimes(1); + expect(poolEnd).toHaveBeenCalledTimes(1); + expect(serveMock).not.toHaveBeenCalled(); + }); + + it("stops the module runtime and closes the pool when the server fails to bind", async () => { + const modulesDirectory = await createTemporaryModulesDirectory(); + const recorder = createEventRecorder(); + await createWorkingFixtureModule(modulesDirectory, "mounted", recorder); + const { poolEnd } = mockPoolAndAuth(); + mockFailingServe(new Error("EADDRINUSE")); + + await expect( + startApi(validConfig(), { modulesDirectoryOverride: modulesDirectory }), + ).rejects.toThrow(); + + expect(recorder.events).toEqual(["mounted:register", "mounted:start", "mounted:stop"]); + expect(poolConstructor).toHaveBeenCalledTimes(1); + expect(poolEnd).toHaveBeenCalledTimes(1); + }); + + it("drains the HTTP server, then stops the module runtime, then closes the pool, in that exact order", async () => { + const modulesDirectory = await createTemporaryModulesDirectory(); + const recorder = createEventRecorder(); + await createWorkingFixtureModule(modulesDirectory, "ordered", recorder); + poolConstructor.mockImplementation(function fakePool() { + return { + end: vi.fn(async () => { + recorder.events.push("pool:end"); + }), + on: vi.fn(), + }; + }); + betterAuthMock.mockReturnValue({ api: { getSession: vi.fn() }, handler: vi.fn() }); + const { server } = mockSuccessfulServe(); + const originalClose = server.close; + server.close = vi.fn((callback?: () => void) => { + recorder.events.push("server:close"); + return originalClose(callback); + }); + + const running = await startApi(validConfig(), { modulesDirectoryOverride: modulesDirectory }); + recorder.events.length = 0; + await running.shutdown(); + + expect(recorder.events).toEqual(["server:close", "ordered:stop", "pool:end"]); + }); + + it("still runs pool cleanup and lets the shutdown rejection surface when the module runtime fails to stop", async () => { + const modulesDirectory = await createTemporaryModulesDirectory(); + const recorder = createEventRecorder(); + await createWorkingFixtureModule(modulesDirectory, "stop-fails", recorder, { + stopThrows: true, + }); + const { poolEnd } = mockPoolAndAuth(); + mockSuccessfulServe(); + + const running = await startApi(validConfig(), { modulesDirectoryOverride: modulesDirectory }); + + await expect(running.shutdown()).rejects.toMatchObject({ + issues: [expect.objectContaining({ code: "MODULE_STOP_FAILED", moduleId: "stop-fails" })], + }); + expect(poolEnd).toHaveBeenCalledTimes(1); + }); + + it("keeps a direct discoveredModulesOverride driving both permission composition and module runtime startup", async () => { + const modulesDirectory = await createTemporaryModulesDirectory(); + const recorder = createEventRecorder(); + await createWorkingFixtureModule(modulesDirectory, "calendar", recorder, { withRoute: true }); + const discovered = await (await import("@ekklesia/core/modules")).discoverModules({ + moduleSdkVersion: "0.1.0", + modulesDirectory, + platformVersion: "0.1.0", + }); + mockPoolAndAuth(); + mockSuccessfulServe(); + + const running = await startApi(validConfig(), { discoveredModulesOverride: discovered }); + + expect(recorder.events).toEqual(["calendar:register", "calendar:start"]); + // Hono records one internal route entry per handler in a route's middleware chain, not one + // per logical route, so entries for the same method/path are deduplicated (preserving first- + // seen order) before asserting mounting order. + const moduleRoutePaths = [ + ...new Set( + running.app.routes + .filter((route) => route.path.startsWith("/api/v1/modules/")) + .map((route) => `${route.method} ${route.path}`), + ), + ]; + expect(moduleRoutePaths).toEqual(["GET /api/v1/modules/calendar/ping"]); + + await running.shutdown(); + }); + + it("performs no filesystem I/O, module import, pool construction, Better Auth construction, or server binding merely by importing the module-route adapter", async () => { + const module = await import("../src/module-http-routes.js"); + + expect(typeof module.mountModuleHttpRoutes).toBe("function"); + expect(poolConstructor).not.toHaveBeenCalled(); + expect(betterAuthMock).not.toHaveBeenCalled(); + expect(serveMock).not.toHaveBeenCalled(); + }); +}); diff --git a/apps/api/tests/module-http-routes.test.ts b/apps/api/tests/module-http-routes.test.ts new file mode 100644 index 0000000..ca38c82 --- /dev/null +++ b/apps/api/tests/module-http-routes.test.ts @@ -0,0 +1,936 @@ +import { + ANONYMOUS_PRINCIPAL, + createAuthenticatedPrincipal, + type ModuleRouteHandler, + type ModuleRouteHandlerContext, + parsePermissionId, + type PermissionId, + type RegisteredModuleHttpRoute, +} from "@ekklesia/contracts"; +import { + type AuthorizationEvaluator, + type AuthorizationStore, + createPermissionRegistry, + type PermissionRegistry, +} from "@ekklesia/core/authorization"; +import type { EkklesiaIdentityRuntime } from "@ekklesia/core/identity"; +import { describe, expect, it, vi } from "vitest"; +import { createApiApp } from "../src/app.js"; +import { ModuleRouteConfigurationError } from "../src/module-http-routes.js"; + +const trustedOrigin = "https://dashboard.example.test"; +const untrustedOrigin = "https://evil.example.test"; +const readPermission = parsePermissionId("calendar.events.read"); +const writePermission = parsePermissionId("calendar.events.write"); + +function fakeRuntime(overrides: Partial = {}): EkklesiaIdentityRuntime { + return { + handleAuthRequest: vi.fn(), + resolvePrincipal: vi.fn().mockResolvedValue(ANONYMOUS_PRINCIPAL), + ...overrides, + }; +} + +function fakeStore(): AuthorizationStore { + return { + createGrant: vi.fn(), + grantPlatformOwner: vi.fn(), + hasActiveGrant: vi.fn().mockResolvedValue(false), + isPlatformOwner: vi.fn().mockResolvedValue(false), + listGrantsForUser: vi.fn().mockResolvedValue([]), + revokeGrant: vi.fn(), + revokePlatformOwner: vi.fn(), + }; +} + +function moduleRoute(fields: RegisteredModuleHttpRoute): RegisteredModuleHttpRoute { + return Object.freeze({ ...fields }); +} + +function buildModuleApp(options: { + readonly evaluator?: AuthorizationEvaluator; + readonly registry: PermissionRegistry; + readonly routes: readonly RegisteredModuleHttpRoute[]; + readonly runtime?: EkklesiaIdentityRuntime; + readonly trustedOrigins?: readonly string[]; +}) { + const evaluator = options.evaluator ?? { + authorize: vi.fn().mockResolvedValue({ allowed: true }), + }; + const runtime = options.runtime ?? fakeRuntime(); + const app = createApiApp({ + authorization: { evaluator, registry: options.registry, store: fakeStore() }, + moduleRoutes: options.routes, + runtime, + trustedOrigins: options.trustedOrigins ?? [trustedOrigin], + }); + return { app, evaluator, runtime }; +} + +function permissionRegistry( + declarations: ReadonlyArray<{ + readonly id: PermissionId; + readonly baseline: "authenticated" | "public" | "restricted"; + }>, +): PermissionRegistry { + return createPermissionRegistry( + declarations.map((declaration) => ({ ...declaration, description: "Test permission." })), + ); +} + +describe("module HTTP route mounting", () => { + it('mounts a "/" route at exactly the module root, never with an added trailing slash', async () => { + const handler = vi.fn(async () => new Response("root", { status: 200 })); + const { app } = buildModuleApp({ + registry: permissionRegistry([{ id: readPermission, baseline: "public" }]), + routes: [ + moduleRoute({ + handler, + method: "GET", + moduleId: "calendar", + path: "/", + permission: readPermission, + }), + ], + }); + + const rootResponse = await app.request("/api/v1/modules/calendar"); + expect(rootResponse.status).toBe(200); + expect(await rootResponse.text()).toBe("root"); + expect(handler).toHaveBeenCalledTimes(1); + + const trailingSlashResponse = await app.request("/api/v1/modules/calendar/"); + expect(trailingSlashResponse.status).toBe(404); + }); + + it("mounts static and nested paths directly under the module prefix", async () => { + const staticHandler = vi.fn(async () => Response.json({ kind: "static" })); + const nestedHandler = vi.fn(async () => Response.json({ kind: "nested" })); + const { app } = buildModuleApp({ + registry: permissionRegistry([{ id: readPermission, baseline: "public" }]), + routes: [ + moduleRoute({ + handler: staticHandler, + method: "GET", + moduleId: "calendar", + path: "/events", + permission: readPermission, + }), + moduleRoute({ + handler: nestedHandler, + method: "GET", + moduleId: "calendar", + path: "/events/upcoming/soon", + permission: readPermission, + }), + ], + }); + + expect(await (await app.request("/api/v1/modules/calendar/events")).json()).toEqual({ + kind: "static", + }); + expect( + await (await app.request("/api/v1/modules/calendar/events/upcoming/soon")).json(), + ).toEqual({ kind: "nested" }); + }); + + it("resolves a same-depth static path and its parameterized sibling by published route order, without silently reordering them", async () => { + // Hono's router matches overlapping patterns in registration order, not by specificity — + // mounting must therefore preserve `StartedModuleRuntime.routes`' exact publication order + // rather than sorting it, or a module author's more-specific-first declaration would silently + // stop taking precedence. The static route is declared first here, matching how a module + // author would order these two routes to get the static path resolved on its own. + const staticHandler = vi.fn(async () => Response.json({ kind: "static" })); + const paramHandler = vi.fn(async (_request, context) => + Response.json({ id: context.params.eventId, kind: "param" }), + ); + const { app } = buildModuleApp({ + registry: permissionRegistry([{ id: readPermission, baseline: "public" }]), + routes: [ + moduleRoute({ + handler: staticHandler, + method: "GET", + moduleId: "calendar", + path: "/events/upcoming", + permission: readPermission, + }), + moduleRoute({ + handler: paramHandler, + method: "GET", + moduleId: "calendar", + path: "/events/:eventId", + permission: readPermission, + }), + ], + }); + + expect(await (await app.request("/api/v1/modules/calendar/events/upcoming")).json()).toEqual({ + kind: "static", + }); + expect(staticHandler).toHaveBeenCalledTimes(1); + expect(paramHandler).not.toHaveBeenCalled(); + + expect(await (await app.request("/api/v1/modules/calendar/events/other-id")).json()).toEqual({ + id: "other-id", + kind: "param", + }); + expect(paramHandler).toHaveBeenCalledTimes(1); + }); + + it("mounts every supported method", async () => { + const methods = ["GET", "POST", "PUT", "PATCH", "DELETE"] as const; + const permissions = new Map( + methods.map((method) => [method, parsePermissionId(`multi.${method.toLowerCase()}.action`)]), + ); + const { app } = buildModuleApp({ + registry: permissionRegistry( + methods.map((method) => ({ + baseline: "public" as const, + id: permissions.get(method) as PermissionId, + })), + ), + routes: methods.map((method) => + moduleRoute({ + handler: async () => Response.json({ method }), + method, + moduleId: "multi", + path: "/action", + permission: permissions.get(method) as PermissionId, + }), + ), + }); + + for (const method of methods) { + const response = await app.request("/api/v1/modules/multi/action", { + headers: method === "GET" ? {} : { origin: trustedOrigin }, + method, + }); + expect(response.status).toBe(200); + expect(await response.json()).toEqual({ method }); + } + }); + + it("keeps two modules' identical local method/path routes independent", async () => { + const alphaPermission = parsePermissionId("alpha.items.list"); + const betaPermission = parsePermissionId("beta.items.list"); + const { app } = buildModuleApp({ + registry: permissionRegistry([ + { id: alphaPermission, baseline: "public" }, + { id: betaPermission, baseline: "public" }, + ]), + routes: [ + moduleRoute({ + handler: async () => Response.json({ moduleId: "alpha" }), + method: "GET", + moduleId: "alpha", + path: "/items", + permission: alphaPermission, + }), + moduleRoute({ + handler: async () => Response.json({ moduleId: "beta" }), + method: "GET", + moduleId: "beta", + path: "/items", + permission: betaPermission, + }), + ], + }); + + expect(await (await app.request("/api/v1/modules/alpha/items")).json()).toEqual({ + moduleId: "alpha", + }); + expect(await (await app.request("/api/v1/modules/beta/items")).json()).toEqual({ + moduleId: "beta", + }); + }); +}); + +describe("module route handler adaptation", () => { + it("adapts the request into a raw Fetch Request and a frozen context with decoded parameters", async () => { + let capturedRequest: Request | undefined; + let capturedContext: ModuleRouteHandlerContext | undefined; + const handler: ModuleRouteHandler = async (request, context) => { + capturedRequest = request; + capturedContext = context; + const body = await request.text(); + return Response.json({ body, url: request.url }); + }; + const runtime = fakeRuntime({ + resolvePrincipal: vi.fn().mockResolvedValue(createAuthenticatedPrincipal("user_1")), + }); + const { app } = buildModuleApp({ + registry: permissionRegistry([{ id: readPermission, baseline: "public" }]), + routes: [ + moduleRoute({ + handler, + method: "POST", + moduleId: "calendar", + path: "/events/:eventId/rsvp", + permission: readPermission, + }), + ], + runtime, + }); + + const response = await app.request("/api/v1/modules/calendar/events/evt-1/rsvp?notify=true", { + body: "hello", + headers: { origin: trustedOrigin, "x-custom": "abc" }, + method: "POST", + }); + const body = (await response.json()) as { body: string; url: string }; + + expect(response.status).toBe(200); + expect(body.body).toBe("hello"); + expect(body.url).toContain("/api/v1/modules/calendar/events/evt-1/rsvp?notify=true"); + expect(capturedRequest).toBeInstanceOf(Request); + expect(capturedRequest?.headers.get("x-custom")).toBe("abc"); + expect(capturedContext).toEqual({ + moduleId: "calendar", + params: { eventId: "evt-1" }, + principal: { kind: "authenticated", userId: "user_1" }, + }); + expect(Object.isFrozen(capturedContext)).toBe(true); + expect(Object.isFrozen(capturedContext?.params)).toBe(true); + }); + + it("gives static and root routes a frozen, empty params object", async () => { + let rootContext: ModuleRouteHandlerContext | undefined; + let staticContext: ModuleRouteHandlerContext | undefined; + const { app } = buildModuleApp({ + registry: permissionRegistry([{ id: readPermission, baseline: "public" }]), + routes: [ + moduleRoute({ + handler: async (_request, context) => { + rootContext = context; + return Response.json({ ok: true }); + }, + method: "GET", + moduleId: "calendar", + path: "/", + permission: readPermission, + }), + moduleRoute({ + handler: async (_request, context) => { + staticContext = context; + return Response.json({ ok: true }); + }, + method: "GET", + moduleId: "calendar", + path: "/events", + permission: readPermission, + }), + ], + }); + + await app.request("/api/v1/modules/calendar"); + await app.request("/api/v1/modules/calendar/events"); + + expect(rootContext?.params).toEqual({}); + expect(Object.isFrozen(rootContext?.params)).toBe(true); + expect(staticContext?.params).toEqual({}); + expect(Object.isFrozen(staticContext?.params)).toBe(true); + }); +}); + +describe("module route authorization", () => { + it("uses the registry's exact baseline for public, authenticated, and restricted module permissions", async () => { + const publicPermission = parsePermissionId("calendar.events.list"); + const authenticatedPermission = parsePermissionId("calendar.events.subscribe"); + const restrictedPermission = parsePermissionId("calendar.events.read"); + const evaluator: AuthorizationEvaluator = { + authorize: vi.fn().mockResolvedValue({ allowed: true }), + }; + const { app } = buildModuleApp({ + evaluator, + registry: permissionRegistry([ + { id: publicPermission, baseline: "public" }, + { id: authenticatedPermission, baseline: "authenticated" }, + { id: restrictedPermission, baseline: "restricted" }, + ]), + routes: [ + moduleRoute({ + handler: async () => Response.json({ ok: true }), + method: "GET", + moduleId: "calendar", + path: "/public", + permission: publicPermission, + }), + moduleRoute({ + handler: async () => Response.json({ ok: true }), + method: "GET", + moduleId: "calendar", + path: "/authenticated", + permission: authenticatedPermission, + }), + moduleRoute({ + handler: async () => Response.json({ ok: true }), + method: "GET", + moduleId: "calendar", + path: "/restricted", + permission: restrictedPermission, + }), + ], + }); + + await app.request("/api/v1/modules/calendar/public"); + await app.request("/api/v1/modules/calendar/authenticated"); + await app.request("/api/v1/modules/calendar/restricted"); + + expect(evaluator.authorize).toHaveBeenNthCalledWith( + 1, + expect.objectContaining({ baseline: "public", permission: publicPermission }), + ); + expect(evaluator.authorize).toHaveBeenNthCalledWith( + 2, + expect.objectContaining({ baseline: "authenticated", permission: authenticatedPermission }), + ); + expect(evaluator.authorize).toHaveBeenNthCalledWith( + 3, + expect.objectContaining({ baseline: "restricted", permission: restrictedPermission }), + ); + }); + + it("does not invoke the handler when authorization denies", async () => { + const handler = vi.fn(async () => Response.json({ ok: true })); + const evaluator: AuthorizationEvaluator = { + authorize: vi.fn().mockResolvedValue({ allowed: false }), + }; + const { app } = buildModuleApp({ + evaluator, + registry: permissionRegistry([{ id: readPermission, baseline: "restricted" }]), + routes: [ + moduleRoute({ + handler, + method: "GET", + moduleId: "calendar", + path: "/events", + permission: readPermission, + }), + ], + }); + + const response = await app.request("/api/v1/modules/calendar/events"); + expect(response.status).toBe(403); + expect(await response.json()).toEqual({ code: "AUTHORIZATION_DENIED" }); + expect(handler).not.toHaveBeenCalled(); + }); + + it("does not invoke the handler when principal resolution fails", async () => { + const handler = vi.fn(async () => Response.json({ ok: true })); + const runtime = fakeRuntime({ + resolvePrincipal: vi.fn().mockRejectedValue(new Error("connection terminated")), + }); + const { app } = buildModuleApp({ + registry: permissionRegistry([{ id: readPermission, baseline: "restricted" }]), + routes: [ + moduleRoute({ + handler, + method: "GET", + moduleId: "calendar", + path: "/events", + permission: readPermission, + }), + ], + runtime, + }); + + const response = await app.request("/api/v1/modules/calendar/events"); + expect(response.status).toBe(503); + expect(handler).not.toHaveBeenCalled(); + }); + + it("does not invoke the handler when the evaluator itself fails", async () => { + const handler = vi.fn(async () => Response.json({ ok: true })); + const evaluator: AuthorizationEvaluator = { + authorize: vi.fn().mockRejectedValue(new Error("connection terminated")), + }; + const { app } = buildModuleApp({ + evaluator, + registry: permissionRegistry([{ id: readPermission, baseline: "restricted" }]), + routes: [ + moduleRoute({ + handler, + method: "GET", + moduleId: "calendar", + path: "/events", + permission: readPermission, + }), + ], + }); + + const response = await app.request("/api/v1/modules/calendar/events"); + expect(response.status).toBe(503); + expect(handler).not.toHaveBeenCalled(); + }); + + it("fails app construction safely when a route names a permission the registry does not register", () => { + expect(() => + buildModuleApp({ + registry: permissionRegistry([]), + routes: [ + moduleRoute({ + handler: vi.fn(), + method: "GET", + moduleId: "calendar", + path: "/events", + permission: readPermission, + }), + ], + }), + ).toThrow(ModuleRouteConfigurationError); + }); + + it("never leaks the offending permission, route path, or module id in the configuration error", () => { + let captured: unknown; + try { + buildModuleApp({ + registry: permissionRegistry([]), + routes: [ + moduleRoute({ + handler: vi.fn(), + method: "GET", + moduleId: "secret-module", + path: "/secret-path", + permission: parsePermissionId("secret-module.secret.permission"), + }), + ], + }); + } catch (error) { + captured = error; + } + + expect(captured).toBeInstanceOf(ModuleRouteConfigurationError); + const text = `${(captured as Error).message}\n${(captured as Error).stack}`; + expect(text).not.toContain("secret-module"); + expect(text).not.toContain("secret-path"); + expect(text).not.toContain("secret.permission"); + }); +}); + +describe("module route trusted-origin enforcement", () => { + it("rejects a state-changing module route with no or untrusted Origin before authorization runs", async () => { + const handler = vi.fn(async () => Response.json({ ok: true })); + const evaluator: AuthorizationEvaluator = { + authorize: vi.fn().mockResolvedValue({ allowed: true }), + }; + const methods = ["POST", "PUT", "PATCH", "DELETE"] as const; + const { app } = buildModuleApp({ + evaluator, + registry: permissionRegistry([{ id: writePermission, baseline: "public" }]), + routes: methods.map((method) => + moduleRoute({ + handler, + method, + moduleId: "calendar", + path: "/events", + permission: writePermission, + }), + ), + }); + + for (const method of methods) { + const noOriginResponse = await app.request("/api/v1/modules/calendar/events", { method }); + expect(noOriginResponse.status).toBe(403); + expect(await noOriginResponse.json()).toEqual({ code: "UNTRUSTED_ORIGIN" }); + + const untrustedResponse = await app.request("/api/v1/modules/calendar/events", { + headers: { origin: untrustedOrigin }, + method, + }); + expect(untrustedResponse.status).toBe(403); + } + expect(handler).not.toHaveBeenCalled(); + expect(evaluator.authorize).not.toHaveBeenCalled(); + }); + + it("allows a trusted-origin state-changing request through to the handler", async () => { + const handler = vi.fn(async () => Response.json({ ok: true })); + const { app } = buildModuleApp({ + registry: permissionRegistry([{ id: writePermission, baseline: "public" }]), + routes: [ + moduleRoute({ + handler, + method: "POST", + moduleId: "calendar", + path: "/events", + permission: writePermission, + }), + ], + }); + + const response = await app.request("/api/v1/modules/calendar/events", { + headers: { origin: trustedOrigin }, + method: "POST", + }); + expect(response.status).toBe(200); + expect(handler).toHaveBeenCalledTimes(1); + }); + + it("does not require an Origin header for a GET module route", async () => { + const handler = vi.fn(async () => Response.json({ ok: true })); + const { app } = buildModuleApp({ + registry: permissionRegistry([{ id: readPermission, baseline: "public" }]), + routes: [ + moduleRoute({ + handler, + method: "GET", + moduleId: "calendar", + path: "/events", + permission: readPermission, + }), + ], + }); + + const response = await app.request("/api/v1/modules/calendar/events"); + expect(response.status).toBe(200); + expect(handler).toHaveBeenCalledTimes(1); + }); +}); + +describe("module route CORS", () => { + it("advertises GET, POST, PUT, PATCH, and DELETE on preflight", async () => { + const { app } = buildModuleApp({ + registry: permissionRegistry([{ id: writePermission, baseline: "public" }]), + routes: [ + moduleRoute({ + handler: vi.fn(), + method: "PUT", + moduleId: "calendar", + path: "/events", + permission: writePermission, + }), + ], + }); + + const response = await app.request("/api/v1/modules/calendar/events", { + headers: { "access-control-request-method": "PUT", origin: trustedOrigin }, + method: "OPTIONS", + }); + + expect(response.status).toBe(204); + const allowedMethods = response.headers.get("access-control-allow-methods") ?? ""; + for (const method of ["GET", "POST", "PUT", "PATCH", "DELETE"]) { + expect(allowedMethods).toContain(method); + } + }); + + it("never invokes authorization or the handler for an OPTIONS preflight", async () => { + const handler = vi.fn(async () => Response.json({ ok: true })); + const evaluator: AuthorizationEvaluator = { + authorize: vi.fn().mockResolvedValue({ allowed: true }), + }; + const { app } = buildModuleApp({ + evaluator, + registry: permissionRegistry([{ id: writePermission, baseline: "public" }]), + routes: [ + moduleRoute({ + handler, + method: "POST", + moduleId: "calendar", + path: "/events", + permission: writePermission, + }), + ], + }); + + await app.request("/api/v1/modules/calendar/events", { + headers: { "access-control-request-method": "POST", origin: trustedOrigin }, + method: "OPTIONS", + }); + + expect(handler).not.toHaveBeenCalled(); + expect(evaluator.authorize).not.toHaveBeenCalled(); + }); + + it("never reflects an untrusted origin for a module route", async () => { + const { app } = buildModuleApp({ + registry: permissionRegistry([{ id: readPermission, baseline: "public" }]), + routes: [ + moduleRoute({ + handler: async () => Response.json({ ok: true }), + method: "GET", + moduleId: "calendar", + path: "/events", + permission: readPermission, + }), + ], + }); + + const response = await app.request("/api/v1/modules/calendar/events", { + headers: { origin: untrustedOrigin }, + }); + expect(response.headers.get("access-control-allow-origin")).toBeNull(); + }); +}); + +describe("module route handler failure sanitization", () => { + it("returns a fixed sanitized 500 when the handler throws, never leaking the caught message", async () => { + const handler = vi.fn(async () => { + throw new Error("password=hunter2 host=db.internal.example"); + }); + const { app } = buildModuleApp({ + registry: permissionRegistry([{ id: readPermission, baseline: "public" }]), + routes: [ + moduleRoute({ + handler, + method: "GET", + moduleId: "calendar", + path: "/events", + permission: readPermission, + }), + ], + }); + + const response = await app.request("/api/v1/modules/calendar/events"); + const text = await response.text(); + + expect(response.status).toBe(500); + expect(JSON.parse(text)).toEqual({ code: "MODULE_HANDLER_FAILED" }); + expect(text).not.toContain("hunter2"); + expect(text).not.toContain("db.internal.example"); + expect(response.headers.get("cache-control")).toBe("no-store"); + }); + + it("returns the same fixed sanitized 500 when the handler rejects", async () => { + const handler = vi.fn(() => Promise.reject(new Error("rejected"))); + const { app } = buildModuleApp({ + registry: permissionRegistry([{ id: readPermission, baseline: "public" }]), + routes: [ + moduleRoute({ + handler, + method: "GET", + moduleId: "calendar", + path: "/events", + permission: readPermission, + }), + ], + }); + + const response = await app.request("/api/v1/modules/calendar/events"); + expect(response.status).toBe(500); + expect(await response.json()).toEqual({ code: "MODULE_HANDLER_FAILED" }); + }); + + it("returns the same fixed sanitized 500 when the handler returns a non-Response value", async () => { + const handler = vi.fn(async () => ({ ok: true }) as unknown as Response); + const { app } = buildModuleApp({ + registry: permissionRegistry([{ id: readPermission, baseline: "public" }]), + routes: [ + moduleRoute({ + handler, + method: "GET", + moduleId: "calendar", + path: "/events", + permission: readPermission, + }), + ], + }); + + const response = await app.request("/api/v1/modules/calendar/events"); + expect(response.status).toBe(500); + expect(await response.json()).toEqual({ code: "MODULE_HANDLER_FAILED" }); + }); + + it("passes through a valid module-owned 4xx/5xx Response unchanged", async () => { + const handler = vi.fn(async () => + Response.json({ code: "MODULE_NOT_FOUND" }, { status: 404 }), + ); + const { app } = buildModuleApp({ + registry: permissionRegistry([{ id: readPermission, baseline: "public" }]), + routes: [ + moduleRoute({ + handler, + method: "GET", + moduleId: "calendar", + path: "/events", + permission: readPermission, + }), + ], + }); + + const response = await app.request("/api/v1/modules/calendar/events"); + expect(response.status).toBe(404); + expect(await response.json()).toEqual({ code: "MODULE_NOT_FOUND" }); + }); +}); + +describe("module route response header policy", () => { + it("strips module-supplied CORS-control headers so the platform policy remains authoritative", async () => { + const handler = vi.fn( + async () => + new Response("body", { + headers: { + "access-control-allow-credentials": "true", + "access-control-allow-methods": "GET, POST, PUT, PATCH, DELETE", + "access-control-allow-origin": "*", + "access-control-expose-headers": "X-Secret", + }, + status: 200, + }), + ); + const { app } = buildModuleApp({ + registry: permissionRegistry([{ id: readPermission, baseline: "public" }]), + routes: [ + moduleRoute({ + handler, + method: "GET", + moduleId: "calendar", + path: "/events", + permission: readPermission, + }), + ], + }); + + const response = await app.request("/api/v1/modules/calendar/events", { + headers: { origin: untrustedOrigin }, + }); + expect(response.headers.get("access-control-allow-origin")).not.toBe("*"); + expect(response.headers.get("access-control-expose-headers")).toBeNull(); + }); + + it("forces Cache-Control: no-store and Vary: Origin, Cookie on a successful module response", async () => { + const handler = vi.fn( + async () => new Response("body", { headers: { "content-type": "text/plain" }, status: 200 }), + ); + const { app } = buildModuleApp({ + registry: permissionRegistry([{ id: readPermission, baseline: "public" }]), + routes: [ + moduleRoute({ + handler, + method: "GET", + moduleId: "calendar", + path: "/events", + permission: readPermission, + }), + ], + }); + + const response = await app.request("/api/v1/modules/calendar/events"); + expect(response.headers.get("cache-control")).toBe("no-store"); + const vary = response.headers.get("vary") ?? ""; + expect(vary).toContain("Origin"); + expect(vary).toContain("Cookie"); + }); + + it("preserves an additional legitimate Vary value from the handler alongside the forced ones", async () => { + const handler = vi.fn( + async () => new Response("body", { headers: { Vary: "Accept-Language" }, status: 200 }), + ); + const { app } = buildModuleApp({ + registry: permissionRegistry([{ id: readPermission, baseline: "public" }]), + routes: [ + moduleRoute({ + handler, + method: "GET", + moduleId: "calendar", + path: "/events", + permission: readPermission, + }), + ], + }); + + const response = await app.request("/api/v1/modules/calendar/events"); + const vary = response.headers.get("vary") ?? ""; + expect(vary).toContain("Accept-Language"); + expect(vary).toContain("Origin"); + expect(vary).toContain("Cookie"); + }); +}); + +describe("module route HEAD semantics", () => { + it("traverses the GET route's permission middleware and returns no body when authorized", async () => { + const handler = vi.fn( + async () => new Response("body-text", { status: 200 }), + ); + const { app } = buildModuleApp({ + registry: permissionRegistry([{ id: readPermission, baseline: "public" }]), + routes: [ + moduleRoute({ + handler, + method: "GET", + moduleId: "calendar", + path: "/events", + permission: readPermission, + }), + ], + }); + + const response = await app.request("/api/v1/modules/calendar/events", { method: "HEAD" }); + expect(response.status).toBe(200); + expect(await response.text()).toBe(""); + expect(handler).toHaveBeenCalledTimes(1); + }); + + it("denies a HEAD request exactly like the GET route would, without invoking the handler", async () => { + const handler = vi.fn( + async () => new Response("body-text", { status: 200 }), + ); + const evaluator: AuthorizationEvaluator = { + authorize: vi.fn().mockResolvedValue({ allowed: false }), + }; + const { app } = buildModuleApp({ + evaluator, + registry: permissionRegistry([{ id: readPermission, baseline: "restricted" }]), + routes: [ + moduleRoute({ + handler, + method: "GET", + moduleId: "calendar", + path: "/events", + permission: readPermission, + }), + ], + }); + + const response = await app.request("/api/v1/modules/calendar/events", { method: "HEAD" }); + expect(response.status).toBe(403); + expect(await response.text()).toBe(""); + expect(handler).not.toHaveBeenCalled(); + }); +}); + +describe("module routes and the rest of the API", () => { + it("does not invoke a handler for an unmatched module path", async () => { + const handler = vi.fn(async () => Response.json({ ok: true })); + const { app } = buildModuleApp({ + registry: permissionRegistry([{ id: readPermission, baseline: "public" }]), + routes: [ + moduleRoute({ + handler, + method: "GET", + moduleId: "calendar", + path: "/events", + permission: readPermission, + }), + ], + }); + + const response = await app.request("/api/v1/modules/calendar/does-not-exist"); + expect(response.status).toBe(404); + expect(await response.json()).toEqual({ code: "NOT_FOUND" }); + expect(handler).not.toHaveBeenCalled(); + }); + + it("never invokes a handler merely by constructing the app", () => { + const handler = vi.fn(async () => Response.json({ ok: true })); + buildModuleApp({ + registry: permissionRegistry([{ id: readPermission, baseline: "public" }]), + routes: [ + moduleRoute({ + handler, + method: "GET", + moduleId: "calendar", + path: "/events", + permission: readPermission, + }), + ], + }); + expect(handler).not.toHaveBeenCalled(); + }); + + it("preserves existing API behavior with zero module routes", async () => { + const { app } = buildModuleApp({ registry: permissionRegistry([]), routes: [] }); + const response = await app.request("/api/v1/principal"); + expect(response.status).toBe(200); + }); +}); diff --git a/docs/development/api.md b/docs/development/api.md index 36b4237..9010016 100644 --- a/docs/development/api.md +++ b/docs/development/api.md @@ -57,6 +57,7 @@ No real values are shown here — configure per environment and never commit the | `POST\|GET /api/auth/*` | Better Auth (mounted directly) | Better Auth's own request/response shapes pass through unmodified — see [Better Auth's email/password docs](https://www.better-auth.com/docs/authentication/email-password) for the exact endpoints (`/sign-up/email`, `/sign-in/email`, `/sign-out`, `/get-session`, ...). | | `GET /api/v1/principal` | Ekklesia | Returns the minimal `EkklesiaPrincipal` contract as JSON. `Cache-Control: no-store`. | | `GET\|POST /api/v1/grants*` | Ekklesia | Explicit authorization grant management, gated by `core.authorization.grants.manage`. See [Authorization](authorization.md#explicit-grant-http-api). | +| `/api/v1/modules//*` | Installed modules | Every route an installed module registered during its `register` hook, gated by that route's own declared permission. See [Module HTTP routes](#module-http-routes). | Better Auth's handler is unreachable outside `/api/auth/*`; every other unmatched path returns a deterministic `404 {"code":"NOT_FOUND"}`. @@ -206,17 +207,74 @@ trusted-proxy IP policy is configured, Better Auth may fall back to a single sha rather than a genuine per-client bucket. This is not yet the final, distributed, production-grade abuse-control design. -## Module permission discovery at startup +## Module discovery, permission composition, and module runtime startup -Before any database pool, Better Auth instance, or HTTP server exists, `startApi()` discovers -every installed module's manifest and composes the one immutable platform permission registry -(Core's own declarations plus every module's) that the rest of startup — and every subsequent -request — enforces against. A discovery or composition failure (an invalid manifest, a module -claiming the reserved `core.*` permission namespace, a duplicate permission declaration, ...) fails -startup outright, before there is anything to close; startup never falls back to a Core-only -registry. See [Authorization](authorization.md#api-startup-discovery-composition-and-enforcement) -for the full flow, and [Module development](module-development/README.md) for the manifest shape -that produces these declarations. +`startApi()` composes production in one fixed order: + +1. Discover every installed module's manifest exactly once and compose the one immutable platform + permission registry (Core's own declarations plus every module's) — `prepareStartupModuleInventory`, + `apps/api/src/platform-permissions.ts`. +2. Construct the one shared PostgreSQL pool. +3. Construct the Core identity runtime, authorization store, and evaluator. +4. Start the module runtime from that exact same discovered inventory + (`startDiscoveredModuleRuntime`) — modules are never rediscovered for this step. +5. Create the Hono application, mounting the started module runtime's published routes. +6. Bind the HTTP server. + +Discovery and permission composition happen before any pool, Better Auth instance, module +entrypoint import, or HTTP server exists, so a discovery or composition failure (an invalid +manifest, a module claiming the reserved `core.*` permission namespace, a duplicate permission +declaration, ...) fails startup outright, before there is anything to close; startup never falls +back to a Core-only registry. Core infrastructure exists before any executable module lifecycle +code runs, and every module route is published only after the *entire* module runtime has +registered and started successfully, so no request can ever reach a module before every module has +started. See [Authorization](authorization.md#api-startup-discovery-composition-and-enforcement) +for the permission-composition detail, and [Module development](module-development/README.md) for +the manifest shape that produces these declarations. + +Unlike discovery/composition/Doctor/`modules:validate` (which only ever read manifests), a +successful production startup does import each module's compiled server entrypoint and run its +`register`/`start` hooks — this is the one place in the platform that deliberately executes module +code, and only after every other startup step already listed has succeeded. + +## Module HTTP routes + +An installed module's routes (declared via `http.route(...)` in its `register` hook — see +[Module development](module-development/README.md#http-routes) for the module-authoring contract) +are mounted under `/api/v1/modules/`, in the module runtime's published order, by +`apps/api/src/module-http-routes.ts`. Mounting never invokes a handler. + +- **Enforcement.** Every route runs through the identical `requirePermission` middleware Core's own + routes use, with the route's own declared permission id as the only permission ever consulted — + its trusted baseline always comes from the composed platform registry, never from the route or + the request. A `public`, `authenticated`, and `restricted` module permission are enforced through + the exact same path; a public-baseline route still resolves and receives an authorized principal. +- **Trusted origin.** `POST`, `PUT`, `PATCH`, and `DELETE` module routes additionally require an + exact trusted `Origin` (see [Origin, CORS, and cookies](#origin-cors-and-cookies)) before + authorization ever runs, exactly like the existing grants-mutation routes. `GET` does not require + one. `OPTIONS` remains entirely platform-owned: a module cannot register it, and a CORS preflight + never reaches authorization or a handler. `GET` inherits Hono's own implicit `HEAD` handling + (same permission check, response body stripped); a module cannot separately author `HEAD`. +- **Handler input.** A handler receives the raw Fetch `Request` (so `Request.url`, headers, and + body all behave normally, and the full query string remains available) plus a frozen context of + exactly `moduleId`, the authorized `EkklesiaPrincipal`, and frozen, Hono-decoded path parameters + (an empty frozen object for a route with none) — never a Hono `Context`, a database connection, + the authorization store, storage, jobs, configuration, or secrets. +- **Handler failure.** A handler that throws, rejects, or returns anything other than a `Response` + becomes a fixed `500 {"code":"MODULE_HANDLER_FAILED"}` with `Cache-Control: no-store` — never the + caught value, a stack, the route's path or permission id, or the module id. A `Response` a + handler deliberately returns — including a 4xx/5xx one — passes through unchanged. +- **CORS and caching.** The platform remains the sole owner of CORS for module routes exactly like + every other `/api/v1/*` route (see [Origin, CORS, and cookies](#origin-cors-and-cookies), now + additionally advertising `PUT`/`PATCH`/`DELETE`); any CORS-control header a handler sets on its + own response is stripped before the response leaves the process. Every module response is forced + to `Cache-Control: no-store` and a `Vary` that always includes `Origin` and `Cookie` (a handler's + own legitimate `Vary` values are preserved alongside those) — deliberately conservative until a + dedicated cache-policy contract exists, since even a public-baseline handler receives an + authorized principal and may personalize its response. +- **Static snapshot.** Routes are part of the same static startup snapshot as the permission + registry: installing, removing, or editing a module's routes takes effect only on the next API + restart. Removing a module makes its former routes return the ordinary `404 {"code":"NOT_FOUND"}`. ## Database connection limits @@ -239,17 +297,25 @@ not two. `startApi()` (the composition function `apps/api/src/server.ts` exports, used by both the executable entry point and tests) returns `{app, server, shutdown}`. `shutdown()` memoizes one shared promise the first time it is called: every caller, concurrent or sequential, awaits the -exact same listener-drain-then-pool-close operation, and that operation runs exactly once no matter -how many times `shutdown()` is called. It stops the HTTP listener from accepting new connections and -lets in-flight requests finish, then closes the one shared PostgreSQL pool (identity and -authorization both read from it — see -[Authorization](authorization.md#shared-database-composition)). The executable entry point's -`SIGINT`/`SIGTERM` wiring is extracted into `installShutdownSignalHandlers(running, exit)` — the -same function production composition uses, but independently testable with a fake `running`/`exit` -so a test can trigger "a signal arrived" without sending a real process signal or invoking the real -`process.exit`. A startup failure after the database pool was already created (identity -construction failure, or the server failing to bind its listen address) always closes that pool -before the failure propagates. +exact same operation, and that operation runs exactly once no matter how many times `shutdown()` +is called. In order: it stops the HTTP listener from accepting new connections and lets in-flight +requests finish; then stops the module runtime (so no module resource stops while a route handler +might still be in flight); then closes the one shared PostgreSQL pool (identity and authorization +both read from it — see [Authorization](authorization.md#shared-database-composition)). Unlike a +startup-failure cleanup path (below), a module-stop failure here is not swallowed — it is a +genuine result a caller should observe — but pool cleanup always still runs first, so a stop +failure never leaves the pool open. The executable entry point's `SIGINT`/`SIGTERM` wiring is +extracted into `installShutdownSignalHandlers(running, exit)` — the same function production +composition uses, but independently testable with a fake `running`/`exit` so a test can trigger "a +signal arrived" without sending a real process signal or invoking the real `process.exit`. + +A startup failure after the database pool was already created always closes that pool before the +failure propagates: identity/authorization construction failing (no module runtime was started +yet); the server failing to bind its listen address; or an app-construction failure, both of which +also stop the module runtime first, since it may have already started successfully. A module +load/register/start failure uses Core's own existing rollback (already-started modules stop, in +reverse order, before the failure propagates) and then closes the pool — there is no separate +runtime handle to stop in that case, since `startDiscoveredModuleRuntime` never returned one. ## A real signup → sign-in → principal → sign-out transcript diff --git a/docs/development/authorization.md b/docs/development/authorization.md index 7bc9243..7f1954c 100644 --- a/docs/development/authorization.md +++ b/docs/development/authorization.md @@ -16,14 +16,15 @@ for the durable architectural decisions this page describes in more implementati An operator can bootstrap exactly one platform owner for a deployment; a platform owner (or anyone they delegate to) can create, list, and revoke *explicit* authorization grants over HTTP for any -registered permission, Core-owned or module-owned; and every enforced route fails closed on -infrastructure trouble instead of silently allowing or denying. It is **not** possible to manage -roles, groups, teams, or tenants; there is no owner-management HTTP endpoint (revoking or -re-granting platform-owner status is not exposed at all); there are no service accounts or API -tokens, so only cookie-authenticated browser callers can reach the mutation routes; a module cannot -yet mount its own HTTP routes; and relationship-derived grants (for example, automatic access from -a roster assignment) cannot yet be created by anything in this repository — the generic grants API -structurally cannot create or revoke them either. +registered permission, Core-owned or module-owned; a module can register its own HTTP routes and +have them mounted, enforced, and reachable in production (see +[API](api.md#module-http-routes)); and every enforced route fails closed on infrastructure trouble +instead of silently allowing or denying. It is **not** possible to manage roles, groups, teams, or +tenants; there is no owner-management HTTP endpoint (revoking or re-granting platform-owner status +is not exposed at all); there are no service accounts or API tokens, so only cookie-authenticated +browser callers can reach mutation routes, Core-owned or module-owned; and relationship-derived +grants (for example, automatic access from a roster assignment) cannot yet be created by anything +in this repository — the generic grants API structurally cannot create or revoke them either. ## Core authorization schema and store @@ -105,27 +106,37 @@ integration layer depends on authorization). ## API startup: discovery, composition, and enforcement -`apps/api` builds its production permission registry from the complete installed-module inventory -every time it starts, in a fixed order, before anything else: +`apps/api` builds its production permission registry, and starts the module runtime, from the +exact same installed-module inventory every time it starts, in a fixed order, before anything +else: 1. Resolve module-discovery configuration (the repository's `modules/` directory, and the platform/Module SDK versions from their own `package.json` files). -2. Discover installed modules (`discoverModules`). -3. Compose the platform permission registry (`composePlatformPermissionRegistry`). +2. Discover installed modules exactly once (`discoverModules`). +3. Compose the platform permission registry from that exact discovered list + (`composePlatformPermissionRegistry`). 4. Construct the shared PostgreSQL pool. -5. Construct Identity and Authorization. -6. Create the Hono application, injecting the composed registry. -7. Bind the HTTP server. +5. Construct Identity, the authorization store, and the evaluator. +6. Start the module runtime from that same discovered list (`startDiscoveredModuleRuntime`) — + modules are never rediscovered for this step. +7. Create the Hono application, injecting the composed registry and mounting the started module + runtime's published routes (see [API](api.md#module-http-routes)). +8. Bind the HTTP server. -`apps/api/src/platform-permissions.ts`'s `composeStartupPermissionRegistry` performs steps 1–3. -Discovery and composition happen strictly before any pool, Better Auth instance, or HTTP server -exists, so a failure there — an invalid manifest, a broken dependency graph, a module claiming the -reserved `core.*` namespace, or a duplicate permission declaration — leaves nothing to close: it -propagates directly out of `startApi()`, and startup never falls back to `CORE_PERMISSION_REGISTRY` -or any Core-only registry. The repository root is resolved from this compiled/source file's own -location (two directories up from `apps/api/dist/*.js` or `apps/api/src/*.ts`, which sit at -identical depth), never `process.cwd()` — a package-scoped test run has `apps/api` as its working -directory, which would otherwise point discovery at the wrong `modules/` directory. +`apps/api/src/platform-permissions.ts`'s `prepareStartupModuleInventory` performs steps 1–3, +returning both the discovered module list and the composed registry so step 6 can start the module +runtime from that identical list rather than rediscovering modules a second time. +(`composeStartupPermissionRegistry` remains available as a registry-only convenience wrapper +around it, for callers — Doctor, `modules:validate`, existing tests — that only need the +registry.) Discovery and composition happen strictly before any pool, Better Auth instance, module +entrypoint import, or HTTP server exists, so a failure there — an invalid manifest, a broken +dependency graph, a module claiming the reserved `core.*` namespace, or a duplicate permission +declaration — leaves nothing to close: it propagates directly out of `startApi()`, and startup +never falls back to `CORE_PERMISSION_REGISTRY` or any Core-only registry. The repository root is +resolved from this compiled/source file's own location (two directories up from +`apps/api/dist/*.js` or `apps/api/src/*.ts`, which sit at identical depth), never `process.cwd()` +— a package-scoped test run has `apps/api` as its working directory, which would otherwise point +discovery at the wrong `modules/` directory. The resulting registry is a **static, immutable snapshot** taken once at startup. There is no hot installation, removal, or dynamic mutation of installed modules or their permissions while the @@ -148,10 +159,11 @@ consults the registry for a grant that already exists — revocation is keyed to and source kind, not to whether its permission is currently registered — so a manager can still revoke a historical grant for a now-unregistered permission. -Module HTTP route registration (a module owning its own mounted endpoints) and relationship-derived -grant creation both remain deferred, unrelated slices — this work only makes a module's *declared* -permission usable through Core's existing, generic explicit-grant HTTP API and enforcement -middleware. +A module's own mounted HTTP routes (`/api/v1/modules//*`) are enforced through this +identical registry and `requirePermission` middleware — see [API](api.md#module-http-routes) for +the route-mounting contract. Relationship-derived grant creation remains a deferred, unrelated +slice — this work only makes a module's *declared* permission usable through Core's existing, +generic explicit-grant HTTP API, enforcement middleware, and now its own mounted routes. ## Doctor and modules:validate @@ -446,12 +458,34 @@ there are or whether the failure happened during startup. `403` and `store.createGrant` never called, exact server-derived-argument assertions, sourceKind-filtered revoke returning 404 for a non-explicit grant, origin enforcement, and store-error mapping. +- `apps/api/tests/module-http-routes.test.ts` — the production route-mounting contract: exact + prefix/path joining (including the non-trailing-slash root case), every supported method, + independent same-path routes across two modules, deterministic static-versus-parameterized + resolution by published order, request/handler adaptation (raw `Request`, frozen context, decoded + and frozen path parameters, empty frozen params for static/root routes), the registry's exact + baseline for `public`/`authenticated`/`restricted` module permissions, denial/infrastructure- + failure paths never invoking the handler, `ModuleRouteConfigurationError` for an inconsistent + route/registry snapshot, trusted-origin enforcement for state-changing methods, CORS (advertised + methods, no reflection for an untrusted origin, no authorization or handler invocation on + preflight), fixed sanitized `500` for a throwing/rejecting/non-`Response`-returning handler with + no raw leakage, a valid module-owned 4xx/5xx response passing through unchanged, forced + `Cache-Control`/`Vary` with a handler's own `Vary` preserved, stripped module-supplied CORS + headers, `HEAD` traversing the same permission middleware as `GET` with no body, and unmatched + paths/zero-route/no-handler-during-construction behavior. - `apps/api/tests/lifecycle.test.ts` — shared-pool-closes-exactly-once coverage described above, plus module-permission composition at startup: discovery/composition run before any pool, Better Auth instance, or HTTP server is constructed; a discovery failure, a reserved-namespace failure, and a generic duplicate-declaration failure each create zero pools/Better Auth instances/servers and remain distinguishable from one another; zero installed modules still - succeeds; and import safety for `platform-permissions.ts`. + succeeds; and import safety for `platform-permissions.ts`. A dedicated block covers module + *runtime* startup and shutdown against real, working (non-throwing) fixture modules: permission + composition and module runtime startup share the exact same discovered inventory, route mounting + order matches discovery order, a load/register/start failure each close the pool exactly once and + bind no server (preserving Core's own rollback for a start failure), an app-construction failure + and a bind failure both stop the module runtime before closing the pool, normal shutdown stops the + server/module-runtime/pool in that exact order, a module stop failure still runs pool cleanup + before the shutdown rejection surfaces, and a direct `discoveredModulesOverride` drives both + composition and runtime startup identically. - `core/tests/authorization-evaluator.test.ts` — includes a malformed permission id denying under every baseline (`public`, `authenticated`, `restricted`), never just `restricted`, and never consulting the grant or platform-owner ports for `public`/`authenticated`. @@ -466,16 +500,23 @@ there are or whether the failure happened during startup. invisible to LIST and unreachable by REVOKE, and an infrastructure-failure scenario (database dropped out from under a live session) asserting a `503` rather than a false `403` or `200`. - `apps/api/tests/api-http.integration.ts`, "installed module permission end-to-end" (live - PostgreSQL) — a real temporary module fixture (structurally valid manifest, a server entrypoint - that throws if ever imported) proves the complete chain against a real API process and real - Doctor run: Doctor recognizes the installed module and passes permission composition without - executing its entrypoint; API startup discovers the fixture and makes its declared permission - enforceable; a bootstrapped platform owner creates a real explicit grant for the module's - permission with an atomic audit row recorded under its exact declared id; a well-formed - permission absent from the composed registry is rejected with no grant or audit write; and, - after a clean shutdown and restart with the module absent, the same permission can no longer be - newly granted while the earlier, still-unrevoked grant remains listed — with ordinary identity - HTTP behavior (sign-up, sign-in, principal resolution) unaffected throughout. + PostgreSQL) — a real temporary module fixture (structurally valid manifest; a real, working + server entrypoint recording register/start/stop markers instead of throwing, and declaring a + restricted `GET /events/:eventId` route plus a restricted, state-changing `POST /events` route + against its own manifest permission) proves the complete chain against a real API process and + real Doctor run: Doctor recognizes the installed module and passes permission composition + without executing its entrypoint (an empty marker set); a successful `startApi()` discovers the + same fixture, imports/registers/starts its runtime exactly once, and makes its declared + permission enforceable; a bootstrapped platform owner creates a real explicit grant for the + module's permission with an atomic audit row recorded under its exact declared id; the module's + own mounted route denies an anonymous caller and then serves the newly granted owner with the + decoded path parameter and authenticated principal; the module's state-changing route requires a + trusted `Origin` exactly like the platform's own mutation routes; a well-formed permission absent + from the composed registry is rejected with no grant or audit write; and, after a clean shutdown + (module stop invoked exactly once) and restart with the module absent, its former route returns + the ordinary `404`, the same permission can no longer be newly granted, and the earlier, + still-unrevoked grant remains listed — with ordinary identity HTTP behavior (sign-up, sign-in, + principal resolution) unaffected throughout. Run the focused suites: @@ -487,10 +528,11 @@ pnpm test:migrations:integration # requires Docker ## Next phase -Deliberately deferred, unrelated slices — see +Module HTTP route registration and production mounting (see [ADR 0014](../architecture/decisions/0014-module-permission-declarations-and-platform-registry.md#deferred) -for the complete list: module HTTP route-registration contracts (a module mounting its own -endpoints), a module runtime-context authorization service, relationship-derived grant creation +for the deferral this closes) is now implemented — see [API](api.md#module-http-routes) and +[Module development](module-development/README.md#http-routes). Deliberately deferred, unrelated +slices remain: a module runtime-context authorization service, relationship-derived grant creation itself (a roster or similar module operation, not the generic grants API), reinstall/reactivation policy for a permission whose owning module was previously removed, roles/groups/teams/tenants, an owner-management HTTP endpoint, and service accounts/API tokens for non-browser automation. diff --git a/docs/module-development/README.md b/docs/module-development/README.md index 429158c..d465eeb 100644 --- a/docs/module-development/README.md +++ b/docs/module-development/README.md @@ -240,11 +240,20 @@ connection, authorization store, grant/platform-owner state, storage, jobs, conf a framework-specific request context, or a raw Better Auth session. The full query string remains available through `Request.url`; there is no separate query or body abstraction. -This pass only defines, validates, and collects routes into `StartedModuleRuntime`'s immutable route -snapshot — Core never invokes a handler here. Mounting routes into the production `apps/api` Hono -application, enforcing each route's permission through `requirePermission`, applying -`requireTrustedOrigin`, mapping handler errors to sanitized responses, and wiring this into -`startApi`'s production lifecycle are later work. +Core defines, validates, and collects routes into `StartedModuleRuntime`'s immutable route +snapshot — it never invokes a handler itself. `apps/api` mounts that exact snapshot into production +at startup, under the `/api/v1/modules/` prefix described above: every route is enforced +through the identical `requirePermission` middleware Core's own routes use, a state-changing method +(`POST`/`PUT`/`PATCH`/`DELETE`) additionally requires a trusted `Origin` before authorization ever +runs, and a handler that throws, rejects, or returns something other than a `Response` becomes a +fixed, sanitized `500 {"code":"MODULE_HANDLER_FAILED"}` — never the caught value, a stack, the +route's path, its permission id, or the module id. The platform remains the sole owner of CORS and +caching for every module route: a handler-supplied CORS-control header is stripped, and every +module response is forced to `Cache-Control: no-store` with `Vary: Origin, Cookie` (in addition to +any of the handler's own legitimate `Vary` values) until a dedicated cache-policy contract exists — +deliberately conservative, since even a public-baseline handler still receives an authorized +principal and may personalize its response. See [API](../development/api.md#module-http-routes) for +the full production mounting, enforcement, and error-response contract. ## Drizzle schema definition -- 2.54.0 From bf76f4e00638dd4df295f127c8d1129b858f9e71 Mon Sep 17 00:00:00 2001 From: ekklesia Date: Sat, 25 Jul 2026 10:14:13 -0500 Subject: [PATCH 3/5] fix: collapse invalid Vary * combination and case-duplicate tokens MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Found during the Phase 5B pass-3 architectural audit: sanitizeModuleResponse's Vary merge used a plain Set, so an existing "Vary: *" from a module handler became "*, Origin, Cookie" (RFC 9110 §12.5.5 forbids combining "*" with any other value), and a differently-cased existing token (e.g. "origin") produced a redundant duplicate alongside the forced "Origin"/"Cookie". mergedVaryHeader now de-duplicates case-insensitively, always emits canonical Origin/Cookie casing, and collapses to a bare "*" whenever one is present, per focused unit tests. The full app's own pre-existing cors() middleware (apps/api/src/app.ts, unchanged here, present since Phase 4B on every /api/v1/* route including /api/v1/principal) still unconditionally appends its own "Vary: Origin" outside this module's control — noted in the new tests, not addressed by this narrow fix. Co-Authored-By: Claude Sonnet 5 --- apps/api/src/module-http-routes.ts | 42 ++++++++--- apps/api/tests/module-http-routes.test.ts | 91 ++++++++++++++++++++++- 2 files changed, 122 insertions(+), 11 deletions(-) diff --git a/apps/api/src/module-http-routes.ts b/apps/api/src/module-http-routes.ts index a73a165..2f5fc1f 100644 --- a/apps/api/src/module-http-routes.ts +++ b/apps/api/src/module-http-routes.ts @@ -52,6 +52,35 @@ function sanitizedModuleHandlerFailure(c: Context): Response { return c.json({ code: "MODULE_HANDLER_FAILED" }, 500); } +/** + * `Vary` lists header *names*, which are themselves case-insensitive, so merging de-duplicates + * case-insensitively — a module's own `origin` must collapse with the forced `Origin` rather than + * appear as a separate, redundant entry — and always emits the canonical `Origin`/`Cookie` casing + * regardless of how (or whether) a module already listed them. A literal `*` (RFC 9110 §12.5.5) + * must never be combined with any other value; since `*` already means "may vary on anything," a + * strictly broader signal than any explicit list, an existing `*` collapses the result to `*` alone + * rather than producing the invalid `*, Origin, Cookie`. + */ +export function mergedVaryHeader(existingVary: string | null): string { + const canonicalByLowercase = new Map(); + for (const token of (existingVary ?? "").split(",")) { + const trimmed = token.trim(); + if (trimmed.length === 0) { + continue; + } + const key = trimmed.toLowerCase(); + if (!canonicalByLowercase.has(key)) { + canonicalByLowercase.set(key, trimmed); + } + } + if (canonicalByLowercase.has("*")) { + return "*"; + } + canonicalByLowercase.set("origin", "Origin"); + canonicalByLowercase.set("cookie", "Cookie"); + return [...canonicalByLowercase.values()].join(", "); +} + /** * The platform owns CORS and caching for every module route: strips any CORS-control header a * handler set (defense in depth — the outer `cors()` middleware already computes the authoritative @@ -59,7 +88,8 @@ function sanitizedModuleHandlerFailure(c: Context): Response { * includes `Origin` and `Cookie`, since even a public-baseline handler receives an authorized * principal and may personalize its response. A separate cache-policy contract is future work; * this is the conservative default until then. A module's own legitimate `Vary` values (if any) - * are preserved alongside the forced ones. + * are preserved alongside the forced ones. Never reads or buffers `response.body` — it is passed + * through to the new `Response` unread, so a streaming body is neither consumed nor blocked here. */ function sanitizeModuleResponse(response: Response): Response { const headers = new Headers(response.headers); @@ -67,15 +97,7 @@ function sanitizeModuleResponse(response: Response): Response { headers.delete(headerName); } headers.set("Cache-Control", "no-store"); - const varyValues = new Set( - (headers.get("Vary") ?? "") - .split(",") - .map((value) => value.trim()) - .filter((value) => value.length > 0), - ); - varyValues.add("Origin"); - varyValues.add("Cookie"); - headers.set("Vary", [...varyValues].join(", ")); + headers.set("Vary", mergedVaryHeader(headers.get("Vary"))); return new Response(response.body, { headers, status: response.status, diff --git a/apps/api/tests/module-http-routes.test.ts b/apps/api/tests/module-http-routes.test.ts index ca38c82..ffc7d58 100644 --- a/apps/api/tests/module-http-routes.test.ts +++ b/apps/api/tests/module-http-routes.test.ts @@ -16,7 +16,7 @@ import { import type { EkklesiaIdentityRuntime } from "@ekklesia/core/identity"; import { describe, expect, it, vi } from "vitest"; import { createApiApp } from "../src/app.js"; -import { ModuleRouteConfigurationError } from "../src/module-http-routes.js"; +import { mergedVaryHeader, ModuleRouteConfigurationError } from "../src/module-http-routes.js"; const trustedOrigin = "https://dashboard.example.test"; const untrustedOrigin = "https://evil.example.test"; @@ -835,6 +835,95 @@ describe("module route response header policy", () => { expect(vary).toContain("Origin"); expect(vary).toContain("Cookie"); }); + + // The following exercise `mergedVaryHeader` directly rather than through the full app: the + // outer, pre-existing `cors()` middleware (`apps/api/src/app.ts`, unchanged by module routes) + // unconditionally appends its own "Vary: Origin" to every response with a dynamic-function + // origin, on top of whatever this module already set — a real Hono behavior, present on every + // route including `/api/v1/principal` since Phase 4B, not something module routing introduces + // or can suppress. Testing the merge function directly proves this module's own logic is + // correct in isolation, without coupling the assertion to that separate middleware's behavior. + describe("mergedVaryHeader", () => { + it("collapses an existing * to a bare * instead of the invalid *, Origin, Cookie", () => { + // RFC 9110 §12.5.5: a Vary field-value of "*" must never be combined with any other value. + expect(mergedVaryHeader("*")).toBe("*"); + expect(mergedVaryHeader("*, Accept-Language")).toBe("*"); + expect(mergedVaryHeader(" * ")).toBe("*"); + }); + + it("merges case-insensitively so a differently-cased Origin/Cookie never appears as a redundant duplicate", () => { + expect(mergedVaryHeader("origin, Accept-Language, COOKIE, origin")).toBe( + "Origin, Accept-Language, Cookie", + ); + }); + + it("forces canonical Origin/Cookie casing even when absent, and tolerates irregular whitespace", () => { + expect(mergedVaryHeader(null)).toBe("Origin, Cookie"); + expect(mergedVaryHeader("")).toBe("Origin, Cookie"); + expect(mergedVaryHeader(" Accept-Language , ORIGIN ")).toBe( + "Accept-Language, Origin, Cookie", + ); + }); + + it("de-duplicates a literal repeated token with identical casing", () => { + expect(mergedVaryHeader("Accept-Language, Accept-Language")).toBe( + "Accept-Language, Origin, Cookie", + ); + }); + }); + + it("still forces Origin and Cookie into Vary end-to-end even when a handler supplies its own casing", async () => { + const handler = vi.fn( + async () => new Response("body", { headers: { Vary: "origin" }, status: 200 }), + ); + const { app } = buildModuleApp({ + registry: permissionRegistry([{ id: readPermission, baseline: "public" }]), + routes: [ + moduleRoute({ + handler, + method: "GET", + moduleId: "calendar", + path: "/events", + permission: readPermission, + }), + ], + }); + + const response = await app.request("/api/v1/modules/calendar/events"); + const vary = response.headers.get("vary") ?? ""; + expect(vary).toContain("Origin"); + expect(vary).toContain("Cookie"); + }); + + it("does not read or buffer a streaming response body while sanitizing headers", async () => { + const chunks = ["first-chunk", "second-chunk"]; + const handler = vi.fn(async () => { + const stream = new ReadableStream({ + start(controller) { + for (const chunk of chunks) { + controller.enqueue(new TextEncoder().encode(chunk)); + } + controller.close(); + }, + }); + return new Response(stream, { status: 200 }); + }); + const { app } = buildModuleApp({ + registry: permissionRegistry([{ id: readPermission, baseline: "public" }]), + routes: [ + moduleRoute({ + handler, + method: "GET", + moduleId: "calendar", + path: "/events", + permission: readPermission, + }), + ], + }); + + const response = await app.request("/api/v1/modules/calendar/events"); + expect(await response.text()).toBe(chunks.join("")); + }); }); describe("module route HEAD semantics", () => { -- 2.54.0 From 1285c90618ef93768fcc3143d009fb98367782e4 Mon Sep 17 00:00:00 2001 From: ekklesia Date: Sat, 25 Jul 2026 10:22:35 -0500 Subject: [PATCH 4/5] docs: record module HTTP architecture Phase 5B, pass 3: adds ADR 0015 recording the durable module HTTP routing and production runtime lifecycle architecture pass 1 and pass 2 implemented, extending ADR 0006 and ADR 0014 without superseding either. Adds narrow cross-reference notes to both, and to the ADR index, pointing at ADR 0015 for the work they deferred, while preserving their historical text and accepted status. Corrects stale forward-looking "pass 2"/future-tense language left over from pass 1 in core/src/modules/runtime.ts and packages/contracts/src/module-http.ts (comment-only; production behavior unchanged), and a shutdown-lifecycle description in docs/development/testing.md that predated the module-runtime-stop step pass 2 added between draining the HTTP server and closing the pool. Full audit against origin/main...HEAD found the architecture, contract, startup ordering, routing, authorization/origin enforcement, and PostgreSQL integration coverage from passes 1-2 sound; the one concrete defect found (Vary-header merge producing an invalid "*, Origin, Cookie" combination and case-sensitive duplicate tokens) was already fixed with focused regression tests in a separate commit (bf76f4e), not amending either checkpoint commit. Co-Authored-By: Claude Sonnet 5 --- core/src/modules/runtime.ts | 6 +- ...apabilities-and-compiled-module-runtime.md | 9 + ...sion-declarations-and-platform-registry.md | 4 + ...dule-http-routing-and-runtime-lifecycle.md | 286 ++++++++++++++++++ docs/architecture/decisions/README.md | 1 + docs/development/testing.md | 19 +- packages/contracts/src/module-http.ts | 10 +- 7 files changed, 321 insertions(+), 14 deletions(-) create mode 100644 docs/architecture/decisions/0015-static-module-http-routing-and-runtime-lifecycle.md diff --git a/core/src/modules/runtime.ts b/core/src/modules/runtime.ts index 42ab1de..88157d9 100644 --- a/core/src/modules/runtime.ts +++ b/core/src/modules/runtime.ts @@ -518,9 +518,9 @@ async function startModules( * Starts the module runtime from a module list the caller already discovered — performs no * module-directory scan or manifest read of its own, and preserves the given list's order exactly. * `startModuleRuntime` below is the convenient discovery-plus-start wrapper most callers want; - * `apps/api`'s production startup (pass 2) will call this form directly so permission composition - * and module HTTP/runtime startup share Phase 5A's exact same one-time discovery rather than - * discovering modules twice. + * `apps/api`'s production startup calls this form directly (`prepareStartupModuleInventory`, + * `apps/api/src/platform-permissions.ts`) so permission composition and module HTTP/runtime + * startup share the exact same one-time discovery rather than discovering modules twice. */ export async function startDiscoveredModuleRuntime( discoveredModules: readonly DiscoveredModule[], diff --git a/docs/architecture/decisions/0006-versioned-capabilities-and-compiled-module-runtime.md b/docs/architecture/decisions/0006-versioned-capabilities-and-compiled-module-runtime.md index 0c3e073..d176af2 100644 --- a/docs/architecture/decisions/0006-versioned-capabilities-and-compiled-module-runtime.md +++ b/docs/architecture/decisions/0006-versioned-capabilities-and-compiled-module-runtime.md @@ -28,6 +28,15 @@ registry, which conflicts with install-by-cloning discovery. > ordering — remains accepted and unchanged. This section is left as originally accepted, as the > historical record of the manifest's prior shape. +> **Cross-reference — see [ADR 0015](0015-static-module-http-routing-and-runtime-lifecycle.md).** +> "HTTP routing... remain[s] outside this slice" (Consequences, below) and "The injected contexts +> contain only the module ID and a capability registrar or resolver" (this section, below) described +> this ADR's own scope accurately at the time. ADR 0015 implements module HTTP routing: it extends +> `ModuleRegistrationContext` (the `register`-only context) with an `http` route registrar, while +> `ModuleRuntimeContext` (the `start`/`stop` context this sentence also describes) remains exactly as +> limited to module ID and capability access as originally decided here. Every other statement in +> this ADR remains accepted and unchanged. + `ekklesia.module.json` remains the one canonical static manifest and moves to `manifestVersion: 2`. Provided capabilities are declarations containing an identifier and an exact semantic version. Required and optional capability dependencies map identifiers to semantic-version diff --git a/docs/architecture/decisions/0014-module-permission-declarations-and-platform-registry.md b/docs/architecture/decisions/0014-module-permission-declarations-and-platform-registry.md index 100a1ee..40b810c 100644 --- a/docs/architecture/decisions/0014-module-permission-declarations-and-platform-registry.md +++ b/docs/architecture/decisions/0014-module-permission-declarations-and-platform-registry.md @@ -223,6 +223,10 @@ current deployment model but not the final word on it. ## Deferred +> **Cross-reference — see [ADR 0015](0015-static-module-http-routing-and-runtime-lifecycle.md).** +> Module HTTP route-registration contracts and their production mounting, listed immediately below +> as deferred, are now implemented. Every other item in this list remains deferred and unimplemented. + Module HTTP route-registration contracts (a module mounting its own endpoints); a `ModuleRuntimeContext` authorization service; relationship-derived grant creation (a roster or similar module operation using `sourceKind: "relationship"`, not the generic explicit-grant API); diff --git a/docs/architecture/decisions/0015-static-module-http-routing-and-runtime-lifecycle.md b/docs/architecture/decisions/0015-static-module-http-routing-and-runtime-lifecycle.md new file mode 100644 index 0000000..4d99ddd --- /dev/null +++ b/docs/architecture/decisions/0015-static-module-http-routing-and-runtime-lifecycle.md @@ -0,0 +1,286 @@ +# 0015: Static module HTTP routing and production runtime lifecycle + +- Status: Accepted +- Date: 2026-07-24 +- Extends: [0006](0006-versioned-capabilities-and-compiled-module-runtime.md) and + [0014](0014-module-permission-declarations-and-platform-registry.md) + +## Context + +ADR 0006 established the compiled module runtime — deterministic discovery, a staged capability +registry, dependency-ordered registration and lifecycle, and the injected context's deliberate +limit to module ID and capability access. ADR 0014 wired module-declared permissions into Core's +authorization system and explicitly deferred two things: a module mounting its own HTTP routes, +and `apps/api`'s permission-composition startup retaining the discovered module list for anything +beyond composition itself. This ADR records the architecture that closes both gaps: a +framework-neutral route contract a module declares during its existing `register` hook, Core's +atomic collection of those routes into the same immutable startup snapshot as capabilities, and +`apps/api`'s production mounting of that snapshot — enforced, origin-checked, and +response-sanitized — under one platform-owned path prefix. + +## Decision + +### One contracts-layer, Fetch-based route definition + +`packages/contracts/src/module-http.ts` is the sole route contract, consumed identically by Core's +module runtime and `apps/api`'s mounting adapter — there is no second, parallel definition in +either layer. It depends only on standard Fetch primitives (`Request`, `Response`), `EkklesiaPrincipal`, +`ModuleId`, and `PermissionId`; it never imports Hono, Node HTTP types, Better Auth, a database +client, or any other framework or Core implementation type. A route definition contains exactly +four fields — `method`, `path`, `permission`, `handler` — validated as an exact-shape object with +no unknown properties; a module cannot smuggle a baseline, a mount prefix, an absolute path, CORS +or origin policy, authentication behavior, grant/platform-owner state, middleware, audit metadata, +or another module's namespace through this contract, because the type has no field for any of +them. `permission` is accepted as a plain string at the authoring boundary (so a module writes a +literal identifier with no parse call) and validated and branded to `PermissionId` by the +contract's own parser, mirroring how `PermissionDeclarationInput`/`PermissionDeclaration` already +split authoring input from validated output (ADR 0014). + +Exactly `GET`, `POST`, `PUT`, `PATCH`, and `DELETE` are authorable, always uppercase; there is no +`OPTIONS` (the platform owns CORS preflight) and no wildcard or all-methods registration. A path is +always local to the module and canonical: it begins with `/`, a non-root path never ends with `/`, +and every segment is either a lowercase kebab-case static segment or a single `:parameterName` +segment — no empty or repeated slashes, no query string or fragment, no backslash or +percent-encoding, no dot segment, and no wildcard/catch-all segment, because none of those +characters can ever satisfy either segment grammar. Malformed input is rejected outright, never +normalized, so a module can never register a path Core silently reinterpreted. + +### Permission ownership is structural, not a runtime policy choice + +Every route — including one meant to be publicly reachable — names exactly one permission the +owning module's own manifest declares (ADR 0014's `manifestVersion: 3` declarations); there is no +way to construct an unprotected, no-permission route through this contract. A route can never +supply a baseline: the trusted `public`/`authenticated`/`restricted` baseline comes exclusively +from the composed platform permission registry at enforcement time, exactly like every other +permission check in the platform. Public access is expressed by referencing a permission whose own +manifest declaration has `baseline: "public"`, never by omitting a permission. Core's module +runtime checks route ownership against only the registering module's own manifest-declared +permissions — never the composed platform registry, never another module's manifest — so a module +can never gate a route with Core's or another module's permission; this is a structural +consequence of what the ownership check is allowed to consult, not a policy a caller could +misconfigure. + +### Atomic validation and publication alongside capabilities + +Core's staged registration (ADR 0006) now stages a module's HTTP routes exactly as it already +stages capability provision: a route is validated and, on failure, rejected immediately at the +`http.route(...)` call site during `register`, using structured `ModuleRuntimeIssue` codes +distinguishing invalid shape/handler, invalid method, invalid path, invalid permission identifier, +an undeclared (unowned) permission, and a duplicate route — never a caught exception's message, +handler source, or a machine path. A module's routes are merged into the shared, eventually +published list only after that module's `register` hook returns without throwing *and* every +capability it declared was actually provided — the same all-or-nothing gate capabilities already +enforce. `StartedModuleRuntime.routes` is populated, and becomes non-empty, only after every +discovered module has registered and started successfully; a failed registration or a failed +startup (with Core's existing reverse-order rollback) publishes no route snapshot at all, exactly +as no capability registry is published in that case either. + +Duplicate detection uses route *shape* — method plus path with every parameter segment collapsed +to one placeholder — not the parameter's chosen name, so `/events/:eventId` and `/events/:id` +collide under the same method exactly like the identical route they are. Detection is scoped to +one module's own routes; two different modules may register the identical local method and path +without conflict, because the production mount prefix (below) namespaces them apart regardless. + +### An immutable, ordered startup snapshot + +`StartedModuleRuntime.routes` is a single frozen array of frozen route records, published once at +startup and never mutated afterward — there is no dynamic route addition or removal while the +process runs. Its order is deterministic: discovered-module order, then registration order within +each module. Because a caller-owned route-definition object could otherwise be mutated after +`http.route(...)` returns, Core never retains the caller's own object — it copies each validated +field into a freshly frozen record, so mutating the original definition afterward cannot alter the +published snapshot. Core never invokes a handler during registration or startup; a handler is only +ever a stored function reference until a route is actually dispatched (a concern entirely owned by +`apps/api`, below). + +### One discovered inventory drives both composition and runtime startup + +`prepareStartupModuleInventory` (`apps/api/src/platform-permissions.ts`) discovers the installed +module inventory exactly once and returns both the discovered module list and the platform +permission registry composed from it, together. `apps/api`'s production startup +(`startApi`, `apps/api/src/server.ts`) passes that same discovered list directly to +`startDiscoveredModuleRuntime` — a Core entry point that performs no discovery of its own and +preserves the given list's order exactly — so permission composition and executable module +startup are provably looking at the same installed-module set, never two independently discovered +ones. `startModuleRuntime` (discover-then-start) remains available as a convenience wrapper for +callers that do not already have a discovered list; production never calls it. The test-only +`discoveredModulesOverride` drives both composition and runtime startup identically, for the same +reason: it stands in for the one real discovered list, never a second, competing one. + +Production startup composes in one fixed order: discover the module inventory and compose the +platform permission registry from it; construct the shared PostgreSQL pool; construct Identity, +the authorization store, and the evaluator; start the module runtime from that same discovered +inventory; create the Hono application and mount the started runtime's published routes; bind the +HTTP server. A discovery or composition failure therefore occurs before any pool, Better Auth +instance, or executable module code exists; a database/identity construction failure occurs before +any module runtime starts; and a module runtime or app-construction failure binds no server. Every +cleanup path preserves the original failure if cleanup itself also fails, matching the sanitized, +non-masking failure discipline the rest of the platform already follows. + +### The platform-owned `/api/v1/modules/` namespace, enforced identically to Core's own routes + +`apps/api/src/module-http-routes.ts` — the one place in the repository below `apps/api` importing +Hono for this purpose — mounts a started runtime's published route snapshot under +`/api/v1/modules/`, in publication order, never sorted or otherwise rewritten. A local +`"/"` maps to the bare module prefix; every other local path appends directly; the platform alone +decides this mapping, and a module has no way to choose or influence its own mount prefix. This +ordering is load-bearing, not cosmetic: Hono's router resolves two routes whose patterns overlap at +the same depth (a static segment and a same-position `:parameter`) by registration order, not by +static-path specificity, so preserving the runtime's exact publication order is what lets a module +author's more-specific-first declaration continue to take precedence after mounting. Mounting uses +one fixed-arity `app.on(method, path, ...middleware, handler)` call per route rather than spreading +a variable-length handler array — Hono's overload resolution is arity-sensitive, and a spread call +silently resolves to an unrelated, incompatible overload. + +Every mounted route is protected by the identical `requirePermission` middleware Core's own routes +use (ADR 0013), consulting `route.permission` against the exact same composed platform registry — +there is no module-specific enforcement path, no wildcard, prefix, namespace, or implication +matching, and a route's baseline always comes from the registry, never the route. A public +permission still resolves a principal and traverses the same authorization check as any other +baseline; only the evaluator's own decision differs. Because the mounted routes and the injected +registry both derive from the same discovered inventory, every mounted route's permission should +already be registered; `apps/api` still checks this invariant explicitly at app-construction time +and fails construction with a fixed, sanitized configuration error if it ever does not hold, rather +than silently mounting an unenforceable route — `requirePermission`'s own per-request registry +lookup remains the live, defense-in-depth enforcement regardless of that check. + +### Trusted origin, and platform-owned `OPTIONS`/`HEAD` + +A state-changing method (`POST`, `PUT`, `PATCH`, `DELETE`) additionally requires an exact trusted +`Origin`, checked before authorization or the handler ever run — the identical `requireTrustedOrigin` +mechanism the explicit-grant mutation routes already use (ADR 0013), for the identical reason: CORS +response headers alone do not stop a browser from *sending* a state-changing cross-origin request, +only from reading its response. `GET` does not require an `Origin`. `OPTIONS` is never authorable by +a module and remains entirely platform-owned: the shared CORS middleware intercepts every preflight +request before any route-specific middleware or handler runs, so a preflight never reaches +authorization or module code. `HEAD` is not separately authorable either; Hono's own router +transparently re-dispatches a `HEAD` request through the identical registered `GET` chain and +strips the response body afterward, so a `HEAD` request is authorized exactly like the `GET` it +mirrors and cannot bypass anything a direct `GET` would not also bypass. The platform's CORS policy +advertises `GET`, `POST`, `PUT`, `PATCH`, `DELETE`, and `OPTIONS`, and reflects only an exact +configured trusted origin — never a wildcard, and never anything module-influenced. + +### A frozen, minimal handler context and sanitized failure boundary + +A handler receives the unmodified Fetch `Request` (`c.req.raw`, so `Request.url`, headers, method, +body, and a streaming request all behave normally, and the full query string remains available +without a competing query abstraction) and a frozen context of exactly `moduleId`, the +already-authorized `EkklesiaPrincipal`, and an immutable, Hono-decoded parameter record — an empty +frozen object for a root or fully static route. A handler never receives a Hono `Context`, a +database connection, the authorization store or evaluator, PostgreSQL, configuration, secrets, or +any other middleware-internal state. Module code is executable, trusted backend plugin code, not a +sandboxed boundary — the same trust model ADR 0006 already established for capability registration +and lifecycle hooks — but its HTTP-facing boundary is still sanitized: a handler that throws, +rejects, or returns anything other than a `Response` becomes a fixed `500 +{"code":"MODULE_HANDLER_FAILED"}` with `Cache-Control: no-store`, and nothing about the caught +value, a stack, the module ID, the route's path, or its permission ID is ever included. A +`Response` a handler deliberately returns — including a 4xx or 5xx one — is never remapped, and a +streaming body is passed through unread rather than consumed or buffered while headers are +sanitized. + +### Platform-controlled caching and CORS on every module response + +The platform remains the sole owner of caching and CORS for a module response: any +`Access-Control-*` header a handler set is stripped (case-insensitively, since header names are +themselves case-insensitive) before the response leaves the process, and every module response is +forced to `Cache-Control: no-store` with a `Vary` that always includes `Origin` and `Cookie` — +merged case-insensitively so a differently-cased duplicate never appears, and collapsed to a bare +`*` rather than an invalid combination whenever an existing `Vary` already contains one (RFC 9110 +§12.5.5 forbids combining `*` with any other value) — while any of a handler's own legitimate +`Vary` values are preserved alongside the forced ones. This is deliberately conservative: even a +public-baseline handler receives an authorized principal and may personalize its response, so +until a dedicated module-declared cache-policy contract exists, no module response is ever treated +as cacheable or origin-independent by default. + +### Discovery, Doctor, and `modules:validate` remain manifest-only; production runtime startup does not + +Module discovery, Doctor's checks, `modules:validate`, and permission composition continue to read +only manifests — none of them imports or executes a module's server entrypoint, register hook, or +any other lifecycle code, exactly as ADR 0006 and ADR 0014 established. A *successful* production +runtime startup is the one deliberate exception: it imports each installed module's compiled server +entrypoint and runs its `register` and `start` hooks, because that is the only way a module's +declared routes and capabilities become real, dispatchable behavior. Entrypoint modules remain +trusted backend plugins, not a sandbox boundary (ADR 0006); their top-level ESM evaluation must +remain side-effect-free, because ESM evaluation cannot be rolled back or unloaded once it has run — +this was already a hard requirement for capability registration and remains exactly as hard a +requirement now that routes are declared the same way, during the same hook. + +### Restart-based static installation, and ordered runtime shutdown + +Installing, removing, or editing a module's routes takes effect only on the next API restart, +exactly like permission composition (ADR 0014) — there is no hot installation, removal, or dynamic +route mutation while the process runs. Removing a module makes its former routes return the +platform's ordinary `404 {"code":"NOT_FOUND"}`, the same fallback every unmatched path already +receives. `RunningApi`'s shutdown stops accepting new HTTP work and drains in-flight requests +first, then stops the module runtime, then closes the shared PostgreSQL pool — in that order, so no +module resource stops while a route handler might still be executing. One memoized shutdown promise +is shared by every concurrent or later caller; each of the three steps runs at most once regardless +of how many times `shutdown()` is called. Unlike a startup-failure cleanup path (which always +swallows a secondary cleanup failure so it never masks the original error), a module-stop failure +during *normal* shutdown is not swallowed — it is a genuine result a caller should observe — but +database cleanup still always completes first, so a stop failure never leaves the pool open. + +## Alternatives + +- **Hono or another framework's own request/response/routing types in the contracts layer** was + rejected: it would leak an implementation-layer framework choice into the one public contract + every module and Core depend on, and would make swapping the HTTP transport a breaking contract + change instead of an internal one. +- **Declaring routes in the static module manifest** (alongside capabilities and permissions) was + rejected: a route's `handler` is executable behavior, not inert data a manifest can express + without executing module code, and duplicating the same route between a manifest and the runtime + definition would create two sources of truth that could silently drift. +- **Permissionless routes, or a route/handler that selects its own baseline** were rejected outright + by the contract's shape: there is no field for either, so no caller can construct a route that + omits a permission or asserts its own trust level. +- **An arbitrary or module-chosen mount prefix** was rejected in favor of a single platform-owned + `/api/v1/modules/` namespace: a module choosing its own path risks colliding with + Core's own routes or another module's, and removes the platform's ability to reason about the + complete route surface from one place. +- **A second module discovery pass, or a second route/runtime registry, for HTTP routes + specifically** was rejected in favor of reusing the exact discovery Phase 5A already performs and + staging routes inside the same `StartedModuleRuntime` capabilities already publish through — + avoiding two independently discovered inventories that could disagree, and two registry + implementations that could drift. +- **Module-owned authorization, CORS, or origin-checking middleware** was rejected: every module + route is enforced through the identical, already-audited `requirePermission`/`requireTrustedOrigin` + mechanism Core's own routes use, so there is exactly one enforcement mechanism to reason about and + audit, not one per module. +- **`OPTIONS` or `HEAD` as module-authorable methods** were rejected: `OPTIONS` is inseparable from + the platform's own CORS policy, and `HEAD` is fully and correctly served by Hono's own transparent + `GET`-chain re-dispatch; making either authorable would only create a second, competing code path + for behavior the platform already provides correctly. +- **Dynamic or hot route mutation** (adding, removing, or replacing a route while the process runs) + was rejected for this phase: it would reopen the same concurrent-mutation and partial-update + hazards ADR 0014 already rejected for permission composition, for no benefit the current + deployment model needs. +- **A module-declared cache policy** (letting a route specify its own `Cache-Control`/`Vary` + behavior) was rejected for this phase: the conservative default (`no-store`, vary on `Origin` and + `Cookie`) is always correct, while a wrong module-declared policy could leak one user's + personalized response to another; a real cache-policy contract is deferred, deliberately, rather + than guessed at now. + +## Consequences + +A module gains a real, dispatchable HTTP surface the moment its `register` hook declares routes +against permissions its own manifest already declares — no second registration step, no +module-specific transport code to write. The integration boundary is safe by construction: the +contracts-layer shape structurally prevents a permissionless route or a module-asserted baseline, +Core's ownership check structurally prevents borrowing another owner's permission, and `apps/api`'s +mounting reuses exactly the enforcement, origin-checking, and response-sanitization machinery +already audited for Core's own routes. The operational cost mirrors ADR 0014's: every module +inventory or route change requires an API restart to take effect, and — because production startup +now deliberately imports and executes trusted module entrypoints, not merely their manifests — a +module's compiled server code must be as trustworthy as any other backend plugin code Core already +runs, since nothing about this design sandboxes it. + +## Deferred + +Module database or Drizzle access; module storage access; jobs and workflow/event automation; +module-owned configuration and secrets services; a `ModuleRuntimeContext` authorization service; +relationship-derived grant operations; reinstall/reactivation semantics for a permission or route +whose owning module was previously removed; roles, groups, teams, and tenants; an owner-management +HTTP endpoint; service accounts and API tokens for non-browser automation; a native-client +trusted-origin exemption policy; a module-declared cache-policy contract; hot module +installation, removal, or reloading; Surfaces and UI; and real product-module endpoints. None of +these are implemented by, or required for, the architecture this ADR records. diff --git a/docs/architecture/decisions/README.md b/docs/architecture/decisions/README.md index 96c8c0b..ce1d199 100644 --- a/docs/architecture/decisions/README.md +++ b/docs/architecture/decisions/README.md @@ -23,3 +23,4 @@ ADRs capture durable choices that affect more than one package or phase. - [0012: Production-equivalent identity origin/CSRF protection under test](0012-identity-origin-csrf-protection-under-test.md) — Accepted - [0013: Core authorization, permission registry, and platform-owner bootstrap](0013-core-authorization-and-owner-bootstrap.md) — Accepted - [0014: Module permission declarations and platform permission registry](0014-module-permission-declarations-and-platform-registry.md) — Accepted +- [0015: Static module HTTP routing and production runtime lifecycle](0015-static-module-http-routing-and-runtime-lifecycle.md) — Accepted diff --git a/docs/development/testing.md b/docs/development/testing.md index 57a1743..aba4e2e 100644 --- a/docs/development/testing.md +++ b/docs/development/testing.md @@ -22,13 +22,18 @@ error-only sanitized logger that strips Better Auth's own message and arguments they contain, pinned 8–128 password-length bounds, sanitized failures, and that a database outage during principal resolution is never silently reported as anonymous), and Core migration discovery (deterministic ordering, the reserved `core` identity, and collision rejection) — all without a -live database. `apps/api`'s own unit tests cover its pure Hono app factory (route shapes, CORS -allow/deny/preflight, deterministic 404 and sanitized error responses), its runtime configuration, -its idempotent shutdown lifecycle (concurrent callers share one server-close-then-pool-close -promise; a startup failure after the pool was already created still closes it; `SIGINT`/`SIGTERM` -wiring is exercised through an injectable exit function rather than the real `process.exit`), and -that importing any of its modules — including the executable server module without calling its -exported `main` — never reads the environment, connects to PostgreSQL, or starts a server. +live database. `apps/api`'s own unit tests cover its pure Hono app factory (route shapes, module +route mounting/enforcement/CORS/response sanitization, CORS allow/deny/preflight, deterministic 404 +and sanitized error responses), its runtime configuration, its idempotent shutdown lifecycle +(concurrent callers share one server-close-then-module-runtime-stop-then-pool-close promise; a +module-stop failure never prevents pool cleanup; a startup failure after the pool was already +created still closes it, stopping the module runtime first if it had already started; +`SIGINT`/`SIGTERM` wiring is exercised through an injectable exit function rather than the real +`process.exit`), and that importing any of its modules — including the executable server module +without calling its exported `main` — never reads the environment, connects to PostgreSQL, imports +a module entrypoint, or starts a server. Real, working (non-throwing) module fixtures prove +production module-runtime startup/shutdown ordering and cleanup against a mocked pool/Better +Auth/server — see [Authorization](authorization.md#tests). `pnpm verify` is intentionally Docker-free. It never plans or applies migrations against a live database. diff --git a/packages/contracts/src/module-http.ts b/packages/contracts/src/module-http.ts index ccbbdd6..4678f4e 100644 --- a/packages/contracts/src/module-http.ts +++ b/packages/contracts/src/module-http.ts @@ -26,7 +26,8 @@ const parameterSegmentPattern = /^:[a-z][a-zA-Z0-9]*$/u; /** * A module-local route path — never the production-mounted `/api/v1/modules/` path a - * caller actually sends (pass 2). Not branded (unlike `PermissionId`): like `PackageRelativePath`, + * caller actually sends (`apps/api/src/module-http-routes.ts` owns that mapping). Not branded + * (unlike `PermissionId`): like `PackageRelativePath`, * this is a validated location, not an opaque cross-system identifier. `"/"` is the module root; * any other canonical path begins with `/`, never ends with `/`, and is composed only of lowercase * kebab-case static segments or single `:parameterName` segments — no empty or repeated slashes, no @@ -126,7 +127,8 @@ export function moduleRouteShapeKey(method: ModuleHttpMethod, path: ModuleRouteP * What a module's route handler receives about the caller and the match — never a database * connection, authorization-store, grant-management, storage, jobs, configuration, secrets, the * Hono `Context`, or a raw Better Auth session. `principal` is already authorized by the time a - * handler runs (pass 2 mounts and enforces); this pass never invokes a handler at all. + * handler runs: `apps/api/src/module-http-routes.ts` mounts and enforces every route before ever + * invoking its handler; this contracts-layer module never invokes a handler itself. */ export interface ModuleRouteHandlerContext { readonly moduleId: ModuleId; @@ -174,8 +176,8 @@ export interface ModuleHttpRouteRegistrar { /** One published route in `StartedModuleRuntime`'s immutable route snapshot — the same validated * fields as `ModuleRouteDefinition` plus the owning module's ID, which the production mount prefix - * (pass 2) uses to keep two modules' routes from ever colliding regardless of their own local - * paths. */ + * (`/api/v1/modules/`, `apps/api/src/module-http-routes.ts`) uses to keep two modules' + * routes from ever colliding regardless of their own local paths. */ export interface RegisteredModuleHttpRoute { readonly moduleId: ModuleId; readonly method: ModuleHttpMethod; -- 2.54.0 From 300f98c29d2ade0f9bc2c6e8a1e93f369fb35c5d Mon Sep 17 00:00:00 2001 From: ekklesia Date: Sat, 25 Jul 2026 10:39:06 -0500 Subject: [PATCH 5/5] fix: normalize final CORS Vary headers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Found during a targeted post-audit review: sanitizeModuleResponse's Vary fix (bf76f4e) canonicalized the module response's own Vary before Hono's own cors() middleware (apps/api/src/app.ts) ran, but cors() unconditionally appends its own "Vary: Origin" afterward, outside module-http-routes.ts's reach. The wire-level response app.fetch actually returns could therefore still read "*, Origin" for a module handler that sets Vary: *, or carry a case-different duplicate Origin token — reproduced first via a new app-level regression test hitting the final response, not sanitizeModuleResponse in isolation. Fixed by wrapping the shared corsMiddleware in withCanonicalVary (apps/api/src/app.ts), which re-canonicalizes whatever Vary cors() leaves behind only after it has fully finished, handling both the normal next()-driven path (via c.header(), since reassigning c.res triggers Hono's own header-preserving merge and would silently undo the fix) and the OPTIONS-preflight path (cors() returns a Response directly without touching c.res). Neither path reads or clones the response body. Refactored module-http-routes.ts's Vary logic into one shared parsing/dedup primitive with two policy-specific callers: mergedVaryHeader (module responses; always forces canonical Origin/Cookie, unchanged behavior) and the new canonicalizeVaryHeader (the app-level CORS boundary; only deduplicates and collapses "*", never injects a token that was not already present — an OPTIONS preflight has no Cookie/personalization concept to force). This also incidentally fixes the same pre-existing Origin duplication on /api/v1/principal (present since Phase 4B, flagged as a remaining concern in the prior pass), since it wraps the one shared middleware every /api/v1/* and /api/auth/* response already passes through. ADR 0015 required no changes: its Vary/CORS ownership statements are architectural, not tied to a specific function, and remain accurate. Co-Authored-By: Claude Sonnet 5 --- apps/api/src/app.ts | 58 ++++++++-- apps/api/src/module-http-routes.ts | 58 +++++++--- apps/api/tests/app.test.ts | 21 ++++ apps/api/tests/module-http-routes.test.ts | 122 +++++++++++++++++++++- 4 files changed, 236 insertions(+), 23 deletions(-) diff --git a/apps/api/src/app.ts b/apps/api/src/app.ts index 3207bd0..c6fd13d 100644 --- a/apps/api/src/app.ts +++ b/apps/api/src/app.ts @@ -6,10 +6,10 @@ import type { } from "@ekklesia/core/authorization"; import { type EkklesiaIdentityRuntime, IdentityRuntimeError } from "@ekklesia/core/identity"; import { Hono } from "hono"; -import type { Context } from "hono"; +import type { Context, MiddlewareHandler } from "hono"; import { cors } from "hono/cors"; import { createGrantsRoutes } from "./grants-routes.js"; -import { mountModuleHttpRoutes } from "./module-http-routes.js"; +import { canonicalizeVaryHeader, mountModuleHttpRoutes } from "./module-http-routes.js"; export interface CreateApiAppAuthorizationOptions { readonly evaluator: AuthorizationEvaluator; @@ -42,6 +42,44 @@ function sanitizedErrorResponse(c: Context, error: unknown, status: 500 | 503) { return c.json({ code }, status); } +/** + * Hono's own `cors()` unconditionally appends its own "Vary: Origin" after every response with a + * dynamic-function `origin` — including one that already has its own `Vary` (module responses + * force `Origin`/`Cookie`; `/api/v1/principal` sets `Origin, Cookie` directly; an `OPTIONS` + * preflight sets its own) — so the header `cors()` itself leaves behind can still be an invalid + * combined "*, Origin" or contain a case-different duplicate. This wraps `middleware` and + * canonicalizes whatever `Vary` remains only *after* it has fully finished, using + * `canonicalizeVaryHeader` — the same shared parsing/dedup primitive `mergedVaryHeader` (module + * responses) builds on, so there is one implementation to keep in sync, not two that could diverge + * — but never `mergedVaryHeader` itself: this boundary wraps *every* response, `OPTIONS` preflights + * included, and must never inject `Origin`/`Cookie` onto a response that never had them, only + * de-duplicate what is already there. Two return shapes have to be handled: a normal request lets + * `middleware` call `next()` and mutate `c.res` in place, so the fix-up goes through `c.header()` + * (reassigning `c.res` directly triggers Hono's own header-preserving merge and would silently + * restore the uncanonicalized value); an `OPTIONS` preflight makes `cors()` return a new `Response` + * directly without ever touching `c.res`, so the fix-up mutates that returned response's headers + * instead. Neither path reads or clones the response body, so a streaming body is untouched either + * way. Status, body, content type, ordinary headers, authorization, trusted-origin behavior, and + * route ordering are all unaffected — only the final `Vary` value changes. + */ +function withCanonicalVary(middleware: MiddlewareHandler): MiddlewareHandler { + return async (c, next) => { + const result = await middleware(c, next); + if (result instanceof Response) { + const existingVary = result.headers.get("Vary"); + if (existingVary !== null) { + result.headers.set("Vary", canonicalizeVaryHeader(existingVary)); + } + return result; + } + const existingVary = c.res.headers.get("Vary"); + if (existingVary !== null) { + c.header("Vary", canonicalizeVaryHeader(existingVary)); + } + return result; + }; +} + /** * Pure Hono application factory: no I/O, no environment reads, no process side effects, and it * never invokes a module route handler while composing routes. Suitable for unit and integration @@ -58,13 +96,15 @@ export function createApiApp(options: CreateApiAppOptions): Hono { const app = new Hono(); - const corsMiddleware = cors({ - allowHeaders: ["Content-Type"], - allowMethods: ["GET", "POST", "PUT", "PATCH", "DELETE", "OPTIONS"], - credentials: true, - maxAge: 600, - origin: (origin) => (origin && allowedOrigins.has(origin) ? origin : undefined), - }); + const corsMiddleware = withCanonicalVary( + cors({ + allowHeaders: ["Content-Type"], + allowMethods: ["GET", "POST", "PUT", "PATCH", "DELETE", "OPTIONS"], + credentials: true, + maxAge: 600, + origin: (origin) => (origin && allowedOrigins.has(origin) ? origin : undefined), + }), + ); app.use("/api/auth/*", corsMiddleware); app.use("/api/v1/*", corsMiddleware); diff --git a/apps/api/src/module-http-routes.ts b/apps/api/src/module-http-routes.ts index 2f5fc1f..3a6faef 100644 --- a/apps/api/src/module-http-routes.ts +++ b/apps/api/src/module-http-routes.ts @@ -53,15 +53,15 @@ function sanitizedModuleHandlerFailure(c: Context): Response { } /** - * `Vary` lists header *names*, which are themselves case-insensitive, so merging de-duplicates - * case-insensitively — a module's own `origin` must collapse with the forced `Origin` rather than - * appear as a separate, redundant entry — and always emits the canonical `Origin`/`Cookie` casing - * regardless of how (or whether) a module already listed them. A literal `*` (RFC 9110 §12.5.5) - * must never be combined with any other value; since `*` already means "may vary on anything," a - * strictly broader signal than any explicit list, an existing `*` collapses the result to `*` alone - * rather than producing the invalid `*, Origin, Cookie`. + * The one shared `Vary` parsing/canonicalization primitive: splits and trims comma-separated + * tokens and de-duplicates them case-insensitively — `Vary` lists header *names*, which are + * themselves case-insensitive, so a module's own `origin` must collapse with a later `Origin` + * rather than appear as a separate, redundant entry — while preserving first-seen casing and + * relative order. Never returns early on `*`; callers decide what a `*` token means for their own + * policy (see `mergedVaryHeader` and `canonicalizeVaryHeader` below, the two callers that build on + * this instead of re-implementing parsing/dedup independently). */ -export function mergedVaryHeader(existingVary: string | null): string { +function parseVaryTokens(existingVary: string | null): Map { const canonicalByLowercase = new Map(); for (const token of (existingVary ?? "").split(",")) { const trimmed = token.trim(); @@ -73,12 +73,46 @@ export function mergedVaryHeader(existingVary: string | null): string { canonicalByLowercase.set(key, trimmed); } } - if (canonicalByLowercase.has("*")) { + return canonicalByLowercase; +} + +/** A literal `*` (RFC 9110 §12.5.5) must never be combined with any other value; since `*` already + * means "may vary on anything," a strictly broader signal than any explicit list, its presence + * collapses the result to `*` alone rather than producing an invalid combination. */ +function serializeVaryTokens(tokens: ReadonlyMap): string { + if (tokens.has("*")) { return "*"; } - canonicalByLowercase.set("origin", "Origin"); - canonicalByLowercase.set("cookie", "Cookie"); - return [...canonicalByLowercase.values()].join(", "); + return [...tokens.values()].join(", "); +} + +/** + * Used only for a module's own response: always emits the canonical `Origin`/`Cookie` casing + * regardless of how (or whether) a module already listed them, since every module response is + * forced to vary on both (see `sanitizeModuleResponse`) — this is a deliberate product policy for + * module responses specifically, not a generic property every response through the platform's CORS + * boundary should have forced onto it (an `OPTIONS` preflight, for one, has no `Cookie` concept at + * all). + */ +export function mergedVaryHeader(existingVary: string | null): string { + const tokens = parseVaryTokens(existingVary); + if (tokens.has("*")) { + return "*"; + } + tokens.set("origin", "Origin"); + tokens.set("cookie", "Cookie"); + return serializeVaryTokens(tokens); +} + +/** + * Used at the app-level CORS boundary (`apps/api/src/app.ts`), which wraps every response — + * `OPTIONS` preflights and Core's own routes included, not only module ones — after Hono's own + * `cors()` has already appended its own `Vary: Origin`. Unlike `mergedVaryHeader`, this never + * injects a token that was not already present; it only de-duplicates and collapses `*`, since a + * generic boundary must not assume every response wants `Origin`/`Cookie` forced onto it. + */ +export function canonicalizeVaryHeader(existingVary: string): string { + return serializeVaryTokens(parseVaryTokens(existingVary)); } /** diff --git a/apps/api/tests/app.test.ts b/apps/api/tests/app.test.ts index a2bbe26..272344c 100644 --- a/apps/api/tests/app.test.ts +++ b/apps/api/tests/app.test.ts @@ -99,6 +99,27 @@ describe("Ekklesia API Hono application", () => { expect(response.headers.get("vary")).toContain("Cookie"); }); + it("never duplicates Origin in Vary even though the route sets it directly and Hono's own CORS middleware also appends it", async () => { + const runtime = fakeRuntime({ + resolvePrincipal: vi.fn().mockResolvedValue(ANONYMOUS_PRINCIPAL), + }); + const app = createApiApp({ + moduleRoutes: Object.freeze([]), + authorization: fakeAuthorization({}), + runtime, + trustedOrigins: [trustedOrigin], + }); + + const response = await app.request("/api/v1/principal", { + headers: { origin: trustedOrigin }, + }); + + const vary = response.headers.get("vary") ?? ""; + const tokens = vary.split(",").map((token) => token.trim().toLowerCase()); + expect(tokens.filter((token) => token === "origin")).toHaveLength(1); + expect(tokens).toContain("cookie"); + }); + it("returns the authenticated principal shape containing only kind and userId", async () => { const runtime = fakeRuntime({ resolvePrincipal: vi.fn().mockResolvedValue(createAuthenticatedPrincipal("user_123")), diff --git a/apps/api/tests/module-http-routes.test.ts b/apps/api/tests/module-http-routes.test.ts index ffc7d58..f2346e0 100644 --- a/apps/api/tests/module-http-routes.test.ts +++ b/apps/api/tests/module-http-routes.test.ts @@ -16,7 +16,11 @@ import { import type { EkklesiaIdentityRuntime } from "@ekklesia/core/identity"; import { describe, expect, it, vi } from "vitest"; import { createApiApp } from "../src/app.js"; -import { mergedVaryHeader, ModuleRouteConfigurationError } from "../src/module-http-routes.js"; +import { + canonicalizeVaryHeader, + mergedVaryHeader, + ModuleRouteConfigurationError, +} from "../src/module-http-routes.js"; const trustedOrigin = "https://dashboard.example.test"; const untrustedOrigin = "https://evil.example.test"; @@ -614,6 +618,35 @@ describe("module route CORS", () => { } }); + it("keeps the OPTIONS preflight's own Vary canonical and never force-injects Cookie, exercising the returned-Response path of the CORS-Vary fix-up", async () => { + const { app } = buildModuleApp({ + registry: permissionRegistry([{ id: writePermission, baseline: "public" }]), + routes: [ + moduleRoute({ + handler: vi.fn(), + method: "PUT", + moduleId: "calendar", + path: "/events", + permission: writePermission, + }), + ], + }); + + const response = await app.request("/api/v1/modules/calendar/events", { + headers: { "access-control-request-method": "PUT", origin: trustedOrigin }, + method: "OPTIONS", + }); + + const vary = response.headers.get("vary") ?? ""; + const tokens = vary.split(",").map((token) => token.trim()); + // Hono's own cors() unconditionally appends "Access-Control-Request-Headers" here since + // allowHeaders is non-empty; that is legitimate and must survive canonicalization. Cookie must + // never be force-injected onto a preflight response, which has no cookie/personalization + // concept at all — unlike a module response, which always forces it (see mergedVaryHeader). + expect(tokens).toEqual(["Origin", "Access-Control-Request-Headers"]); + expect(tokens.filter((token) => token.toLowerCase() === "origin")).toHaveLength(1); + }); + it("never invokes authorization or the handler for an OPTIONS preflight", async () => { const handler = vi.fn(async () => Response.json({ ok: true })); const evaluator: AuthorizationEvaluator = { @@ -872,6 +905,28 @@ describe("module route response header policy", () => { }); }); + // The generic app-level CORS-Vary fix-up (`apps/api/src/app.ts`) uses this instead of + // `mergedVaryHeader`: it must never inject a token (Origin, Cookie, or anything else) that was + // not already present, since it wraps every response through the shared CORS middleware — + // including an OPTIONS preflight, which has no Cookie/personalization concept at all. + describe("canonicalizeVaryHeader", () => { + it("collapses an existing * to a bare * instead of an invalid combination", () => { + expect(canonicalizeVaryHeader("*")).toBe("*"); + expect(canonicalizeVaryHeader("Origin, *")).toBe("*"); + }); + + it("de-duplicates case-insensitively without injecting Origin or Cookie", () => { + expect(canonicalizeVaryHeader("origin, Accept-Encoding, ORIGIN")).toBe( + "origin, Accept-Encoding", + ); + }); + + it("never injects a token that was not already present", () => { + expect(canonicalizeVaryHeader("Accept-Encoding")).toBe("Accept-Encoding"); + expect(canonicalizeVaryHeader("")).toBe(""); + }); + }); + it("still forces Origin and Cookie into Vary end-to-end even when a handler supplies its own casing", async () => { const handler = vi.fn( async () => new Response("body", { headers: { Vary: "origin" }, status: 200 }), @@ -921,9 +976,72 @@ describe("module route response header policy", () => { ], }); - const response = await app.request("/api/v1/modules/calendar/events"); + const response = await app.request("/api/v1/modules/calendar/events", { + headers: { origin: trustedOrigin }, + }); expect(await response.text()).toBe(chunks.join("")); }); + + // These two exercise the *final* response `app.fetch` actually returns — after Hono's own, + // separate `cors()` middleware (`apps/api/src/app.ts`) has also run and unconditionally appended + // its own "Vary: Origin" following every module response, on top of whatever this module's own + // sanitization already set. `sanitizeModuleResponse`'s merge alone cannot prove the wire-level + // result is well-formed, because cors's append happens strictly afterward, outside this file's + // code. + it("keeps the final response's Vary exactly \"*\" end-to-end, even after Hono's own CORS append", async () => { + const handler = vi.fn( + async () => new Response("body", { headers: { Vary: "*" }, status: 200 }), + ); + const { app } = buildModuleApp({ + registry: permissionRegistry([{ id: readPermission, baseline: "public" }]), + routes: [ + moduleRoute({ + handler, + method: "GET", + moduleId: "calendar", + path: "/events", + permission: readPermission, + }), + ], + }); + + const response = await app.request("/api/v1/modules/calendar/events", { + headers: { origin: trustedOrigin }, + }); + expect(response.headers.get("vary")).toBe("*"); + }); + + it("collapses a differently-cased existing token with Hono's own appended Origin into one case-insensitive final token", async () => { + const handler = vi.fn( + async () => + new Response("body", { headers: { Vary: "origin, Accept-Encoding" }, status: 200 }), + ); + const { app } = buildModuleApp({ + registry: permissionRegistry([{ id: readPermission, baseline: "public" }]), + routes: [ + moduleRoute({ + handler, + method: "GET", + moduleId: "calendar", + path: "/events", + permission: readPermission, + }), + ], + }); + + const response = await app.request("/api/v1/modules/calendar/events", { + headers: { origin: trustedOrigin }, + }); + const vary = response.headers.get("vary") ?? ""; + const tokens = vary + .toLowerCase() + .split(",") + .map((token) => token.trim()); + const originOccurrences = tokens.filter((token) => token === "origin").length; + expect(originOccurrences).toBe(1); + expect(tokens).toContain("accept-encoding"); + expect(tokens).toContain("cookie"); + }); }); describe("module route HEAD semantics", () => { -- 2.54.0