diff --git a/backend/src/controllers/cachedEmail.controller.ts b/backend/src/controllers/cachedEmail.controller.ts index 2e82b593..5738758f 100644 --- a/backend/src/controllers/cachedEmail.controller.ts +++ b/backend/src/controllers/cachedEmail.controller.ts @@ -41,22 +41,55 @@ function parseDateParam(v: unknown): Date | undefined { return isNaN(d.getTime()) ? undefined : d; } -// Pentest 91.1 (LOW, 2026-06-21): `accountId=abc` (→ parseInt = NaN) und -// `accountId=` (leer string ist truthy nach `req.query.accountId ? …`) -// kamen vorher als `NaN` im Service an. `if (NaN)` ist falsy → der -// stressfreiEmailId-Filter wurde komplett übersprungen, und der Vertrag -// zeigte Mails aus ALLEN Postfächern. +// Pentest 91.1 (LOW, 2026-06-21): `accountId=abc` → `parseInt` = `NaN` +// → der Ternary gab `NaN` an den Service. `if (NaN)` ist falsy → der +// Postfach-Filter fiel weg, und der Vertrag zeigte Mails aus ALLEN +// Postfächern. // -// Helper akzeptiert nur positive Ganzzahlen; alles andere → undefined, -// das im Service den Filter wegfallen lässt. Wenn wir wollten dass -// invalide Werte 400 werfen statt silent ignoriert zu werden, müssten -// wir das pro Endpunkt entscheiden – die Semantik „Filter weglassen" -// ist hier in Ordnung, weil der `customerId`-/`contractId`-Constraint -// im `where` weiterhin greift (kein Cross-Customer-Leak). -function parsePositiveIntParam(v: unknown): number | undefined { - if (typeof v !== 'string' || v.trim() === '') return undefined; +// Pentest 92 (LOW, 2026-06-21): Bei `accountId=abc` auf Vertrags- +// Endpunkten reichte das silent-undefined nicht – die Mailbox-Isolation +// brach (man sah Mails aus allen Postfächern statt 0). Strict-400, weil +// Verträge per Design IMMER ein bestimmtes Postfach meinen. +// +// Helper hat zwei Modi: +// - default (optional): fehlend/leer → undefined (kein Filter) +// invalid → 400 +// - { required: true }: fehlend/leer → 400 +// invalid → 400 +// Bei 400 schreibt der Helper direkt die Response und gibt `null` +// zurück; der Caller bricht dann mit `return` ab. +function parsePositiveIntQuery( + v: unknown, + fieldLabel: string, + res: Response, + options?: { required?: boolean }, +): number | undefined | null { + const absent = v === undefined || v === '' || (typeof v === 'string' && v.trim() === ''); + if (absent) { + if (options?.required) { + res.status(400).json({ + success: false, + error: `${fieldLabel} ist erforderlich (positive Ganzzahl).`, + } as ApiResponse); + return null; + } + return undefined; + } + if (typeof v !== 'string') { + res.status(400).json({ + success: false, + error: `${fieldLabel} muss als Zahl übergeben werden.`, + } as ApiResponse); + return null; + } const n = parseInt(v, 10); - if (!Number.isFinite(n) || n < 1 || !Number.isInteger(n)) return undefined; + if (!Number.isFinite(n) || n < 1 || !Number.isInteger(n)) { + res.status(400).json({ + success: false, + error: `${fieldLabel} muss eine positive Ganzzahl sein.`, + } as ApiResponse); + return null; + } return n; } @@ -65,7 +98,10 @@ export async function getEmailsForCustomer(req: AuthRequest, res: Response): Pro try { const customerId = parseInt(req.params.customerId); if (!(await canAccessCustomer(req, res, customerId))) return; - const stressfreiEmailId = parsePositiveIntParam(req.query.accountId); + // Customer-Inbox: accountId ist legitim optional (cross-mailbox-Ansicht + // ist erwünscht), aber invalide Werte → 400. + const stressfreiEmailId = parsePositiveIntQuery(req.query.accountId, 'accountId', res); + if (stressfreiEmailId === null) return; const folder = req.query.folder as string | undefined; // INBOX oder SENT const limit = req.query.limit ? parseInt(req.query.limit as string) : 50; const offset = req.query.offset ? parseInt(req.query.offset as string) : 0; @@ -109,7 +145,11 @@ export async function getEmailsForContract(req: AuthRequest, res: Response): Pro const contractId = parseInt(req.params.contractId); if (!(await canAccessContract(req, res, contractId))) return; const folder = req.query.folder as string | undefined; // INBOX oder SENT - const stressfreiEmailId = parsePositiveIntParam(req.query.accountId); + // Vertrags-Endpunkte sind per Design IMMER pro Postfach – fehlt + // accountId, ist die Abfrage semantisch ungültig. Strict-400. + // Frontend hat eh ein `enabled: !!selectedAccountId`-Guard. + const stressfreiEmailId = parsePositiveIntQuery(req.query.accountId, 'accountId', res, { required: true }); + if (stressfreiEmailId === null || stressfreiEmailId === undefined) return; const limit = req.query.limit ? parseInt(req.query.limit as string) : 50; const offset = req.query.offset ? parseInt(req.query.offset as string) : 0; @@ -267,7 +307,10 @@ export async function getContractFolderCounts(req: AuthRequest, res: Response): try { const contractId = parseInt(req.params.contractId); if (!(await canAccessContract(req, res, contractId))) return; - const stressfreiEmailId = parsePositiveIntParam(req.query.accountId); + // Wie getEmailsForContract: Postfach ist required (sonst zeigt der + // Badge eine andere Zahl als die Liste). + const stressfreiEmailId = parsePositiveIntQuery(req.query.accountId, 'accountId', res, { required: true }); + if (stressfreiEmailId === null || stressfreiEmailId === undefined) return; const counts = await cachedEmailService.getFolderCountsForContract(contractId, stressfreiEmailId); @@ -915,8 +958,12 @@ export async function getTrashEmails(req: AuthRequest, res: Response): Promise