moonlock/DECISIONS.md
nevaforget 65ea523b36
All checks were successful
Update PKGBUILD version / update-pkgver (push) Successful in 1s
fix: audit fixes — CString zeroize, FP account check, PAM timeout, blur downscale (v0.6.5)
Address findings from second triple audit (quality, performance, security):

- Wrap PAM CString password in Zeroizing<CString> to wipe on drop (S-H1)
- Add check_account() for pam_acct_mgmt after fingerprint unlock,
  with resume_async() to restart FP on transient failure (S-M1)
- 30s PAM timeout with generation counter to prevent stale result
  interference from parallel auth attempts (S-M3)
- Downscale wallpaper to max 1920px before GPU blur, reducing work
  by ~4x on 4K wallpapers (P-M1)
- exit(1) instead of return on no-monitor after lock.lock() (Q-2.1)
2026-03-30 00:24:43 +02:00

46 lines
4.8 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters

This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.

# Decisions
Architectural and design decisions for Moonlock, in reverse chronological order.
## 2026-03-30 Second audit: zeroize CString, FP account check, PAM timeout, blur downscale
- **Who**: Nyx, Dom
- **Why**: Second triple audit (quality, performance, security) found: CString password copy not zeroized (HIGH), fingerprint unlock bypassing pam_acct_mgmt (MEDIUM), no PAM timeout leaving user locked out on hanging modules (MEDIUM), GPU blur on full wallpaper resolution (MEDIUM), no-monitor edge case doing `return` instead of `exit(1)` (MEDIUM).
- **Tradeoffs**: PAM timeout (30s) uses a generation counter to avoid stale result interference — adds complexity but prevents parallel PAM sessions. FP restart after failed account check re-claims the device, adding a D-Bus round-trip, but prevents permanent FP death on transient failures. Blur downscale to 1920px cap trades negligible quality for ~4x less GPU work on 4K wallpapers.
- **How**: (1) `Zeroizing<CString>` wraps password in auth.rs, `zeroize/std` feature enabled. (2) `check_account()` calls pam_acct_mgmt after FP match; `resume_async()` restarts FP on transient failure. (3) `auth_generation` counter invalidates stale PAM results; 30s timeout re-enables UI. (4) `MAX_BLUR_DIMENSION` caps blur input at 1920px, sigma scaled proportionally. (5) `exit(1)` on no-monitor after `lock.lock()`.
## 2026-03-28 Remove embedded wallpaper from binary
- **Who**: Nyx, Dom
- **Why**: Wallpaper is installed by moonarch to /usr/share/moonarch/wallpaper.jpg. Embedding a 374K JPEG in the binary is redundant. GTK background color (Catppuccin Mocha base) is a clean fallback.
- **Tradeoffs**: Without moonarch installed AND without config, lockscreen shows plain dark background instead of wallpaper. Acceptable — that's the expected minimal state.
- **How**: Remove wallpaper.jpg from GResources, return None from resolve_background_path when no file found, skip background picture creation when no texture available.
## 2026-03-28 Audit-driven security and lifecycle fixes (v0.6.0)
- **Who**: Nyx, Dom
- **Why**: Triple audit (quality, performance, security) revealed a critical D-Bus signal spoofing vector, fingerprint lifecycle bugs, and multi-monitor performance issues.
- **Tradeoffs**: `cleanup_dbus()` extraction adds a method but clarifies the stop/match ownership; `running_flag: Rc<Cell<bool>>` adds a field but prevents race between async restart and stop; sender validation adds a check per signal but closes the only known auth bypass.
- **How**: (1) Validate D-Bus VerifyStatus sender against fprintd's unique bus name. (2) Extract `cleanup_dbus()` from `stop()`, call it on verify-match. (3) `Rc<Cell<bool>>` running flag checked after await in `restart_verify_async`. (4) Consistent 3s D-Bus timeouts. (5) Panic hook before logging. (6) Blur and avatar caches shared across monitors. (7) Peek icon disabled. (8) Symlink rejection for background_path. (9) TOML parse errors logged.
## 2026-03-28 GPU blur via GskBlurNode replaces CPU blur
- **Who**: Nyx, Dom
- **Why**: CPU-side Gaussian blur (`image` crate) blocked the GTK main thread for 500ms2s on 4K wallpapers at cold cache. Disk cache mitigated repeat starts but added ~100 lines of complexity.
- **Tradeoffs**: GPU blur quality is slightly different (box-blur approximation vs true Gaussian), acceptable for wallpaper. Removes `image` and `dirs` dependencies entirely. No disk cache needed.
- **How**: `Snapshot::push_blur()` + `GskRenderer::render_texture()` on `connect_realize`. Blur happens once on the GPU when the widget gets its renderer, producing a concrete `gdk::Texture`. Zero startup latency.
## 2026-03-28 Optional background blur via `image` crate (superseded)
- **Who**: Nyx, Dom
- **Why**: Consistent with moonset/moongreet — blurred wallpaper as lockscreen background is a common UX pattern
- **Tradeoffs**: Adds `image` crate dependency (~15 transitive crates); CPU-side Gaussian blur at load time adds startup latency proportional to image size and sigma. Acceptable because blur runs once and the texture is shared across monitors.
- **How**: `load_background_texture(bg_path, blur_radius)` loads texture, optionally applies `imageops::blur()`, returns `gdk::Texture`. Config option `background_blur: Option<f32>` in TOML.
## 2026-03-28 Shared wallpaper texture pattern (aligned with moonset/moongreet)
- **Who**: Nyx, Dom
- **Why**: Previously loaded wallpaper per-window via `Picture::for_filename()`. Multi-monitor setups decoded the JPEG redundantly. Blur feature requires texture pixel access anyway.
- **Tradeoffs**: Slightly more code in main.rs (texture loaded before window creation), but avoids redundant decoding and enables the blur feature.
- **How**: `load_background_texture()` in lockscreen.rs decodes once, `create_background_picture()` wraps shared `gdk::Texture` in `gtk::Picture`. Same pattern as moonset/moongreet.