fix(realtime): scope events to authorized users, authorize rooms, keep SSE alive
MindSift CI & Docker Release / Build, Test in Container & Push (push) Successful in 1m6s

Phase 0 of the collaboration plan:
- Remove BroadcastToAll; task, task list and workspace folder events now
  only reach users who can see the resource. Task list events carry only
  the list ID, clients reload with their own permissions.
- Reject unauthenticated WebSocket/SSE connections (401). Every room join
  (URL room, JOIN_ROOM, SSE rooms) is checked via a room authorizer;
  /events/rooms requires login and only accepts the caller's own sseId.
- Reorder rejects tasks the caller cannot access or edit (IDOR).
- Lift the server WriteTimeout for SSE streams so they are not cut after 30s.
- Fix folder share removal sending DOC_DELETED to the share ID.
- Tests for all of the above; docs and flaw list updated.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
sandrodachandClaude Opus 5.5 committed 2026-10-28 11:19:14 +01:00
1 parent 74797e3f44
commit bcc06da3ee
10 files changed
+526 -86

No files matched your search

+9 -5
View File
@@ -10,9 +10,9 @@ Schweregrade: **Hoch** (Datenabfluss oder Rechteumgehung möglich), **Mittel** (
| ID | Bereich | Kurzbeschreibung | Schwere |
|---|---|---|---|
| FLAW-01 | Realtime | `BroadcastToAll` sendet Aufgaben- und Listendaten an alle Verbindungen, auch unauthentifizierte | **Hoch** |
| FLAW-02 | Realtime | Räume (WS `JOIN_ROOM`, SSE `/events/rooms`, `?listId=`) ohne Berechtigungsprüfung | **Hoch** |
| FLAW-03 | Aufgaben | `ReorderTasks` ändert beliebige Aufgaben per ID, ohne Zugriffsprüfung (IDOR) | **Hoch** |
| FLAW-01 | Realtime | ~~`BroadcastToAll` sendet Aufgaben- und Listendaten an alle Verbindungen~~ | **Behoben** |
| FLAW-02 | Realtime | ~~Räume ohne Berechtigungsprüfung~~ | **Behoben** |
| FLAW-03 | Aufgaben | ~~`ReorderTasks` ändert beliebige Aufgaben per ID (IDOR)~~ | **Behoben** |
| FLAW-04 | Auth | Fest eincodiertes Fallback-`JWT_SECRET` | **Hoch** |
| FLAW-05 | Benachrichtigungen | Discord-/Webhook-URLs ungeprüft (SSRF) | Mittel |
| FLAW-06 | Aufgaben | Jeder Editor einer Aufgabenliste kann sie per `teamId` an weitere Gruppen freigeben | Mittel |
@@ -24,7 +24,7 @@ Schweregrade: **Hoch** (Datenabfluss oder Rechteumgehung möglich), **Mittel** (
| FLAW-12 | Benachrichtigungen | `notify_before_minutes` wirkungslos; Alarm feuert nach Umplanung nie wieder | Mittel |
| FLAW-13 | Datenbank | Keine Schema-Versionierung; Migrationsfehler werden verschluckt; `migrations/001_init.sql` ist toter Code | Mittel |
| FLAW-14 | Backend | Fehler von Schreiboperationen werden häufig ignoriert (`_, _ =`) | Mittel |
| FLAW-15 | Realtime | `WriteTimeout` von 30 s trifft vermutlich auch SSE-Streams *(nicht live verifiziert)* | Mittel |
| FLAW-15 | Realtime | ~~`WriteTimeout` von 30 s beendet SSE-Streams~~ | **Behoben** |
| FLAW-16 | Setup | `/auth/setup` ist bis zum ersten Konto offen; Demo-Modus legt `admin/admin` an | Mittel |
| FLAW-17 | Workspace | Dokument- und Ordnerfreigaben sind per Migration erzwungen schreibgeschützt; tote Schreibpfade | Niedrig |
| FLAW-18 | Suche | Nur eigene Gedanken und Dokumente, `LIKE` ohne Escaping, keine FTS | Niedrig |
@@ -40,16 +40,19 @@ Schweregrade: **Hoch** (Datenabfluss oder Rechteumgehung möglich), **Mittel** (
## Details
### FLAW-01: Broadcast an alle Verbindungen (Datenabfluss)
- **Status: Behoben.** `BroadcastToAll` existiert nicht mehr. Aufgabenereignisse gehen an Eigentümer und Benutzer der Liste (`GetTaskListUserIDs`), Listenereignisse tragen nur noch die ID, Ordnerereignisse gehen an Benutzer bzw. Ordnerbeteiligte (`GetSharedFolderUserIDs`). Test: `TestTaskEventsOnlyReachAuthorizedUsers`.
- **Fundstellen:** [`internal/ws/hub.go`](../internal/ws/hub.go) `BroadcastToAll`; Aufrufer in [`internal/api/server.go`](../internal/api/server.go): `handleUpdateThought` (`TASK_UPDATED`), `handleDeleteThought` (`TASK_DELETED`), `handleReorderTasks` (`TASK_REORDERED`), `handleUpdateTaskList`, `handleCreateTaskListShare`, `handleDeleteTaskListShare` (`TASK_LIST_UPDATED`), `handleDeleteTaskList`.
- **Problem:** `BroadcastToAll` schreibt an `allClients` und `sseClients`. Dazu zählen auch Verbindungen mit `userID = "anonymous"`, denn `handleWebSocket` und `handleSSE` liegen außerhalb der Auth-Gruppe und lassen jede Verbindung zu. Der vollständige Aufgabeninhalt (Titel, Notizen, Unteraufgaben) und die Listen samt Mitgliedern gehen so an jeden, der `/api/v1/events` öffnet.
- **Maßnahme:** Ereignisse an die berechtigten Nutzer adressieren (wie bei `DOC_*` über `GetAuthorizedDocUsers`), unauthentifizierte Verbindungen ablehnen.
### FLAW-02: Räume ohne Berechtigungsprüfung
- **Status: Behoben.** `/ws` und `/events` verlangen ein gültiges Token (sonst `401`). Jeder Raumbeitritt läuft über `Hub.SetRoomAuthorizer` → `Server.canJoinRoom` (Notiz bzw. Liste muss lesbar sein). `/events/rooms` liegt hinter der Anmeldung und akzeptiert nur die eigene `sseId`. Tests: `TestRealtimeEndpointsRequireAuth`, `TestSSERoomJoinRequiresAccess`, `TestRoomJoinsRequireAuthorization`.
- **Fundstellen:** `Hub.readPump` (`JOIN_ROOM`), `handleSSERooms` (Route `/api/v1/events/rooms` liegt **außerhalb** der Auth-Gruppe), `Hub.Register` (Raum aus `?listId=`/`?spaceId=`).
- **Problem:** Wer eine Raum-ID kennt (`doc:<id>`, Listen-ID), tritt bei und erhält Inhalte wie `DOC_BLOCKS_PATCHED` oder `ITEM_*`. Raum-IDs tauchen in Ereignissen auf und sind nicht geheim. Für den SSE-Beitritt reicht die `sseId`, die der Server selbst im Stream mitsendet.
- **Maßnahme:** Beitritt nur nach Zugriffsprüfung (`GetDocumentByID`, `GetListByID`); `/events/rooms` hinter `authMiddleware`.
### FLAW-03: `ReorderTasks` ohne Zugriffsprüfung (IDOR)
- **Status: Behoben.** `handleReorderTasks` lehnt die ganze Anfrage ab, sobald eine Aufgabe nicht zugreifbar (`404`) oder schreibgeschützt (`403`) ist. Test: `TestReorderRejectsForeignTasks`.
- **Fundstelle:** `Store.ReorderTasks` (`UPDATE thoughts SET sort_order … WHERE id = ?`) und `handleReorderTasks`.
- **Problem:** Die SQL-Anweisung ist weder auf `user_id` noch auf die Liste eingeschränkt. Jeder angemeldete Nutzer kann `sortOrder`, `kanbanStatus` und damit `isCompleted` fremder Aufgaben ändern, sofern er die ID kennt. Die Leser-Prüfung für Aufgabenlisten überspringt Aufgaben, auf die der Nutzer gar keinen Zugriff hat (`GetThoughtByID` liefert dann einen Fehler), und schützt davor nicht.
- **Maßnahme:** Jede ID gegen `accessibleTaskListIDsQuery` bzw. `user_id` prüfen und bei Verstoß ablehnen.
@@ -119,7 +122,8 @@ Schweregrade: **Hoch** (Datenabfluss oder Rechteumgehung möglich), **Mittel** (
- **Problem:** Scheitert ein Schreibzugriff (z. B. `SQLITE_BUSY`), antwortet die API trotzdem mit Erfolg und broadcastet den Stand, der nie gespeichert wurde. Fehler lassen sich nur schwer nachvollziehen.
- **Maßnahme:** Mehrstufige Änderungen in Transaktionen bündeln und Fehler zurückgeben.
### FLAW-15: WriteTimeout und SSE *(nicht live verifiziert)*
### FLAW-15: WriteTimeout und SSE
- **Status: Behoben.** `ServeSSE` hebt die Schreibfrist für die eigene Antwort auf (`http.ResponseController.SetWriteDeadline`). Test mit 1-s-Timeout: `TestSSEStreamOutlivesWriteTimeout`. Der Test hat den Abbruch im alten Code bestätigt.
- **Fundstelle:** [`cmd/server/main.go`](../cmd/server/main.go) `WriteTimeout: 30 * time.Second`.
- **Problem:** Go beendet nach diesem Zeitraum das Schreiben der Antwort. Für den SSE-Fallback (`ServeSSE`) bedeutet das voraussichtlich einen Verbindungsabbruch alle 30 s. WebSockets sind durch Hijacking nicht betroffen. Der Client verbindet sich neu, daher fällt es kaum auf.
- **Maßnahme:** Im SSE-Handler `http.ResponseController.SetWriteDeadline(time.Time{})` setzen oder `WriteTimeout` für diese Route aufheben.