From 1be06e6df9808a5480411fecbcd6d181fafe637a Mon Sep 17 00:00:00 2001 From: kosako Date: Sun, 5 Jul 2026 15:14:23 +0900 Subject: [PATCH] =?UTF-8?q?receiver:=20=E3=83=AB=E3=83=BC=E3=83=86?= =?UTF-8?q?=E3=82=A3=E3=83=B3=E3=82=B0=E3=82=92=E5=AE=A3=E8=A8=80=E7=9A=84?= =?UTF-8?q?=20route=20table=20+=20auth=20policy=20=E3=81=AB=E7=BD=AE?= =?UTF-8?q?=E6=8F=9B=E3=81=99=E3=82=8B=20(#43=20PR-1)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 手続き的 if 連鎖で認証適用を 4 分岐に手で反復していたルーティングを、 { method, pattern, auth, handler } のルート表 + 一元 dispatch に置き換える。 #43/#44 で認証種別が operation / inbox / ingest に増える前の準備で、 挙動同一の純リファクタ。 - 各ルートが auth 種別を宣言し、dispatch が一箇所で適用(宣言漏れは起動時 throw) - 既知 path への未対応 method は 404 → 405 + Allow ヘッダー(監査指摘。唯一の意図した挙動変更) - OPTIONS 204・rate limit の適用順・404 文言・認証対象 endpoint は不変 - テスト 94 → 96 件(405 の固定/パラメータ付き path、404 回帰) Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01TZtNiToBjzkwFc2A6Mw7n4 --- server/receive.js | 91 +++++++++++++++++++------------------------ test/receiver.test.js | 22 +++++++++++ 2 files changed, 62 insertions(+), 51 deletions(-) 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`;