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.

Cargo.lock

@@ -616,7 +616,7 @@ dependencies = [
 
 [[package]]
 name = "moonset"
-version = "0.8.3"
+version = "0.8.5"
 dependencies = [
  "dirs",
  "gdk-pixbuf",

Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "moonset"
-version = "0.8.3"
+version = "0.8.5"
 edition = "2024"
 description = "Wayland session power menu with GTK4 and Layer Shell"
 license = "MIT"

DECISIONS.md

@@ -2,6 +2,20 @@
 
 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

src/config.rs

@@ -62,21 +62,56 @@ pub fn resolve_background_path(config: &Config) -> Option<PathBuf> {
     resolve_background_path_with(config, Path::new(MOONARCH_WALLPAPER))
 }
 
+/// Wallpapers are passed to gdk-pixbuf's image loader; restrict to common image
+/// extensions to reduce the parser-attack surface for user-controlled paths.
+const ALLOWED_BG_EXT: &[&str] = &["jpg", "jpeg", "png", "webp"];
+
+/// Bound wallpaper decode latency (10 MB covers typical 4K JPEGs at Q95).
+const MAX_WALLPAPER_FILE_SIZE: u64 = 10 * 1024 * 1024;
+
+fn is_allowed_wallpaper(path: &Path) -> bool {
+    match path.extension().and_then(|e| e.to_str()) {
+        Some(ext) => ALLOWED_BG_EXT.iter().any(|a| a.eq_ignore_ascii_case(ext)),
+        None => false,
+    }
+}
+
+fn accept_wallpaper(path: &Path) -> bool {
+    if !is_allowed_wallpaper(path) {
+        log::warn!("Wallpaper rejected (extension not in allowlist): {}", path.display());
+        return false;
+    }
+    match path.symlink_metadata() {
+        Ok(meta) if meta.file_type().is_symlink() => {
+            log::warn!("Wallpaper rejected (symlink): {}", path.display());
+            false
+        }
+        Ok(meta) if !meta.is_file() => false,
+        Ok(meta) if meta.len() > MAX_WALLPAPER_FILE_SIZE => {
+            log::warn!(
+                "Wallpaper rejected ({} bytes > {} limit): {}",
+                meta.len(), MAX_WALLPAPER_FILE_SIZE, path.display()
+            );
+            false
+        }
+        Ok(_) => true,
+        Err(_) => false,
+    }
+}
+
 /// Resolve with configurable moonarch wallpaper path (for testing).
 pub fn resolve_background_path_with(config: &Config, moonarch_wallpaper: &Path) -> Option<PathBuf> {
-    // User-configured path — reject symlinks to prevent path traversal
+    // User-configured path — reject symlinks, non-image extensions, and oversized files
     if let Some(ref bg) = config.background_path {
         let path = PathBuf::from(bg);
-        if let Ok(meta) = path.symlink_metadata() {
-            if meta.is_file() && !meta.file_type().is_symlink() {
-                log::debug!("Wallpaper source: config ({})", path.display());
-                return Some(path);
-            }
+        if accept_wallpaper(&path) {
+            log::debug!("Wallpaper source: config ({})", path.display());
+            return Some(path);
         }
     }
 
-    // Moonarch ecosystem default
-    if moonarch_wallpaper.is_file() {
+    // Moonarch ecosystem default — apply the same checks for consistency
+    if accept_wallpaper(moonarch_wallpaper) {
         log::debug!("Wallpaper source: moonarch default ({})", moonarch_wallpaper.display());
         return Some(moonarch_wallpaper.to_path_buf());
     }

src/i18n.rs

@@ -110,15 +110,22 @@ fn read_lang_from_conf(path: &Path) -> Option<String> {
     None
 }
 
-/// Determine the system language from LANG env var or /etc/locale.conf.
+/// Determine the system language from POSIX locale env vars or /etc/locale.conf.
+/// Checks LC_ALL, LC_MESSAGES, LANG in POSIX priority order (LC_ALL overrides
+/// everything; LC_MESSAGES overrides LANG for text categories).
 pub fn detect_locale() -> String {
-    detect_locale_with(env::var("LANG").ok().as_deref(), Path::new(DEFAULT_LOCALE_CONF))
+    let env_val = env::var("LC_ALL")
+        .ok()
+        .filter(|s| !s.is_empty())
+        .or_else(|| env::var("LC_MESSAGES").ok().filter(|s| !s.is_empty()))
+        .or_else(|| env::var("LANG").ok().filter(|s| !s.is_empty()));
+    detect_locale_with(env_val.as_deref(), Path::new(DEFAULT_LOCALE_CONF))
 }
 
 /// Determine locale with configurable inputs (for testing).
 pub fn detect_locale_with(env_lang: Option<&str>, locale_conf_path: &Path) -> String {
     let (raw, source) = if let Some(val) = env_lang.filter(|s| !s.is_empty()) {
-        (Some(val.to_string()), "LANG env")
+        (Some(val.to_string()), "env")
     } else if let Some(val) = read_lang_from_conf(locale_conf_path) {
         (Some(val), "locale.conf")
     } else {

src/main.rs

@@ -88,10 +88,12 @@ fn setup_logging() {
             eprintln!("Failed to create journal logger: {e}");
         }
     }
-    let level = if std::env::var("MOONSET_DEBUG").is_ok() {
-        log::LevelFilter::Debug
-    } else {
-        log::LevelFilter::Info
+    // Require MOONSET_DEBUG=1 to raise verbosity so mere presence (empty
+    // value in a session script) cannot escalate journal noise with path
+    // information an attacker could use.
+    let level = match std::env::var("MOONSET_DEBUG").ok().as_deref() {
+        Some("1") => log::LevelFilter::Debug,
+        _ => log::LevelFilter::Info,
     };
     log::set_max_level(level);
 }

src/panel.rs

@@ -7,7 +7,7 @@ use glib::clone;
 use gtk4::prelude::*;
 use gtk4::{self as gtk, gio};
 use std::cell::RefCell;
-use std::path::Path;
+use std::path::{Path, PathBuf};
 use std::rc::Rc;
 use std::time::Duration;
 
@@ -208,11 +208,16 @@ pub fn create_panel_window(texture: Option<&gdk::Texture>, blur_radius: Option<f
     window.add_css_class("panel");
 
     let strings = load_strings(None);
-    let user = users::get_current_user().unwrap_or_else(|| users::User {
-        username: "user".to_string(),
-        display_name: "User".to_string(),
-        home: dirs::home_dir().unwrap_or_default(),
-        uid: u32::MAX,
+    let user = users::get_current_user().unwrap_or_else(|| {
+        let home = dirs::home_dir().unwrap_or_else(|| {
+            log::warn!("Could not resolve HOME — using an empty path");
+            PathBuf::new()
+        });
+        users::User {
+            username: "user".to_string(),
+            display_name: "User".to_string(),
+            home,
+        }
     });
     log::debug!("User: {} ({})", user.display_name, user.username);
 
@@ -445,7 +450,7 @@ fn on_action_clicked(
     error_label.set_visible(false);
 
     if !action_def.needs_confirm {
-        execute_action(action_def, strings, app, confirm_area, confirm_box, error_label);
+        execute_action(action_def, strings, app, confirm_area, confirm_box, error_label, button_box);
         return;
     }
 
@@ -488,6 +493,8 @@ fn show_confirm(
         confirm_box,
         #[weak]
         error_label,
+        #[weak]
+        button_box,
         move |_| {
             execute_action(
                 &action_def_clone,
@@ -496,6 +503,7 @@ fn show_confirm(
                 &confirm_area,
                 &confirm_box,
                 &error_label,
+                &button_box,
             );
         }
     ));
@@ -543,6 +551,7 @@ fn execute_action(
     confirm_area: &gtk::Box,
     confirm_box: &Rc<RefCell<Option<gtk::Box>>>,
     error_label: &gtk::Label,
+    button_box: &gtk::Box,
 ) {
     dismiss_confirm(confirm_area, confirm_box);
     log::debug!("Executing power action: {}", action_def.name);
@@ -552,6 +561,10 @@ fn execute_action(
     let quit_after = action_def.quit_after;
     let error_message = (action_def.error_attr)(strings).to_string();
 
+    // Desensitize buttons so a double-click or accidental keyboard repeat
+    // cannot fire the same action twice while it is in flight.
+    button_box.set_sensitive(false);
+
     // Use glib::spawn_future_local + gio::spawn_blocking to avoid Send issues
     // with GTK objects. The blocking closure runs on a thread pool, the result
     // is handled back on the main thread.
@@ -560,24 +573,30 @@ fn execute_action(
         app,
         #[weak]
         error_label,
+        #[weak]
+        button_box,
         async move {
-            let result = gio::spawn_blocking(move || action_fn()).await;
+            let result = gio::spawn_blocking(action_fn).await;
 
             match result {
                 Ok(Ok(())) => {
                     if quit_after {
                         fade_out_and_quit(&app);
+                    } else {
+                        button_box.set_sensitive(true);
                     }
                 }
                 Ok(Err(e)) => {
                     log::error!("Power action '{}' failed: {}", action_name, e);
                     error_label.set_text(&error_message);
                     error_label.set_visible(true);
+                    button_box.set_sensitive(true);
                 }
                 Err(_) => {
                     log::error!("Power action '{}' panicked", action_name);
                     error_label.set_text(&error_message);
                     error_label.set_visible(true);
+                    button_box.set_sensitive(true);
                 }
             }
         }

src/power.rs

@@ -52,44 +52,50 @@ fn run_command(action: &'static str, program: &str, args: &[&str]) -> Result<(),
 
     let child_pid = nix::unistd::Pid::from_raw(child.id() as i32);
     let done = Arc::new(AtomicBool::new(false));
+    let timed_out = Arc::new(AtomicBool::new(false));
     let done_clone = done.clone();
+    let timed_out_clone = timed_out.clone();
 
-    let timeout_thread = std::thread::spawn(move || {
-        // Sleep in short intervals so we can exit early when the child finishes
+    let _timeout_thread = std::thread::spawn(move || {
         let interval = Duration::from_millis(100);
         let mut elapsed = Duration::ZERO;
         while elapsed < POWER_TIMEOUT {
             std::thread::sleep(interval);
-            if done_clone.load(Ordering::Relaxed) {
+            if done_clone.load(Ordering::Acquire) {
                 return;
             }
             elapsed += interval;
         }
+        // Record that we fired the kill so we don't misclassify an external
+        // SIGKILL (OOM killer, kill -9) as our timeout.
+        timed_out_clone.store(true, Ordering::Release);
         // ESRCH if the process already exited — harmless
         let _ = nix::sys::signal::kill(child_pid, nix::sys::signal::Signal::SIGKILL);
     });
 
+    // Drop guard ensures the timeout thread sees done=true even if child.wait()
+    // errors out — otherwise the thread sleeps its full 30 s holding a slot in
+    // the gio::spawn_blocking pool.
+    struct DoneGuard(Arc<AtomicBool>);
+    impl Drop for DoneGuard {
+        fn drop(&mut self) {
+            self.0.store(true, Ordering::Release);
+        }
+    }
+    let _done_guard = DoneGuard(done);
+
     let status = child.wait().map_err(|e| PowerError::CommandFailed {
         action,
         message: e.to_string(),
     })?;
 
-    done.store(true, Ordering::Relaxed);
-    let _ = timeout_thread.join();
-
     if status.success() {
         log::debug!("Power action {action} completed");
         Ok(())
     } else {
-        // Check if killed by our timeout (SIGKILL = signal 9)
-        #[cfg(unix)]
-        {
-            use std::os::unix::process::ExitStatusExt;
-            if status.signal() == Some(9) {
-                return Err(PowerError::Timeout { action });
-            }
+        if timed_out.load(Ordering::Acquire) {
+            return Err(PowerError::Timeout { action });
         }
-
         let mut stderr_buf = String::new();
         if let Some(mut stderr) = child.stderr.take() {
             let _ = stderr.read_to_string(&mut stderr_buf);

src/users.rs

@@ -12,7 +12,6 @@ pub struct User {
     pub username: String,
     pub display_name: String,
     pub home: PathBuf,
-    pub uid: u32,
 }
 
 /// Get the currently logged-in user's info from the system.
@@ -37,7 +36,6 @@ pub fn get_current_user() -> Option<User> {
         username: nix_user.name,
         display_name,
         home: nix_user.dir,
-        uid: uid.as_raw(),
     })
 }
 
@@ -65,16 +63,16 @@ pub fn get_avatar_path_with(
     }
 
     // AccountsService icon fallback
-    if let Some(name) = username {
-        if accountsservice_dir.exists() {
-            let icon = accountsservice_dir.join(name);
-            if let Ok(meta) = icon.symlink_metadata() {
-                if meta.file_type().is_symlink() {
-                    log::warn!("Rejecting symlink avatar: {}", icon.display());
-                } else if meta.is_file() {
-                    log::debug!("Avatar: using AccountsService icon ({})", icon.display());
-                    return Some(icon);
-                }
+    if let Some(name) = username
+        && accountsservice_dir.exists()
+    {
+        let icon = accountsservice_dir.join(name);
+        if let Ok(meta) = icon.symlink_metadata() {
+            if meta.file_type().is_symlink() {
+                log::warn!("Rejecting symlink avatar: {}", icon.display());
+            } else if meta.is_file() {
+                log::debug!("Avatar: using AccountsService icon ({})", icon.display());
+                return Some(icon);
             }
         }
     }