4f6aff32ab
continuous-integration/drone/push Build is passing
- API Key Middleware für alle /api/ Endpoints (X-API-Key, Bearer, Query) - API Key Management Routes (generate, revoke, status) - MCP Tools erweitert: 12 Tools (war 3) inkl. bulk ops, dashboard, search - Audit Logging für alle API/MCP Aktionen - docs/API-REFERENCE.md - vollständige API Dokumentation - docs/ARCHITECTURE.md - Architektur-Übersicht - Vorbereitung für NotiBot-Integration
679 lines
21 KiB
Markdown
679 lines
21 KiB
Markdown
# TaskFlow Refactoring Review
|
||
|
||
**Datum:** 2026-02-03
|
||
**Reviewer:** AI Code Review (Senior Level)
|
||
**Scope:** Full-Stack Analyse – Server, Shared, Client
|
||
|
||
---
|
||
|
||
## Executive Summary
|
||
|
||
TaskFlow ist eine funktional umfangreiche Task-Management-App mit Gamification, ADHD-Features, AI-Integration und Social-Features. Der Code funktioniert, hat aber **erhebliche architektonische und sicherheitstechnische Probleme**, die vor einem Production-Release adressiert werden müssen.
|
||
|
||
**Kernprobleme:**
|
||
1. **`routes.ts` ist ein 2345-Zeilen-Monolith** – alle 96 Endpoints in einer Datei, keine klare Trennung
|
||
2. **Kritische Security-Issues** – Debug-Endpoints ohne Auth, 2FA-Codes im Response, hardcoded Session Secret
|
||
3. **Doppelte Route-Registrierungen** – `DELETE /api/tasks/:id` ist zweimal definiert (Zeile 889 + 1277)
|
||
4. **Doppelter Label-Delete** – `storage.deleteLabel()` wird zweimal aufgerufen (Zeile 970 + 974)
|
||
5. **Kein Rate Limiting, kein CORS, kein Helmet** – grundlegende Security-Middleware fehlt komplett
|
||
6. **Massive `any`-Nutzung** – 20+ `any`-Types allein in routes.ts
|
||
7. **N+1 Query-Probleme** in `getTasksForUser()` und Label-Share-Views
|
||
|
||
**Gesamtbewertung:** ⚠️ **Funktional, aber nicht produktionsreif.** Geschätzter Refactoring-Aufwand: 3-4 Wochen für eine Person.
|
||
|
||
---
|
||
|
||
## Kritische Issues
|
||
|
||
### 🔴 SEC-1: Debug-Endpoints ohne Authentifizierung (KRITISCH)
|
||
|
||
**Datei:** `server/routes.ts`, Zeilen 93-113
|
||
|
||
```typescript
|
||
app.post("/api/debug/fix-settings", async (req, res) => {
|
||
await storage.setSystemSettings("registration_enabled", "true");
|
||
// ... resets SMTP, enables registration
|
||
});
|
||
|
||
app.post("/api/debug/force-enable-registration", async (req, res) => {
|
||
await storage.setSystemSettings("registration_enabled", "true");
|
||
});
|
||
```
|
||
|
||
**Problem:** Jeder kann ohne Authentifizierung die Registrierung aktivieren und SMTP-Einstellungen überschreiben. Ein Angreifer kann sich dadurch selbst registrieren und das System kompromittieren.
|
||
|
||
**Fix:** Sofort entfernen oder hinter `isAdmin`-Middleware + Environment-Check (`NODE_ENV !== 'production'`) schützen.
|
||
|
||
---
|
||
|
||
### 🔴 SEC-2: 2FA-Code im API-Response exponiert (KRITISCH)
|
||
|
||
**Datei:** `server/auth.ts`, Zeilen 182 und 262
|
||
|
||
```typescript
|
||
return res.status(200).json({
|
||
message: "2fa_required",
|
||
userId: user.id,
|
||
email: user.email,
|
||
debugCode: code // ← 2FA-Code im Klartext!
|
||
});
|
||
```
|
||
|
||
**Problem:** Der 2FA-Code wird direkt im Login-Response zurückgegeben. Das macht 2FA komplett wirkungslos – jeder Angreifer, der das Login-Request sieht, hat den Code.
|
||
|
||
**Fix:** `debugCode` entfernen. Nur in `NODE_ENV === 'development'` als Console-Log.
|
||
|
||
---
|
||
|
||
### 🔴 SEC-3: Hardcoded Session Secret (KRITISCH)
|
||
|
||
**Datei:** `server/auth.ts`, Zeile 27
|
||
|
||
```typescript
|
||
secret: process.env.SESSION_SECRET || "s3cr3t_m3ss4g3",
|
||
```
|
||
|
||
**Problem:** Wenn `SESSION_SECRET` nicht gesetzt ist, wird ein bekanntes Secret verwendet. Session-Cookies können damit gefälscht werden.
|
||
|
||
**Fix:** Bei fehlendem `SESSION_SECRET` in Production den Server nicht starten lassen:
|
||
```typescript
|
||
if (!process.env.SESSION_SECRET && process.env.NODE_ENV === 'production') {
|
||
throw new Error("SESSION_SECRET must be set in production!");
|
||
}
|
||
```
|
||
|
||
---
|
||
|
||
### 🔴 SEC-4: 2FA Fail-Open – Immer bypass (KRITISCH)
|
||
|
||
**Datei:** `server/auth.ts`, Zeile 170
|
||
|
||
```typescript
|
||
if (true) { // ← IMMER true!
|
||
return res.status(200).json({
|
||
message: "2fa_required",
|
||
...
|
||
});
|
||
}
|
||
```
|
||
|
||
Die Bedingung `if (true)` macht den Code-Pfad, der prüft ob die E-Mail tatsächlich gesendet wurde, irrelevant. Die Variable `sent` wird nie ausgewertet. Der Kommentar sagt "Fail-open for testing" – das muss in Production gefixt werden.
|
||
|
||
---
|
||
|
||
### 🔴 SEC-5: Fehlende Auth-Checks auf mehreren Endpoints
|
||
|
||
| Endpoint | Problem |
|
||
|---|---|
|
||
| `GET /api/labels/:id` | Kein `isAuthenticated()` Check |
|
||
| `POST /api/labels` | Kein expliziter Auth-Check (nur `req.user` wird genutzt) |
|
||
| `PATCH /api/labels/:id` | Kein Auth-Check |
|
||
| `DELETE /api/labels/:id` | Kein Auth-Check, kein Ownership-Check |
|
||
| `GET /api/notes` | Kein Auth-Check |
|
||
| `POST /api/notes` | Kein Auth-Check |
|
||
| `GET /api/goals` | Kein Auth-Check |
|
||
| `POST /api/goals` | Kein Auth-Check |
|
||
| `GET /api/rewards` | Kein Auth-Check |
|
||
| `GET /api/leaderboard` | Kein Auth-Check (gewollt?) |
|
||
|
||
---
|
||
|
||
### 🔴 SEC-6: Fehlende Security-Middleware
|
||
|
||
Nicht vorhanden:
|
||
- **Rate Limiting** – Brute-Force auf Login/2FA möglich
|
||
- **Helmet.js** – Keine Security-Header (CSP, X-Frame-Options, etc.)
|
||
- **CORS** – Keine Origin-Einschränkung
|
||
- **CSRF Protection** – Keine CSRF-Tokens
|
||
- **Input Sanitization** – Kein XSS-Schutz auf user-generated Content
|
||
|
||
---
|
||
|
||
### 🟡 SEC-7: Gemini API-Key in URL exponiert
|
||
|
||
**Datei:** `server/ai.ts`
|
||
|
||
```typescript
|
||
const url = `...?key=${apiKey}`;
|
||
```
|
||
|
||
Google Gemini API-Key wird als Query-Parameter gesendet und landet damit in Server-Logs und Browser-History.
|
||
|
||
---
|
||
|
||
## Architektur-Empfehlungen
|
||
|
||
### ARCH-1: routes.ts aufteilen (Höchste Priorität)
|
||
|
||
**Aktuell:** 1 Datei, 2345 Zeilen, 96 Endpoints, 0 Separation of Concerns.
|
||
|
||
**Vorschlag: Router-Module**
|
||
|
||
```
|
||
server/
|
||
├── routes/
|
||
│ ├── index.ts # Router-Registry
|
||
│ ├── auth.routes.ts # Login, Register, 2FA, Password Reset
|
||
│ ├── admin.routes.ts # User-Mgmt, Settings, Audit Logs
|
||
│ ├── task.routes.ts # CRUD Tasks, Sharing, Subtasks
|
||
│ ├── label.routes.ts # CRUD Labels, Sharing
|
||
│ ├── ai.routes.ts # Chat, Analyze, Schedule, Breakdown
|
||
│ ├── social.routes.ts # Leaderboard, Search, Share All
|
||
│ ├── gamification.routes.ts # Rewards, XP, Goals, Challenges
|
||
│ ├── adhd.routes.ts # Break Logs, Energy, Focus, Body Doubling
|
||
│ ├── push.routes.ts # Push Notifications
|
||
│ └── mcp.routes.ts # MCP Server
|
||
├── middleware/
|
||
│ ├── auth.middleware.ts
|
||
│ ├── rate-limit.middleware.ts
|
||
│ └── validation.middleware.ts
|
||
```
|
||
|
||
**Geschätzter Aufwand:** ~2 Tage, reines Code-Moving ohne Logikänderung.
|
||
|
||
---
|
||
|
||
### ARCH-2: Storage-Interface ist zu breit
|
||
|
||
`IStorage` hat **80+ Methoden** – das ist kein Interface mehr, das ist ein God Object.
|
||
|
||
**Vorschlag: Repository Pattern**
|
||
|
||
```typescript
|
||
interface IUserRepository { ... }
|
||
interface ITaskRepository { ... }
|
||
interface ILabelRepository { ... }
|
||
interface IGamificationRepository { ... }
|
||
// etc.
|
||
```
|
||
|
||
Jedes Repository wird separat implementiert (Mem + DB). Die Service-Layer-Klassen (`GamificationService`, `AiService`) erhalten nur die Repositories, die sie brauchen.
|
||
|
||
---
|
||
|
||
### ARCH-3: MemStorage als separate Implementierung beibehalten, aber isoliert
|
||
|
||
`MemStorage` hat viele Stubs (`throw new Error("Not implemented")`). Das ist fragil. Entweder:
|
||
- **Option A:** MemStorage komplett pflegen (Test-Support)
|
||
- **Option B:** MemStorage entfernen und nur mit DB + Testcontainer arbeiten
|
||
|
||
---
|
||
|
||
### ARCH-4: Service Layer konsistent nutzen
|
||
|
||
**Aktuell:** Services existieren (`GamificationService`, `AiService`, `RecurrenceService`, `EmailService`), aber die Logik ist trotzdem großteils in `routes.ts` verstreut.
|
||
|
||
Beispiel: `calculateXP()` (Zeile 1393) ist eine lose Funktion in routes.ts, obwohl `GamificationService.getXPForSource()` existiert. `calculateXP` wird nie aufgerufen → **Dead Code**.
|
||
|
||
---
|
||
|
||
### ARCH-5: DB Migration ist fragil
|
||
|
||
**Datei:** `server/db.ts`, Zeilen 56-85
|
||
|
||
`runMigrations()` erstellt nur 3 Tabellen (`users`, `labels`, `tasks`) mit minimalem Schema. Das Schema in `shared/schema.ts` hat **20+ Tabellen**. Die Funktion ist veraltet und gefährlich, weil sie existierende Tabellen nicht aktualisiert.
|
||
|
||
**Fix:** Drizzle Kit Migrations (`drizzle-kit generate` + `drizzle-kit migrate`) verwenden statt manueller SQL.
|
||
|
||
---
|
||
|
||
## Code-Qualität
|
||
|
||
### DUP-1: Doppelte DELETE Route für Tasks
|
||
|
||
**Datei:** `server/routes.ts`
|
||
|
||
```
|
||
Zeile 889: app.delete("/api/tasks/:id", ...) ← mit Auth + Ownership-Check
|
||
Zeile 1277: app.delete("/api/tasks/:id", ...) ← OHNE Auth-Check!
|
||
```
|
||
|
||
Express registriert beide, aber nur die **erste** wird ausgeführt (Zeile 889). Die zweite (1277) ist Dead Code, aber ihre Existenz zeigt Copy-Paste-Probleme. **Die zweite hat keinen Auth-Check** – wäre sie die erste, hätte jeder Tasks löschen können.
|
||
|
||
---
|
||
|
||
### DUP-2: Doppelter deleteLabel()-Aufruf
|
||
|
||
**Datei:** `server/routes.ts`, Zeilen 970-974
|
||
|
||
```typescript
|
||
const success = await storage.deleteLabel(req.params.id);
|
||
if (!success) {
|
||
return res.status(404).json({ error: "Label not found" });
|
||
}
|
||
await storage.deleteLabel(req.params.id); // ← Zweiter Delete-Aufruf!
|
||
```
|
||
|
||
Das Label wird zweimal gelöscht. Der zweite Call ist ein No-Op (already deleted), aber es ist ein Bug.
|
||
|
||
---
|
||
|
||
### DUP-3: Doppelte createReward()-Deklaration im Interface
|
||
|
||
**Datei:** `server/storage.ts`, Zeilen 79-80
|
||
|
||
```typescript
|
||
createReward(reward: InsertReward): Promise<Reward>;
|
||
createReward(reward: InsertReward): Promise<Reward>; // Duplikat
|
||
```
|
||
|
||
---
|
||
|
||
### DUP-4: AI Context-Building ist copy-pasted
|
||
|
||
Der User-Context-Block (`User Context: - User ID: ...`) wird in mindestens 3 Stellen fast identisch gebaut:
|
||
- `POST /api/ai/chat` (Zeile ~680)
|
||
- `PUT /api/ai/chat/:messageId` (Zeile ~800)
|
||
- `POST /api/ai/analyze-completion` (Zeile ~740)
|
||
|
||
**Fix:** `buildUserContext(user: User, tasks: Task[]): string` Utility-Funktion extrahieren.
|
||
|
||
---
|
||
|
||
### DUP-5: Auth-Check Pattern wiederholt sich 50+ mal
|
||
|
||
```typescript
|
||
if (!req.isAuthenticated()) return res.sendStatus(401);
|
||
const userId = (req.user as User).id;
|
||
```
|
||
|
||
**Fix:** Express-Middleware:
|
||
```typescript
|
||
function requireAuth(req, res, next) {
|
||
if (!req.isAuthenticated()) return res.sendStatus(401);
|
||
next();
|
||
}
|
||
```
|
||
|
||
---
|
||
|
||
### DUP-6: AuditLog-Erstellung ist boilerplate-intensiv
|
||
|
||
Fast jeder Endpoint hat:
|
||
```typescript
|
||
await storage.createAuditLog({
|
||
userId: (req.user as User).id,
|
||
action: "...",
|
||
entityType: "...",
|
||
entityId: ...,
|
||
details: ...,
|
||
source: "USER"
|
||
});
|
||
```
|
||
|
||
**Fix:** Audit-Middleware oder Decorator-Pattern.
|
||
|
||
---
|
||
|
||
### TYPE-1: any-Types (20+ Vorkommen)
|
||
|
||
| Datei | Zeile | Problem |
|
||
|---|---|---|
|
||
| `routes.ts` | 21 | `isAdmin(req: any, res: any, next: any)` |
|
||
| `routes.ts` | 366 | `const settings: any = {}` |
|
||
| `routes.ts` | 1393 | `calculateXP(task: any)` |
|
||
| `routes.ts` | 1547 | `const updates: any = {}` |
|
||
| `routes.ts` | 1835 | `const exportData: any = {}` |
|
||
| `routes.ts` | 2036 | `(user.adhdSettings as any)` |
|
||
| `storage.ts` | 80+ | ADHD methods: `any` params und returns |
|
||
| `recurrence.ts` | 39 | `const newTask: any = { ... }` |
|
||
| `gamification.ts` | Div. | `awardXP` overloads mit inkonsistenter Signatur |
|
||
|
||
**Fix:** Proper TypeScript Interfaces für alle. Express `Request`/`Response` Types mit `req.user` über Declaration Merging definieren.
|
||
|
||
---
|
||
|
||
### TYPE-2: Inkonsistente GamificationService.awardXP Signatur
|
||
|
||
In `gamification.ts`:
|
||
```typescript
|
||
async awardXP(userId: string, source: string, amount?: number, details?: any)
|
||
```
|
||
|
||
In `routes.ts` Zeile 1925 (log-break):
|
||
```typescript
|
||
await gamificationService.awardXP(userId, 10, 'break_taken', undefined, { breakType });
|
||
```
|
||
|
||
Hier wird `10` als `source` und `'break_taken'` als `amount` übergeben – die Parameter sind vertauscht! Das führt zu falschen XP-Berechnungen.
|
||
|
||
---
|
||
|
||
## Performance
|
||
|
||
### PERF-1: N+1 Queries in getTasksForUser() (DbStorage)
|
||
|
||
**Datei:** `server/storage.ts`, Zeilen ~940-1000
|
||
|
||
```typescript
|
||
async getTasksForUser(userId: string): Promise<Task[]> {
|
||
// Query 1: My tasks (with LEFT JOIN!)
|
||
const resultRaw = await this.db.select(...)...
|
||
// Query 2: Shared tasks links
|
||
const sharedLinks = await this.db.select()...
|
||
// Query 3: Shared tasks by IDs
|
||
sharedTasks = await this.db.select()...
|
||
// Query 4: Global access grants
|
||
const accessGrants = await this.db.select()...
|
||
// Query 5: Tasks from global access
|
||
globalTasks = await this.db.select()...
|
||
// Query 6: Shared labels
|
||
const sharedLabels = await this.db.select()...
|
||
// Query 7: Tasks from shared labels
|
||
tasksFromSharedLabels = await this.db.select()...
|
||
}
|
||
```
|
||
|
||
**7 separate Queries** für einen einzigen API-Call (`GET /api/tasks`). Das wird bei jedem Seitenaufruf ausgeführt.
|
||
|
||
**Fix:** Eine einzige SQL-Query mit UNIONs:
|
||
```sql
|
||
SELECT t.* FROM tasks t WHERE t.user_id = $1
|
||
UNION
|
||
SELECT t.* FROM tasks t JOIN shared_tasks st ON t.id = st.task_id WHERE st.shared_with_user_id = $1
|
||
UNION
|
||
-- etc.
|
||
```
|
||
|
||
---
|
||
|
||
### PERF-2: searchTasks() holt ALLE Tasks und filtert im JS
|
||
|
||
**Datei:** `server/storage.ts` (DbStorage):
|
||
|
||
```typescript
|
||
async searchTasks(query: string, userId: string): Promise<Task[]> {
|
||
const allTasks = await this.getTasksForUser(userId); // 7 Queries!
|
||
if (!query) return allTasks;
|
||
return allTasks.filter(t => t.title.toLowerCase().includes(lowerQuery)...);
|
||
}
|
||
```
|
||
|
||
**Fix:** SQL `ILIKE` direkt in der Query nutzen. Für Volltext: PostgreSQL Full-Text Search oder `pg_trgm`.
|
||
|
||
---
|
||
|
||
### PERF-3: Admin Settings laden jede Einstellung einzeln
|
||
|
||
**Datei:** `server/routes.ts`, Zeile 359-380
|
||
|
||
```typescript
|
||
for (const key of keys) {
|
||
const val = await storage.getSystemSettings(key); // 1 Query pro Key!
|
||
...
|
||
}
|
||
```
|
||
|
||
12 separate DB-Queries für ein Admin-Panel-Load.
|
||
|
||
**Fix:** `getSystemSettingsBulk(keys: string[]): Promise<Record<string, string>>` mit `WHERE key IN (...)`.
|
||
|
||
---
|
||
|
||
### PERF-4: AI Chat holt ALLE Tasks bei jedem Message
|
||
|
||
Jede Chat-Message triggert:
|
||
```typescript
|
||
const tasks = await storage.getTasksForUser(user.id); // 7 Queries
|
||
const activeTasks = tasks.filter(t => t.status !== 'done');
|
||
```
|
||
|
||
**Fix:** Caching (Redis/in-memory mit TTL) oder Summary-Table.
|
||
|
||
---
|
||
|
||
### PERF-5: Fehlende DB-Indizes
|
||
|
||
Schema definiert keine expliziten Indizes. Mindestens benötigt:
|
||
- `tasks.user_id` (jeder Task-Fetch)
|
||
- `tasks.label_id` (Label-basierte Queries)
|
||
- `tasks.status` (Filter nach Status)
|
||
- `tasks.parent_task_id` (Subtask-Lookups)
|
||
- `xp_events.user_id` + `created_at` (Analytics)
|
||
- `shared_tasks.shared_with_user_id` (Sharing-Lookups)
|
||
- `audit_logs.created_at` (Log-Sortierung)
|
||
|
||
---
|
||
|
||
### PERF-6: Label-Share-View hat O(n²) Problem
|
||
|
||
**Datei:** `routes.ts`, Zeile ~1000
|
||
|
||
```typescript
|
||
const sharesWithDetails = await Promise.all(users.map(async u => {
|
||
const shareInfos = await storage.getLabelShares(req.params.id); // ← SAME query in loop!
|
||
const specificShare = shareInfos.find(s => s.sharedWithUserId === u.id);
|
||
...
|
||
}));
|
||
```
|
||
|
||
`getLabelShares()` wird **einmal pro User** aufgerufen, obwohl es immer dasselbe Ergebnis liefert.
|
||
|
||
**Fix:** `getLabelShares()` einmal vor der Loop aufrufen.
|
||
|
||
---
|
||
|
||
## Dead Code & Cleanup
|
||
|
||
### DEAD-1: Unused calculateXP() function
|
||
|
||
**Datei:** `routes.ts`, Zeile 1393-1399
|
||
|
||
```typescript
|
||
const calculateXP = (task: any) => {
|
||
let baseXP = 10;
|
||
if (task.priority === 'high') baseXP += 20;
|
||
...
|
||
};
|
||
```
|
||
|
||
Wird nirgendwo aufgerufen. `GamificationService.getXPForSource()` hat diese Logik.
|
||
|
||
---
|
||
|
||
### DEAD-2: Notes-Endpoints sind Placeholder
|
||
|
||
**Datei:** `routes.ts`, Zeilen 1285-1300
|
||
|
||
```typescript
|
||
app.get("/api/notes", async (req, res) => {
|
||
res.json([]); // Placeholder until storage implementation
|
||
});
|
||
|
||
app.post("/api/notes", async (req, res) => {
|
||
res.status(201).json({ ...result.data, id: "placeholder" });
|
||
});
|
||
```
|
||
|
||
Notes-Schema existiert in `schema.ts`, aber die Storage-Implementation fehlt. Entweder implementieren oder entfernen.
|
||
|
||
---
|
||
|
||
### DEAD-3: Duplizierte DELETE /api/tasks/:id Route
|
||
|
||
Zeile 1277 ist eine Dead-Route (Express nimmt die erste Registrierung).
|
||
|
||
---
|
||
|
||
### DEAD-4: Goals-Endpoint nutzt updateTask statt updateGoal
|
||
|
||
**Datei:** `routes.ts`, Zeile 1759
|
||
|
||
```typescript
|
||
app.patch("/api/goals/:id", async (req, res) => {
|
||
const updated = await storage.updateTask(req.params.id, req.body); // ← WRONG!
|
||
```
|
||
|
||
`updateTask` statt `updateGoal` – das ist ein Bug, kein Dead Code. Goals werden als Tasks aktualisiert!
|
||
|
||
---
|
||
|
||
### DEAD-5: DEFAULT_SYSTEM_PROMPT ist doppelt definiert
|
||
|
||
- In `server/ai.ts` (Zeile 11)
|
||
- In `server/index.ts` (Zeile 87) – nochmal als inline String, aber abweichend
|
||
|
||
---
|
||
|
||
### DEAD-6: Leaderboard Route ist doppelt registriert
|
||
|
||
**Datei:** `client/src/App.tsx`
|
||
|
||
```tsx
|
||
<Route path="/leaderboard"><LeaderboardPage /></Route>
|
||
<Route path="/leaderboard"><LeaderboardPage /></Route> // Duplikat
|
||
```
|
||
|
||
---
|
||
|
||
### DEAD-7: Auskommentierter AiChat Import
|
||
|
||
**Datei:** `client/src/App.tsx`
|
||
|
||
```typescript
|
||
// import { AiChat } from "@/components/AiChat";
|
||
```
|
||
|
||
---
|
||
|
||
## Weitere Issues
|
||
|
||
### MISC-1: Fire-and-Forget AI Processing
|
||
|
||
**Datei:** `routes.ts`, Zeile ~680
|
||
|
||
```typescript
|
||
res.json(userMessage); // Response sofort gesendet
|
||
|
||
(async () => {
|
||
// AI Processing in Background
|
||
const responseContent = await aiService.chat(history, user, context);
|
||
await storage.addMessage({ ... });
|
||
})(); // ← Kein Error-Handling für Client!
|
||
```
|
||
|
||
Der Client bekommt nie die AI-Antwort direkt. Er muss pollen. Das ist fragil. **Besser: Server-Sent Events (SSE) oder WebSocket für AI-Responses.**
|
||
|
||
---
|
||
|
||
### MISC-2: PATCH /api/goals/:id hat keinen Auth-Check
|
||
|
||
Zeile 1759 – kein `isAuthenticated()` Check, kein Ownership-Check. Jeder kann jeden Goal ändern.
|
||
|
||
(Hinweis: Wird aktuell fälschlicherweise über `updateTask` geroutet – siehe DEAD-4.)
|
||
|
||
---
|
||
|
||
### MISC-3: XP Double-Counting Risk
|
||
|
||
**Datei:** `server/storage.ts` (DbStorage), `logXpEvent()`:
|
||
|
||
```typescript
|
||
async logXpEvent(event: InsertXpEvent): Promise<XpEvent> {
|
||
const result = await this.db.insert(schema.xpEvents).values(event).returning();
|
||
if (event.userId) {
|
||
await this.updateUserXP(event.userId, event.amount); // ← Adds XP again!
|
||
}
|
||
return result[0];
|
||
}
|
||
```
|
||
|
||
`GamificationService.awardXP()` ruft zuerst `storage.updateUserXP(userId, xpAmount)` auf, dann `storage.logXpEvent()`. In `logXpEvent()` wird `updateUserXP()` nochmal aufgerufen → **XP wird doppelt vergeben!**
|
||
|
||
---
|
||
|
||
### MISC-4: Fehlende Pagination
|
||
|
||
Keine einzige Liste hat Pagination:
|
||
- `GET /api/tasks` – alle Tasks auf einmal
|
||
- `GET /api/admin/users` – alle User
|
||
- `GET /api/admin/audit-logs` – max 100 (hardcoded)
|
||
- `GET /api/ai/conversations` – alle Conversations
|
||
- `GET /api/user/history` – alle XP Events
|
||
|
||
---
|
||
|
||
## Priorisierte Roadmap
|
||
|
||
### Phase 1: Security Fixes (1-2 Tage) 🔴
|
||
| # | Task | Impact |
|
||
|---|---|---|
|
||
| 1 | Debug-Endpoints entfernen/schützen (SEC-1) | Kritisch |
|
||
| 2 | `debugCode` aus API-Responses entfernen (SEC-2) | Kritisch |
|
||
| 3 | Session Secret Validierung (SEC-3) | Kritisch |
|
||
| 4 | `if (true)` in 2FA-Flow fixen (SEC-4) | Kritisch |
|
||
| 5 | Fehlende Auth-Checks hinzufügen (SEC-5) | Kritisch |
|
||
| 6 | Rate Limiting installieren (`express-rate-limit`) | Hoch |
|
||
| 7 | Helmet.js einbinden | Hoch |
|
||
| 8 | CORS konfigurieren | Hoch |
|
||
|
||
### Phase 2: Bug Fixes (1 Tag) 🟡
|
||
| # | Task | Impact |
|
||
|---|---|---|
|
||
| 1 | Doppelte `DELETE /api/tasks/:id` Route entfernen (DUP-1) | Bug |
|
||
| 2 | Doppelten `deleteLabel()` Call fixen (DUP-2) | Bug |
|
||
| 3 | `PATCH /api/goals/:id` → `updateGoal` statt `updateTask` (DEAD-4) | Bug |
|
||
| 4 | XP Double-Counting fixen (MISC-3) | Bug |
|
||
| 5 | `gamificationService.awardXP()` Parameter-Reihenfolge fixen (TYPE-2) | Bug |
|
||
|
||
### Phase 3: Architecture – routes.ts aufteilen (2-3 Tage)
|
||
| # | Task |
|
||
|---|---|
|
||
| 1 | Router-Module erstellen (10 Dateien) |
|
||
| 2 | Auth-Middleware extrahieren (`requireAuth`, `requireAdmin`) |
|
||
| 3 | Validation-Middleware mit Zod |
|
||
| 4 | Audit-Log-Helper/Middleware |
|
||
| 5 | User-Context-Builder extrahieren |
|
||
|
||
### Phase 4: Performance (2-3 Tage)
|
||
| # | Task |
|
||
|---|---|
|
||
| 1 | `getTasksForUser()` zu einer UNION-Query refactoren |
|
||
| 2 | `searchTasks()` mit SQL ILIKE |
|
||
| 3 | Bulk `getSystemSettings()` |
|
||
| 4 | DB-Indizes hinzufügen |
|
||
| 5 | Pagination für alle Listen |
|
||
|
||
### Phase 5: TypeScript & Code Quality (2-3 Tage)
|
||
| # | Task |
|
||
|---|---|
|
||
| 1 | `any` Types durch proper Interfaces ersetzen |
|
||
| 2 | Express Request Type-Augmentation für `req.user` |
|
||
| 3 | Dead Code entfernen (Notes-Placeholder, calculateXP, doppelte Routes) |
|
||
| 4 | Storage Interface aufteilen (Repository Pattern) |
|
||
| 5 | Consistent Error-Handling mit Custom Error Classes |
|
||
|
||
### Phase 6: Nice-to-Have (ongoing)
|
||
| # | Task |
|
||
|---|---|
|
||
| 1 | SSE/WebSocket für AI-Chat statt Fire-and-Forget |
|
||
| 2 | Drizzle Kit Migrations statt manueller SQL |
|
||
| 3 | MemStorage entweder voll implementieren oder entfernen |
|
||
| 4 | OpenAPI/Swagger Spec generieren |
|
||
| 5 | Integration Tests |
|
||
| 6 | Client-seitige Validierung mit shared Zod Schemas |
|
||
|
||
---
|
||
|
||
## Zusammenfassung nach Datei
|
||
|
||
| Datei | LOC | Zustand | Hauptprobleme |
|
||
|---|---|---|---|
|
||
| `server/routes.ts` | 2345 | 🔴 Kritisch | Monolith, Duplikate, fehlende Auth, Dead Code |
|
||
| `server/storage.ts` | 1634 | 🟡 Problematisch | God-Interface (80+ Methods), any-Types, N+1 |
|
||
| `shared/schema.ts` | 551 | 🟢 OK | Gut strukturiert, fehlende Indizes |
|
||
| `server/auth.ts` | 270 | 🔴 Kritisch | debugCode, hardcoded secret, if(true) |
|
||
| `server/ai.ts` | 550 | 🟡 Problematisch | Sehr lang, aber gut strukturiert. Gemini-Key in URL |
|
||
| `server/gamification.ts` | 160 | 🟢 OK | Inkonsistente awardXP-Signatur |
|
||
| `server/email.ts` | 130 | 🟢 OK | Solide, i18n-Support |
|
||
| `server/push.ts` | 220 | 🟢 OK | Gute Fehlerbehandlung |
|
||
| `server/services/recurrence.ts` | 100 | 🟢 OK | `any`-Cast, sonst sauber |
|
||
| `server/db.ts` | 100 | 🟡 Problematisch | Veraltete Migration-Logik |
|
||
| `server/index.ts` | 140 | 🟡 Problematisch | Doppelter System-Prompt, aber sonst OK |
|
||
| `server/mcp.ts` | 200 | 🟢 OK | Saubere MCP-Implementation |
|
||
| `client/src/App.tsx` | 340 | 🟡 Problematisch | Zu viel Logik, doppelte Route, aber funktional |
|
||
|
||
---
|
||
|
||
*Dieser Report wurde automatisch generiert. Alle Zeilenangaben beziehen sich auf den Stand vom 2026-02-03.*
|