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
8 changes: 8 additions & 0 deletions codex-rs/app-server/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -477,6 +477,14 @@ pub async fn run_main_with_transport(
range: None,
});
}
if let Some(warning) = codex_core::config::missing_system_bwrap_warning() {
config_warnings.push(ConfigWarningNotification {
summary: warning,
details: None,
path: None,
range: None,
});
}

let feedback = CodexFeedback::new();

Expand Down
5 changes: 5 additions & 0 deletions codex-rs/core/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,11 @@ only when the split filesystem policy round-trips through the legacy
cases like `/repo = write`, `/repo/a = none`, `/repo/a/b = write`, where the
more specific writable child must reopen under a denied parent.

The Linux sandbox helper prefers `/usr/bin/bwrap` whenever it is available and
falls back to the vendored bubblewrap path otherwise. When `/usr/bin/bwrap` is
missing, Codex also surfaces a startup warning through its normal notification
path instead of printing directly from the sandbox helper.

### All Platforms

Expects the binary containing `codex-core` to simulate the virtual `apply_patch` CLI when `arg1` is `--codex-run-as-apply-patch`. See the `codex-arg0` crate for details.
13 changes: 13 additions & 0 deletions codex-rs/core/src/config/config_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ use pretty_assertions::assert_eq;

use std::collections::BTreeMap;
use std::collections::HashMap;
use std::path::Path;
use std::time::Duration;
use tempfile::TempDir;

Expand Down Expand Up @@ -5498,6 +5499,18 @@ shell_tool = true
Ok(())
}

#[test]
fn missing_system_bwrap_warning_matches_system_bwrap_presence() {
#[cfg(target_os = "linux")]
assert_eq!(
missing_system_bwrap_warning().is_some(),
!Path::new("/usr/bin/bwrap").is_file()
);

#[cfg(not(target_os = "linux"))]
assert!(missing_system_bwrap_warning().is_none());
}

#[tokio::test]
async fn approvals_reviewer_defaults_to_manual_only_without_guardian_feature() -> std::io::Result<()>
{
Expand Down
18 changes: 18 additions & 0 deletions codex-rs/core/src/config/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -140,12 +140,30 @@ pub(crate) const DEFAULT_AGENT_JOB_MAX_RUNTIME_SECONDS: Option<u64> = None;

pub const CONFIG_TOML_FILE: &str = "config.toml";
const OPENAI_BASE_URL_ENV_VAR: &str = "OPENAI_BASE_URL";
#[cfg(target_os = "linux")]
const SYSTEM_BWRAP_PATH: &str = "/usr/bin/bwrap";
const RESERVED_MODEL_PROVIDER_IDS: [&str; 3] = [
OPENAI_PROVIDER_ID,
OLLAMA_OSS_PROVIDER_ID,
LMSTUDIO_OSS_PROVIDER_ID,
];

#[cfg(target_os = "linux")]
pub fn missing_system_bwrap_warning() -> Option<String> {
if Path::new(SYSTEM_BWRAP_PATH).is_file() {
None
} else {
Some(format!(
"Codex could not find system bubblewrap at {SYSTEM_BWRAP_PATH}. Please install bubblewrap with your package manager. Codex will use the vendored bubblewrap in the meantime."
))
}
}

#[cfg(not(target_os = "linux"))]
pub fn missing_system_bwrap_warning() -> Option<String> {
None
}

fn resolve_sqlite_home_env(resolved_cwd: &Path) -> Option<PathBuf> {
let raw = std::env::var(codex_state::SQLITE_HOME_ENV).ok()?;
let trimmed = raw.trim();
Expand Down
6 changes: 6 additions & 0 deletions codex-rs/exec/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -663,6 +663,12 @@ async fn run_exec_session(args: ExecRunArgs) -> anyhow::Result<()> {
// Print the effective configuration and initial request so users can see what Codex
// is using.
event_processor.print_config_summary(&config, &prompt_summary, &session_configured);
if !json_mode && let Some(message) = codex_core::config::missing_system_bwrap_warning() {
let _ = event_processor.process_event(Event {
id: String::new(),
msg: EventMsg::Warning(codex_protocol::protocol::WarningEvent { message }),
});
}

info!("Codex initialized with event: {session_configured:?}");

Expand Down
15 changes: 11 additions & 4 deletions codex-rs/linux-sandbox/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,20 @@ This crate is responsible for producing:
- the `codex-exec` CLI can check if its arg0 is `codex-linux-sandbox` and, if so, execute as if it were `codex-linux-sandbox`
- this should also be true of the `codex` multitool CLI

On Linux, the bubblewrap pipeline uses the vendored bubblewrap path compiled
into this binary.
On Linux, the bubblewrap pipeline prefers the system `/usr/bin/bwrap` whenever
it is available. If `/usr/bin/bwrap` is missing, the helper still falls back to
the vendored bubblewrap path compiled into this binary.
Codex also surfaces a startup warning when `/usr/bin/bwrap` is missing so users
know it is falling back to the vendored helper.

**Current Behavior**
- Legacy `SandboxPolicy` / `sandbox_mode` configs remain supported.
- Bubblewrap is the default filesystem sandbox pipeline and is standardized on
the vendored path.
- Bubblewrap is the default filesystem sandbox pipeline.
- If `/usr/bin/bwrap` is present, the helper uses it.
- If `/usr/bin/bwrap` is missing, the helper falls back to the vendored
bubblewrap path.
- If `/usr/bin/bwrap` is missing, Codex also surfaces a startup warning instead
of printing directly from the sandbox helper.
- Legacy Landlock + mount protections remain available as an explicit legacy
fallback path.
- Set `features.use_legacy_landlock = true` (or CLI `-c use_legacy_landlock=true`)
Expand Down
134 changes: 134 additions & 0 deletions codex-rs/linux-sandbox/src/launcher.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,134 @@
use std::ffi::CString;
use std::fs::File;
use std::os::fd::AsRawFd;
use std::os::raw::c_char;
use std::os::unix::ffi::OsStrExt;
use std::path::Path;

use crate::vendored_bwrap::exec_vendored_bwrap;
use codex_utils_absolute_path::AbsolutePathBuf;

const SYSTEM_BWRAP_PATH: &str = "/usr/bin/bwrap";

#[derive(Debug, Clone, PartialEq, Eq)]
enum BubblewrapLauncher {
System(AbsolutePathBuf),
Vendored,
}

pub(crate) fn exec_bwrap(argv: Vec<String>, preserved_files: Vec<File>) -> ! {
match preferred_bwrap_launcher() {
BubblewrapLauncher::System(program) => exec_system_bwrap(&program, argv, preserved_files),
BubblewrapLauncher::Vendored => exec_vendored_bwrap(argv, preserved_files),
}
}

fn preferred_bwrap_launcher() -> BubblewrapLauncher {
if !Path::new(SYSTEM_BWRAP_PATH).is_file() {
return BubblewrapLauncher::Vendored;
}

let system_bwrap_path = match AbsolutePathBuf::from_absolute_path(SYSTEM_BWRAP_PATH) {
Ok(path) => path,
Err(err) => panic!("failed to normalize system bubblewrap path {SYSTEM_BWRAP_PATH}: {err}"),
};
BubblewrapLauncher::System(system_bwrap_path)
}

fn exec_system_bwrap(
program: &AbsolutePathBuf,
argv: Vec<String>,
preserved_files: Vec<File>,
) -> ! {
// System bwrap runs across an exec boundary, so preserved fds must survive exec.
make_files_inheritable(&preserved_files);
Comment thread
viyatb-oai marked this conversation as resolved.

let program_path = program.as_path().display().to_string();
let program = CString::new(program.as_path().as_os_str().as_bytes())
.unwrap_or_else(|err| panic!("invalid system bubblewrap path: {err}"));
let cstrings = argv_to_cstrings(&argv);
let mut argv_ptrs: Vec<*const c_char> = cstrings.iter().map(|arg| arg.as_ptr()).collect();
argv_ptrs.push(std::ptr::null());

// SAFETY: `program` and every entry in `argv_ptrs` are valid C strings for
// the duration of the call. On success `execv` does not return.
unsafe {
libc::execv(program.as_ptr(), argv_ptrs.as_ptr());
}
let err = std::io::Error::last_os_error();
panic!("failed to exec system bubblewrap {program_path}: {err}");
}

fn argv_to_cstrings(argv: &[String]) -> Vec<CString> {
let mut cstrings: Vec<CString> = Vec::with_capacity(argv.len());
for arg in argv {
match CString::new(arg.as_str()) {
Ok(value) => cstrings.push(value),
Err(err) => panic!("failed to convert argv to CString: {err}"),
}
}
cstrings
}

fn make_files_inheritable(files: &[File]) {
for file in files {
clear_cloexec(file.as_raw_fd());
}
}

fn clear_cloexec(fd: libc::c_int) {
// SAFETY: `fd` is an owned descriptor kept alive by `files`.
let flags = unsafe { libc::fcntl(fd, libc::F_GETFD) };
if flags < 0 {
let err = std::io::Error::last_os_error();
panic!("failed to read fd flags for preserved bubblewrap file descriptor {fd}: {err}");
}
let cleared_flags = flags & !libc::FD_CLOEXEC;
if cleared_flags == flags {
return;
}

// SAFETY: `fd` is valid and we are only clearing FD_CLOEXEC.
let result = unsafe { libc::fcntl(fd, libc::F_SETFD, cleared_flags) };
if result < 0 {
let err = std::io::Error::last_os_error();
panic!("failed to clear CLOEXEC for preserved bubblewrap file descriptor {fd}: {err}");
}
}

#[cfg(test)]
mod tests {
use super::*;
use pretty_assertions::assert_eq;
use tempfile::NamedTempFile;

#[test]
fn preserved_files_are_made_inheritable_for_system_exec() {
let file = NamedTempFile::new().expect("temp file");
set_cloexec(file.as_file().as_raw_fd());

make_files_inheritable(std::slice::from_ref(file.as_file()));

assert_eq!(fd_flags(file.as_file().as_raw_fd()) & libc::FD_CLOEXEC, 0);
}

fn set_cloexec(fd: libc::c_int) {
let flags = fd_flags(fd);
// SAFETY: `fd` is valid for the duration of the test.
let result = unsafe { libc::fcntl(fd, libc::F_SETFD, flags | libc::FD_CLOEXEC) };
if result < 0 {
let err = std::io::Error::last_os_error();
panic!("failed to set CLOEXEC for test fd {fd}: {err}");
}
}

fn fd_flags(fd: libc::c_int) -> libc::c_int {
// SAFETY: `fd` is valid for the duration of the test.
let flags = unsafe { libc::fcntl(fd, libc::F_GETFD) };
if flags < 0 {
let err = std::io::Error::last_os_error();
panic!("failed to read fd flags for test fd {fd}: {err}");
}
flags
}
}
2 changes: 2 additions & 0 deletions codex-rs/linux-sandbox/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ mod bwrap;
#[cfg(target_os = "linux")]
mod landlock;
#[cfg(target_os = "linux")]
mod launcher;
#[cfg(target_os = "linux")]
mod linux_run_main;
#[cfg(target_os = "linux")]
mod proxy_routing;
Expand Down
8 changes: 3 additions & 5 deletions codex-rs/linux-sandbox/src/linux_run_main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,9 @@ use crate::bwrap::BwrapNetworkMode;
use crate::bwrap::BwrapOptions;
use crate::bwrap::create_bwrap_command_args;
use crate::landlock::apply_sandbox_policy_to_current_thread;
use crate::launcher::exec_bwrap;
use crate::proxy_routing::activate_proxy_routes_in_netns;
use crate::proxy_routing::prepare_host_proxy_route_spec;
use crate::vendored_bwrap::exec_vendored_bwrap;
use crate::vendored_bwrap::run_vendored_bwrap_main;
use codex_protocol::protocol::FileSystemSandboxPolicy;
use codex_protocol::protocol::NetworkSandboxPolicy;
use codex_protocol::protocol::SandboxPolicy;
Expand Down Expand Up @@ -434,7 +433,7 @@ fn run_bwrap_with_proc_fallback(
command_cwd,
options,
);
exec_vendored_bwrap(bwrap_args.args, bwrap_args.preserved_files);
exec_bwrap(bwrap_args.args, bwrap_args.preserved_files);
}

fn bwrap_network_mode(
Expand Down Expand Up @@ -568,8 +567,7 @@ fn run_bwrap_in_child_capture_stderr(bwrap_args: crate::bwrap::BwrapArgs) -> Str
close_fd_or_panic(write_fd, "close write end in bubblewrap child");
}

let exit_code = run_vendored_bwrap_main(&bwrap_args.args, &bwrap_args.preserved_files);
std::process::exit(exit_code);
exec_bwrap(bwrap_args.args, bwrap_args.preserved_files);
}

// Parent: close the write end and read stderr while the child runs.
Expand Down
1 change: 0 additions & 1 deletion codex-rs/linux-sandbox/src/vendored_bwrap.rs
Original file line number Diff line number Diff line change
Expand Up @@ -76,4 +76,3 @@ Notes:
}

pub(crate) use imp::exec_vendored_bwrap;
pub(crate) use imp::run_vendored_bwrap_main;
11 changes: 11 additions & 0 deletions codex-rs/tui/src/app.rs
Original file line number Diff line number Diff line change
Expand Up @@ -275,6 +275,16 @@ fn emit_project_config_warnings(app_event_tx: &AppEventSender, config: &Config)
)));
}

fn emit_missing_system_bwrap_warning(app_event_tx: &AppEventSender) {
let Some(message) = codex_core::config::missing_system_bwrap_warning() else {
return;
};

app_event_tx.send(AppEvent::InsertHistoryCell(Box::new(
history_cell::new_warning_event(message),
)));
}

#[derive(Debug, Clone, PartialEq, Eq)]
struct SessionSummary {
usage_line: String,
Expand Down Expand Up @@ -1963,6 +1973,7 @@ impl App {
let (app_event_tx, mut app_event_rx) = unbounded_channel();
let app_event_tx = AppEventSender::new(app_event_tx);
emit_project_config_warnings(&app_event_tx, &config);
emit_missing_system_bwrap_warning(&app_event_tx);
tui.set_notification_method(config.tui_notification_method);

let harness_overrides =
Expand Down
11 changes: 11 additions & 0 deletions codex-rs/tui_app_server/src/app.rs
Original file line number Diff line number Diff line change
Expand Up @@ -286,6 +286,16 @@ fn emit_project_config_warnings(app_event_tx: &AppEventSender, config: &Config)
)));
}

fn emit_missing_system_bwrap_warning(app_event_tx: &AppEventSender) {
let Some(message) = codex_core::config::missing_system_bwrap_warning() else {
return;
};

app_event_tx.send(AppEvent::InsertHistoryCell(Box::new(
history_cell::new_warning_event(message),
)));
}

#[derive(Debug, Clone, PartialEq, Eq)]
struct SessionSummary {
usage_line: String,
Expand Down Expand Up @@ -2383,6 +2393,7 @@ impl App {
let (app_event_tx, mut app_event_rx) = unbounded_channel();
let app_event_tx = AppEventSender::new(app_event_tx);
emit_project_config_warnings(&app_event_tx, &config);
emit_missing_system_bwrap_warning(&app_event_tx);
tui.set_notification_method(config.tui_notification_method);

let harness_overrides =
Expand Down
Loading