MindSift CI & Docker Release / Build, Test in Container & Push (push) Successful in 8s
Remove outdated claims (CRDT, DocRooms, presence, .md files on disk) and document the actual architecture, API, permissions and known flaws. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
174 lines
17 KiB
Markdown
174 lines
17 KiB
Markdown
# Bekannte Schwachstellen, Risiken & technische Schulden (Flaws)
|
|
|
|
Stand: Code-Review des Branches `dev` (Oktober 2026). Jeder Eintrag wurde gegen den Quellcode geprüft; die Fundstellen sind angegeben. Einträge ohne Live-Test sind als *nicht live verifiziert* markiert.
|
|
|
|
Schweregrade: **Hoch** (Datenabfluss oder Rechteumgehung möglich), **Mittel** (Fehlfunktion oder Schutz greift nicht), **Niedrig** (Wartbarkeit, Komfort, Hygiene).
|
|
|
|
---
|
|
|
|
## Übersicht
|
|
|
|
| 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-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 |
|
|
| FLAW-07 | Auth | API-Key-`scopes` werden nie ausgewertet; ein Key ist ein Vollzugriff | Mittel |
|
|
| FLAW-08 | Auth | Webhook-Ingest mit JWT prüft weder Sperrliste noch `token_version` noch `is_active` | Mittel |
|
|
| FLAW-09 | Auth | Kein Rate-Limit, keine Sperre bei Fehlversuchen, Mindestpasswort 4 Zeichen | Mittel |
|
|
| FLAW-10 | Freigaben | `sharing_mode` wird bei Freigaben nicht durchgesetzt; Benutzerverzeichnis leakt alle Konten | Mittel |
|
|
| FLAW-11 | Workspace | Block-Upsert (`ON CONFLICT(id)`) prüft das Dokument nicht | 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-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 |
|
|
| FLAW-19 | Qualität | Keine Frontend-Tests, `lint` und `test:run` sind Platzhalter | Niedrig |
|
|
| FLAW-20 | Qualität | Sehr große Dateien, duplizierte Zugriffslogik | Niedrig |
|
|
| FLAW-21 | Repo | `LICENSE` fehlt, obwohl README MIT angibt | Niedrig |
|
|
| FLAW-22 | Netzwerk | Token in der URL (WS/SSE), CORS `*`, `CheckOrigin` immer `true` | Niedrig |
|
|
| FLAW-23 | SQLite | Single-Writer-Engpass bei vielen gleichzeitigen Schreibern | Niedrig |
|
|
| FLAW-24 | Gruppen | Organisations-Endpunkte ohne Berechtigungsprüfung | Mittel |
|
|
|
|
---
|
|
|
|
## Details
|
|
|
|
### FLAW-01: Broadcast an alle Verbindungen (Datenabfluss)
|
|
- **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
|
|
- **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)
|
|
- **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.
|
|
|
|
### FLAW-04: Fallback-JWT-Secret
|
|
- **Fundstelle:** [`internal/config/config.go`](../internal/config/config.go): `getEnv("JWT_SECRET", "mindsift-default-selfhosted-jwt-secret-key-2026")`; `docker-compose.yml` und `.env.example` liefern ebenfalls einen Platzhalter.
|
|
- **Problem:** Wer `JWT_SECRET` nicht setzt (oder den Platzhalter behält), läuft mit einem öffentlich bekannten Schlüssel. Damit lassen sich gültige Tokens für jede Benutzer-ID fälschen. `install.sh` erzeugt zwar ein Secret, andere Installationswege nicht.
|
|
- **Maßnahme:** Start verweigern oder Secret beim ersten Start erzeugen und in `/data` ablegen.
|
|
|
|
### FLAW-05: SSRF über Alarm-URLs
|
|
- **Fundstellen:** `handleUpdateUserSettings` (keine Validierung), `sendDiscordNotification`, `sendCustomWebhookNotification`.
|
|
- **Problem:** Jeder Nutzer kann beliebige URLs hinterlegen. Der Server sendet POST-Anfragen dorthin, auch an interne Adressen (`http://169.254.169.254`, Docker-Netz, `localhost`). Über `handleTestUserAlarm` lässt sich das sofort auslösen.
|
|
- **Maßnahme:** Nur `https`, Discord-Host prüfen, private/Loopback-Adressen beim Verbinden sperren.
|
|
|
|
### FLAW-06: Editor kann Aufgabenliste weiterteilen
|
|
- **Fundstelle:** `Store.UpdateTaskList` (`teamID`-Zweig ruft `AddTaskListShare`); `handleUpdateTaskList` prüft nur, dass der Nutzer **kein** Leser ist.
|
|
- **Problem:** `POST /task-lists/{id}/shares` ist Eigentümern vorbehalten. Über `PUT /task-lists/{id}` mit `teamId` kann aber jeder Editor die Liste einer weiteren Gruppe zuweisen (mit Schreibrecht) und die Zuweisung der bisherigen Gruppe aufheben.
|
|
- **Maßnahme:** `teamId`-Änderungen nur für den Eigentümer; das Legacy-Feld `task_lists.team_id` zugunsten von `task_list_shares` abschaffen.
|
|
|
|
### FLAW-07: API-Key-Scopes ohne Wirkung
|
|
- **Fundstellen:** Tabelle `api_keys.scopes` (Standard `ingest,read`), `authMiddleware`.
|
|
- **Problem:** Ein Key mit Präfix `mindsift_sec_` authentifiziert als der Benutzer, mit allen Rechten: alle `/api/v1`-Routen, bei Admins auch `/admin/*`, außerdem das Anlegen weiterer Keys. Für Webhooks (nur Aufgaben anlegen) ist das überdimensioniert.
|
|
- **Maßnahme:** Scopes auswerten; Keys standardmäßig nur auf `/inbox` und `/webhooks/ingest` beschränken.
|
|
|
|
### FLAW-08: Webhook-Ingest umgeht Sitzungsprüfung
|
|
- **Fundstelle:** `handleWebhookIngest`, JWT-Zweig.
|
|
- **Problem:** Das Token wird nur kryptografisch validiert. `IsTokenRevoked`, `token_version` und `is_active` werden nicht geprüft; ein ausgeloggtes oder deaktiviertes Konto kann weiter Aufgaben anlegen (bis Token-Ablauf nach 30 Tagen).
|
|
- **Maßnahme:** Dieselbe Prüfkette wie in `authMiddleware` verwenden (gemeinsame Funktion statt Kopien; siehe auch `authenticateUserFromReq`).
|
|
|
|
### FLAW-09: Kein Schutz gegen Brute-Force
|
|
- **Fundstellen:** `handleLogin`, `handleRegister`, `handleFirstBootSetup`.
|
|
- **Problem:** Weder Rate-Limit noch Sperre; `middleware.RealIP` vertraut `X-Forwarded-For` ungeprüft. Mindestpasswortlänge ist 4 Zeichen. Der Login-Pfad ist so über das Internet raten- und durchprobierbar.
|
|
- **Maßnahme:** Rate-Limit je IP und Konto, längeres Mindestpasswort.
|
|
|
|
### FLAW-10: `sharing_mode` wird nicht durchgesetzt
|
|
- **Fundstellen:** `Store.GetDiscoverableUsers`, `Store.GetCollaborators`; Share-Handler in `server.go`.
|
|
- **Problem:**
|
|
1. Der Modus beeinflusst nur `GetDiscoverableUsers`. In `teams_only` liefert die Funktion trotzdem **alle** Benutzer (`if … mode == "open" || mode == "teams_only" { return GetAllUsers() }`).
|
|
2. `GET /collaborators` ignoriert den Modus komplett und liefert alle aktiven Konten mit E-Mail-Adresse.
|
|
3. Keine der Share-Funktionen prüft den Modus. `strict_private` und `teams_only` verhindern Freigaben nicht, wenn man die Ziel-ID kennt.
|
|
- **Maßnahme:** Eine zentrale Funktion `canShareWith(owner, target)` und in allen Share-Handlern verwenden; Verzeichnisse darauf einschränken.
|
|
|
|
### FLAW-11: Block-Upsert prüft das Dokument nicht
|
|
- **Fundstelle:** `Store.PatchDocumentBlocks`, `INSERT INTO blocks … ON CONFLICT(id) DO UPDATE`.
|
|
- **Problem:** `blocks.id` ist global eindeutig. Existiert die ID bereits in einem **anderen** Dokument, überschreibt der Konflikt-Zweig dessen Inhalt, ohne `document_id` zu prüfen. Voraussetzung: Schreibrecht im eigenen Dokument und Kenntnis einer fremden Block-ID.
|
|
- **Maßnahme:** `DO UPDATE … WHERE blocks.document_id = excluded.document_id`; fremde IDs ablehnen.
|
|
|
|
### FLAW-12: Alarm-Logik
|
|
- **Fundstellen:** `startAlarmTicker`, `GetDueTasksForAlarm`, `UpdateThoughtDetails`, `UserSettingsView`.
|
|
- **Probleme:**
|
|
- `notify_before_minutes` (Einstellungen: „15 Minuten vorher“ usw.) wird nirgends gelesen. Fälligkeiten haben nur ein Datum (`YYYY-MM-DD`), eine Vorlaufzeit ist damit nicht abbildbar.
|
|
- Verglichen wird mit `time.Now()` in der Zeitzone des Containers (ohne `TZ` also UTC).
|
|
- `alarm_sent` wird nie zurückgesetzt. Wird die Fälligkeit nach einem Alarm verschoben, gibt es keinen neuen.
|
|
- Schlagen alle Kanäle fehl, wird jede Minute erneut gesendet (kein Backoff, keine Dead-Letter-Queue).
|
|
- **Maßnahme:** Fälligkeit mit Uhrzeit speichern, `alarm_sent` bei Änderung von `due_date` zurücksetzen, Wiederholungen begrenzen.
|
|
|
|
### FLAW-13: Migrationen
|
|
- **Fundstellen:** `Store.migrate` in [`internal/storage/sqlite.go`](../internal/storage/sqlite.go), [`migrations/001_init.sql`](../migrations/001_init.sql).
|
|
- **Probleme:**
|
|
- Es gibt keine Versionstabelle. Alle `ALTER TABLE`-Anweisungen laufen bei jedem Start erneut; Fehler werden pauschal ignoriert (`_, _ = s.db.Exec(q)`), echte Fehler fallen nicht auf.
|
|
- Mehrere `UPDATE`/`DROP TABLE`-Bereinigungen laufen bei jedem Start.
|
|
- `migrations/001_init.sql` wird nur in das Image kopiert, aber nirgends gelesen, und ist gegenüber dem tatsächlichen Schema veraltet.
|
|
- **Maßnahme:** `schema_migrations`-Tabelle mit nummerierten, einmalig ausgeführten Schritten; die SQL-Datei entweder einbetten oder entfernen.
|
|
|
|
### FLAW-14: Verschluckte Fehler
|
|
- **Fundstellen:** u. a. `PatchDocumentBlocks` (Upserts/Deletes), `AddTaskListShare`, `RemoveTaskListShare`, viele `_, _ = s.db.Exec`.
|
|
- **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)*
|
|
- **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.
|
|
|
|
### FLAW-16: Ersteinrichtung und Demo-Modus
|
|
- **Fundstellen:** `handleFirstBootSetup`, `Store.EnsureDemoAccounts`, `config.Load` (`MINDA_DEMO`, `MINDSIFT_DEMO`, `DEMO_MODE`).
|
|
- **Probleme:** Solange kein Konto existiert, kann jeder `POST /auth/setup` aufrufen und wird Administrator. Mit gesetztem Demo-Flag entstehen Konten `admin/admin` und `user/user`; versehentlich auf einer öffentlichen Instanz aktiv wäre das eine Komplettübernahme.
|
|
- **Maßnahme:** Setup per einmaligem Token aus dem Server-Log absichern; Demo-Modus nur zusammen mit expliziter Warnung/Bestätigung.
|
|
|
|
### FLAW-17: Schreibgeschützte Dokumentfreigaben
|
|
- **Fundstellen:** Migration in `Store.migrate` (`UPDATE doc_shares SET permission = 'read' …`, ebenso `doc_team_shares`, `workspace_folder_shares`), `PatchDocumentBlocks`/`UpdateDocument` (nur Eigentümer).
|
|
- **Hinweis:** Das ist offenbar gewollt: Nur der Eigentümer schreibt in ein Dokument, Empfänger lesen. Es gibt daher **kein** kollaboratives Bearbeiten. Das Schema (`CHECK(permission IN ('read','write'))`), die Zweige für `write` im Code und frühere Dokumentation suggerieren etwas anderes. Es gibt weder Sperren noch Konfliktauflösung, noch Cursor-Presence.
|
|
- **Maßnahme:** Entweder Schreibfreigaben mit Konfliktbehandlung ergänzen oder den toten Code und das Schema bereinigen.
|
|
|
|
### FLAW-18: Suche
|
|
- **Fundstelle:** `Store.SearchAll`.
|
|
- **Probleme:** Durchsucht nur eigene Gedanken (30) und eigene Dokumente (20); geteilte Dokumente, Aufgaben aus geteilten Listen, Einkaufslisten und Blöcke bleiben außen vor. `LIKE '%…%'` ohne Escaping von `%`/`_`, keine Indizes, keine Relevanz.
|
|
- **Maßnahme:** FTS5-Index über Blöcke/Gedanken; Berechtigungen mitberücksichtigen.
|
|
|
|
### FLAW-19: Keine automatisierten Frontend-Prüfungen
|
|
- **Fundstelle:** [`frontend/package.json`](../frontend/package.json): `"lint": "echo 'Linting passed'"`, `"test:run": "echo 'Frontend checks passed'"`.
|
|
- **Problem:** Die Skripte melden immer Erfolg. Die CI (`.gitea/workflows/build-and-publish.yaml`) prüft daher nur Go-Tests und den Vite-Build. Regressionen im UI (z. B. Berechtigungsanzeigen) fallen nicht auf.
|
|
- **Maßnahme:** `tsc --noEmit` und ESLint als echte Skripte, Komponententests für kritische Pfade.
|
|
|
|
### FLAW-20: Dateigröße und Duplikate
|
|
- `internal/storage/sqlite.go` (≈ 5.600 Zeilen), `internal/api/server.go` (≈ 3.000), `WorkspaceView.tsx` (≈ 2.300), `TasksView.tsx` (≈ 2.500), `api.ts` (≈ 1.500).
|
|
- Die Zugriffsabfrage für Aufgabenlisten steht dreimal fast identisch im Code (`accessibleTaskListIDsQuery`, `GetTaskLists`, `GetTaskListByID`). Dokumentzugriff ist ähnlich verteilt. Änderungen an Berechtigungen müssen an mehreren Stellen erfolgen und werden leicht vergessen.
|
|
- **Maßnahme:** In Teilmodule zerlegen (Auth, Dokumente, Aufgaben, Listen, Teams), Zugriffsprüfung zentralisieren.
|
|
|
|
### FLAW-21: Lizenzdatei fehlt
|
|
- README und Badge nennen die MIT-Lizenz und verlinken auf `LICENSE`; die Datei existiert nicht im Repository.
|
|
|
|
### FLAW-22: Netzwerk-Details
|
|
- `authenticateUserFromReq` akzeptiert das Token als `?token=` (nötig für `WebSocket`/`EventSource`, die keine Header setzen). Tokens landen dadurch in Proxy- und Zugriffslogs.
|
|
- `AllowedOrigins: ["*"]` (Auth läuft über `Authorization`-Header, nicht über Cookies, das Risiko ist daher begrenzt) und `CheckOrigin` liefert immer `true`.
|
|
- Ein einmal verbundener WebSocket/SSE-Stream wird bei Logout oder Token-Widerruf nicht getrennt.
|
|
- **Maßnahme:** Kurzlebiges, einmaliges Stream-Ticket statt Langzeit-JWT in der URL; Origin gegen `BASE_URL` prüfen.
|
|
|
|
### FLAW-23: SQLite-Schreibengpass
|
|
- Eine SQLite-Datei (WAL, `busy_timeout`, bis zu 8 offene Verbindungen) erlaubt nur einen gleichzeitigen Schreiber. Für Familien und kleine Teams unkritisch; bei vielen aktiven Editoren sind `SQLITE_BUSY`-Fehler möglich, die wegen FLAW-14 stumm bleiben.
|
|
|
|
### FLAW-24: Organisations-Endpunkte ohne Berechtigungsprüfung
|
|
- **Fundstellen:** `handleDeleteOrganization`, `handleGetOrgTeams`, `handleCreateOrgTeam` (und `Store.DeleteOrganization`) in `server.go` bzw. `sqlite.go`.
|
|
- **Problem:** Diese Handler prüfen weder Eigentümer noch Mitgliedschaft. Jeder angemeldete Nutzer kann mit bekannter ID eine fremde Organisation löschen, deren Gruppen auflisten oder dort Gruppen anlegen. Gruppenbezogene Endpunkte (`/teams/{id}…`) prüfen dagegen korrekt auf Gruppen-Admin. Das Datenmodell kennt Organisationen nur noch rudimentär; im UI werden Gruppen direkt über `POST /teams` angelegt (`orgId` Standard `default`).
|
|
- **Maßnahme:** Organisationen entweder konsequent absichern (Eigentümer/Server-Admin) oder entfernen.
|
|
|
|
---
|
|
|
|
## Aus der früheren Fassung entfernt
|
|
|
|
Die frühere `flaws.md` führte als „gelöst“, was im Code nicht existiert: In-Memory-DocRooms, `clientId`-Echo-Schutz, Cursor-Presence, 1,5-s-Debounce-Flush, `internal/ws/docroom.go`, `frontend/src/lib/useDocumentSync.ts` und Yjs/CRDT. Der Code enthält nur noch `internal/ws/hub.go` (Räume, Benutzer-Kanäle, SSE-Fallback). Die CRDT-Tabellen werden beim Start gelöscht (`DROP TABLE document_crdt_*`). Dokumente werden per REST gespeichert (`PATCH /documents/{id}/blocks`) und als `DOC_BLOCKS_PATCHED` verteilt. Die frühere Aussage zur Token-Versionierung (`token_version`, `revoked_tokens`) trifft dagegen zu und ist in [sharing-and-permissions.md](sharing-and-permissions.md) und [architecture.md](architecture.md) beschrieben.
|