diff --git a/.forgejo/workflows/dev-release.yml b/.forgejo/workflows/dev-release.yml index 9ef75cc..aa31a2d 100644 --- a/.forgejo/workflows/dev-release.yml +++ b/.forgejo/workflows/dev-release.yml @@ -61,7 +61,7 @@ jobs: set -euo pipefail PKG_DIR="/srv/breadway-dl/dev/bread/${VERSION}" mkdir -p "${PKG_DIR}" - for bin in breadd bread bread-emit bread-module-host; do + for bin in breadd bread; do cp "src/target/release/${bin}" "${PKG_DIR}/${bin}-x86_64" strip "${PKG_DIR}/${bin}-x86_64" sha256sum "${PKG_DIR}/${bin}-x86_64" | awk '{print $1}' \ diff --git a/.forgejo/workflows/rc-release.yml b/.forgejo/workflows/rc-release.yml index 6adcf3b..73bce8f 100644 --- a/.forgejo/workflows/rc-release.yml +++ b/.forgejo/workflows/rc-release.yml @@ -32,7 +32,7 @@ jobs: VERSION="${GITHUB_REF_NAME#v}" PKG_DIR="/srv/breadway-dl/beta/bread/${VERSION}" mkdir -p "${PKG_DIR}" - for bin in breadd bread bread-emit bread-module-host; do + for bin in breadd bread; do cp "src/target/release/${bin}" "${PKG_DIR}/${bin}-x86_64" strip "${PKG_DIR}/${bin}-x86_64" sha256sum "${PKG_DIR}/${bin}-x86_64" | awk '{print $1}' \ diff --git a/.forgejo/workflows/release.yml b/.forgejo/workflows/release.yml index 36ab1ae..74a93bb 100644 --- a/.forgejo/workflows/release.yml +++ b/.forgejo/workflows/release.yml @@ -28,7 +28,7 @@ jobs: VERSION="${GITHUB_REF_NAME#v}" PKG_DIR="/srv/breadway-dl/bread/${VERSION}" mkdir -p "${PKG_DIR}" - for bin in breadd bread bread-emit bread-module-host; do + for bin in breadd bread; do cp "src/target/release/${bin}" "${PKG_DIR}/${bin}-x86_64" strip "${PKG_DIR}/${bin}-x86_64" sha256sum "${PKG_DIR}/${bin}-x86_64" | awk '{print $1}' \ @@ -48,13 +48,9 @@ jobs: echo "::error::BAKERY_MINISIGN_SEC_KEY_PATH secret not set — refusing to regenerate index.json unsigned (would leave a stale signature mismatched against fresh content and break bakery for everyone)" exit 1 fi - rm -rf /tmp/bread-ecosystem-ci-* 2>/dev/null || true - # mktemp: a fixed clone path races when multiple repos' release - # workflows run close together on the same self-hosted runner. - ECOSYSTEM_CI_DIR="$(mktemp -d /tmp/bread-ecosystem-ci-XXXXXX)" - git clone https://git.breadway.dev/Breadway/bread-ecosystem.git "${ECOSYSTEM_CI_DIR}" - bash "${ECOSYSTEM_CI_DIR}/scripts/gen-index.sh" - rm -rf "${ECOSYSTEM_CI_DIR}" + rm -rf /tmp/bread-ecosystem-ci + git clone https://git.breadway.dev/Breadway/bread-ecosystem.git /tmp/bread-ecosystem-ci + bash /tmp/bread-ecosystem-ci/scripts/gen-index.sh - name: upload to GitHub Release env: @@ -68,10 +64,6 @@ jobs: gh release upload "${GITHUB_REF_NAME}" --repo Breadway/bread \ "${PKG_DIR}/breadd-x86_64" \ "${PKG_DIR}/bread-x86_64" \ - "${PKG_DIR}/bread-emit-x86_64" \ - "${PKG_DIR}/bread-module-host-x86_64" \ "${PKG_DIR}/breadd-x86_64.sha256" \ "${PKG_DIR}/bread-x86_64.sha256" \ - "${PKG_DIR}/bread-emit-x86_64.sha256" \ - "${PKG_DIR}/bread-module-host-x86_64.sha256" \ --clobber diff --git a/bread-shared/src/apps.rs b/bread-shared/src/apps.rs index 4ecbcc5..cbf7e2c 100644 --- a/bread-shared/src/apps.rs +++ b/bread-shared/src/apps.rs @@ -32,9 +32,7 @@ pub const KNOWN_APPS: &[&str] = &[ /// `workspace`, `window`, and `monitor` added (event families the Hyprland /// and Bluetooth adapters already published under, but that were missing /// from this list) when this became a spoofing-prevention boundary and not -/// just an app-id-conflict one. Since: v1.7 — command-bus exception. -/// `module`, `state`, `widget`, and `reload` are daemon-synthesized -/// families and must stay unclaimable.* +/// just an app-id-conflict one. Since: v1.7 — command-bus exception.* const RESERVED_DOMAINS: &[&str] = &[ "terminal", "git", @@ -55,10 +53,6 @@ const RESERVED_DOMAINS: &[&str] = &[ "workspace", "window", "monitor", - "module", - "state", - "widget", - "reload", ]; /// Whether `id` is a registered sibling-app id. @@ -180,10 +174,6 @@ mod tests { "monitor", "window", "system", - "module", - "state", - "widget", - "reload", ] { assert!( is_reserved_domain(domain), diff --git a/breadd/src/ipc/module_host_bridge.rs b/breadd/src/ipc/module_host_bridge.rs index 6674dc8..164aeb5 100644 --- a/breadd/src/ipc/module_host_bridge.rs +++ b/breadd/src/ipc/module_host_bridge.rs @@ -25,7 +25,6 @@ //! directly from Lua bypasses every check in this file — that's the //! scenario the sandbox exists for, not this file. use std::collections::HashMap; -use std::path::{Component, Path, PathBuf}; use std::time::Duration; use bread_shared::{glob, ModuleHostHello, ModuleHostPush, ModulePermission, PermissionKind}; @@ -350,7 +349,10 @@ impl Server { Ok(json!({ "ok": true })) } "module_host.fs_read" => { - if !permissions.iter().any(|p| p.kind == PermissionKind::FsRead) { + if !permissions + .iter() + .any(|p| p.kind == PermissionKind::FsRead) + { return Err("fs.read not granted to this module".to_string()); } let path = req @@ -409,17 +411,16 @@ impl Server { .and_then(Value::as_str) .ok_or("missing cmd")? .to_string(); - let argv = exec_argv(permissions, &cmd)?; + if !bin_allowed(permissions, &cmd) { + return Err("command is outside this module's granted exec bin scope".to_string()); + } tokio::task::spawn_blocking(move || { - match std::process::Command::new(&argv[0]) - .args(&argv[1..]) - .status() - { + match std::process::Command::new("sh").arg("-c").arg(&cmd).status() { Ok(status) if !status.success() => { - warn!(cmd = %argv[0], code = ?status.code(), "module_host.exec exited non-zero"); + warn!(cmd = %cmd, code = ?status.code(), "module_host.exec exited non-zero"); } Err(e) => { - error!(cmd = %argv[0], error = %e, "module_host.exec failed to spawn"); + error!(cmd = %cmd, error = %e, "module_host.exec failed to spawn"); } _ => {} } @@ -436,17 +437,18 @@ impl Server { .and_then(Value::as_str) .ok_or("missing cmd")? .to_string(); - let argv = exec_argv(permissions, &cmd)?; + if !bin_allowed(permissions, &cmd) { + return Err("command is outside this module's granted exec bin scope".to_string()); + } let timeout_ms = req .params .get("timeout_ms") .and_then(Value::as_u64) .unwrap_or(2000); - let handle = tokio::task::spawn_blocking(move || { - std::process::Command::new(&argv[0]) - .args(&argv[1..]) - .output() - }); + let handle = + tokio::task::spawn_blocking(move || { + std::process::Command::new("sh").arg("-c").arg(&cmd).output() + }); match tokio::time::timeout(Duration::from_millis(timeout_ms + 500), handle).await { Ok(Ok(Ok(out))) => Ok(json!({ "ok": out.status.success(), @@ -472,9 +474,7 @@ impl Server { ( ModuleLoadState::LoadError, ModuleHostOutcome::LoadError( - error - .clone() - .unwrap_or_else(|| "module load failed".to_string()), + error.clone().unwrap_or_else(|| "module load failed".to_string()), ), ) }; @@ -501,14 +501,13 @@ impl Server { /// Belt-and-suspenders path scoping for `fs_read`/`fs_write`: if the /// manifest declared a `path` hint for this permission kind, the requested -/// path (after `~`-expansion and canonicalize) must be a real descendant -/// of at least one granted prefix. No hint at all means this RPC-level -/// check stays permissive (matching Workstream D's existing "un-hinted -/// grant = ungated within that namespace" behavior) — Landlock's own -/// ruleset (built independently in `module_host::apply_sandbox`) does NOT -/// grant a filesystem rule for an un-hinted permission, so the direct -/// `os`/`io` escape hatch remains kernel-denied for that case regardless -/// of what this function returns. +/// path (after `~`-expansion) must fall under at least one granted prefix. +/// No hint at all means this RPC-level check stays permissive (matching +/// Workstream D's existing "un-hinted grant = ungated within that +/// namespace" behavior) — Landlock's own ruleset (built independently in +/// `module_host::apply_sandbox`) does NOT grant a filesystem rule for an +/// un-hinted permission, so the direct `os`/`io` escape hatch remains +/// kernel-denied for that case regardless of what this function returns. fn path_allowed(permissions: &[ModulePermission], kind: PermissionKind, path: &str) -> bool { let hints: Vec<&String> = permissions .iter() @@ -518,93 +517,18 @@ fn path_allowed(permissions: &[ModulePermission], kind: PermissionKind, path: &s if hints.is_empty() { return true; } - let Some(requested) = resolve_scoped_path(path) else { - return false; - }; + let expanded = bread_shared::expand_path(path); hints.iter().any(|hint| { - let Some(granted) = resolve_scoped_path(hint) else { - return false; - }; - // Path::starts_with is component-wise; str::starts_with would let - // `/Wallpapers-evil` match a `/Wallpapers` grant. - requested.starts_with(&granted) + let hint_expanded = bread_shared::expand_path(hint); + expanded.starts_with(&hint_expanded) }) } -/// Canonicalize for containment: existing paths resolve symlinks and `..`; -/// new files canonicalize the parent then re-join the filename. Anything -/// that still contains `..` after lexical normalization is rejected. -fn resolve_scoped_path(path: &str) -> Option { - let expanded = bread_shared::expand_path(path); - if let Ok(canon) = expanded.canonicalize() { - return Some(canon); - } - let parent = expanded.parent().filter(|p| !p.as_os_str().is_empty()); - if let (Some(parent), Some(name)) = (parent, expanded.file_name()) { - if let Ok(parent_canon) = parent.canonicalize() { - return Some(parent_canon.join(name)); - } - } - lexical_abs(&expanded) -} - -fn lexical_abs(path: &Path) -> Option { - let abs = if path.is_absolute() { - path.to_path_buf() - } else { - std::path::absolute(path).ok()? - }; - let mut out = PathBuf::new(); - for c in abs.components() { - match c { - Component::CurDir => {} - Component::ParentDir => { - if !out.pop() { - return None; - } - } - rest => out.push(rest), - } - } - if out.components().any(|c| matches!(c, Component::ParentDir)) { - return None; - } - Some(out) -} - -/// Split `cmd` into argv without a shell. Metacharacters are rejected so a -/// hinted binary cannot smuggle extra commands (`hyprpaper; curl evil`). -fn parse_exec_argv(cmd: &str) -> Option> { - if cmd.chars().any(|c| { - matches!( - c, - '|' | ';' | '&' | '$' | '`' | '\n' | '\r' | '<' | '>' | '(' | ')' - ) - }) { - return None; - } - let argv: Vec = cmd.split_whitespace().map(str::to_string).collect(); - if argv.is_empty() { - None - } else { - Some(argv) - } -} - -/// Parse `cmd` and enforce the exec `bin` hint against argv[0]. -fn exec_argv(permissions: &[ModulePermission], cmd: &str) -> Result, String> { - let argv = parse_exec_argv(cmd) - .ok_or_else(|| "command contains shell metacharacters or is empty".to_string())?; - if !bin_allowed(permissions, &argv[0]) { - return Err("command is outside this module's granted exec bin scope".to_string()); - } - Ok(argv) -} - -/// Same idea as [`path_allowed`] for `exec`'s `bin` hint: compares argv[0] -/// by file name (so `bin = "hyprpaper"` matches `/usr/bin/hyprpaper` as -/// well as a bare `hyprpaper`) or an exact path match. -fn bin_allowed(permissions: &[ModulePermission], program: &str) -> bool { +/// Same idea as [`path_allowed`] for `exec`'s `bin` hint: compares by file +/// name (so `bin = "hyprpaper"` matches a command invoking +/// `/usr/bin/hyprpaper` as well as a bare `hyprpaper`) or an exact leading +/// token match. +fn bin_allowed(permissions: &[ModulePermission], cmd: &str) -> bool { let hints: Vec<&String> = permissions .iter() .filter(|p| p.kind == PermissionKind::Exec) @@ -613,109 +537,16 @@ fn bin_allowed(permissions: &[ModulePermission], program: &str) -> bool { if hints.is_empty() { return true; } - let cmd_leaf = Path::new(program) + let first_word = cmd.split_whitespace().next().unwrap_or(""); + let cmd_leaf = std::path::Path::new(first_word) .file_name() .and_then(|f| f.to_str()) - .unwrap_or(program); + .unwrap_or(first_word); hints.iter().any(|hint| { - let hint_leaf = Path::new(hint.as_str()) + let hint_leaf = std::path::Path::new(hint.as_str()) .file_name() .and_then(|f| f.to_str()) .unwrap_or(hint.as_str()); - cmd_leaf == hint_leaf || program == hint.as_str() + cmd_leaf == hint_leaf || first_word == hint.as_str() }) } - -#[cfg(test)] -mod tests { - use super::*; - - fn fs_grant(kind: PermissionKind, path: &str) -> Vec { - vec![ModulePermission { - kind, - path: Some(path.to_string()), - bin: None, - }] - } - - fn exec_grant(bin: &str) -> Vec { - vec![ModulePermission { - kind: PermissionKind::Exec, - path: None, - bin: Some(bin.to_string()), - }] - } - - #[test] - fn path_allowed_denies_dotdot_escape_from_granted_prefix() { - let tmp = tempfile::TempDir::new().unwrap(); - let wallpapers = tmp.path().join("Wallpapers"); - std::fs::create_dir(&wallpapers).unwrap(); - let granted = wallpapers.to_str().unwrap(); - let perms = fs_grant(PermissionKind::FsRead, granted); - - let escape = wallpapers.join("../../.ssh/id_rsa"); - assert!( - !path_allowed(&perms, PermissionKind::FsRead, escape.to_str().unwrap()), - "Wallpapers/../../.ssh/id_rsa must not match a Wallpapers grant" - ); - } - - #[test] - fn path_allowed_denies_string_prefix_sibling() { - let tmp = tempfile::TempDir::new().unwrap(); - let wallpapers = tmp.path().join("Wallpapers"); - let evil = tmp.path().join("Wallpapers-evil"); - std::fs::create_dir(&wallpapers).unwrap(); - std::fs::create_dir(&evil).unwrap(); - let secret = evil.join("secret"); - std::fs::write(&secret, "x").unwrap(); - let perms = fs_grant(PermissionKind::FsRead, wallpapers.to_str().unwrap()); - assert!(!path_allowed( - &perms, - PermissionKind::FsRead, - secret.to_str().unwrap() - )); - } - - #[test] - fn path_allowed_accepts_real_descendant_and_new_file() { - let tmp = tempfile::TempDir::new().unwrap(); - let wallpapers = tmp.path().join("Wallpapers"); - std::fs::create_dir(&wallpapers).unwrap(); - let existing = wallpapers.join("bg.png"); - std::fs::write(&existing, "x").unwrap(); - let perms = fs_grant(PermissionKind::FsWrite, wallpapers.to_str().unwrap()); - assert!(path_allowed( - &perms, - PermissionKind::FsWrite, - existing.to_str().unwrap() - )); - let new_file = wallpapers.join("new.png"); - assert!(path_allowed( - &perms, - PermissionKind::FsWrite, - new_file.to_str().unwrap() - )); - } - - #[test] - fn exec_argv_denies_shell_chaining_when_bin_hinted() { - let perms = exec_grant("hyprpaper"); - assert!(exec_argv(&perms, "hyprpaper; curl evil").is_err()); - assert!(exec_argv(&perms, "hyprpaper").is_ok()); - assert!(exec_argv(&perms, "/usr/bin/hyprpaper --config x").is_ok()); - assert!(exec_argv(&perms, "curl evil").is_err()); - } - - #[test] - fn parse_exec_argv_rejects_metacharacters() { - assert!(parse_exec_argv("hyprpaper; curl evil").is_none()); - assert!(parse_exec_argv("hyprpaper | curl evil").is_none()); - assert!(parse_exec_argv("hyprpaper && curl evil").is_none()); - assert_eq!( - parse_exec_argv("hyprpaper --config x"), - Some(vec!["hyprpaper".into(), "--config".into(), "x".into()]) - ); - } -}