From ad520e20a7bb2d3670b7bbae3358372e66ad2d4c Mon Sep 17 00:00:00 2001 From: duffyduck Date: Wed, 19 Aug 2026 08:11:44 +0200 Subject: [PATCH] Audit-Log: Pfad-Matching im finish-Handler gefixt (Pentest R165-01) Die Entrauschung aus de0d6bd war live wirkungslos - aber nicht wegen eines Deploy-Miss, sondern weil der Code nie erreicht wurde: auditMiddleware liest req.path erst im res.on('finish')-Handler. Express strippt beim Router-Dispatch den Mount-Prefix aus req.url und stellt ihn nur beim next()-Durchlauf wieder her - ein terminaler Handler (res.json()) ruft nie next(), also bleibt req.path router-relativ (/refresh statt /api/auth/refresh). Saemtliche path.includes('/auth/...')-Checks liefen ins Leere -> Fallback POST->CREATE mit Default-Sensitivitaet CRITICAL. Betraf nicht nur den neuen TOKEN_REFRESH: LOGIN/LOGOUT/LOGIN_FAILED waren im Audit-Stream seit jeher generisch (pre-existing), ebenso das endpoint-Feld. Der SecurityEvent-Stream war nie betroffen (eigene emit-Calls), daher lief das Alerting korrekt. Fix: vollen Pfad einmal synchron beim Eintritt festhalten (req.originalUrl, wird von Express nie mutiert) und downstream ausschliesslich diesen nutzen - determineAction, generateHumanLabel, extractDataSubjectId, manuallyLoggedPaths und endpoint. TOKEN_REFRESH zusaetzlich in die "immer loggen"-Ausnahme. Verifiziert (E2E mit echter Middleware gegen Dev-DB): TOKEN_REFRESH/LOW, TOKEN_REFRESH/HIGH, LOGIN/CRITICAL, LOGIN_FAILED/CRITICAL, LOGOUT/CRITICAL, alle mit vollem endpoint-Pfad. tsc gruen. Co-Authored-By: Claude Opus 5 --- backend/src/middleware/audit.ts | 37 +++++++++++++++++++++------------ docs/todo.md | 26 +++++++++++++++++++++++ 2 files changed, 50 insertions(+), 13 deletions(-) diff --git a/backend/src/middleware/audit.ts b/backend/src/middleware/audit.ts index a6d4dde4..5ae0100f 100644 --- a/backend/src/middleware/audit.ts +++ b/backend/src/middleware/audit.ts @@ -95,10 +95,10 @@ function findResourceMapping(path: string): { type: string; extractId?: (req: Au /** * Extrahiert die betroffene Kunden-ID für DSGVO-Tracking */ -function extractDataSubjectId(req: AuthRequest): number | undefined { +function extractDataSubjectId(req: AuthRequest, fullPath: string): number | undefined { // Aus Route-Parameter const customerId = req.params.customerId || req.params.id; - if (customerId && req.path.includes('/customers')) { + if (customerId && fullPath.includes('/customers')) { return parseInt(customerId); } @@ -174,7 +174,8 @@ function generateHumanLabel( action: AuditAction, resourceType: string, req: AuthRequest, - responseBody: unknown + responseBody: unknown, + fullPath: string ): string { const typeName = RESOURCE_TYPE_LABELS[resourceType] || resourceType; const actionName = ACTION_LABELS[action] || action; @@ -196,7 +197,8 @@ function generateHumanLabel( } // Spezial-Labels für bestimmte Endpunkte - const path = req.path; + // (fullPath statt req.path – siehe Hinweis in auditMiddleware, Pentest R165) + const path = fullPath; // Auth if (path.includes('/auth/login') || path.includes('/auth/customer-login')) { @@ -371,14 +373,23 @@ function generateHumanLabel( export function auditMiddleware(req: AuthRequest, res: Response, next: NextFunction): void { const startTime = Date.now(); + // WICHTIG (Pentest R165): `req.path` ist im res.on('finish')-Handler NICHT mehr der + // volle Pfad. Express strippt beim Router-Dispatch den Mount-Prefix aus `req.url` + // und stellt ihn nur beim `next()`-Durchlauf wieder her – ein Handler, der die + // Response terminiert (res.json()), ruft nie `next()`, also bleibt `req.path` + // router-relativ (`/refresh` statt `/api/auth/refresh`). Deshalb den vollen Pfad + // EINMAL hier synchron festhalten und downstream ausschliesslich diesen nutzen – + // sonst matchen alle Pfad-Checks (/auth/login, /auth/refresh, …) ins Leere. + const fullPath = req.originalUrl?.split('?')[0] || req.path; + // Ausgeschlossene Routen überspringen - if (EXCLUDED_ROUTES.some((route) => req.path.startsWith(route))) { + if (EXCLUDED_ROUTES.some((route) => fullPath.startsWith(route))) { next(); return; } // Resource-Mapping finden - const mapping = findResourceMapping(req.path); + const mapping = findResourceMapping(fullPath); if (!mapping) { // Unbekannte Route - trotzdem loggen mit generischem Typ next(); @@ -414,7 +425,7 @@ export function auditMiddleware(req: AuthRequest, res: Response, next: NextFunct setImmediate(async () => { try { const durationMs = Date.now() - startTime; - const action = determineAction(req.method, req.path, responseSuccess); + const action = determineAction(req.method, fullPath, responseSuccess); // READ-Aktionen nicht loggen (nur Änderungen, Logins und Exporte) if (action === 'READ') return; @@ -427,19 +438,19 @@ export function auditMiddleware(req: AuthRequest, res: Response, next: NextFunct '/api/gdpr', '/api/upload', ]; - // Login/Logout immer loggen - if (action !== 'LOGIN' && action !== 'LOGOUT' && action !== 'LOGIN_FAILED') { - if (manuallyLoggedPaths.some(p => req.originalUrl?.startsWith(p) || req.baseUrl?.startsWith(p))) return; + // Login/Logout/Refresh immer loggen + if (action !== 'LOGIN' && action !== 'LOGOUT' && action !== 'LOGIN_FAILED' && action !== 'TOKEN_REFRESH') { + if (manuallyLoggedPaths.some(p => fullPath.startsWith(p))) return; } const resourceId = mapping.extractId?.(req); - const dataSubjectId = extractDataSubjectId(req); + const dataSubjectId = extractDataSubjectId(req, fullPath); // Audit-Kontext nutzen (wurde vor Response-Ende erfasst) const auditContext = capturedAuditContext; // Menschenlesbares Label generieren - const resourceLabel = generateHumanLabel(action, mapping.type, req, responseBody); + const resourceLabel = generateHumanLabel(action, mapping.type, req, responseBody, fullPath); await createAuditLog({ userId: req.user?.userId, @@ -456,7 +467,7 @@ export function auditMiddleware(req: AuthRequest, res: Response, next: NextFunct resourceType: mapping.type, resourceId, resourceLabel, - endpoint: req.path, + endpoint: fullPath, httpMethod: req.method, ipAddress: getClientIp(req), userAgent: req.headers['user-agent'], diff --git a/docs/todo.md b/docs/todo.md index d5692efa..8b0f381b 100644 --- a/docs/todo.md +++ b/docs/todo.md @@ -97,6 +97,32 @@ isolierte Instanz (keine Multi-Tenancy im Code), Provisioning + Abrechnung ## ✅ Erledigt +- [x] **🐛 Audit-Log: Pfad-Matching kaputt – Auth-Actions generisch (Pentest R165-01)** (2026-08-18) + - Pentester meldete: Entrauschung (`de0d6bd`) live **nicht wirksam** – jeder + `/refresh` weiter `CREATE / CRITICAL / „Anmeldung erstellt“`. Zusatzbefund: + auch `/login` und `/logout` liefen als generisches `CREATE`. + - **Kein Deploy-Miss** (Alerting aus `d599eb3` lief ja live), sondern **toter + Code**: `auditMiddleware` liest `req.path` erst im `res.on('finish')`-Handler. + Express strippt beim Router-Dispatch den Mount-Prefix aus `req.url` und stellt + ihn nur beim `next()`-Durchlauf wieder her – ein terminaler Handler + (`res.json()`) ruft nie `next()`, also bleibt `req.path` router-relativ + (`/refresh` statt `/api/auth/refresh`). Alle `path.includes('/auth/...')`-Checks + liefen ins Leere → Fallback POST→CREATE + Default-Sensitivität CRITICAL. + Empirisch nachgestellt (Mini-Express: ENTRY `/api/auth/refresh` → FINISH `/refresh`). + - Betraf **nicht nur** den neuen `TOKEN_REFRESH`: `LOGIN`/`LOGOUT`/`LOGIN_FAILED` + waren im Audit-Stream **seit jeher** kaputt (pre-existing), ebenso das + `endpoint`-Feld (router-relativ statt voll). Der SecurityEvent-Stream war nie + betroffen (eigene `emit()`-Calls) – daher funktionierte das Alerting korrekt. + - Fix: vollen Pfad **einmal synchron beim Eintritt** festhalten + (`req.originalUrl.split('?')[0]`, wird von Express nie mutiert) und downstream + ausschließlich diesen nutzen – in `determineAction`, `generateHumanLabel`, + `extractDataSubjectId`, `manuallyLoggedPaths` und `endpoint`. `TOKEN_REFRESH` + zusätzlich in die „immer loggen“-Ausnahme aufgenommen. + - Verifiziert (E2E mit echter Middleware gegen Dev-DB, 5 Requests): + `TOKEN_REFRESH/LOW` (Erfolg), `TOKEN_REFRESH/HIGH` (Fehlschlag), + `LOGIN/CRITICAL`, `LOGIN_FAILED/CRITICAL`, `LOGOUT/CRITICAL`, alle mit vollem + `endpoint`-Pfad und korrekten Labels. `tsc` grün. + - [x] **🛡️ Refresh-Fehlschlag: Detection-Gap geschlossen (Pentest R164-01)** (2026-08-18) - Folgefund zum Entrauschen: `determineAction` gab `/auth/refresh` bedingungslos `TOKEN_REFRESH`/LOW → ein **fehlgeschlagener** Refresh (Replay/Brute-Force auf