mirror of
https://github.com/openai/codex.git
synced 2026-10-21 05:22:21 +00:00
Preserve Windows managed deny reads in the sandbox CLI (#42835)
## Why Windows sandbox setup reconciles persistent deny ACLs against the paths supplied for a session. The `codex sandbox --permission-profile` path supplied an empty list, which could discard the profile's deny-read restrictions. ## What changed Resolve deny-read paths from the selected permission profile and workspace roots before starting a Windows sandbox session, and pass them through to setup. ## Testing Add a Windows CLI integration test that launches the same managed-deny profile twice and verifies that denied file reads and DLL loads remain blocked while allowed access still works. GitOrigin-RevId: 6ec8b6a13a33b862d7b152eadbbdde3146de4b7d
This commit is contained in:
@@ -487,8 +487,22 @@ async fn run_command_under_windows_session(
|
||||
use codex_protocol::config_types::WindowsSandboxLevel;
|
||||
use codex_windows_sandbox::WindowsSandboxProxySettingsMode;
|
||||
use codex_windows_sandbox::WindowsSandboxSessionRequest;
|
||||
use codex_windows_sandbox::resolve_windows_deny_read_paths;
|
||||
use codex_windows_sandbox::spawn_windows_sandbox_session_for_level;
|
||||
|
||||
// Setup reconciles persistent deny ACLs against this list. An empty list
|
||||
// would discard the profile's denies, including on subsequent launches.
|
||||
let (mut file_system, _) = permission_profile.to_runtime_permissions();
|
||||
file_system.remove_skip_missing_path_entries();
|
||||
let file_system = file_system.materialize_project_roots_with_workspace_roots(&workspace_roots);
|
||||
let deny_read_paths = match resolve_windows_deny_read_paths(&file_system, &cwd) {
|
||||
Ok(paths) => paths,
|
||||
Err(err) => {
|
||||
eprintln!("windows sandbox failed: {err}");
|
||||
std::process::exit(1);
|
||||
}
|
||||
};
|
||||
|
||||
let empty_paths: &[AbsolutePathBuf] = &[];
|
||||
let spawned = spawn_windows_sandbox_session_for_level(WindowsSandboxSessionRequest {
|
||||
permission_profile,
|
||||
@@ -505,7 +519,7 @@ async fn run_command_under_windows_session(
|
||||
read_roots_override: None,
|
||||
read_roots_include_platform_defaults: false,
|
||||
write_roots_override: None,
|
||||
deny_read_paths_override: empty_paths,
|
||||
deny_read_paths_override: &deny_read_paths,
|
||||
deny_write_paths_override: empty_paths,
|
||||
tty: false,
|
||||
stdin_open: true,
|
||||
|
||||
@@ -12,6 +12,7 @@ codex_rust_crate(
|
||||
"//codex-rs/rmcp-client:test_streamable_http_server",
|
||||
"//codex-rs/cli:codex",
|
||||
"//codex-rs/windows-sandbox-rs:codex-command-runner",
|
||||
"//codex-rs/windows-sandbox-rs:codex-windows-managed-deny-probe",
|
||||
"//codex-rs/windows-sandbox-rs:codex-windows-sandbox-setup",
|
||||
],
|
||||
integration_compile_data_extra = glob(["assets/**"]),
|
||||
|
||||
@@ -30,6 +30,7 @@ use std::collections::HashMap;
|
||||
use std::ffi::OsString;
|
||||
use std::path::Path;
|
||||
use std::path::PathBuf;
|
||||
use std::process::Command;
|
||||
use tempfile::TempDir;
|
||||
|
||||
struct EnvVarGuard {
|
||||
@@ -124,6 +125,143 @@ fn stage_windows_sandbox_helpers() -> anyhow::Result<()> {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn escape_toml_path(path: &Path) -> String {
|
||||
path.display().to_string().replace('\\', "\\\\")
|
||||
}
|
||||
|
||||
fn stage_windows_sandbox_cli(fixture_bin: &Path) -> anyhow::Result<(PathBuf, PathBuf)> {
|
||||
std::fs::create_dir_all(fixture_bin)?;
|
||||
let resources_dir = fixture_bin.join("codex-resources");
|
||||
std::fs::create_dir_all(&resources_dir)?;
|
||||
|
||||
let codex_source = codex_utils_cargo_bin::cargo_bin("codex")?;
|
||||
let codex = fixture_bin.join("codex.exe");
|
||||
std::fs::copy(&codex_source, &codex)
|
||||
.with_context(|| format!("copy {} to {}", codex_source.display(), codex.display()))?;
|
||||
for helper_name in ["codex-windows-sandbox-setup", "codex-command-runner"] {
|
||||
let helper = codex_utils_cargo_bin::cargo_bin(helper_name)?;
|
||||
let destination = resources_dir.join(Path::new(helper_name).with_extension("exe"));
|
||||
std::fs::copy(&helper, &destination)
|
||||
.with_context(|| format!("copy {} to {}", helper.display(), destination.display()))?;
|
||||
}
|
||||
|
||||
let probe_source = codex_utils_cargo_bin::cargo_bin("codex-windows-managed-deny-probe")?;
|
||||
let probe = fixture_bin.join("managed-deny-probe.exe");
|
||||
std::fs::copy(&probe_source, &probe)
|
||||
.with_context(|| format!("copy {} to {}", probe_source.display(), probe.display()))?;
|
||||
Ok((codex, probe))
|
||||
}
|
||||
|
||||
fn assert_managed_deny_probe(output: &std::process::Output, launch: usize) -> anyhow::Result<()> {
|
||||
let stdout = String::from_utf8_lossy(&output.stdout);
|
||||
let stderr = String::from_utf8_lossy(&output.stderr);
|
||||
assert!(
|
||||
output.status.success(),
|
||||
"managed deny probe launch {launch} failed: status={:?}; stdout={stdout}; stderr={stderr}",
|
||||
output.status.code()
|
||||
);
|
||||
assert!(
|
||||
stdout.contains("allowed-read:OK") && stdout.contains("allowed-import:OK"),
|
||||
"managed deny probe launch {launch} lost allowed access: {stdout}"
|
||||
);
|
||||
assert!(
|
||||
stdout.contains("denied-read:DENIED") && stdout.contains("denied-import:DENIED"),
|
||||
"managed deny probe launch {launch} did not enforce denied access: {stdout}"
|
||||
);
|
||||
assert!(
|
||||
!stdout.contains("UNEXPECTED_SUCCESS"),
|
||||
"managed deny probe launch {launch} leaked denied content: {stdout}"
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
#[serial(codex_home)]
|
||||
fn windows_sandbox_cli_preserves_managed_deny_reads_across_launches() -> anyhow::Result<()> {
|
||||
let codex_home =
|
||||
codex_home_for_windows_sandbox_test("windows-cli-managed-deny-read-codex-home")?;
|
||||
|
||||
let fixture = TempDir::new()?;
|
||||
let fixture_root = dunce::canonicalize(fixture.path())?;
|
||||
let work = fixture_root.join("work");
|
||||
let runtime = fixture_root.join("runtime");
|
||||
let denied = runtime.join("denied");
|
||||
let bin = fixture_root.join("bin");
|
||||
std::fs::create_dir_all(&work)?;
|
||||
std::fs::create_dir_all(&denied)?;
|
||||
let (codex, probe) = stage_windows_sandbox_cli(&bin)?;
|
||||
|
||||
let allowed_text = runtime.join("allowed.txt");
|
||||
let denied_text = denied.join("secret.txt");
|
||||
let allowed_module = runtime.join("allowed.dll");
|
||||
let denied_module = denied.join("secret.dll");
|
||||
std::fs::write(&allowed_text, "ALLOW-CONTROL\n")?;
|
||||
std::fs::write(&denied_text, "DENIED-CONTENT\n")?;
|
||||
let system_root = std::env::var_os("SystemRoot").context("resolve SystemRoot")?;
|
||||
let system_module = PathBuf::from(system_root)
|
||||
.join("System32")
|
||||
.join("version.dll");
|
||||
std::fs::copy(&system_module, &allowed_module).with_context(|| {
|
||||
format!(
|
||||
"copy {} to {}",
|
||||
system_module.display(),
|
||||
allowed_module.display()
|
||||
)
|
||||
})?;
|
||||
std::fs::copy(&system_module, &denied_module).with_context(|| {
|
||||
format!(
|
||||
"copy {} to {}",
|
||||
system_module.display(),
|
||||
denied_module.display()
|
||||
)
|
||||
})?;
|
||||
|
||||
std::fs::write(
|
||||
codex_home.path().join("config.toml"),
|
||||
format!(
|
||||
"default_permissions = \"managed-deny-test\"\n\
|
||||
\n\
|
||||
[windows]\n\
|
||||
sandbox = \"elevated\"\n\
|
||||
\n\
|
||||
[shell_environment_policy]\n\
|
||||
inherit = \"all\"\n\
|
||||
\n\
|
||||
[permissions.managed-deny-test.filesystem]\n\
|
||||
\":minimal\" = \"read\"\n\
|
||||
\"{}\" = \"read\"\n\
|
||||
\"{}\" = \"write\"\n\
|
||||
\"{}\" = \"deny\"\n\
|
||||
\n\
|
||||
[permissions.managed-deny-test.network]\n\
|
||||
enabled = false\n",
|
||||
escape_toml_path(&fixture_root),
|
||||
escape_toml_path(&work),
|
||||
escape_toml_path(&denied),
|
||||
),
|
||||
)?;
|
||||
|
||||
for launch in 1..=2 {
|
||||
let output = Command::new(&codex)
|
||||
.current_dir(&work)
|
||||
.env("CODEX_HOME", codex_home.path())
|
||||
.env("CODEX_WINDOWS_ALLOWED_TEXT", &allowed_text)
|
||||
.env("CODEX_WINDOWS_DENIED_TEXT", &denied_text)
|
||||
.env("CODEX_WINDOWS_ALLOWED_MODULE", &allowed_module)
|
||||
.env("CODEX_WINDOWS_DENIED_MODULE", &denied_module)
|
||||
.args(["sandbox", "--permission-profile"])
|
||||
.arg("managed-deny-test")
|
||||
.arg("--cd")
|
||||
.arg(&work)
|
||||
.arg("--")
|
||||
.arg(&probe)
|
||||
.output()?;
|
||||
assert_managed_deny_probe(&output, launch)?;
|
||||
}
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
#[serial(codex_home)]
|
||||
async fn windows_restricted_token_rejects_exact_and_glob_deny_read_policy() -> anyhow::Result<()> {
|
||||
|
||||
@@ -18,6 +18,10 @@ path = "src/bin/setup_main/main.rs"
|
||||
name = "codex-command-runner"
|
||||
path = "src/bin/command_runner/main.rs"
|
||||
|
||||
[[bin]]
|
||||
name = "codex-windows-managed-deny-probe"
|
||||
path = "src/bin/managed_deny_probe/main.rs"
|
||||
|
||||
[lints]
|
||||
workspace = true
|
||||
|
||||
|
||||
@@ -0,0 +1,13 @@
|
||||
// Keep the ACL probe independent of test-harness startup, which writes CODEX_HOME.
|
||||
#[cfg(target_os = "windows")]
|
||||
mod win;
|
||||
|
||||
#[cfg(target_os = "windows")]
|
||||
fn main() {
|
||||
win::main();
|
||||
}
|
||||
|
||||
#[cfg(not(target_os = "windows"))]
|
||||
fn main() {
|
||||
panic!("codex-windows-managed-deny-probe is Windows-only");
|
||||
}
|
||||
@@ -0,0 +1,101 @@
|
||||
use std::ffi::c_void;
|
||||
use std::io::Write as _;
|
||||
use std::os::windows::ffi::OsStrExt;
|
||||
use std::path::Path;
|
||||
use std::path::PathBuf;
|
||||
|
||||
#[link(name = "kernel32")]
|
||||
unsafe extern "system" {
|
||||
#[link_name = "LoadLibraryW"]
|
||||
fn load_library_w(file_name: *const u16) -> *mut c_void;
|
||||
#[link_name = "FreeLibrary"]
|
||||
fn free_library(module: *mut c_void) -> i32;
|
||||
}
|
||||
|
||||
fn load_library(path: &Path) -> std::io::Result<()> {
|
||||
let wide_path: Vec<u16> = path
|
||||
.as_os_str()
|
||||
.encode_wide()
|
||||
.chain(std::iter::once(0))
|
||||
.collect();
|
||||
// SAFETY: `wide_path` is NUL-terminated and remains alive for the call.
|
||||
let module = unsafe { load_library_w(wide_path.as_ptr()) };
|
||||
if module.is_null() {
|
||||
return Err(std::io::Error::last_os_error());
|
||||
}
|
||||
// SAFETY: `module` is a non-null handle returned by `LoadLibraryW` above.
|
||||
if unsafe { free_library(module) } == 0 {
|
||||
return Err(std::io::Error::last_os_error());
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn access_was_denied(err: &std::io::Error) -> bool {
|
||||
err.kind() == std::io::ErrorKind::PermissionDenied || err.raw_os_error() == Some(5)
|
||||
}
|
||||
|
||||
fn probe_path(key: &str) -> PathBuf {
|
||||
let Some(value) = std::env::var_os(key) else {
|
||||
eprintln!("managed deny probe is missing {key}");
|
||||
probe_exit(/*code*/ 21);
|
||||
};
|
||||
PathBuf::from(value)
|
||||
}
|
||||
|
||||
fn probe_exit(code: i32) -> ! {
|
||||
let _ = std::io::stdout().flush();
|
||||
let _ = std::io::stderr().flush();
|
||||
std::process::exit(code);
|
||||
}
|
||||
|
||||
pub(super) fn main() {
|
||||
let allowed_text = probe_path("CODEX_WINDOWS_ALLOWED_TEXT");
|
||||
let denied_text = probe_path("CODEX_WINDOWS_DENIED_TEXT");
|
||||
let allowed_module = probe_path("CODEX_WINDOWS_ALLOWED_MODULE");
|
||||
let denied_module = probe_path("CODEX_WINDOWS_DENIED_MODULE");
|
||||
|
||||
match std::fs::read_to_string(&allowed_text) {
|
||||
Ok(contents) if contents.trim() == "ALLOW-CONTROL" => println!("allowed-read:OK"),
|
||||
Ok(_) => {
|
||||
eprintln!("allowed read returned unexpected contents");
|
||||
probe_exit(/*code*/ 21);
|
||||
}
|
||||
Err(err) => {
|
||||
eprintln!("allowed read failed: {err}");
|
||||
probe_exit(/*code*/ 21);
|
||||
}
|
||||
}
|
||||
if let Err(err) = load_library(&allowed_module) {
|
||||
eprintln!("allowed import failed: {err}");
|
||||
probe_exit(/*code*/ 21);
|
||||
}
|
||||
println!("allowed-import:OK");
|
||||
|
||||
let mut failures = 0;
|
||||
match std::fs::read(&denied_text) {
|
||||
Ok(_) => {
|
||||
println!("denied-read:UNEXPECTED_SUCCESS");
|
||||
failures += 1;
|
||||
}
|
||||
Err(err) if access_was_denied(&err) => println!("denied-read:DENIED"),
|
||||
Err(err) => {
|
||||
eprintln!("denied read failed for a non-access reason: {err}");
|
||||
probe_exit(/*code*/ 21);
|
||||
}
|
||||
}
|
||||
match load_library(&denied_module) {
|
||||
Ok(()) => {
|
||||
println!("denied-import:UNEXPECTED_SUCCESS");
|
||||
failures += 1;
|
||||
}
|
||||
Err(err) if access_was_denied(&err) => println!("denied-import:DENIED"),
|
||||
Err(err) => {
|
||||
eprintln!("denied import failed for a non-access reason: {err}");
|
||||
probe_exit(/*code*/ 21);
|
||||
}
|
||||
}
|
||||
|
||||
if failures != 0 {
|
||||
probe_exit(/*code*/ 20);
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user