diff --git a/crates/nono-cli/tests/symlink_hop_run.rs b/crates/nono-cli/tests/symlink_hop_run.rs new file mode 100644 index 000000000..8ffdb8d1a --- /dev/null +++ b/crates/nono-cli/tests/symlink_hop_run.rs @@ -0,0 +1,68 @@ +//! Kernel-level coverage: a multi-hop symlink grant must resolve under +//! the real Seatbelt sandbox. + +#![cfg(target_os = "macos")] + +use nono_test_support::{Argv, nono_test}; +use std::fs; + +/// `.gitconfig -> hosts/current/gitconfig -> hosts/mymac/gitconfig`, with +/// `hosts/current -> hosts/mymac` a symlinked directory component. +fn write_fixture(workspace: &std::path::Path) -> (std::path::PathBuf, std::path::PathBuf) { + let hosts = workspace.join("hosts"); + let mymac = hosts.join("mymac"); + fs::create_dir_all(&mymac).expect("create hosts/mymac"); + fs::write(mymac.join("gitconfig"), "trusted\n").expect("write real gitconfig"); + fs::write(mymac.join("secret.txt"), "topsecret\n").expect("write sibling secret"); + + let current = hosts.join("current"); + std::os::unix::fs::symlink(&mymac, ¤t).expect("symlink hosts/current -> hosts/mymac"); + + let gitconfig_link = workspace.join(".gitconfig"); + std::os::unix::fs::symlink(current.join("gitconfig"), &gitconfig_link) + .expect("symlink .gitconfig -> hosts/current/gitconfig"); + + (gitconfig_link, mymac.join("secret.txt")) +} + +#[test] +fn multi_hop_symlinked_leaf_resolves_through_symlinked_directory() { + let t = nono_test!("symlink-hop-positive"); + let workspace = t.workspace().to_path_buf(); + let (gitconfig_link, _secret) = write_fixture(&workspace); + + let profile = t.write_profile( + "symlink-hop-positive", + &format!( + r#"{{"meta":{{"name":"t"}},"workdir":{{"access":"readwrite"}},"filesystem":{{"read":["{}"]}}}}"#, + gitconfig_link.display() + ), + ); + + t.run() + .profile(&profile) + .exec(Argv::new("/bin/cat").arg(&gitconfig_link)) + .assert_stdout_contains("trusted"); +} + +#[test] +fn multi_hop_symlinked_leaf_does_not_widen_access_to_sibling_in_traversed_directory() { + let t = nono_test!("symlink-hop-negative"); + let workspace = t.workspace().to_path_buf(); + let (gitconfig_link, secret) = write_fixture(&workspace); + + let profile = t.write_profile( + "symlink-hop-negative", + &format!( + r#"{{"meta":{{"name":"t"}},"workdir":{{"access":"readwrite"}},"filesystem":{{"read":["{}"]}}}}"#, + gitconfig_link.display() + ), + ); + + t.run() + .profile(&profile) + .exec(Argv::new("/bin/cat").arg(&secret)) + .assert_failure( + "granting a multi-hop symlink must not expose sibling files under the traversed directory", + ); +} diff --git a/crates/nono-cli/tests/symlink_hop_run_linux.rs b/crates/nono-cli/tests/symlink_hop_run_linux.rs new file mode 100644 index 000000000..a2f0b4ed0 --- /dev/null +++ b/crates/nono-cli/tests/symlink_hop_run_linux.rs @@ -0,0 +1,68 @@ +//! Linux Landlock parity, proving empirically that `open_path_rule`'s +//! `O_PATH` resolution needs no code change here. + +#![cfg(target_os = "linux")] + +use nono_test_support::{Argv, nono_test}; +use std::fs; + +/// `.gitconfig -> hosts/current/gitconfig -> hosts/mymac/gitconfig`, with +/// `hosts/current -> hosts/mymac` a symlinked directory component. +fn write_fixture(workspace: &std::path::Path) -> (std::path::PathBuf, std::path::PathBuf) { + let hosts = workspace.join("hosts"); + let mymac = hosts.join("mymac"); + fs::create_dir_all(&mymac).expect("create hosts/mymac"); + fs::write(mymac.join("gitconfig"), "trusted\n").expect("write real gitconfig"); + fs::write(mymac.join("secret.txt"), "topsecret\n").expect("write sibling secret"); + + let current = hosts.join("current"); + std::os::unix::fs::symlink(&mymac, ¤t).expect("symlink hosts/current -> hosts/mymac"); + + let gitconfig_link = workspace.join(".gitconfig"); + std::os::unix::fs::symlink(current.join("gitconfig"), &gitconfig_link) + .expect("symlink .gitconfig -> hosts/current/gitconfig"); + + (gitconfig_link, mymac.join("secret.txt")) +} + +#[test] +fn multi_hop_symlinked_leaf_resolves_through_symlinked_directory() { + let t = nono_test!("symlink-hop-positive-linux"); + let workspace = t.workspace().to_path_buf(); + let (gitconfig_link, _secret) = write_fixture(&workspace); + + let profile = t.write_profile( + "symlink-hop-positive-linux", + &format!( + r#"{{"meta":{{"name":"t"}},"workdir":{{"access":"readwrite"}},"filesystem":{{"read":["{}"]}}}}"#, + gitconfig_link.display() + ), + ); + + t.run() + .profile(&profile) + .exec(Argv::new("/bin/cat").arg(&gitconfig_link)) + .assert_stdout_contains("trusted"); +} + +#[test] +fn multi_hop_symlinked_leaf_does_not_widen_access_to_sibling_in_traversed_directory() { + let t = nono_test!("symlink-hop-negative-linux"); + let workspace = t.workspace().to_path_buf(); + let (gitconfig_link, secret) = write_fixture(&workspace); + + let profile = t.write_profile( + "symlink-hop-negative-linux", + &format!( + r#"{{"meta":{{"name":"t"}},"workdir":{{"access":"readwrite"}},"filesystem":{{"read":["{}"]}}}}"#, + gitconfig_link.display() + ), + ); + + t.run() + .profile(&profile) + .exec(Argv::new("/bin/cat").arg(&secret)) + .assert_failure( + "granting a multi-hop symlink must not expose sibling files under the traversed directory", + ); +} diff --git a/crates/nono/src/path.rs b/crates/nono/src/path.rs index 030c8dd06..36847f782 100644 --- a/crates/nono/src/path.rs +++ b/crates/nono/src/path.rs @@ -1,6 +1,7 @@ //! Path utilities shared across nono library and CLI. -use std::path::{Path, PathBuf}; +use std::collections::HashSet; +use std::path::{Component, Path, PathBuf}; /// Canonicalize a path using an ancestor-walk fallback. /// @@ -49,6 +50,79 @@ pub(crate) fn try_canonicalize_ancestor_walk(path: &Path) -> PathBuf { path.to_path_buf() } +/// Matches typical OS `ELOOP`/`MAXSYMLINKS` limits, so a cycle truncates +/// instead of looping forever. +const MAX_SYMLINK_HOPS: usize = 40; + +/// Intermediate hops need their own sandbox grant, since +/// [`try_canonicalize`] only ever sees the original and resolved endpoints. +pub fn collect_symlink_hops(path: &Path) -> Vec { + let mut hops = Vec::new(); + let mut seen = HashSet::new(); + let mut depth = 0usize; + resolve_hops(path, &mut hops, &mut seen, &mut depth); + hops +} + +/// `path` need not be absolute on entry, since a symlink target can be +/// relative to its own parent directory. +fn resolve_hops( + path: &Path, + hops: &mut Vec, + seen: &mut HashSet, + depth: &mut usize, +) -> PathBuf { + let mut result = PathBuf::new(); + + for component in path.components() { + match component { + Component::Prefix(_) | Component::RootDir => { + result.push(component.as_os_str()); + } + Component::CurDir => {} + Component::ParentDir => { + result.pop(); + } + Component::Normal(segment) => { + result.push(segment); + + if *depth >= MAX_SYMLINK_HOPS { + continue; + } + + let Ok(metadata) = std::fs::symlink_metadata(&result) else { + continue; + }; + if !metadata.file_type().is_symlink() { + continue; + } + + *depth += 1; + if !seen.insert(result.clone()) { + continue; + } + hops.push(result.clone()); + + let Ok(target) = std::fs::read_link(&result) else { + continue; + }; + let target = if target.is_absolute() { + target + } else { + result + .parent() + .map(|parent| parent.join(&target)) + .unwrap_or(target) + }; + + result = resolve_hops(&target, hops, seen, depth); + } + } + } + + result +} + #[cfg(test)] mod tests { use super::*; @@ -93,4 +167,101 @@ mod tests { assert_eq!(result, real_file.canonicalize().expect("canonicalize")); } } + + #[cfg(unix)] + #[test] + fn collect_symlink_hops_empty_for_plain_path() { + let dir = tempfile::tempdir().expect("tempdir"); + let canonical_dir = dir.path().canonicalize().expect("canonicalize"); + let real_file = canonical_dir.join("real.txt"); + fs::write(&real_file, "hello").expect("write file"); + assert!(collect_symlink_hops(&real_file).is_empty()); + } + + #[cfg(unix)] + #[test] + fn collect_symlink_hops_single_hop_leaf_symlink() { + let dir = tempfile::tempdir().expect("tempdir"); + let canonical_dir = dir.path().canonicalize().expect("canonicalize"); + let real_file = canonical_dir.join("real.txt"); + fs::write(&real_file, "hello").expect("write file"); + let link = canonical_dir.join("link.txt"); + std::os::unix::fs::symlink(&real_file, &link).expect("symlink"); + + let hops = collect_symlink_hops(&link); + assert_eq!(hops, vec![link]); + } + + /// A symlinked leaf through a symlinked directory component: + /// `.gitconfig -> hosts/current/gitconfig -> hosts/mymac/gitconfig`. + #[cfg(unix)] + #[test] + fn collect_symlink_hops_multi_hop_through_symlinked_directory() { + let dir = tempfile::tempdir().expect("tempdir"); + let canonical_dir = dir.path().canonicalize().expect("canonicalize"); + + let hosts = canonical_dir.join("hosts"); + let mymac = hosts.join("mymac"); + fs::create_dir_all(&mymac).expect("mkdir mymac"); + let real_gitconfig = mymac.join("gitconfig"); + fs::write(&real_gitconfig, "[user]\n").expect("write gitconfig"); + + let current = hosts.join("current"); + std::os::unix::fs::symlink(&mymac, ¤t).expect("symlink dir"); + + let gitconfig_link = canonical_dir.join(".gitconfig"); + std::os::unix::fs::symlink(current.join("gitconfig"), &gitconfig_link) + .expect("symlink leaf"); + + let hops = collect_symlink_hops(&gitconfig_link); + assert_eq!(hops, vec![gitconfig_link.clone(), current]); + + // The fully resolved target is unaffected -- std canonicalization + // still collapses the whole chain in one shot. + assert_eq!( + try_canonicalize(&gitconfig_link), + real_gitconfig.canonicalize().expect("canonicalize") + ); + } + + #[cfg(unix)] + #[test] + fn collect_symlink_hops_cycle_truncates_instead_of_looping() { + let dir = tempfile::tempdir().expect("tempdir"); + let canonical_dir = dir.path().canonicalize().expect("canonicalize"); + let a = canonical_dir.join("a"); + let b = canonical_dir.join("b"); + std::os::unix::fs::symlink(&b, &a).expect("symlink a->b"); + std::os::unix::fs::symlink(&a, &b).expect("symlink b->a"); + + let hops = collect_symlink_hops(&a); + // Must terminate (this test itself is the proof); depth-bounded, so + // only ever sees the two distinct literal hop paths. + assert!(hops.len() <= MAX_SYMLINK_HOPS); + assert!(hops.contains(&a)); + assert!(hops.contains(&b)); + } + + #[cfg(unix)] + #[test] + fn collect_symlink_hops_nonexistent_final_target_keeps_existing_hops() { + let dir = tempfile::tempdir().expect("tempdir"); + let canonical_dir = dir.path().canonicalize().expect("canonicalize"); + + let hosts = canonical_dir.join("hosts"); + let mymac = hosts.join("mymac"); + fs::create_dir_all(&mymac).expect("mkdir mymac"); + // Note: no `gitconfig` file written under mymac -- final target + // does not exist, but the directory symlink hop is still real. + + let current = hosts.join("current"); + std::os::unix::fs::symlink(&mymac, ¤t).expect("symlink dir"); + + let gitconfig_link = canonical_dir.join(".gitconfig"); + std::os::unix::fs::symlink(current.join("gitconfig"), &gitconfig_link) + .expect("symlink leaf"); + + let hops = collect_symlink_hops(&gitconfig_link); + assert_eq!(hops, vec![gitconfig_link, current]); + } } diff --git a/crates/nono/src/sandbox/linux.rs b/crates/nono/src/sandbox/linux.rs index 94ed096ef..0a74e0f67 100644 --- a/crates/nono/src/sandbox/linux.rs +++ b/crates/nono/src/sandbox/linux.rs @@ -3843,6 +3843,33 @@ mod tests { } } + /// `open_path_rule` opens `cap.resolved` with `O_PATH`, so the kernel + /// resolves the whole chain before Landlock sees a literal path. + #[test] + fn test_open_path_rule_resolves_multi_hop_symlink_through_symlinked_directory() { + let dir = tempfile::tempdir().expect("tempdir"); + let canonical_dir = dir.path().canonicalize().expect("canonicalize"); + + let hosts = canonical_dir.join("hosts"); + let mymac = hosts.join("mymac"); + std::fs::create_dir_all(&mymac).expect("mkdir mymac"); + let real_gitconfig = mymac.join("gitconfig"); + std::fs::write(&real_gitconfig, "[user]\n").expect("write gitconfig"); + + let current = hosts.join("current"); + std::os::unix::fs::symlink(&mymac, ¤t).expect("symlink dir"); + + let gitconfig_link = canonical_dir.join(".gitconfig"); + std::os::unix::fs::symlink(current.join("gitconfig"), &gitconfig_link) + .expect("symlink leaf"); + + let cap = crate::capability::FsCapability::new_file(&gitconfig_link, AccessMode::Read) + .expect("gitconfig capability"); + + let rule = open_path_rule(&cap, ABI::V5).expect("open multi-hop symlink rule"); + assert!(rule.access.effective.contains(AccessFs::ReadFile)); + } + #[test] fn test_open_path_rule_rejects_removed_path() { let temp = tempfile::tempdir().expect("tempdir"); diff --git a/crates/nono/src/sandbox/macos.rs b/crates/nono/src/sandbox/macos.rs index f3d661e28..a20543e8c 100644 --- a/crates/nono/src/sandbox/macos.rs +++ b/crates/nono/src/sandbox/macos.rs @@ -9,6 +9,7 @@ use crate::capability::{ AccessMode, CapabilitySet, MACOS_PORT_RANGE_LIMIT, NetworkMode, merge_port_ranges, }; use crate::error::{NonoError, Result}; +use crate::path::collect_symlink_hops; use crate::sandbox::SupportInfo; use std::ffi::{CStr, CString}; use std::os::raw::c_char; @@ -185,6 +186,33 @@ pub fn support_info() -> SupportInfo { fn collect_parent_dirs(caps: &CapabilitySet) -> std::collections::HashSet { let mut parents = std::collections::HashSet::new(); + let insert_ancestors = |parents: &mut std::collections::HashSet, path: &Path| { + let mut current = path.parent(); + while let Some(parent) = current { + let parent_str = parent.to_string_lossy().to_string(); + + // Stop at root + if parent_str == "/" || parent_str.is_empty() { + break; + } + + // If already present, ancestors were processed too - early exit + if !parents.insert(parent_str) { + break; + } + current = parent.parent(); + } + }; + + // Intermediate hops need their own metadata grant, or the kernel + // denies access to them during dereference. + let grant_hops = |parents: &mut std::collections::HashSet, original: &Path| { + for hop in collect_symlink_hops(original) { + insert_ancestors(parents, &hop); + parents.insert(hop.to_string_lossy().to_string()); + } + }; + for cap in caps.fs_capabilities() { // Collect parents for both resolved and original paths. // On macOS, /tmp is a symlink to /private/tmp. If the user passes @@ -197,22 +225,24 @@ fn collect_parent_dirs(caps: &CapabilitySet) -> std::collections::HashSet = if cap.original != cap.resolved { + vec![cap.resolved.as_path(), cap.original.as_path()] + } else { + vec![cap.resolved.as_path()] + }; + + for path in paths_to_walk { + insert_ancestors(&mut parents, path); } + + grant_hops(&mut parents, &cap.original); } parents @@ -795,6 +825,40 @@ fn generate_profile(caps: &CapabilitySet) -> Result { )); current = parent.parent(); } + + // A $PATH dir reached through a multi-hop symlink needs metadata + // grants on the intermediate hops too, or the kernel denies EPERM + // (not ENOENT) dereferencing one, aborting the PATH walk. + for hop in collect_symlink_hops(dir) { + let Some(hop_str) = hop.to_str() else { + continue; + }; + if !seen_dir.contains(hop_str) && seen_ancestor.insert(hop_str.to_string()) { + let escaped = escape_path(hop_str)?; + profile.push_str(&format!( + "(allow file-read-metadata (literal \"{}\"))\n", + escaped + )); + } + let mut current = hop.parent(); + while let Some(parent) = current { + let Some(parent_str) = parent.to_str() else { + break; + }; + if parent_str == "/" || parent_str.is_empty() { + break; + } + if seen_dir.contains(parent_str) || !seen_ancestor.insert(parent_str.to_string()) { + break; + } + let escaped = escape_path(parent_str)?; + profile.push_str(&format!( + "(allow file-read-metadata (literal \"{}\"))\n", + escaped + )); + current = parent.parent(); + } + } } // Network rules @@ -1164,6 +1228,42 @@ mod tests { assert!(profile.contains("(allow file-read-metadata (regex \"^/opt/homebrew/[^/]+$\"))")); } + #[test] + #[cfg(unix)] + fn test_path_metadata_dir_through_multi_hop_symlink_grants_intermediate_hops() { + // Regression: a $PATH dir reached through a multi-hop symlink must + // get metadata grants on the intermediate hops, or the kernel denies + // EPERM (not ENOENT) dereferencing one, aborting the PATH walk. + let dir = tempfile::tempdir().expect("tempdir"); + let real_bin = dir.path().join("real_bin"); + std::fs::create_dir(&real_bin).expect("mkdir real_bin"); + let hop1 = dir.path().join("hop1"); + let hop2 = dir.path().join("hop2"); + std::os::unix::fs::symlink(&real_bin, &hop2).expect("symlink hop2 -> real_bin"); + std::os::unix::fs::symlink(&hop2, &hop1).expect("symlink hop1 -> hop2"); + + let mut caps = CapabilitySet::new(); + caps.add_path_metadata_dir(hop1.clone()); + + let profile = generate_profile(&caps).unwrap(); + + let hop1_str = hop1.to_str().unwrap(); + let hop2_str = hop2.to_str().unwrap(); + assert!( + profile.contains(&format!( + "(allow file-read-metadata (literal \"{hop2_str}\"))" + )), + "intermediate hop hop2 missing metadata grant:\n{profile}" + ); + assert!( + !profile.contains(&format!( + "(allow file-read-metadata (regex \"^{}/[^/]+$\"))", + regex_escape_path_for_seatbelt(hop2_str).unwrap() + )), + "intermediate hop must not get the direct-children regex, only the $PATH dir itself: {hop1_str}" + ); + } + #[test] fn test_support_info() { let info = support_info(); @@ -1189,6 +1289,65 @@ mod tests { assert!(!parents.contains("/")); } + /// A symlinked leaf through a symlinked directory component: + /// `.gitconfig -> hosts/current/gitconfig -> hosts/mymac/gitconfig`. + #[cfg(unix)] + #[test] + fn test_collect_parent_dirs_grants_intermediate_symlink_hop() { + let dir = tempfile::tempdir().unwrap(); + let canonical_dir = dir.path().canonicalize().unwrap(); + + let hosts = canonical_dir.join("hosts"); + let mymac = hosts.join("mymac"); + std::fs::create_dir_all(&mymac).unwrap(); + std::fs::write(mymac.join("gitconfig"), "[user]\n").unwrap(); + + let current = hosts.join("current"); + std::os::unix::fs::symlink(&mymac, ¤t).unwrap(); + + let gitconfig_link = canonical_dir.join(".gitconfig"); + std::os::unix::fs::symlink(current.join("gitconfig"), &gitconfig_link).unwrap(); + + let resolved = gitconfig_link.canonicalize().unwrap(); + + let mut caps = CapabilitySet::new(); + caps.add_fs(FsCapability { + original: gitconfig_link, + resolved, + access: AccessMode::Read, + is_file: true, + source: CapabilitySource::User, + }); + + let parents = collect_parent_dirs(&caps); + + assert!(parents.contains(¤t.to_string_lossy().to_string())); + + // Must not widen authority to a sibling under the same directory. + let sibling = hosts.join("other-host"); + assert!(!parents.contains(&sibling.to_string_lossy().to_string())); + } + + /// A single-hop socket symlink, to isolate `UnixSocketCapability` + /// ancestor grants from the multi-hop case tested above. + #[test] + fn test_collect_parent_dirs_grants_unix_socket_ancestors() { + let mut caps = CapabilitySet::new(); + caps.add_unix_socket(crate::UnixSocketCapability { + original: PathBuf::from("/tmp/test.sock"), + resolved: PathBuf::from("/private/tmp/test.sock"), + scope: crate::SocketScope::File, + mode: crate::UnixSocketMode::Connect, + source: CapabilitySource::User, + }); + + let parents = collect_parent_dirs(&caps); + + assert!(parents.contains("/private/tmp")); + assert!(parents.contains("/tmp")); + assert!(!parents.contains("/")); + } + #[test] fn test_escape_path() { assert_eq!(escape_path("/simple/path").unwrap(), "/simple/path");