Architecture/security/performance audit: prioritized findings (P0-P3)

[DeepDiver1975]

Architecture / Security / Performance Audit — Prioritized Findings

A principal review of the codebase covering performance bottlenecks, architectural drift / duplication, and security vulnerabilities. This is an Avalonia desktop + Android app using raw SQLite and an embedded Kestrel/SignalR sync host.

P0 — Critical

• Sync SQLite rewrite on every mutation — IncidentRepository.Save() does DELETE-all then INSERT-all across 15 tables synchronously on the UI thread (src/LageBuch.Persistence/IncidentRepository.cs:13, src/LageBuch.AppLogic/LocalIncidentSession.cs:256). Make async and/or diff-based.
• No TLS on sync channel — Kestrel host listens on plain HTTP, IPAddress.Any, port 5859 (src/LageBuch.Sync.Hosting/IncidentHost.cs:50, src/LageBuch.App/Services/IncidentHostController.cs:45). PIN + all incident data travel unencrypted.
• 4-digit PIN with no rate limiting — 10,000 combinations, no backoff/lockout (IncidentHost.cs:60, IncidentHostController.cs:41). Brute-forceable.

P1 — High

• File upload sync on UI thread — IncidentFileStore.SaveBytes + Save sync, called from LocalIncidentSession.AddFileAsync which returns Task.CompletedTask while doing sync work (IncidentFileStore.cs:29, LocalIncidentSession.cs:187).
• Full byte arrays in memory for sync file transfer — 25 MB file → base64 in JSON, all in memory, no streaming (src/LageBuch.Sync/RemoteIncidentSession.cs:221).
• PdfAttachmentMerger holds all PDFs in memory as byte[] then temp-files them (src/LageBuch.Documents/PdfAttachmentMerger.cs:17). Use Stream-based merging.
• MIME type mapping duplicated 3× with inconsistent defaults — ContentTypeFor (upper, octet-stream) vs MimeTypeOf (lower, */*) (FilesViewModel.cs:157, AndroidFileDialogService.cs:176). Extract shared helper.
• URL validation + opening duplicated 4× — LinksViewModel.cs:42, AboutViewModel.cs:53, both platform IFileDialogService (StorageProviderFileDialogService.cs:130, AndroidFileDialogService.cs:165).
• Missing CancellationToken on async methods — only RemoteIncidentSession.ConnectAsync/SendAsync take tokens.
• Self-asserted client identity — operator name rides on every SyncCommand, no server-side verification beyond PIN (SessionOperator.cs, SyncCommand.cs). Document as design decision.

P2 — Medium

• LocalIncidentSession / RemoteIncidentSession lockstep — ~25 parallel delegation methods, no generation/shared base (LocalIncidentSession.cs:120, RemoteIncidentSession.cs:153).
• DispatcherTimerTicker._subscribers not thread-safe — unsynchronized List<Action> (DispatcherTimerTicker.cs).
• Unbounded file cache on remote client — no eviction/size limit (RemoteIncidentSession.cs:239).
• JsonRecentFilesStore / JsonLastSaveFolderStore read disk every call — no cache (JsonRecentFilesStore.cs, JsonLastSaveFolderStore.cs).
• SystemAlarmService preloads all WAVs, never cleans temp files (SystemAlarmService.cs:35,93).
• File.ReadAllBytes before size check — OOM risk on oversized files (FilesViewModel.cs:112).
• IncidentWorkspaceViewModel.BuildChildren full rebuild (IncidentWorkspaceViewModel.cs:218).

P3 — Low

• Explicit .DisableCors() on sync endpoints for defense-in-depth (IncidentHost.cs).
• MemoryStream.ToArray() extra copy (SystemAlarmMerger.cs:117).

Positives (no action)

• Fully parameterized SQL — no SQL injection risk.
• No hardcoded secrets.
• Strict file content-type/size allowlist.
• URL scheme allowlist blocks file://, javascript:, ftp://.
• Clean layered architecture, comprehensive test suite.