fix: audit LOW fixes — dead uid, home_dir warn, clippy sweep, debug value (v0.8.5)
All checks were successful
Update PKGBUILD version / update-pkgver (push) Successful in 2s

- 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.
This commit is contained in:
nevaforget 2026-04-24 14:14:11 +02:00
parent 0789e8fc27
commit 8285bcdf44
6 changed files with 37 additions and 25 deletions

2
Cargo.lock generated
View File

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

View File

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

View File

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

View File

@ -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);
}

View File

@ -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);
@ -571,7 +576,7 @@ fn execute_action(
#[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(())) => {

View File

@ -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);
}
}
}