From aa326c4ecb72f56108c2ad37bb130b3f4f62da3c Mon Sep 17 00:00:00 2001 From: Breadway Date: Mon, 31 Aug 2026 15:37:52 +0800 Subject: [PATCH] bread-theme/bread-utils: XDG env hardening and safer temp-file writes - `XDG_CONFIG_HOME` / `XDG_RUNTIME_DIR` are honoured only when absolute (XDG spec; matches `bread_utils::xdg`), in layerrules and output. - `output::atomic_write` pid-suffixes its temp name (`..tmp.`) so two concurrent `bread-theme generate-output` runs writing the same `themes/.css` can't race on one shared `.tmp`. - `hypr::socket_path` reconstructs `/run/user/` from the process's real uid when `XDG_RUNTIME_DIR` is unset, instead of assuming 1000. - `bread-theme --help` prints to stdout (pipeable); usage on an error path (missing args) goes to stderr. --- bread-theme/src/bin/bread-theme.rs | 35 ++++++++++++++++++---------- bread-theme/src/layerrules.rs | 37 ++++++++++++++++++++++++++---- bread-theme/src/output.rs | 15 ++++++++---- bread-utils/src/hypr.rs | 37 ++++++++++++++++++++++++++++-- 4 files changed, 102 insertions(+), 22 deletions(-) diff --git a/bread-theme/src/bin/bread-theme.rs b/bread-theme/src/bin/bread-theme.rs index a6d5b49..790f65a 100644 --- a/bread-theme/src/bin/bread-theme.rs +++ b/bread-theme/src/bin/bread-theme.rs @@ -29,8 +29,9 @@ fn write_and_report(verb: &str) -> ExitCode { } } -fn print_help() { - eprintln!( +fn print_help_to(mut w: impl std::io::Write) { + let _ = write!( + &mut w, "bread-theme — shared stylesheet generator\n\n\ USAGE:\n\ \x20 bread-theme [generate|reload|path|print|layerrules]\n\ @@ -54,19 +55,29 @@ fn print_help() { ); } +/// Usage/help text. Explicitly requested help (`--help`/`-h`) goes to +/// **stdout** so it can be piped/grepped; the same text on an error path +/// (e.g. `generate-output` with no args) goes to stderr via +/// [`print_help_err`]. +fn print_help() { + print_help_to(std::io::stdout()); +} + +fn print_help_err() { + print_help_to(std::io::stderr()); +} + fn generate_output_cmd() -> ExitCode { let args: Vec = std::env::args().skip(2).collect(); - if args.is_empty() - || args - .iter() - .any(|a| matches!(a.as_str(), "-h" | "--help" | "help")) - { + if args.iter().any(|a| matches!(a.as_str(), "-h" | "--help" | "help")) { print_help(); - return if args.is_empty() { - ExitCode::FAILURE - } else { - ExitCode::SUCCESS - }; + return ExitCode::SUCCESS; + } + if args.is_empty() { + // Missing arguments is an error, not a help request — usage goes to + // stderr. + print_help_err(); + return ExitCode::FAILURE; } let output = args[0].as_str(); diff --git a/bread-theme/src/layerrules.rs b/bread-theme/src/layerrules.rs index 76bcca8..a59e629 100644 --- a/bread-theme/src/layerrules.rs +++ b/bread-theme/src/layerrules.rs @@ -16,9 +16,12 @@ use std::path::PathBuf; use crate::shell::ShellTheme; fn config_home() -> PathBuf { + // XDG spec: `XDG_CONFIG_HOME` is only honored when it's an *absolute* + // path; a relative value must be ignored (matches `bread_utils::xdg`). if let Ok(v) = std::env::var("XDG_CONFIG_HOME") { - if !v.is_empty() { - return PathBuf::from(v); + let p = PathBuf::from(&v); + if p.is_absolute() { + return p; } } dirs::home_dir() @@ -110,6 +113,32 @@ mod tests { }); } + /// True if any file under `dir` has a name containing `.tmp.` (a + /// leftover from `output::atomic_write`'s pid-suffixed temp files). + fn has_leftover_tmp(dir: &Path) -> bool { + fn walk(p: &Path) -> bool { + let Ok(entries) = std::fs::read_dir(p) else { + return false; + }; + for entry in entries.flatten() { + let path = entry.path(); + if path.is_dir() { + if walk(&path) { + return true; + } + } else if path + .file_name() + .and_then(|n| n.to_str()) + .is_some_and(|n| n.contains(".tmp.")) + { + return true; + } + } + false + } + walk(dir) + } + /// `load_named("liquid-motion")` resolves through discovery (user dir, /// then system dir, then the compiled-in builtin) — isolate /// `XDG_CONFIG_HOME` to an empty dir so this can't pick up a real @@ -166,7 +195,7 @@ mod tests { assert_eq!(path, dir.join("hypr").join("layerrules.json")); assert!(path.is_file()); // No leftover .tmp file after the atomic rename. - assert!(!path.with_file_name("layerrules.json.tmp").exists()); + assert!(!has_leftover_tmp(dir)); let contents = std::fs::read_to_string(&path).unwrap(); let value: serde_json::Value = serde_json::from_str(&contents).unwrap(); @@ -176,7 +205,7 @@ mod tests { // leave a stale temp file behind. let path2 = write_layerrules_active().unwrap(); assert_eq!(path, path2); - assert!(!path.with_file_name("layerrules.json.tmp").exists()); + assert!(!has_leftover_tmp(dir)); }); } } diff --git a/bread-theme/src/output.rs b/bread-theme/src/output.rs index 1b1f9f4..ebf3a1b 100644 --- a/bread-theme/src/output.rs +++ b/bread-theme/src/output.rs @@ -9,9 +9,12 @@ use crate::{load_palette, stylesheet}; /// Session-scoped `$XDG_RUNTIME_DIR/bread`, same fallback as [`crate::shared_css_path`]. pub(crate) fn runtime_bread_dir() -> PathBuf { + // XDG spec: `XDG_RUNTIME_DIR` is only honored when set to a non-empty, + // *absolute* path; a relative value must be ignored. if let Ok(rt) = std::env::var("XDG_RUNTIME_DIR") { - if !rt.is_empty() { - return PathBuf::from(rt).join("bread"); + let p = PathBuf::from(&rt); + if !rt.is_empty() && p.is_absolute() { + return p.join("bread"); } } dirs::cache_dir() @@ -58,9 +61,13 @@ pub(crate) fn atomic_write(path: &Path, contents: &str) -> std::io::Result<()> { if let Some(parent) = path.parent() { std::fs::create_dir_all(parent)?; } + // Pad the temp name with the pid (matching `bread_utils::atomic`) so two + // concurrent writers for the same target can't race on one shared `.tmp` + // file — e.g. two `bread-theme generate-output` runs writing the same + // `themes/.css` at once. let tmp = match path.file_name().and_then(|n| n.to_str()) { - Some(name) => path.with_file_name(format!("{name}.tmp")), - None => path.with_extension("tmp"), + Some(name) => path.with_file_name(format!(".{name}.tmp.{}", std::process::id())), + None => path.with_extension(format!("tmp.{}", std::process::id())), }; std::fs::write(&tmp, contents)?; std::fs::rename(&tmp, path)?; diff --git a/bread-utils/src/hypr.rs b/bread-utils/src/hypr.rs index def3e98..47bb717 100644 --- a/bread-utils/src/hypr.rs +++ b/bread-utils/src/hypr.rs @@ -36,10 +36,17 @@ pub enum Socket { /// `HYPRLAND_INSTANCE_SIGNATURE` + `XDG_RUNTIME_DIR`. Returns `None` if /// `HYPRLAND_INSTANCE_SIGNATURE` isn't set (Hyprland isn't running, or we're /// not inside a Hyprland session) — `XDG_RUNTIME_DIR` falls back to -/// `/run/user/1000` if unset, matching `breadmon`'s existing fallback. +/// `/run/user/` (this process's real uid, or the historical +/// `/run/user/1000` if that can't be read) when unset. pub fn socket_path(kind: Socket) -> Option { let sig = env::var("HYPRLAND_INSTANCE_SIGNATURE").ok()?; - let rt = env::var("XDG_RUNTIME_DIR").unwrap_or_else(|_| "/run/user/1000".to_string()); + // `XDG_RUNTIME_DIR` is normally `/run/user/`; when it's unset, + // reconstruct that from the process's real uid instead of assuming uid + // 1000, so the socket path is right for any account. + let rt = env::var("XDG_RUNTIME_DIR") + .ok() + .filter(|s| !s.is_empty()) + .unwrap_or_else(fallback_runtime_dir); let file = match kind { Socket::Request => ".socket.sock", Socket::Events => ".socket2.sock", @@ -47,6 +54,21 @@ pub fn socket_path(kind: Socket) -> Option { Some(PathBuf::from(format!("{rt}/hypr/{sig}/{file}"))) } +/// `XDG_RUNTIME_DIR`'s conventional `/run/user/` value, derived from +/// this process's real uid (`Uid:` line in `/proc/self/status`). Falls back +/// to the historical `/run/user/1000` if that can't be read. +fn fallback_runtime_dir() -> String { + let status = std::fs::read_to_string("/proc/self/status").unwrap_or_default(); + for line in status.lines() { + if let Some(rest) = line.strip_prefix("Uid:") { + if let Some(uid) = rest.split_whitespace().next() { + return format!("/run/user/{uid}"); + } + } + } + "/run/user/1000".to_string() +} + /// Send `request` (e.g. `"j/activewindow"`, `"j/monitors"`) to the socket1 /// IPC socket and return the raw response body. Blocking/synchronous — this /// matches every current consumer (breadbox, breadclip), which call it from @@ -222,6 +244,17 @@ mod tests { assert!(win.fullscreen.is_fullscreen()); } + #[test] + fn fallback_runtime_dir_is_user_uid_shaped() { + let dir = fallback_runtime_dir(); + assert!(dir.starts_with("/run/user/"), "got {dir}"); + let uid = dir.trim_start_matches("/run/user/"); + assert!( + !uid.is_empty() && uid.chars().all(|c| c.is_ascii_digit()), + "fallback uid is not numeric: {uid}" + ); + } + // Both env-var-dependent cases share one test function: `set_var`/ // `remove_var` are process-global, and cargo runs tests in parallel // threads by default, so two separate #[test] fns racing on the same