fix: audit LOW fixes — stdout null, utf-8 path, debug value, hidden sessions (v0.8.6)
All checks were successful
Update PKGBUILD version / update-pkgver (push) Successful in 2s
All checks were successful
Update PKGBUILD version / update-pkgver (push) Successful in 2s
- power::run_command: .stdout(Stdio::null()) — the pipe was never drained, structurally fragile even if no current caller hits it. - config: replace to_string_lossy() on relative wallpaper paths with to_str() + log::warn, so non-UTF-8 paths are dropped cleanly instead of being mangled into unopenable U+FFFD strings. - main: require MOONGREET_DEBUG=1 to raise verbosity. Mere presence of the var must not leak socket paths, usernames, and auth round counts into the journal. - sessions: parse Hidden= and NoDisplay= keys, skip entries marked true. Keeps disabled or stub .desktop files out of the session dropdown.
This commit is contained in:
parent
3a1af6471f
commit
b9b6f50974
2
Cargo.lock
generated
2
Cargo.lock
generated
@ -575,7 +575,7 @@ dependencies = [
|
||||
|
||||
[[package]]
|
||||
name = "moongreet"
|
||||
version = "0.8.5"
|
||||
version = "0.8.6"
|
||||
dependencies = [
|
||||
"gdk-pixbuf",
|
||||
"gdk4",
|
||||
|
||||
@ -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"
|
||||
|
||||
@ -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
|
||||
|
||||
@ -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 {
|
||||
|
||||
10
src/main.rs
10
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);
|
||||
}
|
||||
|
||||
@ -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 {
|
||||
|
||||
@ -23,6 +23,8 @@ fn parse_desktop_file(path: &Path, session_type: &str) -> Option<Session> {
|
||||
let mut in_section = false;
|
||||
let mut name: Option<String> = None;
|
||||
let mut exec_cmd: Option<String> = 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<Session> {
|
||||
&& 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());
|
||||
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user