moonset/DECISIONS.md
nevaforget 8285bcdf44
All checks were successful
Update PKGBUILD version / update-pkgver (push) Successful in 2s
fix: audit LOW fixes — dead uid, home_dir warn, clippy sweep, debug value (v0.8.5)
- users::User: drop the unused `uid` field and its getuid() assignment.
  The compiler dead_code warning is gone, and the synthetic `u32::MAX`
  sentinel in the panel fallback is obsolete too.
- panel: surface a log::warn! when dirs::home_dir() returns None instead
  of silently falling back to an empty PathBuf that would make avatars
  look for .face in the current working directory.
- Apply three clippy suggestions: two collapsible if-let + && chains in
  users::get_avatar_path_with and config::resolve_background_path_with,
  and a redundant closure in panel::execute_action's spawn_blocking.
- main: require MOONSET_DEBUG=1 to escalate log verbosity — mere
  presence of the var must not dump path info into the journal.
2026-04-24 14:14:11 +02:00

11 KiB
Raw Permalink Blame History

Decisions

Architectural and design decisions for Moonset, in reverse chronological order.

2026-04-24 Audit LOW fixes: dead uid field, home_dir warn, clippy sweep, debug value (v0.8.5)

  • Who: ClaudeCode, Dom
  • Why: Five LOW findings cleared in one pass. (1) User::uid was populated from getuid() but never read — a compiler dead_code warning for a field on the public API. (2) Falling back to a synthetic user when get_current_user() returned None used uid: u32::MAX, an undocumented sentinel that became moot once uid was removed. (3) dirs::home_dir().unwrap_or_default() silently yielded PathBuf::new() on failure; avatars would then look for .face in the current working directory. (4) cargo clippy flagged three suggestions (two collapsible if, one redundant closure) that had crept in. (5) MOONSET_DEBUG promoted log verbosity on mere presence, leaking path information into the journal.
  • Tradeoffs: Dropping uid from User is a minor API break for any internal caller expecting the field — none existed. The synthetic fallback now surfaces log::warn! when home resolution fails, which should be rare outside of pathological sandbox environments.
  • How: (1) Remove pub uid: u32 from User and the uid: uid.as_raw() assignment in get_current_user. (2) Panel fallback drops the uid field entirely. (3) dirs::home_dir().unwrap_or_else(|| { log::warn!(...); PathBuf::new() }). (4) cargo clippy --fix for the two collapsible ifs, manual collapse of if-let + && chain, redundant closure replaced with the function itself. (5) MOONSET_DEBUG now requires the literal value "1" to escalate to Debug.

2026-04-24 Audit MEDIUM fixes: timeout guard, POSIX locale, button desensitize, wallpaper allowlist (v0.8.4)

  • Who: ClaudeCode, Dom
  • Why: Five MEDIUM findings: (1) run_command's timeout thread leaked a 30 s gio::spawn_blocking slot if child.wait() errored, because done.store(true) ran after the ?. (2) Timeout detection compared status.signal() == Some(9) — a hardcoded signal number that also misclassifies OOM-killer SIGKILL as our timeout. (3) execute_action never desensitized the button_box, so a double-click or accidental keyboard repeat fired the action twice. (4) detect_locale read only LANG, ignoring POSIX priority order (LC_ALL > LC_MESSAGES > LANG) — a common dual-language setup picked the wrong UI language. (5) The wallpaper path was passed to gdk-pixbuf without extension or size restriction, widening the image-parser attack surface and allowing unbounded decode latency.
  • Tradeoffs: The extension allowlist (jpg, jpeg, png, webp) rejects exotic formats users might have used before. The 10 MB size cap rejects uncompressed/high-quality 4K wallpapers; acceptable for a power menu. Memory ordering on the done flag is now Release/Acquire instead of Relaxed — no runtime cost but correct across threads.
  • How: (1) RAII DoneGuard struct sets done.store(true, Release) in its Drop, so the flag fires on every function exit path. A second timed_out AtomicBool distinguishes our SIGKILL from an external one. (2) Replace Some(9) with the timed_out flag check. (3) execute_action now takes button_box: &gtk::Box, calls set_sensitive(false) on entry and re-enables it on error paths; success paths that quit skip the re-enable. All call sites updated. (4) detect_locale reads LC_ALL, LC_MESSAGES, LANG in order, picking the first non-empty value before falling back to /etc/locale.conf. (5) accept_wallpaper helper applies extension allowlist + symlink rejection + MAX_WALLPAPER_FILE_SIZE = 10 MB, and is called for both config-path and Moonarch fallback.

2026-04-24 Audit fix: avoid latent stdout pipe deadlock in run_command (v0.8.3)

  • Who: ClaudeCode, Dom
  • Why: Audit found that run_command piped the child's stdout but never drained it, then called blocking child.wait(). A child writing more than one OS pipe buffer (~64 KB on Linux) would block on write() while the parent blocked in wait() — classic pipe deadlock, broken only by the 30 s SIGKILL timeout. Current callers (systemctl, niri msg, loginctl) do not emit that much output, but the structure was fragile and would bite on any future command or changed behaviour.
  • Tradeoffs: stdout is now fully discarded. If a future caller needs stdout, it will have to drain it concurrently with wait() (separate reader thread).
  • How: Replace .stdout(Stdio::piped()) with .stdout(Stdio::null()) in run_command. stderr stays piped — it is drained after wait(), which is safe because wait() already reaped the child and no further writes can occur.

2026-03-31 Fourth audit: release profile, GResource compression, lock stderr, sync markers

  • Who: ClaudeCode, Dom
  • Why: Fourth triple audit found missing release profile (LTO/strip), uncompressed GResource assets, moonlock stderr suppressed (errors invisible), and duplicated code without sync markers.
  • Tradeoffs: moonlock stderr now inherited instead of null — errors appear in moonset's journal context. Acceptable for debugging, no security leak since moonlock logs to its own journal identifier.
  • How: (1) [profile.release] with LTO, codegen-units=1, strip. (2) compressed="true" on GResource entries. (3) Stdio::inherit() for moonlock stderr in lock(). (4) SYNC comments on duplicated blur/background functions.

2026-03-28 Remove wallpaper from GResource bundle

  • Who: ClaudeCode, Dom
  • Why: All three Moon projects (moonset, moongreet, moonlock) embedded a 374kB fallback wallpaper in the binary via GResource. Moonarch already installs /usr/share/moonarch/wallpaper.jpg at system setup time, making the embedded fallback unnecessary dead weight (~2MB in binary size).
  • Tradeoffs: If /usr/share/moonarch/wallpaper.jpg is missing and no user config exists, moonset shows a solid CSS background instead of a wallpaper. Acceptable — the power menu is functional without a wallpaper image.
  • How: Removed wallpaper.jpg from GResource XML and resources directory. resolve_background_path returns Option<PathBuf>. All wallpaper-related functions handle None gracefully. Binary size dropped from ~3.2MB to ~1.3MB.

2026-03-28 Switch from env_logger to systemd-journal-logger

  • Who: ClaudeCode, Dom
  • Why: moonlock and moongreet already use systemd-journal-logger. moonset used env_logger which writes to stderr — not useful for a GUI app launched via keybind. Journal integration enables journalctl -t moonset and consistent troubleshooting across all three Moon projects.
  • Tradeoffs: Requires systemd at runtime. Graceful fallback to eprintln if journal logger fails. Acceptable since Moonarch targets systemd-based Arch Linux.
  • How: Replace env_logger dep with systemd-journal-logger, add setup_logging() with MOONSET_DEBUG env var for debug-level output. Same pattern as moonlock/moongreet.

2026-03-28 Replace action name dispatch with quit_after field

  • Who: ClaudeCode, Dom
  • Why: Post-action behavior (quit the app or not) was controlled by comparing action_name == "lock" — a magic string duplicated from the action definition. Renaming an action would silently break the dispatch.
  • Tradeoffs: Adds a field to ActionDef that most actions set to false. Acceptable because it makes the contract explicit and testable.
  • How: ActionDef.quit_after: booltrue for lock and logout, false for hibernate/reboot/shutdown.

2026-03-28 GPU blur via GskBlurNode replaces CPU blur

  • Who: ClaudeCode, Dom
  • Why: CPU-side Gaussian blur (image crate) blocked startup and added caching complexity. moonlock already migrated to GPU blur.
  • Tradeoffs: GPU blur quality is slightly different (box-blur approximation vs true Gaussian), acceptable for wallpaper backgrounds. Removes image crate dependency 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. Symmetric with moonlock and moongreet.

2026-03-28 Use absolute paths for system binaries

  • Who: ClaudeCode, Dom
  • Why: Security audit flagged PATH hijacking risk — relative binary names allow a malicious $PATH entry to intercept systemctl, loginctl, etc.
  • Tradeoffs: Hardcoded paths reduce portability to non-Arch distros where binaries may live elsewhere (e.g. /sbin/). Acceptable because Moonarch targets Arch Linux exclusively.
  • How: All five power action wrappers now use /usr/bin/ prefixed paths.

2026-03-28 Implement power action timeout via try_wait polling

  • Who: ClaudeCode, Dom
  • Why: POWER_TIMEOUT and PowerError::Timeout were declared but never wired up. A hanging systemctl hibernate (e.g. blocked NFS mount) would freeze the power menu indefinitely.
  • Tradeoffs: Polling with try_wait() + 100ms sleep is slightly less efficient than a dedicated timeout crate, but avoids adding a dependency for a single use case.
  • How: run_command now polls child.try_wait() against a 30s deadline, kills the child on timeout.

2026-03-28 Centralize GRESOURCE_PREFIX

  • Who: ClaudeCode, Dom
  • Why: The string /dev/moonarch/moonset was duplicated in config.rs, users.rs, and as literal strings in panel.rs and main.rs. Changing the application ID would require edits in 4+ locations.
  • Tradeoffs: Modules now depend on crate::GRESOURCE_PREFIX — tighter coupling to main.rs, but acceptable for an internal constant.
  • How: Single pub(crate) const GRESOURCE_PREFIX in main.rs, referenced everywhere else.

2026-03-28 Remove journal.md

  • Who: ClaudeCode, Dom
  • Why: One-time development notes from the Rust rewrite, never updated after initial session. Overlapped with memory system and git history.
  • Tradeoffs: Historical context lost from the file, but the information is preserved in git history and the memory system.
  • How: Deleted. Useful technical learnings migrated to persistent memory.

2026-03-27 OVERLAY layer instead of TOP

  • Who: ClaudeCode, Dom
  • Why: Waybar occupies the TOP layer. The power menu must appear above it.
  • Tradeoffs: OVERLAY is the highest layer — nothing can render above moonset while it's open. This is intentional for a session power menu.
  • How: setup_layer_shell uses gtk4_layer_shell::Layer::Overlay for the panel window.

2026-03-27 Lock without confirmation

  • Who: ClaudeCode, Dom
  • Why: Lock is immediately reversible (just unlock). All other actions (logout, hibernate, reboot, shutdown) are destructive or disruptive.
  • Tradeoffs: One less click for the most common action. Risk of accidental lock is negligible since unlocking is trivial.
  • How: ActionDef.needs_confirm = false for lock; all others require inline confirmation.

2026-03-27 Niri-specific logout via niri msg action quit

  • Who: ClaudeCode, Dom
  • Why: Moonarch is built exclusively for the Niri compositor. Generic Wayland logout mechanisms don't exist — each compositor has its own.
  • Tradeoffs: Hard dependency on Niri. If the compositor changes, power::logout() must be updated.
  • How: Command::new("/usr/bin/niri").args(["msg", "action", "quit"]).