12 KiB
12 KiB
Watcher - Probleme und TODO-Liste
Stand: 2025-11-19 Analyse: Automatische Code-Review
🔴 KRITISCHE PROBLEME (Sofort beheben!)
1. Schwerwiegender Bug: Passwort überschreibt Username
- Datei:
Watcher/Controllers/UserController.cs:111 - Problem:
user.Username = BCrypt.Net.BCrypt.HashPassword(model.NewPassword); - Auswirkung: Beim Passwort-Ändern wird der Username zerstört, Account wird unbrauchbar
- Fix: Muss
user.Password = ...sein - Status: ❌ Offen
- Priorität: KRITISCH
2. Path Traversal Schwachstelle
- Datei:
Watcher/Controllers/DownloadController.cs:24-35 - Problem:
- Keine Validierung gegen
../Attacken - Leerer String als Extension erlaubt
- Kommentar im Code: "TODO: aktuelles "" für Binaries ist das absolute Gegenteil von sicher"
- Keine Validierung gegen
- Auswirkung: Angreifer könnten beliebige Dateien herunterladen
- Status: ❌ Offen
- Priorität: KRITISCH
3. Ungeschützte Monitoring-Endpoints
- Dateien:
Watcher/Controllers/MonitoringController.cs:112,155,173,237,365Watcher/Controllers/HeartbeatController.cs:31Watcher/Controllers/ApiController.cs:26
- Problem: KEINE Authentifizierung
- Auswirkung: Jeder kann:
- Fake Server registrieren
- Monitoring-Daten manipulieren
- Alle Server-Informationen abrufen
- Heartbeats senden
- Fix: API-Key oder Token-basierte Authentifizierung implementieren
- Status: ❌ Offen
- Priorität: KRITISCH
4. Command Injection
- Datei:
Watcher/Controllers/DatabaseController.cs:176-184 - Problem: Unvalidierter
fileNameinsqlite3Prozess-Aufruf - Auswirkung: Potenzielle Command Injection beim Database Restore
- Fix: Input-Validierung und Sanitization
- Status: ❌ Offen
- Priorität: KRITISCH
🟠 HOHE PRIORITÄT
5. HttpClient Resource Leak
- Datei:
Watcher/Services/UpdateCheckService.cs:24 - Problem:
new HttpClient()im Singleton-Konstruktor - Auswirkung: Socket Exhaustion bei vielen Requests möglich
- Fix:
IHttpClientFactoryverwenden - Status: ❌ Offen
- Priorität: HOCH
6. Default Credentials in Production
- Datei:
Watcher/Program.cs:127-137 - Problem: Standard-User "admin/changeme" wird immer erstellt
- Auswirkung: Bekannte Credentials, Sicherheitsrisiko
- Fix:
- Beim ersten Start Passwort abfragen
- Oder aus Environment Variable lesen
- Oder zufälliges Passwort generieren und loggen
- Status: ❌ Offen
- Priorität: HOCH
7. Race Conditions in Singleton Stores
- Dateien:
Watcher/Services/DashboardStore.csWatcher/Services/UpdateCheckStore.cs
- Problem: Properties werden ohne Thread-Synchronisation von Background Services geschrieben und von Controllern gelesen
- Fix: Lock-Mechanismus oder Concurrent Collections verwenden
- Status: ❌ Offen
- Priorität: HOCH
8. N+1 Query Problem
- Datei:
Watcher/Controllers/ServerController.cs:206-212 - Problem:
SaveChangesAsync()in foreach-Loop - Code:
foreach (var server in servers) { server.IsOnline = (DateTime.UtcNow - server.LastSeen).TotalSeconds <= 120; await _context.SaveChangesAsync(); // In Loop! } - Auswirkung: Ineffizient, potenzielle Database Locks
- Fix: Alle Änderungen sammeln, einmal am Ende speichern
- Status: ❌ Offen
- Priorität: HOCH
9. Synchrone DB-Abfragen in async Methoden
- Datei:
Watcher/Controllers/UserController.cs:28,53,73,101 - Problem:
FirstOrDefault()stattFirstOrDefaultAsync() - Auswirkung: Thread-Pool wird blockiert, reduziert Skalierbarkeit
- Fix: Async-Varianten verwenden
- Status: ❌ Offen
- Priorität: HOCH
10. Ineffiziente Database Queries
- Datei:
Watcher/Controllers/HomeController.cs:77-78 - Problem:
ToList()lädt ALLE Daten vor Filterung - Code:
var servers = _context.Servers.ToList(); var containers = _context.Containers.ToList(); - Auswirkung: Memory-Probleme bei vielen Datensätzen
- Fix: Filter in LINQ-Query vor
ToList() - Status: ❌ Offen
- Priorität: HOCH
11. Database Connections nicht immer mit using
- Datei:
Watcher/Services/DatabaseCheck.cs:33-59 - Problem: Connection wird manuell geschlossen, bei Exception könnte sie offen bleiben
- Fix:
usingStatement verwenden - Status: ❌ Offen
- Priorität: HOCH
🟡 MITTLERE PRIORITÄT
Code-Qualität
12. Generische catch-Blöcke ohne Exception-Logging
- Dateien:
Watcher/Services/NetworkCheck.cs:47Watcher/Controllers/MonitoringController.cs:222-228
- Problem: Leere catch-Blöcke verschlucken alle Exceptions
- Fix: Spezifischen Exception-Typ verwenden und Exception loggen
- Status: ❌ Offen
13. Code-Duplikation in UserController
- Datei:
Watcher/Controllers/UserController.cs:64-90 und 93-119 - Problem: Zwei fast identische Methoden
Edit()undUserSettings() - Fix: Gemeinsame Logik extrahieren
- Status: ❌ Offen
14. Sehr große Controller-Methode
- Datei:
Watcher/Controllers/MonitoringController.cs:237-362 - Problem: ServiceDetection mit 125 Zeilen, viele Verantwortlichkeiten
- Fix: In kleinere Methoden aufteilen, Service-Klasse extrahieren
- Status: ❌ Offen
15. Magic Numbers
- Dateien:
Watcher/Controllers/ServerController.cs:210-120Sekunden hardcodiertWatcher/Services/NetworkCheck.cs:35-"8.8.8.8"hardcodiertWatcher/Services/NetworkCheck.cs:40-3000ms Timeout hardcodiert
- Fix: Als Konstanten oder Konfiguration auslagern
- Status: ❌ Offen
16. Duplizierte Endpoint-Logik
- Datei:
Watcher/Controllers/MonitoringController.cs:421-481 - Problem: GetCpuUsageData, GetRamUsageData, GetGpuUsageData sind nahezu identisch
- Fix: Generische Methode mit Parameter
- Status: ❌ Offen
17. Synchrone I/O in async Methoden
- Datei:
Watcher/Controllers/DatabaseController.cs:46,69,137,149,151,163,174 - Problem:
File.ReadAllBytes(),File.Exists()etc. sind synchron - Fix: Async File I/O verwenden
- Status: ❌ Offen
Sicherheit
18. Kein Brute-Force Schutz
- Datei:
Watcher/Controllers/AuthController.cs:36-72 - Problem: Login hat keine Rate-Limiting, Account Lockout oder CAPTCHA
- Fix: Rate-Limiting Middleware oder Account Lockout implementieren
- Status: ❌ Offen
19. SQL Injection Potential
- Datei:
Watcher/Controllers/DatabaseController.cs:91 - Problem: String-Interpolation für SQL-Query:
$"SELECT * FROM {tableName}" - Hinweis: tableName kommt aus sqlite_master, aber schlechte Praxis
- Fix: Parametrisierte Queries verwenden
- Status: ❌ Offen
20. Fehlende IP-Adress-Validierung
- Datei:
Watcher/Controllers/MonitoringController.cs:129,159,262 - Problem: IP-Adressen werden nicht validiert
- Fix: Regex oder IPAddress.TryParse verwenden
- Status: ❌ Offen
Architektur
21. Kein Repository Pattern
- Problem: DbContext direkt in allen Controllern
- Auswirkung: Schwer testbar, keine Abstraktionsschicht
- Fix: Repository Pattern oder CQRS implementieren
- Status: ❌ Offen
22. Hardcodierte Connection Strings
- Dateien:
Watcher/Services/DatabaseCheck.cs:33Watcher/Controllers/SystemController.cs:37
- Problem:
"Data Source=./persistence/watcher.db"hardcodiert - Fix: Aus Configuration/appsettings.json lesen
- Status: ❌ Offen
23. Keine Dependency Inversion
- Problem: Keine Interfaces für Repositories, direkte DbContext-Abhängigkeit
- Fix: Interface-basierte Dependency Injection
- Status: ❌ Offen
Performanz
24. Fehlende Database Indizes
- Problem: Keine expliziten Indizes für häufig abgefragte Felder
- Beispiele: Server.IPAddress, Container.ContainerId
- Fix: Indizes in DbContext konfigurieren
- Status: ❌ Offen
25. Keine Caching-Strategie
- Problem: Metrics werden jedes Mal neu aus DB geladen
- Fix: In-Memory Cache für Current Metrics implementieren
- Status: ❌ Offen
26. Mehrfache SaveChanges in Loop
- Datei:
Watcher/Controllers/MonitoringController.cs:327,349,359 - Problem: SaveChanges wird in Loop aufgerufen
- Fix: Batch-Updates am Ende
- Status: ❌ Offen
🔵 NIEDRIGE PRIORITÄT / WARTBARKEIT
27. Keine Unit Tests
- Problem: Keine Test-Dateien in der Codebase
- Auswirkung: Keine automatisierte Qualitätssicherung
- Fix: xUnit/NUnit Test-Suite aufbauen
- Status: ❌ Offen
28. Fehlende Integration Tests
- Problem: Kritische Flows wie Login, Monitoring-Daten-Empfang sind ungetestet
- Fix: Integration Tests schreiben
- Status: ❌ Offen
29. Unvollständige TODOs im Code
- Dateien:
MonitoringController.cs:287- "TODO entfernen wenn fertig getestet"MonitoringController.cs:352- "//Todo" (Metrics für Container entfernen)MonitoringController.cs:525- "//TODO" (CalculateMegabit korrekt implementieren)DownloadController.cs:28- "TODO: aktuelles "" für Binaries ist das absolute Gegenteil von sicher"DatabaseCheck.cs:61- "// TODO: LogEvent erstellen"SystemMangement.cs:20- "// Todo: Umstellen auf einmal alle 24h"
- Fix: TODOs abarbeiten oder entfernen
- Status: ❌ Offen
30. Fehlende XML-Dokumentation
- Problem: Keine
///Kommentare für öffentliche APIs - Fix: XML-Kommentare für Controllers und Services hinzufügen
- Status: ❌ Offen
31. Console.WriteLine in Production
- Dateien:
Program.cs:121,125,137,141MonitoringController.cs:288-293
- Problem: Console.WriteLine statt strukturiertes Logging
- Fix: ILogger verwenden
- Status: ❌ Offen
32. Inkonsistente Namenskonventionen
- Datei:
MonitoringController.cs:53-81 - Problem: MetricDto Properties in UPPERCASE (CPU_Load, RAM_Size)
- Fix: PascalCase verwenden (CpuLoad, RamSize)
- Status: ❌ Offen
📊 STATISTIK
- Kritische Probleme: 4
- Hohe Priorität: 7
- Mittlere Priorität: 18
- Niedrige Priorität: 6
- Gesamt: 35 Probleme
🎯 EMPFOHLENE REIHENFOLGE
Sprint 1 - Kritische Sicherheit (Sofort)
- Bug in UserController.cs Zeile 111 beheben
- Authentifizierung für Monitoring-Endpoints
- Path Traversal Validierung in DownloadController
- Command Injection in DatabaseController beheben
Sprint 2 - Stabilität & Performance (Woche 1-2)
- HttpClientFactory implementieren
- N+1 Query Probleme beheben
- Async/Await korrekt verwenden
- Database Connection Management
Sprint 3 - Sicherheit & Konfiguration (Woche 3-4)
- Default Credentials entfernen/anpassbar machen
- Input-Validierung verbessern
- Brute-Force Protection
- Thread-Safety in Stores
Sprint 4 - Architektur (Monat 2)
- Repository Pattern einführen
- Code-Duplikation eliminieren
- Große Methoden refactoren
- Magic Numbers entfernen
Sprint 5 - Qualität (Monat 3)
- Test-Suite aufbauen (Unit + Integration)
- Statische Code-Analyse einrichten
- TODOs abarbeiten
- Dokumentation verbessern
🔧 TOOLS & HILFSMITTEL
Empfohlene Tools
- Statische Analyse: SonarQube, Roslyn Analyzers
- Security Scanning: OWASP Dependency Check, Snyk
- Testing: xUnit, Moq, FluentAssertions
- Code Coverage: Coverlet, ReportGenerator
- Performance: BenchmarkDotNet, MiniProfiler
Code-Qualität Metriken
- Zyklomatische Komplexität: < 10 pro Methode
- Zeilen pro Methode: < 50
- Test Coverage: > 80%
- Code Duplication: < 5%
Letzte Aktualisierung: 2025-11-19 Erstellt durch: Automatische Code-Analyse