nevaforget
b9b6f50974
All checks were successful
Update PKGBUILD version / update-pkgver (push) Successful in 2s
- power::run_command: .stdout(Stdio::null()) — the pipe was never drained, structurally fragile even if no current caller hits it. - config: replace to_string_lossy() on relative wallpaper paths with to_str() + log::warn, so non-UTF-8 paths are dropped cleanly instead of being mangled into unopenable U+FFFD strings. - main: require MOONGREET_DEBUG=1 to raise verbosity. Mere presence of the var must not leak socket paths, usernames, and auth round counts into the journal. - sessions: parse Hidden= and NoDisplay= keys, skip entries marked true. Keeps disabled or stub .desktop files out of the session dropdown.
106 lines
14 KiB
Markdown
106 lines
14 KiB
Markdown
# Decisions
|
||
|
||
## 2026-04-24 – Audit LOW fixes: stdout null, utf-8 path, debug value, hidden sessions (v0.8.6)
|
||
|
||
- **Who**: ClaudeCode, Dom
|
||
- **Why**: Four LOW findings cleared in a single pass. (1) `power::run_command` piped stdout it never read — structurally fragile even though current callers stay well under the pipe buffer. (2) Relative wallpaper paths were resolved via `to_string_lossy`, silently substituting `U+FFFD` for non-UTF-8 bytes and producing a path that cannot be opened. (3) `MOONGREET_DEBUG` escalated log verbosity on mere presence, so an empty variable leaked auth metadata into the journal. (4) `Hidden=true` and `NoDisplay=true` `.desktop` entries appeared in the session dropdown even though they mark disabled or stub sessions.
|
||
- **Tradeoffs**: Gating debug on the literal value `"1"` is slightly stricter than most tools but matches the security-first posture. Filtering Hidden/NoDisplay means legitimately hidden but functional sessions are now unselectable from the greeter — acceptable, that is the convention these keys signal.
|
||
- **How**: (1) `.stdout(Stdio::null())` replaces the unused pipe. (2) `to_string_lossy().to_string()` replaced by `to_str().map(|s| s.to_string())` with a `log::warn!` fallback for non-UTF-8 paths. (3) `match std::env::var("MOONGREET_DEBUG").ok().as_deref()` → `Some("1")` selects Debug, everything else Info. (4) `parse_desktop_file` reads `Hidden=` and `NoDisplay=`, returns `None` if either is `true`.
|
||
|
||
## 2026-04-24 – Audit MEDIUM fixes: FP double-init, async avatar, symlink, FD leak (v0.8.5)
|
||
|
||
- **Who**: ClaudeCode, Dom
|
||
- **Why**: Six MEDIUM findings: (1) i18n test `all_string_fields_nonempty` missed four string fields — future locales could ship empty strings unnoticed. (2) Fast user-switch could spawn two parallel fprintd `init_async` calls because both coroutines saw `fingerprint_probe = None` before either stored its probe. (3) Synchronous avatar decode via `Pixbuf::from_file_at_scale` on the GTK main thread, stalling clicks. (4) Wallpaper `MAX_WALLPAPER_FILE_SIZE = 50 MB` bounded decode at up to ~2 s. (5) Fallback wallpaper path used `is_file()` which follows symlinks, inconsistent with the symlink-rejecting user-config path. (6) After a failed login the cloned `greetd_sock` descriptor remained in shared state until the next user switch, accumulating stale FDs across retries.
|
||
- **Tradeoffs**: The init-race guard uses a bool flag on `GreeterState` + a 25 ms polling yield — cheap and race-free, but introduces a very short latency when a second probe waits. Lowering `MAX_WALLPAPER_FILE_SIZE` to 10 MB and `MAX_AVATAR_FILE_SIZE` to 5 MB caps worst-case decode but rejects legitimately huge (4K raw) wallpapers; acceptable for a greeter. Async avatar decode shows the default icon for a frame or two on cache miss.
|
||
- **How**: (1) Four new `assert!` lines in `i18n::tests::all_string_fields_nonempty`. (2) New `fingerprint_probe_initializing: bool` on `GreeterState`, atomic check-and-set under `borrow_mut`, losing coroutines yield via `glib::timeout_future` until the winning init completes. (3) `set_avatar_from_file` uses `gio::File::read_future` + `Pixbuf::from_stream_at_scale_future` inside a `glib::spawn_future_local`, sets the default icon first, swaps on success. (4) Lower both size constants. (5) `resolve_background_path_with` now applies the same `symlink_metadata` + `!is_symlink` check to the Moonarch fallback. (6) After the login worker returns, `state.greetd_sock.lock().take()` drops the stale clone regardless of login outcome.
|
||
|
||
## 2026-04-24 – Audit fix: shrink password-in-memory window (v0.8.4)
|
||
|
||
- **Who**: ClaudeCode, Dom
|
||
- **Why**: Security audit flagged the GTK password path as holding more copies of the plaintext password in memory than necessary. `attempt_login` wrapped the already-`Zeroizing<String>` caller value into a second `Zeroizing<String>` (`password.to_string()`), and the GTK `GString` backing `entry.text()` persisted in libc malloc'd memory until the allocator reused the page.
|
||
- **Tradeoffs**: The GTK `GString` and the libc `strdup` copy on the PAM FFI boundary remain non-zeroizable — this is an inherent GTK/libc limitation, already documented in CLAUDE.md. This change reduces the Rust-owned copies to one and clears the `PasswordEntry` text field immediately after extraction to shorten the GTK-side window.
|
||
- **How**: (1) `attempt_login` now takes `password: Zeroizing<String>` by value instead of `&str`, moving ownership into the `spawn_blocking` closure. (2) The redundant `Zeroizing::new(password.to_string())` inside `attempt_login` is removed. (3) `password_entry.set_text("")` is called right after the password is extracted from the activate handler, shortening the lifetime of the GTK-internal buffer.
|
||
|
||
## 2026-04-21 – Ship polkit rule in moongreet instead of moonarch (v0.8.3)
|
||
|
||
- **Who**: ClaudeCode, Dom
|
||
- **Why**: Reboot/shutdown from the greeter silently failed on a fresh install. The polkit rule that grants the `greeter` user `org.freedesktop.login1.{reboot,power-off}` lived in the moonarch repo but was never installed by any PKGBUILD. The laptop worked only because the rule had been hand-deployed once.
|
||
- **Tradeoffs**: Rule ownership moves from moonarch (system defaults) to moongreet (greeter-specific auth). Cleaner boundary — moonarch no longer needs to know about the greeter's auth requirements — but it means moongreet is now responsible for a system polkit rule that ties it to a fixed username (`greeter`).
|
||
- **How**: Source file moved to `moongreet/config/polkit/50-moongreet-power.rules`, installed to `/etc/polkit-1/rules.d/` by `moongreet-git/PKGBUILD`. Old file removed from the moonarch repo.
|
||
|
||
## 2026-04-09 – Monitor hotplug via ListModel items-changed
|
||
|
||
- **Who**: ClaudeCode, Dom
|
||
- **Why**: Greeter windows were only created at startup. If a monitor was hotplugged (e.g. HDMI reconnect), it would show no greeter UI. Aligned with moonlock's hotplug fix (same day).
|
||
- **Tradeoffs**: Hotplugged monitors get greeter windows without keyboard input (keyboard stays on the primary monitor). Acceptable — user can still interact on the primary screen.
|
||
- **How**: Connect to `display.monitors().connect_items_changed()` and create new greeter windows for added monitors. Shared state (config, texture, blur_cache) moved to Rc for the closure.
|
||
|
||
## 2026-04-08 – Show greeter UI on all monitors instead of just one
|
||
|
||
- **Who**: ClaudeCode, Dom
|
||
- **Why**: moonlock showed its UI on all monitors via ext-session-lock-v1, but moongreet only showed the login UI on one monitor (compositor-picked) with wallpaper-only windows on the rest. Inconsistent UX across the ecosystem.
|
||
- **Tradeoffs**: Each monitor gets its own full greeter widget tree (slightly more memory), but the UI is lightweight. Screen mirroring (e.g., wl-mirror/screencopy) was considered and rejected — it requires an external process, compositor screencopy support, adds latency, and fights Wayland's per-output model. One-window-per-monitor is the established Wayland pattern (swaylock, hyprlock, moonlock all do this).
|
||
- **How**: Create one `create_greeter_window()` per monitor with `set_monitor()`, only the first gets `KeyboardMode::Exclusive`. Removed `create_wallpaper_window()` (no longer needed). No layer shell fallback keeps single-window mode for development.
|
||
|
||
## 2026-04-06 – Restore explicit gtk-theme in moongreet config
|
||
|
||
- **Who**: ClaudeCode, Dom
|
||
- **Why**: GTK4 under greetd does not reliably read `/etc/xdg/gtk-4.0/settings.ini` — likely requires a settings daemon that doesn't run in the greeter session. moongreet fell back to Adwaita/Colloid-default (blue accent) instead of Colloid-Grey-Dark-Catppuccin.
|
||
- **Tradeoffs**: Reverts `094878f` ("Remove gtk-theme from app config, use system-wide GTK settings instead"). Duplicates the theme name between settings.ini and moongreet.toml, but the explicit set via `set_gtk_theme_name()` is the only reliable path in a greetd context.
|
||
- **How**: Added `gtk-theme = "Colloid-Grey-Dark-Catppuccin"` to example config and deployed `/etc/moongreet/moongreet.toml`.
|
||
|
||
## 2026-04-02 – Replace hardcoded CSS colors with GTK theme variables
|
||
|
||
- **Who**: ClaudeCode, Dom
|
||
- **Why**: moongreet used hardcoded colors (#1a1a2e, white, #ff6b6b) while moonset already used @theme_bg_color, @theme_fg_color, @error_color etc. Inconsistent across the ecosystem and broke theme flexibility.
|
||
- **Tradeoffs**: Depends on the active GTK theme defining standard color variables. Catppuccin Colloid provides all needed vars (@theme_bg_color, @theme_fg_color, @error_color, @success_color, @theme_selected_bg_color). Fallback behavior if a theme lacks vars is GTK's default colors — acceptable.
|
||
- **How**: Replaced all hardcoded hex/named colors with GTK theme variables. Coordinated change across moongreet, moonlock, and moonset (all three now use identical pattern).
|
||
|
||
## 2026-03-31 – Fourth audit: power timeout, timing mitigation, release profile, GREETD_SOCK caching
|
||
|
||
- **Who**: ClaudeCode, Dom
|
||
- **Why**: Fourth triple audit found moongreet power.rs had no timeout on loginctl (greeter could freeze), username enumeration via timing differential, GREETD_SOCK re-read on every login, missing release profile, and missing GResource compression.
|
||
- **Tradeoffs**: 500ms minimum login response time adds slight delay on fast auth but prevents timing-based username enumeration. Power timeout (30s + SIGKILL) matches moonset pattern — aggressive but prevents greeter freeze.
|
||
- **How**: (1) power.rs adapted from moonset with 30s timeout + SIGKILL (nix dependency added). (2) 500ms min response floor in attempt_login via Instant + glib::timeout_future. (3) GREETD_SOCK cached in GreeterState at startup. (4) `[profile.release]` with LTO, codegen-units=1, strip. (5) `compressed="true"` on GResource entries. (6) SYNC comments on duplicated blur/background functions.
|
||
|
||
## 2026-03-30 – Full audit fix: security, quality, performance (v0.6.2)
|
||
|
||
- **Who**: ClaudeCode, Dom
|
||
- **Why**: Three parallel audits (security, code quality, performance) identified 10 actionable findings across the codebase — from world-readable cache dirs to a GPU blur geometry bug to a race condition in fingerprint probing.
|
||
- **Tradeoffs**: `too_many_arguments` Clippy warnings suppressed with `#[allow]` rather than introducing a `UiWidgets` struct — GTK's `clone!` macro with `#[weak]` refs requires individual widget parameters, a struct would fight the idiom. Async avatar loading skipped because `Pixbuf` is `!Send`; cache already prevents repeat loads. TOCTOU socket pre-check removed entirely — `connect()` in login_worker already handles errors, the `metadata()` check gave false security guarantees.
|
||
- **How**: Cache dirs use `DirBuilder::mode(0o700)` instead of `create_dir_all`. Blur config clamped to `0.0..=200.0` with `is_finite()` guard. Blur texture cached in `Rc<RefCell<Option<gdk::Texture>>>` across monitors. FingerprintProbe device proxy cached in `GreeterState` with generation counter to prevent stale async writes. GPU blur geometry fixed (`-pad` origin shift instead of texture stretching). `is_valid_gtk_theme` extracted as testable function. 9 new tests.
|
||
|
||
## 2026-03-29 – Fingerprint authentication via greetd multi-stage PAM
|
||
|
||
- **Who**: ClaudeCode, Dom
|
||
- **Why**: moonlock supports fprintd but moongreet rejected multi-stage auth. Users with enrolled fingerprints couldn't use them at the login screen.
|
||
- **Tradeoffs**: Direct fprintd D-Bus verification (like moonlock) can't start a greetd session — greetd controls session creation via PAM. Using greetd multi-stage means PAM decides the auth order (fingerprint first, then password fallback), not truly parallel. Acceptable — matches standard pam_fprintd behavior.
|
||
- **How**: Replace single-pass auth with a loop over auth_message rounds. Secret prompts get the password, non-secret prompts (fprintd) get None and block until PAM resolves. fprintd D-Bus probe (gio::DBusProxy) only for UI — detecting device availability and enrolled fingers. 60s socket timeout when fingerprint available. Config option `fingerprint-enabled` (default true).
|
||
|
||
## 2026-03-28 – Remove embedded wallpaper from binary
|
||
|
||
- **Who**: ClaudeCode, 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, greeter 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 wallpaper window creation and background picture when no path available.
|
||
|
||
## 2026-03-28 – GPU blur via GskBlurNode replaces CPU blur
|
||
|
||
- **Who**: ClaudeCode, Dom
|
||
- **Why**: CPU-side Gaussian blur (`image` crate) blocked the GTK main thread for 500ms–2s on 4K wallpapers at cold cache. Disk cache and async orchestration added significant complexity.
|
||
- **Tradeoffs**: GPU blur quality is slightly different (box-blur approximation vs true Gaussian), acceptable for wallpaper backgrounds. Removes `image` crate dependency entirely (~15 transitive crates eliminated). 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. Symmetric with moonlock and moonset.
|
||
|
||
## 2026-03-28 – Optional background blur via `image` crate (superseded)
|
||
|
||
- **Who**: ClaudeCode, Dom
|
||
- **Why**: Blurred wallpaper as greeter background is a common UX pattern for login screens
|
||
- **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 blurred `gdk::Texture`. Config option `background-blur: Option<f32>` in `[appearance]` TOML section.
|
||
|
||
## 2026-03-28 – Audit fixes for shared wallpaper texture (v0.4.1)
|
||
- **Who**: ClaudeCode, Dominik
|
||
- **Why**: Quality, performance, and security audits flagged issues in `load_background_texture()`, debug logging, and greetd error handling
|
||
- **Tradeoffs**: GResource path now requires UTF-8 (returns `None` for non-UTF-8 instead of aborting); 50 MB wallpaper limit is generous but prevents OOM; debug logging off by default trades observability for security
|
||
- **How**: GResource branch via `resources_lookup_data()` + `from_bytes()` (no abort), file size limit, error details only at debug level, `MOONGREET_DEBUG` env var for log level, greetd retry path truncation matching `show_greetd_error()`
|