diff --git a/README.md b/README.md index c5fc7ef..8bc739b 100644 --- a/README.md +++ b/README.md @@ -196,7 +196,8 @@ feedback は組み込みの `node:sqlite`（`server/feedback.db`）に保存し - `MAX_IMPORT_ITEMS` / `MAX_FIELD_LENGTH` / `MAX_ARRAY_LENGTH` / `MAX_OBJECT_DEPTH` env（または config の `maxImportItems` / `maxFieldLength` / `maxArrayLength` / `maxObjectDepth`）で、受信 payload の形（`POST /import` 1 回の件数・文字列長・配列長・ネスト深さ）の上限を変更できます。未設定でもローカルで困らない緩いデフォルト（`500` / `20000` / `1000` / `32`）が常に有効です（超過は `413`。`screenshot.dataUrl` は `SCREENSHOT_MAX_BYTES` 側で別途上限） - `RATE_LIMIT_MAX` / `RATE_LIMIT_WINDOW_MS` / `RATE_LIMIT_MAX_CLIENTS` env（または config の `rateLimitMax` / `rateLimitWindowMs` / `rateLimitMaxClients`）で、IP ごとの固定窓レート制限を変更できます（デフォルト `120` req / `60000` ms、超過は `429` + `Retry-After`）。クライアント識別は既定で socket の remoteAddress。reverse proxy 配下では `RECEIVER_TRUST_PROXY` を `1` にしたときだけ `X-Forwarded-For` 先頭を使います（既定では直アクセスがヘッダーで識別を偽装できないようにするため） - `MAX_FEEDBACK_COUNT` env（または config の `maxFeedbackCount`、デフォルト `100000`）を超えると新規保存を `507` で拒否します。`SCREENSHOT_DISK_MAX_BYTES` env（または config の `screenshotDiskMaxBytes`、デフォルト `500000000`）を超える screenshot 書き込みも `507` で拒否します（起動時にディスク使用量を実測し、保存・削除で追跡） -- `RECEIVER_TOKEN` env（または config の `receiverToken`）を設定すると、操作系 endpoint（`POST /import` / `POST /feedback/:id/status` / `DELETE /feedback/:id` / `POST /feedback/:id/github-issue`）に `Authorization: Bearer ` を必須にします。未設定ならローカルは認証なしで動作します（widget の `POST /feedback` と閲覧系 GET は対象外。inbox のブラウザ認証・CORS 制限は別途） +- `RECEIVER_TOKEN` env（または config の `receiverToken`）を設定すると、操作系 endpoint（`POST /import` / `POST /feedback/:id/status` / `DELETE /feedback/:id` / `POST /feedback/:id/github-issue`）と閲覧系 endpoint（`GET /`（inbox）/ `GET /feedback.json` / `GET /screenshots/:file`）に認証を必須にします。API からは `Authorization: Bearer `、ブラウザからは inbox のログインフォーム（`GET /login`）に同じ token を入力します。ログイン後は HMAC 派生値の HttpOnly cookie（有効期限 7 日、`SameSite=Lax`、`publicBaseUrl` が `https://` のとき `Secure` 付き）でセッションが維持され、生 token はブラウザに保存されません。token を変更すると全端末のセッションが即失効します。未設定ならローカルは認証なしで動作します（widget の `POST /feedback` と `GET /widget.js` は設定時も常に公開） +- `ALLOWED_ORIGINS` env（または config の `allowedOrigins`、env はカンマ区切り・config は配列）を設定すると、widget の投稿（`POST /feedback`）の CORS を許可 origin（`scheme://host[:port]` の完全一致）に制限します。JSON POST は必ず preflight されるため、リスト外 origin のブラウザ投稿は本体 POST の前にブラウザ側で遮断されます。未設定なら従来通り全 origin 許可（`*`）で、起動ログに警告が出ます。CORS ヘッダーが付くのは `POST /feedback` のみで、inbox・操作系・screenshot は同一オリジン利用のため CORS 自体を返しません。受信した feedback には `received: { origin, originAllowed }` が保存され、inbox の raw payload から確認できます（Origin ヘッダーは偽装可能なため参考情報です） - `SLACK_WEBHOOK_URL` env を設定すると、受信した feedback を Slack Incoming Webhook にも転送します - `SLACK_IMAGE_MODE` env で Slack 上の screenshot 表示方式を変更できます（`auto` / `link` / `block` / `upload` / `off`） - `SLACK_BOT_TOKEN` と `SLACK_UPLOAD_CHANNEL_ID` env を設定すると、保存済み screenshot を Slack file としてアップロードできます @@ -260,7 +261,25 @@ GITHUB_TOKEN="github_pat_..." GITHUB_REPO="owner/repo" node server/receive.js 設定済みの場合、inbox の各 card に `Create GitHub Issue` ボタンが表示されます。作成された issue には feedback 本文・reviewer・ページ URL・selector・対象位置・viewport・screenshot link・raw payload が含まれます。結果は保存済み payload の `integrations.github` に永続化され、card には issue link（失敗時はエラー）が表示されます。同じ feedback からの二重作成は拒否されます。API から行う場合は `POST /feedback/:id/github-issue` を使います。 -screenshot の画像は GitHub から `publicBaseUrl` に到達できる場合のみ issue 上に表示されます（ローカル receiver のままなら link のみ機能します）。 +screenshot の画像は GitHub から `publicBaseUrl` に到達できる場合のみ issue 上に表示されます（ローカル receiver のままなら link のみ機能します）。`RECEIVER_TOKEN` を設定している場合、GitHub の image proxy は認証を通れないため、issue には画像を埋め込まず `[Open screenshot]` リンクのみを載せます（開くには receiver へのログインが必要です）。 + +### 公開デプロイ（EC2 など） + +receiver はデフォルトでローカルプロトタイプ前提（`127.0.0.1` bind・認証なし・CORS `*`）です。インターネットに公開する場合は次を前提にしてください。 + +- **HTTPS 終端は reverse proxy（nginx / Caddy / ALB など）で行う**: receiver 自体は HTTP のみです。proxy の背後では `HOST` の bind 先を proxy からのみ届く interface に限定し、rate limit がクライアント IP を正しく見るよう `RECEIVER_TRUST_PROXY=1` を設定します +- **`RECEIVER_TOKEN` を必ず設定する**: 未設定のまま公開すると inbox・feedback データ・操作系がすべて無認証で露出します +- **`ALLOWED_ORIGINS` にデモページの origin を列挙する**: widget からの投稿を想定した origin に絞ります +- **`PUBLIC_BASE_URL` を `https://` の公開 URL にする**: Slack / GitHub に載せる screenshot link の到達性に加え、セッション cookie の `Secure` 属性がこの URL のスキームで決まります +- **secrets（`RECEIVER_TOKEN` / `GITHUB_TOKEN` / `SLACK_WEBHOOK_URL` など）は env 注入を推奨**: env は config ファイルより優先されます。config ファイル（`receiver.config.json`）に書く場合は git 管理外・ファイル権限の管理下に置いてください + +```sh +RECEIVER_TOKEN="" \ +ALLOWED_ORIGINS="https://demo.example.com" \ +PUBLIC_BASE_URL="https://feedback.example.com" \ +RECEIVER_TRUST_PROXY=1 \ +HOST=127.0.0.1 PORT=4000 node server/receive.js +``` ## Slack direct mode @@ -326,7 +345,7 @@ GitHub Issue 作成は receiver inbox からの手動操作のみで、自動作 - Slack App / OAuth 連携 - 永続 DB - pixel-perfect なブラウザ screenshot capture -- 認証 +- widget↔receiver のペア認証（ingest key。受信面の認証は `RECEIVER_TOKEN` + inbox ログインで対応済み） - AI PR 連携 ## License diff --git a/server/receive.js b/server/receive.js index 08ce85a..cb8f489 100644 --- a/server/receive.js +++ b/server/receive.js @@ -58,10 +58,27 @@ const GITHUB_ASSIGNEES = normalizeStringList(process.env.GITHUB_ASSIGNEES || con const GITHUB_API_BASE = trimTrailingSlash(process.env.GITHUB_API_BASE || config.githubApiBase || "https://api.github.com"); const GITHUB_TIMEOUT_MS = numberSetting(process.env.GITHUB_TIMEOUT_MS, numberSetting(config.githubTimeoutMs, 8000)); const GITHUB_CONFIGURED = Boolean(GITHUB_TOKEN && GITHUB_REPO); -// Optional shared token guarding the management/operation endpoints (import, -// status, delete, github-issue). Unset = zero-config local dev (no auth) so -// existing local workflows keep working; set it for public/shared deploys. +// Optional shared token guarding the management (import, status, delete, +// github-issue) and read (inbox, feedback.json, screenshots) endpoints. Unset = +// zero-config local dev (no auth) so existing local workflows keep working; set +// it for public/shared deploys. The same token backs both the API bearer auth +// and the inbox login form (a browser session holds an HMAC-derived cookie, not +// the token itself, so rotating the token invalidates every session at once). const RECEIVER_TOKEN = process.env.RECEIVER_TOKEN || config.receiverToken || ""; +// CORS allowlist for the widget ingest route (POST /feedback). Cross-origin +// JSON POSTs always preflight, so origins outside the list are blocked by the +// browser before the payload is sent. Unset keeps the historical open default +// (Access-Control-Allow-Origin: *) so zero-config local runs keep working; the +// startup log warns about it. Entries are exact origins (scheme://host[:port]). +const ALLOWED_ORIGINS = normalizeStringList(process.env.ALLOWED_ORIGINS || config.allowedOrigins).map(trimTrailingSlash); +// Browser sessions for the inbox: the cookie value is +// "." — a derived credential, +// never the raw token, valid for 7 days. Secure is tied to the deploy's public +// URL: an https publicBaseUrl means TLS termination is in place, so the cookie +// must not travel over plain http. +const SESSION_COOKIE_NAME = "patchloop_session"; +const SESSION_TTL_MS = 7 * 24 * 60 * 60 * 1000; +const SESSION_COOKIE_SECURE = PUBLIC_BASE_URL.startsWith("https://"); const IMPORT_BUNDLE_KIND = "patchloop-feedback-bundle"; // v1 wrapped a single feedback object; v2 carries an array (batch export). // Both are accepted so files exported before the batch-download switch still @@ -132,38 +149,92 @@ function safeTokenEqual(provided, expected) { return a.length === b.length && crypto.timingSafeEqual(a, b); } -// Management/operation endpoints (import, status, delete, github-issue) require -// the shared bearer token when RECEIVER_TOKEN is set. Returns true when the -// request may proceed; otherwise responds 401 and returns false. With no token -// configured, auth is disabled (zero-config local dev). The widget ingest path -// (POST /feedback) and read endpoints are intentionally not gated here. -function requireOperationAuth(req, res) { +// The signature covers the expiry, so a client cannot extend its own session, +// and a forged cookie fails the HMAC check without knowledge of the token. +function sessionSignature(expiresAtMs) { + return crypto.createHmac("sha256", RECEIVER_TOKEN).update(String(expiresAtMs)).digest("hex"); +} + +function cookieValue(req, name) { + const header = req.headers["cookie"]; + if (typeof header !== "string") return ""; + for (const pair of header.split(";")) { + const separator = pair.indexOf("="); + if (separator === -1) continue; + if (pair.slice(0, separator).trim() === name) return pair.slice(separator + 1).trim(); + } + return ""; +} + +function hasValidSessionCookie(req) { + const raw = cookieValue(req, SESSION_COOKIE_NAME); + const separator = raw.indexOf("."); + if (separator === -1) return false; + const expiry = raw.slice(0, separator); + const signature = raw.slice(separator + 1); + if (!/^\d+$/.test(expiry) || !signature) return false; + if (Number(expiry) <= Date.now()) return false; + return safeTokenEqual(signature, sessionSignature(expiry)); +} + +// Protected endpoints (management + reads) accept either the shared bearer +// token (curl / API clients) or a valid session cookie (the inbox UI — its +// same-origin fetches carry the HttpOnly cookie automatically, so inbox.js +// needs no auth wiring). With no token configured, auth is disabled +// (zero-config local dev). The widget ingest path (POST /feedback) is +// intentionally not gated here. +function isAuthorizedRequest(req) { if (!RECEIVER_TOKEN) return true; const header = req.headers["authorization"] || ""; if (safeTokenEqual(header, `Bearer ${RECEIVER_TOKEN}`)) return true; - respondJson(res, 401, { ok: false, error: "Unauthorized" }); - return false; + return hasValidSessionCookie(req); +} + +function sessionCookieAttributes() { + return `Path=/; HttpOnly; SameSite=Lax${SESSION_COOKIE_SECURE ? "; Secure" : ""}`; +} + +function issueSessionCookie(res) { + const expiresAtMs = Date.now() + SESSION_TTL_MS; + const value = `${expiresAtMs}.${sessionSignature(expiresAtMs)}`; + res.setHeader("Set-Cookie", `${SESSION_COOKIE_NAME}=${value}; Max-Age=${Math.floor(SESSION_TTL_MS / 1000)}; ${sessionCookieAttributes()}`); } -// 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"]); +function clearSessionCookie(res) { + res.setHeader("Set-Cookie", `${SESSION_COOKIE_NAME}=; Max-Age=0; ${sessionCookieAttributes()}`); +} + +function redirect(res, location) { + res.writeHead(303, { "Location": location }); + res.end(); +} + +// Every route declares its auth and CORS policy so a new endpoint cannot +// silently skip either check; dispatch applies them in one place instead of +// per-branch (#43). Auth kinds: "none" (public), "protected" (bearer token or +// session cookie, 401 on failure — APIs and resources), "page" (same +// credentials, but an unauthenticated browser is redirected to the login form +// instead of getting raw JSON). CORS headers are only emitted for routes with +// cors: true — the inbox, management endpoints, and screenshots are same-origin +// surfaces, so withholding the headers there is a second wall next to auth. +const ROUTE_AUTH_KINDS = new Set(["none", "protected", "page"]); const ROUTES = [ - { method: "POST", pattern: /^\/feedback$/, auth: "none", handler: handlePostFeedback }, - { method: "POST", pattern: /^\/import$/, auth: "operation", handler: handlePostImport }, - { method: "DELETE", pattern: /^\/feedback\/([^/]+)$/, auth: "operation", + { method: "POST", pattern: /^\/feedback$/, auth: "none", cors: true, handler: handlePostFeedback }, + { method: "GET", pattern: /^\/login$/, auth: "none", handler: handleGetLogin }, + { method: "POST", pattern: /^\/login$/, auth: "none", handler: handlePostLogin }, + { method: "POST", pattern: /^\/logout$/, auth: "none", handler: handlePostLogout }, + { method: "POST", pattern: /^\/import$/, auth: "protected", handler: handlePostImport }, + { method: "DELETE", pattern: /^\/feedback\/([^/]+)$/, auth: "protected", handler: (req, res, match) => handleDeleteFeedback(req, res, decodeURIComponent(match[1])) }, - { method: "POST", pattern: /^\/feedback\/([^/]+)\/status$/, auth: "operation", + { method: "POST", pattern: /^\/feedback\/([^/]+)\/status$/, auth: "protected", handler: (req, res, match) => handlePostStatus(req, res, decodeURIComponent(match[1])) }, - { method: "POST", pattern: /^\/feedback\/([^/]+)\/github-issue$/, auth: "operation", + { method: "POST", pattern: /^\/feedback\/([^/]+)\/github-issue$/, auth: "protected", 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: /^\/(?:index\.html)?$/, auth: "page", handler: handleGetInbox }, + { method: "GET", pattern: /^\/feedback\.json$/, auth: "protected", 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 } + { method: "GET", pattern: /^\/screenshots\//, auth: "protected", handler: handleGetScreenshot } ]; for (const route of ROUTES) { @@ -173,8 +244,6 @@ for (const route of ROUTES) { } const server = http.createServer((req, res) => { - setCors(res); - // Routes match the pathname so query strings (e.g. /feedback.json?v=2) // cannot turn a valid endpoint into a 404. let pathname; @@ -184,6 +253,13 @@ const server = http.createServer((req, res) => { pathname = req.url; } + // Method is part of the match (OPTIONS stands in for the preflight) so a 405 + // response on a cors-enabled path does not advertise CORS either. + if (ROUTES.some((route) => route.cors && route.pattern.test(pathname) + && (req.method === "OPTIONS" || req.method === route.method))) { + setCorsHeaders(req, res); + } + if (req.method === "OPTIONS") { res.writeHead(204); res.end(); @@ -204,7 +280,14 @@ const server = http.createServer((req, res) => { allowedMethods.add(route.method); continue; } - if (route.auth === "operation" && !requireOperationAuth(req, res)) return; + if (route.auth !== "none" && !isAuthorizedRequest(req)) { + if (route.auth === "page") { + redirect(res, "/login"); + } else { + respondJson(res, 401, { ok: false, error: "Unauthorized" }); + } + return; + } route.handler(req, res, match); return; } @@ -226,15 +309,25 @@ async function start() { await store.init(); screenshotBytesUsed = await computeScreenshotDirBytes(); + // The configuration summary prints before listen so "listening" is the final + // startup line — the signal (for humans and the test harness) that everything + // above reflects the running server. + console.log(`[PatchLoop receiver] config file: ${config.__loaded ? CONFIG_PATH : "not loaded"}`); + console.log(`[PatchLoop receiver] feedback db: ${DB_PATH}`); + console.log(`[PatchLoop receiver] screenshot dir: ${SCREENSHOT_DIR}`); + console.log(`[PatchLoop receiver] auth: ${RECEIVER_TOKEN ? "enabled (token + inbox login)" : "disabled (no RECEIVER_TOKEN)"}`); + if (ALLOWED_ORIGINS.length > 0) { + console.log(`[PatchLoop receiver] CORS allowlist: ${ALLOWED_ORIGINS.join(", ")}`); + } else { + console.warn("[PatchLoop receiver] CORS: every origin may POST /feedback (*) — set ALLOWED_ORIGINS / allowedOrigins for public deploys"); + } + console.log(`[PatchLoop receiver] Slack webhook: ${SLACK_WEBHOOK_URL ? "enabled" : "disabled"}`); + console.log(`[PatchLoop receiver] Slack image mode: ${SLACK_IMAGE_MODE}`); + console.log(`[PatchLoop receiver] Slack file upload: ${SLACK_BOT_TOKEN && SLACK_UPLOAD_CHANNEL_ID ? "enabled" : "disabled"}`); + console.log(`[PatchLoop receiver] GitHub issues: ${GITHUB_CONFIGURED ? `enabled (${GITHUB_REPO})` : "disabled"}`); + server.listen(PORT, HOST, () => { console.log(`[PatchLoop receiver] listening on http://${HOST}:${PORT}`); - console.log(`[PatchLoop receiver] config file: ${config.__loaded ? CONFIG_PATH : "not loaded"}`); - console.log(`[PatchLoop receiver] feedback db: ${DB_PATH}`); - console.log(`[PatchLoop receiver] screenshot dir: ${SCREENSHOT_DIR}`); - console.log(`[PatchLoop receiver] Slack webhook: ${SLACK_WEBHOOK_URL ? "enabled" : "disabled"}`); - console.log(`[PatchLoop receiver] Slack image mode: ${SLACK_IMAGE_MODE}`); - console.log(`[PatchLoop receiver] Slack file upload: ${SLACK_BOT_TOKEN && SLACK_UPLOAD_CHANNEL_ID ? "enabled" : "disabled"}`); - console.log(`[PatchLoop receiver] GitHub issues: ${GITHUB_CONFIGURED ? `enabled (${GITHUB_REPO})` : "disabled"}`); }); } @@ -243,9 +336,23 @@ start().catch((error) => { process.exit(1); }); -function setCors(res) { - res.setHeader("Access-Control-Allow-Origin", "*"); - res.setHeader("Access-Control-Allow-Methods", "GET, POST, DELETE, OPTIONS"); +// CORS headers for the ingest route only. With no allowlist configured every +// origin may post (historical open default, warned at startup). With an +// allowlist, the request's Origin is echoed back only when it matches; other +// origins get no CORS headers, so the browser blocks the cross-origin POST at +// the preflight — before the payload leaves the page. +function setCorsHeaders(req, res) { + let allowOrigin = "*"; + if (ALLOWED_ORIGINS.length > 0) { + // Origin comparison is exact (scheme://host[:port]); allowlist entries are + // normalized at startup by stripping trailing slashes. + const origin = typeof req.headers.origin === "string" ? req.headers.origin : ""; + res.setHeader("Vary", "Origin"); + if (!ALLOWED_ORIGINS.includes(origin)) return; + allowOrigin = origin; + } + res.setHeader("Access-Control-Allow-Origin", allowOrigin); + res.setHeader("Access-Control-Allow-Methods", "POST, OPTIONS"); res.setHeader("Access-Control-Allow-Headers", "Content-Type"); } @@ -337,7 +444,18 @@ function handlePostFeedback(req, res) { receivedAt: new Date().toISOString(), schemaVersion: DEFAULT_SCHEMA_VERSION, ...payload, - screenshot + screenshot, + // Weak provenance signal for triage (#44 will add an unforgeable ingest + // key): the browser-sent Origin and whether the allowlist would have let + // a browser post it. Origin-less clients (curl, scripts) are not subject + // to CORS, so they record originAllowed: true. Set after the payload + // spread so a crafted payload cannot supply its own value. + received: { + origin: typeof req.headers.origin === "string" ? req.headers.origin : null, + originAllowed: ALLOWED_ORIGINS.length === 0 + || typeof req.headers.origin !== "string" + || ALLOWED_ORIGINS.includes(req.headers.origin) + } }; // Persist before notifying. A failed insert (e.g. a duplicate id -> 409) @@ -653,9 +771,16 @@ function gitHubIssueBody(item) { const screenshotUrl = screenshotUrlFor(item.screenshot); if (screenshotUrl) { lines.push("### Screenshot", ""); - lines.push(`![PatchLoop screenshot](${screenshotUrl})`, ""); - lines.push(`[Open screenshot](${screenshotUrl})`, ""); - lines.push("_The image only renders if the receiver's `publicBaseUrl` is reachable from GitHub._", ""); + if (RECEIVER_TOKEN) { + // With auth enabled GitHub's image proxy (camo) cannot send credentials, + // so an inline embed would always render as a broken image — link only. + lines.push(`[Open screenshot](${screenshotUrl})`, ""); + lines.push("_Sign in to the receiver to view it (auth is enabled)._", ""); + } else { + lines.push(`![PatchLoop screenshot](${screenshotUrl})`, ""); + lines.push(`[Open screenshot](${screenshotUrl})`, ""); + lines.push("_The image only renders if the receiver's `publicBaseUrl` is reachable from GitHub._", ""); + } } else if (item.screenshot && item.screenshot.status) { lines.push(`Screenshot: ${formatScreenshotStatus(item.screenshot)}`, ""); } @@ -674,7 +799,9 @@ function mdTableCell(value) { return String(value ?? "").replaceAll("|", "\\|").replaceAll("\n", " "); } -function readJsonBody(req, res, onJson) { +// Collects the request body (bounded by MAX_BODY_BYTES) and hands the raw text +// to onBody. Shared by the JSON endpoints and the urlencoded login form. +function readRequestBody(req, res, onBody) { let received = 0; const chunks = []; let aborted = false; @@ -695,16 +822,8 @@ function readJsonBody(req, res, onJson) { req.on("end", async () => { if (aborted) return; - let payload; - try { - const raw = Buffer.concat(chunks).toString("utf8"); - payload = raw.trim() ? JSON.parse(raw) : {}; - } catch (_) { - respondJson(res, 400, { ok: false, error: "Invalid JSON" }); - return; - } try { - await onJson(payload); + await onBody(Buffer.concat(chunks).toString("utf8")); } catch (error) { if (!res.headersSent) { respondJson(res, error.statusCode || 500, { ok: false, error: error.message }); @@ -718,6 +837,19 @@ function readJsonBody(req, res, onJson) { }); } +function readJsonBody(req, res, onJson) { + readRequestBody(req, res, async (raw) => { + let payload; + try { + payload = raw.trim() ? JSON.parse(raw) : {}; + } catch (_) { + respondJson(res, 400, { ok: false, error: "Invalid JSON" }); + return; + } + await onJson(payload); + }); +} + // Resolves a bundle (single or batch) into a list of normalized payloads. // Validation runs over every payload up front, so a batch with one bad item // is rejected whole before anything is written. @@ -762,6 +894,8 @@ function normalizeImportedPayload(payload) { delete imported.receivedAt; delete imported.importedAt; delete imported.source; + // Server-owned provenance (set on live ingest); a bundle must not carry it in. + delete imported.received; // Local-only export markers (set by the widget after a batch download) must // never reach the stored record. delete imported.exported; @@ -875,10 +1009,105 @@ function requireNonEmptyString(value, label) { } } +// With auth disabled the login page has no job, so it (and logout) bounce to +// the inbox instead of dead-ending a bookmarked /login. +function handleGetLogin(req, res) { + if (!RECEIVER_TOKEN || isAuthorizedRequest(req)) { + redirect(res, "/"); + return; + } + respondLoginPage(res, 200, false); +} + +function handlePostLogin(req, res) { + if (!RECEIVER_TOKEN) { + req.resume(); + redirect(res, "/"); + return; + } + readRequestBody(req, res, (raw) => { + const token = new URLSearchParams(raw).get("token") || ""; + if (!safeTokenEqual(token, RECEIVER_TOKEN)) { + // Re-rendered with 401 (not a redirect) so the failure is visible to + // both the browser and scripted probes; the per-IP rate limit bounds + // brute-force attempts. + respondLoginPage(res, 401, true); + return; + } + issueSessionCookie(res); + redirect(res, "/"); + }); +} + +function handlePostLogout(req, res) { + req.resume(); + clearSessionCookie(res); + redirect(res, RECEIVER_TOKEN ? "/login" : "/"); +} + +function respondLoginPage(res, status, failed) { + res.writeHead(status, { + "Content-Type": "text/html; charset=utf-8", + "X-Content-Type-Options": "nosniff", + // The page carries no user-controlled content; inline styles keep it + // self-contained (no /static dependency), everything else stays blocked. + "Content-Security-Policy": "default-src 'none'; style-src 'unsafe-inline'; form-action 'self'; base-uri 'none'; frame-ancestors 'none'" + }); + res.end(renderLoginPage(failed)); +} + +function renderLoginPage(failed) { + return ` + + + + + PatchLoop Inbox — Login + + + +

PatchLoop Inbox

+ ${failed ? '

トークンが違います。

' : ""} +
+ + +
+ +`; +} + +// The inbox renders screenshots via their public URL, which can differ from the +// origin the browser used to reach the inbox (e.g. publicBaseUrl behind a +// tunnel), so img-src lists it next to 'self'. +const PUBLIC_ORIGIN = (() => { + try { + return new URL(PUBLIC_BASE_URL).origin; + } catch (_) { + console.warn(`[PatchLoop receiver] publicBaseUrl is not a valid URL, screenshot previews may be blocked by CSP: ${PUBLIC_BASE_URL}`); + return ""; + } +})(); +const INBOX_CSP = `default-src 'none'; script-src 'self'; style-src 'self'; img-src 'self'${PUBLIC_ORIGIN ? ` ${PUBLIC_ORIGIN}` : ""}; connect-src 'self'; form-action 'self'; base-uri 'none'; frame-ancestors 'none'`; + async function handleGetInbox(req, res) { try { const items = await store.list({}); - res.writeHead(200, { "Content-Type": "text/html; charset=utf-8" }); + res.writeHead(200, { + "Content-Type": "text/html; charset=utf-8", + // Everything on the page is same-origin (script, styles, fetches); a + // stored payload that slipped past escaping still could not load or run + // anything external. + "X-Content-Type-Options": "nosniff", + "Content-Security-Policy": INBOX_CSP + }); res.end(renderInbox(items)); } catch (error) { res.writeHead(500, { "Content-Type": "text/plain; charset=utf-8" }); @@ -1582,6 +1811,7 @@ function renderInbox(items) {

PatchLoop Inbox

${items.length} feedback received · raw JSON

+ ${RECEIVER_TOKEN ? '
' : ""} ${renderImportPanel()} ${items.length === 0 ? "" : renderFilterPanel(items)} ${items.length === 0 ? '

まだフィードバックはありません。widget からコメントを送ると、ここに表示されます。

' : cards.join("")} diff --git a/server/receiver.config.example.json b/server/receiver.config.example.json index 7cd9410..d90b8fd 100644 --- a/server/receiver.config.example.json +++ b/server/receiver.config.example.json @@ -18,6 +18,7 @@ "screenshotDiskMaxBytes": 500000000, "trustProxy": false, "receiverToken": "", + "allowedOrigins": [], "slackWebhookUrl": "", "slackImageMode": "auto", "slackBotToken": "", diff --git a/server/static/inbox.css b/server/static/inbox.css index f5e7c40..c59d11f 100644 --- a/server/static/inbox.css +++ b/server/static/inbox.css @@ -49,3 +49,6 @@ code { font: 12px/1.4 ui-monospace, SFMono-Regular, Menlo, Consolas, monospace; details { margin-top: 10px; } summary { cursor: pointer; font-size: 12px; color: #65716d; } pre { background: #14211d; color: #c8d4cf; padding: 12px; border-radius: 6px; overflow-x: auto; font: 12px/1.5 ui-monospace, SFMono-Regular, Menlo, Consolas, monospace; } +.logout-form { margin: -12px 0 18px; } +.logout-form button { min-height: 26px; border: 1px solid #d9e1dd; border-radius: 6px; background: #fff; color: #65716d; padding: 2px 10px; font: inherit; font-size: 11px; font-weight: 800; cursor: pointer; } +.logout-form button:hover { border-color: #8a9590; color: #14211d; } diff --git a/test/receiver.test.js b/test/receiver.test.js index ac10a6d..993823b 100644 --- a/test/receiver.test.js +++ b/test/receiver.test.js @@ -2,6 +2,7 @@ const assert = require("node:assert/strict"); const { spawn } = require("node:child_process"); +const crypto = require("node:crypto"); const fs = require("node:fs/promises"); const http = require("node:http"); const net = require("node:net"); @@ -318,6 +319,30 @@ test("POST /feedback/:id/github-issue serializes concurrent requests (no duplica assert.equal(stored[0].integrations.github.status, "created"); }); +test("GitHub issue body degrades the screenshot embed to a link when auth is enabled", async (t) => { + const github = await startMockGitHub(t, (res) => { + res.writeHead(201, { "Content-Type": "application/json" }); + res.end(JSON.stringify({ number: 8, html_url: "https://github.com/acme/demo/issues/8" })); + }); + const receiver = await startReceiver(t, { + RECEIVER_TOKEN: "s3cret", + GITHUB_TOKEN: "test-token", + GITHUB_REPO: "acme/demo", + GITHUB_API_BASE: github.baseUrl + }); + const payload = feedbackPayload("pl_github_auth"); + await postJson(`${receiver.baseUrl}/feedback`, payload); + + const response = await postJson(`${receiver.baseUrl}/feedback/${payload.id}/github-issue`, {}, { Authorization: "Bearer s3cret" }); + assert.equal(response.status, 201); + + // GitHub's image proxy cannot authenticate, so an inline embed would always + // break; the body keeps only the direct link. + const body = github.requests[0].body.body; + assert.doesNotMatch(body, /!\[PatchLoop screenshot\]/); + assert.match(body, /\[Open screenshot\]\(/); +}); + test("POST /feedback/:id/github-issue persists failures and requires configuration", async (t) => { const unconfigured = await startReceiver(t); const payload = feedbackPayload("pl_github_2"); @@ -984,7 +1009,7 @@ test("POST /feedback keeps omitted screenshot metadata (bytes/maxBytes)", async assert.equal(stored[0].screenshot.maxBytes, 1000); }); -test("operation endpoints require the shared token when RECEIVER_TOKEN is set", async (t) => { +test("protected endpoints require the shared token when RECEIVER_TOKEN is set", async (t) => { const receiver = await startReceiver(t, { RECEIVER_TOKEN: "s3cret" }); const payload = feedbackPayload("pl_auth_1"); @@ -992,9 +1017,20 @@ test("operation endpoints require the shared token when RECEIVER_TOKEN is set", const ingest = await postJson(`${receiver.baseUrl}/feedback`, payload); assert.equal(ingest.status, 201); - // Reads stay open in this slice (inbox/GET auth is a follow-up, #43). + // Reads are protected too: JSON/resources return 401, the inbox page + // redirects an unauthenticated browser to the login form. const read = await fetch(`${receiver.baseUrl}/feedback.json`); - assert.equal(read.status, 200); + assert.equal(read.status, 401); + const inbox = await fetch(receiver.baseUrl, { redirect: "manual" }); + assert.equal(inbox.status, 303); + assert.equal(inbox.headers.get("location"), "/login"); + const stored = await readStoredFeedback(receiver.dbPath); + const screenshotNoToken = await fetch(stored[0].screenshot.url); + assert.equal(screenshotNoToken.status, 401); + + // The widget bundle stays public: demo pages load it cross-origin. + const widget = await fetch(`${receiver.baseUrl}/widget.js`); + assert.notEqual(widget.status, 401); // Every operation endpoint rejects a missing token (permission boundary). const importNoToken = await postJson(`${receiver.baseUrl}/import`, { kind: "patchloop-feedback-bundle", version: 2, feedback: [feedbackPayload("pl_auth_imp")] }); @@ -1006,13 +1042,141 @@ test("operation endpoints require the shared token when RECEIVER_TOKEN is set", const deleteNoToken = await fetch(`${receiver.baseUrl}/feedback/${payload.id}`, { method: "DELETE" }); assert.equal(deleteNoToken.status, 401); - // A wrong token is rejected; the correct bearer token is accepted. + // A wrong token is rejected; the correct bearer token is accepted, for + // reads and operations alike (curl workflows keep working). const badToken = await postJson(`${receiver.baseUrl}/feedback/${payload.id}/status`, { status: "accepted" }, { Authorization: "Bearer nope" }); assert.equal(badToken.status, 401); + const readBearer = await fetch(`${receiver.baseUrl}/feedback.json`, { headers: { Authorization: "Bearer s3cret" } }); + assert.equal(readBearer.status, 200); const ok = await postJson(`${receiver.baseUrl}/feedback/${payload.id}/status`, { status: "accepted" }, { Authorization: "Bearer s3cret" }); assert.equal(ok.status, 200); }); +test("login form issues a session cookie that unlocks the inbox (no raw token in the browser)", async (t) => { + const receiver = await startReceiver(t, { RECEIVER_TOKEN: "s3cret" }); + const payload = feedbackPayload("pl_login_1"); + await postJson(`${receiver.baseUrl}/feedback`, payload); + + // The login page itself is reachable without credentials. + const form = await fetch(`${receiver.baseUrl}/login`); + assert.equal(form.status, 200); + assert.match(await form.text(), /name="token"/); + + // A wrong token re-renders the form as 401 and sets no cookie. + const failed = await fetch(`${receiver.baseUrl}/login`, { + method: "POST", + headers: { "Content-Type": "application/x-www-form-urlencoded" }, + body: "token=nope" + }); + assert.equal(failed.status, 401); + assert.equal(failed.headers.get("set-cookie"), null); + + // The correct token redirects to the inbox with an HttpOnly session cookie + // that is derived (expiry + HMAC) — the raw token never reaches the browser. + const login = await fetch(`${receiver.baseUrl}/login`, { + method: "POST", + headers: { "Content-Type": "application/x-www-form-urlencoded" }, + body: "token=s3cret", + redirect: "manual" + }); + assert.equal(login.status, 303); + assert.equal(login.headers.get("location"), "/"); + const setCookie = login.headers.get("set-cookie"); + assert.match(setCookie, /^patchloop_session=\d+\.[0-9a-f]{64}; Max-Age=604800; Path=\/; HttpOnly; SameSite=Lax$/); + assert.doesNotMatch(setCookie, /s3cret/); + const cookie = setCookie.split(";")[0]; + + // The cookie unlocks the inbox page, reads, and operations — the inbox UI's + // same-origin fetches need no extra wiring. + const inbox = await fetch(receiver.baseUrl, { headers: { Cookie: cookie } }); + assert.equal(inbox.status, 200); + const inboxHtml = await inbox.text(); + assert.match(inboxHtml, /action="\/logout"/); + assert.match(inbox.headers.get("content-security-policy"), /default-src 'none'/); + assert.equal(inbox.headers.get("x-content-type-options"), "nosniff"); + const read = await fetch(`${receiver.baseUrl}/feedback.json`, { headers: { Cookie: cookie } }); + assert.equal(read.status, 200); + const status = await postJson(`${receiver.baseUrl}/feedback/${payload.id}/status`, { status: "accepted" }, { Cookie: cookie }); + assert.equal(status.status, 200); + + // A visit to /login with a valid session bounces back to the inbox. + const revisit = await fetch(`${receiver.baseUrl}/login`, { headers: { Cookie: cookie }, redirect: "manual" }); + assert.equal(revisit.status, 303); + assert.equal(revisit.headers.get("location"), "/"); + + // Tampered and expired cookies are rejected. The expired one carries a valid + // signature over a past expiry, so only the expiry check can catch it. + const tampered = await fetch(`${receiver.baseUrl}/feedback.json`, { headers: { Cookie: `${cookie}ff` } }); + assert.equal(tampered.status, 401); + const pastExpiry = Date.now() - 1000; + const expiredSignature = crypto.createHmac("sha256", "s3cret").update(String(pastExpiry)).digest("hex"); + const expired = await fetch(`${receiver.baseUrl}/feedback.json`, { + headers: { Cookie: `patchloop_session=${pastExpiry}.${expiredSignature}` } + }); + assert.equal(expired.status, 401); + + // Logout clears the cookie and returns to the login form. + const logout = await fetch(`${receiver.baseUrl}/logout`, { method: "POST", headers: { Cookie: cookie }, redirect: "manual" }); + assert.equal(logout.status, 303); + assert.equal(logout.headers.get("location"), "/login"); + assert.match(logout.headers.get("set-cookie"), /^patchloop_session=; Max-Age=0/); +}); + +test("CORS headers are scoped to the ingest route and honor the allowlist", async (t) => { + const receiver = await startReceiver(t, { ALLOWED_ORIGINS: "http://demo.example, http://other.example/" }); + assert.match(receiver.logs, /CORS allowlist: http:\/\/demo\.example, http:\/\/other\.example/); + + // Preflight from an allowlisted origin is granted (origin echoed back). + const allowed = await fetch(`${receiver.baseUrl}/feedback`, { method: "OPTIONS", headers: { Origin: "http://demo.example" } }); + assert.equal(allowed.status, 204); + assert.equal(allowed.headers.get("access-control-allow-origin"), "http://demo.example"); + assert.equal(allowed.headers.get("access-control-allow-methods"), "POST, OPTIONS"); + assert.equal(allowed.headers.get("vary"), "Origin"); + + // An origin outside the list gets no CORS headers, so the browser blocks the + // cross-origin POST at the preflight. + const denied = await fetch(`${receiver.baseUrl}/feedback`, { method: "OPTIONS", headers: { Origin: "http://evil.example" } }); + assert.equal(denied.status, 204); + assert.equal(denied.headers.get("access-control-allow-origin"), null); + + // Non-ingest endpoints emit no CORS headers at all (same-origin surfaces), + // and neither does a 405 on the ingest path (only POST + preflight do). + const inbox = await fetch(receiver.baseUrl, { headers: { Origin: "http://demo.example" } }); + assert.equal(inbox.headers.get("access-control-allow-origin"), null); + const read = await fetch(`${receiver.baseUrl}/feedback.json`, { headers: { Origin: "http://demo.example" } }); + assert.equal(read.headers.get("access-control-allow-origin"), null); + const wrongMethod = await fetch(`${receiver.baseUrl}/feedback`, { headers: { Origin: "http://demo.example" } }); + assert.equal(wrongMethod.status, 405); + assert.equal(wrongMethod.headers.get("access-control-allow-origin"), null); + + // The stored record keeps the provenance signal for triage. + const fromAllowed = await postJson(`${receiver.baseUrl}/feedback`, feedbackPayload("pl_cors_ok"), { Origin: "http://demo.example" }); + assert.equal(fromAllowed.status, 201); + const fromDenied = await postJson(`${receiver.baseUrl}/feedback`, feedbackPayload("pl_cors_ng"), { Origin: "http://evil.example" }); + assert.equal(fromDenied.status, 201); + const fromCurl = await postJson(`${receiver.baseUrl}/feedback`, feedbackPayload("pl_cors_curl")); + assert.equal(fromCurl.status, 201); + + const stored = await readStoredFeedback(receiver.dbPath); + const byId = Object.fromEntries(stored.map((item) => [item.id, item.received])); + assert.deepEqual(byId.pl_cors_ok, { origin: "http://demo.example", originAllowed: true }); + assert.deepEqual(byId.pl_cors_ng, { origin: "http://evil.example", originAllowed: false }); + assert.deepEqual(byId.pl_cors_curl, { origin: null, originAllowed: true }); +}); + +test("without an allowlist, ingest CORS stays open and startup warns", async (t) => { + const receiver = await startReceiver(t); + assert.match(receiver.logs, /CORS: every origin may POST \/feedback/); + + const preflight = await fetch(`${receiver.baseUrl}/feedback`, { method: "OPTIONS", headers: { Origin: "http://anywhere.example" } }); + assert.equal(preflight.status, 204); + assert.equal(preflight.headers.get("access-control-allow-origin"), "*"); + + // Even with CORS open, only the ingest route advertises it. + const inbox = await fetch(receiver.baseUrl); + assert.equal(inbox.headers.get("access-control-allow-origin"), null); +}); + async function startReceiver(t, extraEnv = {}) { const tempDir = await fs.mkdtemp(path.join(os.tmpdir(), "patchloop-receiver-test-")); const port = await getFreePort();