fix(learn): agent workload bug fixes

crates/sandlock-cli/src/learn.rs

@@ -36,7 +36,7 @@ fn classify_path(p: &std::path::Path) -> PathTier {
     if GUARDED_PATHS.iter().any(|s| b == *s) {
         return PathTier::Guarded;
     }
-    // $HOME itself (non-root) is guarded — apps do legitimately write dotfiles there.
+    // $HOME itself (non-root) is guarded: apps do legitimately write dotfiles there.
     if let Ok(home) = std::env::var("HOME") {
         if b == home.as_bytes() && home != "/root" {
             return PathTier::Guarded;
@@ -76,12 +76,25 @@ fn collapse_write_paths(writes: &BTreeSet<PathBuf>) -> Vec<PathBuf> {
         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"/" {
+                eprintln!(
+                    "sandlock learn: WARNING: observed a direct write of '/', refusing to grant it"
+                );
+                continue;
+            }
+            match classify_path(p) {
+                PathTier::Protected | PathTier::Guarded => {
+                    eprintln!(
+                        "sandlock learn: NOTE: observed a direct write to '{}'",
+                        p.display()
+                    );
+                }
+                PathTier::Normal => {}
+            }
             out.insert(p.clone());
             continue;
         }
         let Some(ancestor) = p.ancestors().skip(1).find(|a| a.exists()) else { continue };
-        // "/" is always skipped: granting write access to the filesystem root
-        // is never useful and would override every other policy entry.
         if ancestor.as_os_str().as_encoded_bytes() == b"/" {
             eprintln!(
                 "sandlock learn: WARNING: write collapse for '{}' reaches filesystem root, skipping",
@@ -190,8 +203,21 @@ fn collapse_by_threshold(
 /// 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();
-    b.starts_with(b"/proc/self")
-        || (b.starts_with(b"/proc/") && b.get(6).map_or(false, u8::is_ascii_digit))
+    // /proc/<pid>/... are pid-specific across runs.
+    let proc_numeric_pid = b.starts_with(b"/proc/")
+        && b.get(6).map_or(false, u8::is_ascii_digit);
+    // Under /proc/self, only sub-trees with volatile numeric components are
+    // junk. Stable entries like /proc/self/maps are legitimately needed
+    // across runs (e.g. V8 reads maps on every startup).
+    let proc_self_volatile = b.starts_with(b"/proc/self/fd/")
+        || b.starts_with(b"/proc/self/fdinfo/")
+        || b.starts_with(b"/proc/self/task/")
+        || b.starts_with(b"/proc/self/map_files/")
+        || b == b"/proc/self/fd"
+        || b == b"/proc/self/fdinfo"
+        || b == b"/proc/self/task"
+        || b == b"/proc/self/map_files";
+    proc_numeric_pid || proc_self_volatile
 }
 
 /// A pty's number changes between sessions; granting the directory is
@@ -352,7 +378,7 @@ impl LearnObserver {
             }
             "openat" | "open" => {
                 if let Some(path) = event.path {
-                    let path = canonicalize_or_keep(path);
+                    let path = canonicalize_or_keep(path, event.pid);
                     if let Some(fl) = event.flags {
                         if is_write_open(fl) {
                             self.writes.lock().unwrap().insert(path);
@@ -366,9 +392,13 @@ impl LearnObserver {
             // rights, so the parent dir is what sandlock run needs, not the target.
             "mkdirat" | "mknodat" => {
                 if let Some(p) = event.path {
-                    let p = canonicalize_or_keep(p);
-                    if let Some(parent) = p.parent() {
-                        self.writes.lock().unwrap().insert(parent.to_path_buf());
+                    let p = canonicalize_or_keep(p, event.pid);
+                    // If the target already exists the syscall will fail with
+                    // EEXIST; no MAKE_DIR right on the parent is needed.
+                    if !p.exists() {
+                        if let Some(parent) = p.parent() {
+                            self.writes.lock().unwrap().insert(parent.to_path_buf());
+                        }
                     }
                 }
             }
@@ -396,7 +426,7 @@ impl LearnObserver {
             // dst operates on the link itself, so only the parent is canonicalized.
             "linkat" => {
                 if let Some(src) = event.path {
-                    self.reads.lock().unwrap().insert(canonicalize_or_keep(src));
+                    self.reads.lock().unwrap().insert(canonicalize_or_keep(src, event.pid));
                 }
                 if let Some(dst) = event.path2 {
                     let dst = canonicalize_parent_or_keep(dst);
@@ -408,7 +438,7 @@ impl LearnObserver {
             // truncate: LANDLOCK_ACCESS_FS_TRUNCATE applies to the file itself.
             "truncate" => {
                 if let Some(p) = event.path {
-                    self.writes.lock().unwrap().insert(canonicalize_or_keep(p));
+                    self.writes.lock().unwrap().insert(canonicalize_or_keep(p, event.pid));
                 }
             }
             "bind" => {
@@ -419,7 +449,7 @@ impl LearnObserver {
                 } else if let Some(p) = event.path {
                     // AF_UNIX named bind: Landlock MAKE_SOCK is a directory right,
                     // so the parent dir is what sandlock run needs.
-                    let p = canonicalize_or_keep(p);
+                    let p = canonicalize_or_keep(p, event.pid);
                     if let Some(parent) = p.parent() {
                         self.writes.lock().unwrap().insert(parent.to_path_buf());
                     }
@@ -484,7 +514,24 @@ impl LearnObserver {
 
 /// Resolve symlinks to get the canonical path. Falls back to the original
 /// if the path doesn't exist yet (e.g. COW-intercepted creates).
-fn canonicalize_or_keep(p: PathBuf) -> PathBuf {
+///
+/// /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/<pid>/ 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;
+    }
     std::fs::canonicalize(&p).unwrap_or(p)
 }
 
@@ -644,7 +691,7 @@ pub async fn run(args: LearnArgs) -> Result<()> {
             // other read in the profile.
             if p.as_path() == std::path::Path::new("/") {
                 eprintln!(
-                    "sandlock learn: WARNING: observed a read of '/', refusing to grant it"
+                    "sandlock learn: WARNING: observed a direct read of '/', refusing to grant it"
                 );
                 return false;
             }

crates/sandlock-cli/tests/learn_test.rs

@@ -705,7 +705,7 @@ fn test_write_collapse_skips_root() {
     let write_line = stdout.lines().find(|l| l.starts_with("write = [")).unwrap_or("");
     assert!(!write_line.contains("\"/\""), "write list must not contain \"/\", got: {write_line}");
     assert!(stderr.contains("filesystem root"),
-        "expected 'filesystem root' warning in stderr, got: {stderr}");
+        "expected filesystem root warning in stderr, got: {stderr}");
 }
 
 /// Write collapse to a sensitive path emits a warning and observed-vs-granted diff.
@@ -747,6 +747,83 @@ fn test_write_collapse_skips_protected() {
         "expected 'protected path' error in stderr, got: {stderr}");
 }
 
+/// mkdirat on an existing target (EEXIST) must not add the parent to the write set.
+#[test]
+fn test_mkdirat_eexist_no_write() {
+    let base = tempfile::TempDir::new_in("/var/tmp").expect("tempdir in /var/tmp");
+    let existing = base.path().join("already_there");
+    std::fs::create_dir(&existing).expect("create subdir");
+    let existing_str = existing.to_str().unwrap();
+    let base_str = base.path().to_str().unwrap();
+    // mkdirat fires pre-syscall; without the EEXIST guard the parent
+    // (base_str) would be inserted into the write set.
+    let output = sandlock_bin()
+        .args(["learn", "--", "sh", "-c", &format!("mkdir {existing_str} 2>/dev/null; true")])
+        .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 write_line = stdout.lines().find(|l| l.starts_with("write = [")).unwrap_or("write = []");
+    assert!(!write_line.contains(base_str),
+        "mkdirat on existing target must not insert parent {base_str} into writes, got: {write_line}");
+}
+
+/// Direct write to "/" must be dropped with a warning, not recorded.
+#[test]
+fn test_direct_write_root_skipped() {
+    let dir = format!("/sandlock_learn_root_mkdir_{}", std::process::id());
+    let output = sandlock_bin()
+        .args(["learn", "--", "sh", "-c", &format!("mkdir {dir} 2>/dev/null; true")])
+        .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 stderr = String::from_utf8_lossy(&output.stderr);
+    let write_line = stdout.lines().find(|l| l.starts_with("write = [")).unwrap_or("write = []");
+    assert!(!write_line.contains("\"/\""),
+        "direct write of \"/\" must be dropped, got: {write_line}");
+    assert!(stderr.contains("direct write of '/'"),
+        "expected direct write warning in stderr, got: {stderr}");
+}
+
+/// /proc/self/maps must appear in the profile as /proc/self/maps
+#[test]
+fn test_proc_self_maps_preserved() {
+    let output = sandlock_bin()
+        .args(["learn", "--", "python3", "-c", "open('/proc/self/maps').read()"])
+        .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/maps\""),
+        "/proc/self/maps must be recorded as-is, got: {read_line}");
+    let has_numeric_pid = read_line.split("\"/proc/").skip(1).any(|s| {
+        s.chars().next().map_or(false, |c| c.is_ascii_digit())
+    });
+    assert!(!has_numeric_pid,
+        "must not contain a numeric-pid /proc/<pid>/... entry, got: {read_line}");
+}
+
+/// /proc/self/exe is a symlink to the real binary; it must be resolved to the
+/// binary path, not recorded as /proc/self/exe.
+#[test]
+fn test_proc_self_exe_resolves_to_binary() {
+    let output = sandlock_bin()
+        .args(["learn", "--", "python3", "-c", "import os; os.open('/proc/self/exe', os.O_RDONLY)"])
+        .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/exe\""),
+        "/proc/self/exe must be resolved to the binary path, not recorded as-is: {read_line}");
+}
+
 // ── Merge and canonicalization ────────────────────────────────────────────────
 
 /// --merge unions observations from a new run into an existing profile.

docs/learn.md

@@ -53,11 +53,13 @@ optional, so omitting the ancestor would cause `sandlock run` to abort.
 
 ### Path tiers
 
-| Tier | Paths | Write (auto) | `--collapse N` | `--collapse-prefix` |
-|---|---|---|---|---|
-| **Protected** | `/`, `/root`, `~/.ssh`, `~/.aws`, `~/.kube`, `~/.gnupg` | skip + error | never (keep individual file) | refused unless `--force-sensitive-collapse` |
-| **Guarded** | `/etc`, `/proc`, `/sys`, `/dev`, `/boot`, `/run/secrets` | emit + warning + diff | never (keep individual file) | refused unless `--force-sensitive-collapse` |
-| **Normal** | everything else | collapse freely | collapse freely | collapse freely |
+| Tier | Paths | Write collapse | `--collapse` / `--collapse-prefix` |
+|---|---|---|---|
+| **Protected** | `/`, `/root`, paths ending in `/.ssh` `/.aws` `/.kube` `/.gnupg` | skip + error | never (keep individual files; override with `--force-sensitive-collapse`) |
+| **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. 
 
 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
@@ -81,10 +83,10 @@ cargo build -p sandlock-cli
 Each test spawns a full sandlock process; running too many in
 parallel exhausts kernel limits and causes hangs, use `--test-threads=4`.
 ```bash
-# learn output tests — verify TOML profile content
+# learn output tests - verify TOML profile content
 cargo test -p sandlock-cli --test learn_test -- --test-threads=4
 
-# learn round-trip tests — learn → profile → run end-to-end
+# learn round-trip tests - learn → profile → run end-to-end
 cargo test -p sandlock-cli --test learn_integration -- --test-threads=4
 ```