Mask denied files with /dev/null bind instead of tmpfs, add new test configs

Author: SohamDas2021

src/backends/bubblewrap/common/src/bwrap_command.rs

@@ -7,6 +7,8 @@
 //! arguments without spawning any processes, so it compiles and can be
 //! unit-tested on every host (Windows, macOS, Linux).
 
+use std::collections::HashSet;
+
 use wxc_common::filesystem_resolve::FsIntent;
 use wxc_common::models::{ExecutionRequest, NetworkPolicy, ProxyAddress};
 
@@ -97,6 +99,19 @@ const BASELINE_RO_BIND_PATHS: &[&str] = &[
     "/mnt/wsl/resolv.conf",
 ];
 
+/// Build the complete `bwrap` argument list, masking **every** denied path as a
+/// directory (`--tmpfs`).
+///
+/// This is the pure, classification-free entry point: it performs no filesystem
+/// I/O and so stays unit-testable on every host. Unit tests and any caller that
+/// has not stat'd the denied paths use it. The Bubblewrap runner uses
+/// [`build_args_classified`] instead, passing the set of denied paths it has
+/// determined to be files so those are masked with `--ro-bind /dev/null` (a
+/// `--tmpfs` over a file would replace it with an empty directory).
+pub fn build_args(request: &ExecutionRequest, proxy_address: Option<&ProxyAddress>) -> Vec<String> {
+    build_args_classified(request, proxy_address, &HashSet::new())
+}
+
 /// Build the complete argument list for `bwrap` from the given request.
 ///
 /// The returned vector does **not** include the `bwrap` binary name itself —
@@ -110,7 +125,18 @@ const BASELINE_RO_BIND_PATHS: &[&str] = &[
 /// - strips any caller-supplied `HTTP_PROXY` / `HTTPS_PROXY` / `NO_PROXY`
 ///   entries from `request.env`,
 /// - emits `--setenv` for those keys pointing at the proxy URL.
-pub fn build_args(request: &ExecutionRequest, proxy_address: Option<&ProxyAddress>) -> Vec<String> {
+///
+/// `denied_files` is the set of `deniedPaths` entries that must be masked as
+/// **files** (`--ro-bind /dev/null <path>`) rather than **directories**
+/// (`--tmpfs <path>`); any denied path not in the set is masked as a directory.
+/// The runner builds this set by `symlink_metadata`-probing each denied path, so
+/// this function itself performs no filesystem I/O and remains unit-testable on
+/// every host.
+pub fn build_args_classified(
+    request: &ExecutionRequest,
+    proxy_address: Option<&ProxyAddress>,
+    denied_files: &HashSet<String>,
+) -> Vec<String> {
     // -- Namespace isolation (all unshared by default) ---------------------
     let mut args = vec![
         "--unshare-user",
@@ -182,9 +208,17 @@ pub fn build_args(request: &ExecutionRequest, proxy_address: Option<&ProxyAddres
             FsIntent::ReadOnly => {
                 args.extend(["--ro-bind".into(), mount.path.clone(), mount.path.clone()]);
             }
-            // Denied: mask with an empty tmpfs so contents are invisible.
+            // Denied: mask so contents are invisible. A directory is masked
+            // with an empty `--tmpfs`; a file must be masked with
+            // `--ro-bind /dev/null` instead, because `--tmpfs` over a file would
+            // replace it with an empty *directory* (wrong type). `denied_files`
+            // holds the paths the runner classified as non-directories.
             FsIntent::Denied => {
-                args.extend(["--tmpfs".into(), mount.path.clone()]);
+                if denied_files.contains(&mount.path) {
+                    args.extend(["--ro-bind".into(), "/dev/null".into(), mount.path.clone()]);
+                } else {
+                    args.extend(["--tmpfs".into(), mount.path.clone()]);
+                }
             }
         }
     }
@@ -374,6 +408,74 @@ mod tests {
         );
     }
 
+    /// A denied path classified as a **directory** (not in `denied_files`) is
+    /// masked with an empty `--tmpfs`, matching the default `build_args`.
+    #[test]
+    fn denied_directory_is_masked_with_tmpfs() {
+        let mut r = base_request();
+        r.policy.denied_paths = vec!["/secrets".into()];
+        let denied_files = HashSet::new();
+        let args = build_args_classified(&r, None, &denied_files);
+
+        // tmpfs at /secrets, and no ro-bind of /dev/null onto it.
+        policy_mount_pos(&args, "--tmpfs", "/secrets");
+        assert!(
+            args.windows(3)
+                .all(|w| !(w[0] == "--ro-bind" && w[1] == "/dev/null" && w[2] == "/secrets")),
+            "a directory denied path must not be masked with /dev/null: {args:?}"
+        );
+    }
+
+    /// A denied path classified as a **file** (present in `denied_files`) is
+    /// masked with `--ro-bind /dev/null`, not `--tmpfs` (which would replace the
+    /// file with an empty directory).
+    #[test]
+    fn denied_file_is_masked_with_dev_null() {
+        let mut r = base_request();
+        r.policy.denied_paths = vec!["/etc/shadow".into()];
+        let denied_files = HashSet::from(["/etc/shadow".to_string()]);
+        let args = build_args_classified(&r, None, &denied_files);
+
+        // `--ro-bind /dev/null /etc/shadow` present ...
+        let pos = args
+            .windows(3)
+            .position(|w| w[0] == "--ro-bind" && w[1] == "/dev/null" && w[2] == "/etc/shadow");
+        assert!(
+            pos.is_some(),
+            "a file denied path must be masked with `--ro-bind /dev/null`: {args:?}"
+        );
+        // ... and it is NOT tmpfs-masked.
+        assert!(
+            args.windows(2)
+                .all(|w| !(w[0] == "--tmpfs" && w[1] == "/etc/shadow")),
+            "a file denied path must not be tmpfs-masked: {args:?}"
+        );
+    }
+
+    /// Classification is per-path: in one policy a denied file and a denied
+    /// directory get their respective masks, and the specificity ordering is
+    /// still honored (deep file mask emitted after its shallower rw ancestor).
+    #[test]
+    fn mixed_denied_file_and_dir_masks_each_correctly() {
+        let mut r = base_request();
+        r.policy.readwrite_paths = vec!["/data".into()];
+        r.policy.denied_paths = vec!["/data/secret.txt".into(), "/cache".into()];
+        let denied_files = HashSet::from(["/data/secret.txt".to_string()]);
+        let args = build_args_classified(&r, None, &denied_files);
+
+        // File → /dev/null, after the rw /data parent.
+        let parent = policy_mount_pos(&args, "--bind", "/data");
+        let file_mask = policy_mount_pos(&args, "--ro-bind", "/data/secret.txt");
+        assert!(
+            file_mask > parent,
+            "deep denied file mask (pos {file_mask}) must come after rw /data (pos {parent}): {args:?}"
+        );
+        assert_eq!(args[file_mask + 1], "/dev/null");
+
+        // Dir → tmpfs.
+        policy_mount_pos(&args, "--tmpfs", "/cache");
+    }
+
     #[test]
     fn environment_variables_are_set() {
         let mut r = base_request();

src/backends/bubblewrap/common/src/bwrap_runner.rs

@@ -25,6 +25,7 @@
 //! the runner uses `--unshare-net` for zero-overhead full isolation
 //! without root.
 
+use std::collections::HashSet;
 use std::fmt::Write as FmtWrite;
 use std::os::unix::process::CommandExt;
 use std::process::{Child, ChildStdin, Command, Stdio};
@@ -164,15 +165,35 @@ impl BubblewrapScriptRunner {
             }
         }
 
-        // 2. Build the bwrap argument vector.
-        let args = bwrap_command::build_args(request, proxy.address());
+        // 2. Classify denied paths so files are masked correctly. A `--tmpfs`
+        //    over a file would replace it with an empty directory; denied files
+        //    must instead be masked with `--ro-bind /dev/null`. Classify with
+        //    `symlink_metadata`, which does not follow symlinks, so each path is
+        //    judged by its own type: a denied symlink is therefore treated as a
+        //    non-directory and masked with `/dev/null`, rather than following
+        //    the link to classify (and mask) its target. Directories, and paths
+        //    that cannot be stat'd (missing/unreadable), fall back to `--tmpfs`.
+        let denied_files: HashSet<String> = request
+            .policy
+            .denied_paths
+            .iter()
+            .filter(|p| {
+                std::fs::symlink_metadata(p)
+                    .map(|md| !md.file_type().is_dir())
+                    .unwrap_or(false)
+            })
+            .cloned()
+            .collect();
+
+        // 3. Build the bwrap argument vector.
+        let args = bwrap_command::build_args_classified(request, proxy.address(), &denied_files);
         let _ = writeln!(
             logger,
             "Bubblewrap: spawning bwrap with {} args",
             args.len()
         );
 
-        // 3. Determine whether iptables network rules are needed. When the
+        // 4. Determine whether iptables network rules are needed. When the
         //    cooperative proxy is active we skip iptables entirely (host
         //    enforcement happens at the proxy layer).
         let needs_iptables = needs_iptables_rules(request) && !proxy.is_active();
@@ -209,7 +230,7 @@ impl BubblewrapScriptRunner {
             None
         };
 
-        // 4. Spawn `bwrap`.
+        // 5. Spawn `bwrap`.
         let mut command = Command::new("bwrap");
         command.args(&args);
         match stdio {

tests/configs/bubblewrap_denied_masking.json

@@ -0,0 +1,12 @@
+{
+  "version": "0.8.0-alpha",
+  "containerId": "CLI-Bubblewrap-Denied-Masking",
+  "containment": "bubblewrap",
+  "process": {
+    "commandLine": "B=/mnt/masktest; if cat \"$B/visible.txt\" 2>/dev/null | grep -q VISIBLE_SECRET; then echo VISIBLE_OK; else echo VISIBLE_MISSING; fi; if cat \"$B/secret_file.txt\" 2>/dev/null | grep -q FILE_SECRET; then echo FILE_LEAK; else echo FILE_MASKED_OK; fi; if [ -d \"$B/secret_file.txt\" ]; then echo FILE_IS_DIR_BUG; else echo FILE_NOT_DIR_OK; fi; if ls \"$B/secret_dir\" 2>/dev/null | grep -q .; then echo DIR_LEAK; else echo DIR_MASKED_OK; fi"
+  },
+  "filesystem": {
+    "readwritePaths": ["/mnt/masktest"],
+    "deniedPaths": ["/mnt/masktest/secret_file.txt", "/mnt/masktest/secret_dir"]
+  }
+}

tests/scripts/run_bwrap_all_tests.sh

@@ -37,6 +37,7 @@ run_test "Basic Bubblewrap" "$SCRIPT_DIR/run_bwrap_basic_test.sh"
 run_test "Bubblewrap Filesystem" "$SCRIPT_DIR/run_bwrap_filesystem_test.sh"
 run_test "Bubblewrap Object Validation" "$SCRIPT_DIR/run_bwrap_filesystem_object_test.sh"
 run_test "Bubblewrap Most-Specific Path" "$SCRIPT_DIR/run_bwrap_most_specific_test.sh"
+run_test "Bubblewrap Denied Masking" "$SCRIPT_DIR/run_bwrap_denied_masking_test.sh"
 run_test "Bubblewrap Network Block" "$SCRIPT_DIR/run_bwrap_network_test.sh"
 run_test "Bubblewrap Network Proxy" "$SCRIPT_DIR/run_bwrap_network_proxy_test.sh"
 run_test "Linux Process Default" "$SCRIPT_DIR/run_linux_process_default_test.sh"

tests/scripts/run_bwrap_denied_masking_test.sh

@@ -0,0 +1,105 @@
+#!/bin/bash
+# Bubblewrap denied-path masking test (roadmap item #1).
+#
+# Verifies that a denied FILE and a denied DIRECTORY are each masked with the
+# correct primitive:
+#   - A denied directory is masked with a tmpfs (empty, no leak).
+#   - A denied *file* is masked with a read-only bind of /dev/null. Masking a
+#     file with tmpfs would turn it into a DIRECTORY (bwrap creates the mount
+#     point as a dir), which both changes its type and can break tools that
+#     expect a regular file. Binding /dev/null keeps it a non-directory whose
+#     content is empty.
+#
+# Both fixtures hold secret content that is readable on the host, so any leak
+# inside the sandbox is attributable to the policy, not a broken fixture. A
+# non-denied sibling (visible.txt) inside the same read-write parent acts as a
+# positive control: it MUST be readable in the sandbox, proving the parent
+# mount is really present so that masking of the denied entries is attributable
+# to the deny policy rather than to the tree simply being absent.
+set -euo pipefail
+
+SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
+REPO_DIR="$(dirname "$(dirname "$SCRIPT_DIR")")"
+LXC_EXEC="$REPO_DIR/src/target/release/lxc-exec"
+
+if [ ! -f "$LXC_EXEC" ]; then
+    LXC_EXEC="$REPO_DIR/src/target/debug/lxc-exec"
+fi
+
+if [ ! -f "$LXC_EXEC" ]; then
+    echo "Error: lxc-exec not found. Run build.sh first."
+    exit 1
+fi
+
+BASE="/mnt/masktest"
+VISIBLE="$BASE/visible.txt"
+FILE="$BASE/secret_file.txt"
+DIR="$BASE/secret_dir"
+
+cleanup() { sudo rm -rf "$BASE"; }
+trap cleanup EXIT
+
+# Set up: a readable sibling (positive control), a denied secret file, and a
+# denied directory holding a secret file.
+sudo rm -rf "$BASE"
+sudo mkdir -p "$DIR"
+echo "VISIBLE_SECRET" | sudo tee "$VISIBLE" > /dev/null
+echo "FILE_SECRET" | sudo tee "$FILE" > /dev/null
+echo "DIR_SECRET" | sudo tee "$DIR/inner.txt" > /dev/null
+
+# Sanity: all fixtures are readable on the host.
+if ! sudo cat "$VISIBLE" | grep -q "VISIBLE_SECRET"; then
+    echo "FAIL: fixture setup — visible.txt not readable on host."
+    exit 1
+fi
+if ! sudo cat "$FILE" | grep -q "FILE_SECRET"; then
+    echo "FAIL: fixture setup — secret_file.txt not readable on host."
+    exit 1
+fi
+if ! sudo cat "$DIR/inner.txt" | grep -q "DIR_SECRET"; then
+    echo "FAIL: fixture setup — secret_dir/inner.txt not readable on host."
+    exit 1
+fi
+
+echo "Running Bubblewrap denied-path masking test (denied file + denied dir)..."
+OUTPUT=$("$LXC_EXEC" --experimental \
+    "$REPO_DIR/tests/configs/bubblewrap_denied_masking.json" 2>&1 || true)
+echo "$OUTPUT"
+
+FAIL=0
+
+# Positive control: the non-denied sibling must be readable, proving the parent
+# mount is present (so masking below is attributable to the deny, not absence).
+if echo "$OUTPUT" | grep -q "VISIBLE_OK" && ! echo "$OUTPUT" | grep -q "VISIBLE_MISSING"; then
+    echo "PASS: non-denied sibling readable (parent mount present)."
+else
+    echo "FAIL: non-denied sibling not readable — parent mount missing, test inconclusive."
+    FAIL=1
+fi
+
+if echo "$OUTPUT" | grep -q "FILE_MASKED_OK" && ! echo "$OUTPUT" | grep -q "FILE_LEAK"; then
+    echo "PASS: denied file content masked."
+else
+    echo "FAIL: denied file content leaked."
+    FAIL=1
+fi
+
+if echo "$OUTPUT" | grep -q "FILE_NOT_DIR_OK" && ! echo "$OUTPUT" | grep -q "FILE_IS_DIR_BUG"; then
+    echo "PASS: denied file kept as non-directory (masked with /dev/null, not tmpfs)."
+else
+    echo "FAIL: denied file became a directory (tmpfs-over-file bug)."
+    FAIL=1
+fi
+
+if echo "$OUTPUT" | grep -q "DIR_MASKED_OK" && ! echo "$OUTPUT" | grep -q "DIR_LEAK"; then
+    echo "PASS: denied directory masked empty."
+else
+    echo "FAIL: denied directory content leaked."
+    FAIL=1
+fi
+
+if [ "$FAIL" -ne 0 ]; then
+    exit 1
+fi
+
+echo "Bubblewrap denied-path masking test complete."