mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
## Why The split filesystem policy stack already supports exact and glob `access = none` read restrictions on macOS and Linux. Windows still needed subprocess handling for those deny-read policies without claiming enforcement from a backend that cannot provide it. ## Key finding The unelevated restricted-token backend cannot safely enforce deny-read overlays. Its `WRITE_RESTRICTED` token model is authoritative for write checks, not read denials, so this PR intentionally fails that backend closed when deny-read overrides are present instead of claiming unsupported enforcement. ## What changed This PR adds the Windows deny-read enforcement layer and makes the backend split explicit: - Resolves Windows deny-read filesystem policy entries into concrete ACL targets. - Preserves exact missing paths so they can be materialized and denied before an enforceable sandboxed process starts. - Snapshot-expands existing glob matches into ACL targets for Windows subprocess enforcement. - Honors `glob_scan_max_depth` when expanding Windows deny-read globs. - Plans both the configured lexical path and the canonical target for existing paths so reparse-point aliases are covered. - Threads deny-read overrides through the elevated/logon-user Windows sandbox backend and unified exec. - Applies elevated deny-read ACLs synchronously before command launch rather than delegating them to the background read-grant helper. - Reconciles persistent deny-read ACEs per sandbox principal so policy changes do not leave stale deny-read ACLs behind. - Fails closed on the unelevated restricted-token backend when deny-read overrides are present, because its `WRITE_RESTRICTED` token model is not authoritative for read denials. ## Landed prerequisites These prerequisite PRs are already on `main`: 1. #15979 `feat(permissions): add glob deny-read policy support` 2. #18096 `feat(sandbox): add glob deny-read platform enforcement` 3. #17740 `feat(config): support managed deny-read requirements` This PR targets `main` directly and contains only the Windows deny-read enforcement layer. ## Implementation notes - Exact deny-read paths remain enforceable on the elevated path even when they do not exist yet: Windows materializes the missing path before applying the deny ACE, so the sandboxed command cannot create and read it during the same run. - Existing exact deny paths are preserved lexically until the ACL planner, which then adds the canonical target as a second ACL target when needed. That keeps both the configured alias and the resolved object covered. - Windows ACLs do not consume Codex glob syntax directly, so glob deny-read entries are expanded to the concrete matches that exist before process launch. - Glob traversal deduplicates directory visits within each pattern walk to avoid cycles, without collapsing distinct lexical roots that happen to resolve to the same target. - Persistent deny-read ACL state is keyed by sandbox principal SID, so cleanup only removes ACEs owned by the same backend principal. - Deny-read ACEs are fail-closed on the elevated path: setup aborts if mandatory deny-read ACL application fails. - Unelevated restricted-token sessions reject deny-read overrides early instead of running with a silently unenforceable read policy. ## Verification - `cargo test -p codex-core windows_restricted_token_rejects_unreadable_split_carveouts` - `just fmt` - `just fix -p codex-core` - `just fix -p codex-windows-sandbox` - GitHub Actions rerun is in progress on the pushed head. --------- Co-authored-by: Codex <noreply@openai.com>
120 lines
4.0 KiB
Rust
120 lines
4.0 KiB
Rust
use crate::acl::add_deny_read_ace;
|
|
use crate::acl::revoke_ace;
|
|
use crate::path_normalization::canonicalize_path;
|
|
use anyhow::Context;
|
|
use anyhow::Result;
|
|
use std::collections::HashSet;
|
|
use std::ffi::c_void;
|
|
use std::path::Path;
|
|
use std::path::PathBuf;
|
|
|
|
/// Build the exact ACL paths that should receive a deny-read ACE.
|
|
///
|
|
/// We keep both the lexical policy path and, when it already exists, the
|
|
/// canonical target. The lexical path covers the path users configured and lets
|
|
/// missing exact denies be materialized later; the canonical path also covers
|
|
/// an existing reparse-point target so a sandbox cannot read the same object
|
|
/// through the resolved location.
|
|
pub fn plan_deny_read_acl_paths(paths: &[PathBuf]) -> Vec<PathBuf> {
|
|
let mut planned = Vec::new();
|
|
let mut seen = HashSet::new();
|
|
for path in paths {
|
|
push_planned_path(&mut planned, &mut seen, path.to_path_buf());
|
|
if path.exists() {
|
|
push_planned_path(&mut planned, &mut seen, canonicalize_path(path));
|
|
}
|
|
}
|
|
planned
|
|
}
|
|
|
|
fn push_planned_path(planned: &mut Vec<PathBuf>, seen: &mut HashSet<String>, path: PathBuf) {
|
|
if seen.insert(lexical_path_key(&path)) {
|
|
planned.push(path);
|
|
}
|
|
}
|
|
|
|
pub(crate) fn lexical_path_key(path: &Path) -> String {
|
|
path.to_string_lossy()
|
|
.replace('\\', "/")
|
|
.trim_end_matches('/')
|
|
.to_ascii_lowercase()
|
|
}
|
|
|
|
/// Applies deny-read ACEs to explicit paths. Missing paths are materialized as
|
|
/// directories before the ACE is applied so a sandboxed command cannot create a
|
|
/// previously absent denied path and then read from it in the same run.
|
|
/// If any path fails, deny ACEs applied by this call are revoked before the
|
|
/// error is returned so a one-shot sandbox run does not leave partial state.
|
|
///
|
|
/// # Safety
|
|
/// Caller must pass a valid SID pointer for the sandbox principal being denied.
|
|
pub unsafe fn apply_deny_read_acls(paths: &[PathBuf], psid: *mut c_void) -> Result<Vec<PathBuf>> {
|
|
let planned = plan_deny_read_acl_paths(paths);
|
|
let mut applied = Vec::new();
|
|
let mut seen = HashSet::new();
|
|
let mut added_in_this_call: Vec<PathBuf> = Vec::new();
|
|
for path in planned {
|
|
let result = (|| -> Result<bool> {
|
|
if !path.exists() {
|
|
std::fs::create_dir_all(&path)
|
|
.with_context(|| format!("create deny-read path {}", path.display()))?;
|
|
}
|
|
add_deny_read_ace(&path, psid)
|
|
.with_context(|| format!("apply deny-read ACE to {}", path.display()))
|
|
})();
|
|
let added = match result {
|
|
Ok(added) => added,
|
|
Err(err) => {
|
|
for added_path in &added_in_this_call {
|
|
revoke_ace(added_path, psid);
|
|
}
|
|
return Err(err);
|
|
}
|
|
};
|
|
if added {
|
|
added_in_this_call.push(path.clone());
|
|
}
|
|
push_planned_path(&mut applied, &mut seen, path);
|
|
}
|
|
Ok(applied)
|
|
}
|
|
|
|
#[cfg(test)]
|
|
mod tests {
|
|
use super::plan_deny_read_acl_paths;
|
|
use pretty_assertions::assert_eq;
|
|
use std::collections::HashSet;
|
|
use std::path::PathBuf;
|
|
use tempfile::TempDir;
|
|
|
|
#[test]
|
|
fn plan_preserves_missing_paths() {
|
|
let tmp = TempDir::new().expect("tempdir");
|
|
let missing = tmp.path().join("future-secret.env");
|
|
|
|
assert_eq!(
|
|
plan_deny_read_acl_paths(std::slice::from_ref(&missing)),
|
|
vec![missing]
|
|
);
|
|
}
|
|
|
|
#[test]
|
|
fn plan_includes_existing_canonical_targets() {
|
|
let tmp = TempDir::new().expect("tempdir");
|
|
let existing = tmp.path().join("secret.env");
|
|
std::fs::write(&existing, "secret").expect("write secret");
|
|
|
|
let planned: HashSet<PathBuf> = plan_deny_read_acl_paths(std::slice::from_ref(&existing))
|
|
.into_iter()
|
|
.collect();
|
|
let expected: HashSet<PathBuf> = [
|
|
existing.clone(),
|
|
dunce::canonicalize(&existing).expect("canonical path"),
|
|
]
|
|
.into_iter()
|
|
.collect();
|
|
|
|
assert_eq!(planned, expected);
|
|
}
|
|
}
|