mirror of
https://github.com/openai/codex.git
synced 2026-09-13 11:47:17 +00:00
fix windows sandbox write root capability scope
This commit is contained in:
@@ -82,15 +82,21 @@ pub use identity::require_logon_sandbox_creds;
|
||||
#[cfg(target_os = "windows")]
|
||||
pub use identity::sandbox_setup_is_complete;
|
||||
#[cfg(target_os = "windows")]
|
||||
pub use logging::log_note;
|
||||
#[cfg(target_os = "windows")]
|
||||
pub use logging::LOG_FILE_NAME;
|
||||
#[cfg(target_os = "windows")]
|
||||
pub use logging::log_note;
|
||||
#[cfg(target_os = "windows")]
|
||||
pub use path_normalization::canonicalize_path;
|
||||
#[cfg(target_os = "windows")]
|
||||
pub use policy::SandboxPolicy;
|
||||
#[cfg(target_os = "windows")]
|
||||
pub use policy::parse_policy;
|
||||
#[cfg(target_os = "windows")]
|
||||
pub use policy::SandboxPolicy;
|
||||
pub use process::PipeSpawnHandles;
|
||||
#[cfg(target_os = "windows")]
|
||||
pub use process::StderrMode;
|
||||
#[cfg(target_os = "windows")]
|
||||
pub use process::StdinMode;
|
||||
#[cfg(target_os = "windows")]
|
||||
pub use process::create_process_as_user;
|
||||
#[cfg(target_os = "windows")]
|
||||
@@ -98,11 +104,7 @@ pub use process::read_handle_loop;
|
||||
#[cfg(target_os = "windows")]
|
||||
pub use process::spawn_process_with_pipes;
|
||||
#[cfg(target_os = "windows")]
|
||||
pub use process::PipeSpawnHandles;
|
||||
#[cfg(target_os = "windows")]
|
||||
pub use process::StderrMode;
|
||||
#[cfg(target_os = "windows")]
|
||||
pub use process::StdinMode;
|
||||
pub use setup::SETUP_VERSION;
|
||||
#[cfg(target_os = "windows")]
|
||||
pub use setup::run_elevated_setup;
|
||||
#[cfg(target_os = "windows")]
|
||||
@@ -116,7 +118,11 @@ pub use setup::sandbox_dir;
|
||||
#[cfg(target_os = "windows")]
|
||||
pub use setup::sandbox_secrets_dir;
|
||||
#[cfg(target_os = "windows")]
|
||||
pub use setup::SETUP_VERSION;
|
||||
pub use setup_error::SetupErrorCode;
|
||||
#[cfg(target_os = "windows")]
|
||||
pub use setup_error::SetupErrorReport;
|
||||
#[cfg(target_os = "windows")]
|
||||
pub use setup_error::SetupFailure;
|
||||
#[cfg(target_os = "windows")]
|
||||
pub use setup_error::extract_failure as extract_setup_failure;
|
||||
#[cfg(target_os = "windows")]
|
||||
@@ -126,12 +132,6 @@ pub use setup_error::setup_error_path;
|
||||
#[cfg(target_os = "windows")]
|
||||
pub use setup_error::write_setup_error_report;
|
||||
#[cfg(target_os = "windows")]
|
||||
pub use setup_error::SetupErrorCode;
|
||||
#[cfg(target_os = "windows")]
|
||||
pub use setup_error::SetupErrorReport;
|
||||
#[cfg(target_os = "windows")]
|
||||
pub use setup_error::SetupFailure;
|
||||
#[cfg(target_os = "windows")]
|
||||
pub use token::convert_string_sid_to_sid;
|
||||
#[cfg(target_os = "windows")]
|
||||
pub use token::create_readonly_token_with_cap_from;
|
||||
@@ -142,12 +142,12 @@ pub use token::create_workspace_write_token_with_caps_from;
|
||||
#[cfg(target_os = "windows")]
|
||||
pub use token::get_current_token_for_restriction;
|
||||
#[cfg(target_os = "windows")]
|
||||
pub use windows_impl::CaptureResult;
|
||||
#[cfg(target_os = "windows")]
|
||||
pub use windows_impl::run_windows_sandbox_capture;
|
||||
#[cfg(target_os = "windows")]
|
||||
pub use windows_impl::run_windows_sandbox_legacy_preflight;
|
||||
#[cfg(target_os = "windows")]
|
||||
pub use windows_impl::CaptureResult;
|
||||
#[cfg(target_os = "windows")]
|
||||
pub use winutil::quote_windows_arg;
|
||||
#[cfg(target_os = "windows")]
|
||||
pub use winutil::string_from_sid_bytes;
|
||||
@@ -160,14 +160,14 @@ pub use workspace_acl::protect_workspace_agents_dir;
|
||||
#[cfg(target_os = "windows")]
|
||||
pub use workspace_acl::protect_workspace_codex_dir;
|
||||
|
||||
#[cfg(not(target_os = "windows"))]
|
||||
pub use stub::CaptureResult;
|
||||
#[cfg(not(target_os = "windows"))]
|
||||
pub use stub::apply_world_writable_scan_and_denies;
|
||||
#[cfg(not(target_os = "windows"))]
|
||||
pub use stub::run_windows_sandbox_capture;
|
||||
#[cfg(not(target_os = "windows"))]
|
||||
pub use stub::run_windows_sandbox_legacy_preflight;
|
||||
#[cfg(not(target_os = "windows"))]
|
||||
pub use stub::CaptureResult;
|
||||
|
||||
#[cfg(target_os = "windows")]
|
||||
mod windows_impl {
|
||||
@@ -175,8 +175,8 @@ mod windows_impl {
|
||||
use super::acl::add_deny_write_ace;
|
||||
use super::acl::allow_null_device;
|
||||
use super::acl::revoke_ace;
|
||||
use super::allow::compute_allow_paths;
|
||||
use super::allow::AllowDenyPaths;
|
||||
use super::allow::compute_allow_paths;
|
||||
use super::cap::load_or_create_cap_sids;
|
||||
use super::cap::workspace_cap_sid_for_cwd;
|
||||
use super::env::apply_no_network_to_env;
|
||||
@@ -186,12 +186,11 @@ mod windows_impl {
|
||||
use super::logging::log_start;
|
||||
use super::logging::log_success;
|
||||
use super::path_normalization::canonicalize_path;
|
||||
use super::policy::parse_policy;
|
||||
use super::policy::SandboxPolicy;
|
||||
use super::policy::parse_policy;
|
||||
use super::process::create_process_as_user;
|
||||
use super::token::convert_string_sid_to_sid;
|
||||
use super::token::create_workspace_write_token_with_caps_from;
|
||||
use super::workspace_acl::is_command_cwd_root;
|
||||
use super::workspace_acl::protect_workspace_agents_dir;
|
||||
use super::workspace_acl::protect_workspace_codex_dir;
|
||||
use anyhow::Result;
|
||||
@@ -203,13 +202,13 @@ mod windows_impl {
|
||||
use std::ptr;
|
||||
use windows_sys::Win32::Foundation::CloseHandle;
|
||||
use windows_sys::Win32::Foundation::GetLastError;
|
||||
use windows_sys::Win32::Foundation::SetHandleInformation;
|
||||
use windows_sys::Win32::Foundation::HANDLE;
|
||||
use windows_sys::Win32::Foundation::HANDLE_FLAG_INHERIT;
|
||||
use windows_sys::Win32::Foundation::SetHandleInformation;
|
||||
use windows_sys::Win32::System::Pipes::CreatePipe;
|
||||
use windows_sys::Win32::System::Threading::GetExitCodeProcess;
|
||||
use windows_sys::Win32::System::Threading::WaitForSingleObject;
|
||||
use windows_sys::Win32::System::Threading::INFINITE;
|
||||
use windows_sys::Win32::System::Threading::WaitForSingleObject;
|
||||
|
||||
type PipeHandles = ((HANDLE, HANDLE), (HANDLE, HANDLE), (HANDLE, HANDLE));
|
||||
|
||||
@@ -337,11 +336,12 @@ mod windows_impl {
|
||||
let persist_aces = is_workspace_write;
|
||||
let AllowDenyPaths { allow, deny } =
|
||||
compute_allow_paths(&policy, sandbox_policy_cwd, ¤t_dir, &env_map);
|
||||
let canonical_cwd = canonicalize_path(¤t_dir);
|
||||
let mut guards: Vec<(PathBuf, *mut c_void)> = Vec::new();
|
||||
unsafe {
|
||||
for p in &allow {
|
||||
let psid = if is_workspace_write && is_command_cwd_root(p, &canonical_cwd) {
|
||||
// Scope every writable root to the current workspace so one workspace cannot
|
||||
// persist access for a different workspace via the shared generic capability.
|
||||
let psid = if is_workspace_write {
|
||||
psid_workspace.unwrap_or(psid_generic)
|
||||
} else {
|
||||
psid_generic
|
||||
@@ -534,16 +534,10 @@ mod windows_impl {
|
||||
let current_dir = cwd.to_path_buf();
|
||||
let AllowDenyPaths { allow, deny } =
|
||||
compute_allow_paths(sandbox_policy, sandbox_policy_cwd, ¤t_dir, env_map);
|
||||
let canonical_cwd = canonicalize_path(¤t_dir);
|
||||
|
||||
unsafe {
|
||||
for p in &allow {
|
||||
let psid = if is_command_cwd_root(p, &canonical_cwd) {
|
||||
psid_workspace
|
||||
} else {
|
||||
psid_generic
|
||||
};
|
||||
let _ = add_allow_ace(p, psid);
|
||||
let _ = add_allow_ace(p, psid_workspace);
|
||||
}
|
||||
for p in &deny {
|
||||
let _ = add_deny_write_ace(p, psid_generic);
|
||||
@@ -593,8 +587,8 @@ mod windows_impl {
|
||||
|
||||
#[cfg(not(target_os = "windows"))]
|
||||
mod stub {
|
||||
use anyhow::bail;
|
||||
use anyhow::Result;
|
||||
use anyhow::bail;
|
||||
use codex_protocol::protocol::SandboxPolicy;
|
||||
use std::collections::HashMap;
|
||||
use std::path::Path;
|
||||
|
||||
@@ -4,15 +4,18 @@ mod firewall;
|
||||
|
||||
use anyhow::Context;
|
||||
use anyhow::Result;
|
||||
use base64::engine::general_purpose::STANDARD as BASE64;
|
||||
use base64::Engine;
|
||||
use codex_windows_sandbox::canonicalize_path;
|
||||
use base64::engine::general_purpose::STANDARD as BASE64;
|
||||
use codex_windows_sandbox::LOG_FILE_NAME;
|
||||
use codex_windows_sandbox::SETUP_VERSION;
|
||||
use codex_windows_sandbox::SetupErrorCode;
|
||||
use codex_windows_sandbox::SetupErrorReport;
|
||||
use codex_windows_sandbox::SetupFailure;
|
||||
use codex_windows_sandbox::convert_string_sid_to_sid;
|
||||
use codex_windows_sandbox::ensure_allow_mask_aces_with_inheritance;
|
||||
use codex_windows_sandbox::ensure_allow_write_aces;
|
||||
use codex_windows_sandbox::extract_setup_failure;
|
||||
use codex_windows_sandbox::hide_newly_created_users;
|
||||
use codex_windows_sandbox::is_command_cwd_root;
|
||||
use codex_windows_sandbox::load_or_create_cap_sids;
|
||||
use codex_windows_sandbox::log_note;
|
||||
use codex_windows_sandbox::path_mask_allows;
|
||||
@@ -25,16 +28,11 @@ use codex_windows_sandbox::string_from_sid_bytes;
|
||||
use codex_windows_sandbox::to_wide;
|
||||
use codex_windows_sandbox::workspace_cap_sid_for_cwd;
|
||||
use codex_windows_sandbox::write_setup_error_report;
|
||||
use codex_windows_sandbox::SetupErrorCode;
|
||||
use codex_windows_sandbox::SetupErrorReport;
|
||||
use codex_windows_sandbox::SetupFailure;
|
||||
use codex_windows_sandbox::LOG_FILE_NAME;
|
||||
use codex_windows_sandbox::SETUP_VERSION;
|
||||
use serde::Deserialize;
|
||||
use serde::Serialize;
|
||||
use std::collections::HashSet;
|
||||
use std::ffi::c_void;
|
||||
use std::ffi::OsStr;
|
||||
use std::ffi::c_void;
|
||||
use std::fs::File;
|
||||
use std::io::Write;
|
||||
use std::os::windows::process::CommandExt;
|
||||
@@ -44,17 +42,17 @@ use std::process::Command;
|
||||
use std::process::Stdio;
|
||||
use std::sync::mpsc;
|
||||
use windows_sys::Win32::Foundation::GetLastError;
|
||||
use windows_sys::Win32::Foundation::LocalFree;
|
||||
use windows_sys::Win32::Foundation::HLOCAL;
|
||||
use windows_sys::Win32::Foundation::LocalFree;
|
||||
use windows_sys::Win32::Security::ACL;
|
||||
use windows_sys::Win32::Security::Authorization::ConvertStringSidToSidW;
|
||||
use windows_sys::Win32::Security::Authorization::SetEntriesInAclW;
|
||||
use windows_sys::Win32::Security::Authorization::SetNamedSecurityInfoW;
|
||||
use windows_sys::Win32::Security::Authorization::EXPLICIT_ACCESS_W;
|
||||
use windows_sys::Win32::Security::Authorization::GRANT_ACCESS;
|
||||
use windows_sys::Win32::Security::Authorization::SE_FILE_OBJECT;
|
||||
use windows_sys::Win32::Security::Authorization::SetEntriesInAclW;
|
||||
use windows_sys::Win32::Security::Authorization::SetNamedSecurityInfoW;
|
||||
use windows_sys::Win32::Security::Authorization::TRUSTEE_IS_SID;
|
||||
use windows_sys::Win32::Security::Authorization::TRUSTEE_W;
|
||||
use windows_sys::Win32::Security::ACL;
|
||||
use windows_sys::Win32::Security::CONTAINER_INHERIT_ACE;
|
||||
use windows_sys::Win32::Security::DACL_SECURITY_INFORMATION;
|
||||
use windows_sys::Win32::Security::OBJECT_INHERIT_ACE;
|
||||
@@ -616,7 +614,6 @@ fn run_setup_full(payload: &Payload, log: &mut File, sbx_dir: &Path) -> Result<(
|
||||
}
|
||||
}
|
||||
|
||||
let cap_sid_str = caps.workspace.clone();
|
||||
let sandbox_group_sid_str =
|
||||
string_from_sid_bytes(&sandbox_group_sid).map_err(anyhow::Error::msg)?;
|
||||
let write_mask =
|
||||
@@ -624,8 +621,6 @@ fn run_setup_full(payload: &Payload, log: &mut File, sbx_dir: &Path) -> Result<(
|
||||
let mut grant_tasks: Vec<PathBuf> = Vec::new();
|
||||
|
||||
let mut seen_write_roots: HashSet<PathBuf> = HashSet::new();
|
||||
let canonical_command_cwd = canonicalize_path(&payload.command_cwd);
|
||||
|
||||
for root in &payload.write_roots {
|
||||
if !seen_write_roots.insert(root.clone()) {
|
||||
continue;
|
||||
@@ -638,20 +633,9 @@ fn run_setup_full(payload: &Payload, log: &mut File, sbx_dir: &Path) -> Result<(
|
||||
continue;
|
||||
}
|
||||
let mut need_grant = false;
|
||||
let is_command_cwd = is_command_cwd_root(root, &canonical_command_cwd);
|
||||
let cap_label = if is_command_cwd {
|
||||
"workspace_cap"
|
||||
} else {
|
||||
"cap"
|
||||
};
|
||||
let cap_psid_for_root = if is_command_cwd {
|
||||
workspace_psid
|
||||
} else {
|
||||
cap_psid
|
||||
};
|
||||
for (label, psid) in [
|
||||
("sandbox_group", sandbox_group_psid),
|
||||
(cap_label, cap_psid_for_root),
|
||||
("workspace_cap", workspace_psid),
|
||||
] {
|
||||
let has =
|
||||
match path_mask_allows(root, &[psid], write_mask, /*require_all_bits*/ true) {
|
||||
@@ -692,12 +676,7 @@ fn run_setup_full(payload: &Payload, log: &mut File, sbx_dir: &Path) -> Result<(
|
||||
let (tx, rx) = mpsc::channel::<(PathBuf, Result<bool>)>();
|
||||
std::thread::scope(|scope| {
|
||||
for root in grant_tasks {
|
||||
let is_command_cwd = is_command_cwd_root(&root, &canonical_command_cwd);
|
||||
let sid_strings = if is_command_cwd {
|
||||
vec![sandbox_group_sid_str.clone(), workspace_sid_str.clone()]
|
||||
} else {
|
||||
vec![sandbox_group_sid_str.clone(), cap_sid_str.clone()]
|
||||
};
|
||||
let sid_strings = vec![sandbox_group_sid_str.clone(), workspace_sid_str.clone()];
|
||||
let tx = tx.clone();
|
||||
scope.spawn(move || {
|
||||
// Convert SID strings to psids locally in this thread.
|
||||
|
||||
Reference in New Issue
Block a user