diff --git a/Cargo.lock b/Cargo.lock index fcd3931..4df5c03 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -575,7 +575,7 @@ dependencies = [ [[package]] name = "moongreet" -version = "0.8.5" +version = "0.8.6" dependencies = [ "gdk-pixbuf", "gdk4", diff --git a/Cargo.toml b/Cargo.toml index 05ab797..d85a828 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "moongreet" -version = "0.8.5" +version = "0.8.6" edition = "2024" description = "A greetd greeter for Wayland with GTK4 and Layer Shell" license = "MIT" diff --git a/DECISIONS.md b/DECISIONS.md index ce6b36c..a909799 100644 --- a/DECISIONS.md +++ b/DECISIONS.md @@ -1,5 +1,12 @@ # 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 diff --git a/src/config.rs b/src/config.rs index 4c7f4d6..ee91eee 100644 --- a/src/config.rs +++ b/src/config.rs @@ -68,8 +68,14 @@ pub fn load_config(config_paths: Option<&[PathBuf]>) -> Config { if bg_path.is_absolute() { merged.background_path = Some(bg); } else if let Some(parent) = path.parent() { - merged.background_path = - Some(parent.join(&bg).to_string_lossy().to_string()); + let joined = parent.join(&bg); + match joined.to_str() { + Some(s) => merged.background_path = Some(s.to_string()), + None => log::warn!( + "Ignoring non-UTF-8 background path: {}", + joined.display() + ), + } } } if let Some(blur) = appearance.background_blur { diff --git a/src/main.rs b/src/main.rs index d225afd..b6a2a87 100644 --- a/src/main.rs +++ b/src/main.rs @@ -127,10 +127,12 @@ fn setup_logging() { eprintln!("Failed to create journal logger: {e}"); } } - let level = if std::env::var("MOONGREET_DEBUG").is_ok() { - log::LevelFilter::Debug - } else { - log::LevelFilter::Info + // Require MOONGREET_DEBUG=1 to raise verbosity. Mere presence (e.g. an + // empty value in a session-setup script) must not escalate the journal + // to Debug, which leaks socket paths, usernames, and auth round counts. + let level = match std::env::var("MOONGREET_DEBUG").ok().as_deref() { + Some("1") => log::LevelFilter::Debug, + _ => log::LevelFilter::Info, }; log::set_max_level(level); } diff --git a/src/power.rs b/src/power.rs index 2f361ee..ec8b961 100644 --- a/src/power.rs +++ b/src/power.rs @@ -40,7 +40,9 @@ fn run_command(action: &'static str, program: &str, args: &[&str]) -> Result<(), log::debug!("Power action: {action} ({program} {args:?})"); let mut child = Command::new(program) .args(args) - .stdout(Stdio::piped()) + // stdout is never read; piping without draining would deadlock on any + // command that writes more than one OS pipe buffer before wait() returns. + .stdout(Stdio::null()) .stderr(Stdio::piped()) .spawn() .map_err(|e| PowerError::CommandFailed { diff --git a/src/sessions.rs b/src/sessions.rs index 96d3a6e..09828bf 100644 --- a/src/sessions.rs +++ b/src/sessions.rs @@ -23,6 +23,8 @@ fn parse_desktop_file(path: &Path, session_type: &str) -> Option { let mut in_section = false; let mut name: Option = None; let mut exec_cmd: Option = None; + let mut hidden = false; + let mut no_display = false; for line in content.lines() { let line = line.trim(); @@ -44,9 +46,18 @@ fn parse_desktop_file(path: &Path, session_type: &str) -> Option { && exec_cmd.is_none() { exec_cmd = Some(value.to_string()); + } else if let Some(value) = line.strip_prefix("Hidden=") { + hidden = value.eq_ignore_ascii_case("true"); + } else if let Some(value) = line.strip_prefix("NoDisplay=") { + no_display = value.eq_ignore_ascii_case("true"); } } + if hidden || no_display { + log::debug!("Skipping {}: Hidden/NoDisplay entry", path.display()); + return None; + } + let name = name.filter(|s| !s.is_empty()); let exec_cmd = exec_cmd.filter(|s| !s.is_empty());