fix(sandbox): preserve explicit read-only baseline paths (#910)

Closes #905

Skip proxy baseline read-write enrichment when a path is already explicitly configured as read-only, add regression coverage for the proto and local policy code paths, and document the precedence rule in the sandbox architecture docs.

Signed-off-by: John Myers <9696606+johntmyers@users.noreply.github.com>

Author: johntmyers

architecture/sandbox.md

@@ -452,7 +452,7 @@ Kernel-level error behavior (e.g., Landlock ABI unavailable) depends on `Landloc
 - `BestEffort`: Log a warning and continue without filesystem isolation
 - `HardRequirement`: Return a fatal error, aborting the sandbox
 
-**Baseline path filtering**: System-injected baseline paths (e.g., `/app`) are pre-filtered by `enrich_proto_baseline_paths()` / `enrich_sandbox_baseline_paths()` using `Path::exists()` before they reach Landlock. User-specified paths are not pre-filtered -- they are evaluated at Landlock apply time so misconfigurations surface as warnings or errors.
+**Baseline path filtering**: System-injected baseline paths (e.g., `/app`) are pre-filtered by `enrich_proto_baseline_paths()` / `enrich_sandbox_baseline_paths()` using `Path::exists()` before they reach Landlock. If a baseline `read_write` path is already present in `read_only`, enrichment skips the promotion so explicit policy intent is preserved. User-specified paths are not pre-filtered -- they are evaluated at Landlock apply time so misconfigurations surface as warnings or errors.
 
 ### Seccomp syscall filtering
 

architecture/security-policy.md

@@ -393,7 +393,7 @@ Controls Landlock LSM compatibility behavior. **Static field** -- immutable afte
 
 **Per-path error handling**: `PathFd::new()` (which wraps `open(path, O_PATH | O_CLOEXEC)`) can fail for several reasons beyond path non-existence: `EACCES` (permission denied), `ELOOP` (symlink loop), `ENAMETOOLONG`, `ENOTDIR`. Each failure is classified with a human-readable reason in logs. In `best_effort` mode, the path is skipped and ruleset construction continues. In `hard_requirement` mode, the error is fatal.
 
-**Baseline path filtering**: The enrichment functions (`enrich_proto_baseline_paths`, `enrich_sandbox_baseline_paths`) pre-filter system-injected baseline paths (e.g., `/app`) by checking `Path::exists()` before adding them to the policy. This prevents missing baseline paths from reaching Landlock at all. User-specified paths are not pre-filtered — they are evaluated at Landlock apply time so that misconfigurations surface as warnings (`best_effort`) or errors (`hard_requirement`).
+**Baseline path filtering**: The enrichment functions (`enrich_proto_baseline_paths`, `enrich_sandbox_baseline_paths`) pre-filter system-injected baseline paths (e.g., `/app`) by checking `Path::exists()` before adding them to the policy. This prevents missing baseline paths from reaching Landlock at all. If a baseline `read_write` path is explicitly configured in `read_only`, enrichment skips the promotion and preserves the stricter policy intent. User-specified paths are not pre-filtered — they are evaluated at Landlock apply time so that misconfigurations surface as warnings (`best_effort`) or errors (`hard_requirement`).
 
 **Zero-rule safety check**: If all paths in the ruleset fail to open, `apply()` returns an error rather than calling `restrict_self()` on an empty ruleset. An empty Landlock ruleset with `restrict_self()` would block all filesystem access — the inverse of the intended degradation behavior. This error is caught by the outer `BestEffort` handler, which logs a warning and continues without Landlock.
 

crates/openshell-sandbox/src/lib.rs

@@ -1347,17 +1347,18 @@ fn enrich_proto_baseline_paths(proto: &mut openshell_core::proto::SandboxPolicy)
         }
     }
     for path in &rw {
-        if !fs.read_write.iter().any(|p| p == 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.clone());
-            modified = true;
+        if fs.read_only.iter().any(|p| p == path) || fs.read_write.iter().any(|p| p == path) {
+            continue;
         }
+        if !std::path::Path::new(path).exists() {
+            debug!(
+                path,
+                "Baseline read-write path does not exist, skipping enrichment"
+            );
+            continue;
+        }
+        fs.read_write.push(path.clone());
+        modified = true;
     }
 
     if modified {
@@ -1401,17 +1402,18 @@ fn enrich_sandbox_baseline_paths(policy: &mut SandboxPolicy) {
     }
     for path in &rw {
         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;
+        if policy.filesystem.read_only.contains(&p) || policy.filesystem.read_write.contains(&p) {
+            continue;
         }
+        if !p.exists() {
+            debug!(
+                path,
+                "Baseline read-write path does not exist, skipping enrichment"
+            );
+            continue;
+        }
+        policy.filesystem.read_write.push(p);
+        modified = true;
     }
 
     if modified {
@@ -1429,6 +1431,7 @@ fn enrich_sandbox_baseline_paths(policy: &mut SandboxPolicy) {
 #[cfg(test)]
 mod baseline_tests {
     use super::*;
+    use crate::policy::{FilesystemPolicy, LandlockPolicy, ProcessPolicy};
 
     #[test]
     fn proc_not_in_both_read_only_and_read_write_when_gpu_present() {
@@ -1493,6 +1496,75 @@ mod baseline_tests {
             );
         }
     }
+
+    #[test]
+    fn proto_enrichment_preserves_explicit_read_only_for_baseline_read_write_paths() {
+        let mut policy = openshell_policy::restrictive_default_policy();
+        policy.filesystem = Some(openshell_core::proto::FilesystemPolicy {
+            read_only: vec!["/tmp".to_string()],
+            read_write: vec![],
+            include_workdir: false,
+        });
+        policy.network_policies.insert(
+            "test".into(),
+            openshell_core::proto::NetworkPolicyRule {
+                name: "test-rule".into(),
+                endpoints: vec![openshell_core::proto::NetworkEndpoint {
+                    host: "example.com".into(),
+                    port: 443,
+                    ..Default::default()
+                }],
+                ..Default::default()
+            },
+        );
+
+        enrich_proto_baseline_paths(&mut policy);
+
+        let filesystem = policy.filesystem.expect("filesystem policy");
+        assert!(
+            filesystem.read_only.contains(&"/tmp".to_string()),
+            "explicit read_only baseline path should be preserved"
+        );
+        assert!(
+            !filesystem.read_write.contains(&"/tmp".to_string()),
+            "baseline enrichment must not promote explicit read_only /tmp to read_write"
+        );
+    }
+
+    #[test]
+    fn local_enrichment_preserves_explicit_read_only_for_baseline_read_write_paths() {
+        let mut policy = SandboxPolicy {
+            version: 1,
+            filesystem: FilesystemPolicy {
+                read_only: vec![std::path::PathBuf::from("/tmp")],
+                read_write: vec![],
+                include_workdir: false,
+            },
+            network: NetworkPolicy {
+                mode: NetworkMode::Proxy,
+                proxy: Some(ProxyPolicy { http_addr: None }),
+            },
+            landlock: LandlockPolicy::default(),
+            process: ProcessPolicy::default(),
+        };
+
+        enrich_sandbox_baseline_paths(&mut policy);
+
+        assert!(
+            policy
+                .filesystem
+                .read_only
+                .contains(&std::path::PathBuf::from("/tmp")),
+            "explicit read_only baseline path should be preserved"
+        );
+        assert!(
+            !policy
+                .filesystem
+                .read_write
+                .contains(&std::path::PathBuf::from("/tmp")),
+            "baseline enrichment must not promote explicit read_only /tmp to read_write"
+        );
+    }
 }
 
 /// Load sandbox policy from local files or gRPC.