diff --git a/data/start-hyprcosmic b/data/start-hyprcosmic index 573ee0a..c95652f 100755 --- a/data/start-hyprcosmic +++ b/data/start-hyprcosmic @@ -10,10 +10,21 @@ set -e -# Default to the installed binaries. Override either one to point at an -# in-tree debug build when testing the fork without installing it. -HYPRCOSMIC_SESSION_BIN="${HYPRCOSMIC_SESSION_BIN:-/usr/bin/cosmic-session}" -HYPRCOSMIC_COMP_BIN="${HYPRCOSMIC_COMP_BIN:-cosmic-comp}" +# The forked binaries, installed alongside rather than over the stock ones. +# +# Pointing at /usr/bin/cosmic-session here would be a silent no-op: the system +# binary has no profile module, so the session would come up as ordinary COSMIC +# and the gating would look broken. Installing to a private prefix instead of +# replacing /usr/bin also means `dnf update cosmic-session` cannot clobber the +# fork, and the stock session entry keeps working as the escape hatch. +HYPRCOSMIC_SESSION_BIN="${HYPRCOSMIC_SESSION_BIN:-/usr/libexec/hyprcosmic/cosmic-session}" +HYPRCOSMIC_COMP_BIN="${HYPRCOSMIC_COMP_BIN:-/usr/libexec/hyprcosmic/cosmic-comp}" + +if [[ ! -x "$HYPRCOSMIC_SESSION_BIN" ]]; then + echo "start-hyprcosmic: $HYPRCOSMIC_SESSION_BIN is missing or not executable" >&2 + echo "start-hyprcosmic: log out and choose the stock COSMIC session" >&2 + exit 1 +fi # Selects the component set in cosmic-session's profile module: cosmic-panel, # cosmic-launcher, cosmic-app-library, cosmic-workspaces, cosmic-bg and diff --git a/src/main.rs b/src/main.rs index 8681e78..e8181e4 100644 --- a/src/main.rs +++ b/src/main.rs @@ -358,9 +358,19 @@ async fn start( // Profile extras (waybar, swww, ...) come last, so they start against a // session that already has its compositor-side services up. - for command in profile::Profile::cached().extra() { + for argv in profile::Profile::cached().extra() { + let Some((exe, args)) = argv.split_first() else { + continue; + }; let span = info_span!(parent: None, "hyprcosmic-extra"); - start_component(command.clone(), span, &process_manager, &env_vars).await; + start_process( + Cow::Owned(exe.clone()), + args.to_vec(), + span, + &process_manager, + &env_vars, + ) + .await; } #[cfg(feature = "autostart")] @@ -529,6 +539,24 @@ async fn start_component( return; } + start_process(cmd, Vec::new(), span, process_manager, env_vars).await; +} + +/// Upstream's `start_component` body, with argv threaded through. +/// +/// Profile extras need arguments -- `waybar -c ` is the motivating case -- +/// and `Process::with_executable` treats its whole argument as the program name, +/// so spawning "waybar -c x" would search for a binary with spaces in its name. +/// Splitting the spawn out here gives extras real arguments without a near-copy +/// of this function, and leaves `start_component`'s signature alone so upstream +/// call sites stay untouched. +async fn start_process( + cmd: Cow<'static, str>, + args: Vec, + span: tracing::Span, + process_manager: &ProcessManager, + env_vars: &[(String, String)], +) { let stdout_span = span.clone(); let stderr_span = span.clone(); let stderr_span_clone = stderr_span.clone(); @@ -538,6 +566,7 @@ async fn start_component( .start( Process::new() .with_executable(cmd.clone()) + .with_args(args) .with_env(env_vars.iter().cloned()) .with_on_stdout(move |_, _, line| { let stdout_span = stdout_span.clone(); diff --git a/src/profile.rs b/src/profile.rs index 1f3e3c0..bee6631 100644 --- a/src/profile.rs +++ b/src/profile.rs @@ -25,8 +25,9 @@ pub struct Profile { pub name: String, /// Upstream components to skip. disabled: BTreeSet, - /// Additional commands to launch after the built-in set. - extra: Vec, + /// Additional commands to launch after the built-in set, each already + /// split into argv. + extra: Vec>, } impl Default for Profile { @@ -100,7 +101,7 @@ impl Profile { !self.disabled.contains(component) } - pub fn extra(&self) -> &[String] { + pub fn extra(&self) -> &[Vec] { &self.extra } } @@ -118,25 +119,62 @@ fn extras_path() -> Option { /// A missing file is normal, not an error — most sessions will not have one. /// Deliberately not a shell: each line is exec'd directly, so a stray /// backtick in a config cannot run something unexpected. -fn read_extras(path: &std::path::Path) -> Vec { +fn read_extras(path: &std::path::Path) -> Vec> { let Ok(text) = std::fs::read_to_string(path) else { return Vec::new(); }; parse_extras(&text) } -fn parse_extras(text: &str) -> Vec { +fn parse_extras(text: &str) -> Vec> { text.lines() - .map(|l| match l.find('#') { - Some(i) => &l[..i], - None => l, - }) - .map(str::trim) - .filter(|l| !l.is_empty()) - .map(String::from) + .map(split_argv) + .filter(|argv| !argv.is_empty()) .collect() } +/// Split one line into argv the way a user expects, without being a shell. +/// +/// Quoting groups words that contain spaces, which config paths do, and `#` +/// begins a comment where a word would start -- both matching shell intuition. +/// Nothing else is interpreted: no variable expansion, no globbing, no command +/// substitution. So a backtick or `$(...)` in this file is inert text, and the +/// file cannot be turned into an execution vector by something that can write +/// to it but not to the binaries it names. +fn split_argv(line: &str) -> Vec { + let mut argv = Vec::new(); + let mut word = String::new(); + let mut started = false; + let mut quote: Option = None; + + let mut push = |word: &mut String, started: &mut bool| { + if *started { + argv.push(std::mem::take(word)); + *started = false; + } + }; + + for c in line.chars() { + match quote { + // Closing quote. `started` stays set, so `""` yields an empty arg. + Some(q) if c == q => quote = None, + Some(_) => word.push(c), + None if c == '\'' || c == '"' => { + quote = Some(c); + started = true; + } + None if c == '#' && !started => break, + None if c.is_whitespace() => push(&mut word, &mut started), + None => { + word.push(c); + started = true; + } + } + } + push(&mut word, &mut started); + argv +} + #[cfg(test)] mod tests { use super::*; @@ -182,7 +220,60 @@ mod tests { let text = "\n# a comment\nwaybar\n swww-daemon \n\nrofi -show drun # trailing\n"; assert_eq!( parse_extras(text), - vec!["waybar", "swww-daemon", "rofi -show drun"] + vec![ + vec!["waybar"], + vec!["swww-daemon"], + vec!["rofi", "-show", "drun"], + ] + ); + } + + #[test] + fn extras_split_into_argv_not_one_long_program_name() { + // Regression: `Process::with_executable` takes the whole string as the + // program name, so an unsplit line looks for a binary literally called + // "waybar -c /path". The bar silently never starts. + assert_eq!( + split_argv("waybar -c /etc/waybar/config.jsonc"), + vec!["waybar", "-c", "/etc/waybar/config.jsonc"] + ); + } + + #[test] + fn quotes_hold_arguments_containing_spaces_together() { + assert_eq!( + split_argv("waybar -c '/home/a b/config.jsonc' -s \"/home/a b/style.css\""), + vec![ + "waybar", + "-c", + "/home/a b/config.jsonc", + "-s", + "/home/a b/style.css", + ] + ); + } + + #[test] + fn hash_is_a_comment_between_words_but_data_inside_one() { + // Matches shell intuition, and keeps hex colours usable as arguments. + assert_eq!(split_argv("swaybg -c #1a1b26"), vec!["swaybg", "-c"]); + assert_eq!( + split_argv("swaybg -c '#1a1b26'"), + vec!["swaybg", "-c", "#1a1b26"] + ); + assert_eq!( + split_argv("swaybg --color=#1a1b26"), + vec!["swaybg", "--color=#1a1b26"] + ); + } + + #[test] + fn nothing_is_expanded_so_the_file_is_not_an_execution_vector() { + // A writer of this file gets to name a program and its arguments, and + // nothing more: no subshell, no variable, no glob. + assert_eq!( + split_argv("waybar $(id) `id` $HOME *"), + vec!["waybar", "$(id)", "`id`", "$HOME", "*"] ); }