fix: address audit findings — polling, symlinks, validation, wallpaper removal (v0.7.0)

Three parallel audits (quality, performance, security) identified issues
across the codebase. This commit addresses all remaining findings:

- Replace busy-loop polling in run_command with child.wait() + timeout thread
- Canonicalize ~/.face and AccountsService avatar paths to prevent symlink abuse
- Add detect_locale_with() DI function for testable locale detection
- Move config I/O from activate() to main() to avoid blocking GTK main loop
- Validate background_blur range (0–200), reject invalid values with warning
- Remove embedded wallpaper from GResource — moonarch provides it via filesystem
  (binary size ~3.2MB → ~1.3MB)
This commit is contained in:
nevaforget 2026-03-28 23:09:29 +01:00
parent 71670eb263
commit 5a6900e85a
13 changed files with 242 additions and 95 deletions

View File

@ -3,6 +3,25 @@
All notable changes to this project will be documented in this file. All notable changes to this project will be documented in this file.
Format based on [Keep a Changelog](https://keepachangelog.com/). Format based on [Keep a Changelog](https://keepachangelog.com/).
## [0.7.0] - 2026-03-28
### Added
- Blur validation: `background_blur` must be 0.0200.0 (negative, NaN, infinite, and extreme values are rejected with a warning)
- `detect_locale_with()` testable DI function for locale detection (4 new tests)
- Path canonicalization for `~/.face` and AccountsService avatar paths (resolves symlinks, prevents passing arbitrary files to gdk-pixbuf)
### Changed
- Replace busy-loop polling (`try_wait` + `sleep(100ms)`) in `run_command` with blocking `child.wait()` + timeout thread — eliminates poll latency and thread waste
- Move config loading from `activate()` to `main()` — filesystem I/O no longer blocks the GTK main loop
- Click-to-dismiss now attached to overlay instead of background picture (works with or without wallpaper)
### Removed
- Embedded fallback wallpaper from GResource bundle — moonarch provides `/usr/share/moonarch/wallpaper.jpg` at install time, binary size dropped from ~3.2MB to ~1.3MB
- GResource fallback path in `resolve_background_path` — returns `Option<PathBuf>` now, `None` falls through to CSS background
## [0.6.0] - 2026-03-28 ## [0.6.0] - 2026-03-28
### Added ### Added

View File

@ -17,7 +17,7 @@ Lock, Logout, Hibernate, Reboot, Shutdown.
## Projektstruktur ## Projektstruktur
- `src/` — Rust-Quellcode (main.rs, power.rs, i18n.rs, config.rs, users.rs, panel.rs) - `src/` — Rust-Quellcode (main.rs, power.rs, i18n.rs, config.rs, users.rs, panel.rs)
- `resources/` — GResource-Assets (style.css, wallpaper.jpg komprimiert, default-avatar.svg) - `resources/` — GResource-Assets (style.css, default-avatar.svg)
- `config/` — Beispiel-Konfigurationsdateien - `config/` — Beispiel-Konfigurationsdateien
## Kommandos ## Kommandos
@ -54,6 +54,6 @@ Kurzfassung der wichtigsten Entscheidungen:
- **System-Icons**: Adwaita/Catppuccin liefern alle benötigten symbolischen Icons - **System-Icons**: Adwaita/Catppuccin liefern alle benötigten symbolischen Icons
- **Lock ohne Confirmation**: Lock ist sofort reversibel, braucht kein Confirm - **Lock ohne Confirmation**: Lock ist sofort reversibel, braucht kein Confirm
- **Absolute Pfade für Binaries**: `/usr/bin/systemctl` etc. statt relativer Pfade (Security) - **Absolute Pfade für Binaries**: `/usr/bin/systemctl` etc. statt relativer Pfade (Security)
- **GResource-Bundle**: CSS, Wallpaper (komprimiert) und Default-Avatar sind in die Binary kompiliert - **GResource-Bundle**: CSS und Default-Avatar sind in die Binary kompiliert (Wallpaper kommt vom Dateisystem)
- **Async Power Actions**: `glib::spawn_future_local` + `gio::spawn_blocking` mit 30s Timeout - **Async Power Actions**: `glib::spawn_future_local` + `gio::spawn_blocking` mit 30s Timeout
- **Journal-Logging**: `systemd-journal-logger` statt File-Logging — `journalctl -t moonset`, Debug-Level per `MOONSET_DEBUG` Env-Var - **Journal-Logging**: `systemd-journal-logger` statt File-Logging — `journalctl -t moonset`, Debug-Level per `MOONSET_DEBUG` Env-Var

2
Cargo.lock generated
View File

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

View File

@ -1,6 +1,6 @@
[package] [package]
name = "moonset" name = "moonset"
version = "0.6.0" version = "0.7.0"
edition = "2024" edition = "2024"
description = "Wayland session power menu with GTK4 and Layer Shell" description = "Wayland session power menu with GTK4 and Layer Shell"
license = "MIT" license = "MIT"
@ -14,7 +14,7 @@ gdk-pixbuf = "0.22"
toml = "0.8" toml = "0.8"
dirs = "6" dirs = "6"
serde = { version = "1", features = ["derive"] } serde = { version = "1", features = ["derive"] }
nix = { version = "0.29", features = ["user"] } nix = { version = "0.29", features = ["user", "signal"] }
graphene-rs = { version = "0.22", package = "graphene-rs" } graphene-rs = { version = "0.22", package = "graphene-rs" }
log = "0.4" log = "0.4"
systemd-journal-logger = "2.2" systemd-journal-logger = "2.2"

View File

@ -2,6 +2,13 @@
Architectural and design decisions for Moonset, in reverse chronological order. Architectural and design decisions for Moonset, in reverse chronological order.
## 2026-03-28 Remove wallpaper from GResource bundle
- **Who**: Ragnar, 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 ## 2026-03-28 Switch from env_logger to systemd-journal-logger
- **Who**: Ragnar, Dom - **Who**: Ragnar, Dom

View File

@ -2,7 +2,6 @@
<gresources> <gresources>
<gresource prefix="/dev/moonarch/moonset"> <gresource prefix="/dev/moonarch/moonset">
<file>style.css</file> <file>style.css</file>
<file>wallpaper.jpg</file>
<file>default-avatar.svg</file> <file>default-avatar.svg</file>
</gresource> </gresource>
</gresources> </gresources>

Binary file not shown.

Before

Width:  |  Height:  |  Size: 366 KiB

View File

@ -48,37 +48,44 @@ pub fn load_config(config_paths: Option<&[PathBuf]>) -> Config {
} }
} }
// Validate blur range
if let Some(blur) = merged.background_blur {
if !blur.is_finite() || blur < 0.0 || blur > 200.0 {
log::warn!("Invalid background_blur value {blur}, ignoring");
merged.background_blur = None;
}
}
merged merged
} }
/// Resolve the wallpaper path using the fallback hierarchy. /// Resolve the wallpaper path using the fallback hierarchy.
/// ///
/// Priority: config background_path > Moonarch system default > gresource fallback. /// Priority: config background_path > Moonarch system default.
pub fn resolve_background_path(config: &Config) -> PathBuf { /// Returns None if no wallpaper is available (CSS background shows through).
pub fn resolve_background_path(config: &Config) -> Option<PathBuf> {
resolve_background_path_with(config, Path::new(MOONARCH_WALLPAPER)) resolve_background_path_with(config, Path::new(MOONARCH_WALLPAPER))
} }
/// Resolve with configurable moonarch wallpaper path (for testing). /// Resolve with configurable moonarch wallpaper path (for testing).
pub fn resolve_background_path_with(config: &Config, moonarch_wallpaper: &Path) -> PathBuf { pub fn resolve_background_path_with(config: &Config, moonarch_wallpaper: &Path) -> Option<PathBuf> {
// User-configured path // User-configured path
if let Some(ref bg) = config.background_path { if let Some(ref bg) = config.background_path {
let path = PathBuf::from(bg); let path = PathBuf::from(bg);
if path.is_file() { if path.is_file() {
log::debug!("Wallpaper source: config ({})", path.display()); log::debug!("Wallpaper source: config ({})", path.display());
return path; return Some(path);
} }
} }
// Moonarch ecosystem default // Moonarch ecosystem default
if moonarch_wallpaper.is_file() { if moonarch_wallpaper.is_file() {
log::debug!("Wallpaper source: moonarch default ({})", moonarch_wallpaper.display()); log::debug!("Wallpaper source: moonarch default ({})", moonarch_wallpaper.display());
return moonarch_wallpaper.to_path_buf(); return Some(moonarch_wallpaper.to_path_buf());
} }
// GResource fallback path (loaded from compiled resources at runtime) log::debug!("No wallpaper found, using CSS background");
let prefix = crate::GRESOURCE_PREFIX; None
log::debug!("Wallpaper source: GResource fallback");
PathBuf::from(format!("{prefix}/wallpaper.jpg"))
} }
#[cfg(test)] #[cfg(test)]
@ -164,7 +171,7 @@ mod tests {
}; };
assert_eq!( assert_eq!(
resolve_background_path_with(&config, Path::new("/nonexistent")), resolve_background_path_with(&config, Path::new("/nonexistent")),
wallpaper Some(wallpaper)
); );
} }
@ -175,8 +182,7 @@ mod tests {
..Config::default() ..Config::default()
}; };
let result = resolve_background_path_with(&config, Path::new("/nonexistent")); let result = resolve_background_path_with(&config, Path::new("/nonexistent"));
// Falls through to gresource fallback assert_eq!(result, None);
assert!(result.to_str().unwrap().contains("moonset"));
} }
#[test] #[test]
@ -185,14 +191,14 @@ mod tests {
let moonarch_wp = dir.path().join("wallpaper.jpg"); let moonarch_wp = dir.path().join("wallpaper.jpg");
fs::write(&moonarch_wp, "fake").unwrap(); fs::write(&moonarch_wp, "fake").unwrap();
let config = Config::default(); let config = Config::default();
assert_eq!(resolve_background_path_with(&config, &moonarch_wp), moonarch_wp); assert_eq!(resolve_background_path_with(&config, &moonarch_wp), Some(moonarch_wp));
} }
#[test] #[test]
fn resolve_uses_gresource_fallback_as_last_resort() { fn resolve_returns_none_when_no_wallpaper_available() {
let config = Config::default(); let config = Config::default();
let result = resolve_background_path_with(&config, Path::new("/nonexistent")); let result = resolve_background_path_with(&config, Path::new("/nonexistent"));
assert!(result.to_str().unwrap().contains("wallpaper.jpg")); assert_eq!(result, None);
} }
#[test] #[test]
@ -217,12 +223,52 @@ mod tests {
} }
#[test] #[test]
fn load_config_accepts_negative_blur() { fn load_config_rejects_negative_blur() {
let dir = tempfile::tempdir().unwrap(); let dir = tempfile::tempdir().unwrap();
let conf = dir.path().join("negative.toml"); let conf = dir.path().join("negative.toml");
fs::write(&conf, "background_blur = -5.0\n").unwrap(); fs::write(&conf, "background_blur = -5.0\n").unwrap();
let paths = vec![conf]; let paths = vec![conf];
let config = load_config(Some(&paths)); let config = load_config(Some(&paths));
assert_eq!(config.background_blur, Some(-5.0)); assert_eq!(config.background_blur, None);
}
#[test]
fn load_config_rejects_excessive_blur() {
let dir = tempfile::tempdir().unwrap();
let conf = dir.path().join("huge.toml");
fs::write(&conf, "background_blur = 999.0\n").unwrap();
let paths = vec![conf];
let config = load_config(Some(&paths));
assert_eq!(config.background_blur, None);
}
#[test]
fn load_config_accepts_valid_blur_range() {
let dir = tempfile::tempdir().unwrap();
let conf = dir.path().join("valid.toml");
fs::write(&conf, "background_blur = 50.0\n").unwrap();
let paths = vec![conf];
let config = load_config(Some(&paths));
assert_eq!(config.background_blur, Some(50.0));
}
#[test]
fn load_config_accepts_zero_blur() {
let dir = tempfile::tempdir().unwrap();
let conf = dir.path().join("zero.toml");
fs::write(&conf, "background_blur = 0.0\n").unwrap();
let paths = vec![conf];
let config = load_config(Some(&paths));
assert_eq!(config.background_blur, Some(0.0));
}
#[test]
fn load_config_accepts_max_blur() {
let dir = tempfile::tempdir().unwrap();
let conf = dir.path().join("max.toml");
fs::write(&conf, "background_blur = 200.0\n").unwrap();
let paths = vec![conf];
let config = load_config(Some(&paths));
assert_eq!(config.background_blur, Some(200.0));
} }
} }

View File

@ -112,9 +112,14 @@ fn read_lang_from_conf(path: &Path) -> Option<String> {
/// Determine the system language from LANG env var or /etc/locale.conf. /// Determine the system language from LANG env var or /etc/locale.conf.
pub fn detect_locale() -> String { pub fn detect_locale() -> String {
let (raw, source) = if let Some(val) = env::var("LANG").ok().filter(|s| !s.is_empty()) { detect_locale_with(env::var("LANG").ok().as_deref(), Path::new(DEFAULT_LOCALE_CONF))
(Some(val), "LANG env") }
} else if let Some(val) = read_lang_from_conf(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")
} else if let Some(val) = read_lang_from_conf(locale_conf_path) {
(Some(val), "locale.conf") (Some(val), "locale.conf")
} else { } else {
(None, "default") (None, "default")
@ -264,6 +269,40 @@ mod tests {
} }
} }
// -- detect_locale_with tests --
#[test]
fn detect_locale_uses_env_lang() {
let result = detect_locale_with(Some("de_DE.UTF-8"), Path::new("/nonexistent"));
assert_eq!(result, "de");
}
#[test]
fn detect_locale_falls_back_to_conf_file() {
let dir = tempfile::tempdir().unwrap();
let conf = dir.path().join("locale.conf");
let mut f = fs::File::create(&conf).unwrap();
writeln!(f, "LANG=de_DE.UTF-8").unwrap();
let result = detect_locale_with(None, &conf);
assert_eq!(result, "de");
}
#[test]
fn detect_locale_ignores_empty_env_lang() {
let dir = tempfile::tempdir().unwrap();
let conf = dir.path().join("locale.conf");
let mut f = fs::File::create(&conf).unwrap();
writeln!(f, "LANG=fr_FR.UTF-8").unwrap();
let result = detect_locale_with(Some(""), &conf);
assert_eq!(result, "fr");
}
#[test]
fn detect_locale_defaults_to_english() {
let result = detect_locale_with(None, Path::new("/nonexistent"));
assert_eq!(result, "en");
}
#[test] #[test]
fn error_messages_contain_failed() { fn error_messages_contain_failed() {
let s = load_strings(Some("en")); let s = load_strings(Some("en"));

View File

@ -11,6 +11,7 @@ use gdk4 as gdk;
use gtk4::prelude::*; use gtk4::prelude::*;
use gtk4::{self as gtk, gio}; use gtk4::{self as gtk, gio};
use gtk4_layer_shell::LayerShell; use gtk4_layer_shell::LayerShell;
use std::path::PathBuf;
pub(crate) const GRESOURCE_PREFIX: &str = "/dev/moonarch/moonset"; pub(crate) const GRESOURCE_PREFIX: &str = "/dev/moonarch/moonset";
@ -42,7 +43,7 @@ fn setup_layer_shell(
window.set_anchor(gtk4_layer_shell::Edge::Right, true); window.set_anchor(gtk4_layer_shell::Edge::Right, true);
} }
fn activate(app: &gtk::Application) { fn activate(app: &gtk::Application, bg_path: &Option<PathBuf>, blur_radius: Option<f32>) {
let display = match gdk::Display::default() { let display = match gdk::Display::default() {
Some(d) => d, Some(d) => d,
None => { None => {
@ -53,16 +54,14 @@ fn activate(app: &gtk::Application) {
load_css(&display); load_css(&display);
// Resolve wallpaper once, decode texture once, share across all windows. // Decode texture once (if wallpaper available), share across all windows.
// Blur is applied on the GPU via GskBlurNode at first widget realization, // Blur is applied on the GPU via GskBlurNode at first widget realization,
// then cached and reused by all subsequent windows. // then cached and reused by all subsequent windows.
let config = config::load_config(None); let texture = panel::load_background_texture(bg_path.as_deref());
let bg_path = config::resolve_background_path(&config);
let texture = panel::load_background_texture(&bg_path);
let blur_cache = panel::new_blur_cache(); let blur_cache = panel::new_blur_cache();
// Panel on focused output (no set_monitor → compositor picks focused) // Panel on focused output (no set_monitor → compositor picks focused)
let panel = panel::create_panel_window(&texture, config.background_blur, &blur_cache, app); let panel = panel::create_panel_window(texture.as_ref(), blur_radius, &blur_cache, app);
setup_layer_shell(&panel, true, gtk4_layer_shell::Layer::Overlay); setup_layer_shell(&panel, true, gtk4_layer_shell::Layer::Overlay);
panel.present(); panel.present();
@ -70,7 +69,7 @@ fn activate(app: &gtk::Application) {
let monitors = display.monitors(); let monitors = display.monitors();
for i in 0..monitors.n_items() { for i in 0..monitors.n_items() {
if let Some(monitor) = monitors.item(i).and_then(|obj| obj.downcast::<gdk::Monitor>().ok()) { if let Some(monitor) = monitors.item(i).and_then(|obj| obj.downcast::<gdk::Monitor>().ok()) {
let wallpaper = panel::create_wallpaper_window(&texture, config.background_blur, &blur_cache, app); let wallpaper = panel::create_wallpaper_window(texture.as_ref(), blur_radius, &blur_cache, app);
setup_layer_shell(&wallpaper, false, gtk4_layer_shell::Layer::Top); setup_layer_shell(&wallpaper, false, gtk4_layer_shell::Layer::Top);
wallpaper.set_monitor(Some(&monitor)); wallpaper.set_monitor(Some(&monitor));
wallpaper.present(); wallpaper.present();
@ -104,10 +103,16 @@ fn main() {
// Register compiled GResources // Register compiled GResources
gio::resources_register_include!("moonset.gresource").expect("Failed to register resources"); gio::resources_register_include!("moonset.gresource").expect("Failed to register resources");
// Load config and resolve wallpaper path before GTK app start —
// no GTK types needed, avoids blocking the main loop.
let config = config::load_config(None);
let bg_path = config::resolve_background_path(&config);
let blur_radius = config.background_blur;
let app = gtk::Application::builder() let app = gtk::Application::builder()
.application_id("dev.moonarch.moonset") .application_id("dev.moonarch.moonset")
.build(); .build();
app.connect_activate(activate); app.connect_activate(move |app| activate(app, &bg_path, blur_radius));
app.run(); app.run();
} }

View File

@ -87,19 +87,17 @@ pub fn action_definitions() -> Vec<ActionDef> {
} }
/// Load the wallpaper as a texture once, for sharing across all windows. /// Load the wallpaper as a texture once, for sharing across all windows.
/// Blur is applied on the GPU via GskBlurNode at widget realization time. /// Returns None if no wallpaper path is configured (CSS background shows through).
pub fn load_background_texture(bg_path: &Path) -> gdk::Texture { pub fn load_background_texture(bg_path: Option<&Path>) -> Option<gdk::Texture> {
let fallback = format!("{}/wallpaper.jpg", crate::GRESOURCE_PREFIX); let bg_path = bg_path?;
log::debug!("Background: {}", bg_path.display()); log::debug!("Background: {}", bg_path.display());
if bg_path.starts_with(crate::GRESOURCE_PREFIX) { let file = gio::File::for_path(bg_path);
let resource_path = bg_path.to_str().unwrap_or(&fallback); match gdk::Texture::from_file(&file) {
gdk::Texture::from_resource(resource_path) Ok(texture) => Some(texture),
} else { Err(e) => {
let file = gio::File::for_path(bg_path); log::warn!("Failed to load wallpaper {}: {e}", bg_path.display());
gdk::Texture::from_file(&file).unwrap_or_else(|_| { None
gdk::Texture::from_resource(&fallback) }
})
} }
} }
@ -141,14 +139,16 @@ pub fn new_blur_cache() -> BlurCache {
} }
/// Create a wallpaper-only window for secondary monitors. /// Create a wallpaper-only window for secondary monitors.
pub fn create_wallpaper_window(texture: &gdk::Texture, blur_radius: Option<f32>, blur_cache: &BlurCache, app: &gtk::Application) -> gtk::ApplicationWindow { pub fn create_wallpaper_window(texture: Option<&gdk::Texture>, blur_radius: Option<f32>, blur_cache: &BlurCache, app: &gtk::Application) -> gtk::ApplicationWindow {
let window = gtk::ApplicationWindow::builder() let window = gtk::ApplicationWindow::builder()
.application(app) .application(app)
.build(); .build();
window.add_css_class("wallpaper"); window.add_css_class("wallpaper");
let background = create_background_picture(texture, blur_radius, blur_cache); if let Some(texture) = texture {
window.set_child(Some(&background)); let background = create_background_picture(texture, blur_radius, blur_cache);
window.set_child(Some(&background));
}
// Fade-in on map // Fade-in on map
window.connect_map(|w| { window.connect_map(|w| {
@ -165,7 +165,7 @@ pub fn create_wallpaper_window(texture: &gdk::Texture, blur_radius: Option<f32>,
} }
/// Create the main panel window with action buttons and confirm flow. /// Create the main panel window with action buttons and confirm flow.
pub fn create_panel_window(texture: &gdk::Texture, blur_radius: Option<f32>, blur_cache: &BlurCache, app: &gtk::Application) -> gtk::ApplicationWindow { pub fn create_panel_window(texture: Option<&gdk::Texture>, blur_radius: Option<f32>, blur_cache: &BlurCache, app: &gtk::Application) -> gtk::ApplicationWindow {
let window = gtk::ApplicationWindow::builder() let window = gtk::ApplicationWindow::builder()
.application(app) .application(app)
.build(); .build();
@ -187,9 +187,11 @@ pub fn create_panel_window(texture: &gdk::Texture, blur_radius: Option<f32>, blu
let overlay = gtk::Overlay::new(); let overlay = gtk::Overlay::new();
window.set_child(Some(&overlay)); window.set_child(Some(&overlay));
// Background wallpaper // Background wallpaper (if available, otherwise CSS background shows through)
let background = create_background_picture(texture, blur_radius, blur_cache); if let Some(texture) = texture {
overlay.set_child(Some(&background)); let background = create_background_picture(texture, blur_radius, blur_cache);
overlay.set_child(Some(&background));
}
// Click on background dismisses the menu // Click on background dismisses the menu
let click_controller = gtk::GestureClick::new(); let click_controller = gtk::GestureClick::new();
@ -200,7 +202,7 @@ pub fn create_panel_window(texture: &gdk::Texture, blur_radius: Option<f32>, blu
fade_out_and_quit(&app); fade_out_and_quit(&app);
} }
)); ));
background.add_controller(click_controller); overlay.add_controller(click_controller);
// Centered content box // Centered content box
let content_box = gtk::Box::new(gtk::Orientation::Vertical, 0); let content_box = gtk::Box::new(gtk::Orientation::Vertical, 0);

View File

@ -4,7 +4,9 @@
use std::fmt; use std::fmt;
use std::io::Read; use std::io::Read;
use std::process::{Command, Stdio}; use std::process::{Command, Stdio};
use std::time::{Duration, Instant}; use std::sync::atomic::{AtomicBool, Ordering};
use std::sync::Arc;
use std::time::Duration;
const POWER_TIMEOUT: Duration = Duration::from_secs(30); const POWER_TIMEOUT: Duration = Duration::from_secs(30);
@ -30,6 +32,10 @@ impl fmt::Display for PowerError {
impl std::error::Error for PowerError {} impl std::error::Error for PowerError {}
/// Run a command with timeout and return a PowerError on failure. /// Run a command with timeout and return a PowerError on failure.
///
/// Uses blocking `child.wait()` with a separate timeout thread that sends
/// SIGKILL after POWER_TIMEOUT. This runs inside `gio::spawn_blocking`,
/// so blocking is expected.
fn run_command(action: &'static str, program: &str, args: &[&str]) -> Result<(), PowerError> { fn run_command(action: &'static str, program: &str, args: &[&str]) -> Result<(), PowerError> {
log::debug!("Power action: {action} ({program} {args:?})"); log::debug!("Power action: {action} ({program} {args:?})");
let mut child = Command::new(program) let mut child = Command::new(program)
@ -42,40 +48,54 @@ fn run_command(action: &'static str, program: &str, args: &[&str]) -> Result<(),
message: e.to_string(), message: e.to_string(),
})?; })?;
let deadline = Instant::now() + POWER_TIMEOUT; let child_pid = nix::unistd::Pid::from_raw(child.id() as i32);
loop { let done = Arc::new(AtomicBool::new(false));
match child.try_wait() { let done_clone = done.clone();
Ok(Some(status)) => {
if status.success() { let timeout_thread = std::thread::spawn(move || {
log::debug!("Power action {action} completed"); // Sleep in short intervals so we can exit early when the child finishes
} let interval = Duration::from_millis(100);
if !status.success() { let mut elapsed = Duration::ZERO;
let mut stderr_buf = String::new(); while elapsed < POWER_TIMEOUT {
if let Some(mut stderr) = child.stderr.take() { std::thread::sleep(interval);
let _ = stderr.read_to_string(&mut stderr_buf); if done_clone.load(Ordering::Relaxed) {
} return;
return Err(PowerError::CommandFailed {
action,
message: format!("exit code {}: {}", status, stderr_buf.trim()),
});
}
return Ok(());
} }
Ok(None) => { elapsed += interval;
if Instant::now() >= deadline { }
let _ = child.kill(); // ESRCH if the process already exited — harmless
let _ = child.wait(); let _ = nix::sys::signal::kill(child_pid, nix::sys::signal::Signal::SIGKILL);
return Err(PowerError::Timeout { action }); });
}
std::thread::sleep(Duration::from_millis(100)); let status = child.wait().map_err(|e| PowerError::CommandFailed {
} action,
Err(e) => { message: e.to_string(),
return Err(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 });
} }
} }
let mut stderr_buf = String::new();
if let Some(mut stderr) = child.stderr.take() {
let _ = stderr.read_to_string(&mut stderr_buf);
}
Err(PowerError::CommandFailed {
action,
message: format!("exit code {}: {}", status, stderr_buf.trim()),
})
} }
} }

View File

@ -52,20 +52,27 @@ pub fn get_avatar_path_with(
username: Option<&str>, username: Option<&str>,
accountsservice_dir: &Path, accountsservice_dir: &Path,
) -> Option<PathBuf> { ) -> Option<PathBuf> {
// ~/.face takes priority // ~/.face takes priority — canonicalize to resolve symlinks
let face = home.join(".face"); let face = home.join(".face");
if face.exists() { if face.exists() {
log::debug!("Avatar: using ~/.face"); if let Ok(canonical) = std::fs::canonicalize(&face) {
return Some(face); log::debug!("Avatar: using ~/.face ({})", canonical.display());
return Some(canonical);
}
// canonicalize failed (e.g. permissions) — skip rather than return unresolved symlink
log::warn!("Avatar: ~/.face exists but canonicalize failed, skipping");
} }
// AccountsService icon // AccountsService icon — also canonicalize for consistency
if let Some(name) = username { if let Some(name) = username {
if accountsservice_dir.exists() { if accountsservice_dir.exists() {
let icon = accountsservice_dir.join(name); let icon = accountsservice_dir.join(name);
if icon.exists() { if icon.exists() {
log::debug!("Avatar: using AccountsService icon"); if let Ok(canonical) = std::fs::canonicalize(&icon) {
return Some(icon); log::debug!("Avatar: using AccountsService icon ({})", canonical.display());
return Some(canonical);
}
log::warn!("Avatar: AccountsService icon exists but canonicalize failed, skipping");
} }
} }
} }
@ -100,7 +107,8 @@ mod tests {
let face = dir.path().join(".face"); let face = dir.path().join(".face");
fs::write(&face, "fake image").unwrap(); fs::write(&face, "fake image").unwrap();
let path = get_avatar_path_with(dir.path(), None, Path::new("/nonexistent")); let path = get_avatar_path_with(dir.path(), None, Path::new("/nonexistent"));
assert_eq!(path, Some(face)); let expected = fs::canonicalize(&face).unwrap();
assert_eq!(path, Some(expected));
} }
#[test] #[test]
@ -111,7 +119,8 @@ mod tests {
let icon = icons_dir.join("testuser"); let icon = icons_dir.join("testuser");
fs::write(&icon, "fake image").unwrap(); fs::write(&icon, "fake image").unwrap();
let path = get_avatar_path_with(dir.path(), Some("testuser"), &icons_dir); let path = get_avatar_path_with(dir.path(), Some("testuser"), &icons_dir);
assert_eq!(path, Some(icon)); let expected = fs::canonicalize(&icon).unwrap();
assert_eq!(path, Some(expected));
} }
#[test] #[test]
@ -124,7 +133,8 @@ mod tests {
let icon = icons_dir.join("testuser"); let icon = icons_dir.join("testuser");
fs::write(&icon, "fake image").unwrap(); fs::write(&icon, "fake image").unwrap();
let path = get_avatar_path_with(dir.path(), Some("testuser"), &icons_dir); let path = get_avatar_path_with(dir.path(), Some("testuser"), &icons_dir);
assert_eq!(path, Some(face)); let expected = fs::canonicalize(&face).unwrap();
assert_eq!(path, Some(expected));
} }
#[test] #[test]