Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 42 additions & 2 deletions codex-rs/windows-sandbox-rs/src/elevated/runner_client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ use std::time::Instant;
use windows_sys::Win32::Foundation::CloseHandle;
use windows_sys::Win32::Foundation::DUPLICATE_SAME_ACCESS;
use windows_sys::Win32::Foundation::DuplicateHandle;
use windows_sys::Win32::Foundation::ERROR_LOGON_FAILURE;
use windows_sys::Win32::Foundation::ERROR_NOT_FOUND;
use windows_sys::Win32::Foundation::GetLastError;
use windows_sys::Win32::Foundation::HANDLE;
Expand All @@ -49,11 +50,29 @@ const RUNNER_SPAWN_READY_POLL_INTERVAL: Duration = Duration::from_millis(50);
const RUNNER_ERROR_MODE_FLAGS: u32 = 0x0001 | 0x0002;
const WAIT_OBJECT_0: u32 = 0;

#[derive(Debug)]
struct RunnerLogonError {
code: u32,
}

impl std::fmt::Display for RunnerLogonError {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
write!(f, "CreateProcessWithLogonW failed: {}", self.code)
}
}

impl std::error::Error for RunnerLogonError {}

pub(crate) struct RunnerTransport {
pipe_write: File,
pipe_read: File,
}

pub(crate) fn is_stale_sandbox_creds_error(err: &anyhow::Error) -> bool {
err.downcast_ref::<RunnerLogonError>()
.is_some_and(|err| err.code == ERROR_LOGON_FAILURE)
}

impl RunnerTransport {
pub(crate) fn send_spawn_request(&mut self, request: SpawnRequest) -> Result<()> {
let spawn_request = FramedMessage {
Expand Down Expand Up @@ -275,12 +294,12 @@ pub(crate) fn spawn_runner_transport(
SetErrorMode(previous_error_mode);
}
if spawn_res == 0 {
let err = unsafe { GetLastError() } as i32;
let err = unsafe { GetLastError() };
unsafe {
CloseHandle(h_pipe_in);
CloseHandle(h_pipe_out);
}
return Err(anyhow::anyhow!("CreateProcessWithLogonW failed: {err}"));
return Err(RunnerLogonError { code: err }.into());
}
let expected_runner_pid = pi.dwProcessId;

Expand Down Expand Up @@ -393,3 +412,24 @@ fn wait_for_complete_frame(pipe_read: &File, timeout: Duration) -> Result<()> {
std::thread::sleep(RUNNER_SPAWN_READY_POLL_INTERVAL);
}
}

#[cfg(test)]
mod tests {
use super::RunnerLogonError;
use super::is_stale_sandbox_creds_error;
use pretty_assertions::assert_eq;
use windows_sys::Win32::Foundation::ERROR_LOGON_FAILURE;
use windows_sys::Win32::Foundation::ERROR_NOT_FOUND;

#[test]
fn stale_sandbox_creds_error_recognizes_logon_failures() {
assert_eq!(
[ERROR_LOGON_FAILURE, ERROR_NOT_FOUND].map(|code| {
let err =
anyhow::Error::new(RunnerLogonError { code }).context("runner launch failed");
is_stale_sandbox_creds_error(&err)
}),
[true, false]
);
}
}
34 changes: 30 additions & 4 deletions codex-rs/windows-sandbox-rs/src/elevated_impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ mod windows_impl {
use crate::env::ensure_non_interactive_pager;
use crate::env::inherit_path_env;
use crate::env::normalize_null_device_env;
use crate::identity::refresh_logon_sandbox_creds;
use crate::identity::require_logon_sandbox_creds;
use crate::ipc_framed::EmptyPayload;
use crate::ipc_framed::FramedMessage;
Expand All @@ -43,6 +44,7 @@ mod windows_impl {
use crate::logging::log_start;
use crate::logging::log_success;
use crate::resolved_permissions::ResolvedWindowsSandboxPermissions;
use crate::runner_client::is_stale_sandbox_creds_error;
use crate::runner_client::spawn_runner_transport;
use crate::sandbox_utils::ensure_codex_home_exists;
use crate::sandbox_utils::inject_git_safe_directory;
Expand Down Expand Up @@ -137,7 +139,7 @@ mod windows_impl {

let logs_base_dir: Option<&Path> = Some(sandbox_base.as_path());
log_start(&command, logs_base_dir);
let sandbox_creds = require_logon_sandbox_creds(
let mut sandbox_creds = require_logon_sandbox_creds(
&permissions,
cwd,
&env_map,
Expand Down Expand Up @@ -192,13 +194,37 @@ mod windows_impl {
stdin_open: false,
use_private_desktop,
};
let transport = spawn_runner_transport(
let transport = match spawn_runner_transport(
codex_home,
cwd,
&sandbox_creds,
logs_base_dir,
spawn_request,
)?;
spawn_request.clone(),
) {
Ok(transport) => transport,
Err(err) if is_stale_sandbox_creds_error(&err) => {
sandbox_creds = refresh_logon_sandbox_creds(
&permissions,
cwd,
&env_map,
codex_home,
read_roots_override,
read_roots_include_platform_defaults,
write_roots_override,
&deny_read_paths_override,
&deny_write_paths_override,
proxy_enforced,
)?;
spawn_runner_transport(
codex_home,
cwd,
&sandbox_creds,
logs_base_dir,
spawn_request,
)?
}
Err(err) => return Err(err),
};
let (pipe_write, mut pipe_read) = transport.into_files();
let cancel_writer = spawn_cancel_writer(&pipe_write, cancellation)?;

Expand Down
70 changes: 70 additions & 0 deletions codex-rs/windows-sandbox-rs/src/identity.rs
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,19 @@ fn load_users(codex_home: &Path) -> Result<Option<SandboxUsersFile>> {
}
}

fn remove_sandbox_users_file(codex_home: &Path, reason: &str) -> Result<()> {
let path = sandbox_users_path(codex_home);
debug_log(
&format!("{reason}; deleting {}", path.display()),
Some(codex_home),
);
match fs::remove_file(&path) {
Ok(()) => Ok(()),
Err(err) if err.kind() == std::io::ErrorKind::NotFound => Ok(()),
Err(err) => Err(err).with_context(|| format!("delete {}", path.display())),
}
}

fn decode_password(record: &SandboxUserRecord) -> Result<String> {
let blob = BASE64_STANDARD
.decode(record.password.as_bytes())
Expand Down Expand Up @@ -233,3 +246,60 @@ pub fn require_logon_sandbox_creds(
password: identity.password,
})
}

#[allow(clippy::too_many_arguments)]
pub(crate) fn refresh_logon_sandbox_creds(
permissions: &ResolvedWindowsSandboxPermissions,
command_cwd: &Path,
env_map: &HashMap<String, String>,
codex_home: &Path,
read_roots_override: Option<&[PathBuf]>,
read_roots_include_platform_defaults: bool,
write_roots_override: Option<&[PathBuf]>,
deny_read_paths_override: &[PathBuf],
deny_write_paths_override: &[PathBuf],
proxy_enforced: bool,
) -> Result<SandboxCreds> {
remove_sandbox_users_file(codex_home, "sandbox user login failed")?;
require_logon_sandbox_creds(
permissions,
command_cwd,
env_map,
codex_home,
read_roots_override,
read_roots_include_platform_defaults,
write_roots_override,
deny_read_paths_override,
deny_write_paths_override,
proxy_enforced,
)
}

#[cfg(test)]
mod tests {
use super::remove_sandbox_users_file;
use crate::setup::sandbox_users_path;
use std::fs;
use tempfile::TempDir;

#[test]
fn remove_sandbox_users_file_deletes_existing_file() {
let codex_home = TempDir::new().expect("tempdir");
let users_path = sandbox_users_path(codex_home.path());
fs::create_dir_all(users_path.parent().expect("sandbox secrets dir"))
.expect("create sandbox secrets dir");
fs::write(&users_path, "users").expect("write users");

remove_sandbox_users_file(codex_home.path(), "stale creds").expect("remove users");
assert!(!users_path.exists());
}

#[test]
fn remove_sandbox_users_file_ignores_missing_file() {
let codex_home = TempDir::new().expect("tempdir");
let users_path = sandbox_users_path(codex_home.path());

remove_sandbox_users_file(codex_home.path(), "stale creds").expect("remove users");
assert!(!users_path.exists());
}
}
71 changes: 59 additions & 12 deletions codex-rs/windows-sandbox-rs/src/unified_exec/backends/elevated.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,16 @@ use super::windows_common::make_runner_resizer;
use super::windows_common::start_runner_pipe_writer;
use super::windows_common::start_runner_stdin_writer;
use super::windows_common::start_runner_stdout_reader;
use crate::identity::SandboxCreds;
use crate::identity::refresh_logon_sandbox_creds;
use crate::ipc_framed::EmptyPayload;
use crate::ipc_framed::FramedMessage;
use crate::ipc_framed::IPC_PROTOCOL_VERSION;
use crate::ipc_framed::Message;
use crate::ipc_framed::SpawnRequest;
use crate::resolved_permissions::ResolvedWindowsSandboxPermissions;
use crate::runner_client::RunnerTransport;
use crate::runner_client::is_stale_sandbox_creds_error;
use crate::runner_client::spawn_runner_transport;
use crate::spawn_prep::prepare_elevated_spawn_context_for_permissions;
use anyhow::Result;
Expand All @@ -23,6 +27,26 @@ use tokio::sync::broadcast;
use tokio::sync::mpsc;
use tokio::sync::oneshot;

async fn spawn_runner_transport_task(
codex_home: PathBuf,
cwd: PathBuf,
sandbox_creds: SandboxCreds,
logs_base_dir: Option<PathBuf>,
spawn_request: SpawnRequest,
) -> Result<RunnerTransport> {
tokio::task::spawn_blocking(move || -> Result<_> {
spawn_runner_transport(
&codex_home,
&cwd,
&sandbox_creds,
logs_base_dir.as_deref(),
spawn_request,
)
})
.await
.map_err(|err| anyhow::anyhow!("runner handshake task failed: {err}"))?
}

#[allow(clippy::too_many_arguments)]
pub(crate) async fn spawn_windows_sandbox_session_elevated_for_permission_profile(
permission_profile: &PermissionProfile,
Expand Down Expand Up @@ -55,7 +79,7 @@ pub(crate) async fn spawn_windows_sandbox_session_elevated_for_permission_profil
workspace_roots,
)?;
let elevated = prepare_elevated_spawn_context_for_permissions(
permissions,
permissions.clone(),
codex_home,
cwd,
&mut env_map,
Expand Down Expand Up @@ -83,19 +107,42 @@ pub(crate) async fn spawn_windows_sandbox_session_elevated_for_permission_profil
};
let codex_home = codex_home.to_path_buf();
let cwd = cwd.to_path_buf();
let sandbox_creds = elevated.sandbox_creds.clone();
let sandbox_creds = elevated.sandbox_creds;
let logs_base_dir = elevated.logs_base_dir.clone();
let transport = tokio::task::spawn_blocking(move || -> Result<_> {
spawn_runner_transport(
&codex_home,
&cwd,
&sandbox_creds,
logs_base_dir.as_deref(),
spawn_request,
)
})
let transport = match spawn_runner_transport_task(
codex_home.clone(),
cwd.clone(),
sandbox_creds,
logs_base_dir.clone(),
spawn_request.clone(),
)
.await
.map_err(|err| anyhow::anyhow!("runner handshake task failed: {err}"))??;
{
Ok(transport) => transport,
Err(err) if is_stale_sandbox_creds_error(&err) => {
let sandbox_creds = refresh_logon_sandbox_creds(
&permissions,
&cwd,
&env_map,
&codex_home,
read_roots_override,
read_roots_include_platform_defaults,
write_roots_override,
&deny_read_paths_override,
&deny_write_paths_override,
/*proxy_enforced*/ false,
)?;
spawn_runner_transport_task(
codex_home,
cwd,
sandbox_creds,
logs_base_dir,
spawn_request,
)
.await?
}
Err(err) => return Err(err),
};
let (pipe_write, pipe_read) = transport.into_files();

let (writer_tx, writer_rx) = mpsc::channel::<Vec<u8>>(128);
Expand Down
Loading