diff --git a/docs/reading-cost.md b/docs/reading-cost.md index a185fe4..9676b31 100644 --- a/docs/reading-cost.md +++ b/docs/reading-cost.md @@ -388,3 +388,20 @@ Read most (opens and searches): - `todo.md` 5 - `examples/plugins/admin/plugin.ts` 1 + +## Reading cost, chunk dd4d721e79d4, finished 2026-11-08T18:52:37Z + +90532 new input tokens before the first commit. + +Read most (opens and searches): + +- `todo.md` 8 +- `README.md` 7 +- `src/plugin-host/plugin.ts` 6 +- `src/plugin-host/plugin.test.ts` 4 +- `src/ui/chrome.ts` 4 +- `AGENTS.md` 3 +- `examples/plugins/scheduling/plugin.ts` 3 +- `src/plugin-host/plugin-api.ts` 3 +- `src/ui/chrome.test.ts` 3 +- `CHANGELOG.md` 2 diff --git a/src/function-limits.test.ts b/src/function-limits.test.ts index 5e30019..3828047 100644 --- a/src/function-limits.test.ts +++ b/src/function-limits.test.ts @@ -13,8 +13,7 @@ const LIMITS = { lines: 50, complexity: 20 }; // Each function over a limit, at its measurement. A ceiling follows its function down, and a metric // leaves the entry once it fits, so a function that shrank can't grow back. const grandfathered: Record = { - "src/http/app.ts createApp": { lines: 291, complexity: 28 }, - "src/http/app.ts createApp > handleRequest": { lines: 166, complexity: 43 }, + "src/http/app.ts createApp": { lines: 144, complexity: 26 }, "src/auth/flow-view.ts buildFlowView": { complexity: 24 }, "src/auth/hydra-admin.ts createHydraAdmin": { lines: 87 }, "src/auth/kratos-admin.ts createKratosAdmin": { lines: 68 }, diff --git a/src/http/app-plugins.test.ts b/src/http/app-plugins.test.ts index 8474599..1f5309c 100644 --- a/src/http/app-plugins.test.ts +++ b/src/http/app-plugins.test.ts @@ -13,6 +13,7 @@ import { staticJwks } from "../auth/jwks.ts"; import type { Identity } from "../auth/kratos-admin.ts"; import { KratosError, type Session } from "../auth/kratos-public.ts"; import { SESSION_COOKIE } from "../auth/login.ts"; +import { createLogger } from "../logger.ts"; import type { Plugin } from "../plugin-host/plugin.ts"; import { ecJwk, fakeKeto, mintJwt, startApp, stubAdmin, withWhoami } from "../test-tooling/app-harness.ts"; @@ -224,6 +225,48 @@ test("session re-mint: an expired JWT backed by a live Kratos session is silentl assert.equal(outage.headers.get("set-cookie"), null); }); +test("session re-mint runs only for an expired token with all three Ory clients wired; the new cookie is Secure under SECURE_COOKIES", async (t) => { + const identity: Identity = { id: "u1", traits: { email: "a@b.c" } }; + const nowSec = Math.floor(Date.now() / 1000); + const freshJwt = mintJwt({ email: "a@b.c", exp: nowSec + 600, permissions: ["demo:read"], sub: "u1" }); + let whoamiCalls = 0; + const kratos = withWhoami(async (o) => { whoamiCalls++; return (o?.tokenizeAs ? { active: true, identity, tokenized: freshJwt } : { active: true, identity }) as Session; }); + const keto = fakeKeto([{ namespace: "Permission", object: "demo:read", relation: "granted", subject_id: "user:u1" }], { check: async () => true }); + const kratosAdmin = stubAdmin({}); + const logged: string[] = []; + const log = createLogger({ format: "json", level: "info", stderr: (l) => logged.push(l), stdout: (l) => logged.push(l) }); + const valid = `${SESSION_COOKIE}=${freshJwt}`; + const expired = `${SESSION_COOKIE}=${mintJwt({ email: "a@b.c", exp: nowSec - 600, permissions: ["demo:read"], sub: "u1" })}`; + const get = async (options: Parameters[0], cookie: string): Promise => { + const app = createApp({ jwks: staticJwks([ecJwk]), log, plugins: [demoPlugin], ...options }); + await new Promise((r) => app.listen(0, r)); + t.after(() => app.close()); + return fetch(`http://localhost:${(app.address() as AddressInfo).port}/demo/secret`, { headers: { cookie }, redirect: "manual" }); + }; + const sessionCookieOf = (res: Response): string | undefined => res.headers.getSetCookie().find((c) => c.startsWith(`${SESSION_COOKIE}=`)); + + // A valid token, or one that is invalid but not expired, never reaches Ory. + const ok = await get({ keto, kratos, kratosAdmin }, valid); + assert.equal(ok.status, 200); + assert.equal(sessionCookieOf(ok), undefined); + assert.equal((await get({ keto, kratos, kratosAdmin }, `${SESSION_COOKIE}=not-a-jwt`)).status, 303); + assert.equal(whoamiCalls, 0); + + // An expired token with any one of the three clients missing stays anonymous, without trying. + for (const wired of [{ kratos, kratosAdmin }, { keto, kratosAdmin }, { keto, kratos }]) { + const res = await get(wired, expired); + assert.equal(res.status, 303, Object.keys(wired).join("+")); + assert.equal(sessionCookieOf(res), undefined); + } + assert.equal(whoamiCalls, 0); + assert.ok(!logged.some((l) => l.includes("session re-mint failed")), "no re-mint was attempted"); + + // All three wired on an https deployment: the re-minted cookie carries Secure. + const reminted = await get({ keto, kratos, kratosAdmin, secureCookies: true }, expired); + assert.equal(reminted.status, 200); + assert.match(sessionCookieOf(reminted) ?? "", /; Secure(;|$)/); +}); + test("guards map to responses: requireSession → /login, a failed can/check → 403, success runs the handler", async (t) => { const keto = { check: async (tuple: { object: string }) => tuple.object === "open" } as unknown as Parameters[0]; const guarded: Plugin = { @@ -285,19 +328,42 @@ test("plugin hooks: onRequest can short-circuit a request and onResponse observe onResponse: (c, r) => void seen.push(`${c.url.pathname}:${r && "html" in r ? r.html : "?"}`), }, id: "hooked", - routes: [{ handler: () => ({ html: "handler ran" }), method: "GET", path: "/ok" }], + routes: [ + { handler: () => ({ html: "handler ran" }), method: "GET", path: "/ok" }, + { handler: () => { seen.push("blocked handler ran"); return { html: "behind the hook" }; }, method: "GET", path: "/blocked" }, + ], }; const url = await startApp(t, [hooked]); - // onRequest short-circuits before routing — handler never runs; a fresh CSRF cookie still rides the - // response so a form the hook renders has its matching double-submit cookie. + // onRequest short-circuits before routing — the route behind it never runs; a fresh CSRF cookie + // still rides the response so a form the hook renders has its matching double-submit cookie. const blocked = await fetch(url + "/hooked/blocked"); assert.equal(blocked.status, 403); assert.match(await blocked.text(), /blocked by hook/); assert.match(blocked.headers.get("set-cookie") ?? "", /plainpages_csrf=/); + // Again with that cookie, so no Set-Cookie is due once the response is sent. + await (await fetch(url + "/hooked/blocked", { headers: { cookie: blocked.headers.get("set-cookie")?.split(";")[0] ?? "" } })).text(); + assert.ok(!seen.includes("blocked handler ran")); // A normal route runs the handler; onResponse observed its result. assert.match(await (await fetch(url + "/hooked/ok")).text(), /handler ran/); assert.ok(seen.includes("/hooked/ok:handler ran")); }); +test("plugin hooks: the request routes by the method and path it arrived with, whatever onRequest does to req", async (t) => { + const rewriter: Plugin = { + apiVersion: "1.0.0", + hooks: { onRequest: (c) => { c.req.method = "POST"; c.req.url = "/rewriter/other"; } }, + id: "rewriter", + routes: [ + { handler: () => ({ html: "original route" }), method: "GET", path: "/original" }, + { handler: () => ({ html: "rewritten route" }), method: "POST", path: "/other" }, + ], + }; + const url = await startApp(t, [rewriter]); + + const res = await fetch(url + "/rewriter/original"); + assert.equal(res.status, 200); + assert.match(await res.text(), /original route/); +}); + diff --git a/src/http/app-request-log.test.ts b/src/http/app-request-log.test.ts index 869111e..8b16905 100644 --- a/src/http/app-request-log.test.ts +++ b/src/http/app-request-log.test.ts @@ -15,13 +15,15 @@ async function requestLineOf(lines: string[]): Promise> throw new Error("no request line was logged"); } -async function serve(t: TestContext, trustedProxies: string[], plugins: Plugin[] = []): Promise<{ lines: string[]; url: string }> { +// `lines` is stdout (info); `problems` is stderr (warn and error). +async function serve(t: TestContext, trustedProxies: string[], plugins: Plugin[] = []): Promise<{ lines: string[]; problems: string[]; url: string }> { const lines: string[] = []; - const log = createLogger({ format: "json", level: "info", stderr: () => {}, stdout: (l: string) => lines.push(l) }); + const problems: string[] = []; + const log = createLogger({ format: "json", level: "info", stderr: (l: string) => problems.push(l), stdout: (l: string) => lines.push(l) }); const app = createApp({ log, plugins, trustedProxies }); await new Promise((r) => app.listen(0, r)); t.after(() => app.close()); - return { lines, url: `http://localhost:${(app.address() as AddressInfo).port}` }; + return { lines, problems, url: `http://localhost:${(app.address() as AddressInfo).port}` }; } test("the request line names the caller, the site and the matched route in OpenTelemetry's terms", async (t) => { @@ -64,3 +66,25 @@ test("a listed proxy lets X-Forwarded-For name the caller; an unmatched path car assert.equal(line["http.response.status_code"], 404); assert.equal(line["http.route"], undefined); }); + +test("an answered request goes no further: a refused visitor never reaches the handler, and nothing logs a problem", async (t) => { + const ran: string[] = []; + const plugin: Plugin = { + apiVersion: "1.0.0", + id: "demo", + routes: [ + { handler: () => ({ html: "open" }), method: "GET", path: "/open", public: true }, + { handler: () => { ran.push("gated"); return { html: "gated" }; }, method: "GET", path: "/gated", permission: "demo:read" }, + ], + }; + const { lines, problems, url } = await serve(t, [], [plugin]); + // A plugin route, a refused plugin route, a built-in route, a refused built-in route, an asset. + const paths = ["/demo/open", "/demo/gated", "/", "/dashboard", "/public/css/styles.css"]; + for (const path of paths) await (await fetch(url + path, { redirect: "manual" })).text(); + // A request that fell through to a later stage would log its error after its response closed. + while (lines.filter((l) => l.includes('"request"')).length < paths.length) await new Promise((r) => setTimeout(r, 10)); + await new Promise((r) => setTimeout(r, 100)); + + assert.deepEqual(ran, []); + assert.deepEqual(problems, []); +}); diff --git a/src/http/app.test.ts b/src/http/app.test.ts index 4b89d0f..9ec296f 100644 --- a/src/http/app.test.ts +++ b/src/http/app.test.ts @@ -2,7 +2,7 @@ import assert from "node:assert/strict"; import { randomUUID } from "node:crypto"; import { cpSync, mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs"; import { request as httpRequest } from "node:http"; -import type { AddressInfo } from "node:net"; +import { type AddressInfo, connect } from "node:net"; import { tmpdir } from "node:os"; import { dirname, join } from "node:path"; import { after, before, test } from "node:test"; @@ -214,6 +214,9 @@ test("static serving: GET sends body + content-type, HEAD headers only, unsafe p // Encoded traversal and a NUL byte are refused before touching the filesystem. assert.equal((await fetch(base + "/public/..%2f..%2fapp.ts")).status, 403); assert.equal((await fetch(base + "/public/%00")).status, 403); + + // Only GET/HEAD read an asset; any other method is routed like a page, and nothing answers it. + assert.equal((await fetch(base + "/public/css/styles.css", { method: "POST" })).status, 404); }); test("every response carries the security headers; HSTS follows SECURE_COOKIES", async (t) => { @@ -235,6 +238,20 @@ test("every response carries the security headers; HSTS follows SECURE_COOKIES", assert.match(res.headers.get("strict-transport-security") ?? "", /max-age=\d+/); }); +test("the CSRF cookie is set only when the request carries no valid token, and is Secure under SECURE_COOKIES", async (t) => { + const first = await fetch(base + "/"); + const csrf = first.headers.getSetCookie().find((c) => c.startsWith(`${CSRF_COOKIE}=`)) ?? ""; + assert.ok(csrf, "a first visit mints the token"); + assert.doesNotMatch(csrf, /; Secure/); // http dev + assert.deepEqual((await fetch(base + "/", { headers: { cookie: csrf.split(";")[0] ?? "" } })).headers.getSetCookie(), []); + + const secure = createApp({ secureCookies: true }); + await new Promise((r) => secure.listen(0, r)); + t.after(() => secure.close()); + const res = await fetch(`http://localhost:${(secure.address() as AddressInfo).port}/`); + assert.match(res.headers.getSetCookie().find((c) => c.startsWith(`${CSRF_COOKIE}=`)) ?? "", /; Secure(;|$)/); +}); + test("Cache-Control: pages and JSON are private, no-store unless the route sets its own", async (t) => { const NO_STORE = "private, no-store"; const csrf = (await fetch(base + "/")).headers.get("set-cookie")?.split(";")[0] ?? ""; @@ -305,6 +322,18 @@ function rawGet(port: number, path: string, host: string, method = "GET"): Promi }); } +// An HTTP/1.0 request with no Host header (Node refuses a Host-less HTTP/1.1 one with a 400); +// resolves to the raw response head. +function rawHttp10(port: number, path: string): Promise { + return new Promise((resolve, reject) => { + let raw = ""; + const socket = connect(port, "127.0.0.1", () => socket.write(`GET ${path} HTTP/1.0\r\n\r\n`)); + socket.on("data", (c) => (raw += c)); + socket.on("end", () => resolve(raw)); + socket.on("error", reject); + }); +} + test("APP_URL canonical-host redirect: an off-host visitor is 308'd to the configured origin (path+query kept)", async (t) => { // Reach the app on any host and it sends you to APP_URL's, so the browser, the themed form and the // cross-origin Kratos POST share ONE cookie host. Same-host requests pass straight through. @@ -329,6 +358,22 @@ test("APP_URL canonical-host redirect: an off-host visitor is 308'd to the confi // A 308 must not replay a cross-host POST — non-GET/HEAD is left alone (not canonicalised). const post = await rawGet(port, "/dashboard", `127.0.0.1:${port}`, "POST"); assert.notEqual(post.status, 308); + + // HEAD is canonicalised like GET. + const head = await rawGet(port, "/dashboard", `127.0.0.1:${port}`, "HEAD"); + assert.equal(head.status, 308); + assert.equal(head.location, "http://canonical.example:3000/dashboard"); + + // An HTTP/1.0 request may omit Host; with no host to compare it is served as-is. + assert.match(await rawHttp10(port, "/missing"), /^HTTP\/1\.[01] 404 /); +}); + +test("an APP_URL with no host (file:///srv) ⇒ no canonical redirect", async (t) => { + const app = createApp({ appUrl: "file:///srv" }); + await new Promise((r) => app.listen(0, r)); + t.after(() => app.close()); + const port = (app.address() as AddressInfo).port; + assert.equal((await rawGet(port, "/missing", `127.0.0.1:${port}`)).status, 404); }); test("no APP_URL configured ⇒ no canonical redirect (unit-test apps and host-agnostic deploys unaffected)", async () => { diff --git a/src/http/app.ts b/src/http/app.ts index c6fd9b4..2b84dfa 100644 --- a/src/http/app.ts +++ b/src/http/app.ts @@ -24,7 +24,7 @@ import type { KratosPublic } from "../auth/kratos-public.ts"; import { createLogger, type Log } from "../logger.ts"; import { remintSession } from "../auth/login.ts"; import { DEFAULT_MENU, type MenuConfig } from "../ui/menu-config.ts"; -import { declaredPermissions, fullPath, type Plugin, type RouteHandler, type RouteResult } from "../plugin-host/plugin.ts"; +import { declaredPermissions, fullPath, type PermissionDecl, type Plugin, type RouteHandler, type RouteResult } from "../plugin-host/plugin.ts"; import type { PluginSettings } from "../plugin-host/settings.ts"; import type { SystemCapabilities } from "../plugin-host/system.ts"; import { allows, type Gate } from "../auth/gate.ts"; @@ -34,7 +34,7 @@ import { parseTrustedProxies } from "./request-fields.ts"; import { trackRequest } from "./request-tracking.ts"; import { securityHeaders } from "./security-headers.ts"; import { localPath } from "./safe-url.ts"; -import { publicAssets } from "./static.ts"; +import { type PublicAssets, publicAssets } from "./static.ts"; import { renderPluginView } from "../plugin-host/view-resolver.ts"; const NO_STORE = "private, no-store"; @@ -72,8 +72,6 @@ export function createApp(options: AppOptions = {}): Server { const revoke = denylist ? (sub: string): void => denylist.revoke(sub) : undefined; const cache = options.cache ?? false; const canonical = options.appUrl ? new URL(options.appUrl) : undefined; - const canonicalHost = canonical?.host; // host[:port], default ports omitted — matches the Host header - const canonicalOrigin = canonical?.origin; // scheme + host[:port], no trailing slash const csrfSecret = options.csrfSecret ?? randomBytes(32).toString("hex"); // server passes config; tests pass their own const secureCookies = options.secureCookies ?? false; const trustedProxies = parseTrustedProxies(options.trustedProxies); @@ -141,11 +139,6 @@ export function createApp(options: AppOptions = {}): Server { const viewsFor = (ctx: RequestContext): ViewRenderer => (view, data) => render(view, { ...data, ...i18nLocals(localsOf(ctx)), assetHref: assets.href }); const pluginViewsFor = (ctx: RequestContext, id: string): ViewRenderer => (view, data) => renderView(id, view, { ...data, ...i18nLocals(localsOf(ctx)), assetHref: assets.href }); - const sendHtml = (res: ServerResponse, status: number, html: string): void => { - res.writeHead(status, { "content-type": "text/html; charset=utf-8" }); - res.end(html); - }; - // The public landing "/", ungated. A plugin may own it via `home`; else the built-in intro page. const serveHome = async (ctx: RequestContext, csrf: RequestCsrf, contextFor: PluginContextFactory): Promise => { csrf.setCookie(); @@ -183,9 +176,10 @@ export function createApp(options: AppOptions = {}): Server { { handler: serveDashboard, method: "GET", path: "/dashboard", session: true }, ]; - // The request handler. Run inside runWithLog (request-tracking.ts) so the per-request logger is ambient: every - // outbound fetch (the Ory clients via tracedFetch) and any deep module joins this request's trace - // and correlation with no logger threaded through their signatures. + const host: Host = { anyRequestHooks, anyResponseHooks, assets, authOptions, builtinRoutes, csrfSecret, i18n, jwks, keto, kratos, kratosAdmin, menu, permissionCatalog, pluginViewsFor, plugins, secureCookies, settingsCatalog, system, viewsFor }; + + // A stage that returns a boolean returns true once it has answered the request, and false to + // pass the request to the next stage. const handleRequest = async (req: IncomingMessage, res: ServerResponse, reqLog: Log): Promise => { // Error pages can render before this request has a context at all (a throw on the way to one), // so they start on the built-in English and switch to the visitor's locale once it is resolved. @@ -193,169 +187,227 @@ export function createApp(options: AppOptions = {}): Server { try { const method = req.method ?? "GET"; const url = new URL(req.url ?? "/", "http://localhost"); - const pathname = url.pathname; - // Set before any branch so every response — static/redirect/error included — inherits them // (writeHead merges these with its own headers; a plugin's RouteResult.headers can override). for (const [name, value] of secHeaderEntries) res.setHeader(name, value); - - // Before auth: assets don't need a verified user, and the JWT cookie rides every request. - if (pathname.startsWith("/public/") && (method === "GET" || method === "HEAD")) { - await assets.serve(url, req, res, (err) => reqLog.error("static file error", { error: String(err) })); - return; - } - - // A cache in front of us must key on the language. Set after the static branch: an asset is - // the same bytes in every language, and a Vary there fragments its entry per raw header. - res.setHeader("vary", "accept-language"); - // Fail closed: a cache in front of us must not keep a page that may carry a user's data. Not - // even the signed-out `/` is shared: it and the signed-in page have one URL, and a cache keyed - // on URL and Accept-Language would hand the signed-out one to a signed-in visitor. A route that - // is the same for everyone sets its own value in RouteResult.headers; the host doesn't police it. - res.setHeader("cache-control", NO_STORE); - - // Canonical host (APP_URL): send an off-host visitor to the configured origin so the browser, - // the themed forms and the cross-origin Kratos POST share one cookie host — otherwise the - // host-scoped Kratos CSRF cookie is lost and login dumps onto /error. GET/HEAD only: a 308 - // must not replay a cross-host POST. - if (canonicalHost && (method === "GET" || method === "HEAD") && req.headers.host !== undefined && req.headers.host !== canonicalHost) { - res.writeHead(308, { location: canonicalOrigin + (req.url ?? "/") }).end(); - return; - } - - // `explicit` (the URL asked for a locale) is what makes the choice travel: the chrome, this - // request's redirects and ctx.localeHref then carry ?locale onto the links they emit. - const { explicit, locale } = i18n.resolve({ acceptLanguage: req.headers["accept-language"], param: url.searchParams.get("locale") }); - const carryLocale = (href: string): string => localeHref(href, explicit ? locale : null); - const t = i18n.translator(locale); - - // A lapsed token still backed by a live Kratos session is silently re-minted — "stay signed - // in". The only place the hot path touches Ory. - let user: User | null = null; - if (jwks) { - const auth = await resolveSession(req.headers.cookie, jwks, authOptions); - user = auth.user; - if (!user && auth.expired && keto && kratos && kratosAdmin) { - try { - const reminted = await remintSession({ keto, kratosAdmin, kratosPublic: kratos }, req.headers.cookie, { secure: secureCookies }); - user = reminted.user; - res.appendHeader("set-cookie", reminted.setCookie); - } catch (err) { - // Ory unreachable — degrade to anonymous instead of 500ing every lapsed request. Leave - // the cookie alone: it can re-mint once Ory recovers. - reqLog.warn("session re-mint failed (Ory unreachable?)", { error: String(err) }); - } - } - } - const csrf = ensureCsrfToken(req.headers.cookie, csrfSecret); - const csrfMint: RequestCsrf = { - setCookie: (): void => { if (csrf.fresh) res.appendHeader("set-cookie", csrfCookie(csrf.token, { secure: secureCookies })); }, - token: csrf.token, - }; - const verifyCsrf = (submitted: string | null | undefined): boolean => - verifyCsrfRequest({ cookieHeader: req.headers.cookie, secret: csrfSecret, submitted }); - // Chrome composes the whole menu, so it is memoized and resolved lazily — a json/redirect - // handler, or the public "/" with a standalone home, never pays for it. - let chromeMemo: PageChrome | undefined; - const chrome = (): PageChrome => (chromeMemo ??= buildPluginChrome({ csrfToken: csrf.token, currentPath: pathname, localeHref: carryLocale, menu, plugins, t, translatorFor: (id) => i18n.translator(locale, id), user })); - - // A plugin's context gets the plugin's own translator — its catalog first, then core. - const i18nFor = (pluginId?: string) => ({ - locale, - localeHref: carryLocale, - locales: i18n.available, - t: pluginId === undefined ? t : i18n.translator(locale, pluginId), - }); - - // Base context (no route params), for the built-in routes. Every plugin-owned render — a - // landing slot, a hook short-circuit, a plugin route — gets `contextFor(id)` instead. - const shared = { assetHref: assets.href, chrome, declaredPermissions: permissionCatalog, declaredSettings: settingsCatalog, log: reqLog, user, verifyCsrf, ...(system ? { system } : {}) }; - const ctx = buildContext(req, res, { ...shared, ...i18nFor() }); - const contextFor = (pluginId: string, params?: Record): RequestContext => buildContext(req, res, { ...shared, ...i18nFor(pluginId), ...(params ? { params } : {}) }); - renderPage = viewsFor(ctx); - - // Plugin onRequest hooks run before routing and may short-circuit the request. - if (anyRequestHooks) { - const short = await runRequestHooks(plugins, contextFor); - if (short) { - // Like every other page-emitting path, so a form the hook renders has its matching cookie. - csrfMint.setCookie(); - await sendResult(res, short.result, pluginViewsFor(short.ctx, short.plugin.id), carryLocale); - return; - } - } - - // Anonymous → sign in, remembering the page as return_to; a signed-in user who simply lacks - // the permission gets the 403 page. - const refuse = async (gate: Gate, gateCtx: RequestContext): Promise => { - if (!gateCtx.user) { res.writeHead(303, { location: carryLocale(loginRedirect(gateCtx)) }).end(); return; } - reqLog.warn("forbidden: missing permission", { path: pathname, required: gate.permission ?? "", sub: gateCtx.user.id }); - sendHtml(res, 403, await renderPage("403", {})); - }; - - const match = matchRoute(plugins, method, pathname); - if (match) { - reqLog.context["http.route"] = fullPath(match.plugin.id, match.route.path); - const routeCtx = contextFor(match.plugin.id, match.params); - if (!allows(match.route, routeCtx.user)) { - await refuse(match.route, routeCtx); - return; - } - csrfMint.setCookie(); - const result = (await match.route.handler(routeCtx)) ?? null; - // The responding plugin observes its own route, params and all; the others get a plain - // context for their own id (never another plugin's params). - if (anyResponseHooks) await runResponseHooks(plugins, (id) => (id === match.plugin.id ? routeCtx : contextFor(id)), result); - await sendResult(res, result, pluginViewsFor(routeCtx, match.plugin.id), carryLocale); - return; - } - - const builtin = matchBuiltinRoute(builtinRoutes, method, pathname); - if (builtin) { - reqLog.context["http.route"] = builtin.path; - if (!allows(builtin, ctx.user)) { await refuse(builtin, ctx); return; } - await sendResult(res, await builtin.handler(ctx, csrfMint, contextFor), viewsFor(ctx), carryLocale); - return; - } - - // Known path, wrong method → 405 with Allow; otherwise nothing here → 404. - const allow = allowedMethods(plugins, pathname); - if (allow.length) { - res.writeHead(405, { allow: allow.join(", "), "content-type": "text/plain; charset=utf-8" }).end("Method Not Allowed"); - return; - } - sendHtml(res, 404, await renderPage("404", {})); + // Before the session is resolved: an asset needs no user, and verifying the JWT cookie that + // every asset request carries would be wasted work. + if (await serveStatic(assets, method, url, req, res, reqLog)) return; + setPageCacheHeaders(res); + if (redirectToCanonicalHost(canonical, method, req, res)) return; + const state = await prepareRequest(host, req, res, method, url, reqLog); + renderPage = viewsFor(state.ctx); + if (await answerFromHook(host, state)) return; + if (await answerPluginRoute(host, state)) return; + if (await answerBuiltinRoute(host, state)) return; + await answerUnrouted(host, state); } catch (err) { - // A guard thrown anywhere in handling maps to a response (not a 500): a `location` ⇒ a - // redirect (requireSession → /login), otherwise the status renders the error page. - if (err instanceof GuardError) { - if (res.headersSent) return void res.end(); - if (err.location) return void res.writeHead(303, { location: err.location }).end(); - try { - return void sendHtml(res, err.status, await renderPage("403", {})); - } catch (renderErr) { - // Same last resort as the 500 branch below: a throw here would leave the socket open - // (this catch is the one that would have handled it), so end the response ourselves. - reqLog.error("error page render failed", { error: renderErr instanceof Error ? (renderErr.stack ?? renderErr.message) : String(renderErr) }); - return void res.writeHead(err.status, { "content-type": "text/plain; charset=utf-8" }).end("Forbidden"); - } - } - reqLog.error("unhandled request error", { error: err instanceof Error ? (err.stack ?? err.message) : String(err) }); - if (res.headersSent) return void res.end(); // a partial body is already on the wire - try { - // Render before writing: if the 500 page itself throws, headers stay unsent - // and we fall back to plain text below instead of a half-written response. - sendHtml(res, 500, await renderPage("500", {})); - } catch (renderErr) { - reqLog.error("error page render failed", { error: renderErr instanceof Error ? (renderErr.stack ?? renderErr.message) : String(renderErr) }); - res.writeHead(500, { "content-type": "text/plain; charset=utf-8" }).end("Internal Server Error"); - } + await answerError(err, res, reqLog, renderPage); } }; return createServer((req, res) => trackRequest(log, trustedProxies, req, res, handleRequest)); } +// What the request stages read from createApp, all fixed at boot. +interface Host { + anyRequestHooks: boolean; + anyResponseHooks: boolean; + assets: PublicAssets; + authOptions: VerifyOptions; + builtinRoutes: BuiltinRoute[]; + csrfSecret: string; + i18n: I18n; + jwks: JwksProvider | undefined; + keto: KetoClient | undefined; + kratos: KratosPublic | undefined; + kratosAdmin: KratosAdmin | undefined; + menu: MenuConfig; + permissionCatalog: PermissionDecl[]; + pluginViewsFor: (ctx: RequestContext, id: string) => ViewRenderer; + plugins: Plugin[]; + secureCookies: boolean; + settingsCatalog: readonly PluginSettings[]; + system: SystemCapabilities | undefined; + viewsFor: (ctx: RequestContext) => ViewRenderer; +} + +// A request once its visitor is known: what every stage after prepareRequest reads. +interface RequestState { + carryLocale: (href: string) => string; + contextFor: (pluginId: string, params?: Record) => RequestContext; + csrf: RequestCsrf; + ctx: RequestContext; // the base context (no route params), for the built-in routes + // Read before the onRequest hooks run, so a hook that edits `req` doesn't reroute the request. + method: string; + pathname: string; +} + +async function serveStatic(assets: PublicAssets, method: string, url: URL, req: IncomingMessage, res: ServerResponse, reqLog: Log): Promise { + if (!url.pathname.startsWith("/public/") || (method !== "GET" && method !== "HEAD")) return false; + await assets.serve(url, req, res, (err) => reqLog.error("static file error", { error: String(err) })); + return true; +} + +function setPageCacheHeaders(res: ServerResponse): void { + // A cache in front of us must key on the language. Set after serveStatic: an asset is + // the same bytes in every language, and a Vary there fragments its entry per raw header. + res.setHeader("vary", "accept-language"); + // Fail closed: a cache in front of us must not keep a page that may carry a user's data. Not + // even the signed-out `/` is shared: it and the signed-in page have one URL, and a cache keyed + // on URL and Accept-Language would hand the signed-out one to a signed-in visitor. A route that + // is the same for everyone sets its own value in RouteResult.headers; the host doesn't police it. + res.setHeader("cache-control", NO_STORE); +} + +// Canonical host (APP_URL): send an off-host visitor to the configured origin so the browser, +// the themed forms and the cross-origin Kratos POST share one cookie host — otherwise the +// host-scoped Kratos CSRF cookie is lost and login dumps onto /error. GET/HEAD only: a 308 +// must not replay a cross-host POST. `URL.host` omits a default port, as the Host header does. +function redirectToCanonicalHost(canonical: URL | undefined, method: string, req: IncomingMessage, res: ServerResponse): boolean { + if (!canonical?.host || (method !== "GET" && method !== "HEAD") || req.headers.host === undefined || req.headers.host === canonical.host) return false; + res.writeHead(308, { location: canonical.origin + (req.url ?? "/") }).end(); + return true; +} + +// Resolves the visitor's locale, user and CSRF token, and builds the contexts handlers run on. +async function prepareRequest(host: Host, req: IncomingMessage, res: ServerResponse, method: string, url: URL, reqLog: Log): Promise { + const { i18n } = host; + // `explicit` (the URL asked for a locale) is what makes the choice travel: the chrome, this + // request's redirects and ctx.localeHref then carry ?locale onto the links they emit. + const { explicit, locale } = i18n.resolve({ acceptLanguage: req.headers["accept-language"], param: url.searchParams.get("locale") }); + const carryLocale = (href: string): string => localeHref(href, explicit ? locale : null); + const t = i18n.translator(locale); + const user = await resolveUser(host, req, res, reqLog); + const { fresh, token } = ensureCsrfToken(req.headers.cookie, host.csrfSecret); + const csrf: RequestCsrf = { + setCookie: (): void => { if (fresh) res.appendHeader("set-cookie", csrfCookie(token, { secure: host.secureCookies })); }, + token, + }; + const verifyCsrf = (submitted: string | null | undefined): boolean => + verifyCsrfRequest({ cookieHeader: req.headers.cookie, secret: host.csrfSecret, submitted }); + // Chrome composes the whole menu, so it is memoized and resolved lazily — a json/redirect + // handler, or the public "/" with a standalone home, never pays for it. + let chromeMemo: PageChrome | undefined; + const chrome = (): PageChrome => (chromeMemo ??= buildPluginChrome({ csrfToken: token, currentPath: url.pathname, localeHref: carryLocale, menu: host.menu, plugins: host.plugins, t, translatorFor: (id) => i18n.translator(locale, id), user })); + const shared = { assetHref: host.assets.href, chrome, declaredPermissions: host.permissionCatalog, declaredSettings: host.settingsCatalog, locale, localeHref: carryLocale, locales: i18n.available, log: reqLog, user, verifyCsrf, ...(host.system ? { system: host.system } : {}) }; + return { + carryLocale, + // The plugin's own translator: its catalog first, then core. + contextFor: (pluginId, params) => buildContext(req, res, { ...shared, t: i18n.translator(locale, pluginId), ...(params ? { params } : {}) }), + csrf, + ctx: buildContext(req, res, { ...shared, t }), + method, + pathname: url.pathname, + }; +} + +// A lapsed token still backed by a live Kratos session is silently re-minted — "stay signed +// in". The only place the hot path touches Ory. +async function resolveUser(host: Host, req: IncomingMessage, res: ServerResponse, reqLog: Log): Promise { + const { jwks, keto, kratos, kratosAdmin } = host; + if (!jwks) return null; + const auth = await resolveSession(req.headers.cookie, jwks, host.authOptions); + if (auth.user || !auth.expired || !keto || !kratos || !kratosAdmin) return auth.user; + try { + const reminted = await remintSession({ keto, kratosAdmin, kratosPublic: kratos }, req.headers.cookie, { secure: host.secureCookies }); + res.appendHeader("set-cookie", reminted.setCookie); + return reminted.user; + } catch (err) { + // Ory unreachable — degrade to anonymous instead of 500ing every lapsed request. Leave + // the cookie alone: it can re-mint once Ory recovers. + reqLog.warn("session re-mint failed (Ory unreachable?)", { error: String(err) }); + return null; + } +} + +async function answerFromHook(host: Host, { carryLocale, contextFor, csrf, ctx }: RequestState): Promise { + if (!host.anyRequestHooks) return false; + const short = await runRequestHooks(host.plugins, contextFor); + if (!short) return false; + // Like every other page-emitting path, so a form the hook renders has its matching cookie. + csrf.setCookie(); + await sendResult(ctx.res, short.result, host.pluginViewsFor(short.ctx, short.plugin.id), carryLocale); + return true; +} + +async function answerPluginRoute(host: Host, state: RequestState): Promise { + const { carryLocale, contextFor, csrf, ctx } = state; + const match = matchRoute(host.plugins, state.method, state.pathname); + if (!match) return false; + ctx.log.context["http.route"] = fullPath(match.plugin.id, match.route.path); + const routeCtx = contextFor(match.plugin.id, match.params); + if (!allows(match.route, routeCtx.user)) { + await refuse(host, state, match.route, routeCtx); + return true; + } + csrf.setCookie(); + const result = (await match.route.handler(routeCtx)) ?? null; + // The responding plugin observes its own route, params and all; the others get a plain + // context for their own id (never another plugin's params). + if (host.anyResponseHooks) await runResponseHooks(host.plugins, (id) => (id === match.plugin.id ? routeCtx : contextFor(id)), result); + await sendResult(ctx.res, result, host.pluginViewsFor(routeCtx, match.plugin.id), carryLocale); + return true; +} + +async function answerBuiltinRoute(host: Host, state: RequestState): Promise { + const { carryLocale, contextFor, csrf, ctx } = state; + const builtin = matchBuiltinRoute(host.builtinRoutes, state.method, state.pathname); + if (!builtin) return false; + ctx.log.context["http.route"] = builtin.path; + if (!allows(builtin, ctx.user)) await refuse(host, state, builtin, ctx); + else await sendResult(ctx.res, await builtin.handler(ctx, csrf, contextFor), host.viewsFor(ctx), carryLocale); + return true; +} + +async function answerUnrouted(host: Host, { ctx, pathname }: RequestState): Promise { + const allow = allowedMethods(host.plugins, pathname); + if (allow.length) { + ctx.res.writeHead(405, { allow: allow.join(", "), "content-type": "text/plain; charset=utf-8" }).end("Method Not Allowed"); + return; + } + sendHtml(ctx.res, 404, await host.viewsFor(ctx)("404", {})); +} + +// Anonymous → sign in, remembering the page as return_to; a signed-in user who simply lacks +// the permission gets the 403 page. +async function refuse(host: Host, { carryLocale, ctx, pathname }: RequestState, gate: Gate, gateCtx: RequestContext): Promise { + if (!gateCtx.user) { ctx.res.writeHead(303, { location: carryLocale(loginRedirect(gateCtx)) }).end(); return; } + ctx.log.warn("forbidden: missing permission", { path: pathname, required: gate.permission ?? "", sub: gateCtx.user.id }); + sendHtml(ctx.res, 403, await host.viewsFor(ctx)("403", {})); +} + +async function answerError(err: unknown, res: ServerResponse, reqLog: Log, renderPage: ViewRenderer): Promise { + // A GuardError maps to its redirect (requireSession → /login) or its status page, not a 500. + if (err instanceof GuardError) { + if (res.headersSent) return void res.end(); + if (err.location) return void res.writeHead(303, { location: err.location }).end(); + try { + return void sendHtml(res, err.status, await renderPage("403", {})); + } catch (renderErr) { + // answerError runs inside handleRequest's catch, so a throw here would escape and leave the + // socket open. End the response here, as the 500 branch below does. + reqLog.error("error page render failed", { error: renderErr instanceof Error ? (renderErr.stack ?? renderErr.message) : String(renderErr) }); + return void res.writeHead(err.status, { "content-type": "text/plain; charset=utf-8" }).end("Forbidden"); + } + } + reqLog.error("unhandled request error", { error: err instanceof Error ? (err.stack ?? err.message) : String(err) }); + if (res.headersSent) return void res.end(); // a partial body is already on the wire + try { + // Render before writing: if the 500 page itself throws, headers stay unsent + // and we fall back to plain text below instead of a half-written response. + sendHtml(res, 500, await renderPage("500", {})); + } catch (renderErr) { + reqLog.error("error page render failed", { error: renderErr instanceof Error ? (renderErr.stack ?? renderErr.message) : String(renderErr) }); + res.writeHead(500, { "content-type": "text/plain; charset=utf-8" }).end("Internal Server Error"); + } +} + +function sendHtml(res: ServerResponse, status: number, html: string): void { + res.writeHead(status, { "content-type": "text/html; charset=utf-8" }); + res.end(html); +} + // The Referer as a host-relative path, when it is one of ours — the page a form was submitted // from. Anything off-origin or malformed is discarded rather than trusted into a link. function sameOriginPath(req: IncomingMessage): string | null { diff --git a/todo.md b/todo.md index 3ba0351..9c2746f 100644 --- a/todo.md +++ b/todo.md @@ -6,13 +6,13 @@ `Bar = 1` -`Next ID = 59` +`Next ID = 60` ## Items | ID | Release | Blocked by | Exempt | Item | L | E | I | R | S | Moves | Score | |---|---|---|---|---|---|---|---|---|---|---|---| -| 50 | 0.5.0 | | | **Split `handleRequest` into named stages, so each fits the 50-line limit and the stage order is written once.** | 3 | 1 | 3 | 5 | 7 | code health by 7 | 3.7 | +| 59 | 0.5.0 | | | **Lower the reading cost of the files a chunk reads most before its first commit.** | 2 | 2 | 1 | 3 | 4 | code health by 3 | 9.0 | | 2 | 0.5.0 | | | **Rename the admin example plugin to a name that says what it administers.** | 3 | 1 | 2 | 4 | 6 | 9 by 3, code health by 2 | 2.6 | | 5 | 0.5.0 | | | **List grants that no installed plugin declares, with a remove action.** | 2 | 2 | 2 | 4 | 4 | 2 by 3 | 2.1 | | 41 | 0.5.0 | | | **Apply a permission save as one Keto call, so it cannot stop part-way.** | 2 | 2 | 2 | 4 | 3 | 2 by 2 | 2.1 | @@ -28,13 +28,12 @@ ## Details -### 50. Split `handleRequest` into named stages, so each fits the 50-line limit and the stage order is written once. +### 59. Lower the reading cost of the files a chunk reads most before its first commit. -The hardest place in all eight scoring seats of PR #200: `src/http/app.ts` `handleRequest` is about -165 lines at complexity 43, in `createApp`'s closure, and the order of its stages (headers, static, -canonical host, locale, session, CSRF, contexts, hooks, dispatch) is carried by comments only. Remove -it from `grandfathered` in `src/function-limits.test.ts`. Behaviour must not change; `app-*.test.ts` -is the net. Ships alone (R 5). +`reading-cost` reports a rise: the last 5 chunks' median is 87251 new input tokens before their +first commit, the 5 before them 70583. The files the last 5 chunks read most: `todo.md` 34 times, +`README.md` 24, `src/plugin-host/plugin.ts` 11, `src/plugin-host/plugin.test.ts` 10, `AGENTS.md` 6. +Cut or split what a chunk reads without needing it, then run `reading-cost --reset`. ### 2. Rename the admin example plugin to a name that says what it administers.