From 77eb482e4d0b63339bbc897492debae0dffdaedb Mon Sep 17 00:00:00 2001 From: Soham Das Date: Wed, 1 Jul 2026 13:04:00 -0700 Subject: [PATCH 1/3] Add filesystem-policy delegation check --- .../bubblewrap/common/src/bwrap_runner.rs | 7 + src/backends/lxc/common/src/lxc_runner.rs | 7 + .../wslc/common/src/wsl_container_runner.rs | 8 + src/core/wxc_common/src/filesystem_access.rs | 308 ++++++++++++++++++ src/core/wxc_common/src/lib.rs | 1 + 5 files changed, 331 insertions(+) create mode 100644 src/core/wxc_common/src/filesystem_access.rs diff --git a/src/backends/bubblewrap/common/src/bwrap_runner.rs b/src/backends/bubblewrap/common/src/bwrap_runner.rs index f428ef84..bf237616 100644 --- a/src/backends/bubblewrap/common/src/bwrap_runner.rs +++ b/src/backends/bubblewrap/common/src/bwrap_runner.rs @@ -120,6 +120,13 @@ impl SandboxBackend for BubblewrapScriptRunner { Ok(None) => request, Err(msg) => return Err(ScriptResponse::error(&msg)), }; + // Delegation check (D3): reject any policy path the invoking user cannot + // access, so the sandbox never gains access the caller lacks. Runs AFTER + // object normalization so it is evaluated against the already-tightened + // intents (a path moved rw -> denied must not then require write access). + if let Err(msg) = wxc_common::filesystem_access::check_delegation(&request.policy) { + return Err(ScriptResponse::error(&msg)); + } let child = self.spawn_bwrap(request, logger, stdio)?; Ok(Box::new(BubblewrapSandboxProcess::new(child))) } diff --git a/src/backends/lxc/common/src/lxc_runner.rs b/src/backends/lxc/common/src/lxc_runner.rs index afe73194..002ce544 100644 --- a/src/backends/lxc/common/src/lxc_runner.rs +++ b/src/backends/lxc/common/src/lxc_runner.rs @@ -109,6 +109,13 @@ impl LxcScriptRunner { Ok(None) => request, Err(msg) => return ScriptResponse::error(&msg), }; + // Delegation check (D3): reject any policy path the invoking user cannot + // access, so the sandbox never gains access the caller lacks. Runs AFTER + // object normalization so it is evaluated against the already-tightened + // intents. + if let Err(msg) = wxc_common::filesystem_access::check_delegation(&request.policy) { + return ScriptResponse::error(&msg); + } // Validate required LXC fields if self.config.distribution.is_empty() || self.config.release.is_empty() { diff --git a/src/backends/wslc/common/src/wsl_container_runner.rs b/src/backends/wslc/common/src/wsl_container_runner.rs index bb1ef41f..183aec52 100644 --- a/src/backends/wslc/common/src/wsl_container_runner.rs +++ b/src/backends/wslc/common/src/wsl_container_runner.rs @@ -878,6 +878,14 @@ impl WSLContainerRunner { Ok(None) => request, Err(msg) => return ScriptResponse::error(&msg), }; + // Delegation check (D3): reject any policy path the invoking user cannot + // access, so the sandbox never gains access the caller lacks. Runs AFTER + // object normalization so it is evaluated against the already-tightened + // intents. On Windows this covers directory readwrite paths (the common + // WSLC case). + if let Err(msg) = wxc_common::filesystem_access::check_delegation(&request.policy) { + return ScriptResponse::error(&msg); + } // -- Init: COM + SDK + preflight -- let sdk = match Self::init_and_load_sdk(logger) { diff --git a/src/core/wxc_common/src/filesystem_access.rs b/src/core/wxc_common/src/filesystem_access.rs new file mode 100644 index 00000000..a5bac8cf --- /dev/null +++ b/src/core/wxc_common/src/filesystem_access.rs @@ -0,0 +1,308 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +//! Filesystem-policy **delegation check** (roadmap item D3). +//! +//! Enforces that the sandbox is never granted more filesystem access than the +//! invoking user already holds: every `readwritePaths` entry requires the user +//! to have read+write access, and every `readonlyPaths` entry requires read +//! access. `deniedPaths` are unbounded (denying access needs no access) and are +//! not checked. A path the user cannot access is **rejected** so a sandboxed +//! process can't reach files the caller themselves couldn't. +//! +//! This does file I/O (`access(2)` / `CreateFileW`), so — like the object-based +//! normalization in [`crate::filesystem_object`] and per design review — it runs +//! in each backend runner **close to the point of enforcement**, NOT in +//! `config_parser` (which stays string-only). Two reasons: +//! +//! - **Correctness:** mount targets may not exist when the config is parsed +//! (they can be created between parse and launch), so a parse-time check would +//! skip them; checking just before the backend builds its mounts sees the real +//! filesystem state. +//! - **TOCTOU:** doing the check adjacent to enforcement shrinks the window in +//! which the filesystem can change between the check and the mount. +//! +//! When both this and object normalization are wired into a runner, +//! [`crate::filesystem_object::normalize_object_conflicts`] must run **first**, +//! so delegation is checked against the already-tightened intents (a path moved +//! `rw → denied` must not then be required to have write access). + +use crate::models::ContainerPolicy; + +/// The access the invoking user must hold to delegate a path to the sandbox. +#[derive(Clone, Copy, PartialEq, Eq, Debug)] +enum AccessMode { + /// Read access only (for `readonlyPaths`). + Read, + /// Read and write access (for `readwritePaths`). + ReadWrite, +} + +impl AccessMode { + /// The JSON list name this mode maps to, and the access phrase, for the + /// rejection message. + fn list_name(self) -> &'static str { + match self { + AccessMode::Read => "readonlyPaths", + AccessMode::ReadWrite => "readwritePaths", + } + } + + fn access_phrase(self) -> &'static str { + match self { + AccessMode::Read => "read", + AccessMode::ReadWrite => "read+write", + } + } +} + +/// Checks whether the invoking user holds the requested access to `path`. +/// +/// Returns `Some(true)` / `Some(false)` when the result is determinable, or +/// `None` when it cannot be determined (e.g. the path does not exist — that case +/// is surfaced separately by the existence warning, so delegation skips it). +/// +/// On Unix this uses `access(2)` against the real UID (the invoking user), which +/// covers both files and directories — fully implementing spec D3 for the Linux +/// backends (LXC, Bubblewrap). +/// +/// On Windows it probes the caller's effective access with `CreateFileW` +/// (requesting `GENERIC_READ` / `GENERIC_READ | GENERIC_WRITE`, opened with +/// `FILE_FLAG_BACKUP_SEMANTICS` so directories are covered too). A successful +/// open means the access is granted; an `ERROR_ACCESS_DENIED` failure means it +/// is not; any other failure is treated as undeterminable (`None`) and skipped. +/// This covers files *and* directories — including the common WSLC +/// `readwritePaths` directory case — implementing spec D3 for the WSLC backend. +#[cfg(unix)] +fn user_can_access(path: &str, mode: AccessMode) -> Option { + use std::ffi::CString; + + if std::fs::metadata(path).is_err() { + return None; + } + let c_path = CString::new(path).ok()?; + let mask = match mode { + AccessMode::Read => libc::R_OK, + AccessMode::ReadWrite => libc::R_OK | libc::W_OK, + }; + // SAFETY: `c_path` is a valid NUL-terminated C string for the duration of the call. + let rc = unsafe { libc::access(c_path.as_ptr(), mask) }; + Some(rc == 0) +} + +#[cfg(windows)] +fn user_can_access(path: &str, mode: AccessMode) -> Option { + use windows::core::PCWSTR; + use windows::Win32::Foundation::{ + CloseHandle, GetLastError, ERROR_ACCESS_DENIED, GENERIC_READ, GENERIC_WRITE, + }; + use windows::Win32::Storage::FileSystem::{ + CreateFileW, FILE_FLAG_BACKUP_SEMANTICS, FILE_SHARE_DELETE, FILE_SHARE_READ, + FILE_SHARE_WRITE, OPEN_EXISTING, + }; + + // Probe the caller's effective access by asking the OS to open the object + // with the required rights. FILE_FLAG_BACKUP_SEMANTICS lets the same call + // open directories as well as files, so — unlike a plain `File::open` — this + // covers directory WRITE access (mapped by the OS to FILE_ADD_FILE / + // FILE_ADD_SUBDIRECTORY), which is the common WSLC `readwritePaths` case. + let wide: Vec = path.encode_utf16().chain(std::iter::once(0)).collect(); + let desired = match mode { + AccessMode::Read => GENERIC_READ.0, + AccessMode::ReadWrite => GENERIC_READ.0 | GENERIC_WRITE.0, + }; + let share = FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE; + + // SAFETY: `wide` is a local NUL-terminated buffer; all other pointers are NULL. + let handle = unsafe { + CreateFileW( + PCWSTR(wide.as_ptr()), + desired, + share, + None, + OPEN_EXISTING, + FILE_FLAG_BACKUP_SEMANTICS, + None, + ) + }; + match handle { + Ok(h) if !h.is_invalid() => { + // SAFETY: `h` is a valid handle returned by CreateFileW. + unsafe { + let _ = CloseHandle(h); + } + Some(true) + } + _ => { + // Only an explicit access denial is a delegation failure. Any other + // error (non-existent path — surfaced separately by the existence + // warning — sharing violation, etc.) is undeterminable and skipped + // rather than rejected, matching the Unix `None`-on-missing behavior. + // SAFETY: reads the thread-local last error set by the failed call above. + let err = unsafe { GetLastError() }; + if err == ERROR_ACCESS_DENIED { + Some(false) + } else { + None + } + } + } +} + +#[cfg(not(any(unix, windows)))] +fn user_can_access(_path: &str, _mode: AccessMode) -> Option { + None +} + +/// Validates the delegation constraint (spec D3): the sandbox receives no more +/// access than the invoking user holds. `readwritePaths` require the user to +/// have read+write access and `readonlyPaths` require read access; `deniedPaths` +/// are unbounded and not checked. Paths whose access cannot be determined (e.g. +/// non-existent paths) are skipped rather than rejected. +/// +/// Returns the rejection message for the first path that fails, or `Ok(())` when +/// every checkable path is within the caller's access. Callers surface the +/// message as their backend-appropriate error (e.g. `ScriptResponse::error`). +pub fn check_delegation(policy: &ContainerPolicy) -> Result<(), String> { + for (paths, mode) in [ + (&policy.readonly_paths, AccessMode::Read), + (&policy.readwrite_paths, AccessMode::ReadWrite), + ] { + for path in paths { + if user_can_access(path, mode) == Some(false) { + return Err(format!( + "Filesystem path '{}' ({}): the invoking user does not have {} access, \ + so it cannot be delegated to the sandbox", + path, + mode.list_name(), + mode.access_phrase(), + )); + } + } + } + Ok(()) +} + +#[cfg(test)] +mod tests { + use super::*; + + fn policy(rw: &[&str], ro: &[&str]) -> ContainerPolicy { + ContainerPolicy { + readwrite_paths: rw.iter().map(|s| s.to_string()).collect(), + readonly_paths: ro.iter().map(|s| s.to_string()).collect(), + ..Default::default() + } + } + + #[test] + fn accessible_file_is_delegable() { + let dir = tempfile::tempdir().unwrap(); + let file = dir.path().join("data.txt"); + std::fs::write(&file, b"test").unwrap(); + let f = file.to_str().unwrap(); + + assert_eq!(user_can_access(f, AccessMode::Read), Some(true)); + assert_eq!(user_can_access(f, AccessMode::ReadWrite), Some(true)); + assert!(check_delegation(&policy(&[f], &[])).is_ok()); + } + + #[test] + fn accessible_directory_is_delegable() { + // Directory read+write is the common WSLC `readwritePaths` case; it must + // be enforced on both Unix and Windows. + let dir = tempfile::tempdir().unwrap(); + let d = dir.path().to_str().unwrap(); + + assert_eq!(user_can_access(d, AccessMode::Read), Some(true)); + assert_eq!(user_can_access(d, AccessMode::ReadWrite), Some(true)); + assert!(check_delegation(&policy(&[d], &[])).is_ok()); + } + + #[test] + fn nonexistent_path_is_skipped() { + // A non-existent path can't be access-checked; delegation skips it + // (existence is surfaced separately as a warning, not a delegation error). + assert_eq!( + user_can_access("/definitely/not/here/mxc-xyz", AccessMode::Read), + None + ); + assert!(check_delegation(&policy(&["/definitely/not/here/mxc-xyz"], &[])).is_ok()); + } + + #[test] + fn empty_policy_is_ok() { + assert!(check_delegation(&ContainerPolicy::default()).is_ok()); + } + + #[cfg(unix)] + #[test] + fn unreadable_path_is_rejected() { + use std::os::unix::fs::PermissionsExt; + + // Root bypasses permission checks, so this case is only meaningful as a + // non-root user. + if unsafe { libc::geteuid() } == 0 { + return; + } + + let dir = tempfile::tempdir().unwrap(); + let file = dir.path().join("secret.txt"); + std::fs::write(&file, b"secret").unwrap(); + // Remove all permissions so the invoking user cannot read it. + std::fs::set_permissions(&file, std::fs::Permissions::from_mode(0o000)).unwrap(); + let f = file.to_str().unwrap(); + + assert_eq!(user_can_access(f, AccessMode::Read), Some(false)); + let err = check_delegation(&policy(&[], &[f])).unwrap_err(); + assert!( + err.contains("does not have read access"), + "expected delegation rejection, got: {err}" + ); + + // Restore permissions so the tempdir can be cleaned up. + let _ = std::fs::set_permissions(&file, std::fs::Permissions::from_mode(0o600)); + } + + #[cfg(windows)] + #[test] + fn unreadable_path_is_rejected() { + use std::process::Command; + + let dir = tempfile::tempdir().unwrap(); + let file = dir.path().join("secret.txt"); + std::fs::write(&file, b"secret").unwrap(); + let f = file.to_str().unwrap(); + + // Deny read to Everyone (well-known SID S-1-1-0 — locale/domain + // independent). A deny ACE blocks FILE_READ_DATA even for the owner + // (whose implicit rights are only READ_CONTROL / WRITE_DAC), so a + // GENERIC_READ open must fail with ERROR_ACCESS_DENIED. Parent-dir full + // control still lets the tempdir delete the child on cleanup. + let status = Command::new("icacls") + .args([f, "/deny", "*S-1-1-0:(R)"]) + .output() + .expect("icacls should run"); + assert!( + status.status.success(), + "icacls deny failed: {}", + String::from_utf8_lossy(&status.stderr) + ); + + assert_eq!( + user_can_access(f, AccessMode::Read), + Some(false), + "a path with an explicit deny-read ACE must be reported as inaccessible" + ); + let err = check_delegation(&policy(&[], &[f])).unwrap_err(); + assert!( + err.contains("does not have read access"), + "expected delegation rejection, got: {err}" + ); + + // Remove the deny ACE so the tempdir can clean up without surprises. + let _ = Command::new("icacls") + .args([f, "/remove:d", "*S-1-1-0"]) + .output(); + } +} diff --git a/src/core/wxc_common/src/lib.rs b/src/core/wxc_common/src/lib.rs index cbb6382d..7ffce801 100644 --- a/src/core/wxc_common/src/lib.rs +++ b/src/core/wxc_common/src/lib.rs @@ -7,6 +7,7 @@ pub mod cmdline; pub mod config_parser; pub mod encoding; pub mod error; +pub mod filesystem_access; pub mod filesystem_object; pub mod id; pub mod log_symbols; From e1623d1e382b076989a2922c0a68ed47e4df1a0b Mon Sep 17 00:00:00 2001 From: Soham Das Date: Wed, 1 Jul 2026 14:07:02 -0700 Subject: [PATCH 2/3] Addressed PR comments --- src/core/wxc_common/src/filesystem_access.rs | 77 ++++++++++++++++---- 1 file changed, 64 insertions(+), 13 deletions(-) diff --git a/src/core/wxc_common/src/filesystem_access.rs b/src/core/wxc_common/src/filesystem_access.rs index a5bac8cf..781af0d7 100644 --- a/src/core/wxc_common/src/filesystem_access.rs +++ b/src/core/wxc_common/src/filesystem_access.rs @@ -10,8 +10,7 @@ //! not checked. A path the user cannot access is **rejected** so a sandboxed //! process can't reach files the caller themselves couldn't. //! -//! This does file I/O (`access(2)` / `CreateFileW`), so — like the object-based -//! normalization in [`crate::filesystem_object`] and per design review — it runs +//! This does file I/O (`access(2)` / `CreateFileW`), so it runs //! in each backend runner **close to the point of enforcement**, NOT in //! `config_parser` (which stays string-only). Two reasons: //! @@ -22,10 +21,10 @@ //! - **TOCTOU:** doing the check adjacent to enforcement shrinks the window in //! which the filesystem can change between the check and the mount. //! -//! When both this and object normalization are wired into a runner, -//! [`crate::filesystem_object::normalize_object_conflicts`] must run **first**, -//! so delegation is checked against the already-tightened intents (a path moved -//! `rw → denied` must not then be required to have write access). +//! When object-based filesystem normalization is also wired into a runner +//! (tightening conflicting path intents to the most restrictive), it must run +//! **first**, so delegation is checked against the already-tightened intents (a +//! path moved `rw → denied` must not then be required to have write access). use crate::models::ContainerPolicy; @@ -62,9 +61,18 @@ impl AccessMode { /// `None` when it cannot be determined (e.g. the path does not exist — that case /// is surfaced separately by the existence warning, so delegation skips it). /// -/// On Unix this uses `access(2)` against the real UID (the invoking user), which -/// covers both files and directories — fully implementing spec D3 for the Linux -/// backends (LXC, Bubblewrap). +/// On Unix this uses `access(2)`, which checks the requested permissions +/// against the process's **real** UID/GID (the invoking user) and covers both +/// files and directories. This has full effect for **Bubblewrap**, which runs +/// unprivileged. +/// +/// Caveat — **root**: `access(2)` short-circuits for the superuser, so `R_OK` +/// (and, on most filesystems, `W_OK`) always succeed for a real UID of 0 +/// regardless of the file mode. The **LXC** backend runs as root, so the +/// delegation check is effectively a no-op there; it only has teeth for an +/// unprivileged caller (Bubblewrap). Meaningfully enforcing delegation under a +/// root LXC would require checking against the pre-elevation user (e.g. +/// `SUDO_UID`/`SUDO_GID`) — tracked as a follow-up. /// /// On Windows it probes the caller's effective access with `CreateFileW` /// (requesting `GENERIC_READ` / `GENERIC_READ | GENERIC_WRITE`, opened with @@ -77,9 +85,6 @@ impl AccessMode { fn user_can_access(path: &str, mode: AccessMode) -> Option { use std::ffi::CString; - if std::fs::metadata(path).is_err() { - return None; - } let c_path = CString::new(path).ok()?; let mask = match mode { AccessMode::Read => libc::R_OK, @@ -87,7 +92,18 @@ fn user_can_access(path: &str, mode: AccessMode) -> Option { }; // SAFETY: `c_path` is a valid NUL-terminated C string for the duration of the call. let rc = unsafe { libc::access(c_path.as_ptr(), mask) }; - Some(rc == 0) + if rc == 0 { + return Some(true); + } + // access(2) failed. Distinguish genuine non-existence — which is skipped + // here and surfaced separately by the existence warning — from an access + // denial, which is the exact case delegation must catch (including a parent + // directory the caller cannot traverse, reported as EACCES). Only a missing + // path yields `None`; every other errno is treated as "not accessible". + match std::io::Error::last_os_error().raw_os_error() { + Some(libc::ENOENT) | Some(libc::ENOTDIR) => None, + _ => Some(false), + } } #[cfg(windows)] @@ -264,6 +280,41 @@ mod tests { let _ = std::fs::set_permissions(&file, std::fs::Permissions::from_mode(0o600)); } + #[cfg(unix)] + #[test] + fn untraversable_parent_is_rejected_not_skipped() { + use std::os::unix::fs::PermissionsExt; + + // Root bypasses permission checks, so this case is only meaningful as a + // non-root user. + if unsafe { libc::geteuid() } == 0 { + return; + } + + // A readable file inside a directory the caller cannot traverse. The file + // exists, but reaching it requires execute (traversal) on the parent — + // which we remove. access(2) then reports EACCES, and the check must + // treat that as "not accessible" (a real delegation failure), NOT skip it + // as if the path were missing. + let dir = tempfile::tempdir().unwrap(); + let locked = dir.path().join("locked"); + std::fs::create_dir(&locked).unwrap(); + let file = locked.join("data.txt"); + std::fs::write(&file, b"data").unwrap(); + std::fs::set_permissions(&locked, std::fs::Permissions::from_mode(0o000)).unwrap(); + let f = file.to_str().unwrap(); + + assert_eq!( + user_can_access(f, AccessMode::Read), + Some(false), + "an untraversable parent (EACCES) must be reported as inaccessible, not skipped" + ); + assert!(check_delegation(&policy(&[], &[f])).is_err()); + + // Restore traversal so the tempdir can recurse in during cleanup. + let _ = std::fs::set_permissions(&locked, std::fs::Permissions::from_mode(0o755)); + } + #[cfg(windows)] #[test] fn unreadable_path_is_rejected() { From 761cbec06179e5a4e9fdfb14d3d00a7471651b71 Mon Sep 17 00:00:00 2001 From: Soham Das Date: Thu, 2 Jul 2026 16:34:08 -0700 Subject: [PATCH 3/3] Align Windows delegation check to Unix fail-closed posture MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The delegation access-probe used asymmetric error handling across platforms: Unix skipped only genuine-missing paths and rejected everything else (fail-closed), while Windows rejected only ERROR_ACCESS_DENIED and skipped every other failure (fail-open). A sharing/lock violation or transient I/O error on a real, existing file was therefore silently allowed on Windows but rejected on Unix — the opposite posture for a security delegation gate. Align Windows to the Unix posture: skip only ERROR_FILE_NOT_FOUND / ERROR_PATH_NOT_FOUND (surfaced separately by the existence warning); treat every other failure, including access-denied and ambiguous errors, as 'cannot confirm access' and reject. Add a Windows test locking the fail-closed behavior for an ambiguous (ERROR_INVALID_NAME) error. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/core/wxc_common/src/filesystem_access.rs | 42 +++++++++++++++----- 1 file changed, 31 insertions(+), 11 deletions(-) diff --git a/src/core/wxc_common/src/filesystem_access.rs b/src/core/wxc_common/src/filesystem_access.rs index 781af0d7..57444902 100644 --- a/src/core/wxc_common/src/filesystem_access.rs +++ b/src/core/wxc_common/src/filesystem_access.rs @@ -77,9 +77,13 @@ impl AccessMode { /// On Windows it probes the caller's effective access with `CreateFileW` /// (requesting `GENERIC_READ` / `GENERIC_READ | GENERIC_WRITE`, opened with /// `FILE_FLAG_BACKUP_SEMANTICS` so directories are covered too). A successful -/// open means the access is granted; an `ERROR_ACCESS_DENIED` failure means it -/// is not; any other failure is treated as undeterminable (`None`) and skipped. -/// This covers files *and* directories — including the common WSLC +/// open means the access is granted. Matching the Unix posture, the check +/// **fails closed**: only a genuinely missing path (`ERROR_FILE_NOT_FOUND` / +/// `ERROR_PATH_NOT_FOUND` — surfaced separately by the existence warning) is +/// skipped (`None`); every other failure, including `ERROR_ACCESS_DENIED` and +/// ambiguous errors (sharing/lock violation, transient I/O), is treated as +/// "cannot confirm access" and rejected (`Some(false)`) rather than silently +/// allowed. This covers files *and* directories — including the common WSLC /// `readwritePaths` directory case — implementing spec D3 for the WSLC backend. #[cfg(unix)] fn user_can_access(path: &str, mode: AccessMode) -> Option { @@ -110,7 +114,8 @@ fn user_can_access(path: &str, mode: AccessMode) -> Option { fn user_can_access(path: &str, mode: AccessMode) -> Option { use windows::core::PCWSTR; use windows::Win32::Foundation::{ - CloseHandle, GetLastError, ERROR_ACCESS_DENIED, GENERIC_READ, GENERIC_WRITE, + CloseHandle, GetLastError, ERROR_FILE_NOT_FOUND, ERROR_PATH_NOT_FOUND, GENERIC_READ, + GENERIC_WRITE, }; use windows::Win32::Storage::FileSystem::{ CreateFileW, FILE_FLAG_BACKUP_SEMANTICS, FILE_SHARE_DELETE, FILE_SHARE_READ, @@ -150,16 +155,17 @@ fn user_can_access(path: &str, mode: AccessMode) -> Option { Some(true) } _ => { - // Only an explicit access denial is a delegation failure. Any other - // error (non-existent path — surfaced separately by the existence - // warning — sharing violation, etc.) is undeterminable and skipped - // rather than rejected, matching the Unix `None`-on-missing behavior. + // Fail closed, mirroring the Unix posture: skip ONLY a genuinely + // missing path (surfaced separately by the existence warning). Every + // other failure — access denied, sharing/lock violation, transient + // I/O — means we cannot confirm the caller has the access, which for + // a security delegation gate must reject rather than silently allow. // SAFETY: reads the thread-local last error set by the failed call above. let err = unsafe { GetLastError() }; - if err == ERROR_ACCESS_DENIED { - Some(false) - } else { + if err == ERROR_FILE_NOT_FOUND || err == ERROR_PATH_NOT_FOUND { None + } else { + Some(false) } } } @@ -251,6 +257,20 @@ mod tests { assert!(check_delegation(&ContainerPolicy::default()).is_ok()); } + #[cfg(windows)] + #[test] + fn ambiguous_error_fails_closed() { + // A path Windows rejects for a reason OTHER than not-found or + // access-denied (here an invalid name containing an illegal `<` + // character → ERROR_INVALID_NAME) must fail closed — `Some(false)` → + // rejected — not be silently skipped. This locks the Unix/Windows + // posture alignment: only a genuinely-missing path is skipped; every + // other failure is treated as "cannot confirm access" and rejected. + let invalid = "C:\\mxc_invalid