mirror of
https://github.com/outbackdingo/hyprcosmic-session.git
synced 2026-08-25 14:53:23 +00:00
Give profile extras real argv, and point the session entry at the fork
Two defects in the previous commit, both of which would have failed silently at the greeter rather than loudly at build time. `start_component` passes its whole string to `Process::with_executable`, which treats it as the program name. An autostart line like `waybar -c <path>` was therefore looked up as a binary literally called "waybar -c <path>", and the bar would simply never appear. The unit test asserting that "rofi -show drun" parses as one entry implied arguments worked; they did not. Extras are now split into argv and spawned through a `start_process` helper, which is upstream's `start_component` body with args threaded through -- `start_component` keeps its signature so no upstream call site changes. The splitter honours quoting, because config paths contain spaces, and treats `#` as a comment only where a word would start, so `--color=#1a1b26` survives. It expands nothing else: no variables, globs or command substitution, so a file that names programs cannot be escalated into running arbitrary shell. `start-hyprcosmic` also defaulted HYPRCOSMIC_SESSION_BIN to /usr/bin/cosmic-session -- the stock binary, which has no profile module. The session would have come up as ordinary COSMIC with the gating apparently doing nothing. It now points at /usr/libexec/hyprcosmic, so the fork installs alongside the packaged COSMIC instead of over it, a dnf update cannot clobber it, and the stock entry stays as the escape hatch. It fails loudly with a pointer back to that entry if the binary is missing. Verified: 9 unit tests, including a regression test for the argv defect and one asserting nothing is expanded; `cargo build -j1` clean.
This commit is contained in:
+15
-4
@@ -10,10 +10,21 @@
|
|||||||
|
|
||||||
set -e
|
set -e
|
||||||
|
|
||||||
# Default to the installed binaries. Override either one to point at an
|
# The forked binaries, installed alongside rather than over the stock ones.
|
||||||
# in-tree debug build when testing the fork without installing it.
|
#
|
||||||
HYPRCOSMIC_SESSION_BIN="${HYPRCOSMIC_SESSION_BIN:-/usr/bin/cosmic-session}"
|
# Pointing at /usr/bin/cosmic-session here would be a silent no-op: the system
|
||||||
HYPRCOSMIC_COMP_BIN="${HYPRCOSMIC_COMP_BIN:-cosmic-comp}"
|
# 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,
|
# Selects the component set in cosmic-session's profile module: cosmic-panel,
|
||||||
# cosmic-launcher, cosmic-app-library, cosmic-workspaces, cosmic-bg and
|
# cosmic-launcher, cosmic-app-library, cosmic-workspaces, cosmic-bg and
|
||||||
|
|||||||
+31
-2
@@ -358,9 +358,19 @@ async fn start(
|
|||||||
|
|
||||||
// Profile extras (waybar, swww, ...) come last, so they start against a
|
// Profile extras (waybar, swww, ...) come last, so they start against a
|
||||||
// session that already has its compositor-side services up.
|
// 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");
|
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")]
|
#[cfg(feature = "autostart")]
|
||||||
@@ -529,6 +539,24 @@ async fn start_component(
|
|||||||
return;
|
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 <path>` 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<String>,
|
||||||
|
span: tracing::Span,
|
||||||
|
process_manager: &ProcessManager,
|
||||||
|
env_vars: &[(String, String)],
|
||||||
|
) {
|
||||||
let stdout_span = span.clone();
|
let stdout_span = span.clone();
|
||||||
let stderr_span = span.clone();
|
let stderr_span = span.clone();
|
||||||
let stderr_span_clone = stderr_span.clone();
|
let stderr_span_clone = stderr_span.clone();
|
||||||
@@ -538,6 +566,7 @@ async fn start_component(
|
|||||||
.start(
|
.start(
|
||||||
Process::new()
|
Process::new()
|
||||||
.with_executable(cmd.clone())
|
.with_executable(cmd.clone())
|
||||||
|
.with_args(args)
|
||||||
.with_env(env_vars.iter().cloned())
|
.with_env(env_vars.iter().cloned())
|
||||||
.with_on_stdout(move |_, _, line| {
|
.with_on_stdout(move |_, _, line| {
|
||||||
let stdout_span = stdout_span.clone();
|
let stdout_span = stdout_span.clone();
|
||||||
|
|||||||
+104
-13
@@ -25,8 +25,9 @@ pub struct Profile {
|
|||||||
pub name: String,
|
pub name: String,
|
||||||
/// Upstream components to skip.
|
/// Upstream components to skip.
|
||||||
disabled: BTreeSet<String>,
|
disabled: BTreeSet<String>,
|
||||||
/// Additional commands to launch after the built-in set.
|
/// Additional commands to launch after the built-in set, each already
|
||||||
extra: Vec<String>,
|
/// split into argv.
|
||||||
|
extra: Vec<Vec<String>>,
|
||||||
}
|
}
|
||||||
|
|
||||||
impl Default for Profile {
|
impl Default for Profile {
|
||||||
@@ -100,7 +101,7 @@ impl Profile {
|
|||||||
!self.disabled.contains(component)
|
!self.disabled.contains(component)
|
||||||
}
|
}
|
||||||
|
|
||||||
pub fn extra(&self) -> &[String] {
|
pub fn extra(&self) -> &[Vec<String>] {
|
||||||
&self.extra
|
&self.extra
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@@ -118,25 +119,62 @@ fn extras_path() -> Option<PathBuf> {
|
|||||||
/// A missing file is normal, not an error — most sessions will not have one.
|
/// 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
|
/// Deliberately not a shell: each line is exec'd directly, so a stray
|
||||||
/// backtick in a config cannot run something unexpected.
|
/// backtick in a config cannot run something unexpected.
|
||||||
fn read_extras(path: &std::path::Path) -> Vec<String> {
|
fn read_extras(path: &std::path::Path) -> Vec<Vec<String>> {
|
||||||
let Ok(text) = std::fs::read_to_string(path) else {
|
let Ok(text) = std::fs::read_to_string(path) else {
|
||||||
return Vec::new();
|
return Vec::new();
|
||||||
};
|
};
|
||||||
parse_extras(&text)
|
parse_extras(&text)
|
||||||
}
|
}
|
||||||
|
|
||||||
fn parse_extras(text: &str) -> Vec<String> {
|
fn parse_extras(text: &str) -> Vec<Vec<String>> {
|
||||||
text.lines()
|
text.lines()
|
||||||
.map(|l| match l.find('#') {
|
.map(split_argv)
|
||||||
Some(i) => &l[..i],
|
.filter(|argv| !argv.is_empty())
|
||||||
None => l,
|
|
||||||
})
|
|
||||||
.map(str::trim)
|
|
||||||
.filter(|l| !l.is_empty())
|
|
||||||
.map(String::from)
|
|
||||||
.collect()
|
.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<String> {
|
||||||
|
let mut argv = Vec::new();
|
||||||
|
let mut word = String::new();
|
||||||
|
let mut started = false;
|
||||||
|
let mut quote: Option<char> = 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)]
|
#[cfg(test)]
|
||||||
mod tests {
|
mod tests {
|
||||||
use super::*;
|
use super::*;
|
||||||
@@ -182,7 +220,60 @@ mod tests {
|
|||||||
let text = "\n# a comment\nwaybar\n swww-daemon \n\nrofi -show drun # trailing\n";
|
let text = "\n# a comment\nwaybar\n swww-daemon \n\nrofi -show drun # trailing\n";
|
||||||
assert_eq!(
|
assert_eq!(
|
||||||
parse_extras(text),
|
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", "*"]
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user