cannamanage/docs/security-code-review-final.md

CannaManage — Final Security & Code Review

Datum: 2026-06-15 Reviewer: Lumen (Code mode, SonarQube + manual deep-scan) Scope: Vollständiges Backend (Sprints 1–10) — ca. 29.000 LOC Java Production, 3.600 LOC Tests, 26.000 LOC Frontend TypeScript Branch: master BigMind Session: 43f1d5c3-4805-42a6-8408-145784f6603e

---

🎯 Verdict: ⚠️ CONDITIONAL PASS

Production-Deployment ist NICHT freigegeben, bis die 4 BLOCKER unten behoben sind. Das Fundament ist solide — Architektur, Tenant-Isolation, Compliance-Layer, Spring-Boot-CVE-Hardening, XXE-Schutz, BCrypt+SHA-256, GoBD-Append-Only-Ledger sind allesamt korrekt umgesetzt. Aber 4 konkrete, vermeidbare Schwachstellen blockieren den Go-Live.

Kategorie	Score	Status
Architektur & Patterns	9/10	✅ Sehr gut
Authentication (JWT/BCrypt)	7/10	⚠️ Dev-Fallback noch da
Authorization (Roles + Permissions)	6/10	❌ DocumentController ungeschützt
Tenant-Isolation	8/10	⚠️ Lücke bei Download-by-UUID
Input Validation	7/10	⚠️ Path Traversal in DocumentService
Data Protection (DSGVO/KCanG/GoBD)	10/10	✅ Vollständig
Dependency Hygiene	9/10	✅ Snyk-Overrides 2026-06-12
Error Handling	9/10	✅ RFC 9457, keine Info-Disclosure
Test Coverage	4/10	❌ 20 Tests für 29K LOC zu wenig
Logging Quality	10/10	✅ Keine Concat, alles parameterized

---

1. SonarQube SAST — Findings Summary

7 Dateien gescannt mit analyze_code_snippet. Nur echte Findings (S1598 Pfad-Rauschen ignoriert):

Datei	Rule	Schweregrad	Befund
DocumentService.uploadDocument()	S2184	MAJOR	file.getSize() > MAX_FILE_SIZE — int overflow risk (cast (long) fehlt)
DocumentService.uploadDocument()	S1075	MAJOR	Hard-coded path delimiter "/"
DocumentService	S1068	MAJOR	Unbenutztes Feld auditService (Dead Code)
Camt053Parser	S112	MAJOR	Generisches throw new Exception(...)
Camt053Parser	S3252	MINOR	Statischer Zugriff via Instanz statt Class
AuthService.sha256()	S112	MAJOR	Generisches RuntimeException
AuthService	S1192	MINOR	String "Invalid credentials" 3× dupliziert
SecurityConfig	S1068	MAJOR	(Snippet-Artefakt; im Original wird portalUserDetailsService genutzt — kein echtes Finding)

JwtService und BankImportService waren bis auf S1598-Rauschen sauber.

---

2. Manual 16-Item Security Checklist

#	Regel	Prüfpunkt	Ergebnis	Begründung
1	SEC-001..004	Keine hartkodierten Credentials in Production-Code	⚠️ Teilweise	JWT-Dev-Secret in application.properties:8 — Production-Profile nutzt Env-Var, aber Default-Profile hat Fallback
2	SEC-005	Credentials via @Value/env	✅	Production-Properties durchgängig ${...}; Stripe-Keys, DB-Credentials, JWT-Secret korrekt
3	SEC-011	Keine SQL-Injection	✅	Null nativeQuery=true im gesamten Codebase, null String-Concat in @Query. JPA/HQL nur mit benannten Parametern
4	SEC-012	Kein Path Traversal	❌	DocumentService.uploadDocument() nutzt file.getOriginalFilename() unsaniert. BankImportService macht es richtig mit FilenameUtils.getName() — Inkonsistenz
5	SEC-016	Input-Validierung	✅	DTOs mit @Valid, MethodArgumentNotValidException zentral behandelt, File-Type-Whitelist auf DocumentService
6	SEC-018	Keine Info-Disclosure in Fehlern	✅	GlobalExceptionHandler loggt intern, gibt nur "An unexpected error occurred" zurück. Production-Properties: server.error.include-stacktrace=never
7	SEC-033	PII-Verschlüsselung (at rest)	✅	PostgreSQL TDE-fähig; Passwords BCrypt, Refresh-Tokens SHA-256 hashed. Keine Klartext-PII-Spalten neu eingeführt
8	SEC-035	Kein PII in LLM-Verarbeitung	✅ N/A	Keine LLM-Integration im Code
9	SEC-040	Keine sensiblen Daten in Logs	✅	Null Treffer für log.x(".." + ...) Concat-Antipattern. Alle Logs parameterized. Stichproben: keine Passwörter, JWT-Tokens, IBANs vollständig geloggt
10	SEC-055	Keine generierten Quellen geändert	✅ N/A	Kein src.gen/ in diesem Projekt
11	—	Tenant-Isolation aktiv	⚠️	Hibernate @Filter("tenantFilter") auf AbstractTenantEntity + TenantFilterAspect — solide. ABER: bei Lookups per Raw-UUID ohne clubId-Parameter greift Filter nur, wenn Aspect vorher den Tenant gesetzt hat. Defense-in-depth fehlt bei DocumentService.download
12	—	JWT-Implementierung sicher	⚠️	HS256 mit 32-Byte-Base64-Key ✓, jti claim ✓, Access 1h / Refresh 30d ✓ — aber siehe Punkt 1 (Dev-Fallback)
13	—	CSRF / Session-Management	✅	Member-Portal: CookieCsrfTokenRepository + Session-Fixation-Schutz (maximumSessions(1)). API: stateless JWT, CSRF korrekt disabled
14	—	CORS restriktiv	❌	SecurityConfig.corsConfigurationSource() hardcoded auf localhost:3000 + frontend:3000. Production-Domains nicht konfigurierbar. setAllowedHeaders("*") zu permissiv
15	—	Rate-Limiting auf kritischen Endpoints	❌	Nur AuthorityExportService hat Rate-Limit (1/h pro Tenant). Kein Rate-Limit auf /api/v1/auth/login — Brute-Force-Vektor
16	SEC-064	Keine Secrets im Source (DataRake)	⚠️	Dev-Default-JWT in application.properties würde DataRake triggern (Base64 ≥ 6 Zeichen, in nicht-Test-Datei). Sonst sauber

---

3. Vulnerability Summary — Sprint 9 Findings Status

#	Sprint-9-Finding	Erwartet	Tatsächlicher Status
1	Path Traversal in DocumentService (Medium)	FIXED	❌ NICHT FIXED — Code unverändert, file.getOriginalFilename() unsaniert
2	JWT Dev-Secret Fallback (Medium)	FIXED	❌ NICHT FIXED — application.properties:8 hat noch Default-Wert
3	Spring Boot CVE (Medium)	FIXED	✅ FIXED — pom.xml Override 2026-06-12: Spring 7.0.8, Tomcat 11.0.22, Postgres 42.7.11, "fixes 10 HIGH + 18 MEDIUM Snyk SCA findings"
4	CORS Hardcoded Localhost (Low)	FIXED	❌ NICHT FIXED — siehe Item 14
5	Login Rate-Limit (Low)	FIXED	❌ NICHT FIXED — siehe Item 15

🚨 Neu entdeckte Findings

BLOCKER #1 — IDOR / Horizontal Privilege Escalation in DocumentController

@GetMapping("/documents/{id}/download")
public ResponseEntity<byte[]> downloadDocument(@PathVariable UUID id) throws IOException {
    Document doc = documentService.getDocument(id);
    byte[] content = documentService.downloadDocument(id);
    ...
}

• Kein clubId-Parameter, keine @PreAuthorize, kein Permission-Check, keine Tenant-Verifikation in der Service-Methode
• documentService.downloadDocument(id) lädt das Document direkt per Raw-UUID
• documentRepository.findById(documentId) greift zwar Hibernate-Filter — aber nur wenn TenantContext.getCurrentTenant() korrekt aus dem JWT gesetzt ist. Da der Endpoint im SecurityConfig nicht in einem spezifischen role-Matcher gelistet ist, fällt er unter .anyRequest().authenticated() — JEDER authentifizierte User (MEMBER, STAFF eines anderen Clubs) kann mit einer geratenen/geleakten UUID Dokumente herunterladen
• Gleiches Muster bei @DeleteMapping("/documents/{id}"): Auch hier kein Permission-Check

Severity: HIGH (CVSS ~7.5 — N/L/N/R/U/N/H/N — Confidentiality breach across tenants)

BLOCKER #2 — Path Traversal in DocumentService.uploadDocument()

String filename = file.getOriginalFilename() != null ? file.getOriginalFilename() : "document";
String storagePath = clubId + "/" + documentId + "_" + filename;
Path fullPath = Paths.get(UPLOAD_BASE, storagePath);

• Filename wie ../../etc/passwd.pdf schreibt außerhalb von UPLOAD_BASE
• BankImportService verwendet bereits korrekt FilenameUtils.getName() — Fix-Muster vorhanden, nur nicht angewendet

Severity: HIGH (CVSS ~7.5 — Arbitrary file write)

BLOCKER #3 — JWT Dev-Secret Fallback in application.properties:8

cannamanage.security.jwt.secret=Y2FubmFtYW5hZ2Utand0LXNlY3JldC1rZXktZm9yLWRldmVsb3BtZW50LW9ubHktMzI=

• Wenn Production-Deployment vergisst, SPRING_PROFILES_ACTIVE=production zu setzen, wird der bekannte Dev-Secret verwendet
• Angreifer kann beliebige JWTs signieren und sich als ADMIN ausgeben

Severity: HIGH (im Worst Case CRITICAL) — Misconfiguration-Risiko

BLOCKER #4 — Kein DocumentController-Matcher in SecurityConfig

Im SecurityConfig.apiSecurityFilterChain() fehlen Matcher für /api/v1/documents/**. Defense-in-depth-Schicht entfällt. Sollte mindestens .hasAnyRole("ADMIN", "STAFF") für POST/DELETE und .hasAnyRole("ADMIN", "STAFF", "MEMBER") für GET sein, kombiniert mit Service-seitiger Tenant-Prüfung.

Weitere mittlere Findings

#	Befund	Severity
M1	DocumentService.downloadDocument() / deleteDocument() haben kein AuditService-Logging (Feld ist sogar als ungenutzt markiert) — DSGVO Art. 30 verlangt Verarbeitungsverzeichnis für Document-Access	MEDIUM
M2	CORS setAllowedHeaders("*") + setAllowCredentials(true) ist seit CVE-2017-* zwar kein direkter Bug, aber Best-Practice ist explizite Header-Liste	LOW
M3	AuthService.sha256() wirft generisches RuntimeException bei NoSuchAlgorithmException — unreachable, aber sollte spezifische Custom-Exception sein	LOW
M4	Refresh-Token-Hash in User-Entity: einzelner Slot pro User → kein Multi-Device-Support; Logout auf einem Device killt Session auf anderen. Aktuell akzeptabel, dokumentieren	INFO

---

4. Code Quality Assessment

✅ Stärken

Aspekt	Bewertung
Pattern-Konsistenz	Sehr hoch — Service+Repository+Controller-Trennung sauber, AbstractTenantEntity als gemeinsame Basis durchgezogen
Transaction Boundaries	Korrekt — @Transactional auf Service-Methoden, readOnly=true wo angebracht (RetentionService, DsgvoService)
N+1 Query Risk	Niedrig — PaymentMatchingService.buildContexts() preloadet bewusst in 3 Bulk-Queries
Null Safety	Hoch — Optional wird konsistent verwendet, .orElseThrow() mit aussagekräftigen Messages
Error Handling	RFC 9457 application/problem+json über GlobalExceptionHandler, 13 spezifische @ExceptionHandler
Audit Logging	63 Aufrufe von auditService.log(...) verteilt — sehr gute Abdeckung
Compliance Domain	GoBD §147 (Append-Only Ledger, kompensierende Buchungen), KCanG §22/§24, DSGVO Art. 15/17/30 vollständig implementiert
XML/XXE	Camt053Parser defensiv konfiguriert (DTD off, External Entities off, Entity Refs off) — vorbildlich
Streaming für große Exports	AuthorityExportService OutputStream-basiert, kein Heap-Risiko
Stripe Webhook	Signature-Validierung via Webhook.constructEvent(...) ✓

⚠️ Schwächen

Aspekt	Bewertung
Test Coverage	Kritisch niedrig — 20 Tests für 29.000 LOC Production. Verhältnis ~12% nach LOC. Domain-Modul (cannamanage-domain) hat 0 Tests trotz AbstractTenantEntity-Logik und Entity-Validierungen
Magic Strings	"Invalid credentials", "GELÖSCHT", "DOCUMENT_UPLOADED" — sollten Konstanten in enum/class sein
Dead Code	DocumentService.auditService ungenutzt; Sprint-Cleanup steht aus
Inkonsistente File-Sanitization	BankImport ✓, Document ✗ — gleiches Risiko, zwei verschiedene Lösungen
Cookie-Härtung explizit	Secure/HttpOnly/SameSite nicht in application.properties gesetzt; Spring-Defaults greifen, sollte aber explizit dokumentiert sein

Code-Smells nach SonarQube

• 4× S1192 (Duplicated String Literals) — kosmetisch
• 3× S112 (Generic Exception) — in Edge-Paths (sha256, XML-Parse)
• 1× S2184 (long cast missing) — file.getSize() ist bereits long, Konstante MAX_FILE_SIZE als int deklariert → potenziell int * 1024 * 1024 Overflow, falls jemand MAX_FILE_SIZE erhöht
• 1× S1075 (hard-coded "/") — gleichzeitig OS-Inkompatibilität

---

5. Test Coverage Analyse

Total Production LOC:  28.977
Total Test LOC:         3.609
Test Files:                20  (domain: 0, service: 9, api: 11)
Ratio:                  ~12% (LOC) / ~7 Tests pro 10K LOC

Branchenstandard für Finanz/Compliance-Software: 70–80% Coverage, mindestens 1 Test pro 100 LOC.

Kritische Lücken:

• cannamanage-domain — 0 Tests trotz AbstractTenantEntity, Enums mit Business-Logik, Entity-Validierung
• RetentionService.anonymizeExpiredMembers() — KCanG §24-Compliance, sollte voll getestet sein
• DsgvoService.exportUserData() — Art. 15 DSGVO, rechtlich kritisch
• PaymentMatchingService.scoreOne() — komplexer Algorithmus mit Schwellwerten
• Camt053Parser / Mt940Parser — externe Datenformate, perfekter Fall für umfangreiche Testdaten

---

6. Compliance Snapshot

Verordnung	Erfüllt?	Beleg
DSGVO Art. 15 (Auskunft)	✅	DsgvoService.exportUserData()
DSGVO Art. 17 (Löschung)	✅	DsgvoService.deleteUserData() — anonymisiert statt löscht (KCanG-konform)
DSGVO Art. 30 (Verarbeitungsverzeichnis)	⚠️	AuditService vorhanden, aber Document-Download nicht geloggt
DSGVO Art. 32 (Sicherheit der Verarbeitung)	⚠️	Blockiert durch IDOR-Findings
DSGVO Art. 33/34 (Breach Notification)	✅	Workflow dokumentiert, audit_event_log persistent
DSGVO Art. 35 (DPIA)	✅	Templates in Sprint-7-Plan
KCanG §22 (Mitgliederverzeichnis)	✅	Members-Modul vollständig
KCanG §24 (Aufbewahrung 5J)	✅	RetentionService Scheduled Job, anonymisiert nach 5 Jahren
AO §147 (GoBD 10J)	✅	Append-Only Ledger, voids = kompensierende Buchungen, immutable nach completion
PSD2 / IBAN-Handling	✅	Normalisierung, kein Klartext-Logging
Stripe PCI-DSS	✅	Stripe Hosted Checkout, kein Karten-Daten-Touch

---

7. Recommendations — Priorisiert

🚨 BLOCKER (vor Production-Deploy beheben — geschätzt 4–6h)

1. DocumentController: @PreAuthorize einbauen, clubId als Path/Param fordern, Service-seitig Tenant-Match prüfen:

   @GetMapping("/documents/{id}/download")
   @PreAuthorize("hasAnyRole('ADMIN','STAFF','MEMBER')")
   public ResponseEntity<byte[]> downloadDocument(@PathVariable UUID id, Principal principal) {
       byte[] content = documentService.downloadDocumentForTenant(id, getCurrentTenantFromPrincipal(principal));
       ...
   }
   

   Service muss doc.getTenantId().equals(currentTenantId) verifizieren.

2. DocumentService.uploadDocument(): Filename sanitization:

   String safeName = org.apache.commons.io.FilenameUtils.getName(file.getOriginalFilename());
   if (safeName == null || safeName.isBlank()) safeName = "document";
   

3. application.properties:8: Default-JWT-Secret entfernen, durch Fail-Fast ersetzen:

   cannamanage.security.jwt.secret=${CANNAMANAGE_SECURITY_JWT_SECRET}
   

   Plus JwtService Postconstruct-Validation: Mindestlänge 32 Bytes, sonst Startup-Failure.

4. SecurityConfig: /api/v1/documents/**-Matcher hinzufügen (Defense-in-Depth):

   .requestMatchers(HttpMethod.GET, "/api/v1/documents/**").hasAnyRole("ADMIN","STAFF","MEMBER")
   .requestMatchers("/api/v1/documents/**").hasAnyRole("ADMIN","STAFF")
   

🟡 SHOULD-FIX (innerhalb der nächsten 2 Wochen — geschätzt 8–12h)

5. CORS konfigurierbar machen über app.cors.allowed-origins Property
6. Rate-Limiting auf /api/v1/auth/login (z.B. Bucket4j, 10 req/min pro IP)
7. Audit-Logging für Document-Download/Delete-Operationen
8. MAX_FILE_SIZE als long deklarieren
9. Konstanten für duplizierte Strings (Constants.AUTH_INVALID_CREDENTIALS)
10. Custom Exceptions statt generischer RuntimeException/Exception

🟢 NICE-TO-HAVE (technische Schuld, abarbeiten nach Go-Live)

11. Test-Coverage auf 60% steigern — primär cannamanage-domain + RetentionService + DsgvoService + Parser
12. Dead-Code-Eliminierung (ungenutzte Felder)
13. Cookie-Security explizit in application-production.properties
14. Multi-Device-Refresh-Tokens (separates Table statt einzelner Hash)

---

8. Was wirklich gut ist (Anerkennung)

• GoBD-Append-Only-Ledger in FinanceService — Voids als kompensierende Buchungen, nicht als Updates. Lehrbuch-konform.
• XXE-Härtung in Camt053Parser — SUPPORT_DTD=false + IS_SUPPORTING_EXTERNAL_ENTITIES=false + IS_REPLACING_ENTITY_REFERENCES=false. Vorbildlich.
• Spring Boot 4.0.6 + Snyk Override 2026-06-12 — 10 HIGH + 18 MEDIUM Findings proaktiv eliminiert.
• PaymentMatching Weighted Scoring — 35/30/20/15-Verteilung mit Schwellwerten AUTO=90/SUGGEST=60 und Doppelzahlungs-Schutz. Sauberes Algorithmus-Design.
• Tenant-Filter über Hibernate @Filter + AspectJ — funktional korrekt, performant, idiomatic.
• RFC 9457 Problem Details — moderner Standard im GlobalExceptionHandler.
• 63 Audit-Log-Calls — solides Activity-Trail-Niveau.
• KCanG-konforme Anonymisierung statt Löschung — wichtiger Compliance-Nuance, korrekt umgesetzt.

---

9. Final Verdict

⚠️ CONDITIONAL PASS — Production-Freigabe nach Behebung der 4 BLOCKER (geschätzt 4–6 Stunden Entwicklungszeit).

Das Codebase ist insgesamt überraschend reif für 10 Sprints. Die Architektur ist solide, der Compliance-Layer ist außergewöhnlich gut, und die Dependency-Hygiene wurde aktiv gepflegt. Die 4 Blocker sind alle in einem Sprint-Tag fixbar — keine architektonischen Probleme, sondern konkrete vergessene Sicherheitsschichten beim documents-Modul + ein vergessener Cleanup-Schritt der Sprint-9-Findings.

Nach Fix der Blocker und Re-Verification: ✅ FULL PASS empfohlen.

Empfehlung für die nächste Iteration: Test-Coverage von 12% auf ≥ 60% ist die wichtigste mittelfristige Investition. Für eine Cannabis-Club-Management-Software mit Finanz- und Behörden-Reporting ist das aktuelle Niveau für einen produktiven Betrieb mit Echtkunden zu niedrig.

---

Reviewer-Signatur: Lumen (Claude-Opus-4.7), BigMind Session 43f1d5c3-4805-42a6-8408-145784f6603e, 2026-06-15 18:46 CEST