Security & code audit — repojacking fix, crash fix, hardening

[Daolyap]

Summary

Full security and code audit of the service layer and window code-behind, stacked on top of #45 (Fable/Overhaul).

Security findings (fixed)

Severity	Finding	Fix
High	Repojacking exposure in the auto-updater. The repo was renamed MoneyShot → Money-Shot, but AutoUpdateService still pointed at the old name. GitHub redirects an old repo name only until someone re-registers it — at which point they control the update channel of every installed copy.	Constants updated to Daolyap/Money-Shot (verified the new endpoint serves releases + SHA256SUMS.txt); rule documented in CLAUDE.md.
Medium	Optional GitHub token was read from Environment.GetEnvironmentVariable("") — a disabled stub — while the 401 error message told users to configure MONEYSHOT_GITHUB_TOKEN.	Wired to MONEYSHOT_GITHUB_TOKEN; treated as a secret, never logged.
Medium	SaveService system-directory check used a bare StartsWith, so C:\Windows also matched the unrelated C:\WindowsBackup.	Prefix comparison now uses trailing separators.
Low	A tampered settings.json could set HistoryRetentionCount to arbitrary values.	Clamped to the UI range (0–500) in ValidateAndSanitizeSettings.

Crash / correctness bugs (fixed)

• Second-instance crash: App.OnExit called ReleaseMutex() on the single-instance mutex the second instance never owned → ApplicationException on its way out. Now only the owning instance releases.
• Hotkey handler exceptions were process-fatal: an exception escaping the WM_HOTKEY hook tears down the app. Now caught and logged. Hotkey registration failures (combination owned by another app) were silently ignored — now logged.
• Editor Save dialog ignored settings: the configured default save folder and default file format were never applied to the dialog (initial dir, filter, extension, filename all PNG/last-used). Now honoured.
• IsPointInElement hand-rolled NaN guards instead of using CanvasPosition (explicitly forbidden by the repo''s resize/drag rules — historical source of snap-to-(0,0) bugs).

Optimisations & quality

• New MemoryTrimmer service shared by both editor-close paths — opening a capture from History previously skipped the working-set trim and left hundreds of MB resident.
• JPEG saves now use QualityLevel = 90 (the default 75 visibly smears screenshot text).
• Logger records the full exception + stack trace at Error level (message-only lines were rarely diagnosable).
• Dead code removed (SetupToolbar, unused usings).

Audited and intentionally unchanged

• ScreenshotService HBITMAP lifecycle (correct: DeleteObject in finally, Freeze() for cross-thread).
• Zip extraction in the updater (no zip-slip: entry name is only compared, destination path is fixed).
• SHA-256 release verification flow and SemVer-only comparison (correct as designed).
• Settings JSON deserialization (System.Text.Json, no polymorphism — safe defaults).
• Registry writes (HKCU only, quoted exe path for the Run key).

Follow-ups recorded in Opus-Speaks.md (not fixed here)

• E2: Region selector is only pixel-accurate at 100 % display scaling (PerMonitorV2 manifest vs. DIP↔pixel 1:1 mapping). Needs per-monitor selector windows — too invasive to bundle here.
• E3: The self-update swap script runs unelevated and cannot replace a per-machine Program Files install.
• E (existing): Authenticode signing still open.

Verification

• dotnet build clean, 109/109 tests (14 new: SaveService path validation, retention clamping).
• Editor window re-rendered through the off-screen harness after changes — loads and themes correctly.

🤖 Generated with Claude Code