From b32160d5467c9806a1531fb2a839ca236308c885 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hartmut=20N=C3=B6renberg?= Date: Fri, 17 Apr 2026 09:03:24 +0200 Subject: [PATCH] security: default-deny /api middleware allowlist (#44) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously middleware.ts listed /api/ as a public prefix, so any new API route added under /api/** was served without a session check unless the developer remembered to self-authenticate it. The middleware now returns 404 for any /api path not explicitly allowlisted (auth, trpc, sse, cron, reports, health, ready, perf) — adding a new API route is a deliberate allowlist edit. verifyCronSecret was already fail-closed when CRON_SECRET is unset; added unit tests. Co-Authored-By: Claude Opus 4.7 --- apps/web/src/lib/cron-auth.test.ts | 55 ++++++++++++++++++++++++++++++ apps/web/src/middleware.test.ts | 51 +++++++++++++++++++++++++-- apps/web/src/middleware.ts | 52 +++++++++++++++++++++------- 3 files changed, 142 insertions(+), 16 deletions(-) create mode 100644 apps/web/src/lib/cron-auth.test.ts diff --git a/apps/web/src/lib/cron-auth.test.ts b/apps/web/src/lib/cron-auth.test.ts new file mode 100644 index 0000000..359c691 --- /dev/null +++ b/apps/web/src/lib/cron-auth.test.ts @@ -0,0 +1,55 @@ +import { afterEach, beforeEach, describe, expect, it } from "vitest"; +import { verifyCronSecret } from "./cron-auth.js"; + +describe("verifyCronSecret — fail-closed when CRON_SECRET missing", () => { + const original = process.env["CRON_SECRET"]; + + afterEach(() => { + if (original === undefined) delete process.env["CRON_SECRET"]; + else process.env["CRON_SECRET"] = original; + }); + + it("returns 401 when CRON_SECRET is unset", async () => { + delete process.env["CRON_SECRET"]; + const req = new Request("http://localhost/api/cron/x", { + headers: { Authorization: "Bearer whatever" }, + }); + const res = verifyCronSecret(req); + expect(res).not.toBeNull(); + expect(res?.status).toBe(401); + }); + + it("returns 401 when CRON_SECRET is empty string", async () => { + process.env["CRON_SECRET"] = ""; + const req = new Request("http://localhost/api/cron/x", { + headers: { Authorization: "Bearer whatever" }, + }); + const res = verifyCronSecret(req); + expect(res).not.toBeNull(); + expect(res?.status).toBe(401); + }); + + it("returns 401 when Authorization header is missing", () => { + process.env["CRON_SECRET"] = "real-secret"; + const req = new Request("http://localhost/api/cron/x"); + const res = verifyCronSecret(req); + expect(res?.status).toBe(401); + }); + + it("returns 401 when Authorization header mismatches", () => { + process.env["CRON_SECRET"] = "real-secret"; + const req = new Request("http://localhost/api/cron/x", { + headers: { Authorization: "Bearer wrong-secret" }, + }); + const res = verifyCronSecret(req); + expect(res?.status).toBe(401); + }); + + it("returns null (allow) when Authorization header matches", () => { + process.env["CRON_SECRET"] = "real-secret"; + const req = new Request("http://localhost/api/cron/x", { + headers: { Authorization: "Bearer real-secret" }, + }); + expect(verifyCronSecret(req)).toBeNull(); + }); +}); diff --git a/apps/web/src/middleware.test.ts b/apps/web/src/middleware.test.ts index e6cfafc..df52416 100644 --- a/apps/web/src/middleware.test.ts +++ b/apps/web/src/middleware.test.ts @@ -4,9 +4,8 @@ import { NextRequest } from "next/server"; // Simulate an authenticated session so the middleware does not redirect // and CSP headers are set on every response. vi.mock("./server/auth-edge.js", () => ({ - auth: (handler: (req: NextRequest & { auth: object | null }) => unknown) => - (req: NextRequest) => - handler(Object.assign(req, { auth: { user: { id: "test-user", email: "test@test.com" } } })), + auth: (handler: (req: NextRequest & { auth: object | null }) => unknown) => (req: NextRequest) => + handler(Object.assign(req, { auth: { user: { id: "test-user", email: "test@test.com" } } })), })); async function importMiddleware(nodeEnv: string) { @@ -82,3 +81,49 @@ describe("middleware — Content-Security-Policy", () => { } }); }); + +describe("middleware — API allowlist (default-deny)", () => { + afterEach(() => { + vi.unstubAllEnvs(); + vi.resetModules(); + }); + + it("allows allowlisted API routes through", async () => { + const middleware = await importMiddleware("production"); + for (const url of [ + "http://localhost:3100/api/trpc/project.list", + "http://localhost:3100/api/auth/signin", + "http://localhost:3100/api/sse/timeline", + "http://localhost:3100/api/cron/health-check", + "http://localhost:3100/api/reports/allocations", + "http://localhost:3100/api/health", + "http://localhost:3100/api/ready", + "http://localhost:3100/api/perf", + ]) { + const res = await middleware(new NextRequest(url)); + expect(res.status).not.toBe(404); + } + }); + + it("returns 404 for non-allowlisted /api/* routes", async () => { + const middleware = await importMiddleware("production"); + for (const url of [ + "http://localhost:3100/api/debug", + "http://localhost:3100/api/internal/secret", + "http://localhost:3100/api/admin/users", + ]) { + const res = await middleware(new NextRequest(url)); + expect(res.status).toBe(404); + } + }); +}); + +describe("isApiAllowlisted helper", () => { + it("exported via module for testing", async () => { + const { isApiAllowlisted } = await import("./middleware.js"); + expect(isApiAllowlisted("/api/trpc/foo")).toBe(true); + expect(isApiAllowlisted("/api/debug")).toBe(false); + expect(isApiAllowlisted("/api/healthz")).toBe(false); + expect(isApiAllowlisted("/api/health")).toBe(true); + }); +}); diff --git a/apps/web/src/middleware.ts b/apps/web/src/middleware.ts index 14c0fba..9c5dd97 100644 --- a/apps/web/src/middleware.ts +++ b/apps/web/src/middleware.ts @@ -1,22 +1,39 @@ import { NextResponse } from "next/server"; import { auth } from "./server/auth-edge.js"; -// Paths that are accessible without a session. -// Everything else requires a valid JWT session. -const PUBLIC_PREFIXES = [ - "/auth/", // signin, forgot-password, reset-password - "/api/", // tRPC, health, auth endpoints — these manage their own auth - "/invite/", // public invite acceptance flow +// UI routes that are accessible without a session (login page, reset flow, +// public invite acceptance). All other UI routes redirect unauthenticated +// visitors to /auth/signin. +const PUBLIC_UI_PREFIXES = ["/auth/", "/invite/"]; + +// API allowlist — only routes listed here are served. Everything else under +// `/api/*` returns 404. Each allowlisted route MUST perform its own +// authentication (session check via auth(), CRON_SECRET bearer header, etc.) +// because the edge middleware cannot do Node-only work like Prisma queries. +// Prefix entries must end with `/`; exact entries match only the literal +// pathname. A new /api route therefore requires a deliberate allowlist edit, +// preventing accidental default-public exposure (security ticket #44). +export const SELF_AUTH_API_PREFIXES = [ + "/api/auth/", + "/api/trpc/", + "/api/sse/", + "/api/cron/", + "/api/reports/", ]; -function isPublicPath(pathname: string): boolean { - return PUBLIC_PREFIXES.some((prefix) => pathname.startsWith(prefix)); +export const SELF_AUTH_API_EXACT = ["/api/health", "/api/ready", "/api/perf"]; + +export function isApiAllowlisted(pathname: string): boolean { + if (SELF_AUTH_API_EXACT.includes(pathname)) return true; + return SELF_AUTH_API_PREFIXES.some((p) => pathname.startsWith(p)); +} + +function isPublicUiPath(pathname: string): boolean { + return PUBLIC_UI_PREFIXES.some((prefix) => pathname.startsWith(prefix)); } function buildCsp(nonce: string, isProd: boolean): string { - const scriptSrc = isProd - ? `'self' 'nonce-${nonce}'` - : `'self' 'unsafe-eval' 'unsafe-inline'`; + const scriptSrc = isProd ? `'self' 'nonce-${nonce}'` : `'self' 'unsafe-eval' 'unsafe-inline'`; const imgSrc = isProd ? "'self' data: blob:" : "'self' data: blob: https:"; @@ -36,8 +53,17 @@ function buildCsp(nonce: string, isProd: boolean): string { export default auth(function middleware(request) { const { pathname } = request.nextUrl; - // Redirect unauthenticated requests for protected routes to signin - if (!isPublicPath(pathname) && !request.auth) { + // /api/* — default-deny. Only allowlisted routes pass; everything else 404s. + // Allowlisted routes are responsible for their own auth check (they are + // reached in the route handler, not here, because edge middleware cannot do + // Prisma queries). + if (pathname.startsWith("/api/")) { + if (!isApiAllowlisted(pathname)) { + return NextResponse.json({ error: "Not Found" }, { status: 404 }); + } + // fall through — continue to add CSP headers + } else if (!isPublicUiPath(pathname) && !request.auth) { + // UI route requires a session. Redirect to signin. const signInUrl = new URL("/auth/signin", request.url); signInUrl.searchParams.set("callbackUrl", request.url); return NextResponse.redirect(signInUrl);