Files
CapaKraken/review-report.md
T
2026-03-15 09:29:19 +01:00

4.7 KiB

Review-Report — 2026-03-15

Ergebnis: Bestanden

Alle Quality Gates bestanden. Keine kritischen Probleme gefunden. Zwei Minor-Issues behoben waehrend des Reviews.

Quality Gates

Gate Status Details
Engine Tests 254/254 bestanden (17 Dateien)
Staffing Tests 37/37 bestanden (3 Dateien)
API Tests 209/209 bestanden (21 Dateien)
Application Tests 67/67 bestanden (15 Dateien)
Gesamt 567/567 Tests bestanden
TypeScript (web) 0 Fehler
TypeScript (api) 0 Fehler
Paketabhaengigkeiten Keine Zyklen

Architektur-Checkliste

  • Keine zirkulaeren Abhaengigkeiten — engine, staffing, ui sind sauber isoliert
  • engine und staffing haben keine DB-Imports
  • Alle 24 tRPC-Router in packages/api/src/router/index.ts registriert
  • SSE-Events: 10 Emitter fuer Allocation, Project, Budget, Vacation, Role, Notification
  • SSE-Debouncing (50ms) aktiv in event-bus.ts

TypeScript & Typsicherheit

  • Keine unkommentierten any-Types — alle Vorkommnisse haben // eslint-disable-next-line oder sind intentionale Casts
  • Prisma-Enums an Client-Grenzen mit as unknown as SharedType gecastet
  • JSONB-Felder korrekt gecastet
  • Nullable FKs mit optional chaining behandelt
  • exactOptionalPropertyTypes Pattern eingehalten (Spread statt { key: undefined })

Datenbank & Prisma

  • Geldbetraege als Integer-Cents
  • Kein unsicheres Raw-SQL in App-Routern — einziges $executeRaw in resource.ts nutzt tagged template literals (parameterisiert)
  • Composite Indexes fuer Assignment, DemandRequirement, Vacation vorhanden

Sicherheit

  • protectedProcedure erfordert jetzt session.user UND dbUser (gehaertet)
  • Vacation create/cancel: Ownership-Check fuer USER-Rolle
  • user.list auf adminProcedure eingeschraenkt
  • entitlement.getBalance: Ownership-Check fuer USER-Rolle
  • Allocation listView/list/listDemands/listAssignments: Anonymisierung angewendet
  • Security Headers in next.config.ts (X-Frame-Options, X-Content-Type-Options, Referrer-Policy, Permissions-Policy)
  • Keine Passwoerter/Secrets in tRPC-Responses — settings.ts gibt nur hasApiKey/hasSmtpPassword Booleans zurueck
  • Alle Mutations hinter managerProcedure oder adminProcedure
  • Kein publicProcedure in Routern

UI & Komponenten

  • AppShell: Kollabierbare Navigationsgruppen (Estimating, ACN-Orga)
  • Tailwind-Opacity nur in 5er-Schritten (CSS-Fix fuer /92, /88, /94)
  • trpc.role.list korrekt als Array behandelt

Waehrend des Reviews behoben

Minor: assignment-bookings Test-Sync

  • Datei: packages/application/src/__tests__/assignment-bookings.test.ts
  • Problem: clientId: true wurde zum project Select in list-assignment-bookings.ts hinzugefuegt, aber 3 Test-Assertions nicht aktualisiert (pre-existing)
  • Fix: clientId: true in alle 3 Test-Select-Assertions eingefuegt → 67/67 Tests bestanden

Offene Items (nicht-blockierend)

Diese stammen aus dem Security-Audit und sind im Backlog (docs/security-audit-2026-03-15.md):

  1. SSE Event-Filterung — Events werden global an alle authentifizierten User gebroadcastet, ohne Rollen-/Projekt-Scoping
  2. Rate Limiting — Kein Rate Limiter auf Auth, Admin-Tests, oder API-Endpunkten
  3. Passwort-Policy — Nur min(8) ohne Komplexitaetsanforderungen
  4. xlsx Parser — Bekannte Advisories (Prototype Pollution, ReDoS) in xlsx@0.18.5
  5. JWT maxAge — Auth.js Default 30 Tage ohne explizite Konfiguration
  6. Next.js Middleware — Kein middleware.ts fuer serverseitige Route-Protection

Empfehlungen

  1. SSE-Event-Filterung priorisieren — groesstes verbleibendes Datenschutz-Risiko
  2. Rate Limiting mit rate-limiter-flexible + Redis einbauen (Auth zuerst)
  3. xlsx durch exceljs oder SheetJS Pro ersetzen fuer untrusted Parsing
  4. middleware.ts fuer /(app)/ Routen einfuegen

Learnings-Vorschlag fuer LEARNINGS.md

### Security: protectedProcedure muss dbUser pruefen (2026-03-15)
`protectedProcedure` pruefte nur `session.user`, nicht ob der DB-User noch existiert.
Geloest: `dbUser`-Check in die Middleware eingefuegt. Stale Sessions werden jetzt abgelehnt.
Konsequenz: Alle Test-Caller muessen ein gueltiges `dbUser`-Objekt mitgeben.

### Security: IDOR-Checks bei Self-Service-Endpunkten (2026-03-15)
Vacation create/cancel und Entitlement getBalance hatten keine Ownership-Checks.
USER-Rolle konnte Aktionen fuer beliebige Ressourcen ausfuehren.
Pattern: Bei `protectedProcedure`-Endpunkten die eine `resourceId` akzeptieren,
immer `resource.userId === ctx.dbUser.id` pruefen fuer nicht-privilegierte Rollen.