From ad81a7c93ee2aa1108029f4dbca235b28f1323f9 Mon Sep 17 00:00:00 2001 From: duffyduck Date: Tue, 2 Jun 2026 13:51:32 +0200 Subject: [PATCH] Pentest 64.1 LOW: ApiError-Klasse, Race-Lock liefert jetzt 400 statt 500 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit assertNoRecentDuplicateDocument warf einen generischen Error → die Catch-Blöcke in den drei ContractDocument-Schreibpfaden mappten das auf 500, obwohl es klar eine 400-Class-Situation (Caller-Fehler: Duplikat-Submit) ist. Neuer ApiError-Helper in utils/apiError: - ApiError(statusCode, message) – einfache Subklasse von Error mit explizitem HTTP-Status. assertNoRecentDuplicateDocument wirft jetzt ApiError(400, ...). Catch-Blöcke gehärtet (Service-Pattern: `error instanceof ApiError ? error.statusCode : `): - contract.controller uploadContractDocument: 400-Default bleibt, ApiError wird honoriert; bonus: multer-Datei wird bei Reject jetzt gelöscht (war vorher orphaned bei Lock-Reject). - cachedEmail.controller saveEmailAsContractDocument: 500-Default, ApiError → 400. - cachedEmail.controller saveAttachmentAsContractDocument: dito. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../src/controllers/cachedEmail.controller.ts | 10 ++++++++-- backend/src/controllers/contract.controller.ts | 9 ++++++++- .../contractStatusScheduler.service.ts | 5 ++++- backend/src/utils/apiError.ts | 18 ++++++++++++++++++ 4 files changed, 38 insertions(+), 4 deletions(-) create mode 100644 backend/src/utils/apiError.ts diff --git a/backend/src/controllers/cachedEmail.controller.ts b/backend/src/controllers/cachedEmail.controller.ts index 8a0e4446..53ff44c0 100644 --- a/backend/src/controllers/cachedEmail.controller.ts +++ b/backend/src/controllers/cachedEmail.controller.ts @@ -9,6 +9,7 @@ import { fetchAttachment, appendToSent, ImapCredentials } from '../services/imap import { getImapSmtpSettings } from '../services/emailProvider/emailProviderService.js'; import { decrypt } from '../utils/encryption.js'; import { sanitizeNotes, stripHtml, validateContractDocumentType, validateOptionalIsoDate } from '../utils/sanitize.js'; +import { ApiError } from '../utils/apiError.js'; import { logChange } from '../services/audit.service.js'; import { ApiResponse, AuthRequest } from '../types/index.js'; import { getCustomerTargets, getContractTargets, getIdentityDocumentTargets, getBankCardTargets, documentTargets } from '../config/documentTargets.config.js'; @@ -1914,8 +1915,11 @@ export async function saveEmailAsContractDocument(req: AuthRequest, res: Respons res.json({ success: true, data: doc } as ApiResponse); } catch (error) { console.error('saveEmailAsContractDocument error:', error); + // Pentest 64.1: ApiError mit eigenem statusCode (z.B. 400 vom Race- + // Lock) statt pauschal 500. + const status = error instanceof ApiError ? error.statusCode : 500; const errorMessage = error instanceof Error ? error.message : 'Unbekannter Fehler'; - res.status(500).json({ success: false, error: `Fehler beim Speichern: ${errorMessage}` } as ApiResponse); + res.status(status).json({ success: false, error: `Fehler beim Speichern: ${errorMessage}` } as ApiResponse); } } @@ -2226,8 +2230,10 @@ export async function saveAttachmentAsContractDocument(req: AuthRequest, res: Re res.json({ success: true, data: doc } as ApiResponse); } catch (error) { console.error('saveAttachmentAsContractDocument error:', error); + // Pentest 64.1: ApiError mit eigenem statusCode statt pauschal 500. + const status = error instanceof ApiError ? error.statusCode : 500; const errorMessage = error instanceof Error ? error.message : 'Unbekannter Fehler'; - res.status(500).json({ + res.status(status).json({ success: false, error: `Fehler beim Speichern: ${errorMessage}`, } as ApiResponse); diff --git a/backend/src/controllers/contract.controller.ts b/backend/src/controllers/contract.controller.ts index 6cb509f2..9ab89165 100644 --- a/backend/src/controllers/contract.controller.ts +++ b/backend/src/controllers/contract.controller.ts @@ -9,6 +9,7 @@ import { recordPredecessorFinalReading } from '../services/customer.service.js'; import { ApiResponse, AuthRequest } from '../types/index.js'; import { logChange } from '../services/audit.service.js'; import { sanitizeContract, sanitizeContractStrict, sanitizeContracts, sanitizeContractsStrict, stripHtml, sanitizeNotes, validateContractDocumentType, validateOptionalIsoDate } from '../utils/sanitize.js'; +import { ApiError } from '../utils/apiError.js'; import { canAccessContract } from '../utils/accessControl.js'; import { maybeActivateOnDeliveryConfirmation, withContractDocumentLock } from '../services/contractStatusScheduler.service.js'; @@ -783,7 +784,13 @@ export async function uploadContractDocument(req: AuthRequest, res: Response): P res.status(201).json({ success: true, data: doc } as ApiResponse); } catch (error) { - res.status(400).json({ + // Pentest 64.1: ApiError mit eigenem statusCode honorieren (z.B. 400 + // vom Race-Lock); fallback bleibt 400 für sonstige ContractDocument- + // Schreibfehler. + const status = error instanceof ApiError ? error.statusCode : 400; + // Multer hat die Datei schon geschrieben – bei Reject räumen. + if (req.file?.path) try { fs.unlinkSync(req.file.path); } catch { /* ignore */ } + res.status(status).json({ success: false, error: error instanceof Error ? error.message : 'Fehler beim Hochladen', } as ApiResponse); diff --git a/backend/src/services/contractStatusScheduler.service.ts b/backend/src/services/contractStatusScheduler.service.ts index b03e8b11..89305f49 100644 --- a/backend/src/services/contractStatusScheduler.service.ts +++ b/backend/src/services/contractStatusScheduler.service.ts @@ -10,6 +10,7 @@ import cron from 'node-cron'; import prisma from '../lib/prisma.js'; import { createAuditLog, logChange } from './audit.service.js'; +import { ApiError } from '../utils/apiError.js'; async function runExpireCheck(): Promise { const today = new Date(); @@ -110,7 +111,9 @@ export async function assertNoRecentDuplicateDocument( select: { id: true }, }); if (recent) { - throw new Error('Ein Dokument dieses Typs wurde vor wenigen Sekunden bereits angelegt – bitte kurz warten und Seite neu laden.'); + // Pentest 64.1: ApiError(400) statt generischem Error – Caller + // mappt das auf 400 Bad Request statt pauschal 500. + throw new ApiError(400, 'Ein Dokument dieses Typs wurde vor wenigen Sekunden bereits angelegt – bitte kurz warten und Seite neu laden.'); } } diff --git a/backend/src/utils/apiError.ts b/backend/src/utils/apiError.ts new file mode 100644 index 00000000..d4cc2421 --- /dev/null +++ b/backend/src/utils/apiError.ts @@ -0,0 +1,18 @@ +/** + * Erlaubt Service-/Helper-Funktionen, einen Fehler mit explizitem HTTP- + * Status nach oben zu reichen. Controller können in ihrem `catch` per + * `instanceof ApiError` den Status auslesen statt pauschal 500 zu liefern. + * + * Pentest 64.1 (LOW, 2026-06-02): Race-Lock (assertNoRecentDuplicate- + * Document) warf einen generischen Error → catch hat 500 zurückgegeben, + * obwohl die Fehlermeldung "Dokument vor wenigen Sekunden bereits + * angelegt" eindeutig eine 400-Class-Situation ist. + */ +export class ApiError extends Error { + readonly statusCode: number; + constructor(statusCode: number, message: string) { + super(message); + this.name = 'ApiError'; + this.statusCode = statusCode; + } +}