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>
88 lines
2.9 KiB
Rust
88 lines
2.9 KiB
Rust
use crate::acl::revoke_ace;
|
|
use crate::deny_read_acl::apply_deny_read_acls;
|
|
use crate::deny_read_acl::lexical_path_key;
|
|
use crate::setup::sandbox_dir;
|
|
use anyhow::Context;
|
|
use anyhow::Result;
|
|
use serde::Deserialize;
|
|
use serde::Serialize;
|
|
use std::collections::BTreeMap;
|
|
use std::collections::HashSet;
|
|
use std::ffi::c_void;
|
|
use std::path::Path;
|
|
use std::path::PathBuf;
|
|
|
|
const DENY_READ_ACL_STATE_FILE: &str = "deny_read_acl_state.json";
|
|
|
|
#[derive(Default, Deserialize, Serialize)]
|
|
struct PersistentDenyReadAclState {
|
|
principals: BTreeMap<String, Vec<PathBuf>>,
|
|
}
|
|
|
|
/// Reconciles the persistent deny-read ACEs owned by one sandbox principal.
|
|
///
|
|
/// Workspace-write and elevated sandbox sessions intentionally leave ACLs in
|
|
/// place after a command exits, because descendants may outlive the launcher.
|
|
/// That makes the ACL set stateful across runs. Persist the paths applied for
|
|
/// each SID, apply the new desired set first, and only then revoke stale paths
|
|
/// from the same SID so profile changes do not leave old deny-read ACEs behind.
|
|
///
|
|
/// # Safety
|
|
/// Caller must pass a valid SID pointer matching `principal_sid`.
|
|
pub unsafe fn sync_persistent_deny_read_acls(
|
|
codex_home: &Path,
|
|
principal_sid: &str,
|
|
desired_paths: &[PathBuf],
|
|
psid: *mut c_void,
|
|
) -> Result<Vec<PathBuf>> {
|
|
let state_path = sandbox_dir(codex_home).join(DENY_READ_ACL_STATE_FILE);
|
|
let mut state = load_state(&state_path)?;
|
|
let previous_paths = state
|
|
.principals
|
|
.get(principal_sid)
|
|
.cloned()
|
|
.unwrap_or_default();
|
|
|
|
let applied_paths = unsafe { apply_deny_read_acls(desired_paths, psid) }?;
|
|
let desired_keys = applied_paths
|
|
.iter()
|
|
.map(|path| lexical_path_key(path))
|
|
.collect::<HashSet<_>>();
|
|
|
|
for path in previous_paths {
|
|
if !desired_keys.contains(&lexical_path_key(&path)) {
|
|
revoke_ace(&path, psid);
|
|
}
|
|
}
|
|
|
|
if applied_paths.is_empty() {
|
|
state.principals.remove(principal_sid);
|
|
} else {
|
|
state
|
|
.principals
|
|
.insert(principal_sid.to_string(), applied_paths.clone());
|
|
}
|
|
store_state(&state_path, &state)?;
|
|
|
|
Ok(applied_paths)
|
|
}
|
|
|
|
fn load_state(path: &Path) -> Result<PersistentDenyReadAclState> {
|
|
match std::fs::read(path) {
|
|
Ok(bytes) => serde_json::from_slice(&bytes)
|
|
.with_context(|| format!("parse deny-read ACL state {}", path.display())),
|
|
Err(err) if err.kind() == std::io::ErrorKind::NotFound => {
|
|
Ok(PersistentDenyReadAclState::default())
|
|
}
|
|
Err(err) => {
|
|
Err(err).with_context(|| format!("read deny-read ACL state {}", path.display()))
|
|
}
|
|
}
|
|
}
|
|
|
|
fn store_state(path: &Path, state: &PersistentDenyReadAclState) -> Result<()> {
|
|
let bytes = serde_json::to_vec_pretty(state).context("serialize deny-read ACL state")?;
|
|
std::fs::write(path, bytes)
|
|
.with_context(|| format!("write deny-read ACL state {}", path.display()))
|
|
}
|