diff --git a/server/receive.js b/server/receive.js index 24c7421..08ce85a 100644 --- a/server/receive.js +++ b/server/receive.js @@ -145,6 +145,33 @@ function requireOperationAuth(req, res) { return false; } +// Every route declares its auth policy so a new endpoint cannot silently +// skip the check; dispatch applies it in one place instead of per-branch +// (#43). Auth kinds grow to inbox/ingest (plus per-route CORS) in the +// follow-up hardening slice. +const ROUTE_AUTH_KINDS = new Set(["none", "operation"]); +const ROUTES = [ + { method: "POST", pattern: /^\/feedback$/, auth: "none", handler: handlePostFeedback }, + { method: "POST", pattern: /^\/import$/, auth: "operation", handler: handlePostImport }, + { method: "DELETE", pattern: /^\/feedback\/([^/]+)$/, auth: "operation", + handler: (req, res, match) => handleDeleteFeedback(req, res, decodeURIComponent(match[1])) }, + { method: "POST", pattern: /^\/feedback\/([^/]+)\/status$/, auth: "operation", + handler: (req, res, match) => handlePostStatus(req, res, decodeURIComponent(match[1])) }, + { method: "POST", pattern: /^\/feedback\/([^/]+)\/github-issue$/, auth: "operation", + handler: (req, res, match) => handlePostGitHubIssue(req, res, decodeURIComponent(match[1])) }, + { method: "GET", pattern: /^\/(?:index\.html)?$/, auth: "none", handler: handleGetInbox }, + { method: "GET", pattern: /^\/feedback\.json$/, auth: "none", handler: handleGetFeedbackJson }, + { method: "GET", pattern: /^\/widget\.js$/, auth: "none", handler: handleGetWidgetScript }, + { method: "GET", pattern: /^\/static\//, auth: "none", handler: handleGetStaticAsset }, + { method: "GET", pattern: /^\/screenshots\//, auth: "none", handler: handleGetScreenshot } +]; + +for (const route of ROUTES) { + if (!ROUTE_AUTH_KINDS.has(route.auth)) { + throw new Error(`Unknown auth kind "${route.auth}" for route ${route.method} ${route.pattern}`); + } +} + const server = http.createServer((req, res) => { setCors(res); @@ -169,60 +196,22 @@ const server = http.createServer((req, res) => { return; } - if (req.method === "POST" && pathname === "/feedback") { - handlePostFeedback(req, res); - return; - } - - if (req.method === "POST" && pathname === "/import") { - if (!requireOperationAuth(req, res)) return; - handlePostImport(req, res); - return; - } - - const deleteMatch = req.method === "DELETE" && /^\/feedback\/([^/]+)$/.exec(pathname); - if (deleteMatch) { - if (!requireOperationAuth(req, res)) return; - handleDeleteFeedback(req, res, decodeURIComponent(deleteMatch[1])); - return; - } - - const statusMatch = req.method === "POST" && /^\/feedback\/([^/]+)\/status$/.exec(pathname); - if (statusMatch) { - if (!requireOperationAuth(req, res)) return; - handlePostStatus(req, res, decodeURIComponent(statusMatch[1])); - return; - } - - const githubMatch = req.method === "POST" && /^\/feedback\/([^/]+)\/github-issue$/.exec(pathname); - if (githubMatch) { - if (!requireOperationAuth(req, res)) return; - handlePostGitHubIssue(req, res, decodeURIComponent(githubMatch[1])); - return; - } - - if (req.method === "GET" && (pathname === "/" || pathname === "/index.html")) { - handleGetInbox(req, res); - return; - } - - if (req.method === "GET" && pathname === "/feedback.json") { - handleGetFeedbackJson(req, res); - return; - } - - if (req.method === "GET" && pathname === "/widget.js") { - handleGetWidgetScript(req, res); - return; - } - - if (req.method === "GET" && pathname.startsWith("/static/")) { - handleGetStaticAsset(req, res); + const allowedMethods = new Set(); + for (const route of ROUTES) { + const match = route.pattern.exec(pathname); + if (!match) continue; + if (route.method !== req.method) { + allowedMethods.add(route.method); + continue; + } + if (route.auth === "operation" && !requireOperationAuth(req, res)) return; + route.handler(req, res, match); return; } - if (req.method === "GET" && pathname.startsWith("/screenshots/")) { - handleGetScreenshot(req, res); + if (allowedMethods.size > 0) { + allowedMethods.add("OPTIONS"); + respondJson(res, 405, { ok: false, error: "Method Not Allowed" }, { "Allow": [...allowedMethods].join(", ") }); return; } diff --git a/test/receiver.test.js b/test/receiver.test.js index 6129572..ac10a6d 100644 --- a/test/receiver.test.js +++ b/test/receiver.test.js @@ -724,6 +724,28 @@ test("preflight OPTIONS requests are not rate limited", async (t) => { assert.equal((await fetch(url)).status, 429); }); +test("known paths reject unsupported methods with 405 and an Allow header", async (t) => { + const receiver = await startReceiver(t); + + const fixed = await fetch(`${receiver.baseUrl}/feedback`, { method: "GET" }); + assert.equal(fixed.status, 405); + assert.equal(fixed.headers.get("allow"), "POST, OPTIONS"); + assert.deepEqual(await fixed.json(), { ok: false, error: "Method Not Allowed" }); + + // Parameterized paths are recognized across methods too. + const parameterized = await fetch(`${receiver.baseUrl}/feedback/pl_x`, { method: "PUT" }); + assert.equal(parameterized.status, 405); + assert.equal(parameterized.headers.get("allow"), "DELETE, OPTIONS"); +}); + +test("unknown paths still return 404", async (t) => { + const receiver = await startReceiver(t); + + const response = await fetch(`${receiver.baseUrl}/no-such-route`); + assert.equal(response.status, 404); + assert.equal(await response.text(), "Not Found"); +}); + test("X-Forwarded-For is ignored for rate limiting unless trust proxy is set", async (t) => { const receiver = await startReceiver(t, { RATE_LIMIT_MAX: "1" }); const url = `${receiver.baseUrl}/feedback.json`;