agent/codex
2
0
Fork 0
mirror of https://github.com/pchuan98/codex.git synced 2026-07-01 00:31:56 +08:00
Code Issues Packages Projects Releases Wiki Activity

recover stale Windows sandbox credentials (#27944)

Browse Source 
## Why

The elevated Windows sandbox persists dedicated sandbox account
credentials so later commands can launch without reprovisioning. If
those persisted credentials drift from the actual Windows account
password, `CreateProcessWithLogonW` fails with `ERROR_LOGON_FAILURE` and
Codex currently surfaces that as a hard runner launch failure.

This change makes that failure self-healing. When Windows specifically
rejects the sandbox login, Codex now treats the persisted sandbox
credentials as stale, regenerates them through the existing setup path,
and retries the runner launch once.

## What Changed

- Preserve `CreateProcessWithLogonW` failures as a typed runner logon
error so callers can distinguish `ERROR_LOGON_FAILURE` from unrelated
launch failures.
- Add a sandbox credential refresh helper that deletes the persisted
`sandbox_users.json` record and reuses `require_logon_sandbox_creds()`
to reprovision credentials through the established setup flow.
- Retry elevated runner startup after stale-credential failures in both
the legacy elevated capture path and unified exec elevated backend.
- Add focused tests for stale logon failure detection and persisted
sandbox user file removal.

## Validation

- `git diff --check`
- `cargo test -p codex-windows-sandbox`
This commit is contained in:
iceweasel-oai
2026-06-15 13:54:24 -07:00
committed by GitHub
Unverified
parent fc1fb682a7
commit c0b36d234a
4 changed files with 201 additions and 18 deletions
Download Patch File Download Diff File Expand all files Collapse all files
codex-rs/windows-sandbox-rs/src/elevated/runner_client.rs
+42 -2
View File
@@ -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;
@@ -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 {
@@ -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;
@@ -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]
);
}
}
codex-rs/windows-sandbox-rs/src/elevated_impl.rs
+30 -4
View File
@@ -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;
@@ -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;
@@ -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,
@@ -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)?;
codex-rs/windows-sandbox-rs/src/identity.rs
+70
View File
@@ -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())
@@ -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());
}
}
codex-rs/windows-sandbox-rs/src/unified_exec/backends/elevated.rs
+59 -12
View File
@@ -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;
@@ -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,
@@ -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,
@@ -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);