refactor(sandbox): deduplicate baseline path enrichment logic

Extract shared helpers for the two enrich_*_baseline_paths functions:
- baseline_paths() collects all proxy + GPU paths in one place
- missing_baseline_paths() filters to existing, not-yet-present paths

This removes the duplicated "check exists, skip if missing, push"
pattern that was repeated four times in each enrichment function.

Author: pimlock

crates/openshell-sandbox/src/lib.rs

@@ -968,6 +968,55 @@ fn gpu_baseline_read_write_paths() -> Vec<std::path::PathBuf> {
     paths
 }
 
+/// Filter baseline paths to those that exist on disk and are not already
+/// present in `existing`.
+///
+/// Baseline paths are system-injected, not user-specified.  Skip paths that
+/// do not exist in this container image to avoid noisy warnings from Landlock
+/// and, more critically, to prevent a single missing baseline path from
+/// abandoning the entire Landlock ruleset under best-effort mode (see
+/// issue #664).
+fn missing_baseline_paths(
+    candidates: &[std::path::PathBuf],
+    existing: &[impl AsRef<std::path::Path>],
+) -> Vec<std::path::PathBuf> {
+    candidates
+        .iter()
+        .filter(|p| {
+            if existing.iter().any(|e| e.as_ref() == p.as_path()) {
+                return false;
+            }
+            if !p.exists() {
+                debug!(path = %p.display(), "Baseline path does not exist, skipping enrichment");
+                return false;
+            }
+            true
+        })
+        .cloned()
+        .collect()
+}
+
+/// All baseline paths that should be added for a proxy-mode sandbox,
+/// split into (read_only, read_write).  Includes GPU paths when GPU
+/// devices are detected.
+fn baseline_paths() -> (Vec<std::path::PathBuf>, Vec<std::path::PathBuf>) {
+    let mut ro: Vec<std::path::PathBuf> = PROXY_BASELINE_READ_ONLY
+        .iter()
+        .map(|&p| p.into())
+        .collect();
+    let mut rw: Vec<std::path::PathBuf> = PROXY_BASELINE_READ_WRITE
+        .iter()
+        .map(|&p| p.into())
+        .collect();
+
+    if has_gpu_devices() {
+        ro.extend(gpu_baseline_read_only_paths());
+        rw.extend(gpu_baseline_read_write_paths());
+    }
+
+    (ro, rw)
+}
+
 /// Ensure a proto `SandboxPolicy` includes the baseline filesystem paths
 /// required for proxy-mode sandboxes.  Paths are only added if missing;
 /// user-specified paths are never removed.
@@ -986,84 +1035,30 @@ fn enrich_proto_baseline_paths(proto: &mut openshell_core::proto::SandboxPolicy)
             ..Default::default()
         });
 
-    let mut modified = false;
-    for &path in PROXY_BASELINE_READ_ONLY {
-        if !fs.read_only.iter().any(|p| p.as_str() == path) {
-            // Baseline paths are system-injected, not user-specified. Skip
-            // paths that do not exist in this container image to avoid noisy
-            // warnings from Landlock and, more critically, to prevent a single
-            // missing baseline path from abandoning the entire Landlock
-            // ruleset under best-effort mode (see issue #664).
-            if !std::path::Path::new(path).exists() {
-                debug!(
-                    path,
-                    "Baseline read-only path does not exist, skipping enrichment"
-                );
-                continue;
-            }
-            fs.read_only.push(path.to_string());
-            modified = true;
-        }
-    }
-    for &path in PROXY_BASELINE_READ_WRITE {
-        if !fs.read_write.iter().any(|p| p.as_str() == path) {
-            if !std::path::Path::new(path).exists() {
-                debug!(
-                    path,
-                    "Baseline read-write path does not exist, skipping enrichment"
-                );
-                continue;
-            }
-            fs.read_write.push(path.to_string());
-            modified = true;
-        }
-    }
+    let (ro, rw) = baseline_paths();
 
-    if has_gpu_devices() {
-        for path in gpu_baseline_read_only_paths() {
-            let path_str = path.to_string_lossy();
-            if !fs.read_only.iter().any(|p| p.as_str() == path_str.as_ref())
-                && !fs
-                    .read_write
-                    .iter()
-                    .any(|p| p.as_str() == path_str.as_ref())
-            {
-                if !path.exists() {
-                    debug!(
-                        path = %path.display(),
-                        "GPU baseline read-only path does not exist, skipping enrichment"
-                    );
-                    continue;
-                }
-                fs.read_only.push(path_str.into_owned());
-                modified = true;
-            }
-        }
-        for path in gpu_baseline_read_write_paths() {
-            let path_str = path.to_string_lossy();
-            if !fs
-                .read_write
-                .iter()
-                .any(|p| p.as_str() == path_str.as_ref())
-            {
-                if !path.exists() {
-                    debug!(
-                        path = %path.display(),
-                        "GPU baseline read-write path does not exist, skipping enrichment"
-                    );
-                    continue;
-                }
-                fs.read_write.push(path_str.into_owned());
-                modified = true;
-            }
-        }
+    // For read-only, skip paths already in either list (read_write subsumes read_only).
+    let both: Vec<&str> = fs
+        .read_only
+        .iter()
+        .chain(&fs.read_write)
+        .map(String::as_str)
+        .collect();
+    let ro_add = missing_baseline_paths(&ro, &both);
+    let rw_add = missing_baseline_paths(&rw, &fs.read_write);
+
+    let added = ro_add.len() + rw_add.len();
+    for p in ro_add {
+        fs.read_only.push(p.to_string_lossy().into_owned());
+    }
+    for p in rw_add {
+        fs.read_write.push(p.to_string_lossy().into_owned());
     }
 
-    if modified {
+    if added > 0 {
         info!("Enriched policy with baseline filesystem paths for proxy mode");
     }
-
-    modified
+    added > 0
 }
 
 /// Ensure a `SandboxPolicy` (Rust type) includes the baseline filesystem
@@ -1074,70 +1069,22 @@ fn enrich_sandbox_baseline_paths(policy: &mut SandboxPolicy) {
         return;
     }
 
-    let mut modified = false;
-    for &path in PROXY_BASELINE_READ_ONLY {
-        let p = std::path::PathBuf::from(path);
-        if !policy.filesystem.read_only.contains(&p) {
-            // Baseline paths are system-injected — skip non-existent paths to
-            // avoid Landlock ruleset abandonment (issue #664).
-            if !p.exists() {
-                debug!(
-                    path,
-                    "Baseline read-only path does not exist, skipping enrichment"
-                );
-                continue;
-            }
-            policy.filesystem.read_only.push(p);
-            modified = true;
-        }
-    }
-    for &path in PROXY_BASELINE_READ_WRITE {
-        let p = std::path::PathBuf::from(path);
-        if !policy.filesystem.read_write.contains(&p) {
-            if !p.exists() {
-                debug!(
-                    path,
-                    "Baseline read-write path does not exist, skipping enrichment"
-                );
-                continue;
-            }
-            policy.filesystem.read_write.push(p);
-            modified = true;
-        }
-    }
+    let (ro, rw) = baseline_paths();
 
-    if has_gpu_devices() {
-        for p in gpu_baseline_read_only_paths() {
-            if !policy.filesystem.read_only.contains(&p)
-                && !policy.filesystem.read_write.contains(&p)
-            {
-                if !p.exists() {
-                    debug!(
-                        path = %p.display(),
-                        "GPU baseline read-only path does not exist, skipping enrichment"
-                    );
-                    continue;
-                }
-                policy.filesystem.read_only.push(p);
-                modified = true;
-            }
-        }
-        for p in gpu_baseline_read_write_paths() {
-            if !policy.filesystem.read_write.contains(&p) {
-                if !p.exists() {
-                    debug!(
-                        path = %p.display(),
-                        "GPU baseline read-write path does not exist, skipping enrichment"
-                    );
-                    continue;
-                }
-                policy.filesystem.read_write.push(p);
-                modified = true;
-            }
-        }
-    }
+    let both: Vec<&std::path::PathBuf> = policy
+        .filesystem
+        .read_only
+        .iter()
+        .chain(&policy.filesystem.read_write)
+        .collect();
+    let ro_add = missing_baseline_paths(&ro, &both);
+    let rw_add = missing_baseline_paths(&rw, &policy.filesystem.read_write);
+
+    let added = ro_add.len() + rw_add.len();
+    policy.filesystem.read_only.extend(ro_add);
+    policy.filesystem.read_write.extend(rw_add);
 
-    if modified {
+    if added > 0 {
         info!("Enriched policy with baseline filesystem paths for proxy mode");
     }
 }