From c4eb9dde1fb22713c7eec9efac41a02d887dadd6 Mon Sep 17 00:00:00 2001 From: Lilleman auf Larv Date: Sun, 8 Nov 2026 19:57:54 +0100 Subject: [PATCH] =?UTF-8?q?Answer=20security=20round=201:=200=20?= =?UTF-8?q?=F0=9F=94=B4,=200=20=F0=9F=9F=A1=20(0=20misread);=20route=20on?= =?UTF-8?q?=20the=20method=20and=20path=20read=20before=20the=20hooks?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/http/app-plugins.test.ts | 17 +++++++++++++++++ src/http/app.ts | 17 +++++++++++------ 2 files changed, 28 insertions(+), 6 deletions(-) diff --git a/src/http/app-plugins.test.ts b/src/http/app-plugins.test.ts index 8474599..2105737 100644 --- a/src/http/app-plugins.test.ts +++ b/src/http/app-plugins.test.ts @@ -301,3 +301,20 @@ test("plugin hooks: onRequest can short-circuit a request and onResponse observe 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.ts b/src/http/app.ts index 38f40ae..0430ca2 100644 --- a/src/http/app.ts +++ b/src/http/app.ts @@ -238,6 +238,9 @@ interface RequestState { 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; } // Before auth: assets don't need a verified user, and the JWT cookie rides every request. @@ -298,6 +301,8 @@ async function prepareRequest(host: Host, req: IncomingMessage, res: ServerRespo contextFor: (pluginId, params) => buildContext(req, res, { ...shared, t: i18n.translator(locale, pluginId), ...(params ? { params } : {}) }), csrf, ctx: buildContext(req, res, { ...shared, t }), + method: req.method ?? "GET", + pathname: url.pathname, }; } @@ -333,7 +338,7 @@ async function answerFromHook(host: Host, { carryLocale, contextFor, csrf, ctx } async function answerPluginRoute(host: Host, state: RequestState): Promise { const { carryLocale, contextFor, csrf, ctx } = state; - const match = matchRoute(host.plugins, ctx.req.method ?? "GET", ctx.url.pathname); + 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); @@ -352,7 +357,7 @@ async function answerPluginRoute(host: Host, state: RequestState): Promise { const { carryLocale, contextFor, csrf, ctx } = state; - const builtin = matchBuiltinRoute(host.builtinRoutes, ctx.req.method ?? "GET", ctx.url.pathname); + 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); @@ -361,8 +366,8 @@ async function answerBuiltinRoute(host: Host, state: RequestState): Promise { - const allow = allowedMethods(host.plugins, ctx.url.pathname); +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; @@ -372,9 +377,9 @@ async function answerUnrouted(host: Host, { ctx }: RequestState): Promise // 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 }: RequestState, gate: Gate, gateCtx: RequestContext): Promise { +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: ctx.url.pathname, required: gate.permission ?? "", sub: gateCtx.user.id }); + ctx.log.warn("forbidden: missing permission", { path: pathname, required: gate.permission ?? "", sub: gateCtx.user.id }); sendHtml(ctx.res, 403, await host.viewsFor(ctx)("403", {})); }