Align deny-read matching with executor path semantics (#41209)

## Why

Read-deny policies must use the target executor's path convention so URI-based
policy checks and native filesystem enumeration enforce the same rules.

## What changed

- Prepare deny roots and glob matchers from `PathUri` policy context, including
  executor-relative working directories and home-relative patterns.
- Match Windows globs case-insensitively with normalized separators, while
  preserving byte-oriented POSIX matching for non-UTF-8 paths.
- Fail closed for malformed paths, incompatible path conventions, unresolved
  home-relative patterns, and invalid globs.
- Make Windows deny-read discovery use case-insensitive ripgrep glob matching.

## Testing

Added coverage for Windows URI conventions, executor home expansion,
case-insensitive `.env` discovery, malformed paths, non-UTF-8 names, and
canonical directory-link targets.

GitOrigin-RevId: 36001219a2e9b36acfce8972dc1d3bbc304271c5

codex-rs/protocol/src/permissions.rs

@@ -1,3 +1,4 @@
+use std::borrow::Cow;
 use std::collections::HashSet;
 use std::ffi::OsStr;
 use std::io;
@@ -9,6 +10,7 @@ use codex_utils_absolute_path::canonicalize_preserving_symlinks;
 use codex_utils_path_uri::LegacyAppPathString;
 use codex_utils_path_uri::PathConvention;
 use codex_utils_path_uri::PathUri;
+use globset::Candidate;
 use globset::GlobBuilder;
 use globset::GlobMatcher;
 use schemars::JsonSchema;
@@ -286,7 +288,10 @@ struct FileSystemSemanticSignature {
 
 /// Runtime matcher for read-deny entries in a filesystem sandbox policy.
 pub struct ReadDenyMatcher {
-    denied_candidates: Vec<Vec<PathBuf>>,
+    cwd: AbsolutePathBuf,
+    user_home_dir: Option<PathUri>,
+    temporary_directories: Vec<PathUri>,
+    denied_roots: Vec<PathUri>,
     deny_read_matchers: Vec<GlobMatcher>,
     invalid_pattern: bool,
 }
@@ -332,32 +337,24 @@ impl ReadDenyMatcher {
         if !file_system_sandbox_policy.has_denied_read_restrictions() {
             return Ok(None);
         }
-
+        let cwd = AbsolutePathBuf::from_absolute_path(cwd)
-        // Exact roots are stored as all meaningful path spellings we can derive
+            .map_err(|err| format!("invalid read-deny cwd: {err}"))?;
-        // cheaply. This lets direct tool checks catch both a symlink path and
+        let cwd_uri = PathUri::from_abs_path(&cwd);
-        // its canonical target without changing the policy entries themselves.
+        let user_home_dir = PathUri::from_host_native_path("~").ok();
-        let denied_candidates = file_system_sandbox_policy
+        let temporary_directories = local_temporary_directories();
-            .get_unreadable_roots_with_cwd(cwd)
+        let context = FileSystemSandboxPolicyContext {
-            .into_iter()
+            cwd: &cwd_uri,
-            .map(|path| normalized_and_canonical_candidates(path.as_path()))
+            workspace_roots: std::slice::from_ref(&cwd_uri),
-            .collect();
+            user_home_dir: user_home_dir.as_ref(),
-        // Pattern entries stay as policy-level globs. They are matched at read
+            temporary_directories: Some(&temporary_directories),
-        // time here instead of being snapshotted to startup filesystem state.
+        };
-        let mut invalid_pattern = false;
+        let (denied_roots, deny_read_matchers, invalid_pattern) = file_system_sandbox_policy
-        let mut deny_read_matchers = Vec::new();
+            .prepare_deny_read_matcher(&context, invalid_glob_behavior)?;
-        for pattern in file_system_sandbox_policy.get_unreadable_globs_with_cwd(cwd) {
-            match build_glob_matcher(&pattern) {
-                Ok(matcher) => deny_read_matchers.push(matcher),
-                Err(err) => match invalid_glob_behavior {
-                    InvalidDenyReadGlobBehavior::FailClosed => invalid_pattern = true,
-                    InvalidDenyReadGlobBehavior::ReturnError => {
-                        return Err(format!("invalid deny-read glob pattern `{pattern}`: {err}"));
-                    }
-                },
-            }
-        }
         Ok(Some(Self {
-            denied_candidates,
+            cwd,
+            user_home_dir,
+            temporary_directories,
+            denied_roots,
             deny_read_matchers,
             invalid_pattern,
         }))
@@ -365,10 +362,24 @@ impl ReadDenyMatcher {
 
     /// Returns whether `path` is denied by the policy used to build this matcher.
     pub fn is_read_denied(&self, path: &Path) -> bool {
-        if self.invalid_pattern {
+        let Some(path) = resolve_candidate_path(path, self.cwd.as_path()) else {
             return true;
-        }
+        };
-        self.is_read_denied_candidates(&normalized_and_canonical_candidates(path))
+        let path = PathUri::from(path);
+        let cwd = PathUri::from_abs_path(&self.cwd);
+        let context = FileSystemSandboxPolicyContext {
+            cwd: &cwd,
+            workspace_roots: std::slice::from_ref(&cwd),
+            user_home_dir: self.user_home_dir.as_ref(),
+            temporary_directories: Some(&self.temporary_directories),
+        };
+        FileSystemSandboxPolicy::matches_prepared_read_deny(
+            &path,
+            &context,
+            &self.denied_roots,
+            &self.deny_read_matchers,
+            self.invalid_pattern,
+        )
     }
 
     /// Checks an enumerated path using a canonical location already resolved by
@@ -379,39 +390,7 @@ impl ReadDenyMatcher {
     /// must be resolved separately. Do not reuse these locations across walks:
     /// a later operation must observe newly created files and changed links.
     pub fn is_read_denied_with_canonical_path(&self, path: &Path, canonical_path: &Path) -> bool {
-        let candidates = [path, canonical_path].map(|candidate| {
+        self.is_read_denied(path) || self.is_read_denied(canonical_path)
-            AbsolutePathBuf::from_absolute_path(candidate)
-                .map(AbsolutePathBuf::into_path_buf)
-                .unwrap_or_else(|_| candidate.to_path_buf())
-        });
-        self.is_read_denied_candidates(&candidates)
-    }
-
-    fn is_read_denied_candidates(&self, path_candidates: &[PathBuf]) -> bool {
-        if self.invalid_pattern {
-            // Direct tool reads fail closed on malformed deny patterns. Silent
-            // allow would turn a config typo into a policy bypass.
-            return true;
-        }
-
-        // Check exact roots against each candidate spelling before evaluating
-        // glob matchers. Exact entries are subtree denies; glob entries match
-        // according to the pattern compiler's path-separator rules.
-        if self.denied_candidates.iter().any(|denied_candidates| {
-            path_candidates.iter().any(|candidate| {
-                denied_candidates.iter().any(|denied_candidate| {
-                    candidate == denied_candidate || candidate.starts_with(denied_candidate)
-                })
-            })
-        }) {
-            return true;
-        }
-
-        self.deny_read_matchers.iter().any(|matcher| {
-            path_candidates
-                .iter()
-                .any(|candidate| matcher.is_match(candidate))
-        })
     }
 }
 
@@ -990,6 +969,128 @@ impl FileSystemSandboxPolicy {
         .then_some(metadata_name)
     }
 
+    fn prepare_deny_read_matcher(
+        &self,
+        context: &FileSystemSandboxPolicyContext<'_>,
+        invalid_glob_behavior: InvalidDenyReadGlobBehavior,
+    ) -> Result<(Vec<PathUri>, Vec<GlobMatcher>, bool), String> {
+        let file_system_root = file_system_root(context);
+        let denied_roots = self
+            .resolved_entries(context)
+            .into_iter()
+            .filter(|(_, access)| *access == FileSystemAccessMode::Deny)
+            .filter(|(root, _)| {
+                !file_system_root.as_ref().is_some_and(|file_system_root| {
+                    root.starts_with(file_system_root) && file_system_root.starts_with(root)
+                })
+            })
+            .map(|(root, _)| root)
+            .collect();
+        let Some(convention) = context.cwd.infer_path_convention() else {
+            return Ok((denied_roots, Vec::new(), true));
+        };
+        let mut deny_read_matchers = Vec::new();
+        let mut invalid_pattern = false;
+        let patterns = match self.deny_read_globs(context) {
+            Ok(patterns) => patterns,
+            Err(err) => match invalid_glob_behavior {
+                InvalidDenyReadGlobBehavior::FailClosed => {
+                    return Ok((denied_roots, Vec::new(), true));
+                }
+                InvalidDenyReadGlobBehavior::ReturnError => return Err(err),
+            },
+        };
+        for pattern in patterns {
+            match build_glob_matcher(&pattern, convention) {
+                Ok(matcher) => deny_read_matchers.push(matcher),
+                Err(err) => match invalid_glob_behavior {
+                    InvalidDenyReadGlobBehavior::FailClosed => invalid_pattern = true,
+                    InvalidDenyReadGlobBehavior::ReturnError => {
+                        return Err(format!("invalid deny-read glob pattern `{pattern}`: {err}"));
+                    }
+                },
+            }
+        }
+        Ok((denied_roots, deny_read_matchers, invalid_pattern))
+    }
+
+    fn matches_prepared_read_deny(
+        path: &PathUri,
+        context: &FileSystemSandboxPolicyContext<'_>,
+        denied_roots: &[PathUri],
+        deny_read_matchers: &[GlobMatcher],
+        invalid_pattern: bool,
+    ) -> bool {
+        let Some(convention) = context.cwd.infer_path_convention() else {
+            return true;
+        };
+        if path.infer_path_convention() != Some(convention)
+            || path.lexical_depth().is_none()
+            || context.cwd.lexical_depth().is_none()
+        {
+            return true;
+        }
+        if invalid_pattern {
+            return true;
+        }
+        denied_roots.iter().any(|root| path.starts_with(root))
+            || deny_read_matchers.iter().any(|matcher| {
+                let path = match convention {
+                    PathConvention::Posix => path.decoded_path_bytes(),
+                    PathConvention::Windows => Cow::Owned(
+                        path.inferred_native_path_string()
+                            .replace('\\', "/")
+                            .into_bytes(),
+                    ),
+                };
+                matcher.is_match_candidate(&Candidate::from_bytes(path.as_ref()))
+            })
+    }
+
+    fn deny_read_globs(
+        &self,
+        context: &FileSystemSandboxPolicyContext<'_>,
+    ) -> Result<Vec<String>, String> {
+        self.entries
+            .iter()
+            .filter(|entry| entry.access == FileSystemAccessMode::Deny)
+            .filter_map(|entry| {
+                let FileSystemPath::GlobPattern { pattern } = &entry.path else {
+                    return None;
+                };
+                let is_windows =
+                    context.cwd.infer_path_convention() == Some(PathConvention::Windows);
+                let home_relative = pattern.strip_prefix("~/").or_else(|| {
+                    is_windows.then(|| pattern.strip_prefix("~\\")).flatten()
+                });
+                let (root, pattern) = match home_relative {
+                    Some(suffix) => match context.user_home_dir {
+                        Some(home) => (
+                            home,
+                            suffix.trim_start_matches(|separator| {
+                                separator == '/' || is_windows && separator == '\\'
+                            }),
+                        ),
+                        None => {
+                            return Some(Err(format!(
+                                "unable to resolve deny-read glob pattern `{pattern}` without executor home"
+                            )));
+                        }
+                    },
+                    None => (context.cwd, pattern.as_str()),
+                };
+                Some(
+                    root
+                        .join(pattern)
+                        .map(|path| path.inferred_native_path_string())
+                        .map_err(|_| {
+                            format!("unable to resolve deny-read glob pattern `{pattern}`")
+                        }),
+                )
+            })
+            .collect()
+    }
+
     /// Replaces symbolic `:workspace_roots` entries with absolute paths resolved
     /// against `cwd`.
     ///
@@ -1890,39 +1991,19 @@ fn absolute_root_path_for_cwd(cwd: &AbsolutePathBuf) -> AbsolutePathBuf {
         .unwrap_or_else(|err| panic!("cwd root must be an absolute path: {err}"))
 }
 
-fn normalized_and_canonical_candidates(path: &Path) -> Vec<PathBuf> {
+fn build_glob_matcher(pattern: &str, convention: PathConvention) -> Result<GlobMatcher, String> {
-    // Compare the lexical absolute form plus the canonical target when it
-    // exists. Missing paths still need the lexical candidate so future-created
-    // denied paths remain blocked by direct tool checks.
-    let mut candidates = Vec::new();
-
-    if let Ok(normalized) = AbsolutePathBuf::from_absolute_path(path) {
-        push_unique(&mut candidates, normalized.to_path_buf());
-    } else {
-        push_unique(&mut candidates, path.to_path_buf());
-    }
-
-    if let Ok(canonical) = path.canonicalize()
-        && let Ok(canonical_absolute) = AbsolutePathBuf::from_absolute_path(canonical)
-    {
-        push_unique(&mut candidates, canonical_absolute.to_path_buf());
-    }
-
-    candidates
-}
-
-fn push_unique(candidates: &mut Vec<PathBuf>, candidate: PathBuf) {
-    if !candidates.iter().any(|existing| existing == &candidate) {
-        candidates.push(candidate);
-    }
-}
-
-fn build_glob_matcher(pattern: &str) -> Result<GlobMatcher, String> {
     // Keep `*` and `?` within a single path component and preserve an unclosed
     // `[` as a literal so matcher behavior stays aligned with config parsing.
-    GlobBuilder::new(pattern)
+    let pattern = if convention == PathConvention::Windows {
+        pattern.replace('\\', "/")
+    } else {
+        pattern.to_string()
+    };
+    GlobBuilder::new(&pattern)
         .literal_separator(true)
         .allow_unclosed_class(true)
+        .backslash_escape(convention == PathConvention::Posix)
+        .case_insensitive(convention == PathConvention::Windows)
         .build()
         .map(|glob| glob.compile_matcher())
         .map_err(|err| err.to_string())
@@ -2624,6 +2705,121 @@ mod tests {
         );
     }
 
+    #[test]
+    fn uri_deny_and_metadata_matcher_use_executor_paths() {
+        let path = |path| PathUri::parse(path).expect("valid path URI");
+        let cwd = path("file:///C:/workspace");
+        let roots = [cwd.clone()];
+        let context = FileSystemSandboxPolicyContext {
+            cwd: &cwd,
+            workspace_roots: &roots,
+            user_home_dir: None,
+            temporary_directories: None,
+        };
+        let policy = FileSystemSandboxPolicy::restricted(vec![
+            FileSystemSandboxEntry::new(cwd.clone().into(), FileSystemAccessMode::Write),
+            FileSystemSandboxEntry::new(path("file:///C:/").into(), FileSystemAccessMode::Deny),
+            FileSystemSandboxEntry::new(
+                path("file:///C:/workspace/.codex").into(),
+                FileSystemAccessMode::Read,
+            ),
+            unreadable_glob_entry(r"C:\workspace\**\*.env".to_string()),
+        ]);
+        let (roots, globs, invalid) = policy
+            .prepare_deny_read_matcher(&context, InvalidDenyReadGlobBehavior::ReturnError)
+            .expect("remote deny matcher");
+
+        assert!(roots.is_empty());
+        assert!(FileSystemSandboxPolicy::matches_prepared_read_deny(
+            &path("file:///c:/WORKSPACE/app/.ENV"),
+            &context,
+            &roots,
+            &globs,
+            invalid,
+        ));
+        for candidate in ["file:///%00/bad/path/YQ", "file:///C:/workspace/%2Fsecret"] {
+            assert!(FileSystemSandboxPolicy::matches_prepared_read_deny(
+                &path(candidate),
+                &context,
+                &roots,
+                &globs,
+                invalid,
+            ));
+        }
+        assert!(!policy.can_write_path(&path("file:///C:/workspace/.codex/config"), &context));
+        assert_eq!(
+            policy.metadata_write_denial(&path("file:///C:/workspace/.codex/config"), &context),
+            Some(".codex"),
+        );
+    }
+
+    #[test]
+    fn uri_deny_globs_use_executor_home() {
+        for (cwd, home, pattern, candidate) in [
+            (
+                "file:///workspace",
+                "file:///home/executor",
+                "~/private/*.env",
+                "file:///home/executor/private/secret.env",
+            ),
+            (
+                "file:///workspace",
+                "file:///home/executor",
+                "~//private/*.env",
+                "file:///home/executor/private/secret.env",
+            ),
+            (
+                "file:///C:/workspace",
+                "file:///C:/Users/executor",
+                r"~\private\*.env",
+                "file:///C:/Users/executor/private/secret.env",
+            ),
+            (
+                "file:///C:/workspace",
+                "file:///C:/Users/executor",
+                r"~\\private\*.env",
+                "file:///C:/Users/executor/private/secret.env",
+            ),
+        ] {
+            let cwd = PathUri::parse(cwd).expect("executor cwd");
+            let home = PathUri::parse(home).expect("executor home");
+            let candidate = PathUri::parse(candidate).expect("denied path");
+            let context = FileSystemSandboxPolicyContext {
+                cwd: &cwd,
+                workspace_roots: std::slice::from_ref(&cwd),
+                user_home_dir: Some(&home),
+                temporary_directories: None,
+            };
+            let policy = FileSystemSandboxPolicy::restricted(vec![unreadable_glob_entry(
+                pattern.to_string(),
+            )]);
+            let (roots, globs, invalid) = policy
+                .prepare_deny_read_matcher(&context, InvalidDenyReadGlobBehavior::ReturnError)
+                .expect("home-relative deny glob");
+
+            assert!(FileSystemSandboxPolicy::matches_prepared_read_deny(
+                &candidate, &context, &roots, &globs, invalid,
+            ));
+
+            let without_home = FileSystemSandboxPolicyContext {
+                user_home_dir: None,
+                ..context
+            };
+            assert!(
+                policy
+                    .prepare_deny_read_matcher(
+                        &without_home,
+                        InvalidDenyReadGlobBehavior::ReturnError,
+                    )
+                    .is_err()
+            );
+            let (_, _, invalid) = policy
+                .prepare_deny_read_matcher(&without_home, InvalidDenyReadGlobBehavior::FailClosed)
+                .expect("missing executor home fails closed");
+            assert!(invalid);
+        }
+    }
+
     #[test]
     fn unknown_special_paths_are_ignored_by_legacy_bridge() -> std::io::Result<()> {
         let policy = FileSystemSandboxPolicy::restricted(vec![
@@ -4091,7 +4287,7 @@ mod tests {
 
     #[cfg(unix)]
     #[test]
-    fn canonical_target_matches_denied_symlink_alias() {
+    fn native_read_deny_matching_uses_uri_matcher_semantics() {
         let temp = TempDir::new().expect("tempdir");
         let real_dir = temp.path().join("real");
         let alias_dir = temp.path().join("alias");
@@ -4102,8 +4298,17 @@ mod tests {
         std::fs::write(&secret, "secret").expect("write secret");
         let alias_secret = alias_dir.join("secret.txt");
 
-        let policy = deny_policy(&real_dir);
+        for (denied_root, candidate) in [(&real_dir, &alias_secret), (&alias_dir, &secret)] {
-        assert!(is_read_denied(&alias_secret, &policy, temp.path()));
+            let policy = deny_policy(denied_root);
+            let denied_root_uri =
+                PathUri::from_host_native_path(denied_root).expect("deny root URI");
+            let candidate_uri = PathUri::from_host_native_path(candidate).expect("candidate URI");
+
+            assert_eq!(
+                is_read_denied(candidate, &policy, temp.path()),
+                candidate_uri.starts_with(&denied_root_uri)
+            );
+        }
     }
 
     #[test]
@@ -4137,6 +4342,17 @@ mod tests {
         ));
 
         assert!(is_read_denied(&denied, &policy, temp.path()));
+
+        #[cfg(unix)]
+        {
+            use std::os::unix::ffi::OsStrExt;
+
+            let non_utf8 = denied
+                .parent()
+                .expect("parent")
+                .join(OsStr::from_bytes(b"secret\xff.txt"));
+            assert!(is_read_denied(&non_utf8, &policy, temp.path()));
+        }
     }
 
     #[test]

codex-rs/utils/path-uri/src/lib.rs

@@ -186,6 +186,13 @@ impl PathUri {
         self.0.path()
     }
 
+    /// Returns the percent-decoded URI path without requiring valid UTF-8.
+    ///
+    /// The URL authority is not included.
+    pub fn decoded_path_bytes(&self) -> Cow<'_, [u8]> {
+        urlencoding::decode_binary(self.encoded_path().as_bytes())
+    }
+
     fn windows_identity_path_bytes(&self) -> Option<Cow<'_, [u8]>> {
         if self.infer_path_convention() != Some(PathConvention::Windows)
             || self.opaque_fallback_bytes().is_some()

codex-rs/windows-sandbox-rs/src/deny_read_resolver.rs

@@ -103,6 +103,7 @@ fn ripgrep_files(scan_plan: &GlobScanPlan) -> Result<Option<Vec<PathBuf>>, Strin
         .arg("--files")
         .arg("--hidden")
         .arg("--no-ignore")
+        .arg("--glob-case-insensitive")
         .arg("--null");
     if let Some(max_depth) = scan_plan.max_depth {
         command.arg("--max-depth").arg(max_depth.to_string());
@@ -541,22 +542,36 @@ mod tests {
         let cwd = AbsolutePathBuf::from_absolute_path(tmp.path()).expect("absolute cwd");
         let root_env = tmp.path().join(".env");
         let nested_env = tmp.path().join("app").join(".env");
+        let uppercase_env = tmp.path().join("app").join("SECRET.ENV");
         let notes = tmp.path().join("app").join("notes.txt");
         std::fs::create_dir_all(notes.parent().expect("parent")).expect("create parent");
         std::fs::write(&root_env, "secret").expect("write root env");
         std::fs::write(&nested_env, "secret").expect("write nested env");
+        std::fs::write(&uppercase_env, "secret").expect("write uppercase env");
         std::fs::write(&notes, "notes").expect("write notes");
         let policy = FileSystemSandboxPolicy::restricted(vec![unreadable_glob_entry(format!(
             "{}/**/*.env",
             tmp.path().display()
         ))]);
 
+        if let Some(paths) = super::ripgrep_files(&GlobScanPlan {
+            root: tmp.path().to_path_buf(),
+            max_depth: None,
+            globs: vec!["**/*.env".to_string()],
+        })
+        .expect("case-insensitive ripgrep scan")
+        {
+            assert!(paths.contains(&uppercase_env));
+        }
         let actual: HashSet<PathBuf> = resolve_windows_deny_read_paths(&policy, &cwd)
             .expect("resolve")
             .into_iter()
             .map(AbsolutePathBuf::into_path_buf)
             .collect();
-        let expected = [root_env, nested_env].into_iter().collect();
+        let mut expected = [root_env, nested_env].into_iter().collect::<HashSet<_>>();
+        if cfg!(windows) {
+            expected.insert(uppercase_env);
+        }
 
         assert_eq!(actual, expected);
     }

codex-rs/windows-sandbox-rs/src/deny_read_resolver_access_tests.rs

@@ -222,7 +222,15 @@ fn native_walk_preserves_canonical_matches_through_directory_links_and_cycles()
     let matcher = ReadDenyMatcher::try_new(&policy, cwd.as_path())
         .expect("valid matcher")
         .expect("deny restriction");
-    assert!(matcher.is_read_denied(&alias.join("secret.env")));
+    assert!(
+        matcher.is_read_denied_with_canonical_path(
+            &alias.join("secret.env"),
+            &target
+                .canonicalize()
+                .expect("canonical target")
+                .join("secret.env"),
+        )
+    );
     let mut paths = Vec::new();
     collect_existing_glob_directory_matches(
         &walk,