343 lines
12 KiB
Markdown
343 lines
12 KiB
Markdown
# 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"
|
|
- **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,365`
|
|
- `Watcher/Controllers/HeartbeatController.cs:31`
|
|
- `Watcher/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 `fileName` in `sqlite3` Prozess-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:** `IHttpClientFactory` verwenden
|
|
- **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.cs`
|
|
- `Watcher/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:**
|
|
```csharp
|
|
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()` statt `FirstOrDefaultAsync()`
|
|
- **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:**
|
|
```csharp
|
|
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:** `using` Statement 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:47`
|
|
- `Watcher/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()` und `UserSettings()`
|
|
- **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` - `120` Sekunden hardcodiert
|
|
- `Watcher/Services/NetworkCheck.cs:35` - `"8.8.8.8"` hardcodiert
|
|
- `Watcher/Services/NetworkCheck.cs:40` - `3000` ms 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:33`
|
|
- `Watcher/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,141`
|
|
- `MonitoringController.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)
|
|
1. Bug in UserController.cs Zeile 111 beheben
|
|
2. Authentifizierung für Monitoring-Endpoints
|
|
3. Path Traversal Validierung in DownloadController
|
|
4. Command Injection in DatabaseController beheben
|
|
|
|
### Sprint 2 - Stabilität & Performance (Woche 1-2)
|
|
5. HttpClientFactory implementieren
|
|
6. N+1 Query Probleme beheben
|
|
7. Async/Await korrekt verwenden
|
|
8. Database Connection Management
|
|
|
|
### Sprint 3 - Sicherheit & Konfiguration (Woche 3-4)
|
|
9. Default Credentials entfernen/anpassbar machen
|
|
10. Input-Validierung verbessern
|
|
11. Brute-Force Protection
|
|
12. Thread-Safety in Stores
|
|
|
|
### Sprint 4 - Architektur (Monat 2)
|
|
13. Repository Pattern einführen
|
|
14. Code-Duplikation eliminieren
|
|
15. Große Methoden refactoren
|
|
16. Magic Numbers entfernen
|
|
|
|
### Sprint 5 - Qualität (Monat 3)
|
|
17. Test-Suite aufbauen (Unit + Integration)
|
|
18. Statische Code-Analyse einrichten
|
|
19. TODOs abarbeiten
|
|
20. 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 |