Skip to content

Fix Windows fs-helper sandbox bypass #27950

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
iceweasel-oai wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Fix Windows fs-helper sandbox bypass #27950

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
dc06032
Fix Windows fs helper sandbox launch
iceweasel-oai Jun 12, 2026
82a5089
Fix fs helper path URI rebase fallout
iceweasel-oai Jun 12, 2026
c11e2f3
Fix non-Windows direct-spawn clippy
iceweasel-oai Jun 15, 2026
ee3f14b
Remove unused PathExt import
iceweasel-oai Jun 15, 2026
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
4 changes: 4 additions & 0 deletions codex-rs/Cargo.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions codex-rs/arg0/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,5 +26,8 @@ dotenvy = { workspace = true }
tempfile = { workspace = true }
tokio = { workspace = true, features = ["rt-multi-thread"] }

[target.'cfg(windows)'.dependencies]
codex-windows-sandbox = { workspace = true }

@chatgpt-codex-connector chatgpt-codex-connector Bot Jun 12, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Refresh Bazel lock for the dependency change

This Cargo.toml change adds a Windows-only dependency but MODULE.bazel.lock is unchanged, so Bazel crate resolution can drift and just bazel-lock-check/Bazel builds can fail for the Windows dependency set; refresh and include the Bazel lock with the dependency edit. guidance

Useful? React with 👍 / 👎.


[dev-dependencies]
pretty_assertions = { workspace = true }
6 changes: 6 additions & 0 deletions codex-rs/arg0/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ use codex_exec_server::CODEX_FS_HELPER_ARG1;
use codex_install_context::InstallContext;
use codex_sandboxing::landlock::CODEX_LINUX_SANDBOX_ARG0;
use codex_utils_home_dir::find_codex_home;
#[cfg(target_os = "windows")]
use codex_windows_sandbox::CODEX_WINDOWS_SANDBOX_ARG1;
#[cfg(unix)]
use std::os::unix::fs::symlink;
use tempfile::TempDir;
Expand Down Expand Up @@ -99,6 +101,10 @@ pub fn arg0_dispatch() -> Option<Arg0PathEntryGuard> {
if argv1 == CODEX_FS_HELPER_ARG1 {
codex_exec_server::run_fs_helper_main();
}
#[cfg(target_os = "windows")]
if argv1 == CODEX_WINDOWS_SANDBOX_ARG1 {
codex_windows_sandbox::run_windows_sandbox_wrapper_main();
}
if argv1 == CODEX_CORE_APPLY_PATCH_ARG1 {
let patch_arg = args.next().and_then(|s| s.to_str().map(str::to_owned));
let exit_code = match patch_arg {
Expand Down
259 changes: 23 additions & 236 deletions codex-rs/cli/src/debug_sandbox.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -369,52 +369,30 @@ async fn run_command_under_windows_session(
) -> ! {
use codex_core::windows_sandbox::WindowsSandboxLevelExt;
use codex_protocol::config_types::WindowsSandboxLevel;
use codex_windows_sandbox::spawn_windows_sandbox_session_elevated_for_permission_profile;
use codex_windows_sandbox::spawn_windows_sandbox_session_legacy;
use codex_windows_sandbox::WindowsSandboxSessionRequest;
use codex_windows_sandbox::spawn_windows_sandbox_session_for_level;

let permission_profile = config.permissions.effective_permission_profile();

let use_elevated = matches!(
WindowsSandboxLevel::from_config(config),
WindowsSandboxLevel::Elevated
);

let spawned = if use_elevated {
spawn_windows_sandbox_session_elevated_for_permission_profile(
&permission_profile,
workspace_roots.as_slice(),
config.codex_home.as_path(),
command,
cwd.as_path(),
env,
None,
/*read_roots_override*/ None,
/*read_roots_include_platform_defaults*/ false,
/*write_roots_override*/ None,
/*deny_read_paths_override*/ &[],
/*deny_write_paths_override*/ &[],
/*tty*/ false,
/*stdin_open*/ true,
config.permissions.windows_sandbox_private_desktop,
)
.await
} else {
spawn_windows_sandbox_session_legacy(
&permission_profile,
workspace_roots.as_slice(),
config.codex_home.as_path(),
command,
cwd.as_path(),
env,
None,
/*additional_deny_read_paths*/ &[],
/*additional_deny_write_paths*/ &[],
/*tty*/ false,
/*stdin_open*/ true,
config.permissions.windows_sandbox_private_desktop,
)
.await
};
let empty_paths: &[AbsolutePathBuf] = &[];
let spawned = spawn_windows_sandbox_session_for_level(WindowsSandboxSessionRequest {
permission_profile: &permission_profile,
workspace_roots: workspace_roots.as_slice(),
codex_home: config.codex_home.as_path(),
command,
cwd: cwd.as_path(),
env_map: env,
windows_sandbox_level: WindowsSandboxLevel::from_config(config),
timeout_ms: None,
read_roots_override: None,
read_roots_include_platform_defaults: false,
write_roots_override: None,
deny_read_paths_override: empty_paths,
deny_write_paths_override: empty_paths,
tty: false,
stdin_open: true,
use_private_desktop: config.permissions.windows_sandbox_private_desktop,
})
.await;

let spawned = match spawned {
Ok(spawned) => spawned,
Expand All @@ -424,63 +402,7 @@ async fn run_command_under_windows_session(
}
};

let session = std::sync::Arc::new(spawned.session);
let tokio_runtime = tokio::runtime::Handle::current();
// Give large or slow tail output a better chance to finish draining
// without letting rare EOF issues hang the wrapper indefinitely.
let output_drain_timeout = std::time::Duration::from_secs(5);
// A helper thread watches our stdin. When the input source closes it,
// the thread tells the main async code so we can also close stdin for
// the sandboxed child process.
let (stdin_eof_tx, stdin_eof_rx) = tokio::sync::oneshot::channel();

// Start background threads that copy stdin/stdout/stderr. We
// intentionally do not keep their JoinHandles; dropping the handle does
// not stop the thread, it just means we are not going to wait on it
// later.
drop(windows_stdio_bridge::spawn_input_forwarder(
std::io::stdin(),
session.writer_sender(),
stdin_eof_tx,
));
let (stdout_forwarder, stdout_forwarder_done_rx) = windows_stdio_bridge::spawn_output_forwarder(
tokio_runtime.clone(),
spawned.stdout_rx,
std::io::stdout(),
);
drop(stdout_forwarder);
let (stderr_forwarder, stderr_forwarder_done_rx) = windows_stdio_bridge::spawn_output_forwarder(
tokio_runtime.clone(),
spawned.stderr_rx,
std::io::stderr(),
);
drop(stderr_forwarder);

let stdin_close_task = tokio::spawn({
let session = std::sync::Arc::clone(&session);
async move {
let _ = stdin_eof_rx.await;
session.close_stdin();
}
});

let mut exit_rx = spawned.exit_rx;
let exit_code = tokio::select! {
res = &mut exit_rx => res.unwrap_or(-1),
res = tokio::signal::ctrl_c() => {
if let Ok(()) = res {
session.request_terminate();
}
exit_rx.await.unwrap_or(-1)
}
};

stdin_close_task.abort();
let _ = tokio::time::timeout(output_drain_timeout, async {
let _ = stdout_forwarder_done_rx.await;
let _ = stderr_forwarder_done_rx.await;
})
.await;
let exit_code = codex_windows_sandbox::forward_sandbox_session_stdio(spawned).await;
std::process::exit(exit_code);
}

Expand Down Expand Up @@ -515,141 +437,6 @@ async fn spawn_debug_sandbox_child(
.spawn()
}

#[cfg(target_os = "windows")]
mod windows_stdio_bridge {
use std::io::Read;
use std::io::Write;

use tokio::sync::mpsc;
use tokio::sync::oneshot;

const STDIN_FORWARD_CHUNK_SIZE: usize = 8 * 1024;

pub(super) fn spawn_input_forwarder<R>(
mut input: R,
writer_tx: mpsc::Sender<Vec<u8>>,
stdin_eof_tx: oneshot::Sender<()>,
) -> std::thread::JoinHandle<()>
where
R: Read + Send + 'static,
{
std::thread::spawn(move || {
let mut buffer = [0_u8; STDIN_FORWARD_CHUNK_SIZE];
loop {
match input.read(&mut buffer) {
Ok(0) => break,
Ok(n) => {
if writer_tx.blocking_send(buffer[..n].to_vec()).is_err() {
break;
}
}
Err(err) if err.kind() == std::io::ErrorKind::Interrupted => continue,
Err(err) => {
eprintln!("windows sandbox stdin forwarder failed: {err}");
break;
}
}
}
let _ = stdin_eof_tx.send(());
})
}

pub(super) fn spawn_output_forwarder<W>(
tokio_runtime: tokio::runtime::Handle,
output_rx: mpsc::Receiver<Vec<u8>>,
mut writer: W,
) -> (std::thread::JoinHandle<()>, oneshot::Receiver<()>)
where
W: Write + Send + 'static,
{
let (done_tx, done_rx) = oneshot::channel();
// The sandbox session emits output on Tokio channels, but writing to the
// caller's stdio is simplest from a dedicated blocking thread.
let handle = std::thread::spawn(move || {
let mut output_rx = output_rx;
while let Some(chunk) = tokio_runtime.block_on(output_rx.recv()) {
if let Err(err) = writer.write_all(&chunk) {
eprintln!("windows sandbox output forwarder failed to write: {err}");
break;
}
if let Err(err) = writer.flush() {
eprintln!("windows sandbox output forwarder failed to flush: {err}");
break;
}
}
let _ = done_tx.send(());
});
(handle, done_rx)
}

#[cfg(test)]
mod tests {
use std::sync::Mutex;

use pretty_assertions::assert_eq;

use super::*;

#[tokio::test]
async fn input_forwarder_sends_chunks_and_reports_eof() -> anyhow::Result<()> {
let (writer_tx, mut writer_rx) = tokio::sync::mpsc::channel::<Vec<u8>>(4);
let (stdin_closed_tx, stdin_closed_rx) = tokio::sync::oneshot::channel();
let input = std::io::Cursor::new(b"first\nsecond\n".to_vec());

let forwarder = spawn_input_forwarder(input, writer_tx, stdin_closed_tx);
let mut received = Vec::new();
while let Some(chunk) = writer_rx.recv().await {
received.extend_from_slice(&chunk);
}
stdin_closed_rx.await?;
forwarder.join().expect("stdin forwarder should finish");

assert_eq!(received, b"first\nsecond\n".to_vec());
Ok(())
}

#[tokio::test]
async fn output_forwarder_writes_all_chunks() -> anyhow::Result<()> {
#[derive(Clone, Default)]
struct SharedWriter(std::sync::Arc<Mutex<Vec<u8>>>);

impl std::io::Write for SharedWriter {
fn write(&mut self, buf: &[u8]) -> std::io::Result<usize> {
let mut guard = self
.0
.lock()
.map_err(|_| std::io::Error::other("writer poisoned"))?;
guard.extend_from_slice(buf);
Ok(buf.len())
}

fn flush(&mut self) -> std::io::Result<()> {
Ok(())
}
}

let runtime = tokio::runtime::Handle::current();
let (output_tx, output_rx) = tokio::sync::mpsc::channel::<Vec<u8>>(4);
let writer = SharedWriter::default();
let sink = std::sync::Arc::clone(&writer.0);

let (forwarder, done_rx) = spawn_output_forwarder(runtime, output_rx, writer);
output_tx.send(b"alpha".to_vec()).await?;
output_tx.send(b"beta".to_vec()).await?;
drop(output_tx);
forwarder.join().expect("output forwarder should finish");
done_rx.await?;

let output = sink
.lock()
.map_err(|_| anyhow::anyhow!("writer poisoned"))?
.clone();
assert_eq!(output, b"alphabeta".to_vec());
Ok(())
}
}
}

async fn load_debug_sandbox_config(
cli_overrides: Vec<(String, TomlValue)>,
codex_linux_sandbox_exe: Option<PathBuf>,
Expand Down
1 change: 1 addition & 0 deletions codex-rs/core/src/session/turn_context.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -343,6 +343,7 @@ impl TurnContext {
FileSystemSandboxContext {
permissions,
cwd: Some(cwd.clone()),
workspace_roots: self.config.effective_workspace_roots(),
windows_sandbox_level: self.windows_sandbox_level,
windows_sandbox_private_desktop: self
.config
Expand Down
1 change: 1 addition & 0 deletions codex-rs/core/src/tools/runtimes/apply_patch.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@ impl ApplyPatchRuntime {
Some(FileSystemSandboxContext {
permissions,
cwd: Some(attempt.sandbox_cwd.clone()),
workspace_roots: attempt.workspace_roots.to_vec(),
windows_sandbox_level: attempt.windows_sandbox_level,
windows_sandbox_private_desktop: attempt.windows_sandbox_private_desktop,
use_legacy_landlock: attempt.use_legacy_landlock,
Expand Down
1 change: 0 additions & 1 deletion codex-rs/core/src/tools/runtimes/mod_tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ use codex_sandboxing::SandboxType;
use codex_utils_absolute_path::AbsolutePathBuf;
use codex_utils_path_uri::PathUri;
use core_test_support::PathBufExt;
use core_test_support::PathExt;
use pretty_assertions::assert_eq;
use std::path::PathBuf;
use std::process::Command;
Expand Down
Loading
Loading