From 3c62b7b77350023789ffeebf59bdd05af5af38f6 Mon Sep 17 00:00:00 2001 From: Claude Fable 5 Date: Thu, 30 Jul 2026 17:10:35 +0200 Subject: [PATCH] #197: security response headers and an explicitly restrictive CORS policy MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Hand-rolled middleware instead of helmet: the header set is small enough to own, every value is a deliberate decision, and the api gains no transitive dependency. HSTS (no includeSubDomains — the api cannot speak for sibling subdomains), nosniff, Referrer-Policy no-referrer, X-Frame-Options SAMEORIGIN (not DENY: the plugin sandbox frame embeds same-origin and its CSP has no frame-ancestors, so this header governs), and a minimal deny-all Permissions-Policy. CORS grants no foreign origin anything; only the APP_BASE_URL origin is ever echoed (where browsers do not consult CORS anyway), with Vary: Origin on every response. No preflight handling — same-origin requests never preflight, and cross-origin API access is cookie-less by design (PAT/Bearer). Wired via the AppModule MiddlewareConsumer so createTestApp boots the identical middleware. Fences: security-headers.e2e.test.ts (header set, foreign origin gets no ACAO) and a frame assertion in plugins.e2e.db.test.ts (framing stays possible). Rationale table in security.md. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_0168Ph5uBmHm8X28CSVpbpnJ --- apps/api/src/app.module.ts | 11 ++- .../src/common/security-headers.e2e.test.ts | 74 +++++++++++++++++++ .../src/common/security-headers.middleware.ts | 54 ++++++++++++++ apps/api/src/plugins/plugins.e2e.db.test.ts | 7 ++ docs/architecture/security.md | 33 +++++++++ docs/vs-nfd/20-massnahmenplan.md | 2 +- 6 files changed, 178 insertions(+), 3 deletions(-) create mode 100644 apps/api/src/common/security-headers.e2e.test.ts create mode 100644 apps/api/src/common/security-headers.middleware.ts diff --git a/apps/api/src/app.module.ts b/apps/api/src/app.module.ts index d9c8486..d040d53 100644 --- a/apps/api/src/app.module.ts +++ b/apps/api/src/app.module.ts @@ -1,4 +1,4 @@ -import { Module } from '@nestjs/common'; +import { MiddlewareConsumer, Module, NestModule } from '@nestjs/common'; import { APP_FILTER } from '@nestjs/core'; import { LoggerModule } from 'nestjs-pino'; @@ -8,6 +8,7 @@ import { AuthModule } from './auth/auth.module'; import { BackupModule } from './backup/backup.module'; import { ApiExceptionFilter } from './common/api-exception.filter'; import { maskTokenParam } from './common/mask-token-param'; +import { SecurityHeadersMiddleware } from './common/security-headers.middleware'; import { CommentsModule } from './comments/comments.module'; import { CompactionModule } from './compaction/compaction.module'; import { AppConfig } from './config/app-config.service'; @@ -104,4 +105,10 @@ import { VersionsModule } from './versions/versions.module'; ], providers: [{ provide: APP_FILTER, useClass: ApiExceptionFilter }], }) -export class AppModule {} +export class AppModule implements NestModule { + configure(consumer: MiddlewareConsumer): void { + // Module-level (not main.ts) so createTestApp boots the identical + // security-header/CORS middleware — see security-headers.middleware.ts. + consumer.apply(SecurityHeadersMiddleware).forRoutes('{*path}'); + } +} diff --git a/apps/api/src/common/security-headers.e2e.test.ts b/apps/api/src/common/security-headers.e2e.test.ts new file mode 100644 index 0000000..e5cecf9 --- /dev/null +++ b/apps/api/src/common/security-headers.e2e.test.ts @@ -0,0 +1,74 @@ +import { INestApplication } from '@nestjs/common'; +import { Test } from '@nestjs/testing'; +import request from 'supertest'; +import { afterAll, beforeAll, describe, expect, it } from 'vitest'; + +import { AppModule } from '../app.module'; + +// Boots the AppModule without a database (like health.e2e.test.ts): the +// middleware under test runs before any route logic, so the always-on +// healthz endpoint is a representative response (issue #197). +describe('security response headers & CORS (e2e, issue #197)', () => { + let app: INestApplication; + const appOrigin = 'http://localhost:5173'; // APP_BASE_URL default origin + + beforeAll(async () => { + process.env.NODE_ENV = 'test'; + process.env.DATABASE_URL ??= 'postgresql://nobody:nothing@127.0.0.1:59999/absent'; + const moduleRef = await Test.createTestingModule({ imports: [AppModule] }).compile(); + app = moduleRef.createNestApplication(); + app.setGlobalPrefix('api/v1'); + await app.init(); + }); + + afterAll(async () => { + await app.close(); + }); + + it('stamps the full header set on a representative response', async () => { + const res = await request(app.getHttpServer()).get('/api/v1/healthz').expect(200); + expect(res.headers['strict-transport-security']).toBe('max-age=31536000'); + expect(res.headers['x-content-type-options']).toBe('nosniff'); + expect(res.headers['referrer-policy']).toBe('no-referrer'); + // SAMEORIGIN, not DENY — the plugin sandbox frame is embedded + // same-origin (plugins.e2e.db.test.ts asserts the frame side). + expect(res.headers['x-frame-options']).toBe('SAMEORIGIN'); + expect(res.headers['permissions-policy']).toBe( + 'camera=(), microphone=(), geolocation=(), payment=(), usb=()', + ); + }); + + it('stamps the headers on error responses too (unknown route)', async () => { + const res = await request(app.getHttpServer()).get('/api/v1/does-not-exist').expect(404); + expect(res.headers['x-content-type-options']).toBe('nosniff'); + expect(res.headers['x-frame-options']).toBe('SAMEORIGIN'); + }); + + it('grants a foreign origin nothing (no ACAO), while varying on Origin', async () => { + const res = await request(app.getHttpServer()) + .get('/api/v1/healthz') + .set('Origin', 'https://attacker.example') + .expect(200); + expect(res.headers['access-control-allow-origin']).toBeUndefined(); + expect(res.headers['access-control-allow-credentials']).toBeUndefined(); + expect(res.headers.vary).toContain('Origin'); + }); + + it("echoes only the app's own origin, with the credentials rule stated", async () => { + const res = await request(app.getHttpServer()) + .get('/api/v1/healthz') + .set('Origin', appOrigin) + .expect(200); + expect(res.headers['access-control-allow-origin']).toBe(appOrigin); + expect(res.headers['access-control-allow-credentials']).toBe('true'); + }); + + it('leaves a foreign preflight ungranted (no CORS response headers)', async () => { + const res = await request(app.getHttpServer()) + .options('/api/v1/healthz') + .set('Origin', 'https://attacker.example') + .set('Access-Control-Request-Method', 'POST'); + expect(res.headers['access-control-allow-origin']).toBeUndefined(); + expect(res.headers['access-control-allow-methods']).toBeUndefined(); + }); +}); diff --git a/apps/api/src/common/security-headers.middleware.ts b/apps/api/src/common/security-headers.middleware.ts new file mode 100644 index 0000000..80bc3f6 --- /dev/null +++ b/apps/api/src/common/security-headers.middleware.ts @@ -0,0 +1,54 @@ +import { Injectable, NestMiddleware } from '@nestjs/common'; +import type { NextFunction, Request, Response } from 'express'; + +import { AppConfig } from '../config/app-config.service'; + +/** + * Security response headers and the CORS stance for every api response + * (issue #197). Hand-rolled instead of `helmet`: the header set is small + * enough to own, every value below is a deliberate decision, and the api + * gains no transitive dependency. Wired via the AppModule's + * MiddlewareConsumer so the test harness (createTestApp) exercises the + * exact production middleware — rationale per header in + * docs/architecture/security.md §Security response headers & CORS. + */ +@Injectable() +export class SecurityHeadersMiddleware implements NestMiddleware { + /** The one origin the SPA is served from; the only origin CORS ever echoes. */ + private readonly allowedOrigin: string; + + constructor(config: AppConfig) { + this.allowedOrigin = new URL(config.env.APP_BASE_URL).origin; + } + + use(req: Request, res: Response, next: NextFunction): void { + // No includeSubDomains: the api cannot speak for sibling subdomains it + // does not control (e.g. a support desk on the same apex). Browsers + // ignore HSTS over plain http, so sending it unconditionally is safe. + res.setHeader('Strict-Transport-Security', 'max-age=31536000'); + res.setHeader('X-Content-Type-Options', 'nosniff'); + // Page paths are permission-scoped knowledge — leak them to no one. + res.setHeader('Referrer-Policy', 'no-referrer'); + // SAMEORIGIN, deliberately not DENY: the plugin sandbox (ADR 0008) + // embeds /api/v1/plugins///frame same-origin, and the + // frame's own CSP carries no frame-ancestors — this header governs. + res.setHeader('X-Frame-Options', 'SAMEORIGIN'); + // Deny the powerful features outright; nothing in the app uses them. + res.setHeader( + 'Permissions-Policy', + 'camera=(), microphone=(), geolocation=(), payment=(), usb=()', + ); + + // CORS: no foreign origin is granted anything — only the app's own + // origin is ever echoed (where browsers do not consult CORS anyway, as + // same-origin; the echo states the decision rather than enabling a + // caller). Same-origin requests never preflight, so no OPTIONS + // handling is needed. Vary on every response keeps caches honest. + res.vary('Origin'); + if (req.headers.origin === this.allowedOrigin) { + res.setHeader('Access-Control-Allow-Origin', this.allowedOrigin); + res.setHeader('Access-Control-Allow-Credentials', 'true'); + } + next(); + } +} diff --git a/apps/api/src/plugins/plugins.e2e.db.test.ts b/apps/api/src/plugins/plugins.e2e.db.test.ts index 4743eda..316ad44 100644 --- a/apps/api/src/plugins/plugins.e2e.db.test.ts +++ b/apps/api/src/plugins/plugins.e2e.db.test.ts @@ -145,6 +145,13 @@ describe.skipIf(!hasTestDb)('plugins install (e2e, issue #71)', () => { expect(csp).toMatch(/connect-src https?:\/\/[^ ]+\/api\/v1\/plugins\/framer\/1\.0\.0\//); expect(csp).toMatch(/frame-src https?:\/\/[^ ]+\/api\/v1\/plugins\/framer\/1\.0\.0\//); + // Framing stays possible under the global security headers (issue #197): + // the host app embeds this document same-origin, X-Frame-Options is + // SAMEORIGIN (never DENY), and the frame's CSP carries no + // frame-ancestors that could override it. + expect(frame.headers['x-frame-options']).toBe('SAMEORIGIN'); + expect(csp).not.toContain('frame-ancestors'); + // Only the installed current version has a frame; anything else 404s. await api().get('/api/v1/plugins/framer/9.9.9/frame').expect(404); await api().get('/api/v1/plugins/ghost/1.0.0/frame').expect(404); diff --git a/docs/architecture/security.md b/docs/architecture/security.md index 32005ca..a91ba42 100644 --- a/docs/architecture/security.md +++ b/docs/architecture/security.md @@ -73,6 +73,39 @@ or sloppy plugin authors, compromised dependencies. vectors. (The plaintext cache row itself remains until purge; the index is the concern here because it is queryable.) +## Security response headers & CORS (issue #197) + +Every api response carries this header set, stamped by a hand-rolled +15-line middleware (`apps/api/src/common/security-headers.middleware.ts`) +rather than `helmet` — the set is small enough to own, each value is a +deliberate decision, and the api gains no transitive dependency. It is +wired through the AppModule's `MiddlewareConsumer`, so the e2e harness +boots the exact production middleware. + +| Header | Value | Why | +| --------------------------- | -------------------------------------------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `Strict-Transport-Security` | `max-age=31536000` | One year, **no** `includeSubDomains` — the api cannot speak for sibling subdomains it does not control. Browsers ignore HSTS over plain http, so it is sent unconditionally. | +| `X-Content-Type-Options` | `nosniff` | No MIME sniffing, anywhere (the uploads path relies on this too, see above). | +| `Referrer-Policy` | `no-referrer` | Page paths are permission-scoped knowledge; leak them to no destination. | +| `X-Frame-Options` | `SAMEORIGIN` | Deliberately **not** `DENY`: the plugin sandbox (ADR 0008) embeds `/api/v1/plugins///frame` same-origin, and the frame's CSP has no `frame-ancestors` — this header governs its framing. | +| `Permissions-Policy` | `camera=(), microphone=(), geolocation=(), payment=(), usb=()` | Powerful browser features denied outright; nothing in the app uses them. | + +**CORS** is a stated decision, not an implicit default: no foreign origin +is granted anything. The middleware echoes `Access-Control-Allow-Origin` +(plus `Allow-Credentials: true`) only for the `APP_BASE_URL` origin +itself — where browsers never consult CORS anyway, since the SPA calls +the api same-origin (the dev server proxies `/api`). The echo documents +the stance rather than enabling a caller; consequently there is no +preflight handling (same-origin requests never preflight), and every +response carries `Vary: Origin` for cache correctness. Cross-origin API +access is cookie-less by design anyway (PAT/Bearer, see Public API), and +non-browser clients are unaffected by CORS. + +Regression fence: `apps/api/src/common/security-headers.e2e.test.ts` +(header set, foreign origin gets no ACAO) and the frame assertion in +`plugins.e2e.db.test.ts`. TLS termination itself is the reverse proxy's +job (out of scope, below). + ## Plugin sandboxing (ADR 0008, operational) - Code plugins: opaque-origin iframes, no network (`connect-src 'none'`), diff --git a/docs/vs-nfd/20-massnahmenplan.md b/docs/vs-nfd/20-massnahmenplan.md index 538d169..db3dccc 100644 --- a/docs/vs-nfd/20-massnahmenplan.md +++ b/docs/vs-nfd/20-massnahmenplan.md @@ -104,7 +104,7 @@ chain`_ oder entfernen · 2 AT · #194 - [x] **Papierkorb aus dem Suchindex** entfernen statt query-seitig filtern · 2 AT · #195 - [x] **Retention-Job für `audit_log`** · 1 AT · #196 -- [ ] **Security-Header** (helmet), CORS explizit restriktiv · 1 AT · #197 +- [x] **Security-Header** (helmet), CORS explizit restriktiv · 1 AT · #197 - [ ] **SBOM in CI** (CycloneDX/syft) + Lizenzreport als Artefakt · 1–2 AT · #202 - [ ] `deploy/compose/.env` prüfen, Beispieldatei statt Realdatei · 0,5 AT · #198 - [ ] **Attachment-Integritätshashes** · +2–3 AT · #199 ⟵ neu aus Roadmap