diff --git a/docs/backends/seatbelt/seatbelt-backend.md b/docs/backends/seatbelt/seatbelt-backend.md index 718681674e..70cf46c4d9 100644 --- a/docs/backends/seatbelt/seatbelt-backend.md +++ b/docs/backends/seatbelt/seatbelt-backend.md @@ -115,18 +115,23 @@ given path. `deniedPaths` sits outside that ordering and always outranks. Before a path reaches the profile, MXC: -1. **Expands `~`** against `$HOME`. -2. **Collapses** `//`, `/./`, and trailing `/`. Rejects `..` (see above). -3. **Resolves symlinked roots** — `/etc`, `/tmp`, `/var` → `/private/…`; - `/home` → `/System/Volumes/Data/home`. +1. **Expands `~`** against `$HOME` and resolves relative paths from MXC's + working directory. +2. **Resolves symlinks**, including files, directory ancestors, chains, and + macOS roots such as `/tmp` → `/private/tmp`. +3. **Applies `..` after following symlinks** and collapses redundant separators + and `.` segments. Missing trailing components are supported. 4. **Re-applies precedence** (`denied` > `readonly` > `readwrite`) to the *resolved* paths. -Steps 3 and 4 aren't cosmetic. Those roots are symlinks, and the kernel fully -resolves a path before matching it — a rule written against the unresolved path -is silently dead. Step 4 catches two spellings of the same path -(`readonlyPaths: ["/private/tmp/x"]` vs `readwritePaths: ["/tmp/x"]`) that the -shared parser can't see are identical. +For example, if `/work/link` points to `/data/nested`, +`/work/link/../secret.txt` applies to `/data/secret.txt`, not +`/work/secret.txt`. A deny on a symlink protects its real target and other +symlink aliases, even under a broader read-write grant. + +Paths are resolved before launch. Dangling symlinks, symlink loops, inaccessible +ancestors, and targets whose resolved names cannot be represented as UTF-8 are +rejected rather than producing rules that might not protect the intended path. `/Users` needs no rewriting — it's a firmlink, not a symlink. @@ -584,7 +589,6 @@ once you can see the rules that were emitted. | Command not found, or a tool can't find its libraries | The environment is always cleared and `PATH` resets to `/usr/bin:/bin:/usr/sbin:/sbin` | Add `process.env`, and `readonlyPaths` for the install prefix (e.g. `/opt/homebrew`) | | A client is configured with `HTTP_PROXY` but reaches nothing | It's ignoring the proxy vars. Outbound is kernel-scoped to the proxy port, so it can't connect anywhere else | Use a proxy-aware client — see [Proxy support](#proxy-support-what-is-and-isnt-enforced) | | A test runner or build tool fails spawning workers | `nestedPty: false`, or its IPC socket sits under a `readonlyPaths`/`deniedPaths` entry | Leave `nestedPty` at `true`; put socket directories in `readwritePaths` | -| The same config runs on Linux but is rejected on macOS | A `..` segment in a path — Seatbelt-only rejection | Pass the fully resolved path | ### What gets rejected @@ -614,12 +618,8 @@ rejected before Seatbelt validation. | Config | Why it's rejected | Do this instead | |---|---|---| -| Any path containing a `..` segment | macOS resolves `..` *after* following symlinks, so a lexically-resolved rule can silently point elsewhere (`/tmp/..` is `/private`, not `/`) | Pass the fully resolved path | - -This one is **Seatbelt-only** — the shared parser accepts `..`, so a -cross-backend policy using it will run on Linux and fail on macOS. That's -deliberate: the alternative is a rule that matches nothing, which for -`deniedPaths` would fail *open*. +| A dangling symlink, symlink loop, or inaccessible path | The target cannot be resolved reliably for a Seatbelt path filter | Repair the link or make the path accessible to MXC | +| A path resolving to a non-UTF-8 name | The profile cannot represent the target faithfully | Use a target with a UTF-8 name | #### Streaming / GUI diff --git a/src/mxc-sdk/src/backends/bubblewrap/common/bwrap_runner.rs b/src/mxc-sdk/src/backends/bubblewrap/common/bwrap_runner.rs index 2e5df33d06..6faf277536 100644 --- a/src/mxc-sdk/src/backends/bubblewrap/common/bwrap_runner.rs +++ b/src/mxc-sdk/src/backends/bubblewrap/common/bwrap_runner.rs @@ -1162,7 +1162,14 @@ fn resolve_denied_paths( let mut files = HashSet::new(); for p in &policy.denied_paths { if let Some(resolved) = resolve_through_symlinks(Path::new(p)) { - let resolved = resolved.to_str().ok_or_else(|| { + if let Some(error) = resolved.error { + logger.log_line(&format!( + "Bubblewrap: deniedPaths entry '{p}' could not be fully resolved ({error}); \ + masking '{}' instead.", + resolved.path.display() + )); + } + let resolved = resolved.path.to_str().ok_or_else(|| { format!( "Bubblewrap: deniedPaths entry '{p}' resolves to a non-UTF-8 host path that \ cannot be safely masked; refusing to start." diff --git a/src/mxc-sdk/src/backends/bubblewrap/common/proxy_network.rs b/src/mxc-sdk/src/backends/bubblewrap/common/proxy_network.rs index 54798332e7..c5a9915dc9 100644 --- a/src/mxc-sdk/src/backends/bubblewrap/common/proxy_network.rs +++ b/src/mxc-sdk/src/backends/bubblewrap/common/proxy_network.rs @@ -1675,10 +1675,14 @@ fn is_nameserver_line(line: &str) -> bool { fn resolver_target_of(start: &Path) -> Result { let resolved = resolve_through_symlinks(start) .ok_or_else(|| format!("{} is not a resolvable path", start.display()))?; + if let Some(error) = resolved.error { + return Err(format!( + "{} could not be resolved ({error})", + start.display() + )); + } + let resolved = resolved.path; - // A chain that loops or dangles leaves canonicalization unfinished, so the - // walk hands back path text that is still a link. Binding there aborts the - // sandbox, which is worse than the broken DNS being fixed. let metadata = fs::symlink_metadata(&resolved) .map_err(|error| format!("{} could not be read ({error})", resolved.display()))?; if !metadata.file_type().is_file() { @@ -3483,8 +3487,7 @@ mod tests { assert!(resolver_target_of(&dangling).is_err()); } - /// A link loop leaves canonicalization unfinished, so the walk hands back a - /// path that is still a link; binding there would abort the sandbox. + /// Reject a link loop before attempting to mount the resolver file. #[test] fn the_resolver_target_gives_up_on_a_link_loop() { let dir = tempfile::tempdir().unwrap(); @@ -3495,7 +3498,7 @@ mod tests { std::os::unix::fs::symlink(&first, &second).unwrap(); let error = resolver_target_of(&first).expect_err("a loop has no target"); - assert!(error.contains("regular file"), "got: {error}"); + assert!(error.contains("could not be resolved"), "got: {error}"); } /// A directory would be mounted over as if it were the resolver file. diff --git a/src/mxc-sdk/src/backends/seatbelt/common/mod.rs b/src/mxc-sdk/src/backends/seatbelt/common/mod.rs index f128f503da..2254437b5e 100644 --- a/src/mxc-sdk/src/backends/seatbelt/common/mod.rs +++ b/src/mxc-sdk/src/backends/seatbelt/common/mod.rs @@ -3,9 +3,9 @@ //! `seatbelt_common` — shared library for the macOS sandbox backend. //! -//! - [`profile_builder`] is platform-agnostic (pure string generation) so it -//! compiles and is fully unit-tested on every host. This lets reviewers -//! validate profile generation without a Mac. +//! - [`profile_builder`] formats prepared inputs without host discovery, so it +//! is unit-tested on every host. +//! - macOS-only preparation resolves paths and discovers host directories. //! - [`seatbelt_policy`] owns the backend's network invariants and effective- //! policy helpers. Also platform-agnostic, so the rules are unit-tested //! everywhere even though only macOS can execute them. @@ -18,5 +18,7 @@ pub mod default_env; pub mod profile_builder; pub mod seatbelt_policy; +#[cfg(target_os = "macos")] +pub(crate) mod profile_preparation; #[cfg(target_os = "macos")] pub mod seatbelt_runner; diff --git a/src/mxc-sdk/src/backends/seatbelt/common/profile_builder.rs b/src/mxc-sdk/src/backends/seatbelt/common/profile_builder.rs index b61b86cde9..cc43f5a631 100644 --- a/src/mxc-sdk/src/backends/seatbelt/common/profile_builder.rs +++ b/src/mxc-sdk/src/backends/seatbelt/common/profile_builder.rs @@ -1,11 +1,11 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. -//! Pure builder that converts an [`ExecutionRequest`] into a TinyScheme sandbox +//! Builder that converts an [`ExecutionRequest`] into a TinyScheme sandbox //! profile string suitable for `sandbox_init()` or `sandbox-exec -p `. //! -//! This module is platform-agnostic — it is just string generation — so it -//! is unit-tested on every host (Windows / Linux / macOS) in CI. +//! Pure profile generation, unit-tested on every host. Filesystem and environment +//! discovery happen before this module receives its inputs. //! //! # Profile shape //! @@ -25,25 +25,32 @@ //! operation), so trailing deny rules take precedence over earlier allow //! rules — the behavior callers expect from MXC's `denied_paths`. -use std::fmt::Write as _; -use std::fs; -use std::path::{Path, PathBuf}; - use crate::mxc_common::filesystem_resolve::{resolve_path_plan, FsIntent}; use crate::mxc_common::host_is_canonical_loopback; use crate::mxc_common::models::{ClipboardPolicy, ContainerPolicy, ExecutionRequest, ProxyAddress}; use crate::seatbelt_common::seatbelt_policy; +use std::fmt::Write as _; + +/// Prepared filesystem grants for profile generation. +#[derive(Debug, Default)] +pub struct ProfileInputs { + pub readonly_paths: Vec, + pub readwrite_paths: Vec, + pub denied_paths: Vec, + pub developer_dir: Option, + pub user_keychains: Option, +} /// Build a complete Seatbelt sandbox profile, scoping cooperative proxy /// reachability to the resolved address supplied by the runner. /// -/// `pub` (not `pub(crate)`) so it stays a reachable API root: `profile_builder` -/// is compiled and unit-tested on every host, but its only in-crate caller -/// (`seatbelt_runner`) is `cfg(target_os = "macos")`, so on other targets a -/// narrower visibility would make the whole profile-building chain dead code. +/// Paths come from `inputs`; profile generation never inspects the host. +/// +/// Public to keep profile generation reachable on hosts without the macOS runner. pub fn build_profile_with_proxy( request: &ExecutionRequest, proxy_address: Option<&ProxyAddress>, + inputs: &ProfileInputs, ) -> Result { if let Some(override_profile) = request .seatbelt @@ -67,7 +74,7 @@ pub fn build_profile_with_proxy( // Filesystem — read-only system paths every process needs. out.push_str(SYSTEM_READ_ALLOW); - write_developer_dir_rule(&mut out); + write_developer_dir_rule(&mut out, inputs.developer_dir.as_deref()); // Pseudo-terminal access — when the executor binary runs under a pty // the sandboxed shell inherits that TTY, so it sees a real terminal @@ -77,11 +84,11 @@ pub fn build_profile_with_proxy( out.push_str(TTY_ALLOW); // Policy-derived allow rules. - let resolved = ResolvedPaths::from_policy(&request.policy)?; + let resolved = ResolvedPaths::from_inputs(inputs); write_filesystem_allow(&mut out, &resolved); write_network_rules(&mut out, request, proxy_address); write_nested_pty_rules(&mut out, request); - write_keychain_rules(&mut out, request)?; + write_keychain_rules(&mut out, request, inputs.user_keychains.as_deref())?; write_extra_seatbelt_rules(&mut out, request); write_ui_rules(&mut out, request); @@ -143,107 +150,9 @@ const SYSTEM_READ_ALLOW: &str = "\ (literal \"/dev/urandom\")) "; -/// Where `xcode-select` records the active developer directory. The second -/// entry is the older path some releases still populate. -const DEVELOPER_DIR_LINKS: [&str; 2] = [ - "/private/var/db/xcode_select_link", - "/private/var/select/developer_dir", -]; - -/// Resolve the active Xcode / Command Line Tools developer directory. -/// -/// The `/usr/bin` stubs (`python3`, `git`) are `xcrun` shims that `dlopen` -/// `libxcrun.dylib` from here, so they cannot start unless it is readable. -/// `SYSTEM_READ_ALLOW` reaches a Command Line Tools install through its -/// `/Library` grant, but an Xcode-selected host puts it under `/Applications`. -/// -/// `DEVELOPER_DIR` is ignored because any caller can set it. An untrusted link -/// is skipped, so resolution continues to the legacy one. -fn active_developer_dir() -> Option { - DEVELOPER_DIR_LINKS - .iter() - .map(Path::new) - .filter(|link| link_is_root_controlled(link)) - .filter_map(|link| fs::read_link(link).ok()) - .find(|target| target.is_absolute() && target.is_dir()) -} - -/// Whether root alone decides what `link` resolves to. -/// -/// Requires both a root-owned symlink and root-owned, non-group/world-writable -/// directories above it: replacing a symlink is a directory operation, so the -/// link's own ownership settles nothing by itself. -#[cfg(unix)] -fn link_is_root_controlled(link: &Path) -> bool { - use std::os::unix::fs::{MetadataExt as _, PermissionsExt as _}; - - let Ok(link_meta) = fs::symlink_metadata(link) else { - return false; - }; - if !link_meta.file_type().is_symlink() || link_meta.uid() != 0 { - return false; - } - // `ancestors()` starts at the link itself; the directories follow. - link.ancestors().skip(1).all(|dir| { - fs::metadata(dir) - .is_ok_and(|meta| dir_is_root_controlled(meta.uid(), meta.permissions().mode())) - }) -} - -/// Windows only: this module compiles everywhere but `std::os::unix` does not. -/// There is no `xcode-select` there, so nothing to trust. -#[cfg(not(unix))] -fn link_is_root_controlled(_link: &Path) -> bool { - false -} - -/// Pure so the rule is testable without a root-owned fixture. Sticky -/// world-writable directories are rejected too — none of the real -/// `xcode-select` locations are sticky, so refusing costs nothing. -#[cfg(unix)] -fn dir_is_root_controlled(uid: u32, mode: u32) -> bool { - const NON_OWNER_WRITE: u32 = 0o022; - - uid == 0 && mode & NON_OWNER_WRITE == 0 -} - -/// Emit the read-only grant for the active developer directory. -fn write_developer_dir_rule(out: &mut String) { - if let Some(dir) = active_developer_dir() { - push_developer_dir_rule(out, &developer_dir_grant_root(&dir)); - } -} - -/// Widen a developer directory to the `.app` bundle containing it, if any. -/// -/// The tools `xcrun` dispatches load frameworks from -/// `/Contents/SharedFrameworks`, outside `Contents/Developer`, so -/// the bundle root is the smallest subtree that lets them run. A Command Line -/// Tools install has no `.app` ancestor and is returned as-is. -fn developer_dir_grant_root(dir: &Path) -> PathBuf { - enclosing_app_bundle(dir).map_or_else(|| dir.to_path_buf(), Path::to_path_buf) -} - -/// The bundle for which `dir` is exactly `.app/Contents/Developer`. -fn enclosing_app_bundle(dir: &Path) -> Option<&Path> { - if dir.file_name()? != "Developer" { - return None; - } - let contents = dir.parent()?; - if contents.file_name()? != "Contents" { - return None; - } - let bundle = contents.parent()?; - bundle - .extension()? - .eq_ignore_ascii_case("app") - .then_some(bundle) -} - -/// Emit the grant for an already-resolved path. Split out so it can be tested -/// independently of what is installed on the test host. -fn push_developer_dir_rule(out: &mut String, dir: &Path) { - let Some(path) = dir.to_str() else { +/// Emit the read-only grant for the prepared developer directory. +fn write_developer_dir_rule(out: &mut String, developer_dir: Option<&str>) { + let Some(path) = developer_dir else { return; }; out.push_str(";; --- active developer directory (xcrun shims in /usr/bin) ---\n"); @@ -265,17 +174,7 @@ const TTY_ALLOW: &str = "\ (allow file-read* (subpath \"/dev/fd\")) "; -/// The policy path lists resolved into the form Seatbelt matches, with the -/// most-restrictive-wins precedence (`deny` > `readonly` > `readwrite`) -/// re-applied. -/// -/// The shared parser applies that precedence to the *raw* strings, which is not -/// enough once paths are resolved: two spellings of the same path -/// (`readonlyPaths: ["/private/tmp/x"]` and `readwritePaths: ["/tmp/x"]`) -/// differ as strings, so both survive the parser, then resolve to the same -/// filter here. Seatbelt is last-match-wins and the read-write rule is emitted -/// after the read-only one, so without this the *weaker* grant would win and -/// silently make a read-only path writable. +/// Prepared paths with `deny` > `readonly` > `readwrite` precedence applied. struct ResolvedPaths { readonly: Vec, readwrite: Vec, @@ -283,30 +182,25 @@ struct ResolvedPaths { } impl ResolvedPaths { - fn from_policy(policy: &ContainerPolicy) -> Result { - let denied = resolve_all(&policy.denied_paths)?; - let readonly = resolve_all(&policy.readonly_paths)?; - let mut readwrite = resolve_all(&policy.readwrite_paths)?; + fn from_inputs(inputs: &ProfileInputs) -> Self { + let denied = inputs.denied_paths.clone(); + let mut readonly = inputs.readonly_paths.clone(); + let mut readwrite = inputs.readwrite_paths.clone(); // `denied` is emitted last and would override either allow anyway, but // dropping the path keeps the emitted profile an honest description of // the effective policy. - let mut readonly: Vec = readonly; readonly.retain(|p| !denied.contains(p)); readwrite.retain(|p| !denied.contains(p) && !readonly.contains(p)); - Ok(Self { + Self { readonly, readwrite, denied, - }) + } } } -fn resolve_all(paths: &[String]) -> Result, String> { - paths.iter().map(|p| resolve_policy_path(p)).collect() -} - fn write_filesystem_allow(out: &mut String, paths: &ResolvedPaths) { if paths.readonly.is_empty() && paths.readwrite.is_empty() { return; @@ -636,17 +530,17 @@ fn write_nested_pty_rules(out: &mut String, request: &ExecutionRequest) { /// stores under `/Library/Keychains` and `/System/Library/Keychains` /// are already covered by the baseline `/Library` and `/System` /// read-only allows, so we don't re-add them here. -fn write_keychain_rules(out: &mut String, request: &ExecutionRequest) -> Result<(), String> { +fn write_keychain_rules( + out: &mut String, + request: &ExecutionRequest, + user_keychains: Option<&str>, +) -> Result<(), String> { let enabled = request.seatbelt.as_ref().is_some_and(|c| c.keychain_access); if !enabled { return Ok(()); } - // Seatbelt only applies on macOS. On other hosts the option is a - // no-op so workspace clippy / cross-platform tests don't have to - // care about `$HOME` (Windows CI doesn't set it). - if !cfg!(target_os = "macos") { - return Ok(()); - } + let user_keychains = user_keychains + .ok_or_else(|| "User keychain directory was not prepared for keychainAccess".to_string())?; out.push_str(";; --- keychainAccess: Mach IPC for Keychain (securityd, prefs, XPC, LS) ---\n"); out.push_str("(allow mach-lookup\n"); @@ -672,14 +566,9 @@ fn write_keychain_rules(out: &mut String, request: &ExecutionRequest) -> Result< out.push_str(" (subpath \"/private/var/protected/trustd\")\n"); out.push_str(" (subpath \"/private/var/db/mds\"))\n"); - let home = std::env::var("HOME").map_err(|_| { - "HOME environment variable not set; cannot expand '~/Library/Keychains' for keychainAccess" - .to_string() - })?; - let user_keychains = format!("{home}/Library/Keychains"); out.push_str(";; --- keychainAccess: user keychain DB + XPC/folder caches (read+write) ---\n"); out.push_str("(allow file-read* file-write*\n"); - let _ = writeln!(out, " (subpath {})", quote_scheme(&user_keychains)); + let _ = writeln!(out, " (subpath {})", quote_scheme(user_keychains)); out.push_str(" (subpath \"/private/var/folders\"))\n"); Ok(()) } @@ -702,107 +591,6 @@ fn write_extra_seatbelt_rules(out: &mut String, request: &ExecutionRequest) { out.push_str(")\n"); } -/// Resolve a caller-supplied policy path into the form Seatbelt matches: -/// expand a leading `~`, normalize redundant lexical segments, then rewrite the -/// symlinked macOS root directories. -/// -/// All three steps are required. See [`expand_tilde`], [`normalize_lexical`] -/// and [`resolve_macos_root_symlinks`]. -pub(crate) fn resolve_policy_path(path: &str) -> Result { - let expanded = expand_tilde(path)?; - Ok(resolve_macos_root_symlinks(&normalize_lexical(&expanded)?)) -} - -/// Collapse the lexical spellings of a path that leave a Seatbelt rule dead: -/// repeated separators (`//tmp`), `.` segments (`/./tmp`), and a trailing `/`. -/// -/// The kernel canonicalizes an accessed path before matching it against a -/// profile filter, but it does not canonicalize the filter, so `(subpath -/// "//tmp/secret")` never matches anything. For `deniedPaths` that fails -/// **open**, so these spellings have to be folded away rather than passed -/// through. -/// -/// A `..` segment is rejected instead of being resolved. macOS resolves `..` -/// *physically* — after following symlinks — so resolving it lexically can move -/// the rule somewhere else entirely: `/tmp/..` is `/private`, not `/`. Silently -/// widening an allow rule to `/` would be worse than the dead rule, and leaving -/// it in place keeps the deny fail-open, so an unresolvable path is a config -/// error the caller must fix by passing the resolved path. -fn normalize_lexical(path: &str) -> Result { - if path.split('/').any(|seg| seg == "..") { - return Err(format!( - "Filesystem path '{path}' contains a '..' segment. macOS resolves '..' after \ - following symlinks, so the generated sandbox rule could not be matched \ - reliably; specify the fully resolved path instead." - )); - } - - let joined = path - .split('/') - .filter(|seg| !seg.is_empty() && *seg != ".") - .collect::>() - .join("/"); - - Ok(if path.starts_with('/') { - format!("/{joined}") - } else { - joined - }) -} - -/// The macOS root directories that are symlinks, and their targets. -/// -/// `/etc`, `/tmp`, and `/var` point into `/private`; `/home` points at the -/// data volume. `/Users` is deliberately absent — it is a *firmlink*, not a -/// symlink, so `/Users/...` is already the canonical path the kernel matches. -const MACOS_SYMLINKED_ROOTS: [(&str, &str); 4] = [ - ("/etc", "/private/etc"), - ("/tmp", "/private/tmp"), - ("/var", "/private/var"), - ("/home", "/System/Volumes/Data/home"), -]; - -/// Rewrite a leading symlinked macOS root directory to its real target. -/// -/// The kernel resolves a path fully before matching it against a profile's -/// `subpath` / `literal` filters, so a rule written against the unresolved -/// path is dead: `(subpath "/tmp/work")` never matches, because the kernel -/// only ever sees `/private/tmp/work`. That silently voided every policy path -/// under these roots — including the automatic `$TMPDIR` grant, which resolves -/// to `/var/folders/...` on macOS. -/// -/// Only a whole leading path segment is rewritten, so `/variable` is left -/// alone. Paths already written against the real target pass through -/// unchanged, because none of the targets is itself under a symlinked root. -fn resolve_macos_root_symlinks(path: &str) -> String { - for (root, target) in MACOS_SYMLINKED_ROOTS { - let Some(rest) = path.strip_prefix(root) else { - continue; - }; - if rest.is_empty() || rest.starts_with('/') { - return format!("{target}{rest}"); - } - } - path.to_string() -} - -/// Expand a leading `~` or `~/` to the current user's home directory. -/// Returns an error if `HOME` is not set and the path requires expansion. -pub(crate) fn expand_tilde(path: &str) -> Result { - if path == "~" || path.starts_with("~/") { - let home = std::env::var("HOME").map_err(|_| { - format!("HOME environment variable not set; cannot expand '{path}' in seatbelt profile") - })?; - if path == "~" { - Ok(home) - } else { - Ok(format!("{home}/{}", &path[2..])) - } - } else { - Ok(path.to_string()) - } -} - /// Quote a string for use as a TinyScheme string literal, escaping /// embedded backslashes and double-quotes. fn quote_scheme(s: &str) -> String { @@ -834,6 +622,24 @@ mod tests { build_profile_with_proxy(request, request.policy.network_proxy.address.as_ref()) } + fn build_profile_with_proxy( + request: &ExecutionRequest, + proxy_address: Option<&ProxyAddress>, + ) -> Result { + let inputs = ProfileInputs { + readonly_paths: request.policy.readonly_paths.clone(), + readwrite_paths: request.policy.readwrite_paths.clone(), + denied_paths: request.policy.denied_paths.clone(), + user_keychains: request + .seatbelt + .as_ref() + .is_some_and(|config| config.keychain_access) + .then(|| "/Users/profile-test/Library/Keychains".into()), + ..Default::default() + }; + super::build_profile_with_proxy(request, proxy_address, &inputs) + } + fn req() -> ExecutionRequest { ExecutionRequest { script_code: "echo hi".to_string(), @@ -859,7 +665,7 @@ mod tests { #[test] fn readonly_paths_emit_subpath_allows() { let mut r = req(); - r.policy.readonly_paths = vec!["/opt/tools".into(), "/var/data".into()]; + r.policy.readonly_paths = vec!["/opt/tools".into(), "/private/var/data".into()]; let p = build_profile(&r).unwrap(); assert!(p.contains("policy.readonlyPaths")); assert!(p.contains("(allow file-read*")); @@ -871,7 +677,7 @@ mod tests { #[test] fn readwrite_paths_emit_read_and_write_allows() { let mut r = req(); - r.policy.readwrite_paths = vec!["/tmp/output".into()]; + r.policy.readwrite_paths = vec!["/private/tmp/output".into()]; let p = build_profile(&r).unwrap(); assert!(p.contains("(allow file-read* file-write*")); assert!(p.contains("(subpath \"/private/tmp/output\")")); @@ -880,8 +686,8 @@ mod tests { #[test] fn denied_paths_appear_after_allows_to_override() { let mut r = req(); - r.policy.readwrite_paths = vec!["/tmp".into()]; - r.policy.denied_paths = vec!["/tmp/secret".into()]; + r.policy.readwrite_paths = vec!["/private/tmp".into()]; + r.policy.denied_paths = vec!["/private/tmp/secret".into()]; let p = build_profile(&r).unwrap(); let allow_idx = p.find("(allow file-read* file-write*").unwrap(); let deny_idx = p @@ -1379,7 +1185,7 @@ mod tests { let mut r = req(); // Hypothetical adversarial input — we never want a path to break out // of the quoted string and inject Scheme. - r.policy.readonly_paths = vec!["/tmp/a\"b\\c".into()]; + r.policy.readonly_paths = vec!["/private/tmp/a\"b\\c".into()]; let p = build_profile(&r).unwrap(); assert!(p.contains("(subpath \"/private/tmp/a\\\"b\\\\c\")")); } @@ -1403,7 +1209,7 @@ mod tests { #[test] fn readwrite_paths_emit_unix_socket_ops() { let mut r = req(); - r.policy.readwrite_paths = vec!["/tmp/output".into()]; + r.policy.readwrite_paths = vec!["/private/tmp/output".into()]; let p = build_profile(&r).unwrap(); let idx = p.find(RW_RULE).expect("readwrite rule"); assert!(p[idx..].contains("(subpath \"/private/tmp/output\")")); @@ -1414,7 +1220,7 @@ mod tests { // AF_UNIX bind/connect follow the filesystem policy, so a default-deny // network policy with denied ingress must still permit them. let mut r = req(); - r.policy.readwrite_paths = vec!["/tmp/output".into()]; + r.policy.readwrite_paths = vec!["/private/tmp/output".into()]; r.policy.network_egress = Some(egress(NetworkAction::Deny)); let p = build_profile(&r).unwrap(); assert!(p.contains(RW_RULE)); @@ -1435,7 +1241,7 @@ mod tests { // `denied_paths_deny_unix_socket_ops_after_allows`. let mut r = req(); r.policy.network_egress = Some(egress(NetworkAction::Allow)); - r.policy.denied_paths = vec!["/tmp/secret".into()]; + r.policy.denied_paths = vec!["/private/tmp/secret".into()]; let p = build_profile(&r).unwrap(); let deny_idx = p.find(DENY_RULE).expect("deny must cover network-outbound"); assert!(p[deny_idx..].contains("(subpath \"/private/tmp/secret\")")); @@ -1444,7 +1250,7 @@ mod tests { #[test] fn readonly_paths_do_not_get_unix_socket_ops() { let mut r = req(); - r.policy.readonly_paths = vec!["/tmp/input".into()]; + r.policy.readonly_paths = vec!["/private/tmp/input".into()]; let p = build_profile(&r).unwrap(); assert!(!p.contains(RW_RULE)); // The only socket mention for a read-only path is the explicit removal. @@ -1455,8 +1261,8 @@ mod tests { #[test] fn denied_paths_deny_unix_socket_ops_after_allows() { let mut r = req(); - r.policy.readwrite_paths = vec!["/tmp".into()]; - r.policy.denied_paths = vec!["/tmp/secret".into()]; + r.policy.readwrite_paths = vec!["/private/tmp".into()]; + r.policy.denied_paths = vec!["/private/tmp/secret".into()]; let p = build_profile(&r).unwrap(); let deny_idx = p.find(DENY_RULE).expect("deny must cover socket ops"); let allow_idx = p.find(RW_RULE).expect("allow rule"); @@ -1474,8 +1280,8 @@ mod tests { // reaches it through `file-read*`, so the deny has to name it too or a // denied path still answers `stat()`. let mut r = req(); - r.policy.readwrite_paths = vec!["/tmp".into()]; - r.policy.denied_paths = vec!["/tmp/secret".into()]; + r.policy.readwrite_paths = vec!["/private/tmp".into()]; + r.policy.denied_paths = vec!["/private/tmp/secret".into()]; let p = build_profile(&r).unwrap(); assert!(p.contains("(allow file-read-metadata)")); let deny_idx = p @@ -1484,69 +1290,11 @@ mod tests { assert!(p[deny_idx..].contains("(subpath \"/private/tmp/secret\")")); } - #[test] - fn lexical_spellings_normalize_to_the_same_rule() { - // Each of these accesses `/private/tmp/secret` at the kernel level, so - // each must produce the same filter — otherwise a deny written with a - // redundant spelling is dead and fails open. - for spelling in [ - "/tmp/secret", - "//tmp/secret", - "/./tmp/secret", - "/tmp//secret", - "/tmp/./secret/", - "///tmp/secret//", - ] { - assert_eq!( - resolve_policy_path(spelling).unwrap(), - "/private/tmp/secret", - "spelling {spelling} must normalize" - ); - } - } - - #[test] - fn lexical_normalization_is_idempotent_and_preserves_root() { - assert_eq!(resolve_policy_path("/").unwrap(), "/"); - assert_eq!(resolve_policy_path("//").unwrap(), "/"); - let once = resolve_policy_path("//tmp/./secret/").unwrap(); - assert_eq!(resolve_policy_path(&once).unwrap(), once); - } - - #[test] - fn parent_segments_are_rejected_rather_than_resolved() { - // `/tmp/..` is `/private` on macOS, not `/`, so resolving lexically - // would silently widen an allow and leave a deny dead. - for path in ["/private/var/../tmp", "/tmp/..", "/..", "/a/b/../c"] { - let err = resolve_policy_path(path).unwrap_err(); - assert!(err.contains(".."), "{path} must be rejected, got {err}"); - } - // A path merely *containing* dots in a segment name is fine. - assert_eq!( - resolve_policy_path("/tmp/..secret").unwrap(), - "/private/tmp/..secret" - ); - } - - #[test] - fn denied_path_with_redundant_spelling_still_denies() { - let mut r = req(); - r.policy.readwrite_paths = vec!["/tmp".into()]; - r.policy.denied_paths = vec!["//tmp/./secret/".into()]; - let p = build_profile(&r).unwrap(); - let deny_idx = p.find(DENY_RULE).expect("deny rule"); - assert!(p[deny_idx..].contains("(subpath \"/private/tmp/secret\")")); - } - #[test] fn nested_readonly_keeps_write_away_from_broader_readwrite() { - // `/tmp` resolves to `/private/tmp`, which is an ancestor of the - // read-only entry. The read-only `allow` names only `file-read*`, so - // it says nothing about write or socket ops and cannot displace the - // broader grant on its own — hence the explicit removal, emitted - // *after* the read-write allow. + // The nested read-only rule must remove the broader write grant. let mut r = req(); - r.policy.readwrite_paths = vec!["/tmp".into()]; + r.policy.readwrite_paths = vec!["/private/tmp".into()]; r.policy.readonly_paths = vec!["/private/tmp/secret".into()]; let p = build_profile(&r).unwrap(); @@ -1564,7 +1312,7 @@ mod tests { // The mirror case: the deeper read-write entry must survive, otherwise // depth ordering would have simply inverted the bug. let mut r = req(); - r.policy.readonly_paths = vec!["/tmp".into()]; + r.policy.readonly_paths = vec!["/private/tmp".into()]; r.policy.readwrite_paths = vec!["/private/tmp/build".into()]; let p = build_profile(&r).unwrap(); @@ -1591,7 +1339,7 @@ mod tests { // asserted — it has no bearing on the outcome. let mut r = req(); r.policy.network_egress = Some(egress(NetworkAction::Allow)); - r.policy.readonly_paths = vec!["/tmp/ro".into()]; + r.policy.readonly_paths = vec!["/private/tmp/ro".into()]; let p = build_profile(&r).unwrap(); let strip_idx = p.find(RO_STRIP).expect("read-only strip"); @@ -1599,14 +1347,10 @@ mod tests { } #[test] - fn readonly_wins_over_readwrite_for_aliased_spellings() { - // The parser's most-restrictive-wins pass compares raw strings, so - // these two spellings both survive it and only collide once resolved. - // Seatbelt is last-match-wins and readwrite is emitted second, so - // without resolved-path precedence the read-only intent would be lost. + fn readonly_wins_over_readwrite_for_the_same_prepared_path() { let mut r = req(); r.policy.readonly_paths = vec!["/private/tmp/x".into()]; - r.policy.readwrite_paths = vec!["/tmp/x".into()]; + r.policy.readwrite_paths = vec!["/private/tmp/x".into()]; let p = build_profile(&r).unwrap(); assert!(fs_section(&p).contains("(subpath \"/private/tmp/x\")")); @@ -1617,11 +1361,11 @@ mod tests { } #[test] - fn denied_wins_over_aliased_readonly_and_readwrite() { + fn denied_wins_over_the_same_prepared_readonly_and_readwrite() { let mut r = req(); r.policy.denied_paths = vec!["/private/tmp/x".into()]; - r.policy.readonly_paths = vec!["/tmp/x".into()]; - r.policy.readwrite_paths = vec!["//tmp/./x".into()]; + r.policy.readonly_paths = vec!["/private/tmp/x".into()]; + r.policy.readwrite_paths = vec!["/private/tmp/x".into()]; let p = build_profile(&r).unwrap(); assert!(!p.contains(FS_SECTION), "profile:\n{p}"); @@ -1633,8 +1377,8 @@ mod tests { #[test] fn distinct_paths_survive_resolved_precedence() { let mut r = req(); - r.policy.readonly_paths = vec!["/tmp/ro".into()]; - r.policy.readwrite_paths = vec!["/tmp/rw".into()]; + r.policy.readonly_paths = vec!["/private/tmp/ro".into()]; + r.policy.readwrite_paths = vec!["/private/tmp/rw".into()]; let p = build_profile(&r).unwrap(); assert!(fs_section(&p).contains("(subpath \"/private/tmp/ro\")")); @@ -1642,46 +1386,6 @@ mod tests { assert!(p[rw_idx..].contains("(subpath \"/private/tmp/rw\")")); } - #[test] - fn macos_root_symlinks_are_resolved() { - assert_eq!(resolve_macos_root_symlinks("/tmp"), "/private/tmp"); - assert_eq!(resolve_macos_root_symlinks("/var"), "/private/var"); - assert_eq!(resolve_macos_root_symlinks("/etc"), "/private/etc"); - assert_eq!( - resolve_macos_root_symlinks("/var/folders/qj/T"), - "/private/var/folders/qj/T" - ); - assert_eq!( - resolve_macos_root_symlinks("/home/me"), - "/System/Volumes/Data/home/me" - ); - } - - #[test] - fn macos_root_resolution_only_matches_whole_segments() { - // A prefix that merely starts with the same letters is not a match. - assert_eq!(resolve_macos_root_symlinks("/variable"), "/variable"); - assert_eq!(resolve_macos_root_symlinks("/tmpfs/x"), "/tmpfs/x"); - assert_eq!(resolve_macos_root_symlinks("/etcd"), "/etcd"); - assert_eq!(resolve_macos_root_symlinks("/homebrew"), "/homebrew"); - // Already-resolved and unrelated paths pass through untouched. - assert_eq!( - resolve_macos_root_symlinks("/private/tmp/x"), - "/private/tmp/x" - ); - assert_eq!(resolve_macos_root_symlinks("/Users/me/w"), "/Users/me/w"); - assert_eq!(resolve_macos_root_symlinks(""), ""); - } - - #[test] - fn macos_root_resolution_is_idempotent() { - // Re-resolving a resolved path must be a no-op — no target is itself - // under a symlinked root. - for (_, target) in MACOS_SYMLINKED_ROOTS { - assert_eq!(resolve_macos_root_symlinks(target), target); - } - } - #[test] fn empty_policy_still_compiles_to_valid_profile() { let r = req(); @@ -1861,11 +1565,6 @@ mod tests { assert!(!p.contains("/private/var/db/mds")); } - // Keychain rules expand `~/Library/Keychains` from $HOME at build - // time, so the tests that exercise `keychain_access: true` are gated - // to macOS (the only OS where this code path is actually used and - // where $HOME is reliably set in CI). - #[cfg(target_os = "macos")] #[test] fn keychain_access_true_allows_securityd_mach_services() { let mut r = req(); @@ -1883,7 +1582,6 @@ mod tests { assert!(p.contains("(global-name-regex #\"^com\\.apple\\.lsd\\.\")")); } - #[cfg(target_os = "macos")] #[test] fn keychain_access_true_allows_filesystem_paths() { let mut r = req(); @@ -1897,10 +1595,8 @@ mod tests { // re-emit them here. assert!(p.contains("(subpath \"/private/var/db/mds\")")); // Read+write surfaces - let home = std::env::var("HOME").expect("HOME must be set in test env"); - let user_keychains = format!("{home}/Library/Keychains"); assert!( - p.contains(&format!("(subpath \"{user_keychains}\")")), + p.contains("(subpath \"/Users/profile-test/Library/Keychains\")"), "missing user keychain subpath" ); assert!(p.contains("(subpath \"/private/var/folders\")")); @@ -1943,13 +1639,8 @@ mod tests { #[test] fn developer_dir_rule_grants_read_only_subpath() { let mut out = String::new(); - push_developer_dir_rule( - &mut out, - Path::new("/Applications/Xcode.app/Contents/Developer"), - ); - assert!(out.contains( - "(allow file-read* (subpath \"/Applications/Xcode.app/Contents/Developer\"))" - )); + write_developer_dir_rule(&mut out, Some("/Applications/Xcode.app")); + assert!(out.contains("(allow file-read* (subpath \"/Applications/Xcode.app\"))")); // The xcrun shims only need to read it; a write grant would widen the // sandbox for no benefit. assert!(!out.contains("file-write")); @@ -1958,155 +1649,57 @@ mod tests { #[test] fn developer_dir_rule_escapes_embedded_quotes() { let mut out = String::new(); - push_developer_dir_rule(&mut out, Path::new("/tmp/we\"ird/Developer")); + write_developer_dir_rule(&mut out, Some("/tmp/we\"ird/Developer")); assert!(out.contains("(subpath \"/tmp/we\\\"ird/Developer\")")); } - #[test] - fn xcode_developer_dir_widens_to_the_app_bundle() { - let root = - developer_dir_grant_root(Path::new("/Applications/Xcode_26.6.app/Contents/Developer")); - assert_eq!(root, Path::new("/Applications/Xcode_26.6.app")); - } - - #[test] - fn command_line_tools_dir_is_not_widened() { - let dir = Path::new("/Library/Developer/CommandLineTools"); - assert_eq!(developer_dir_grant_root(dir), dir); - } - - #[test] - fn developer_dir_widening_is_case_insensitive_on_the_extension() { - let root = - developer_dir_grant_root(Path::new("/Applications/Xcode.APP/Contents/Developer")); - assert_eq!(root, Path::new("/Applications/Xcode.APP")); - } - - #[test] - fn widening_accepts_any_bundle_name_and_install_location() { - // xcode-select fixes the Contents/Developer suffix, never the prefix. - for (dir, want) in [ - ( - "/Applications/Xcode-beta.app/Contents/Developer", - "/Applications/Xcode-beta.app", - ), - ( - "/Applications/Xcode_26.6.app/Contents/Developer", - "/Applications/Xcode_26.6.app", - ), - ( - "/Volumes/Build/Xcode.app/Contents/Developer", - "/Volumes/Build/Xcode.app", - ), - ] { - assert_eq!(developer_dir_grant_root(Path::new(dir)), Path::new(want)); - } - } - - #[test] - fn a_dir_deeper_inside_a_bundle_is_not_widened_to_it() { - // Only the exact Contents/Developer layout widens. - for dir in [ - "/Applications/Evil.app/Contents/Developer/usr/bin", - "/Applications/Evil.app/a/b/Developer", - "/Applications/Evil.app/Contents/Developer2", - ] { - assert_eq!(developer_dir_grant_root(Path::new(dir)), Path::new(dir)); - } - } - - #[test] - fn widening_handles_non_normalized_paths_without_overreaching() { - // These all normalize to the same directory, so they still widen. - for dir in [ - "/Applications/Xcode.app/Contents/Developer/", - "/Applications/Xcode.app/Contents//Developer", - "/Applications/Xcode.app/Contents/./Developer", - ] { - assert_eq!( - developer_dir_grant_root(Path::new(dir)), - Path::new("/Applications/Xcode.app") - ); - } - // `..` is not normalized away, so it fails the layout match. - let parent_ref = "/Applications/Xcode.app/Contents/Developer/.."; - assert_eq!( - developer_dir_grant_root(Path::new(parent_ref)), - Path::new(parent_ref) - ); - } - #[test] fn developer_dir_absent_emits_nothing() { let mut out = String::new(); - if active_developer_dir().is_none() { - write_developer_dir_rule(&mut out); - assert!(out.is_empty()); - } - } - - #[cfg(unix)] - #[test] - fn only_a_root_owned_unwritable_dir_is_trusted() { - assert!(dir_is_root_controlled(0, 0o755)); - assert!(dir_is_root_controlled(0, 0o700)); - for mode in [0o775, 0o757, 0o777, 0o1777] { - assert!(!dir_is_root_controlled(0, mode), "mode {mode:o}"); - } - for uid in [1, 501, 1000] { - assert!(!dir_is_root_controlled(uid, 0o755), "uid {uid}"); - } - } - - #[cfg(unix)] - #[test] - fn a_link_the_caller_controls_is_not_trusted() { - use std::os::unix::fs::{symlink, MetadataExt as _}; - - let dir = std::env::temp_dir().join(format!("mxc-devdir-trust-{}", std::process::id())); - let _ = fs::remove_dir_all(&dir); - fs::create_dir_all(dir.join("target")).expect("fixture dir"); - - let link = dir.join("link"); - symlink(dir.join("target"), &link).expect("fixture symlink"); - // Only decisive when the test user is not root. - if fs::metadata(&dir).expect("fixture metadata").uid() != 0 { - assert!(!link_is_root_controlled(&link)); - } - - // True for any user: `xcode-select` writes a symlink, nothing else. - let plain = dir.join("plain"); - fs::write(&plain, "").expect("fixture file"); - assert!(!link_is_root_controlled(&plain)); - assert!(!link_is_root_controlled(&dir.join("missing"))); - - fs::remove_dir_all(&dir).expect("fixture cleanup"); + write_developer_dir_rule(&mut out, None); + assert!(out.is_empty()); } - #[cfg(target_os = "macos")] #[test] - fn the_real_xcode_select_links_are_trusted_when_present() { - // The check must accept a stock host, or the grant silently vanishes. - for link in DEVELOPER_DIR_LINKS.iter().map(Path::new) { - if fs::symlink_metadata(link).is_ok() { - assert!( - link_is_root_controlled(link), - "rejected a stock link: {}", - link.display() - ); - } + fn profile_uses_prepared_paths_instead_of_host_discovery() { + let mut request = req(); + request.policy.readonly_paths = vec!["/unprepared/path".into()]; + request.seatbelt = Some(SeatbeltConfig { + keychain_access: true, + ..Default::default() + }); + let inputs = ProfileInputs { + readonly_paths: vec!["/prepared/readonly".into()], + readwrite_paths: vec!["/prepared/readwrite".into()], + denied_paths: vec!["/prepared/denied".into()], + developer_dir: Some("/Applications/ProfileTest.app".into()), + user_keychains: Some("/Users/prepared/Library/Keychains".into()), + }; + let profile = super::build_profile_with_proxy(&request, None, &inputs).unwrap(); + for path in [ + "/prepared/readonly", + "/prepared/readwrite", + "/prepared/denied", + "/Applications/ProfileTest.app", + "/Users/prepared/Library/Keychains", + ] { + assert!( + profile.contains(&format!("(subpath \"{path}\")")), + "{profile}" + ); } + assert!(!profile.contains("/unprepared/path"), "{profile}"); } #[test] - fn baseline_profile_grants_the_active_developer_dir() { - // The documented baseline promise is that standard tools work, and - // the /usr/bin xcrun shims cannot start without this directory. - let Some(dir) = active_developer_dir() else { - return; - }; - let granted = developer_dir_grant_root(&dir); - let p = build_profile(&req()).unwrap(); - assert!(p.contains(&format!("(subpath \"{}\")", granted.display()))); + fn keychain_access_requires_a_prepared_user_directory() { + let mut request = req(); + request.seatbelt = Some(SeatbeltConfig { + keychain_access: true, + ..Default::default() + }); + let error = + super::build_profile_with_proxy(&request, None, &ProfileInputs::default()).unwrap_err(); + assert!(error.contains("not prepared for keychainAccess"), "{error}"); } } diff --git a/src/mxc-sdk/src/backends/seatbelt/common/profile_preparation.rs b/src/mxc-sdk/src/backends/seatbelt/common/profile_preparation.rs new file mode 100644 index 0000000000..b17b627f47 --- /dev/null +++ b/src/mxc-sdk/src/backends/seatbelt/common/profile_preparation.rs @@ -0,0 +1,383 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +//! macOS filesystem and environment discovery for Seatbelt profile generation. + +use std::fs; +use std::path::{Path, PathBuf}; + +use crate::mxc_common::models::ExecutionRequest; +use crate::seatbelt_common::profile_builder::ProfileInputs; + +const DEVELOPER_DIR_LINKS: [&str; 2] = [ + "/private/var/db/xcode_select_link", + "/private/var/select/developer_dir", +]; + +pub(crate) fn prepare_profile(request: &ExecutionRequest) -> Result { + if request + .seatbelt + .as_ref() + .is_some_and(|config| config.profile_override.is_some()) + { + return Ok(ProfileInputs::default()); + } + + let developer_dir = active_developer_dir() + .map(|dir| developer_dir_grant_root(&dir)) + .and_then(|dir| dir.into_os_string().into_string().ok()); + let denied_paths = resolve_all(&request.policy.denied_paths)?; + let readonly_paths = resolve_all(&request.policy.readonly_paths)?; + let readwrite_paths = resolve_all(&request.policy.readwrite_paths)?; + let user_keychains = if request.seatbelt.as_ref().is_some_and(|c| c.keychain_access) { + let home = std::env::var("HOME").map_err(|_| { + "HOME environment variable not set; cannot expand '~/Library/Keychains' for keychainAccess" + .to_string() + })?; + Some(format!("{home}/Library/Keychains")) + } else { + None + }; + + Ok(ProfileInputs { + readonly_paths, + readwrite_paths, + denied_paths, + developer_dir, + user_keychains, + }) +} + +fn resolve_all(paths: &[String]) -> Result, String> { + paths.iter().map(|p| resolve_policy_path(p)).collect() +} + +/// Follow symlinks before applying `..`, retaining missing trailing names. +fn resolve_policy_path(path: &str) -> Result { + let expanded = expand_tilde(path)?; + let resolution_error = + |error| format!("Filesystem path '{path}' could not be resolved: {error}"); + let absolute = std::path::absolute(&expanded).map_err(resolution_error)?; + let resolved = crate::mxc_common::filesystem_symlink::resolve_through_symlinks(&absolute) + .ok_or_else(|| format!("Filesystem path '{path}' has no resolved target"))?; + if let Some(error) = resolved.error { + return Err(resolution_error(error)); + } + resolved + .path + .into_os_string() + .into_string() + .map_err(|_| format!("Filesystem path '{path}' resolves to a non-UTF-8 path")) +} + +pub(crate) fn expand_tilde(path: &str) -> Result { + if path == "~" || path.starts_with("~/") { + let home = std::env::var("HOME").map_err(|_| { + format!("HOME environment variable not set; cannot expand '{path}' in seatbelt profile") + })?; + if path == "~" { + Ok(home) + } else { + Ok(format!("{home}/{}", &path[2..])) + } + } else { + Ok(path.to_string()) + } +} + +/// Ignore caller-controlled DEVELOPER_DIR; only trust the system's selection. +fn active_developer_dir() -> Option { + DEVELOPER_DIR_LINKS + .iter() + .map(Path::new) + .filter(|link| link_is_root_controlled(link)) + .filter_map(|link| fs::read_link(link).ok()) + .find(|target| target.is_absolute() && target.is_dir()) +} + +/// Replacing a link is controlled by its parent, not just the link's owner. +fn link_is_root_controlled(link: &Path) -> bool { + use std::os::unix::fs::{MetadataExt as _, PermissionsExt as _}; + + let Ok(link_meta) = fs::symlink_metadata(link) else { + return false; + }; + if !link_meta.file_type().is_symlink() || link_meta.uid() != 0 { + return false; + } + link.ancestors().skip(1).all(|dir| { + fs::metadata(dir) + .is_ok_and(|meta| dir_is_root_controlled(meta.uid(), meta.permissions().mode())) + }) +} + +fn dir_is_root_controlled(uid: u32, mode: u32) -> bool { + uid == 0 && mode & 0o022 == 0 +} + +/// Xcode shims also load frameworks outside Contents/Developer. +fn developer_dir_grant_root(dir: &Path) -> PathBuf { + enclosing_app_bundle(dir).map_or_else(|| dir.to_path_buf(), Path::to_path_buf) +} + +fn enclosing_app_bundle(dir: &Path) -> Option<&Path> { + if dir.file_name()? != "Developer" { + return None; + } + let contents = dir.parent()?; + if contents.file_name()? != "Contents" { + return None; + } + let bundle = contents.parent()?; + bundle + .extension()? + .eq_ignore_ascii_case("app") + .then_some(bundle) +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::mxc_common::models::SeatbeltConfig; + use crate::seatbelt_common::profile_builder::build_profile_with_proxy; + + #[test] + fn lexical_spellings_normalize_to_the_same_rule() { + for spelling in [ + "/tmp/secret", + "//tmp/secret", + "/./tmp/secret", + "/tmp//secret", + "/tmp/./secret/", + "///tmp/secret//", + ] { + assert_eq!( + resolve_policy_path(spelling).unwrap(), + "/private/tmp/secret", + "spelling {spelling} must normalize" + ); + } + } + + #[test] + fn lexical_normalization_is_idempotent_and_preserves_root() { + assert_eq!(resolve_policy_path("/").unwrap(), "/"); + assert_eq!(resolve_policy_path("//").unwrap(), "/"); + let once = resolve_policy_path("//tmp/./secret/").unwrap(); + assert_eq!(resolve_policy_path(&once).unwrap(), once); + } + + #[test] + fn parent_segments_follow_symlinked_roots() { + for (path, expected) in [ + ("/private/var/../tmp", "/private/tmp"), + ("/tmp/..", "/private"), + ("/..", "/"), + ("/a/b/../c", "/a/c"), + ] { + assert_eq!(resolve_policy_path(path).unwrap(), expected); + } + assert_eq!( + resolve_policy_path("/tmp/..secret").unwrap(), + "/private/tmp/..secret" + ); + } + + #[test] + fn user_symlinks_resolve_before_parent_segments_and_missing_tails() { + let dir = tempfile::tempdir().unwrap(); + let root = fs::canonicalize(dir.path()).unwrap(); + fs::create_dir_all(root.join("real/nested")).unwrap(); + let link = root.join("link"); + std::os::unix::fs::symlink("real/nested", &link).unwrap(); + for suffix in ["../secret", "missing/../../secret"] { + let path = link.join(suffix); + assert_eq!( + resolve_policy_path(path.to_str().unwrap()).unwrap(), + root.join("real/secret").to_str().unwrap() + ); + } + } + + #[test] + fn unresolved_symlinks_fail_before_parent_segments_can_hide_them() { + let dir = tempfile::tempdir().unwrap(); + for (name, target) in [("dangling", "missing"), ("loop", "loop")] { + let link = dir.path().join(name); + std::os::unix::fs::symlink(target, &link).unwrap(); + for suffix in ["", "child", "../secret"] { + let path = link.join(suffix); + let error = resolve_policy_path(path.to_str().unwrap()).unwrap_err(); + assert!(error.contains("could not be resolved"), "{error}"); + } + } + } + + #[test] + fn prepared_aliases_keep_the_strictest_profile_rules() { + let dir = tempfile::tempdir().unwrap(); + let target = fs::canonicalize(dir.path()).unwrap().join("target"); + fs::write(&target, "secret").unwrap(); + let link = dir.path().join("alias"); + std::os::unix::fs::symlink("target", &link).unwrap(); + let mut request = ExecutionRequest::default(); + request.policy.readonly_paths = vec![target.to_str().unwrap().into()]; + request.policy.readwrite_paths = vec![link.to_str().unwrap().into()]; + let inputs = prepare_profile(&request).unwrap(); + let profile = build_profile_with_proxy(&request, None, &inputs).unwrap(); + let filter = format!("(subpath \"{}\")", target.display()); + assert!(profile.contains(&filter)); + assert!(!profile.contains("(allow file-read* file-write* network-bind network-outbound")); + + request.policy.denied_paths = vec![link.to_str().unwrap().into()]; + let inputs = prepare_profile(&request).unwrap(); + let profile = build_profile_with_proxy(&request, None, &inputs).unwrap(); + assert!(!profile.contains(";; --- policy.readonlyPaths / policy.readwritePaths")); + assert!(profile.contains(&filter)); + } + + #[test] + fn denied_path_with_redundant_spelling_still_denies() { + let mut request = ExecutionRequest::default(); + request.policy.readwrite_paths = vec!["/tmp".into()]; + request.policy.denied_paths = vec!["//tmp/./secret/".into()]; + let inputs = prepare_profile(&request).unwrap(); + let profile = build_profile_with_proxy(&request, None, &inputs).unwrap(); + let deny = profile + .find("(deny file-read* file-read-metadata file-write* network-bind network-outbound") + .unwrap(); + assert!(profile[deny..].contains("(subpath \"/private/tmp/secret\")")); + } + + #[test] + fn profile_override_does_not_resolve_ignored_paths() { + let dir = tempfile::tempdir().unwrap(); + let link = dir.path().join("loop"); + std::os::unix::fs::symlink("loop", &link).unwrap(); + let mut request = ExecutionRequest { + seatbelt: Some(SeatbeltConfig { + profile_override: Some("(version 1)\n(allow default)".into()), + keychain_access: true, + ..Default::default() + }), + ..Default::default() + }; + request.policy.denied_paths = vec![link.to_str().unwrap().into()]; + let inputs = prepare_profile(&request).unwrap(); + let profile = build_profile_with_proxy(&request, None, &inputs).unwrap(); + assert_eq!(profile, "(version 1)\n(allow default)"); + } + + #[test] + fn only_a_root_owned_unwritable_dir_is_trusted() { + assert!(dir_is_root_controlled(0, 0o755)); + assert!(dir_is_root_controlled(0, 0o700)); + for mode in [0o775, 0o757, 0o777, 0o1777] { + assert!(!dir_is_root_controlled(0, mode), "mode {mode:o}"); + } + for uid in [1, 501, 1000] { + assert!(!dir_is_root_controlled(uid, 0o755), "uid {uid}"); + } + } + + #[test] + fn a_link_the_caller_controls_is_not_trusted() { + use std::os::unix::fs::{symlink, MetadataExt as _}; + + let dir = tempfile::tempdir().unwrap(); + fs::create_dir(dir.path().join("target")).unwrap(); + let link = dir.path().join("link"); + symlink(dir.path().join("target"), &link).unwrap(); + if fs::metadata(dir.path()).unwrap().uid() != 0 { + assert!(!link_is_root_controlled(&link)); + } + let plain = dir.path().join("plain"); + fs::write(&plain, "").unwrap(); + assert!(!link_is_root_controlled(&plain)); + assert!(!link_is_root_controlled(&dir.path().join("missing"))); + } + + #[test] + fn the_real_xcode_select_links_are_trusted_when_present() { + for link in DEVELOPER_DIR_LINKS.iter().map(Path::new) { + if fs::symlink_metadata(link).is_ok() { + assert!( + link_is_root_controlled(link), + "rejected a stock link: {}", + link.display() + ); + } + } + } + + #[test] + fn developer_directory_grants_cover_only_the_enclosing_xcode_bundle() { + for (dir, expected) in [ + ( + "/Applications/Xcode_26.6.app/Contents/Developer", + "/Applications/Xcode_26.6.app", + ), + ( + "/Library/Developer/CommandLineTools", + "/Library/Developer/CommandLineTools", + ), + ( + "/Applications/Xcode.APP/Contents/Developer", + "/Applications/Xcode.APP", + ), + ( + "/Applications/Xcode-beta.app/Contents/Developer", + "/Applications/Xcode-beta.app", + ), + ( + "/Volumes/Build/Xcode.app/Contents/Developer", + "/Volumes/Build/Xcode.app", + ), + ( + "/Applications/Evil.app/Contents/Developer/usr/bin", + "/Applications/Evil.app/Contents/Developer/usr/bin", + ), + ( + "/Applications/Evil.app/a/b/Developer", + "/Applications/Evil.app/a/b/Developer", + ), + ( + "/Applications/Evil.app/Contents/Developer2", + "/Applications/Evil.app/Contents/Developer2", + ), + ( + "/Applications/Xcode.app/Contents/Developer/", + "/Applications/Xcode.app", + ), + ( + "/Applications/Xcode.app/Contents//Developer", + "/Applications/Xcode.app", + ), + ( + "/Applications/Xcode.app/Contents/./Developer", + "/Applications/Xcode.app", + ), + ( + "/Applications/Xcode.app/Contents/Developer/..", + "/Applications/Xcode.app/Contents/Developer/..", + ), + ] { + assert_eq!( + developer_dir_grant_root(Path::new(dir)), + Path::new(expected) + ); + } + } + + #[test] + fn baseline_profile_grants_the_active_developer_dir() { + let Some(dir) = active_developer_dir() else { + return; + }; + let request = ExecutionRequest::default(); + let inputs = prepare_profile(&request).unwrap(); + let profile = build_profile_with_proxy(&request, None, &inputs).unwrap(); + let granted = developer_dir_grant_root(&dir); + assert!(profile.contains(&format!("(subpath \"{}\")", granted.display()))); + } +} diff --git a/src/mxc-sdk/src/backends/seatbelt/common/seatbelt_runner.rs b/src/mxc-sdk/src/backends/seatbelt/common/seatbelt_runner.rs index 3db2e8b936..c83a40f7e1 100644 --- a/src/mxc-sdk/src/backends/seatbelt/common/seatbelt_runner.rs +++ b/src/mxc-sdk/src/backends/seatbelt/common/seatbelt_runner.rs @@ -41,6 +41,7 @@ use crate::mxc_pty::{LivePty, PtySize as UnixPtySize}; use crate::seatbelt_common::default_env::DEFAULT_SANDBOX_PATH; use crate::seatbelt_common::default_env::{env_pairs, resolved_env}; use crate::seatbelt_common::profile_builder::build_profile_with_proxy; +use crate::seatbelt_common::profile_preparation::{expand_tilde, prepare_profile}; #[link(name = "proc")] unsafe extern "C" { @@ -308,7 +309,9 @@ impl SandboxBackend for SeatbeltScriptRunner { crate::mxc_common::proxy_env::redact_proxy_url(&address.to_url()) )); } - let profile = build_profile_with_proxy(request, proxy_address).map_err(error_response)?; + let inputs = prepare_profile(request).map_err(error_response)?; + let profile = + build_profile_with_proxy(request, proxy_address, &inputs).map_err(error_response)?; log_generated_profile(&profile, logger); // Effective posture, not the raw flag: a `guiAccess` the UI policy @@ -930,10 +933,7 @@ const UNRESOLVED_WORKING_DIRECTORY: &str = "/"; /// The same value feeds the environment, so the default `HOME` cannot name a /// directory the child never entered. fn resolved_working_directory_opt(request: &ExecutionRequest) -> Option { - let expand = |path: &str| { - crate::seatbelt_common::profile_builder::expand_tilde(path) - .unwrap_or_else(|_| path.to_string()) - }; + let expand = |path: &str| expand_tilde(path).unwrap_or_else(|_| path.to_string()); request .resolved_working_directory_with(|path| Path::new(&expand(path)).is_dir()) .map(|resolved| expand(resolved.path)) @@ -1127,7 +1127,8 @@ mod tests { // Guards the seam: a re-derived or stale profile must not be logged. let mut request = base_request(); request.policy.readonly_paths = vec!["/tmp/mxc-profile-log-probe".into()]; - let profile = build_profile_with_proxy(&request, None).expect("profile builds"); + let inputs = prepare_profile(&request).expect("profile inputs resolve"); + let profile = build_profile_with_proxy(&request, None, &inputs).expect("profile builds"); assert_eq!( extract_profile(&logged_profile(&profile)), diff --git a/src/mxc-sdk/src/core/mxc_common/filesystem_symlink.rs b/src/mxc-sdk/src/core/mxc_common/filesystem_symlink.rs index 65773d2182..5ef5accecd 100644 --- a/src/mxc-sdk/src/core/mxc_common/filesystem_symlink.rs +++ b/src/mxc-sdk/src/core/mxc_common/filesystem_symlink.rs @@ -1,7 +1,7 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. -//! Symlink resolution for host paths the Linux backends mount over. +//! Host-path symlink resolution for Linux backends and macOS Seatbelt. //! //! The Linux containment backends realise filesystem policy as bind mounts, and //! bwrap cannot create a mount point when any component of the destination — @@ -16,6 +16,13 @@ use std::path::{Component, Path, PathBuf}; +/// The resolved path and the first failure encountered during traversal. +#[derive(Debug)] +pub(crate) struct SymlinkResolution { + pub(crate) path: PathBuf, + pub(crate) error: Option, +} + /// Resolve every symlink in `path` (leaf and ancestors) to a real filesystem /// path, tolerating trailing components that do not exist yet. /// @@ -28,30 +35,43 @@ use std::path::{Component, Path, PathBuf}; /// so the result matches the target the kernel's path resolution would reach. /// Returns `None` only for an empty path. /// -/// A chain that loops or dangles leaves canonicalization unable to finish, so -/// the result is the lexical text and is still a symlink. Callers that are -/// about to mount over the result must check what they got. +/// Unresolvable components are recorded in `error`, even if a later `..` removes +/// them. The path remains available for backends that can mask unresolved links. /// /// A naive backward walk that collected `file_name()` silently dropped `..` /// components (Rust returns `None` for a `..` file name) and reconstructed the /// wrong target: `/link/missing/../secret` became `/real/missing/secret` /// instead of `/real/secret`, so the mask landed on a bystander path and the /// real denied target stayed exposed. -pub(crate) fn resolve_through_symlinks(path: &Path) -> Option { +pub(crate) fn resolve_through_symlinks(path: &Path) -> Option { let mut result = PathBuf::new(); + let mut failure = None; + let mut resolve_prefix = |prefix: &Path| { + // macOS realpath can fold `file/..` without checking that file is a directory. + let resolved = match std::fs::symlink_metadata(prefix) { + Err(error) if error.kind() == std::io::ErrorKind::NotFound => return None, + Err(error) => Err(error), + Ok(_) => std::fs::canonicalize(prefix), + }; + match resolved { + Ok(real) => Some(real), + Err(error) => { + failure.get_or_insert(error); + None + } + } + }; for component in path.components() { match component { Component::Prefix(_) | Component::RootDir => result.push(component.as_os_str()), Component::CurDir => {} Component::ParentDir => { + let _ = resolve_prefix(&result.join("..")); result.pop(); } Component::Normal(name) => { result.push(name); - // Canonicalize the prefix so far so symlinks are followed while - // it still exists; once a component is missing, canonicalize - // fails and the remaining tail is folded lexically above. - if let Ok(real) = std::fs::canonicalize(&result) { + if let Some(real) = resolve_prefix(&result) { result = real; } } @@ -60,7 +80,10 @@ pub(crate) fn resolve_through_symlinks(path: &Path) -> Option { if result.as_os_str().is_empty() { None } else { - Some(result) + Some(SymlinkResolution { + path: result, + error: failure, + }) } } @@ -83,7 +106,9 @@ mod tests { let link = root.join("link.conf"); std::os::unix::fs::symlink(&target, &link).unwrap(); - assert_eq!(resolve_through_symlinks(&link), Some(target)); + let resolved = resolve_through_symlinks(&link).unwrap(); + assert!(resolved.error.is_none(), "{resolved:?}"); + assert_eq!(resolved.path, target); } /// The case a leaf-only walk misses: the destination still names a symlink @@ -97,10 +122,9 @@ mod tests { std::fs::write(&file, b"x").unwrap(); std::os::unix::fs::symlink(root.join("real"), root.join("link")).unwrap(); - assert_eq!( - resolve_through_symlinks(&root.join("link/sub/f.conf")), - Some(file) - ); + let resolved = resolve_through_symlinks(&root.join("link/sub/f.conf")).unwrap(); + assert!(resolved.error.is_none(), "{resolved:?}"); + assert_eq!(resolved.path, file); } /// `..` must be applied to the directory the symlink really points at. @@ -113,10 +137,9 @@ mod tests { std::fs::write(&sibling, b"x").unwrap(); std::os::unix::fs::symlink(root.join("real"), root.join("link")).unwrap(); - assert_eq!( - resolve_through_symlinks(&root.join("link/../sibling.conf")), - Some(sibling) - ); + let resolved = resolve_through_symlinks(&root.join("link/../sibling.conf")).unwrap(); + assert!(resolved.error.is_none(), "{resolved:?}"); + assert_eq!(resolved.path, sibling); } /// A path whose tail does not exist yet still resolves its real ancestors, @@ -128,14 +151,12 @@ mod tests { std::fs::create_dir(root.join("real")).unwrap(); std::os::unix::fs::symlink(root.join("real"), root.join("link")).unwrap(); - assert_eq!( - resolve_through_symlinks(&root.join("link/absent.conf")), - Some(root.join("real/absent.conf")) - ); + let resolved = resolve_through_symlinks(&root.join("link/absent.conf")).unwrap(); + assert!(resolved.error.is_none(), "{resolved:?}"); + assert_eq!(resolved.path, root.join("real/absent.conf")); } - /// Canonicalization cannot finish, so the caller is handed path text that - /// is still a link rather than an error. + /// Unresolved link text remains available to mount backends. #[test] fn a_loop_yields_unresolved_text() { let dir = tempfile::tempdir().unwrap(); @@ -146,8 +167,9 @@ mod tests { std::os::unix::fs::symlink(&first, &second).unwrap(); let resolved = resolve_through_symlinks(&first).expect("a non-empty path resolves"); + assert!(resolved.error.is_some()); assert!( - std::fs::symlink_metadata(&resolved) + std::fs::symlink_metadata(&resolved.path) .expect("the link node exists") .file_type() .is_symlink(), @@ -155,8 +177,31 @@ mod tests { ); } + #[test] + fn missing_prefix_and_parent_segments_do_not_hide_broken_links() { + let dir = tempfile::tempdir().unwrap(); + let root = temp_root(&dir); + for (name, target) in [("loop", "loop"), ("dangling", "missing-target")] { + std::os::unix::fs::symlink(target, root.join(name)).unwrap(); + let path = root.join("missing/..").join(name).join("../secret"); + let resolved = resolve_through_symlinks(&path).unwrap(); + assert!(resolved.error.is_some(), "{resolved:?}"); + assert_eq!(resolved.path, root.join("secret")); + } + } + + #[test] + fn parent_traversal_cannot_treat_a_file_as_a_directory() { + let dir = tempfile::tempdir().unwrap(); + let root = temp_root(&dir); + std::fs::write(root.join("file"), b"x").unwrap(); + let resolved = resolve_through_symlinks(&root.join("file/../secret")).unwrap(); + assert!(resolved.error.is_some(), "{resolved:?}"); + assert_eq!(resolved.path, root.join("secret")); + } + #[test] fn an_empty_path_has_no_target() { - assert_eq!(resolve_through_symlinks(Path::new("")), None); + assert!(resolve_through_symlinks(Path::new("")).is_none()); } } diff --git a/src/mxc-sdk/src/core/mxc_common/mod.rs b/src/mxc-sdk/src/core/mxc_common/mod.rs index 96cf41da23..0852561723 100644 --- a/src/mxc-sdk/src/core/mxc_common/mod.rs +++ b/src/mxc-sdk/src/core/mxc_common/mod.rs @@ -72,9 +72,8 @@ pub mod system_dir; #[cfg(unix)] pub mod interruptible_reader; -// Linux-only: used by the backends that realise filesystem policy as bind -// mounts, which must resolve a destination before mounting over it. -#[cfg(target_os = "linux")] +// Host-path resolution shared by Linux backends and Seatbelt. +#[cfg(any(target_os = "linux", target_os = "macos"))] pub(crate) mod filesystem_symlink; /// Crate-wide lock and guards for tests that mutate process environment. diff --git a/tests/configs/seatbelt_path_dotdot_rejected.json b/tests/configs/seatbelt_path_dotdot.json similarity index 53% rename from tests/configs/seatbelt_path_dotdot_rejected.json rename to tests/configs/seatbelt_path_dotdot.json index a193890ff5..a78f420921 100644 --- a/tests/configs/seatbelt_path_dotdot_rejected.json +++ b/tests/configs/seatbelt_path_dotdot.json @@ -3,8 +3,9 @@ "version": "1.0.0", "containment": "seatbelt", "process": { - "commandLine": "echo PATH_DOTDOT_SHOULD_NOT_RUN", + "commandLine": "cat '{{TESTDIR}}/alias/secret.txt'", + "cwd": "/", "timeout": 10000 }, - "filesystem": { "readonlyPaths": ["/private/tmp/../etc"] } + "filesystem": { "readonlyPaths": ["{{TESTDIR}}/alias/../alias"] } } diff --git a/tests/scripts/run_seatbelt_all_tests.sh b/tests/scripts/run_seatbelt_all_tests.sh index 955cf169b3..0b41fd64fe 100755 --- a/tests/scripts/run_seatbelt_all_tests.sh +++ b/tests/scripts/run_seatbelt_all_tests.sh @@ -71,6 +71,7 @@ run_test() { run_test "Seatbelt Basic" "$SCRIPT_DIR/run_seatbelt_basic_test.sh" run_test "Seatbelt Filesystem" "$SCRIPT_DIR/run_seatbelt_filesystem_test.sh" run_test "Seatbelt Path Resolution" "$SCRIPT_DIR/run_seatbelt_path_resolution_test.sh" +run_test "Seatbelt Symlinks" "$SCRIPT_DIR/run_seatbelt_symlink_test.sh" run_test "Seatbelt UNIX Sockets" "$SCRIPT_DIR/run_seatbelt_unix_socket_test.sh" run_test "Seatbelt Directional Network" "$SCRIPT_DIR/run_seatbelt_network_test.sh" run_test "Seatbelt Rejections" "$SCRIPT_DIR/run_seatbelt_rejections_test.sh" diff --git a/tests/scripts/run_seatbelt_path_resolution_test.sh b/tests/scripts/run_seatbelt_path_resolution_test.sh index 3fc160f4e5..49bfd12bcb 100755 --- a/tests/scripts/run_seatbelt_path_resolution_test.sh +++ b/tests/scripts/run_seatbelt_path_resolution_test.sh @@ -1,10 +1,5 @@ #!/bin/bash -# Seatbelt path resolution: rejected path shapes, /tmp symlink aliasing, and -# how overlapping grants on the same directory resolve. -# -# These matter because a grant that silently resolves to a different directory -# than the caller wrote is either a hole (too broad) or a mystery denial (too -# narrow), and neither is visible from the config text alone. +# Seatbelt path resolution, /tmp aliases, and overlapping filesystem grants. set -uo pipefail SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" @@ -22,13 +17,8 @@ echo "PATH_SECRET_CONTENT" >"$TESTDIR/alias/secret.txt" echo "PATH_SECRET_CONTENT" >"$TESTDIR/rw/inner/secret.txt" chmod -R a+rX "$TESTDIR" -# Refused rather than resolved: macOS applies '..' after following symlinks, -# so the rule the profile would carry need not name the directory the caller -# meant. -expect_rejected "a path containing '..' is refused" \ - "seatbelt_path_dotdot_rejected.json" \ - "contains a '..' segment" \ - "PATH_DOTDOT_SHOULD_NOT_RUN" +run_config "$(render seatbelt_path_dotdot.json TESTDIR "$TESTDIR")" +expect_ok "a grant containing '..' covers the resolved directory" "PATH_SECRET_CONTENT" # A grant written as /tmp/x must cover the same bytes reached as /private/tmp/x. run_config "$(render seatbelt_path_tmp_alias.json TESTDIR "$TESTDIR" TESTDIR_TMP "$TESTDIR_TMP")" diff --git a/tests/scripts/run_seatbelt_symlink_test.sh b/tests/scripts/run_seatbelt_symlink_test.sh new file mode 100755 index 0000000000..135dcc7df8 --- /dev/null +++ b/tests/scripts/run_seatbelt_symlink_test.sh @@ -0,0 +1,230 @@ +#!/bin/bash +# Symlink policy must protect the target, not just the spelling in the config. +set -uo pipefail + +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +# shellcheck source=lib/seatbelt_common.sh +. "$SCRIPT_DIR/lib/seatbelt_common.sh" + +[ "$(id -u)" -ne 0 ] || fail "run the symlink tests as an unprivileged user" +require_python3_probe + +TESTDIR="$(mktemp -d /private/tmp/mxc-seatbelt-symlink.XXXXXX)" || + fail "could not create the symlink fixture directory" +trap 'rm -rf "$TESTDIR" "$SEATBELT_TMP"' EXIT + +OUT=$(/usr/bin/python3 - "$MXC_EXEC_MAC" "$DEVDIR" "$TESTDIR" <<'PY' +import base64 +import json +from pathlib import Path +import shlex +import subprocess +import sys + +binary, developer_dir, test_dir = sys.argv[1:] +root = Path(test_dir) +real_file = root / "real-file.txt" +real_dir = root / "real-dir" +real_dir.mkdir() +(real_dir / "nested").mkdir() +inner_file = real_dir / "secret.txt" +file_link = root / "file-link" +file_link.symlink_to(real_file) +file_alias = root / "file-alias" +file_alias.symlink_to("real-file.txt") +dir_link = root / "dir-link" +dir_link.symlink_to("real-dir", target_is_directory=True) +dir_chain = root / "dir-chain" +dir_chain.symlink_to("dir-link", target_is_directory=True) +traversal_link = root / "traversal-link" +traversal_link.symlink_to("real-dir/nested", target_is_directory=True) +file_paths = [real_file, file_link, file_alias, real_dir / ".." / "real-file.txt"] +dir_paths = [ + inner_file, dir_link / "secret.txt", dir_chain / "secret.txt", + traversal_link / ".." / "secret.txt", +] +directories = [real_dir, dir_link, dir_chain, traversal_link / ".."] +secret = "SYMLINK_SECRET\n" +control = root / "secret.txt" + +probe = r""" +import errno, json, sys +from pathlib import Path + +root, access, paths, directories = sys.argv[1:] +paths = json.loads(paths) +directories = json.loads(directories) +control = Path(root) / "secret.txt" +assert control.read_text() == "SYMLINK_CONTROL\n", "sibling read control failed" +control.write_text("SYMLINK_CONTROL_WRITTEN\n") +failures = [] + +def check(operation, allowed, label): + try: + operation() + except OSError as error: + if allowed or error.errno not in (errno.EPERM, errno.EACCES): + raise + else: + if not allowed: + failures.append(label) + +def read(path): + assert path.read_text().startswith("SYMLINK_SECRET\n"), "unexpected target content" + +def write(path): + with path.open("a") as stream: + stream.write("SYMLINK_WRITE\n") + +def create(path): + with path.open("x") as stream: + stream.write("SYMLINK_CREATED\n") + +for name in paths: + path = Path(name) + check(lambda: read(path), access != "deny", f"read allowed: {path}") + check(lambda: write(path), access == "rw", f"write allowed: {path}") +for index, name in enumerate(directories): + path = Path(name) / f"created-{index}.txt" + check(lambda: create(path), access == "rw", f"creation allowed: {path}") +if failures: + raise AssertionError("; ".join(failures)) +print("SYMLINK_PROBE_OK") +""" + +cases = [ + ("broad allow control", {}, file_paths + dir_paths, directories, "rw"), + ("deny a file symlink", {"deniedPaths": [file_link]}, file_paths, [], "deny"), + ("deny a chained directory symlink", {"deniedPaths": [dir_chain]}, dir_paths, directories, "deny"), + ("deny a file through a symlinked parent", + {"deniedPaths": [dir_link / "secret.txt"]}, dir_paths, [], "deny"), + ("deny the real file also blocks symlink aliases", + {"deniedPaths": [real_file]}, file_paths, [], "deny"), + ("readonly symlink overrides readwrite target", + {"readwritePaths": [root, real_file], "readonlyPaths": [file_link]}, file_paths, [], "ro"), + ("deny a policy path containing parent traversal", + {"deniedPaths": [real_dir / ".." / "real-file.txt"]}, file_paths, [], "deny"), + ("deny follows a symlink before parent traversal", + {"deniedPaths": [traversal_link / ".." / "secret.txt"]}, dir_paths, [], "deny"), + ("readonly follows a symlink before parent traversal", + {"readwritePaths": [root, inner_file], + "readonlyPaths": [traversal_link / ".." / "secret.txt"]}, dir_paths, [], "ro"), + ("readwrite follows a symlink before parent traversal", + {"readwritePaths": [traversal_link / "..", control]}, dir_paths, directories, "rw"), + ("deny folds parent traversal in a missing tail", + {"deniedPaths": [dir_link / "missing" / ".." / "secret.txt"]}, dir_paths, [], "deny"), + ("deny a not-yet-created file through a symlinked parent", + {"deniedPaths": [dir_link / "created-0.txt"]}, [], [real_dir], "deny"), +] + +def execute(filesystem, command): + policy = {"readwritePaths": [root], "readonlyPaths": [], "deniedPaths": []} + policy.update(filesystem) + policy["readonlyPaths"] = [developer_dir] + policy["readonlyPaths"] + request = { + "version": "1.0.0", + "containment": "seatbelt", + "process": { + "commandLine": command, + "cwd": "/", + "timeout": 20000, + }, + "filesystem": {key: [str(path) for path in paths] for key, paths in policy.items()}, + } + encoded = base64.b64encode(json.dumps(request).encode()).decode() + return subprocess.run( + [binary, "--config-base64", encoded], + stdin=subprocess.DEVNULL, + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + text=True, + start_new_session=True, + timeout=30, + ) + +failures = [] +for label, filesystem, paths, directory_paths, access in cases: + for path in (real_file, inner_file): + path.write_text(secret) + control.write_text("SYMLINK_CONTROL\n") + for index in range(len(directories)): + created = real_dir / f"created-{index}.txt" + if created.exists(): + created.unlink() + for path in paths: + if path.read_text() != secret: + raise AssertionError(f"host cannot read the fixture: {path}") + with path.open("a"): + pass + command = [ + "/usr/bin/python3", "-I", "-c", probe, str(root), access, + json.dumps([str(path) for path in paths]), + json.dumps([str(path) for path in directory_paths]), + ] + result = execute(filesystem, "exec " + " ".join(shlex.quote(arg) for arg in command)) + errors = [] + if result.returncode != 0 or "SYMLINK_PROBE_OK" not in result.stdout.splitlines(): + errors.append(f"exit {result.returncode}\n{result.stdout}\n{result.stderr}") + if control.read_text() != "SYMLINK_CONTROL_WRITTEN\n": + errors.append("sandbox did not write the allowed sibling control") + if access != "rw": + for path in (real_file, inner_file): + if path.read_text() != secret: + errors.append(f"protected host file changed: {path}") + if directory_paths and any(real_dir.glob("created-*.txt")): + errors.append("a file was created in the denied directory") + if errors: + failures.append(label) + print(f"FAIL: {label}\n" + "\n".join(errors), flush=True) + else: + print(f"PASS: {label}", flush=True) + +dangling = root / "dangling" +dangling.symlink_to("missing-target") +loop = root / "loop" +loop.symlink_to("loop") +unreadable = root / "unreadable" +unreadable.mkdir() +try: + unreadable.chmod(0) + try: + (unreadable / "secret.txt").stat() + except PermissionError: + pass + else: + raise AssertionError("host permission-denied control failed") + for label, field, path in [ + ("deny a dangling symlink", "deniedPaths", dangling), + ("readonly through a dangling ancestor", "readonlyPaths", dangling / "child"), + ("readwrite through a symlink loop", "readwritePaths", loop), + ("deny a symlink loop before parent traversal", "deniedPaths", loop / ".." / "real-file.txt"), + ("deny an inaccessible path", "deniedPaths", unreadable / "secret.txt"), + ("missing prefix does not hide an inaccessible target", "deniedPaths", + root / "missing" / ".." / "unreadable" / "secret.txt"), + ("missing prefix and parent traversal do not hide a dangling link", "deniedPaths", + root / "missing" / ".." / "dangling" / ".." / "real-file.txt"), + ("missing prefix and parent traversal do not hide a symlink loop", "deniedPaths", + root / "missing" / ".." / "loop" / ".." / "real-file.txt"), + ("parent traversal cannot treat a file as a directory", "deniedPaths", + real_file / ".." / "secret.txt"), + ("parent traversal does not hide an inaccessible directory", "deniedPaths", + unreadable / ".." / "real-file.txt"), + ]: + result = execute({field: [path]}, "echo SYMLINK_INVALID_POLICY_RAN") + output = result.stdout + result.stderr + if (result.returncode == 0 or "SYMLINK_INVALID_POLICY_RAN" in output + or "could not be resolved" not in output or str(path) not in output): + failures.append(label) + print(f"FAIL: {label}\nexit {result.returncode}\n{output}", flush=True) + else: + print(f"PASS: {label}", flush=True) +finally: + unreadable.chmod(0o700) +if failures: + raise AssertionError(f"{len(failures)} symlink policy case(s) failed") +PY +) || fail "symlink policy does not match the expected access rules" "$OUT" + +printf '%s\n' "$OUT" +pass "symlink and real-target access follow the same filesystem policy" +summary "Seatbelt symlinks"