99 lines
4.7 KiB
Markdown
99 lines
4.7 KiB
Markdown
# 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
|
|
|
|
- [x] Keine zirkulaeren Abhaengigkeiten — engine, staffing, ui sind sauber isoliert
|
|
- [x] `engine` und `staffing` haben keine DB-Imports
|
|
- [x] Alle 24 tRPC-Router in `packages/api/src/router/index.ts` registriert
|
|
- [x] SSE-Events: 10 Emitter fuer Allocation, Project, Budget, Vacation, Role, Notification
|
|
- [x] SSE-Debouncing (50ms) aktiv in event-bus.ts
|
|
|
|
## TypeScript & Typsicherheit
|
|
|
|
- [x] Keine unkommentierten `any`-Types — alle Vorkommnisse haben `// eslint-disable-next-line` oder sind intentionale Casts
|
|
- [x] Prisma-Enums an Client-Grenzen mit `as unknown as SharedType` gecastet
|
|
- [x] JSONB-Felder korrekt gecastet
|
|
- [x] Nullable FKs mit optional chaining behandelt
|
|
- [x] `exactOptionalPropertyTypes` Pattern eingehalten (Spread statt `{ key: undefined }`)
|
|
|
|
## Datenbank & Prisma
|
|
|
|
- [x] Geldbetraege als Integer-Cents
|
|
- [x] Kein unsicheres Raw-SQL in App-Routern — einziges `$executeRaw` in resource.ts nutzt tagged template literals (parameterisiert)
|
|
- [x] Composite Indexes fuer Assignment, DemandRequirement, Vacation vorhanden
|
|
|
|
## Sicherheit
|
|
|
|
- [x] `protectedProcedure` erfordert jetzt `session.user` UND `dbUser` (gehaertet)
|
|
- [x] Vacation create/cancel: Ownership-Check fuer USER-Rolle
|
|
- [x] `user.list` auf `adminProcedure` eingeschraenkt
|
|
- [x] `entitlement.getBalance`: Ownership-Check fuer USER-Rolle
|
|
- [x] Allocation listView/list/listDemands/listAssignments: Anonymisierung angewendet
|
|
- [x] Security Headers in next.config.ts (X-Frame-Options, X-Content-Type-Options, Referrer-Policy, Permissions-Policy)
|
|
- [x] Keine Passwoerter/Secrets in tRPC-Responses — `settings.ts` gibt nur `hasApiKey`/`hasSmtpPassword` Booleans zurueck
|
|
- [x] Alle Mutations hinter `managerProcedure` oder `adminProcedure`
|
|
- [x] Kein `publicProcedure` in Routern
|
|
|
|
## UI & Komponenten
|
|
|
|
- [x] AppShell: Kollabierbare Navigationsgruppen (Estimating, ACN-Orga)
|
|
- [x] Tailwind-Opacity nur in 5er-Schritten (CSS-Fix fuer `/92`, `/88`, `/94`)
|
|
- [x] `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.
|
|
```
|