mirror of
https://github.com/openai/codex.git
synced 2026-05-29 23:40:29 +00:00
Enforce Windows protected metadata targets
This commit is contained in:
@@ -39,6 +39,7 @@ mod windows_impl {
|
||||
use crate::logging::log_success;
|
||||
use crate::policy::SandboxPolicy;
|
||||
use crate::policy::parse_policy;
|
||||
use crate::protected_metadata::prepare_protected_metadata_targets;
|
||||
use crate::runner_client::spawn_runner_transport;
|
||||
use crate::sandbox_utils::ensure_codex_home_exists;
|
||||
use crate::sandbox_utils::inject_git_safe_directory;
|
||||
@@ -80,6 +81,8 @@ mod windows_impl {
|
||||
|
||||
let logs_base_dir: Option<&Path> = Some(sandbox_base.as_path());
|
||||
log_start(&command, logs_base_dir);
|
||||
let protected_metadata_guard =
|
||||
prepare_protected_metadata_targets(protected_metadata_targets);
|
||||
let sandbox_creds = require_logon_sandbox_creds(
|
||||
&policy,
|
||||
sandbox_policy_cwd,
|
||||
@@ -154,7 +157,7 @@ mod windows_impl {
|
||||
|
||||
let mut stdout = Vec::new();
|
||||
let mut stderr = Vec::new();
|
||||
let (exit_code, timed_out) = loop {
|
||||
let (mut exit_code, timed_out) = loop {
|
||||
let msg = read_frame(&mut pipe_read)?
|
||||
.ok_or_else(|| anyhow::anyhow!("runner pipe closed before exit"))?;
|
||||
match msg.message {
|
||||
@@ -178,6 +181,12 @@ mod windows_impl {
|
||||
}
|
||||
};
|
||||
|
||||
let protected_metadata_violations =
|
||||
protected_metadata_guard.cleanup_created_monitored_paths()?;
|
||||
if !protected_metadata_violations.is_empty() && exit_code == 0 {
|
||||
exit_code = 1;
|
||||
}
|
||||
|
||||
if exit_code == 0 {
|
||||
log_success(&command, logs_base_dir);
|
||||
} else {
|
||||
|
||||
@@ -170,6 +170,8 @@ pub use process::read_handle_loop;
|
||||
#[cfg(target_os = "windows")]
|
||||
pub use process::spawn_process_with_pipes;
|
||||
#[cfg(target_os = "windows")]
|
||||
pub use protected_metadata::protected_metadata_existing_deny_paths;
|
||||
#[cfg(target_os = "windows")]
|
||||
pub use session::spawn_windows_sandbox_session_elevated;
|
||||
#[cfg(target_os = "windows")]
|
||||
pub use session::spawn_windows_sandbox_session_legacy;
|
||||
@@ -272,6 +274,7 @@ mod windows_impl {
|
||||
use super::path_normalization::canonicalize_path;
|
||||
use super::policy::SandboxPolicy;
|
||||
use super::process::create_process_as_user;
|
||||
use super::protected_metadata::prepare_protected_metadata_targets;
|
||||
use super::sandbox_utils::ensure_codex_home_exists;
|
||||
use super::spawn_prep::prepare_legacy_spawn_context;
|
||||
use super::token::convert_string_sid_to_sid;
|
||||
@@ -366,7 +369,7 @@ mod windows_impl {
|
||||
mut env_map: HashMap<String, String>,
|
||||
timeout_ms: Option<u64>,
|
||||
additional_deny_write_paths: &[PathBuf],
|
||||
_protected_metadata_targets: &[ProtectedMetadataTarget],
|
||||
protected_metadata_targets: &[ProtectedMetadataTarget],
|
||||
use_private_desktop: bool,
|
||||
) -> Result<CaptureResult> {
|
||||
let common = prepare_legacy_spawn_context(
|
||||
@@ -436,6 +439,11 @@ mod windows_impl {
|
||||
let persist_aces = is_workspace_write;
|
||||
let AllowDenyPaths { allow, mut deny } =
|
||||
compute_allow_paths(&policy, sandbox_policy_cwd, ¤t_dir, &env_map);
|
||||
let protected_metadata_guard =
|
||||
prepare_protected_metadata_targets(protected_metadata_targets);
|
||||
for path in protected_metadata_guard.deny_paths() {
|
||||
deny.insert(path.clone());
|
||||
}
|
||||
for path in additional_deny_write_paths {
|
||||
if path.exists() {
|
||||
deny.insert(path.clone());
|
||||
@@ -586,11 +594,16 @@ mod windows_impl {
|
||||
let _ = t_err.join();
|
||||
let stdout = rx_out.recv().unwrap_or_default();
|
||||
let stderr = rx_err.recv().unwrap_or_default();
|
||||
let exit_code = if timed_out {
|
||||
let mut exit_code = if timed_out {
|
||||
128 + 64
|
||||
} else {
|
||||
exit_code_u32 as i32
|
||||
};
|
||||
let protected_metadata_violations =
|
||||
protected_metadata_guard.cleanup_created_monitored_paths()?;
|
||||
if !protected_metadata_violations.is_empty() && exit_code == 0 {
|
||||
exit_code = 1;
|
||||
}
|
||||
|
||||
if exit_code == 0 {
|
||||
log_success(&command, logs_base_dir);
|
||||
|
||||
@@ -1,5 +1,3 @@
|
||||
#![allow(dead_code)]
|
||||
|
||||
use crate::setup::ProtectedMetadataMode;
|
||||
use crate::setup::ProtectedMetadataTarget;
|
||||
use anyhow::Context;
|
||||
|
||||
@@ -8,6 +8,7 @@ use base64::Engine;
|
||||
use base64::engine::general_purpose::STANDARD as BASE64;
|
||||
use codex_otel::StatsigMetricsSettings;
|
||||
use codex_windows_sandbox::LOG_FILE_NAME;
|
||||
use codex_windows_sandbox::ProtectedMetadataMode;
|
||||
use codex_windows_sandbox::ProtectedMetadataTarget;
|
||||
use codex_windows_sandbox::SETUP_VERSION;
|
||||
use codex_windows_sandbox::SetupErrorCode;
|
||||
@@ -25,6 +26,7 @@ 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;
|
||||
use codex_windows_sandbox::protected_metadata_existing_deny_paths;
|
||||
use codex_windows_sandbox::sandbox_bin_dir;
|
||||
use codex_windows_sandbox::sandbox_dir;
|
||||
use codex_windows_sandbox::sandbox_secrets_dir;
|
||||
@@ -88,7 +90,6 @@ struct Payload {
|
||||
write_roots: Vec<PathBuf>,
|
||||
#[serde(default)]
|
||||
deny_write_paths: Vec<PathBuf>,
|
||||
#[allow(dead_code)]
|
||||
#[serde(default)]
|
||||
protected_metadata_targets: Vec<ProtectedMetadataTarget>,
|
||||
proxy_ports: Vec<u16>,
|
||||
@@ -820,6 +821,66 @@ fn run_setup_full(payload: &Payload, log: &mut File, sbx_dir: &Path) -> Result<(
|
||||
}
|
||||
}
|
||||
|
||||
for target in &payload.protected_metadata_targets {
|
||||
if !matches!(target.mode, ProtectedMetadataMode::ExistingDeny) {
|
||||
continue;
|
||||
}
|
||||
let deny_paths = protected_metadata_existing_deny_paths(&target.path);
|
||||
if deny_paths.is_empty() {
|
||||
log_line(
|
||||
log,
|
||||
&format!(
|
||||
"protected metadata {} missing during setup; skipping",
|
||||
target.path.display()
|
||||
),
|
||||
)?;
|
||||
continue;
|
||||
}
|
||||
|
||||
for path in deny_paths {
|
||||
if !seen_deny_paths.insert(path.clone()) {
|
||||
continue;
|
||||
}
|
||||
if std::fs::symlink_metadata(&path).is_err() {
|
||||
log_line(
|
||||
log,
|
||||
&format!(
|
||||
"protected metadata {} missing during setup; skipping",
|
||||
path.display()
|
||||
),
|
||||
)?;
|
||||
continue;
|
||||
}
|
||||
|
||||
let canonical_path = canonicalize_path(&path);
|
||||
let deny_psid = if canonical_path.starts_with(&canonical_command_cwd) {
|
||||
workspace_psid
|
||||
} else {
|
||||
cap_psid
|
||||
};
|
||||
|
||||
match unsafe { add_deny_write_ace(&path, deny_psid) } {
|
||||
Ok(true) => {
|
||||
log_line(
|
||||
log,
|
||||
&format!("applied deny ACE to protect metadata {}", path.display()),
|
||||
)?;
|
||||
}
|
||||
Ok(false) => {}
|
||||
Err(err) => {
|
||||
refresh_errors.push(format!(
|
||||
"metadata deny ACE failed on {}: {err}",
|
||||
path.display()
|
||||
));
|
||||
log_line(
|
||||
log,
|
||||
&format!("metadata deny ACE failed on {}: {err}", path.display()),
|
||||
)?;
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
lock_sandbox_dir(
|
||||
&sandbox_bin_dir(&payload.codex_home),
|
||||
&payload.real_user,
|
||||
|
||||
@@ -212,6 +212,7 @@ pub(crate) fn allow_null_device_for_workspace_write(is_workspace_write: bool) {
|
||||
}
|
||||
}
|
||||
|
||||
#[allow(clippy::too_many_arguments)]
|
||||
pub(crate) fn apply_legacy_session_acl_rules(
|
||||
policy: &SandboxPolicy,
|
||||
sandbox_policy_cwd: &Path,
|
||||
@@ -220,9 +221,11 @@ pub(crate) fn apply_legacy_session_acl_rules(
|
||||
psid_generic: &LocalSid,
|
||||
psid_workspace: Option<&LocalSid>,
|
||||
persist_aces: bool,
|
||||
additional_deny_paths: &[PathBuf],
|
||||
) -> Vec<PathBuf> {
|
||||
let AllowDenyPaths { allow, deny } =
|
||||
let AllowDenyPaths { allow, mut deny } =
|
||||
compute_allow_paths(policy, sandbox_policy_cwd, current_dir, env_map);
|
||||
deny.extend(additional_deny_paths.iter().cloned());
|
||||
let mut guards: Vec<PathBuf> = Vec::new();
|
||||
let canonical_cwd = canonicalize_path(current_dir);
|
||||
unsafe {
|
||||
|
||||
@@ -7,6 +7,7 @@ use crate::ipc_framed::EmptyPayload;
|
||||
use crate::ipc_framed::FramedMessage;
|
||||
use crate::ipc_framed::Message;
|
||||
use crate::ipc_framed::SpawnRequest;
|
||||
use crate::protected_metadata::prepare_protected_metadata_targets;
|
||||
use crate::runner_client::spawn_runner_transport;
|
||||
use crate::setup::ProtectedMetadataTarget;
|
||||
use crate::spawn_prep::prepare_elevated_spawn_context;
|
||||
@@ -43,6 +44,7 @@ pub(crate) async fn spawn_windows_sandbox_session_elevated(
|
||||
protected_metadata_targets,
|
||||
)?;
|
||||
|
||||
let protected_metadata_guard = prepare_protected_metadata_targets(protected_metadata_targets);
|
||||
let spawn_request = SpawnRequest {
|
||||
command: command.clone(),
|
||||
cwd: cwd.to_path_buf(),
|
||||
@@ -102,6 +104,7 @@ pub(crate) async fn spawn_windows_sandbox_session_elevated(
|
||||
stdout_tx,
|
||||
stderr_rx.as_ref().map(|(tx, _rx)| tx.clone()),
|
||||
exit_tx,
|
||||
Some(protected_metadata_guard),
|
||||
);
|
||||
|
||||
Ok(finish_driver_spawn(
|
||||
|
||||
@@ -10,6 +10,8 @@ use crate::process::StderrMode;
|
||||
use crate::process::StdinMode;
|
||||
use crate::process::read_handle_loop;
|
||||
use crate::process::spawn_process_with_pipes;
|
||||
use crate::protected_metadata::ProtectedMetadataGuard;
|
||||
use crate::protected_metadata::prepare_protected_metadata_targets;
|
||||
use crate::setup::ProtectedMetadataTarget;
|
||||
use crate::spawn_prep::LocalSid;
|
||||
use crate::spawn_prep::allow_null_device_for_workspace_write;
|
||||
@@ -206,10 +208,11 @@ fn finalize_exit(
|
||||
output_join: std::thread::JoinHandle<()>,
|
||||
guards: Vec<PathBuf>,
|
||||
cap_sid: Option<String>,
|
||||
protected_metadata_guard: ProtectedMetadataGuard,
|
||||
logs_base_dir: Option<&Path>,
|
||||
command: Vec<String>,
|
||||
) {
|
||||
let exit_code = {
|
||||
let mut exit_code = {
|
||||
let mut raw_exit = 1u32;
|
||||
if let Ok(guard) = process_handle.lock()
|
||||
&& let Some(handle) = guard.as_ref()
|
||||
@@ -223,6 +226,21 @@ fn finalize_exit(
|
||||
};
|
||||
|
||||
let _ = output_join.join();
|
||||
let protected_metadata_failure =
|
||||
match protected_metadata_guard.cleanup_created_monitored_paths() {
|
||||
Ok(paths) => {
|
||||
if !paths.is_empty() && exit_code == 0 {
|
||||
exit_code = 1;
|
||||
}
|
||||
None
|
||||
}
|
||||
Err(err) => {
|
||||
if exit_code == 0 {
|
||||
exit_code = 1;
|
||||
}
|
||||
Some(format!("protected metadata cleanup failed: {err:#}"))
|
||||
}
|
||||
};
|
||||
let _ = exit_tx.send(exit_code);
|
||||
|
||||
unsafe {
|
||||
@@ -236,7 +254,9 @@ fn finalize_exit(
|
||||
}
|
||||
}
|
||||
|
||||
if exit_code == 0 {
|
||||
if let Some(message) = protected_metadata_failure {
|
||||
log_failure(&command, &message, logs_base_dir);
|
||||
} else if exit_code == 0 {
|
||||
log_success(&command, logs_base_dir);
|
||||
} else {
|
||||
log_failure(&command, &format!("exit code {exit_code}"), logs_base_dir);
|
||||
@@ -290,7 +310,7 @@ pub(crate) async fn spawn_windows_sandbox_session_legacy(
|
||||
timeout_ms: Option<u64>,
|
||||
tty: bool,
|
||||
stdin_open: bool,
|
||||
_protected_metadata_targets: &[ProtectedMetadataTarget],
|
||||
protected_metadata_targets: &[ProtectedMetadataTarget],
|
||||
use_private_desktop: bool,
|
||||
) -> Result<SpawnedProcess> {
|
||||
let common = prepare_legacy_spawn_context(
|
||||
@@ -309,6 +329,9 @@ pub(crate) async fn spawn_windows_sandbox_session_legacy(
|
||||
allow_null_device_for_workspace_write(common.is_workspace_write);
|
||||
|
||||
let persist_aces = common.is_workspace_write;
|
||||
let protected_metadata_guard = prepare_protected_metadata_targets(protected_metadata_targets);
|
||||
let additional_deny_write_paths: Vec<PathBuf> =
|
||||
protected_metadata_guard.deny_paths().cloned().collect();
|
||||
let guards = apply_legacy_session_acl_rules(
|
||||
&common.policy,
|
||||
sandbox_policy_cwd,
|
||||
@@ -317,6 +340,7 @@ pub(crate) async fn spawn_windows_sandbox_session_legacy(
|
||||
&security.psid_generic,
|
||||
security.psid_workspace.as_ref(),
|
||||
persist_aces,
|
||||
&additional_deny_write_paths,
|
||||
);
|
||||
|
||||
let (writer_tx, writer_rx) = mpsc::channel::<Vec<u8>>(128);
|
||||
@@ -408,6 +432,7 @@ pub(crate) async fn spawn_windows_sandbox_session_legacy(
|
||||
output_join,
|
||||
guards_for_wait,
|
||||
cap_sid_for_wait,
|
||||
protected_metadata_guard,
|
||||
common.logs_base_dir.as_deref(),
|
||||
command_for_wait,
|
||||
);
|
||||
|
||||
@@ -6,6 +6,7 @@ use crate::ipc_framed::ResizePayload;
|
||||
use crate::ipc_framed::StdinPayload;
|
||||
use crate::ipc_framed::decode_bytes;
|
||||
use crate::ipc_framed::encode_bytes;
|
||||
use crate::protected_metadata::ProtectedMetadataGuard;
|
||||
use anyhow::Result;
|
||||
use codex_utils_pty::ProcessDriver;
|
||||
use codex_utils_pty::SpawnedProcess;
|
||||
@@ -97,6 +98,7 @@ pub(crate) fn start_runner_stdout_reader(
|
||||
stdout_tx: broadcast::Sender<Vec<u8>>,
|
||||
stderr_tx: Option<broadcast::Sender<Vec<u8>>>,
|
||||
exit_tx: oneshot::Sender<i32>,
|
||||
protected_metadata_guard: Option<ProtectedMetadataGuard>,
|
||||
) {
|
||||
std::thread::spawn(move || {
|
||||
loop {
|
||||
@@ -140,7 +142,27 @@ pub(crate) fn start_runner_stdout_reader(
|
||||
}
|
||||
}
|
||||
Message::Exit { payload } => {
|
||||
let _ = exit_tx.send(payload.exit_code);
|
||||
let mut exit_code = payload.exit_code;
|
||||
if let Some(protected_metadata_guard) = protected_metadata_guard {
|
||||
match protected_metadata_guard.cleanup_created_monitored_paths() {
|
||||
Ok(paths) => {
|
||||
if !paths.is_empty() && exit_code == 0 {
|
||||
exit_code = 1;
|
||||
}
|
||||
}
|
||||
Err(err) => {
|
||||
send_runner_error(
|
||||
&format!("protected metadata cleanup failed: {err:#}"),
|
||||
&stdout_tx,
|
||||
stderr_tx.as_ref(),
|
||||
);
|
||||
if exit_code == 0 {
|
||||
exit_code = 1;
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
let _ = exit_tx.send(exit_code);
|
||||
break;
|
||||
}
|
||||
Message::Error { payload } => {
|
||||
|
||||
Reference in New Issue
Block a user