#189: make the CSRF origin check fail closed

A cookie-carrying mutation without Origin and Referer (or with an
unparsable one) is now rejected with 403 csrf_origin_mismatch instead
of passing unchecked. The exception for non-browser clients stays
structural: PAT/bearer requests carry no session cookie and never reach
the check, and a request that does carry the cookie is always checked.

The test harness injects the matching Origin (supertest simulates a
browser page of this instance) with an explicit suppression header for
the negative cases; the Playwright fixture contexts send the header on
their manual seeding calls; release-qa.sh pins APP_BASE_URL and sends
the matching Origin. Dedicated spec covers: missing headers 403,
mismatch 403, unparsable 403, match passes, GETs untouched, PAT
mutation without headers passes, cookie+bearer still checked.

Refs #189

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0168Ph5uBmHm8X28CSVpbpnJ

apps/api/src/auth/auth.guard.ts

@@ -105,13 +105,27 @@ export class AuthGuard implements CanActivate {
     return true;
   }
 
+  /**
+   * Fail closed (#189): a cookie-carrying mutation must prove its origin —
+   * browsers always send `Origin` on cross- and same-origin mutations, so a
+   * missing header means "not a browser page of ours" and is rejected like a
+   * mismatch. Non-browser clients (curl, scripts) either send a matching
+   * `Origin` explicitly or authenticate with a PAT/bearer token and no
+   * cookie, which never reaches this check — the exception for them is
+   * structural (bound to the cookie), never a header loophole.
+   */
   private assertSameOrigin(request: Request): void {
     const origin = request.headers.origin ?? request.headers.referer;
-    // Non-browser clients (curl, supertest) send neither header; SameSite
-    // cookies already stop cross-site browser requests without Origin.
-    if (!origin) return;
+    if (!origin) throw new ForbiddenException({ code: 'csrf_origin_mismatch' });
     const expected = new URL(this.config.env.APP_BASE_URL).origin;
-    if (new URL(origin).origin !== expected) {
+    let actual: string;
+    try {
+      actual = new URL(origin).origin;
+    } catch {
+      // An unparsable Origin/Referer is a broken or hostile client, not ours.
+      throw new ForbiddenException({ code: 'csrf_origin_mismatch' });
+    }
+    if (actual !== expected) {
       throw new ForbiddenException({ code: 'csrf_origin_mismatch' });
     }
   }

apps/api/src/auth/csrf.e2e.db.test.ts

@@ -0,0 +1,178 @@
+import { INestApplication } from '@nestjs/common';
+import { PrismaClient } from '@prisma/client';
+import request from 'supertest';
+import { afterAll, beforeAll, describe, expect, it } from 'vitest';
+
+import { InstanceSettingsService } from '../settings/instance-settings.service';
+import { SUPPRESS_ORIGIN_HEADER, createTestApp, sessionCookieOf } from '../testing/test-app';
+import { createTestPrisma, hasTestDb, uniqueSuffix } from '../testing/test-db';
+import { UsersService } from '../users/users.service';
+
+/**
+ * CSRF origin check, fail closed (issue #189): a cookie-carrying mutation
+ * without `Origin` and `Referer` is rejected exactly like a mismatch, and
+ * the exception for non-browser clients is structural — PAT/bearer requests
+ * carry no cookie and never reach the check. Cookie-authenticated requests
+ * never benefit from any header-based bypass.
+ */
+describe.skipIf(!hasTestDb)('csrf origin check (e2e, issue #189)', () => {
+  let app: INestApplication;
+  let prisma: PrismaClient;
+  const suffix = uniqueSuffix();
+  const password = 'csrf fail closed pass 1';
+  const ids: Record<string, string> = {};
+  const cookies: Record<string, string> = {};
+  let pondId: string;
+  let pondSlug: string;
+  let patToken: string;
+
+  const api = () => request(app.getHttpServer());
+
+  async function makeUser(handle: string): Promise<void> {
+    const users = app.get(UsersService);
+    const username = `csrf-${handle}-${suffix}`;
+    const user = await users.createUser({
+      username,
+      email: `${username}@example.org`,
+      displayName: `Csrf ${handle}`,
+      password,
+      locale: 'en',
+    });
+    await users.markEmailVerified(user.id);
+    ids[handle] = user.id;
+    cookies[handle] = sessionCookieOf(
+      await api()
+        .post('/api/v1/auth/login')
+        .send({ usernameOrEmail: username, password })
+        .expect(200),
+    );
+  }
+
+  beforeAll(async () => {
+    prisma = createTestPrisma();
+    await prisma.rateLimit.deleteMany({});
+    app = await createTestApp();
+    for (const handle of ['owner', 'siteadmin']) {
+      await makeUser(handle);
+    }
+    await prisma.user.update({ where: { id: ids.siteadmin! }, data: { isSiteAdmin: true } });
+    // Per-user quota override, never the instance default (shared database).
+    await api()
+      .put(`/api/v1/admin/quotas/user/${ids.owner!}/additional_ponds`)
+      .set('Cookie', cookies.siteadmin!)
+      .send({ value: 5 })
+      .expect(200);
+
+    // A pond opted into the public API, and a write-scope PAT for it.
+    const pond = await api()
+      .post('/api/v1/ponds')
+      .set('Cookie', cookies.owner!)
+      .send({ name: `CSRF Pond ${suffix}` })
+      .expect(201);
+    pondId = pond.body.id;
+    pondSlug = pond.body.slug;
+    await app.get(InstanceSettingsService).set('api.enabled', true, ids.siteadmin!);
+    await api()
+      .patch(`/api/v1/ponds/${pondId}`)
+      .set('Cookie', cookies.owner!)
+      .send({ apiEnabled: true })
+      .expect(200);
+    const pat = await api()
+      .post('/api/v1/users/me/api-tokens')
+      .set('Cookie', cookies.owner!)
+      .send({ name: 'csrf-write', scope: 'write' })
+      .expect(201);
+    patToken = pat.body.token;
+  });
+
+  afterAll(async () => {
+    const all = Object.values(ids);
+    await prisma.instanceSetting.deleteMany({ where: { key: 'api.enabled' } });
+    await prisma.quotaOverride.deleteMany({ where: { subjectId: { in: all } } });
+    await prisma.auditEntry.deleteMany({ where: { actorId: { in: all } } });
+    await prisma.apiToken.deleteMany({ where: { userId: { in: all } } });
+    const ponds = await prisma.pond.findMany({
+      where: { ownerId: { in: all } },
+      select: { id: true },
+    });
+    const pondIds = ponds.map((p) => p.id);
+    await prisma.pageVersion.deleteMany({ where: { page: { pondId: { in: pondIds } } } });
+    await prisma.page.deleteMany({ where: { pondId: { in: pondIds } } });
+    await prisma.roleGrant.deleteMany({ where: { pondId: { in: pondIds } } });
+    await prisma.pondUsage.deleteMany({ where: { pondId: { in: pondIds } } });
+    await prisma.pond.deleteMany({ where: { id: { in: pondIds } } });
+    await prisma.session.deleteMany({ where: { userId: { in: all } } });
+    await prisma.userIdentity.deleteMany({ where: { userId: { in: all } } });
+    await prisma.rateLimit.deleteMany({});
+    await prisma.user.deleteMany({ where: { id: { in: all } } });
+    await prisma.$disconnect();
+    await app.close();
+  });
+
+  it('rejects a cookie mutation that sends neither Origin nor Referer', async () => {
+    const res = await api()
+      .patch(`/api/v1/ponds/${pondId}`)
+      .set('Cookie', cookies.owner!)
+      .set(SUPPRESS_ORIGIN_HEADER, '1')
+      .send({ name: `CSRF Pond ${suffix}` })
+      .expect(403);
+    expect(res.body.code).toBe('csrf_origin_mismatch');
+  });
+
+  it('rejects a cookie mutation from a mismatching origin (kept behaviour)', async () => {
+    const res = await api()
+      .patch(`/api/v1/ponds/${pondId}`)
+      .set('Cookie', cookies.owner!)
+      .set('Origin', 'https://evil.example')
+      .send({ name: `CSRF Pond ${suffix}` })
+      .expect(403);
+    expect(res.body.code).toBe('csrf_origin_mismatch');
+  });
+
+  it('rejects a cookie mutation with an unparsable Origin instead of erroring', async () => {
+    const res = await api()
+      .patch(`/api/v1/ponds/${pondId}`)
+      .set('Cookie', cookies.owner!)
+      .set('Origin', 'not a url')
+      .send({ name: `CSRF Pond ${suffix}` })
+      .expect(403);
+    expect(res.body.code).toBe('csrf_origin_mismatch');
+  });
+
+  it('accepts a cookie mutation from the matching origin', async () => {
+    await api()
+      .patch(`/api/v1/ponds/${pondId}`)
+      .set('Cookie', cookies.owner!)
+      .send({ name: `CSRF Pond ${suffix}` })
+      .expect(200);
+  });
+
+  it('leaves cookie reads untouched — the check binds to mutations', async () => {
+    await api()
+      .get('/api/v1/auth/me')
+      .set('Cookie', cookies.owner!)
+      .set(SUPPRESS_ORIGIN_HEADER, '1')
+      .expect(200);
+  });
+
+  it('lets a PAT mutation through without either header — no cookie, no check', async () => {
+    await api()
+      .post(`/api/public/v1/ponds/${pondSlug}/pages`)
+      .set('Authorization', `Bearer ${patToken}`)
+      .set(SUPPRESS_ORIGIN_HEADER, '1')
+      .send({ title: `CSRF PAT page ${suffix}` })
+      .expect(201);
+  });
+
+  it('enforces the check when a request carries both cookie and bearer token', async () => {
+    // Cookie-authenticated requests never benefit from the bearer exception.
+    const res = await api()
+      .patch(`/api/v1/ponds/${pondId}`)
+      .set('Cookie', cookies.owner!)
+      .set('Authorization', `Bearer ${patToken}`)
+      .set(SUPPRESS_ORIGIN_HEADER, '1')
+      .send({ name: `CSRF Pond ${suffix}` })
+      .expect(403);
+    expect(res.body.code).toBe('csrf_origin_mismatch');
+  });
+});

apps/api/src/testing/test-app.ts

@@ -6,8 +6,17 @@ import { INestApplication } from '@nestjs/common';
 import { Test, TestingModuleBuilder } from '@nestjs/testing';
 import type { NestExpressApplication } from '@nestjs/platform-express';
 import cookieParser from 'cookie-parser';
+import type { NextFunction, Request, Response } from 'express';
 
 import { AppModule } from '../app.module';
+import { AppConfig } from '../config/app-config.service';
+
+/**
+ * Suppresses the test harness's automatic `Origin` header (below) for a
+ * single request — the way the CSRF spec simulates a client that sends
+ * neither `Origin` nor `Referer` (issue #189).
+ */
+export const SUPPRESS_ORIGIN_HEADER = 'x-test-suppress-origin';
 
 /**
  * Boots the full application for e2e tests, mirroring main.ts middleware.
@@ -32,6 +41,21 @@ export async function createTestApp(
   const moduleRef = await (customize ? customize(base) : base).compile();
   const app = moduleRef.createNestApplication<NestExpressApplication>();
   app.use(cookieParser());
+  // Browsers always send `Origin` on mutations, and since #189 the CSRF
+  // check fails closed without it. supertest simulates a browser page of
+  // this instance, so the harness injects the matching header — absence is
+  // simulated explicitly via SUPPRESS_ORIGIN_HEADER, never by accident.
+  const expectedOrigin = new URL(moduleRef.get(AppConfig).env.APP_BASE_URL).origin;
+  app.use((req: Request, _res: Response, next: NextFunction) => {
+    if (SUPPRESS_ORIGIN_HEADER in req.headers) {
+      delete req.headers[SUPPRESS_ORIGIN_HEADER];
+      delete req.headers.origin;
+      delete req.headers.referer;
+    } else {
+      req.headers.origin ??= expectedOrigin;
+    }
+    next();
+  });
   // Mirrors main.ts: base64 Yjs page state needs more than Express's 100kb default.
   app.useBodyParser('json', { limit: '8mb' });
   // Mirrors main.ts: the public API (issue #104) declares its full path.

apps/web/e2e/helpers.ts

@@ -22,7 +22,14 @@ export async function contextForUser(
   }
   const storageState = await api.storageState();
   await api.dispose();
-  return browser.newContext({ baseURL, storageState });
+  // The CSRF check fails closed (#189): cookie mutations must carry a
+  // matching Origin. The browser sends it on its own fetches; this default
+  // covers the specs' manual `context.request.*` seeding calls too.
+  return browser.newContext({
+    baseURL,
+    storageState,
+    extraHTTPHeaders: { origin: new URL(baseURL).origin },
+  });
 }
 
 /** Latest mail for an address from the Mailpit REST api. */

deploy/release-qa.sh

@@ -33,6 +33,9 @@ NET="$P-net"
 PGPASS="rqa-$(date +%s)"
 ADMIN_PASS="release qa admin pass 1"
 SESSION=""
+# Cookie mutations must prove their origin since #189 (CSRF fail-closed).
+# The api pins APP_BASE_URL to this value and every api_curl sends it.
+QA_ORIGIN="http://release-qa.local"
 
 log() { echo "release-qa: $*"; }
 fail() { echo "release-qa: FAILED — $*" >&2; exit 1; }
@@ -50,7 +53,7 @@ trap cleanup EXIT
 api_curl() { # api_curl <method> <path> [json-body]
   METHOD=$1; APIPATH=$2; BODY=${3:-}
   docker run --rm --network "$NET" curlimages/curl:8.10.1 \
-    -s ${SESSION:+-H "Cookie: $SESSION"} -X "$METHOD" \
+    -s ${SESSION:+-H "Cookie: $SESSION"} -H "Origin: $QA_ORIGIN" -X "$METHOD" \
     ${BODY:+-H 'Content-Type: application/json' --data "$BODY"} \
     "http://$P-api:3000/api/v1$APIPATH"
 }
@@ -58,7 +61,7 @@ api_curl() { # api_curl <method> <path> [json-body]
 start_api() { # start_api <tag>
   docker rm -f "$P-api" >/dev/null 2>&1 || true
   docker run -d --name "$P-api" --network "$NET" \
-    -e DATABASE_URL="$1" \
+    -e DATABASE_URL="$1" -e APP_BASE_URL="$QA_ORIGIN" \
     -e SETUP_ADMIN_USERNAME=qa-admin -e SETUP_ADMIN_EMAIL=qa-admin@example.org \
     -e SETUP_ADMIN_PASSWORD="$ADMIN_PASS" \
     -v "$P-uploads":/data/uploads -v "$P-plugins":/data/plugins -v "$P-backups":/data/backups:ro \

docs/architecture/security.md

@@ -15,6 +15,13 @@ or sloppy plugin authors, compromised dependencies.
   Secure, SameSite=Lax cookies; CSRF protected by SameSite + origin checks
   on mutating requests (double-submit token for the file-download edge
   cases).
+- The origin check **fails closed** (issue #189): a cookie-carrying
+  mutation without `Origin` and `Referer` (or with an unparsable one) is
+  rejected with `403 csrf_origin_mismatch`. Non-browser clients
+  authenticate with a PAT/bearer token and no cookie, which never reaches
+  the check — the exception is structural, not a header loophole; a
+  request that does carry the session cookie is always checked. Scripted
+  cookie clients must send `Origin: <APP_BASE_URL>`.
 - E-mail verification (double opt-in) before an account can create content;
   password reset via single-use hashed tokens; both rate-limited.
 - Rate limiting (DB-backed) on login, signup, reset, and API; lockout

docs/vs-nfd/20-massnahmenplan.md

@@ -91,7 +91,7 @@ chain`_
       (`packages/shared/src/token-crypto.ts`). Einzeln wären es 5–6 AT._
       Achtung: Unsubscribe-Tokens leben lang in versandten Mails →
       Dual-Verify-Fenster einplanen.
-- [ ] **CSRF fail-closed** — fehlendes Origin _und_ Referer wird derzeit
+- [x] **CSRF fail-closed** — fehlendes Origin _und_ Referer wird derzeit
       durchgelassen · 1 AT · #189
 - [ ] **Session-Timeout konfigurierbar**, Default deutlich unter 30 Tagen,
       separates Idle-Timeout · 1–2 AT · #190