From 8e130d34e1532f759424467c3075cefefcc4bd29 Mon Sep 17 00:00:00 2001 From: Cong Wang Date: Wed, 9 Sep 2026 17:12:42 -0700 Subject: [PATCH 1/3] learn: keep bare /proc/self and resolve parents via event pid canonicalize_or_keep rewrote "/proc/self" to "/proc/" but only mapped results back when they carried a trailing "//" prefix, so an open of /proc/self itself came out as a numeric-pid path and was dropped as junk. Use Path::strip_prefix on both sides so the bare directory and its entries round-trip the same way, and stop matching unrelated names that merely start with "/proc/self". canonicalize_parent_or_keep still resolved against the supervisor's own /proc/self, so rename or unlink targets under /proc/self/cwd landed on the wrong process. Route it through the same pid-aware helper. The exe test only asserted that /proc/self/exe was absent, which a dropped read would also satisfy; it now checks the resolved interpreter path is present. Signed-off-by: Cong Wang --- crates/sandlock-cli/src/learn.rs | 44 +++++++++++++------------ crates/sandlock-cli/tests/learn_test.rs | 25 ++++++++++++++ 2 files changed, 48 insertions(+), 21 deletions(-) diff --git a/crates/sandlock-cli/src/learn.rs b/crates/sandlock-cli/src/learn.rs index 6bf372ce..9295cb68 100644 --- a/crates/sandlock-cli/src/learn.rs +++ b/crates/sandlock-cli/src/learn.rs @@ -4,7 +4,7 @@ //! usable by `sandlock run -p`. use std::collections::{BTreeMap, BTreeSet, HashMap, HashSet}; -use std::path::PathBuf; +use std::path::{Path, PathBuf}; use std::sync::{Arc, Mutex}; use anyhow::{anyhow, Result}; @@ -406,7 +406,7 @@ impl LearnObserver { // only the parent is canonicalized to avoid following the symlink. "unlinkat" | "symlinkat" => { if let Some(p) = event.path { - let p = canonicalize_parent_or_keep(p); + let p = canonicalize_parent_or_keep(p, event.pid); if let Some(parent) = p.parent() { self.writes.lock().unwrap().insert(parent.to_path_buf()); } @@ -416,7 +416,7 @@ impl LearnObserver { // Both operate on the link itself, not its target. "renameat2" => { for p in [event.path, event.path2].into_iter().flatten() { - let p = canonicalize_parent_or_keep(p); + let p = canonicalize_parent_or_keep(p, event.pid); if let Some(parent) = p.parent() { self.writes.lock().unwrap().insert(parent.to_path_buf()); } @@ -429,7 +429,7 @@ impl LearnObserver { self.reads.lock().unwrap().insert(canonicalize_or_keep(src, event.pid)); } if let Some(dst) = event.path2 { - let dst = canonicalize_parent_or_keep(dst); + let dst = canonicalize_parent_or_keep(dst, event.pid); if let Some(parent) = dst.parent() { self.writes.lock().unwrap().insert(parent.to_path_buf()); } @@ -517,27 +517,30 @@ impl LearnObserver { /// /// /proc/self paths are canonicalized via the event pid so symlinks /// (e.g. /proc/self/exe) resolve against the workload, not the supervisor. -/// Results still under /proc// are mapped back to /proc/self/. +/// Results still under /proc/ are mapped back to /proc/self. fn canonicalize_or_keep(p: PathBuf, pid: u32) -> PathBuf { - let b = p.as_os_str().as_encoded_bytes(); - if b.starts_with(b"/proc/self") { - let suffix = &b[b"/proc/self".len()..]; - let pid_path = PathBuf::from(format!("/proc/{}{}", pid, String::from_utf8_lossy(suffix))); - let resolved = std::fs::canonicalize(&pid_path).unwrap_or(pid_path); - let resolved_b = resolved.as_os_str().as_encoded_bytes(); - let pid_prefix = format!("/proc/{}/", pid); - if resolved_b.starts_with(pid_prefix.as_bytes()) { - let rest = &resolved_b[pid_prefix.len() - 1..]; // keep leading / - return PathBuf::from(format!("/proc/self{}", String::from_utf8_lossy(rest))); - } - return resolved; + let proc_self = Path::new("/proc/self"); + let Ok(rest) = p.strip_prefix(proc_self) else { + return std::fs::canonicalize(&p).unwrap_or(p); + }; + let proc_pid = Path::new("/proc").join(pid.to_string()); + let pid_path = join_rest(&proc_pid, rest); + let resolved = std::fs::canonicalize(&pid_path).unwrap_or(pid_path); + match resolved.strip_prefix(&proc_pid) { + Ok(rest) => join_rest(proc_self, rest), + Err(_) => resolved, } - std::fs::canonicalize(&p).unwrap_or(p) +} + +/// PathBuf::join appends a separator even for an empty component, which +/// would turn the bare directory into "dir/". +fn join_rest(base: &Path, rest: &Path) -> PathBuf { + if rest.as_os_str().is_empty() { base.to_path_buf() } else { base.join(rest) } } /// Canonicalize only the parent directory and rejoin the final component. /// Used for syscalls that operate on the link itself (unlink, rename, symlink). -fn canonicalize_parent_or_keep(p: PathBuf) -> PathBuf { +fn canonicalize_parent_or_keep(p: PathBuf, pid: u32) -> PathBuf { let file_name = match p.file_name() { Some(n) => n.to_owned(), None => return p, @@ -546,8 +549,7 @@ fn canonicalize_parent_or_keep(p: PathBuf) -> PathBuf { Some(par) => par, None => return p, }; - let canonical_parent = std::fs::canonicalize(parent).unwrap_or_else(|_| parent.to_path_buf()); - canonical_parent.join(file_name) + canonicalize_or_keep(parent.to_path_buf(), pid).join(file_name) } pub async fn run(args: LearnArgs) -> Result<()> { diff --git a/crates/sandlock-cli/tests/learn_test.rs b/crates/sandlock-cli/tests/learn_test.rs index b77a8595..18c28273 100644 --- a/crates/sandlock-cli/tests/learn_test.rs +++ b/crates/sandlock-cli/tests/learn_test.rs @@ -822,6 +822,31 @@ fn test_proc_self_exe_resolves_to_binary() { let read_line = stdout.lines().find(|l| l.starts_with("read = [")).unwrap_or(""); assert!(!read_line.contains("\"/proc/self/exe\""), "/proc/self/exe must be resolved to the binary path, not recorded as-is: {read_line}"); + let interp = std::process::Command::new("python3") + .args(["-c", "import sys; print(sys.executable)"]) + .output() + .expect("run python3"); + let interp = String::from_utf8_lossy(&interp.stdout).trim().to_string(); + let interp = std::fs::canonicalize(&interp).expect("canonicalize python3"); + let expected = format!("\"{}\"", interp.display()); + assert!(read_line.contains(&expected), + "expected resolved interpreter {expected} in reads, got: {read_line}"); +} + +/// Opening /proc/self itself (no sub-entry) must be recorded as /proc/self, +/// not resolved to /proc/ and then dropped as junk. +#[test] +fn test_proc_self_bare_preserved() { + let output = sandlock_bin() + .args(["learn", "--", "python3", "-c", "import os; os.listdir('/proc/self')"]) + .output() + .expect("failed to run sandlock learn"); + assert!(output.status.success(), + "sandlock learn failed: stderr={}", String::from_utf8_lossy(&output.stderr)); + let stdout = String::from_utf8_lossy(&output.stdout); + let read_line = stdout.lines().find(|l| l.starts_with("read = [")).unwrap_or(""); + assert!(read_line.contains("\"/proc/self\""), + "/proc/self must be recorded as-is, got: {read_line}"); } // ── Merge and canonicalization ──────────────────────────────────────────────── From 36cd3d2152238888c804cad03b8d0d914d623754 Mon Sep 17 00:00:00 2001 From: Cong Wang Date: Wed, 9 Sep 2026 17:13:05 -0700 Subject: [PATCH 2/3] learn: share one filesystem-root check The direct-write, ancestor-walk, and read filters each spelled the "/" guard differently (byte compare vs Path compare), which hides that they are the same rule. Signed-off-by: Cong Wang --- crates/sandlock-cli/src/learn.rs | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/crates/sandlock-cli/src/learn.rs b/crates/sandlock-cli/src/learn.rs index 9295cb68..268ec5d5 100644 --- a/crates/sandlock-cli/src/learn.rs +++ b/crates/sandlock-cli/src/learn.rs @@ -76,7 +76,7 @@ fn collapse_write_paths(writes: &BTreeSet) -> Vec { if is_junk_path(p) { continue; } let p = &fold_session_path(p.clone()); if p.exists() { - if p.as_os_str().as_encoded_bytes() == b"/" { + if is_fs_root(p) { eprintln!( "sandlock learn: WARNING: observed a direct write of '/', refusing to grant it" ); @@ -95,7 +95,7 @@ fn collapse_write_paths(writes: &BTreeSet) -> Vec { continue; } let Some(ancestor) = p.ancestors().skip(1).find(|a| a.exists()) else { continue }; - if ancestor.as_os_str().as_encoded_bytes() == b"/" { + if is_fs_root(&ancestor) { eprintln!( "sandlock learn: WARNING: write collapse for '{}' reaches filesystem root, skipping", p.display() @@ -200,6 +200,10 @@ fn collapse_by_threshold( out.into_iter().collect() } +fn is_fs_root(p: &Path) -> bool { + p == Path::new("/") +} + /// Returns true for pid-specific paths that are meaningless across runs. fn is_junk_path(p: &std::path::Path) -> bool { let b = p.as_os_str().as_encoded_bytes(); @@ -691,7 +695,7 @@ pub async fn run(args: LearnArgs) -> Result<()> { // whose cwd is the workdir root lists it routinely (python's -c // puts the cwd on sys.path), and granting it would subsume every // other read in the profile. - if p.as_path() == std::path::Path::new("/") { + if is_fs_root(p) { eprintln!( "sandlock learn: WARNING: observed a direct read of '/', refusing to grant it" ); From 0e74b870843a800e3e27e84a613fc3b3431d658c Mon Sep 17 00:00:00 2001 From: Cong Wang Date: Wed, 9 Sep 2026 17:13:05 -0700 Subject: [PATCH 3/3] docs: match learn direct-write note to the code The paragraph said a warning is printed for direct writes to Protected or Guarded paths, but collapse_write_paths prints a NOTE and still records the path. Also drop the trailing whitespace. Signed-off-by: Cong Wang --- docs/learn.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/learn.md b/docs/learn.md index 2008af89..7445c55a 100644 --- a/docs/learn.md +++ b/docs/learn.md @@ -59,7 +59,7 @@ optional, so omitting the ancestor would cause `sandlock run` to abort. | **Guarded** | `$HOME`, `/etc`, `/proc`, `/sys`, `/dev`, `/boot`, `/run/secrets` | emit + warning + diff | never (keep individual files; override with `--force-sensitive-collapse`) | | **Normal** | everything else | collapse freely | collapse freely | -The tiers apply to write collapse only. **The only path dropped from direct writes/reads is `/`**: granting it would subsume every other entry in the profile. Also, warning is printed to stderr when the direct write path is Protected or Guarded. +The tiers apply to write collapse only. **The only path dropped from direct writes and reads is `/`**: granting it would subsume every other entry in the profile. A direct write to a Protected or Guarded path is still recorded, with a NOTE printed to stderr. When a write collapse lands on a guarded path, a warning is printed to stderr along with an **observed-vs-granted diff**, the list of siblings