From 85ce91a5b329f1e999eca87354e8b80e2ba74f3d Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Sat, 21 Feb 2026 00:34:08 -0800 Subject: [PATCH 1/8] refactor(core): move embedded system skills into codex-skills crate (#12435) ## Why `codex-core` was carrying the embedded system-skill sample assets (and a `build.rs` that walks those files to register rerun triggers). Those assets change infrequently, but any change under `codex-core` still ties them to `codex-core`'s build/cache lifecycle. This change moves the embedded system-skills packaging into a dedicated `codex-skills` crate so it can be cached independently. That reduces unnecessary invalidation/rebuild pressure on `codex-core` when the skills bundle is the only thing that changes. ## What Changed - Added a new `codex-rs/skills` crate (`codex-skills`) with: - `Cargo.toml` - `BUILD.bazel` - `build.rs` to track skill asset file changes for Cargo rebuilds - `src/lib.rs` containing the embedded system-skills install/cache logic previously in `codex-core` - Moved the embedded sample skill assets from `codex-rs/core/src/skills/assets/samples` to `codex-rs/skills/src/assets/samples`. - Updated `codex-rs/core/Cargo.toml` to depend on `codex-skills` and removed `codex-core`'s direct `include_dir` dependency. - Removed `codex-core`'s `build.rs`. - Replaced `codex-rs/core/src/skills/system.rs` implementation with a thin re-export wrapper to keep existing `codex-core` call sites unchanged. - Updated workspace manifests/lockfile (`codex-rs/Cargo.toml`, `codex-rs/Cargo.lock`) for the new crate. --- codex-rs/Cargo.lock | 11 +- codex-rs/Cargo.toml | 2 + codex-rs/core/Cargo.toml | 3 +- codex-rs/core/src/skills/system.rs | 198 +----------------- codex-rs/skills/BUILD.bazel | 15 ++ codex-rs/skills/Cargo.toml | 19 ++ codex-rs/{core => skills}/build.rs | 2 +- .../assets/samples/skill-creator/SKILL.md | 0 .../samples/skill-creator/agents/openai.yaml | 0 .../assets/skill-creator-small.svg | 0 .../skill-creator/assets/skill-creator.png | Bin .../assets/samples/skill-creator/license.txt | 0 .../skill-creator/references/openai_yaml.md | 0 .../scripts/generate_openai_yaml.py | 0 .../skill-creator/scripts/init_skill.py | 0 .../skill-creator/scripts/quick_validate.py | 0 .../samples/skill-installer/LICENSE.txt | 0 .../assets/samples/skill-installer/SKILL.md | 0 .../skill-installer/agents/openai.yaml | 0 .../assets/skill-installer-small.svg | 0 .../assets/skill-installer.png | Bin .../skill-installer/scripts/github_utils.py | 0 .../scripts/install-skill-from-github.py | 0 .../skill-installer/scripts/list-skills.py | 0 codex-rs/skills/src/lib.rs | 195 +++++++++++++++++ 25 files changed, 245 insertions(+), 200 deletions(-) create mode 100644 codex-rs/skills/BUILD.bazel create mode 100644 codex-rs/skills/Cargo.toml rename codex-rs/{core => skills}/build.rs (89%) rename codex-rs/{core/src/skills => skills/src}/assets/samples/skill-creator/SKILL.md (100%) rename codex-rs/{core/src/skills => skills/src}/assets/samples/skill-creator/agents/openai.yaml (100%) rename codex-rs/{core/src/skills => skills/src}/assets/samples/skill-creator/assets/skill-creator-small.svg (100%) rename codex-rs/{core/src/skills => skills/src}/assets/samples/skill-creator/assets/skill-creator.png (100%) rename codex-rs/{core/src/skills => skills/src}/assets/samples/skill-creator/license.txt (100%) rename codex-rs/{core/src/skills => skills/src}/assets/samples/skill-creator/references/openai_yaml.md (100%) rename codex-rs/{core/src/skills => skills/src}/assets/samples/skill-creator/scripts/generate_openai_yaml.py (100%) rename codex-rs/{core/src/skills => skills/src}/assets/samples/skill-creator/scripts/init_skill.py (100%) rename codex-rs/{core/src/skills => skills/src}/assets/samples/skill-creator/scripts/quick_validate.py (100%) rename codex-rs/{core/src/skills => skills/src}/assets/samples/skill-installer/LICENSE.txt (100%) rename codex-rs/{core/src/skills => skills/src}/assets/samples/skill-installer/SKILL.md (100%) rename codex-rs/{core/src/skills => skills/src}/assets/samples/skill-installer/agents/openai.yaml (100%) rename codex-rs/{core/src/skills => skills/src}/assets/samples/skill-installer/assets/skill-installer-small.svg (100%) rename codex-rs/{core/src/skills => skills/src}/assets/samples/skill-installer/assets/skill-installer.png (100%) rename codex-rs/{core/src/skills => skills/src}/assets/samples/skill-installer/scripts/github_utils.py (100%) rename codex-rs/{core/src/skills => skills/src}/assets/samples/skill-installer/scripts/install-skill-from-github.py (100%) rename codex-rs/{core/src/skills => skills/src}/assets/samples/skill-installer/scripts/list-skills.py (100%) create mode 100644 codex-rs/skills/src/lib.rs diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index 42521a48df..03276a7adb 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -1644,6 +1644,7 @@ dependencies = [ "codex-rmcp-client", "codex-secrets", "codex-shell-command", + "codex-skills", "codex-state", "codex-utils-absolute-path", "codex-utils-cargo-bin", @@ -1663,7 +1664,6 @@ dependencies = [ "futures", "http 1.4.0", "image", - "include_dir", "indexmap 2.13.0", "indoc", "insta", @@ -2189,6 +2189,15 @@ dependencies = [ "which", ] +[[package]] +name = "codex-skills" +version = "0.0.0" +dependencies = [ + "codex-utils-absolute-path", + "include_dir", + "thiserror 2.0.18", +] + [[package]] name = "codex-state" version = "0.0.0" diff --git a/codex-rs/Cargo.toml b/codex-rs/Cargo.toml index 8ff0c456b8..d23ed3c27b 100644 --- a/codex-rs/Cargo.toml +++ b/codex-rs/Cargo.toml @@ -17,6 +17,7 @@ members = [ "cli", "config", "shell-command", + "skills", "core", "hooks", "secrets", @@ -112,6 +113,7 @@ codex-responses-api-proxy = { path = "responses-api-proxy" } codex-rmcp-client = { path = "rmcp-client" } codex-secrets = { path = "secrets" } codex-shell-command = { path = "shell-command" } +codex-skills = { path = "skills" } codex-state = { path = "state" } codex-stdio-to-uds = { path = "stdio-to-uds" } codex-tui = { path = "tui" } diff --git a/codex-rs/core/Cargo.toml b/codex-rs/core/Cargo.toml index bda19d78ea..15f9d7d11c 100644 --- a/codex-rs/core/Cargo.toml +++ b/codex-rs/core/Cargo.toml @@ -3,7 +3,6 @@ edition.workspace = true license.workspace = true name = "codex-core" version.workspace = true -build = "build.rs" [lib] doctest = false @@ -34,6 +33,7 @@ codex-async-utils = { workspace = true } codex-client = { workspace = true } codex-config = { workspace = true } codex-shell-command = { workspace = true } +codex-skills = { workspace = true } codex-execpolicy = { workspace = true } codex-file-search = { workspace = true } codex-git = { workspace = true } @@ -58,7 +58,6 @@ env-flags = { workspace = true } eventsource-stream = { workspace = true } futures = { workspace = true } http = { workspace = true } -include_dir = { workspace = true } indexmap = { workspace = true } indoc = { workspace = true } keyring = { workspace = true, features = ["crypto-rust"] } diff --git a/codex-rs/core/src/skills/system.rs b/codex-rs/core/src/skills/system.rs index cf8404096d..42b48ea3a5 100644 --- a/codex-rs/core/src/skills/system.rs +++ b/codex-rs/core/src/skills/system.rs @@ -1,196 +1,2 @@ -use codex_utils_absolute_path::AbsolutePathBuf; -use include_dir::Dir; -use std::collections::hash_map::DefaultHasher; -use std::fs; -use std::hash::Hash; -use std::hash::Hasher; -use std::path::Path; -use std::path::PathBuf; - -use thiserror::Error; - -const SYSTEM_SKILLS_DIR: Dir = - include_dir::include_dir!("$CARGO_MANIFEST_DIR/src/skills/assets/samples"); - -const SYSTEM_SKILLS_DIR_NAME: &str = ".system"; -const SKILLS_DIR_NAME: &str = "skills"; -const SYSTEM_SKILLS_MARKER_FILENAME: &str = ".codex-system-skills.marker"; -const SYSTEM_SKILLS_MARKER_SALT: &str = "v1"; - -/// Returns the on-disk cache location for embedded system skills. -/// -/// This is typically located at `CODEX_HOME/skills/.system`. -pub(crate) fn system_cache_root_dir(codex_home: &Path) -> PathBuf { - AbsolutePathBuf::try_from(codex_home) - .and_then(|codex_home| system_cache_root_dir_abs(&codex_home)) - .map(AbsolutePathBuf::into_path_buf) - .unwrap_or_else(|_| { - codex_home - .join(SKILLS_DIR_NAME) - .join(SYSTEM_SKILLS_DIR_NAME) - }) -} - -fn system_cache_root_dir_abs(codex_home: &AbsolutePathBuf) -> std::io::Result { - codex_home - .join(SKILLS_DIR_NAME)? - .join(SYSTEM_SKILLS_DIR_NAME) -} - -/// Installs embedded system skills into `CODEX_HOME/skills/.system`. -/// -/// Clears any existing system skills directory first and then writes the embedded -/// skills directory into place. -/// -/// To avoid doing unnecessary work on every startup, a marker file is written -/// with a fingerprint of the embedded directory. When the marker matches, the -/// install is skipped. -pub(crate) fn install_system_skills(codex_home: &Path) -> Result<(), SystemSkillsError> { - let codex_home = AbsolutePathBuf::try_from(codex_home) - .map_err(|source| SystemSkillsError::io("normalize codex home dir", source))?; - let skills_root_dir = codex_home - .join(SKILLS_DIR_NAME) - .map_err(|source| SystemSkillsError::io("resolve skills root dir", source))?; - fs::create_dir_all(skills_root_dir.as_path()) - .map_err(|source| SystemSkillsError::io("create skills root dir", source))?; - - let dest_system = system_cache_root_dir_abs(&codex_home) - .map_err(|source| SystemSkillsError::io("resolve system skills cache root dir", source))?; - - let marker_path = dest_system - .join(SYSTEM_SKILLS_MARKER_FILENAME) - .map_err(|source| SystemSkillsError::io("resolve system skills marker path", source))?; - let expected_fingerprint = embedded_system_skills_fingerprint(); - if dest_system.as_path().is_dir() - && read_marker(&marker_path).is_ok_and(|marker| marker == expected_fingerprint) - { - return Ok(()); - } - - if dest_system.as_path().exists() { - fs::remove_dir_all(dest_system.as_path()) - .map_err(|source| SystemSkillsError::io("remove existing system skills dir", source))?; - } - - write_embedded_dir(&SYSTEM_SKILLS_DIR, &dest_system)?; - fs::write(marker_path.as_path(), format!("{expected_fingerprint}\n")) - .map_err(|source| SystemSkillsError::io("write system skills marker", source))?; - Ok(()) -} - -fn read_marker(path: &AbsolutePathBuf) -> Result { - Ok(fs::read_to_string(path.as_path()) - .map_err(|source| SystemSkillsError::io("read system skills marker", source))? - .trim() - .to_string()) -} - -fn embedded_system_skills_fingerprint() -> String { - let mut items = Vec::new(); - collect_fingerprint_items(&SYSTEM_SKILLS_DIR, &mut items); - items.sort_unstable_by(|(a, _), (b, _)| a.cmp(b)); - - let mut hasher = DefaultHasher::new(); - SYSTEM_SKILLS_MARKER_SALT.hash(&mut hasher); - for (path, contents_hash) in items { - path.hash(&mut hasher); - contents_hash.hash(&mut hasher); - } - format!("{:x}", hasher.finish()) -} - -fn collect_fingerprint_items(dir: &Dir<'_>, items: &mut Vec<(String, Option)>) { - for entry in dir.entries() { - match entry { - include_dir::DirEntry::Dir(subdir) => { - items.push((subdir.path().to_string_lossy().to_string(), None)); - collect_fingerprint_items(subdir, items); - } - include_dir::DirEntry::File(file) => { - let mut file_hasher = DefaultHasher::new(); - file.contents().hash(&mut file_hasher); - items.push(( - file.path().to_string_lossy().to_string(), - Some(file_hasher.finish()), - )); - } - } - } -} - -/// Writes the embedded `include_dir::Dir` to disk under `dest`. -/// -/// Preserves the embedded directory structure. -fn write_embedded_dir(dir: &Dir<'_>, dest: &AbsolutePathBuf) -> Result<(), SystemSkillsError> { - fs::create_dir_all(dest.as_path()) - .map_err(|source| SystemSkillsError::io("create system skills dir", source))?; - - for entry in dir.entries() { - match entry { - include_dir::DirEntry::Dir(subdir) => { - let subdir_dest = dest.join(subdir.path()).map_err(|source| { - SystemSkillsError::io("resolve system skills subdir", source) - })?; - fs::create_dir_all(subdir_dest.as_path()).map_err(|source| { - SystemSkillsError::io("create system skills subdir", source) - })?; - write_embedded_dir(subdir, dest)?; - } - include_dir::DirEntry::File(file) => { - let path = dest.join(file.path()).map_err(|source| { - SystemSkillsError::io("resolve system skills file", source) - })?; - if let Some(parent) = path.as_path().parent() { - fs::create_dir_all(parent).map_err(|source| { - SystemSkillsError::io("create system skills file parent", source) - })?; - } - fs::write(path.as_path(), file.contents()) - .map_err(|source| SystemSkillsError::io("write system skill file", source))?; - } - } - } - - Ok(()) -} - -#[derive(Debug, Error)] -pub(crate) enum SystemSkillsError { - #[error("io error while {action}: {source}")] - Io { - action: &'static str, - #[source] - source: std::io::Error, - }, -} - -impl SystemSkillsError { - fn io(action: &'static str, source: std::io::Error) -> Self { - Self::Io { action, source } - } -} - -#[cfg(test)] -mod tests { - use super::SYSTEM_SKILLS_DIR; - use super::collect_fingerprint_items; - - #[test] - fn fingerprint_traverses_nested_entries() { - let mut items = Vec::new(); - collect_fingerprint_items(&SYSTEM_SKILLS_DIR, &mut items); - let mut paths: Vec = items.into_iter().map(|(path, _)| path).collect(); - paths.sort_unstable(); - - assert!( - paths - .binary_search_by(|probe| probe.as_str().cmp("skill-creator/SKILL.md")) - .is_ok() - ); - assert!( - paths - .binary_search_by(|probe| probe.as_str().cmp("skill-creator/scripts/init_skill.py")) - .is_ok() - ); - } -} +pub(crate) use codex_skills::install_system_skills; +pub(crate) use codex_skills::system_cache_root_dir; diff --git a/codex-rs/skills/BUILD.bazel b/codex-rs/skills/BUILD.bazel new file mode 100644 index 0000000000..1c3fc16954 --- /dev/null +++ b/codex-rs/skills/BUILD.bazel @@ -0,0 +1,15 @@ +load("//:defs.bzl", "codex_rust_crate") + +codex_rust_crate( + name = "skills", + crate_name = "codex_skills", + compile_data = glob( + include = ["**"], + exclude = [ + "**/* *", + "BUILD.bazel", + "Cargo.toml", + ], + allow_empty = True, + ), +) diff --git a/codex-rs/skills/Cargo.toml b/codex-rs/skills/Cargo.toml new file mode 100644 index 0000000000..babb5a177a --- /dev/null +++ b/codex-rs/skills/Cargo.toml @@ -0,0 +1,19 @@ +[package] +edition.workspace = true +license.workspace = true +name = "codex-skills" +version.workspace = true +build = "build.rs" + +[lib] +doctest = false +name = "codex_skills" +path = "src/lib.rs" + +[lints] +workspace = true + +[dependencies] +codex-utils-absolute-path = { workspace = true } +include_dir = { workspace = true } +thiserror = { workspace = true } diff --git a/codex-rs/core/build.rs b/codex-rs/skills/build.rs similarity index 89% rename from codex-rs/core/build.rs rename to codex-rs/skills/build.rs index 587415a3fd..db4ef68416 100644 --- a/codex-rs/core/build.rs +++ b/codex-rs/skills/build.rs @@ -2,7 +2,7 @@ use std::fs; use std::path::Path; fn main() { - let samples_dir = Path::new("src/skills/assets/samples"); + let samples_dir = Path::new("src/assets/samples"); if !samples_dir.exists() { return; } diff --git a/codex-rs/core/src/skills/assets/samples/skill-creator/SKILL.md b/codex-rs/skills/src/assets/samples/skill-creator/SKILL.md similarity index 100% rename from codex-rs/core/src/skills/assets/samples/skill-creator/SKILL.md rename to codex-rs/skills/src/assets/samples/skill-creator/SKILL.md diff --git a/codex-rs/core/src/skills/assets/samples/skill-creator/agents/openai.yaml b/codex-rs/skills/src/assets/samples/skill-creator/agents/openai.yaml similarity index 100% rename from codex-rs/core/src/skills/assets/samples/skill-creator/agents/openai.yaml rename to codex-rs/skills/src/assets/samples/skill-creator/agents/openai.yaml diff --git a/codex-rs/core/src/skills/assets/samples/skill-creator/assets/skill-creator-small.svg b/codex-rs/skills/src/assets/samples/skill-creator/assets/skill-creator-small.svg similarity index 100% rename from codex-rs/core/src/skills/assets/samples/skill-creator/assets/skill-creator-small.svg rename to codex-rs/skills/src/assets/samples/skill-creator/assets/skill-creator-small.svg diff --git a/codex-rs/core/src/skills/assets/samples/skill-creator/assets/skill-creator.png b/codex-rs/skills/src/assets/samples/skill-creator/assets/skill-creator.png similarity index 100% rename from codex-rs/core/src/skills/assets/samples/skill-creator/assets/skill-creator.png rename to codex-rs/skills/src/assets/samples/skill-creator/assets/skill-creator.png diff --git a/codex-rs/core/src/skills/assets/samples/skill-creator/license.txt b/codex-rs/skills/src/assets/samples/skill-creator/license.txt similarity index 100% rename from codex-rs/core/src/skills/assets/samples/skill-creator/license.txt rename to codex-rs/skills/src/assets/samples/skill-creator/license.txt diff --git a/codex-rs/core/src/skills/assets/samples/skill-creator/references/openai_yaml.md b/codex-rs/skills/src/assets/samples/skill-creator/references/openai_yaml.md similarity index 100% rename from codex-rs/core/src/skills/assets/samples/skill-creator/references/openai_yaml.md rename to codex-rs/skills/src/assets/samples/skill-creator/references/openai_yaml.md diff --git a/codex-rs/core/src/skills/assets/samples/skill-creator/scripts/generate_openai_yaml.py b/codex-rs/skills/src/assets/samples/skill-creator/scripts/generate_openai_yaml.py similarity index 100% rename from codex-rs/core/src/skills/assets/samples/skill-creator/scripts/generate_openai_yaml.py rename to codex-rs/skills/src/assets/samples/skill-creator/scripts/generate_openai_yaml.py diff --git a/codex-rs/core/src/skills/assets/samples/skill-creator/scripts/init_skill.py b/codex-rs/skills/src/assets/samples/skill-creator/scripts/init_skill.py similarity index 100% rename from codex-rs/core/src/skills/assets/samples/skill-creator/scripts/init_skill.py rename to codex-rs/skills/src/assets/samples/skill-creator/scripts/init_skill.py diff --git a/codex-rs/core/src/skills/assets/samples/skill-creator/scripts/quick_validate.py b/codex-rs/skills/src/assets/samples/skill-creator/scripts/quick_validate.py similarity index 100% rename from codex-rs/core/src/skills/assets/samples/skill-creator/scripts/quick_validate.py rename to codex-rs/skills/src/assets/samples/skill-creator/scripts/quick_validate.py diff --git a/codex-rs/core/src/skills/assets/samples/skill-installer/LICENSE.txt b/codex-rs/skills/src/assets/samples/skill-installer/LICENSE.txt similarity index 100% rename from codex-rs/core/src/skills/assets/samples/skill-installer/LICENSE.txt rename to codex-rs/skills/src/assets/samples/skill-installer/LICENSE.txt diff --git a/codex-rs/core/src/skills/assets/samples/skill-installer/SKILL.md b/codex-rs/skills/src/assets/samples/skill-installer/SKILL.md similarity index 100% rename from codex-rs/core/src/skills/assets/samples/skill-installer/SKILL.md rename to codex-rs/skills/src/assets/samples/skill-installer/SKILL.md diff --git a/codex-rs/core/src/skills/assets/samples/skill-installer/agents/openai.yaml b/codex-rs/skills/src/assets/samples/skill-installer/agents/openai.yaml similarity index 100% rename from codex-rs/core/src/skills/assets/samples/skill-installer/agents/openai.yaml rename to codex-rs/skills/src/assets/samples/skill-installer/agents/openai.yaml diff --git a/codex-rs/core/src/skills/assets/samples/skill-installer/assets/skill-installer-small.svg b/codex-rs/skills/src/assets/samples/skill-installer/assets/skill-installer-small.svg similarity index 100% rename from codex-rs/core/src/skills/assets/samples/skill-installer/assets/skill-installer-small.svg rename to codex-rs/skills/src/assets/samples/skill-installer/assets/skill-installer-small.svg diff --git a/codex-rs/core/src/skills/assets/samples/skill-installer/assets/skill-installer.png b/codex-rs/skills/src/assets/samples/skill-installer/assets/skill-installer.png similarity index 100% rename from codex-rs/core/src/skills/assets/samples/skill-installer/assets/skill-installer.png rename to codex-rs/skills/src/assets/samples/skill-installer/assets/skill-installer.png diff --git a/codex-rs/core/src/skills/assets/samples/skill-installer/scripts/github_utils.py b/codex-rs/skills/src/assets/samples/skill-installer/scripts/github_utils.py similarity index 100% rename from codex-rs/core/src/skills/assets/samples/skill-installer/scripts/github_utils.py rename to codex-rs/skills/src/assets/samples/skill-installer/scripts/github_utils.py diff --git a/codex-rs/core/src/skills/assets/samples/skill-installer/scripts/install-skill-from-github.py b/codex-rs/skills/src/assets/samples/skill-installer/scripts/install-skill-from-github.py similarity index 100% rename from codex-rs/core/src/skills/assets/samples/skill-installer/scripts/install-skill-from-github.py rename to codex-rs/skills/src/assets/samples/skill-installer/scripts/install-skill-from-github.py diff --git a/codex-rs/core/src/skills/assets/samples/skill-installer/scripts/list-skills.py b/codex-rs/skills/src/assets/samples/skill-installer/scripts/list-skills.py similarity index 100% rename from codex-rs/core/src/skills/assets/samples/skill-installer/scripts/list-skills.py rename to codex-rs/skills/src/assets/samples/skill-installer/scripts/list-skills.py diff --git a/codex-rs/skills/src/lib.rs b/codex-rs/skills/src/lib.rs new file mode 100644 index 0000000000..99cde06f6f --- /dev/null +++ b/codex-rs/skills/src/lib.rs @@ -0,0 +1,195 @@ +use codex_utils_absolute_path::AbsolutePathBuf; +use include_dir::Dir; +use std::collections::hash_map::DefaultHasher; +use std::fs; +use std::hash::Hash; +use std::hash::Hasher; +use std::path::Path; +use std::path::PathBuf; + +use thiserror::Error; + +const SYSTEM_SKILLS_DIR: Dir = include_dir::include_dir!("$CARGO_MANIFEST_DIR/src/assets/samples"); + +const SYSTEM_SKILLS_DIR_NAME: &str = ".system"; +const SKILLS_DIR_NAME: &str = "skills"; +const SYSTEM_SKILLS_MARKER_FILENAME: &str = ".codex-system-skills.marker"; +const SYSTEM_SKILLS_MARKER_SALT: &str = "v1"; + +/// Returns the on-disk cache location for embedded system skills. +/// +/// This is typically located at `CODEX_HOME/skills/.system`. +pub fn system_cache_root_dir(codex_home: &Path) -> PathBuf { + AbsolutePathBuf::try_from(codex_home) + .and_then(|codex_home| system_cache_root_dir_abs(&codex_home)) + .map(AbsolutePathBuf::into_path_buf) + .unwrap_or_else(|_| { + codex_home + .join(SKILLS_DIR_NAME) + .join(SYSTEM_SKILLS_DIR_NAME) + }) +} + +fn system_cache_root_dir_abs(codex_home: &AbsolutePathBuf) -> std::io::Result { + codex_home + .join(SKILLS_DIR_NAME)? + .join(SYSTEM_SKILLS_DIR_NAME) +} + +/// Installs embedded system skills into `CODEX_HOME/skills/.system`. +/// +/// Clears any existing system skills directory first and then writes the embedded +/// skills directory into place. +/// +/// To avoid doing unnecessary work on every startup, a marker file is written +/// with a fingerprint of the embedded directory. When the marker matches, the +/// install is skipped. +pub fn install_system_skills(codex_home: &Path) -> Result<(), SystemSkillsError> { + let codex_home = AbsolutePathBuf::try_from(codex_home) + .map_err(|source| SystemSkillsError::io("normalize codex home dir", source))?; + let skills_root_dir = codex_home + .join(SKILLS_DIR_NAME) + .map_err(|source| SystemSkillsError::io("resolve skills root dir", source))?; + fs::create_dir_all(skills_root_dir.as_path()) + .map_err(|source| SystemSkillsError::io("create skills root dir", source))?; + + let dest_system = system_cache_root_dir_abs(&codex_home) + .map_err(|source| SystemSkillsError::io("resolve system skills cache root dir", source))?; + + let marker_path = dest_system + .join(SYSTEM_SKILLS_MARKER_FILENAME) + .map_err(|source| SystemSkillsError::io("resolve system skills marker path", source))?; + let expected_fingerprint = embedded_system_skills_fingerprint(); + if dest_system.as_path().is_dir() + && read_marker(&marker_path).is_ok_and(|marker| marker == expected_fingerprint) + { + return Ok(()); + } + + if dest_system.as_path().exists() { + fs::remove_dir_all(dest_system.as_path()) + .map_err(|source| SystemSkillsError::io("remove existing system skills dir", source))?; + } + + write_embedded_dir(&SYSTEM_SKILLS_DIR, &dest_system)?; + fs::write(marker_path.as_path(), format!("{expected_fingerprint}\n")) + .map_err(|source| SystemSkillsError::io("write system skills marker", source))?; + Ok(()) +} + +fn read_marker(path: &AbsolutePathBuf) -> Result { + Ok(fs::read_to_string(path.as_path()) + .map_err(|source| SystemSkillsError::io("read system skills marker", source))? + .trim() + .to_string()) +} + +fn embedded_system_skills_fingerprint() -> String { + let mut items = Vec::new(); + collect_fingerprint_items(&SYSTEM_SKILLS_DIR, &mut items); + items.sort_unstable_by(|(a, _), (b, _)| a.cmp(b)); + + let mut hasher = DefaultHasher::new(); + SYSTEM_SKILLS_MARKER_SALT.hash(&mut hasher); + for (path, contents_hash) in items { + path.hash(&mut hasher); + contents_hash.hash(&mut hasher); + } + format!("{:x}", hasher.finish()) +} + +fn collect_fingerprint_items(dir: &Dir<'_>, items: &mut Vec<(String, Option)>) { + for entry in dir.entries() { + match entry { + include_dir::DirEntry::Dir(subdir) => { + items.push((subdir.path().to_string_lossy().to_string(), None)); + collect_fingerprint_items(subdir, items); + } + include_dir::DirEntry::File(file) => { + let mut file_hasher = DefaultHasher::new(); + file.contents().hash(&mut file_hasher); + items.push(( + file.path().to_string_lossy().to_string(), + Some(file_hasher.finish()), + )); + } + } + } +} + +/// Writes the embedded `include_dir::Dir` to disk under `dest`. +/// +/// Preserves the embedded directory structure. +fn write_embedded_dir(dir: &Dir<'_>, dest: &AbsolutePathBuf) -> Result<(), SystemSkillsError> { + fs::create_dir_all(dest.as_path()) + .map_err(|source| SystemSkillsError::io("create system skills dir", source))?; + + for entry in dir.entries() { + match entry { + include_dir::DirEntry::Dir(subdir) => { + let subdir_dest = dest.join(subdir.path()).map_err(|source| { + SystemSkillsError::io("resolve system skills subdir", source) + })?; + fs::create_dir_all(subdir_dest.as_path()).map_err(|source| { + SystemSkillsError::io("create system skills subdir", source) + })?; + write_embedded_dir(subdir, dest)?; + } + include_dir::DirEntry::File(file) => { + let path = dest.join(file.path()).map_err(|source| { + SystemSkillsError::io("resolve system skills file", source) + })?; + if let Some(parent) = path.as_path().parent() { + fs::create_dir_all(parent).map_err(|source| { + SystemSkillsError::io("create system skills file parent", source) + })?; + } + fs::write(path.as_path(), file.contents()) + .map_err(|source| SystemSkillsError::io("write system skill file", source))?; + } + } + } + + Ok(()) +} + +#[derive(Debug, Error)] +pub enum SystemSkillsError { + #[error("io error while {action}: {source}")] + Io { + action: &'static str, + #[source] + source: std::io::Error, + }, +} + +impl SystemSkillsError { + fn io(action: &'static str, source: std::io::Error) -> Self { + Self::Io { action, source } + } +} + +#[cfg(test)] +mod tests { + use super::SYSTEM_SKILLS_DIR; + use super::collect_fingerprint_items; + + #[test] + fn fingerprint_traverses_nested_entries() { + let mut items = Vec::new(); + collect_fingerprint_items(&SYSTEM_SKILLS_DIR, &mut items); + let mut paths: Vec = items.into_iter().map(|(path, _)| path).collect(); + paths.sort_unstable(); + + assert!( + paths + .binary_search_by(|probe| probe.as_str().cmp("skill-creator/SKILL.md")) + .is_ok() + ); + assert!( + paths + .binary_search_by(|probe| probe.as_str().cmp("skill-creator/scripts/init_skill.py")) + .is_ok() + ); + } +} From f5d7a7456854454a289e5f1179f6e35b2481c622 Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Sat, 21 Feb 2026 00:44:55 -0800 Subject: [PATCH 2/8] chore: delete empty codex-rs/code file (#12440) This file was added in https://github.com/openai/codex/pull/4195, but I think it may have been a mistake? --- codex-rs/code | 0 1 file changed, 0 insertions(+), 0 deletions(-) delete mode 100644 codex-rs/code diff --git a/codex-rs/code b/codex-rs/code deleted file mode 100644 index e69de29bb2..0000000000 From e7b6f38b58932e1f0d3512e5e69e4ad58d9022f1 Mon Sep 17 00:00:00 2001 From: pakrym-oai Date: Sat, 21 Feb 2026 00:50:27 -0800 Subject: [PATCH 3/8] Delete AggregatedStream (#12441) Used only in test --- codex-rs/codex-api/src/endpoint/aggregate.rs | 163 ------------------- codex-rs/codex-api/src/endpoint/mod.rs | 1 - codex-rs/codex-api/src/lib.rs | 1 - codex-rs/codex-api/tests/sse_end_to_end.rs | 68 -------- 4 files changed, 233 deletions(-) delete mode 100644 codex-rs/codex-api/src/endpoint/aggregate.rs diff --git a/codex-rs/codex-api/src/endpoint/aggregate.rs b/codex-rs/codex-api/src/endpoint/aggregate.rs deleted file mode 100644 index 6d8e785c82..0000000000 --- a/codex-rs/codex-api/src/endpoint/aggregate.rs +++ /dev/null @@ -1,163 +0,0 @@ -use crate::common::ResponseEvent; -use crate::common::ResponseStream; -use crate::error::ApiError; -use codex_protocol::models::ContentItem; -use codex_protocol::models::ReasoningItemContent; -use codex_protocol::models::ResponseItem; -use futures::Stream; -use std::collections::VecDeque; -use std::pin::Pin; -use std::task::Context; -use std::task::Poll; - -/// Stream adapter that merges token deltas into a single assistant message per turn. -pub struct AggregatedStream { - inner: ResponseStream, - cumulative: String, - cumulative_reasoning: String, - pending: VecDeque, -} - -impl Stream for AggregatedStream { - type Item = Result; - - fn poll_next(self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll> { - let this = self.get_mut(); - - if let Some(ev) = this.pending.pop_front() { - return Poll::Ready(Some(Ok(ev))); - } - - loop { - match Pin::new(&mut this.inner).poll_next(cx) { - Poll::Pending => return Poll::Pending, - Poll::Ready(None) => return Poll::Ready(None), - Poll::Ready(Some(Err(err))) => return Poll::Ready(Some(Err(err))), - Poll::Ready(Some(Ok(ResponseEvent::OutputItemDone(item)))) => { - let is_assistant_message = matches!( - &item, - ResponseItem::Message { role, .. } if role == "assistant" - ); - - if is_assistant_message { - if this.cumulative.is_empty() - && let ResponseItem::Message { content, .. } = &item - && let Some(text) = content.iter().find_map(|c| match c { - ContentItem::OutputText { text } => Some(text), - _ => None, - }) - { - this.cumulative.push_str(text); - } - continue; - } - - return Poll::Ready(Some(Ok(ResponseEvent::OutputItemDone(item)))); - } - Poll::Ready(Some(Ok(ResponseEvent::ServerReasoningIncluded(included)))) => { - return Poll::Ready(Some(Ok(ResponseEvent::ServerReasoningIncluded(included)))); - } - Poll::Ready(Some(Ok(ResponseEvent::RateLimits(snapshot)))) => { - return Poll::Ready(Some(Ok(ResponseEvent::RateLimits(snapshot)))); - } - Poll::Ready(Some(Ok(ResponseEvent::ModelsEtag(etag)))) => { - return Poll::Ready(Some(Ok(ResponseEvent::ModelsEtag(etag)))); - } - Poll::Ready(Some(Ok(ResponseEvent::ServerModel(model)))) => { - return Poll::Ready(Some(Ok(ResponseEvent::ServerModel(model)))); - } - Poll::Ready(Some(Ok(ResponseEvent::Completed { - response_id, - token_usage, - can_append: _can_append, - }))) => { - let mut emitted_any = false; - - if !this.cumulative_reasoning.is_empty() { - let aggregated_reasoning = ResponseItem::Reasoning { - id: String::new(), - summary: Vec::new(), - content: Some(vec![ReasoningItemContent::ReasoningText { - text: std::mem::take(&mut this.cumulative_reasoning), - }]), - encrypted_content: None, - }; - this.pending - .push_back(ResponseEvent::OutputItemDone(aggregated_reasoning)); - emitted_any = true; - } - - if !this.cumulative.is_empty() { - let aggregated_message = ResponseItem::Message { - id: None, - role: "assistant".to_string(), - content: vec![ContentItem::OutputText { - text: std::mem::take(&mut this.cumulative), - }], - end_turn: None, - phase: None, - }; - this.pending - .push_back(ResponseEvent::OutputItemDone(aggregated_message)); - emitted_any = true; - } - - if emitted_any { - this.pending.push_back(ResponseEvent::Completed { - response_id: response_id.clone(), - token_usage: token_usage.clone(), - can_append: false, - }); - if let Some(ev) = this.pending.pop_front() { - return Poll::Ready(Some(Ok(ev))); - } - } - - return Poll::Ready(Some(Ok(ResponseEvent::Completed { - response_id, - token_usage, - can_append: false, - }))); - } - Poll::Ready(Some(Ok(ResponseEvent::Created))) => continue, - Poll::Ready(Some(Ok(ResponseEvent::OutputTextDelta(delta)))) => { - this.cumulative.push_str(&delta); - continue; - } - Poll::Ready(Some(Ok(ResponseEvent::ReasoningContentDelta { - delta, - content_index: _, - }))) => { - this.cumulative_reasoning.push_str(&delta); - continue; - } - Poll::Ready(Some(Ok(ResponseEvent::ReasoningSummaryDelta { .. }))) => continue, - Poll::Ready(Some(Ok(ResponseEvent::ReasoningSummaryPartAdded { .. }))) => continue, - Poll::Ready(Some(Ok(ResponseEvent::OutputItemAdded(item)))) => { - return Poll::Ready(Some(Ok(ResponseEvent::OutputItemAdded(item)))); - } - } - } - } -} - -pub trait AggregateStreamExt { - fn aggregate(self) -> AggregatedStream; -} - -impl AggregateStreamExt for ResponseStream { - fn aggregate(self) -> AggregatedStream { - AggregatedStream::new(self) - } -} - -impl AggregatedStream { - fn new(inner: ResponseStream) -> Self { - AggregatedStream { - inner, - cumulative: String::new(), - cumulative_reasoning: String::new(), - pending: VecDeque::new(), - } - } -} diff --git a/codex-rs/codex-api/src/endpoint/mod.rs b/codex-rs/codex-api/src/endpoint/mod.rs index 981643904e..6a748e533d 100644 --- a/codex-rs/codex-api/src/endpoint/mod.rs +++ b/codex-rs/codex-api/src/endpoint/mod.rs @@ -1,4 +1,3 @@ -pub mod aggregate; pub mod compact; pub mod memories; pub mod models; diff --git a/codex-rs/codex-api/src/lib.rs b/codex-rs/codex-api/src/lib.rs index 23c08d7e2b..b9152ec576 100644 --- a/codex-rs/codex-api/src/lib.rs +++ b/codex-rs/codex-api/src/lib.rs @@ -25,7 +25,6 @@ pub use crate::common::ResponseEvent; pub use crate::common::ResponseStream; pub use crate::common::ResponsesApiRequest; pub use crate::common::create_text_param_for_request; -pub use crate::endpoint::aggregate::AggregateStreamExt; pub use crate::endpoint::compact::CompactClient; pub use crate::endpoint::memories::MemoriesClient; pub use crate::endpoint::models::ModelsClient; diff --git a/codex-rs/codex-api/tests/sse_end_to_end.rs b/codex-rs/codex-api/tests/sse_end_to_end.rs index 9f29033783..ca840d1b9f 100644 --- a/codex-rs/codex-api/tests/sse_end_to_end.rs +++ b/codex-rs/codex-api/tests/sse_end_to_end.rs @@ -3,7 +3,6 @@ use std::time::Duration; use anyhow::Result; use async_trait::async_trait; use bytes::Bytes; -use codex_api::AggregateStreamExt; use codex_api::AuthProvider; use codex_api::Provider; use codex_api::ResponseEvent; @@ -14,7 +13,6 @@ use codex_client::Request; use codex_client::Response; use codex_client::StreamResponse; use codex_client::TransportError; -use codex_protocol::models::ContentItem; use codex_protocol::models::ResponseItem; use futures::StreamExt; use http::HeaderMap; @@ -172,69 +170,3 @@ async fn responses_stream_parses_items_and_completed_end_to_end() -> Result<()> Ok(()) } - -#[tokio::test] -async fn responses_stream_aggregates_output_text_deltas() -> Result<()> { - let delta1 = serde_json::json!({ - "type": "response.output_text.delta", - "delta": "Hello, " - }); - - let delta2 = serde_json::json!({ - "type": "response.output_text.delta", - "delta": "world" - }); - - let completed = serde_json::json!({ - "type": "response.completed", - "response": { "id": "resp-agg" } - }); - - let body = build_responses_body(vec![delta1, delta2, completed]); - let transport = FixtureSseTransport::new(body); - let client = ResponsesClient::new(transport, provider("openai"), NoAuth); - - let stream = client - .stream( - serde_json::json!({"echo": true}), - HeaderMap::new(), - Compression::None, - None, - ) - .await?; - - let mut stream = stream.aggregate(); - let mut events = Vec::new(); - while let Some(ev) = stream.next().await { - events.push(ev?); - } - - let events: Vec = events - .into_iter() - .filter(|ev| !matches!(ev, ResponseEvent::RateLimits(_))) - .collect(); - - assert_eq!(events.len(), 2); - - match &events[0] { - ResponseEvent::OutputItemDone(ResponseItem::Message { content, .. }) => { - let mut aggregated = String::new(); - for item in content { - if let ContentItem::OutputText { text } = item { - aggregated.push_str(text); - } - } - assert_eq!(aggregated, "Hello, world"); - } - other => panic!("unexpected first event: {other:?}"), - } - - match &events[1] { - ResponseEvent::Completed { response_id, .. } => { - assert_eq!(response_id, "resp-agg"); - } - other => panic!("unexpected second event: {other:?}"), - } - - Ok(()) -} From b3202cbd581721774b73d2d9c90c56cc43974425 Mon Sep 17 00:00:00 2001 From: viyatb-oai Date: Sat, 21 Feb 2026 10:16:34 -0800 Subject: [PATCH 4/8] feat(linux-sandbox): implement proxy-only egress via TCP-UDS-TCP bridge (#11293) ## Summary - Implement Linux proxy-only routing in `codex-rs/linux-sandbox` with a two-stage bridge: host namespace `loopback TCP proxy endpoint -> UDS`, then bwrap netns `loopback TCP listener -> host UDS`. - Add hidden `--proxy-route-spec` plumbing for outer-to-inner stage handoff. - Fail closed in proxy mode when no valid loopback proxy endpoints can be routed. - Introduce explicit network seccomp modes: `Restricted` (legacy restricted networking) and `ProxyRouted` (allow INET/INET6 for routed proxy access, deny `AF_UNIX` and `socketpair`). - Enforce that proxy bridge/routing is bwrap-only by validating `--apply-seccomp-then-exec` requires `--use-bwrap-sandbox`. - Keep landlock-only flows unchanged (no proxy bridge behavior outside bwrap). --------- Co-authored-by: Codex <199175422+chatgpt-codex-connector[bot]@users.noreply.github.com> --- codex-rs/Cargo.lock | 2 + codex-rs/linux-sandbox/Cargo.toml | 2 + codex-rs/linux-sandbox/README.md | 5 + codex-rs/linux-sandbox/src/bwrap.rs | 34 +- codex-rs/linux-sandbox/src/landlock.rs | 168 +++- codex-rs/linux-sandbox/src/lib.rs | 2 + codex-rs/linux-sandbox/src/linux_run_main.rs | 168 ++-- .../linux-sandbox/src/linux_run_main_tests.rs | 159 ++++ codex-rs/linux-sandbox/src/proxy_routing.rs | 796 ++++++++++++++++++ .../tests/suite/managed_proxy.rs | 313 +++++++ codex-rs/linux-sandbox/tests/suite/mod.rs | 1 + 11 files changed, 1501 insertions(+), 149 deletions(-) create mode 100644 codex-rs/linux-sandbox/src/linux_run_main_tests.rs create mode 100644 codex-rs/linux-sandbox/src/proxy_routing.rs create mode 100644 codex-rs/linux-sandbox/tests/suite/managed_proxy.rs diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index 03276a7adb..31bc8d65a7 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -1921,9 +1921,11 @@ dependencies = [ "pkg-config", "pretty_assertions", "seccompiler", + "serde", "serde_json", "tempfile", "tokio", + "url", ] [[package]] diff --git a/codex-rs/linux-sandbox/Cargo.toml b/codex-rs/linux-sandbox/Cargo.toml index 63af5c81fa..88da2d7479 100644 --- a/codex-rs/linux-sandbox/Cargo.toml +++ b/codex-rs/linux-sandbox/Cargo.toml @@ -23,7 +23,9 @@ codex-utils-absolute-path = { workspace = true } landlock = { workspace = true } libc = { workspace = true } seccompiler = { workspace = true } +serde = { workspace = true, features = ["derive"] } serde_json = { workspace = true } +url = { workspace = true } [target.'cfg(target_os = "linux")'.dev-dependencies] pretty_assertions = { workspace = true } diff --git a/codex-rs/linux-sandbox/README.md b/codex-rs/linux-sandbox/README.md index 0abd641708..b03919c963 100644 --- a/codex-rs/linux-sandbox/README.md +++ b/codex-rs/linux-sandbox/README.md @@ -28,6 +28,11 @@ into this binary. - When enabled, the helper isolates the PID namespace via `--unshare-pid`. - When enabled and network is restricted without proxy routing, the helper also isolates the network namespace via `--unshare-net`. +- In managed proxy mode, the helper uses `--unshare-net` plus an internal + TCP->UDS->TCP routing bridge so tool traffic reaches only configured proxy + endpoints. +- In managed proxy mode, after the bridge is live, seccomp blocks new + AF_UNIX/socketpair creation for the user command. - When enabled, it mounts a fresh `/proc` via `--proc /proc` by default, but you can skip this in restrictive container environments with `--no-proc`. diff --git a/codex-rs/linux-sandbox/src/bwrap.rs b/codex-rs/linux-sandbox/src/bwrap.rs index 854774b781..d1fefc1747 100644 --- a/codex-rs/linux-sandbox/src/bwrap.rs +++ b/codex-rs/linux-sandbox/src/bwrap.rs @@ -49,8 +49,8 @@ pub(crate) enum BwrapNetworkMode { Isolated, /// Intended proxy-only mode. /// - /// Bubblewrap does not currently enforce proxy-only egress, so this is - /// treated as isolated for fail-closed behavior. + /// Bubblewrap enforces this by unsharing the network namespace. The + /// proxy-routing bridge is established by the helper process after startup. ProxyOnly, } @@ -323,6 +323,7 @@ fn find_first_non_existent_component(target_path: &Path) -> Option { mod tests { use super::*; use codex_protocol::protocol::SandboxPolicy; + use codex_utils_absolute_path::AbsolutePathBuf; use pretty_assertions::assert_eq; #[test] @@ -373,4 +374,33 @@ mod tests { ] ); } + + #[test] + fn mounts_dev_before_writable_dev_binds() { + let sandbox_policy = SandboxPolicy::WorkspaceWrite { + writable_roots: vec![AbsolutePathBuf::try_from(Path::new("/dev")).expect("/dev path")], + read_only_access: Default::default(), + network_access: false, + exclude_tmpdir_env_var: true, + exclude_slash_tmp: true, + }; + + let args = create_filesystem_args(&sandbox_policy, Path::new("/")).expect("bwrap fs args"); + assert_eq!( + args, + vec![ + "--ro-bind".to_string(), + "/".to_string(), + "/".to_string(), + "--dev".to_string(), + "/dev".to_string(), + "--bind".to_string(), + "/dev".to_string(), + "/dev".to_string(), + "--bind".to_string(), + "/".to_string(), + "/".to_string(), + ] + ); + } } diff --git a/codex-rs/linux-sandbox/src/landlock.rs b/codex-rs/linux-sandbox/src/landlock.rs index 621bfd5b60..f9477a1264 100644 --- a/codex-rs/linux-sandbox/src/landlock.rs +++ b/codex-rs/linux-sandbox/src/landlock.rs @@ -43,22 +43,26 @@ pub(crate) fn apply_sandbox_policy_to_current_thread( cwd: &Path, apply_landlock_fs: bool, allow_network_for_proxy: bool, + proxy_routed_network: bool, ) -> Result<()> { - let install_network_seccomp = - should_install_network_seccomp(sandbox_policy, allow_network_for_proxy); + let network_seccomp_mode = network_seccomp_mode( + sandbox_policy, + allow_network_for_proxy, + proxy_routed_network, + ); // `PR_SET_NO_NEW_PRIVS` is required for seccomp, but it also prevents // setuid privilege elevation. Many `bwrap` deployments rely on setuid, so // we avoid this unless we need seccomp or we are explicitly using the // legacy Landlock filesystem pipeline. - if install_network_seccomp + if network_seccomp_mode.is_some() || (apply_landlock_fs && !sandbox_policy.has_full_disk_write_access()) { set_no_new_privs()?; } - if install_network_seccomp { - install_network_seccomp_filter_on_current_thread()?; + if let Some(mode) = network_seccomp_mode { + install_network_seccomp_filter_on_current_thread(mode)?; } if apply_landlock_fs && !sandbox_policy.has_full_disk_write_access() { @@ -80,6 +84,12 @@ pub(crate) fn apply_sandbox_policy_to_current_thread( Ok(()) } +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum NetworkSeccompMode { + Restricted, + ProxyRouted, +} + fn should_install_network_seccomp( sandbox_policy: &SandboxPolicy, allow_network_for_proxy: bool, @@ -89,6 +99,20 @@ fn should_install_network_seccomp( !sandbox_policy.has_full_network_access() || allow_network_for_proxy } +fn network_seccomp_mode( + sandbox_policy: &SandboxPolicy, + allow_network_for_proxy: bool, + proxy_routed_network: bool, +) -> Option { + if !should_install_network_seccomp(sandbox_policy, allow_network_for_proxy) { + None + } else if proxy_routed_network { + Some(NetworkSeccompMode::ProxyRouted) + } else { + Some(NetworkSeccompMode::Restricted) + } +} + /// Enable `PR_SET_NO_NEW_PRIVS` so seccomp can be applied safely. fn set_no_new_privs() -> Result<()> { let result = unsafe { libc::prctl(libc::PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0) }; @@ -135,51 +159,87 @@ fn install_filesystem_landlock_rules_on_current_thread( Ok(()) } -/// Installs a seccomp filter that blocks outbound network access except for -/// AF_UNIX domain sockets. +/// Installs a seccomp filter for Linux network sandboxing. /// /// The filter is applied to the current thread so only the sandboxed child /// inherits it. -fn install_network_seccomp_filter_on_current_thread() -> std::result::Result<(), SandboxErr> { +fn install_network_seccomp_filter_on_current_thread( + mode: NetworkSeccompMode, +) -> std::result::Result<(), SandboxErr> { + fn deny_syscall(rules: &mut BTreeMap>, nr: i64) { + rules.insert(nr, vec![]); // empty rule vec = unconditional match + } + // Build rule map. let mut rules: BTreeMap> = BTreeMap::new(); - // Helper – insert unconditional deny rule for syscall number. - let mut deny_syscall = |nr: i64| { - rules.insert(nr, vec![]); // empty rule vec = unconditional match - }; + deny_syscall(&mut rules, libc::SYS_ptrace); + deny_syscall(&mut rules, libc::SYS_io_uring_setup); + deny_syscall(&mut rules, libc::SYS_io_uring_enter); + deny_syscall(&mut rules, libc::SYS_io_uring_register); - deny_syscall(libc::SYS_connect); - deny_syscall(libc::SYS_accept); - deny_syscall(libc::SYS_accept4); - deny_syscall(libc::SYS_bind); - deny_syscall(libc::SYS_listen); - deny_syscall(libc::SYS_getpeername); - deny_syscall(libc::SYS_getsockname); - deny_syscall(libc::SYS_shutdown); - deny_syscall(libc::SYS_sendto); - deny_syscall(libc::SYS_sendmmsg); - // NOTE: allowing recvfrom allows some tools like: `cargo clippy` to run - // with their socketpair + child processes for sub-proc management - // deny_syscall(libc::SYS_recvfrom); - deny_syscall(libc::SYS_recvmmsg); - deny_syscall(libc::SYS_getsockopt); - deny_syscall(libc::SYS_setsockopt); - deny_syscall(libc::SYS_ptrace); - deny_syscall(libc::SYS_io_uring_setup); - deny_syscall(libc::SYS_io_uring_enter); - deny_syscall(libc::SYS_io_uring_register); + match mode { + NetworkSeccompMode::Restricted => { + deny_syscall(&mut rules, libc::SYS_connect); + deny_syscall(&mut rules, libc::SYS_accept); + deny_syscall(&mut rules, libc::SYS_accept4); + deny_syscall(&mut rules, libc::SYS_bind); + deny_syscall(&mut rules, libc::SYS_listen); + deny_syscall(&mut rules, libc::SYS_getpeername); + deny_syscall(&mut rules, libc::SYS_getsockname); + deny_syscall(&mut rules, libc::SYS_shutdown); + deny_syscall(&mut rules, libc::SYS_sendto); + deny_syscall(&mut rules, libc::SYS_sendmmsg); + // NOTE: allowing recvfrom allows some tools like: `cargo clippy` + // to run with their socketpair + child processes for sub-proc + // management. + // deny_syscall(&mut rules, libc::SYS_recvfrom); + deny_syscall(&mut rules, libc::SYS_recvmmsg); + deny_syscall(&mut rules, libc::SYS_getsockopt); + deny_syscall(&mut rules, libc::SYS_setsockopt); - // For `socket` we allow AF_UNIX (arg0 == AF_UNIX) and deny everything else. - let unix_only_rule = SeccompRule::new(vec![SeccompCondition::new( - 0, // first argument (domain) - SeccompCmpArgLen::Dword, - SeccompCmpOp::Ne, - libc::AF_UNIX as u64, - )?])?; + // For `socket` we allow AF_UNIX (arg0 == AF_UNIX) and deny + // everything else. + let unix_only_rule = SeccompRule::new(vec![SeccompCondition::new( + 0, // first argument (domain) + SeccompCmpArgLen::Dword, + SeccompCmpOp::Ne, + libc::AF_UNIX as u64, + )?])?; - rules.insert(libc::SYS_socket, vec![unix_only_rule.clone()]); - rules.insert(libc::SYS_socketpair, vec![unix_only_rule]); // always deny (Unix can use socketpair but fine, keep open?) + rules.insert(libc::SYS_socket, vec![unix_only_rule.clone()]); + rules.insert(libc::SYS_socketpair, vec![unix_only_rule]); + } + NetworkSeccompMode::ProxyRouted => { + // In proxy-routed mode we allow IP sockets in the isolated + // namespace (used to reach the local TCP bridge) but deny all + // other socket families, including AF_UNIX. This prevents + // bypassing the routed bridge via new Unix sockets and narrows the + // socket surface in proxy-only mode. + let deny_non_ip_socket = SeccompRule::new(vec![ + SeccompCondition::new( + 0, + SeccompCmpArgLen::Dword, + SeccompCmpOp::Ne, + libc::AF_INET as u64, + )?, + SeccompCondition::new( + 0, + SeccompCmpArgLen::Dword, + SeccompCmpOp::Ne, + libc::AF_INET6 as u64, + )?, + ])?; + let deny_unix_socketpair = SeccompRule::new(vec![SeccompCondition::new( + 0, + SeccompCmpArgLen::Dword, + SeccompCmpOp::Eq, + libc::AF_UNIX as u64, + )?])?; + rules.insert(libc::SYS_socket, vec![deny_non_ip_socket]); + rules.insert(libc::SYS_socketpair, vec![deny_unix_socketpair]); + } + } let filter = SeccompFilter::new( rules, @@ -203,6 +263,8 @@ fn install_network_seccomp_filter_on_current_thread() -> std::result::Result<(), #[cfg(test)] mod tests { + use super::NetworkSeccompMode; + use super::network_seccomp_mode; use super::should_install_network_seccomp; use codex_protocol::protocol::SandboxPolicy; use pretty_assertions::assert_eq; @@ -234,4 +296,28 @@ mod tests { true )); } + + #[test] + fn managed_proxy_routes_use_proxy_routed_seccomp_mode() { + assert_eq!( + network_seccomp_mode(&SandboxPolicy::DangerFullAccess, true, true), + Some(NetworkSeccompMode::ProxyRouted) + ); + } + + #[test] + fn restricted_network_without_proxy_routing_uses_restricted_mode() { + assert_eq!( + network_seccomp_mode(&SandboxPolicy::new_read_only_policy(), false, false), + Some(NetworkSeccompMode::Restricted) + ); + } + + #[test] + fn full_network_without_managed_proxy_skips_network_seccomp_mode() { + assert_eq!( + network_seccomp_mode(&SandboxPolicy::DangerFullAccess, false, false), + None + ); + } } diff --git a/codex-rs/linux-sandbox/src/lib.rs b/codex-rs/linux-sandbox/src/lib.rs index 3347f3f92d..e364c19251 100644 --- a/codex-rs/linux-sandbox/src/lib.rs +++ b/codex-rs/linux-sandbox/src/lib.rs @@ -10,6 +10,8 @@ mod landlock; #[cfg(target_os = "linux")] mod linux_run_main; #[cfg(target_os = "linux")] +mod proxy_routing; +#[cfg(target_os = "linux")] mod vendored_bwrap; #[cfg(target_os = "linux")] diff --git a/codex-rs/linux-sandbox/src/linux_run_main.rs b/codex-rs/linux-sandbox/src/linux_run_main.rs index 84e7cb34fc..c9fdd06a32 100644 --- a/codex-rs/linux-sandbox/src/linux_run_main.rs +++ b/codex-rs/linux-sandbox/src/linux_run_main.rs @@ -10,6 +10,8 @@ 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::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; @@ -44,11 +46,15 @@ pub struct LandlockCommand { /// Internal compatibility flag. /// /// By default, restricted-network sandboxing uses isolated networking. - /// If set, sandbox setup switches to proxy-only network mode - /// (currently enforced the same as isolated networking). + /// If set, sandbox setup switches to proxy-only network mode with + /// managed routing bridges. #[arg(long = "allow-network-for-proxy", hide = true, default_value_t = false)] pub allow_network_for_proxy: bool, + /// Internal route spec used for managed proxy routing in bwrap mode. + #[arg(long = "proxy-route-spec", hide = true)] + pub proxy_route_spec: Option, + /// When set, skip mounting a fresh `/proc` even though PID isolation is /// still enabled. This is primarily intended for restrictive container /// environments that deny `--proc /proc`. @@ -74,6 +80,7 @@ pub fn run_main() -> ! { use_bwrap_sandbox, apply_seccomp_then_exec, allow_network_for_proxy, + proxy_route_spec, no_proc, command, } = LandlockCommand::parse(); @@ -81,15 +88,26 @@ pub fn run_main() -> ! { if command.is_empty() { panic!("No command specified to execute."); } + ensure_inner_stage_mode_is_valid(apply_seccomp_then_exec, use_bwrap_sandbox); // Inner stage: apply seccomp/no_new_privs after bubblewrap has already // established the filesystem view. if apply_seccomp_then_exec { + if allow_network_for_proxy { + let spec = proxy_route_spec + .as_deref() + .unwrap_or_else(|| panic!("managed proxy mode requires --proxy-route-spec")); + if let Err(err) = activate_proxy_routes_in_netns(spec) { + panic!("error activating Linux proxy routing bridge: {err}"); + } + } + let proxy_routing_active = allow_network_for_proxy; if let Err(e) = apply_sandbox_policy_to_current_thread( &sandbox_policy, &sandbox_policy_cwd, false, allow_network_for_proxy, + proxy_routing_active, ) { panic!("error applying Linux sandbox restrictions: {e:?}"); } @@ -102,6 +120,7 @@ pub fn run_main() -> ! { &sandbox_policy_cwd, false, allow_network_for_proxy, + false, ) { panic!("error applying Linux sandbox restrictions: {e:?}"); } @@ -112,11 +131,20 @@ pub fn run_main() -> ! { // Outer stage: bubblewrap first, then re-enter this binary in the // sandboxed environment to apply seccomp. This path never falls back // to legacy Landlock on failure. + let proxy_route_spec = + if allow_network_for_proxy { + Some(prepare_host_proxy_route_spec().unwrap_or_else(|err| { + panic!("failed to prepare host proxy routing bridge: {err}") + })) + } else { + None + }; let inner = build_inner_seccomp_command( &sandbox_policy_cwd, &sandbox_policy, use_bwrap_sandbox, allow_network_for_proxy, + proxy_route_spec, command, ); run_bwrap_with_proc_fallback( @@ -134,12 +162,19 @@ pub fn run_main() -> ! { &sandbox_policy_cwd, true, allow_network_for_proxy, + false, ) { panic!("error applying legacy Linux sandbox restrictions: {e:?}"); } exec_or_panic(command); } +fn ensure_inner_stage_mode_is_valid(apply_seccomp_then_exec: bool, use_bwrap_sandbox: bool) { + if apply_seccomp_then_exec && !use_bwrap_sandbox { + panic!("--apply-seccomp-then-exec requires --use-bwrap-sandbox"); + } +} + fn run_bwrap_with_proc_fallback( sandbox_policy_cwd: &Path, sandbox_policy: &codex_protocol::protocol::SandboxPolicy, @@ -147,14 +182,15 @@ fn run_bwrap_with_proc_fallback( mount_proc: bool, allow_network_for_proxy: bool, ) -> ! { + let network_mode = bwrap_network_mode(sandbox_policy, allow_network_for_proxy); let mut mount_proc = mount_proc; - if mount_proc && !preflight_proc_mount_support(sandbox_policy_cwd, sandbox_policy) { + if mount_proc && !preflight_proc_mount_support(sandbox_policy_cwd, sandbox_policy, network_mode) + { eprintln!("codex-linux-sandbox: bwrap could not mount /proc; retrying with --no-proc"); mount_proc = false; } - let network_mode = bwrap_network_mode(sandbox_policy, allow_network_for_proxy); let options = BwrapOptions { mount_proc, network_mode, @@ -202,19 +238,29 @@ fn build_bwrap_argv( fn preflight_proc_mount_support( sandbox_policy_cwd: &Path, sandbox_policy: &codex_protocol::protocol::SandboxPolicy, + network_mode: BwrapNetworkMode, ) -> bool { + let preflight_argv = + build_preflight_bwrap_argv(sandbox_policy_cwd, sandbox_policy, network_mode); + let stderr = run_bwrap_in_child_capture_stderr(preflight_argv); + !is_proc_mount_failure(stderr.as_str()) +} + +fn build_preflight_bwrap_argv( + sandbox_policy_cwd: &Path, + sandbox_policy: &codex_protocol::protocol::SandboxPolicy, + network_mode: BwrapNetworkMode, +) -> Vec { let preflight_command = vec![resolve_true_command()]; - let preflight_argv = build_bwrap_argv( + build_bwrap_argv( preflight_command, sandbox_policy, sandbox_policy_cwd, BwrapOptions { mount_proc: true, - network_mode: BwrapNetworkMode::FullAccess, + network_mode, }, - ); - let stderr = run_bwrap_in_child_capture_stderr(preflight_argv); - !is_proc_mount_failure(stderr.as_str()) + ) } fn resolve_true_command() -> String { @@ -318,6 +364,7 @@ fn build_inner_seccomp_command( sandbox_policy: &codex_protocol::protocol::SandboxPolicy, use_bwrap_sandbox: bool, allow_network_for_proxy: bool, + proxy_route_spec: Option, command: Vec, ) -> Vec { let current_exe = match std::env::current_exe() { @@ -342,6 +389,10 @@ fn build_inner_seccomp_command( } if allow_network_for_proxy { inner.push("--allow-network-for-proxy".to_string()); + let proxy_route_spec = proxy_route_spec + .unwrap_or_else(|| panic!("managed proxy mode requires a proxy route spec")); + inner.push("--proxy-route-spec".to_string()); + inner.push(proxy_route_spec); } inner.push("--".to_string()); inner.extend(command); @@ -371,100 +422,5 @@ fn exec_or_panic(command: Vec) -> ! { panic!("Failed to execvp {}: {err}", command[0].as_str()); } -#[cfg(test)] -mod tests { - use super::*; - use codex_protocol::protocol::SandboxPolicy; - use pretty_assertions::assert_eq; - - #[test] - fn detects_proc_mount_invalid_argument_failure() { - let stderr = "bwrap: Can't mount proc on /newroot/proc: Invalid argument"; - assert_eq!(is_proc_mount_failure(stderr), true); - } - - #[test] - fn detects_proc_mount_operation_not_permitted_failure() { - let stderr = "bwrap: Can't mount proc on /newroot/proc: Operation not permitted"; - assert_eq!(is_proc_mount_failure(stderr), true); - } - - #[test] - fn detects_proc_mount_permission_denied_failure() { - let stderr = "bwrap: Can't mount proc on /newroot/proc: Permission denied"; - assert_eq!(is_proc_mount_failure(stderr), true); - } - - #[test] - fn ignores_non_proc_mount_errors() { - let stderr = "bwrap: Can't bind mount /dev/null: Operation not permitted"; - assert_eq!(is_proc_mount_failure(stderr), false); - } - - #[test] - fn inserts_bwrap_argv0_before_command_separator() { - let argv = build_bwrap_argv( - vec!["/bin/true".to_string()], - &SandboxPolicy::new_read_only_policy(), - Path::new("/"), - BwrapOptions { - mount_proc: true, - network_mode: BwrapNetworkMode::FullAccess, - }, - ); - assert_eq!( - argv, - vec![ - "bwrap".to_string(), - "--new-session".to_string(), - "--die-with-parent".to_string(), - "--ro-bind".to_string(), - "/".to_string(), - "/".to_string(), - "--dev".to_string(), - "/dev".to_string(), - "--unshare-pid".to_string(), - "--proc".to_string(), - "/proc".to_string(), - "--argv0".to_string(), - "codex-linux-sandbox".to_string(), - "--".to_string(), - "/bin/true".to_string(), - ] - ); - } - - #[test] - fn inserts_unshare_net_when_network_isolation_requested() { - let argv = build_bwrap_argv( - vec!["/bin/true".to_string()], - &SandboxPolicy::new_read_only_policy(), - Path::new("/"), - BwrapOptions { - mount_proc: true, - network_mode: BwrapNetworkMode::Isolated, - }, - ); - assert_eq!(argv.contains(&"--unshare-net".to_string()), true); - } - - #[test] - fn inserts_unshare_net_when_proxy_only_network_mode_requested() { - let argv = build_bwrap_argv( - vec!["/bin/true".to_string()], - &SandboxPolicy::new_read_only_policy(), - Path::new("/"), - BwrapOptions { - mount_proc: true, - network_mode: BwrapNetworkMode::ProxyOnly, - }, - ); - assert_eq!(argv.contains(&"--unshare-net".to_string()), true); - } - - #[test] - fn proxy_only_mode_takes_precedence_over_full_network_policy() { - let mode = bwrap_network_mode(&SandboxPolicy::DangerFullAccess, true); - assert_eq!(mode, BwrapNetworkMode::ProxyOnly); - } -} +#[path = "linux_run_main_tests.rs"] +mod tests; diff --git a/codex-rs/linux-sandbox/src/linux_run_main_tests.rs b/codex-rs/linux-sandbox/src/linux_run_main_tests.rs new file mode 100644 index 0000000000..11471afbe7 --- /dev/null +++ b/codex-rs/linux-sandbox/src/linux_run_main_tests.rs @@ -0,0 +1,159 @@ +#[cfg(test)] +use super::*; +#[cfg(test)] +use codex_protocol::protocol::SandboxPolicy; + +#[test] +fn detects_proc_mount_invalid_argument_failure() { + let stderr = "bwrap: Can't mount proc on /newroot/proc: Invalid argument"; + assert!(is_proc_mount_failure(stderr)); +} + +#[test] +fn detects_proc_mount_operation_not_permitted_failure() { + let stderr = "bwrap: Can't mount proc on /newroot/proc: Operation not permitted"; + assert!(is_proc_mount_failure(stderr)); +} + +#[test] +fn detects_proc_mount_permission_denied_failure() { + let stderr = "bwrap: Can't mount proc on /newroot/proc: Permission denied"; + assert!(is_proc_mount_failure(stderr)); +} + +#[test] +fn ignores_non_proc_mount_errors() { + let stderr = "bwrap: Can't bind mount /dev/null: Operation not permitted"; + assert!(!is_proc_mount_failure(stderr)); +} + +#[test] +fn inserts_bwrap_argv0_before_command_separator() { + let argv = build_bwrap_argv( + vec!["/bin/true".to_string()], + &SandboxPolicy::new_read_only_policy(), + Path::new("/"), + BwrapOptions { + mount_proc: true, + network_mode: BwrapNetworkMode::FullAccess, + }, + ); + assert_eq!( + argv, + vec![ + "bwrap".to_string(), + "--new-session".to_string(), + "--die-with-parent".to_string(), + "--ro-bind".to_string(), + "/".to_string(), + "/".to_string(), + "--dev".to_string(), + "/dev".to_string(), + "--unshare-pid".to_string(), + "--proc".to_string(), + "/proc".to_string(), + "--argv0".to_string(), + "codex-linux-sandbox".to_string(), + "--".to_string(), + "/bin/true".to_string(), + ] + ); +} + +#[test] +fn inserts_unshare_net_when_network_isolation_requested() { + let argv = build_bwrap_argv( + vec!["/bin/true".to_string()], + &SandboxPolicy::new_read_only_policy(), + Path::new("/"), + BwrapOptions { + mount_proc: true, + network_mode: BwrapNetworkMode::Isolated, + }, + ); + assert!(argv.contains(&"--unshare-net".to_string())); +} + +#[test] +fn inserts_unshare_net_when_proxy_only_network_mode_requested() { + let argv = build_bwrap_argv( + vec!["/bin/true".to_string()], + &SandboxPolicy::new_read_only_policy(), + Path::new("/"), + BwrapOptions { + mount_proc: true, + network_mode: BwrapNetworkMode::ProxyOnly, + }, + ); + assert!(argv.contains(&"--unshare-net".to_string())); +} + +#[test] +fn proxy_only_mode_takes_precedence_over_full_network_policy() { + let mode = bwrap_network_mode(&SandboxPolicy::DangerFullAccess, true); + assert_eq!(mode, BwrapNetworkMode::ProxyOnly); +} + +#[test] +fn managed_proxy_preflight_argv_is_wrapped_for_full_access_policy() { + let mode = bwrap_network_mode(&SandboxPolicy::DangerFullAccess, true); + let argv = build_preflight_bwrap_argv(Path::new("/"), &SandboxPolicy::DangerFullAccess, mode); + assert!(argv.iter().any(|arg| arg == "--")); +} + +#[test] +fn managed_proxy_inner_command_includes_route_spec() { + let args = build_inner_seccomp_command( + Path::new("/tmp"), + &SandboxPolicy::new_read_only_policy(), + true, + true, + Some("{\"routes\":[]}".to_string()), + vec!["/bin/true".to_string()], + ); + + assert!(args.iter().any(|arg| arg == "--proxy-route-spec")); + assert!(args.iter().any(|arg| arg == "{\"routes\":[]}")); +} + +#[test] +fn non_managed_inner_command_omits_route_spec() { + let args = build_inner_seccomp_command( + Path::new("/tmp"), + &SandboxPolicy::new_read_only_policy(), + true, + false, + None, + vec!["/bin/true".to_string()], + ); + + assert!(!args.iter().any(|arg| arg == "--proxy-route-spec")); +} + +#[test] +fn managed_proxy_inner_command_requires_route_spec() { + let result = std::panic::catch_unwind(|| { + build_inner_seccomp_command( + Path::new("/tmp"), + &SandboxPolicy::new_read_only_policy(), + true, + true, + None, + vec!["/bin/true".to_string()], + ) + }); + assert!(result.is_err()); +} + +#[test] +fn apply_seccomp_then_exec_without_bwrap_panics() { + let result = std::panic::catch_unwind(|| ensure_inner_stage_mode_is_valid(true, false)); + assert!(result.is_err()); +} + +#[test] +fn valid_inner_stage_modes_do_not_panic() { + ensure_inner_stage_mode_is_valid(false, false); + ensure_inner_stage_mode_is_valid(false, true); + ensure_inner_stage_mode_is_valid(true, true); +} diff --git a/codex-rs/linux-sandbox/src/proxy_routing.rs b/codex-rs/linux-sandbox/src/proxy_routing.rs new file mode 100644 index 0000000000..f57472c8a1 --- /dev/null +++ b/codex-rs/linux-sandbox/src/proxy_routing.rs @@ -0,0 +1,796 @@ +use serde::Deserialize; +use serde::Serialize; +use std::collections::BTreeMap; +use std::collections::HashMap; +use std::fs::DirBuilder; +use std::fs::File; +use std::fs::Permissions; +use std::io; +use std::io::Read; +use std::io::Write; +use std::net::IpAddr; +use std::net::Ipv4Addr; +use std::net::SocketAddr; +use std::net::TcpListener; +use std::net::TcpStream; +use std::os::fd::FromRawFd; +use std::os::unix::fs::DirBuilderExt; +use std::os::unix::fs::PermissionsExt; +use std::os::unix::net::UnixListener; +use std::os::unix::net::UnixStream; +use std::path::Path; +use std::path::PathBuf; +use std::time::Duration; +use url::Url; + +const PROXY_ENV_KEYS: &[&str] = &[ + "HTTP_PROXY", + "HTTPS_PROXY", + "ALL_PROXY", + "FTP_PROXY", + "YARN_HTTP_PROXY", + "YARN_HTTPS_PROXY", + "NPM_CONFIG_HTTP_PROXY", + "NPM_CONFIG_HTTPS_PROXY", + "NPM_CONFIG_PROXY", + "BUNDLE_HTTP_PROXY", + "BUNDLE_HTTPS_PROXY", + "PIP_PROXY", + "DOCKER_HTTP_PROXY", + "DOCKER_HTTPS_PROXY", +]; + +const PROXY_SOCKET_DIR_PREFIX: &str = "codex-linux-sandbox-proxy-"; +const HOST_BRIDGE_READY: u8 = 1; +const LOOPBACK_INTERFACE_NAME: &[u8] = b"lo"; + +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] +pub(crate) struct ProxyRouteSpec { + routes: Vec, +} + +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] +struct ProxyRouteEntry { + env_key: String, + uds_path: PathBuf, +} + +#[derive(Debug, Clone, PartialEq, Eq)] +struct PlannedProxyRoute { + env_key: String, + endpoint: SocketAddr, +} + +#[derive(Debug, Clone, PartialEq, Eq)] +struct ProxyRoutePlan { + routes: Vec, + has_proxy_config: bool, +} + +pub(crate) fn prepare_host_proxy_route_spec() -> io::Result { + let env: HashMap = std::env::vars().collect(); + let plan = plan_proxy_routes(&env); + + if plan.routes.is_empty() { + let message = if plan.has_proxy_config { + "managed proxy mode requires parseable loopback proxy endpoints" + } else { + "managed proxy mode requires proxy environment variables" + }; + return Err(io::Error::new(io::ErrorKind::InvalidInput, message)); + } + + let socket_parent_dir = proxy_socket_parent_dir(); + let _ = cleanup_stale_proxy_socket_dirs_in(socket_parent_dir.as_path()); + + let socket_dir = create_proxy_socket_dir()?; + let mut socket_by_endpoint: BTreeMap = BTreeMap::new(); + let mut next_index = 0usize; + for route in &plan.routes { + if socket_by_endpoint.contains_key(&route.endpoint) { + continue; + } + let socket_path = socket_dir.join(format!("proxy-route-{next_index}.sock")); + next_index += 1; + socket_by_endpoint.insert(route.endpoint, socket_path); + } + + let mut host_bridge_pids = Vec::with_capacity(socket_by_endpoint.len()); + for (endpoint, socket_path) in &socket_by_endpoint { + host_bridge_pids.push(spawn_host_bridge(*endpoint, socket_path)?); + } + spawn_proxy_socket_dir_cleanup_worker(socket_dir, host_bridge_pids)?; + + let mut routes = Vec::with_capacity(plan.routes.len()); + for route in plan.routes { + let Some(uds_path) = socket_by_endpoint.get(&route.endpoint) else { + return Err(io::Error::other(format!( + "missing UDS path for endpoint {}", + route.endpoint + ))); + }; + routes.push(ProxyRouteEntry { + env_key: route.env_key, + uds_path: uds_path.clone(), + }); + } + + serde_json::to_string(&ProxyRouteSpec { routes }).map_err(io::Error::other) +} + +pub(crate) fn activate_proxy_routes_in_netns(serialized_spec: &str) -> io::Result<()> { + let spec: ProxyRouteSpec = serde_json::from_str(serialized_spec).map_err(io::Error::other)?; + + if spec.routes.is_empty() { + return Err(io::Error::new( + io::ErrorKind::InvalidInput, + "proxy routing spec contained no routes", + )); + } + + let mut local_port_by_uds_path: BTreeMap = BTreeMap::new(); + for route in &spec.routes { + if local_port_by_uds_path.contains_key(&route.uds_path) { + continue; + } + let local_port = spawn_local_bridge(route.uds_path.as_path())?; + local_port_by_uds_path.insert(route.uds_path.clone(), local_port); + } + + for route in spec.routes { + let Some(local_port) = local_port_by_uds_path.get(&route.uds_path) else { + return Err(io::Error::other(format!( + "missing local bridge port for UDS path {}", + route.uds_path.display() + ))); + }; + let original_value = std::env::var(&route.env_key).map_err(|_| { + io::Error::new( + io::ErrorKind::NotFound, + format!("missing proxy env key {}", route.env_key), + ) + })?; + let Some(rewritten) = rewrite_proxy_env_value(&original_value, *local_port) else { + return Err(io::Error::new( + io::ErrorKind::InvalidInput, + format!("could not rewrite proxy URL for env key {}", route.env_key), + )); + }; + // SAFETY: this helper process is single-threaded at this point, and + // env mutation happens before execing the user command. + unsafe { + std::env::set_var(route.env_key, rewritten); + } + } + + Ok(()) +} + +fn plan_proxy_routes(env: &HashMap) -> ProxyRoutePlan { + let mut routes = Vec::new(); + let mut has_proxy_config = false; + + for (key, value) in env { + if !is_proxy_env_key(key) { + continue; + } + + let trimmed = value.trim(); + if trimmed.is_empty() { + continue; + } + has_proxy_config = true; + + let Some(endpoint) = parse_loopback_proxy_endpoint(trimmed) else { + continue; + }; + routes.push(PlannedProxyRoute { + env_key: key.clone(), + endpoint, + }); + } + + routes.sort_by(|left, right| left.env_key.cmp(&right.env_key)); + ProxyRoutePlan { + routes, + has_proxy_config, + } +} + +fn is_proxy_env_key(key: &str) -> bool { + let upper = key.to_ascii_uppercase(); + PROXY_ENV_KEYS.contains(&upper.as_str()) +} + +fn parse_loopback_proxy_endpoint(proxy_url: &str) -> Option { + let candidate = if proxy_url.contains("://") { + proxy_url.to_string() + } else { + format!("http://{proxy_url}") + }; + + let parsed = Url::parse(&candidate).ok()?; + let host = parsed.host_str()?; + if !is_loopback_host(host) { + return None; + } + + let scheme = parsed.scheme().to_ascii_lowercase(); + let port = parsed + .port() + .unwrap_or_else(|| default_proxy_port(scheme.as_str())); + if port == 0 { + return None; + } + + let ip = if host.eq_ignore_ascii_case("localhost") { + IpAddr::V4(Ipv4Addr::LOCALHOST) + } else { + host.parse::().ok()? + }; + if ip.is_loopback() { + Some(SocketAddr::new(ip, port)) + } else { + None + } +} + +fn is_loopback_host(host: &str) -> bool { + host.eq_ignore_ascii_case("localhost") || host == "127.0.0.1" || host == "::1" +} + +fn default_proxy_port(scheme: &str) -> u16 { + match scheme { + "https" => 443, + "socks5" | "socks5h" | "socks4" | "socks4a" => 1080, + _ => 80, + } +} + +fn rewrite_proxy_env_value(proxy_url: &str, local_port: u16) -> Option { + let had_scheme = proxy_url.contains("://"); + let candidate = if had_scheme { + proxy_url.to_string() + } else { + format!("http://{proxy_url}") + }; + + let mut parsed = Url::parse(&candidate).ok()?; + parsed.set_host(Some("127.0.0.1")).ok()?; + parsed.set_port(Some(local_port)).ok()?; + let mut rewritten = parsed.to_string(); + if !had_scheme { + rewritten = rewritten + .strip_prefix("http://") + .unwrap_or(rewritten.as_str()) + .to_string(); + } + if !proxy_url.ends_with('/') + && !proxy_url.contains('?') + && !proxy_url.contains('#') + && rewritten.ends_with('/') + { + rewritten.pop(); + } + Some(rewritten) +} + +fn create_proxy_socket_dir() -> io::Result { + let temp_dir = proxy_socket_parent_dir(); + let pid = std::process::id(); + for attempt in 0..128 { + let candidate = temp_dir.join(format!("{PROXY_SOCKET_DIR_PREFIX}{pid}-{attempt}")); + // The bridge UDS paths live under a shared temp root, so the per-run + // directory should not be traversable by other processes. + let mut dir_builder = DirBuilder::new(); + dir_builder.mode(0o700); + match dir_builder.create(&candidate) { + Ok(()) => return Ok(candidate), + Err(err) if err.kind() == io::ErrorKind::AlreadyExists => continue, + Err(err) => return Err(err), + } + } + Err(io::Error::new( + io::ErrorKind::AlreadyExists, + format!( + "failed to allocate proxy routing temp dir under {}", + temp_dir.display() + ), + )) +} + +fn proxy_socket_parent_dir() -> PathBuf { + if let Some(codex_home) = std::env::var_os("CODEX_HOME") { + let candidate = PathBuf::from(codex_home).join("tmp"); + if ensure_private_proxy_socket_parent_dir(candidate.as_path()).is_ok() { + return candidate; + } + } + std::env::temp_dir() +} + +fn ensure_private_proxy_socket_parent_dir(path: &Path) -> io::Result<()> { + let mut dir_builder = DirBuilder::new(); + dir_builder.recursive(true); + dir_builder.mode(0o700); + match dir_builder.create(path) { + Ok(()) => {} + Err(err) if err.kind() == io::ErrorKind::AlreadyExists => {} + Err(err) => return Err(err), + } + std::fs::set_permissions(path, Permissions::from_mode(0o700)) +} + +fn cleanup_stale_proxy_socket_dirs_in(temp_dir: &Path) -> io::Result<()> { + for entry in std::fs::read_dir(temp_dir)? { + let entry = match entry { + Ok(entry) => entry, + Err(_) => continue, + }; + let file_type = match entry.file_type() { + Ok(file_type) => file_type, + Err(_) => continue, + }; + if !file_type.is_dir() { + continue; + } + + let file_name = entry.file_name(); + let file_name = file_name.to_string_lossy(); + let Some(owner_pid) = parse_proxy_socket_dir_owner_pid(file_name.as_ref()) else { + continue; + }; + if is_pid_alive(owner_pid) { + continue; + } + + let _ = cleanup_proxy_socket_dir(entry.path().as_path()); + } + + Ok(()) +} + +fn parse_proxy_socket_dir_owner_pid(file_name: &str) -> Option { + let suffix = file_name.strip_prefix(PROXY_SOCKET_DIR_PREFIX)?; + let (pid_raw, _) = suffix.split_once('-')?; + pid_raw.parse::().ok().filter(|pid| *pid != 0) +} + +fn is_pid_alive(pid: u32) -> bool { + let Ok(pid) = libc::pid_t::try_from(pid) else { + return false; + }; + is_pid_alive_raw(pid) +} + +fn is_pid_alive_raw(pid: libc::pid_t) -> bool { + let status = unsafe { libc::kill(pid, 0) }; + if status == 0 { + return true; + } + let err = io::Error::last_os_error(); + !matches!(err.raw_os_error(), Some(libc::ESRCH)) +} + +fn spawn_proxy_socket_dir_cleanup_worker( + socket_dir: PathBuf, + host_bridge_pids: Vec, +) -> io::Result<()> { + let pid = unsafe { libc::fork() }; + if pid < 0 { + return Err(io::Error::last_os_error()); + } + + if pid == 0 { + loop { + if host_bridge_pids + .iter() + .copied() + .all(|bridge_pid| !is_pid_alive_raw(bridge_pid)) + { + break; + } + std::thread::sleep(Duration::from_millis(100)); + } + + let _ = cleanup_proxy_socket_dir(socket_dir.as_path()); + unsafe { libc::_exit(0) }; + } + + Ok(()) +} + +fn cleanup_proxy_socket_dir(socket_dir: &Path) -> io::Result<()> { + for _ in 0..20 { + match std::fs::remove_dir_all(socket_dir) { + Ok(()) => return Ok(()), + Err(err) if err.kind() == io::ErrorKind::NotFound => return Ok(()), + Err(_) => std::thread::sleep(Duration::from_millis(100)), + } + } + + match std::fs::remove_dir_all(socket_dir) { + Ok(()) => Ok(()), + Err(err) if err.kind() == io::ErrorKind::NotFound => Ok(()), + Err(err) => Err(err), + } +} + +fn spawn_host_bridge(endpoint: SocketAddr, uds_path: &Path) -> io::Result { + let (read_fd, write_fd) = create_ready_pipe()?; + let pid = unsafe { libc::fork() }; + if pid < 0 { + let err = io::Error::last_os_error(); + close_fd(read_fd)?; + close_fd(write_fd)?; + return Err(err); + } + + if pid == 0 { + if close_fd(read_fd).is_err() { + unsafe { libc::_exit(1) }; + } + let result = run_host_bridge(endpoint, uds_path, write_fd); + if result.is_err() { + unsafe { libc::_exit(1) }; + } + unsafe { libc::_exit(0) }; + } + + close_fd(write_fd)?; + let mut ready = [0_u8; 1]; + let mut read_file = unsafe { File::from_raw_fd(read_fd) }; + read_file.read_exact(&mut ready)?; + if ready[0] != HOST_BRIDGE_READY { + return Err(io::Error::other( + "host bridge did not acknowledge readiness", + )); + } + Ok(pid) +} + +fn run_host_bridge(endpoint: SocketAddr, uds_path: &Path, ready_fd: libc::c_int) -> io::Result<()> { + set_parent_death_signal()?; + if uds_path.exists() { + std::fs::remove_file(uds_path)?; + } + let listener = UnixListener::bind(uds_path)?; + + let mut ready_file = unsafe { File::from_raw_fd(ready_fd) }; + ready_file.write_all(&[HOST_BRIDGE_READY])?; + drop(ready_file); + + loop { + let (unix_stream, _) = listener.accept()?; + std::thread::spawn(move || { + let tcp_stream = match TcpStream::connect(endpoint) { + Ok(stream) => stream, + Err(_) => return, + }; + let _ = proxy_bidirectional(tcp_stream, unix_stream); + }); + } +} + +fn spawn_local_bridge(uds_path: &Path) -> io::Result { + let (read_fd, write_fd) = create_ready_pipe()?; + let pid = unsafe { libc::fork() }; + if pid < 0 { + let err = io::Error::last_os_error(); + close_fd(read_fd)?; + close_fd(write_fd)?; + return Err(err); + } + + if pid == 0 { + if close_fd(read_fd).is_err() { + unsafe { libc::_exit(1) }; + } + let result = run_local_bridge(uds_path, write_fd); + if result.is_err() { + unsafe { libc::_exit(1) }; + } + unsafe { libc::_exit(0) }; + } + + close_fd(write_fd)?; + let mut port_bytes = [0_u8; 2]; + let mut read_file = unsafe { File::from_raw_fd(read_fd) }; + read_file.read_exact(&mut port_bytes)?; + Ok(u16::from_be_bytes(port_bytes)) +} + +fn run_local_bridge(uds_path: &Path, ready_fd: libc::c_int) -> io::Result<()> { + set_parent_death_signal()?; + let listener = bind_local_loopback_listener()?; + let port = listener.local_addr()?.port(); + + let mut ready_file = unsafe { File::from_raw_fd(ready_fd) }; + ready_file.write_all(&port.to_be_bytes())?; + drop(ready_file); + + let uds_path = uds_path.to_path_buf(); + loop { + let (tcp_stream, _) = listener.accept()?; + let socket_path = uds_path.clone(); + std::thread::spawn(move || { + let unix_stream = match UnixStream::connect(socket_path) { + Ok(stream) => stream, + Err(_) => return, + }; + let _ = proxy_bidirectional(tcp_stream, unix_stream); + }); + } +} + +fn bind_local_loopback_listener() -> io::Result { + match TcpListener::bind((Ipv4Addr::LOCALHOST, 0)) { + Ok(listener) => Ok(listener), + Err(bind_err) => { + let should_retry_after_lo_up = matches!( + bind_err.raw_os_error(), + Some(errno) if errno == libc::EADDRNOTAVAIL || errno == libc::ENETUNREACH + ); + if !should_retry_after_lo_up { + return Err(bind_err); + } + + ensure_loopback_interface_up()?; + TcpListener::bind((Ipv4Addr::LOCALHOST, 0)) + } + } +} + +fn ensure_loopback_interface_up() -> io::Result<()> { + let fd = unsafe { libc::socket(libc::AF_INET, libc::SOCK_DGRAM | libc::SOCK_CLOEXEC, 0) }; + if fd < 0 { + return Err(io::Error::last_os_error()); + } + + let mut ifreq = unsafe { std::mem::zeroed::() }; + for (index, byte) in LOOPBACK_INTERFACE_NAME.iter().copied().enumerate() { + ifreq.ifr_name[index] = byte as libc::c_char; + } + + let read_flags_result = + unsafe { libc::ioctl(fd, libc::SIOCGIFFLAGS as libc::Ioctl, &mut ifreq) }; + if read_flags_result < 0 { + let err = io::Error::last_os_error(); + let _ = close_fd(fd); + return Err(err); + } + + let current_flags = unsafe { ifreq.ifr_ifru.ifru_flags }; + let up_flag = libc::IFF_UP as libc::c_short; + if (current_flags & up_flag) != up_flag { + ifreq.ifr_ifru.ifru_flags = current_flags | up_flag; + let set_flags_result = + unsafe { libc::ioctl(fd, libc::SIOCSIFFLAGS as libc::Ioctl, &ifreq) }; + if set_flags_result < 0 { + let err = io::Error::last_os_error(); + let _ = close_fd(fd); + return Err(err); + } + } + + let mut addr_req = unsafe { std::mem::zeroed::() }; + for (index, byte) in LOOPBACK_INTERFACE_NAME.iter().copied().enumerate() { + addr_req.ifr_name[index] = byte as libc::c_char; + } + let loopback_addr = libc::sockaddr_in { + sin_family: libc::AF_INET as libc::sa_family_t, + sin_port: 0, + sin_addr: libc::in_addr { + s_addr: libc::htonl(libc::INADDR_LOOPBACK), + }, + sin_zero: [0; 8], + }; + unsafe { + addr_req.ifr_ifru.ifru_addr = + *(&loopback_addr as *const libc::sockaddr_in as *const libc::sockaddr); + } + let set_addr_result = unsafe { libc::ioctl(fd, libc::SIOCSIFADDR as libc::Ioctl, &addr_req) }; + if set_addr_result < 0 { + let err = io::Error::last_os_error(); + let allow_existing_or_immutable_addr = + matches!(err.raw_os_error(), Some(libc::EEXIST | libc::EPERM)); + if !allow_existing_or_immutable_addr { + let _ = close_fd(fd); + return Err(err); + } + } + + close_fd(fd) +} + +fn set_parent_death_signal() -> io::Result<()> { + let res = unsafe { libc::prctl(libc::PR_SET_PDEATHSIG, libc::SIGTERM) }; + if res != 0 { + Err(io::Error::last_os_error()) + } else if unsafe { libc::getppid() } == 1 { + Err(io::Error::other("parent process already exited")) + } else { + Ok(()) + } +} + +fn proxy_bidirectional(mut tcp_stream: TcpStream, mut unix_stream: UnixStream) -> io::Result<()> { + let mut tcp_reader = tcp_stream.try_clone()?; + let mut unix_writer = unix_stream.try_clone()?; + let tcp_to_unix = std::thread::spawn(move || std::io::copy(&mut tcp_reader, &mut unix_writer)); + let unix_to_tcp = std::io::copy(&mut unix_stream, &mut tcp_stream); + let tcp_to_unix = tcp_to_unix + .join() + .map_err(|_| io::Error::other("bridge thread panicked"))?; + tcp_to_unix?; + unix_to_tcp?; + Ok(()) +} + +fn create_ready_pipe() -> io::Result<(libc::c_int, libc::c_int)> { + let mut pipe_fds = [0; 2]; + let res = unsafe { libc::pipe2(pipe_fds.as_mut_ptr(), libc::O_CLOEXEC) }; + if res != 0 { + return Err(io::Error::last_os_error()); + } + Ok((pipe_fds[0], pipe_fds[1])) +} + +fn close_fd(fd: libc::c_int) -> io::Result<()> { + let res = unsafe { libc::close(fd) }; + if res < 0 { + return Err(io::Error::last_os_error()); + } + Ok(()) +} + +#[cfg(test)] +mod tests { + use super::PROXY_SOCKET_DIR_PREFIX; + use super::ProxyRouteEntry; + use super::ProxyRouteSpec; + use super::cleanup_proxy_socket_dir; + use super::cleanup_stale_proxy_socket_dirs_in; + use super::default_proxy_port; + use super::is_proxy_env_key; + use super::parse_loopback_proxy_endpoint; + use super::parse_proxy_socket_dir_owner_pid; + use super::plan_proxy_routes; + use super::rewrite_proxy_env_value; + use pretty_assertions::assert_eq; + use std::collections::HashMap; + use std::net::SocketAddr; + use std::path::PathBuf; + + #[test] + fn recognizes_proxy_env_keys_case_insensitively() { + assert_eq!(is_proxy_env_key("HTTP_PROXY"), true); + assert_eq!(is_proxy_env_key("http_proxy"), true); + assert_eq!(is_proxy_env_key("PATH"), false); + } + + #[test] + fn parses_loopback_proxy_endpoint() { + let endpoint = parse_loopback_proxy_endpoint("http://127.0.0.1:43128"); + assert_eq!( + endpoint, + Some( + "127.0.0.1:43128" + .parse::() + .expect("valid socket") + ) + ); + } + + #[test] + fn ignores_non_loopback_proxy_endpoint() { + assert_eq!( + parse_loopback_proxy_endpoint("http://example.com:3128"), + None + ); + } + + #[test] + fn plan_proxy_routes_only_includes_valid_loopback_endpoints() { + let mut env = HashMap::new(); + env.insert( + "HTTP_PROXY".to_string(), + "http://127.0.0.1:43128".to_string(), + ); + env.insert( + "HTTPS_PROXY".to_string(), + "http://example.com:3128".to_string(), + ); + env.insert("PATH".to_string(), "/usr/bin".to_string()); + + let plan = plan_proxy_routes(&env); + assert_eq!(plan.has_proxy_config, true); + assert_eq!(plan.routes.len(), 1); + assert_eq!(plan.routes[0].env_key, "HTTP_PROXY"); + assert_eq!( + plan.routes[0].endpoint, + "127.0.0.1:43128" + .parse::() + .expect("valid socket") + ); + } + + #[test] + fn rewrites_proxy_url_to_local_loopback_port() { + let rewritten = + rewrite_proxy_env_value("socks5h://127.0.0.1:8081", 43210).expect("rewritten value"); + assert_eq!(rewritten, "socks5h://127.0.0.1:43210"); + } + + #[test] + fn default_proxy_ports_match_expected_schemes() { + assert_eq!(default_proxy_port("http"), 80); + assert_eq!(default_proxy_port("https"), 443); + assert_eq!(default_proxy_port("socks5h"), 1080); + } + + #[test] + fn cleanup_proxy_socket_dir_removes_bridge_artifacts() { + let root = tempfile::tempdir().expect("tempdir should create"); + let socket_dir = root.path().join("codex-linux-sandbox-proxy-test"); + std::fs::create_dir(&socket_dir).expect("socket dir should create"); + let marker = socket_dir.join("bridge.sock"); + std::fs::write(&marker, b"test").expect("marker should write"); + + cleanup_proxy_socket_dir(socket_dir.as_path()).expect("cleanup should succeed"); + + assert_eq!(socket_dir.exists(), false); + } + + #[test] + fn proxy_route_spec_serialization_omits_proxy_urls() { + let spec = ProxyRouteSpec { + routes: vec![ProxyRouteEntry { + env_key: "HTTP_PROXY".to_string(), + uds_path: PathBuf::from("/tmp/proxy-route-0.sock"), + }], + }; + let serialized = serde_json::to_string(&spec).expect("proxy route spec should serialize"); + + assert_eq!( + serialized, + r#"{"routes":[{"env_key":"HTTP_PROXY","uds_path":"/tmp/proxy-route-0.sock"}]}"# + ); + } + + #[test] + fn parse_proxy_socket_dir_owner_pid_reads_owner_pid() { + assert_eq!( + parse_proxy_socket_dir_owner_pid("codex-linux-sandbox-proxy-1234-0"), + Some(1234) + ); + assert_eq!( + parse_proxy_socket_dir_owner_pid("codex-linux-sandbox-proxy-x"), + None + ); + assert_eq!(parse_proxy_socket_dir_owner_pid("not-a-proxy-dir"), None); + } + + #[test] + fn cleanup_stale_proxy_socket_dirs_removes_dead_pid_directories() { + let root = tempfile::tempdir().expect("tempdir should create"); + let dead_dir = root + .path() + .join(format!("{PROXY_SOCKET_DIR_PREFIX}{}-0", u32::MAX)); + std::fs::create_dir(&dead_dir).expect("dead dir should create"); + + let alive_dir = root + .path() + .join(format!("{PROXY_SOCKET_DIR_PREFIX}{}-1", std::process::id())); + std::fs::create_dir(&alive_dir).expect("alive dir should create"); + + let unrelated_dir = root.path().join("unrelated-proxy-dir"); + std::fs::create_dir(&unrelated_dir).expect("unrelated dir should create"); + + cleanup_stale_proxy_socket_dirs_in(root.path()).expect("stale cleanup should succeed"); + + assert_eq!(dead_dir.exists(), false); + assert_eq!(alive_dir.exists(), true); + assert_eq!(unrelated_dir.exists(), true); + } +} diff --git a/codex-rs/linux-sandbox/tests/suite/managed_proxy.rs b/codex-rs/linux-sandbox/tests/suite/managed_proxy.rs new file mode 100644 index 0000000000..e27930cdb5 --- /dev/null +++ b/codex-rs/linux-sandbox/tests/suite/managed_proxy.rs @@ -0,0 +1,313 @@ +#![cfg(target_os = "linux")] +#![allow(clippy::unwrap_used)] + +use codex_core::config::types::ShellEnvironmentPolicy; +use codex_core::exec_env::create_env; +use codex_protocol::protocol::SandboxPolicy; +use pretty_assertions::assert_eq; +use std::collections::HashMap; +use std::io::Read; +use std::io::Write; +use std::net::Ipv4Addr; +use std::net::TcpListener; +use std::process::Output; +use std::process::Stdio; +use std::time::Duration; +use tokio::process::Command; + +const BWRAP_UNAVAILABLE_ERR: &str = "build-time bubblewrap is not available in this build."; +const NETWORK_TIMEOUT_MS: u64 = 4_000; +const MANAGED_PROXY_PERMISSION_ERR_SNIPPETS: &[&str] = &[ + "loopback: Failed RTM_NEWADDR", + "loopback: Failed RTM_NEWLINK", + "setting up uid map: Permission denied", + "No permissions to create a new namespace", + "error isolating Linux network namespace for proxy mode", +]; + +const PROXY_ENV_KEYS: &[&str] = &[ + "HTTP_PROXY", + "HTTPS_PROXY", + "ALL_PROXY", + "FTP_PROXY", + "YARN_HTTP_PROXY", + "YARN_HTTPS_PROXY", + "NPM_CONFIG_HTTP_PROXY", + "NPM_CONFIG_HTTPS_PROXY", + "NPM_CONFIG_PROXY", + "BUNDLE_HTTP_PROXY", + "BUNDLE_HTTPS_PROXY", + "PIP_PROXY", + "DOCKER_HTTP_PROXY", + "DOCKER_HTTPS_PROXY", +]; + +fn create_env_from_core_vars() -> HashMap { + let policy = ShellEnvironmentPolicy::default(); + create_env(&policy, None) +} + +fn strip_proxy_env(env: &mut HashMap) { + for key in PROXY_ENV_KEYS { + env.remove(*key); + let lower = key.to_ascii_lowercase(); + env.remove(lower.as_str()); + } +} + +fn is_bwrap_unavailable_output(output: &Output) -> bool { + String::from_utf8_lossy(&output.stderr).contains(BWRAP_UNAVAILABLE_ERR) +} + +async fn should_skip_bwrap_tests() -> bool { + let mut env = create_env_from_core_vars(); + strip_proxy_env(&mut env); + + let output = run_linux_sandbox_direct( + &["bash", "-c", "true"], + &SandboxPolicy::new_read_only_policy(), + false, + env, + NETWORK_TIMEOUT_MS, + ) + .await; + is_bwrap_unavailable_output(&output) +} + +fn is_managed_proxy_permission_error(stderr: &str) -> bool { + MANAGED_PROXY_PERMISSION_ERR_SNIPPETS + .iter() + .any(|snippet| stderr.contains(snippet)) +} + +async fn managed_proxy_skip_reason() -> Option { + if should_skip_bwrap_tests().await { + return Some("vendored bwrap was not built in this environment".to_string()); + } + + let mut env = create_env_from_core_vars(); + strip_proxy_env(&mut env); + env.insert("HTTP_PROXY".to_string(), "http://127.0.0.1:9".to_string()); + + let output = run_linux_sandbox_direct( + &["bash", "-c", "true"], + &SandboxPolicy::DangerFullAccess, + true, + env, + NETWORK_TIMEOUT_MS, + ) + .await; + if output.status.success() { + return None; + } + + let stderr = String::from_utf8_lossy(&output.stderr); + if is_managed_proxy_permission_error(stderr.as_ref()) { + return Some(format!( + "managed proxy requires kernel namespace privileges unavailable here: {}", + stderr.trim() + )); + } + + None +} + +async fn run_linux_sandbox_direct( + command: &[&str], + sandbox_policy: &SandboxPolicy, + allow_network_for_proxy: bool, + env: HashMap, + timeout_ms: u64, +) -> Output { + let cwd = match std::env::current_dir() { + Ok(cwd) => cwd, + Err(err) => panic!("cwd should exist: {err}"), + }; + let policy_json = match serde_json::to_string(sandbox_policy) { + Ok(policy_json) => policy_json, + Err(err) => panic!("policy should serialize: {err}"), + }; + + let mut args = vec![ + "--sandbox-policy-cwd".to_string(), + cwd.to_string_lossy().to_string(), + "--sandbox-policy".to_string(), + policy_json, + "--use-bwrap-sandbox".to_string(), + ]; + if allow_network_for_proxy { + args.push("--allow-network-for-proxy".to_string()); + } + args.push("--".to_string()); + args.extend(command.iter().map(|entry| (*entry).to_string())); + + let mut cmd = Command::new(env!("CARGO_BIN_EXE_codex-linux-sandbox")); + cmd.args(args) + .current_dir(cwd) + .env_clear() + .envs(env) + .stdin(Stdio::null()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()); + let output = match tokio::time::timeout(Duration::from_millis(timeout_ms), cmd.output()).await { + Ok(output) => output, + Err(err) => panic!("sandbox command should not time out: {err}"), + }; + match output { + Ok(output) => output, + Err(err) => panic!("sandbox command should execute: {err}"), + } +} + +#[tokio::test] +async fn managed_proxy_mode_fails_closed_without_proxy_env() { + if let Some(skip_reason) = managed_proxy_skip_reason().await { + eprintln!("skipping managed proxy test: {skip_reason}"); + return; + } + + let mut env = create_env_from_core_vars(); + strip_proxy_env(&mut env); + + let output = run_linux_sandbox_direct( + &["bash", "-c", "true"], + &SandboxPolicy::DangerFullAccess, + true, + env, + NETWORK_TIMEOUT_MS, + ) + .await; + + assert_eq!(output.status.success(), false); + let stderr = String::from_utf8_lossy(&output.stderr); + assert!( + stderr.contains("managed proxy mode requires proxy environment variables"), + "expected fail-closed managed-proxy message, got stderr: {stderr}" + ); +} + +#[tokio::test] +async fn managed_proxy_mode_routes_through_bridge_and_blocks_direct_egress() { + if let Some(skip_reason) = managed_proxy_skip_reason().await { + eprintln!("skipping managed proxy test: {skip_reason}"); + return; + } + + let listener = TcpListener::bind((Ipv4Addr::LOCALHOST, 0)).expect("bind proxy listener"); + let proxy_port = listener + .local_addr() + .expect("proxy listener local addr") + .port(); + let (request_tx, request_rx) = std::sync::mpsc::channel(); + std::thread::spawn(move || { + let (mut stream, _) = listener.accept().expect("accept proxy connection"); + stream + .set_read_timeout(Some(Duration::from_secs(3))) + .expect("set read timeout"); + let mut buf = [0_u8; 4096]; + let read = stream.read(&mut buf).expect("read proxy request"); + let request = String::from_utf8_lossy(&buf[..read]).to_string(); + request_tx.send(request).expect("send proxy request"); + stream + .write_all(b"HTTP/1.1 200 OK\r\nContent-Length: 2\r\n\r\nOK") + .expect("write proxy response"); + }); + + let mut env = create_env_from_core_vars(); + strip_proxy_env(&mut env); + env.insert( + "HTTP_PROXY".to_string(), + format!("http://127.0.0.1:{proxy_port}"), + ); + + let routed_output = run_linux_sandbox_direct( + &[ + "bash", + "-c", + "proxy=\"${HTTP_PROXY#*://}\"; host=\"${proxy%%:*}\"; port=\"${proxy##*:}\"; exec 3<>/dev/tcp/${host}/${port}; printf 'GET http://example.com/ HTTP/1.1\\r\\nHost: example.com\\r\\n\\r\\n' >&3; IFS= read -r line <&3; printf '%s\\n' \"$line\"", + ], + &SandboxPolicy::DangerFullAccess, + true, + env.clone(), + NETWORK_TIMEOUT_MS, + ) + .await; + + assert_eq!( + routed_output.status.success(), + true, + "expected routed command to execute successfully; status={:?}; stdout={}; stderr={}", + routed_output.status.code(), + String::from_utf8_lossy(&routed_output.stdout), + String::from_utf8_lossy(&routed_output.stderr) + ); + let stdout = String::from_utf8_lossy(&routed_output.stdout); + assert!( + stdout.contains("HTTP/1.1 200 OK"), + "expected bridge-routed proxy response, got stdout: {stdout}" + ); + + let request = request_rx + .recv_timeout(Duration::from_secs(3)) + .expect("expected proxy request"); + assert!( + request.contains("GET http://example.com/ HTTP/1.1"), + "expected HTTP proxy absolute-form request, got request: {request}" + ); + + let direct_egress_output = run_linux_sandbox_direct( + &["bash", "-c", "echo hi > /dev/tcp/192.0.2.1/80"], + &SandboxPolicy::DangerFullAccess, + true, + env, + NETWORK_TIMEOUT_MS, + ) + .await; + assert_eq!(direct_egress_output.status.success(), false); +} + +#[tokio::test] +async fn managed_proxy_mode_denies_af_unix_creation_for_user_command() { + if let Some(skip_reason) = managed_proxy_skip_reason().await { + eprintln!("skipping managed proxy test: {skip_reason}"); + return; + } + + let python_available = Command::new("bash") + .arg("-c") + .arg("command -v python3 >/dev/null") + .status() + .await + .expect("python3 probe should execute") + .success(); + if !python_available { + eprintln!("skipping managed proxy AF_UNIX test: python3 is unavailable"); + return; + } + + let mut env = create_env_from_core_vars(); + strip_proxy_env(&mut env); + env.insert("HTTP_PROXY".to_string(), "http://127.0.0.1:9".to_string()); + + let output = run_linux_sandbox_direct( + &[ + "python3", + "-c", + "import socket,sys\ntry:\n socket.socket(socket.AF_UNIX, socket.SOCK_STREAM)\nexcept PermissionError:\n sys.exit(0)\nexcept OSError:\n sys.exit(2)\nsys.exit(1)\n", + ], + &SandboxPolicy::DangerFullAccess, + true, + env, + NETWORK_TIMEOUT_MS, + ) + .await; + + assert_eq!( + output.status.code(), + Some(0), + "expected AF_UNIX creation to be denied cleanly for user command; status={:?}; stdout={}; stderr={}", + output.status.code(), + String::from_utf8_lossy(&output.stdout), + String::from_utf8_lossy(&output.stderr) + ); +} diff --git a/codex-rs/linux-sandbox/tests/suite/mod.rs b/codex-rs/linux-sandbox/tests/suite/mod.rs index d2a6bfa148..8ef44a61a4 100644 --- a/codex-rs/linux-sandbox/tests/suite/mod.rs +++ b/codex-rs/linux-sandbox/tests/suite/mod.rs @@ -1,2 +1,3 @@ // Aggregates all former standalone integration tests as modules. mod landlock; +mod managed_proxy; From 5a635f3427101033564070a35a5e99ae534cd941 Mon Sep 17 00:00:00 2001 From: sayan-oai Date: Sat, 21 Feb 2026 11:39:02 -0800 Subject: [PATCH 5/8] profile-level model_catalog_json overrie (#12410) enable `model-catalog_json` config value on `ConfigProfile` as well --- codex-rs/core/config.schema.json | 8 ++++++++ codex-rs/core/src/config/mod.rs | 7 ++++++- codex-rs/core/src/config/profile.rs | 2 ++ 3 files changed, 16 insertions(+), 1 deletion(-) diff --git a/codex-rs/core/config.schema.json b/codex-rs/core/config.schema.json index 23963196a1..addbcbe313 100644 --- a/codex-rs/core/config.schema.json +++ b/codex-rs/core/config.schema.json @@ -440,6 +440,14 @@ "model": { "type": "string" }, + "model_catalog_json": { + "allOf": [ + { + "$ref": "#/definitions/AbsolutePathBuf" + } + ], + "description": "Optional path to a JSON model catalog (applied on startup only)." + }, "model_instructions_file": { "allOf": [ { diff --git a/codex-rs/core/src/config/mod.rs b/codex-rs/core/src/config/mod.rs index ca3978b73d..ddc5aa9cdd 100644 --- a/codex-rs/core/src/config/mod.rs +++ b/codex-rs/core/src/config/mod.rs @@ -1908,7 +1908,12 @@ impl Config { let review_model = override_review_model.or(cfg.review_model); let check_for_update_on_startup = cfg.check_for_update_on_startup.unwrap_or(true); - let model_catalog = load_model_catalog(cfg.model_catalog_json.clone())?; + let model_catalog = load_model_catalog( + config_profile + .model_catalog_json + .clone() + .or(cfg.model_catalog_json.clone()), + )?; let log_dir = cfg .log_dir diff --git a/codex-rs/core/src/config/profile.rs b/codex-rs/core/src/config/profile.rs index 3c7d575cdc..61b02adcd0 100644 --- a/codex-rs/core/src/config/profile.rs +++ b/codex-rs/core/src/config/profile.rs @@ -27,6 +27,8 @@ pub struct ConfigProfile { pub plan_mode_reasoning_effort: Option, pub model_reasoning_summary: Option, pub model_verbosity: Option, + /// Optional path to a JSON model catalog (applied on startup only). + pub model_catalog_json: Option, pub personality: Option, pub chatgpt_base_url: Option, /// Optional path to a file containing model instructions. From a6b2bacb5b8bb485a52d979d6d53bf1e639811d7 Mon Sep 17 00:00:00 2001 From: Eric Traut Date: Sat, 21 Feb 2026 11:55:03 -0800 Subject: [PATCH 6/8] Prevent replayed runtime events from forcing active status (#12420) Fixes #11852 Resume replay was applying transient runtime events (`TurnStarted`, `StreamError`) as if they were live, which could leave the TUI stuck in a stale `Working` / `Reconnecting...` state after resuming an interrupted reconnect. This change makes replay transcript-oriented for these events by: - skipping retry-status restoration for replayed non-stream events - ignoring replayed `TurnStarted` for task-running state - ignoring replayed `StreamError` for reconnect/status UI Also adds TUI regression tests and snapshot coverage for the interrupted reconnect replay case. --- codex-rs/tui/src/chatwidget.rs | 36 +++- ...ayed_interrupted_reconnect_footer_row.snap | 6 + codex-rs/tui/src/chatwidget/tests.rs | 154 ++++++++++++++++++ 3 files changed, 189 insertions(+), 7 deletions(-) create mode 100644 codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__replayed_interrupted_reconnect_footer_row.snap diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index 238a60730c..9bfddd18e4 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -832,6 +832,12 @@ fn remap_placeholders_for_message(message: UserMessage, next_label: &mut usize) } } +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +enum ReplayKind { + ResumeInitialMessages, + ThreadSnapshot, +} + impl ChatWidget { /// Synchronize the bottom-pane "task running" indicator with the current lifecycles. /// @@ -4002,13 +4008,13 @@ impl ChatWidget { continue; } // `id: None` indicates a synthetic/fake id coming from replay. - self.dispatch_event_msg(None, msg, true); + self.dispatch_event_msg(None, msg, Some(ReplayKind::ResumeInitialMessages)); } } pub(crate) fn handle_codex_event(&mut self, event: Event) { let Event { id, msg } = event; - self.dispatch_event_msg(Some(id), msg, false); + self.dispatch_event_msg(Some(id), msg, None); } pub(crate) fn handle_codex_event_replay(&mut self, event: Event) { @@ -4016,7 +4022,7 @@ impl ChatWidget { if matches!(msg, EventMsg::ShutdownComplete) { return; } - self.dispatch_event_msg(None, msg, true); + self.dispatch_event_msg(None, msg, Some(ReplayKind::ThreadSnapshot)); } /// Dispatch a protocol `EventMsg` to the appropriate handler. @@ -4024,9 +4030,17 @@ impl ChatWidget { /// `id` is `Some` for live events and `None` for replayed events from /// `replay_initial_messages()`. Callers should treat `None` as a "fake" id /// that must not be used to correlate follow-up actions. - fn dispatch_event_msg(&mut self, id: Option, msg: EventMsg, from_replay: bool) { + fn dispatch_event_msg( + &mut self, + id: Option, + msg: EventMsg, + replay_kind: Option, + ) { + let from_replay = replay_kind.is_some(); + let is_resume_initial_replay = + matches!(replay_kind, Some(ReplayKind::ResumeInitialMessages)); let is_stream_error = matches!(&msg, EventMsg::StreamError(_)); - if !is_stream_error { + if !is_resume_initial_replay && !is_stream_error { self.restore_retry_status_header_if_present(); } @@ -4061,7 +4075,11 @@ impl ChatWidget { self.on_agent_reasoning_final(); } EventMsg::AgentReasoningSectionBreak(_) => self.on_reasoning_section_break(), - EventMsg::TurnStarted(_) => self.on_task_started(), + EventMsg::TurnStarted(_) => { + if !is_resume_initial_replay { + self.on_task_started(); + } + } EventMsg::TurnComplete(TurnCompleteEvent { last_agent_message, .. }) => self.on_task_complete(last_agent_message, from_replay), @@ -4151,7 +4169,11 @@ impl ChatWidget { message, additional_details, .. - }) => self.on_stream_error(message, additional_details), + }) => { + if !is_resume_initial_replay { + self.on_stream_error(message, additional_details); + } + } EventMsg::UserMessage(ev) => { if from_replay { self.on_user_message_event(ev); diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__replayed_interrupted_reconnect_footer_row.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__replayed_interrupted_reconnect_footer_row.snap new file mode 100644 index 0000000000..eb3183f574 --- /dev/null +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__replayed_interrupted_reconnect_footer_row.snap @@ -0,0 +1,6 @@ +--- +source: tui/src/chatwidget/tests.rs +assertion_line: 7060 +expression: header +--- +› Ask Codex to do anything diff --git a/codex-rs/tui/src/chatwidget/tests.rs b/codex-rs/tui/src/chatwidget/tests.rs index b9f5ddf814..e634d3ba39 100644 --- a/codex-rs/tui/src/chatwidget/tests.rs +++ b/codex-rs/tui/src/chatwidget/tests.rs @@ -7228,6 +7228,160 @@ async fn stream_error_updates_status_indicator() { assert_eq!(status.details(), Some(details)); } +#[tokio::test] +async fn replayed_turn_started_does_not_mark_task_running() { + let (mut chat, _rx, _op_rx) = make_chatwidget_manual(None).await; + + chat.replay_initial_messages(vec![EventMsg::TurnStarted(TurnStartedEvent { + turn_id: "turn-1".to_string(), + model_context_window: None, + collaboration_mode_kind: ModeKind::Default, + })]); + + assert!(!chat.bottom_pane.is_task_running()); + assert!(chat.bottom_pane.status_widget().is_none()); +} + +#[tokio::test] +async fn thread_snapshot_replayed_turn_started_marks_task_running() { + let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(None).await; + + chat.handle_codex_event_replay(Event { + id: "turn-1".into(), + msg: EventMsg::TurnStarted(TurnStartedEvent { + turn_id: "turn-1".to_string(), + model_context_window: None, + collaboration_mode_kind: ModeKind::Default, + }), + }); + + drain_insert_history(&mut rx); + assert!(chat.bottom_pane.is_task_running()); + let status = chat + .bottom_pane + .status_widget() + .expect("status indicator should be visible"); + assert_eq!(status.header(), "Working"); +} + +#[tokio::test] +async fn replayed_stream_error_does_not_set_retry_status_or_status_indicator() { + let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(None).await; + chat.set_status_header("Idle".to_string()); + + chat.replay_initial_messages(vec![EventMsg::StreamError(StreamErrorEvent { + message: "Reconnecting... 2/5".to_string(), + codex_error_info: Some(CodexErrorInfo::Other), + additional_details: Some("Idle timeout waiting for SSE".to_string()), + })]); + + let cells = drain_insert_history(&mut rx); + assert!( + cells.is_empty(), + "expected no history cell for replayed StreamError event" + ); + assert_eq!(chat.current_status_header, "Idle"); + assert!(chat.retry_status_header.is_none()); + assert!(chat.bottom_pane.status_widget().is_none()); +} + +#[tokio::test] +async fn thread_snapshot_replayed_stream_recovery_restores_previous_status_header() { + let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(None).await; + + chat.handle_codex_event_replay(Event { + id: "task".into(), + msg: EventMsg::TurnStarted(TurnStartedEvent { + turn_id: "turn-1".to_string(), + model_context_window: None, + collaboration_mode_kind: ModeKind::Default, + }), + }); + drain_insert_history(&mut rx); + + chat.handle_codex_event_replay(Event { + id: "retry".into(), + msg: EventMsg::StreamError(StreamErrorEvent { + message: "Reconnecting... 1/5".to_string(), + codex_error_info: Some(CodexErrorInfo::Other), + additional_details: None, + }), + }); + drain_insert_history(&mut rx); + + chat.handle_codex_event_replay(Event { + id: "delta".into(), + msg: EventMsg::AgentMessageDelta(AgentMessageDeltaEvent { + delta: "hello".to_string(), + }), + }); + + let status = chat + .bottom_pane + .status_widget() + .expect("status indicator should be visible"); + assert_eq!(status.header(), "Working"); + assert_eq!(status.details(), None); + assert!(chat.retry_status_header.is_none()); +} + +#[tokio::test] +async fn resume_replay_interrupted_reconnect_does_not_leave_stale_working_state() { + let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(None).await; + chat.set_status_header("Idle".to_string()); + + chat.replay_initial_messages(vec![ + EventMsg::TurnStarted(TurnStartedEvent { + turn_id: "turn-1".to_string(), + model_context_window: None, + collaboration_mode_kind: ModeKind::Default, + }), + EventMsg::StreamError(StreamErrorEvent { + message: "Reconnecting... 1/5".to_string(), + codex_error_info: Some(CodexErrorInfo::Other), + additional_details: None, + }), + EventMsg::AgentMessageDelta(AgentMessageDeltaEvent { + delta: "hello".to_string(), + }), + ]); + + let cells = drain_insert_history(&mut rx); + assert!( + cells.is_empty(), + "expected no history cells for replayed interrupted reconnect sequence" + ); + assert!(!chat.bottom_pane.is_task_running()); + assert!(chat.bottom_pane.status_widget().is_none()); + assert_eq!(chat.current_status_header, "Idle"); + assert!(chat.retry_status_header.is_none()); +} + +#[tokio::test] +async fn replayed_interrupted_reconnect_footer_row_snapshot() { + let (mut chat, _rx, _op_rx) = make_chatwidget_manual(None).await; + + chat.replay_initial_messages(vec![ + EventMsg::TurnStarted(TurnStartedEvent { + turn_id: "turn-1".to_string(), + model_context_window: None, + collaboration_mode_kind: ModeKind::Default, + }), + EventMsg::StreamError(StreamErrorEvent { + message: "Reconnecting... 2/5".to_string(), + codex_error_info: Some(CodexErrorInfo::Other), + additional_details: Some("Idle timeout waiting for SSE".to_string()), + }), + ]); + + let header = render_bottom_first_row(&chat, 80); + assert!( + !header.contains("Reconnecting") && !header.contains("Working"), + "expected replayed interrupted reconnect to avoid active status row, got {header:?}" + ); + assert_snapshot!("replayed_interrupted_reconnect_footer_row", header); +} + #[tokio::test] async fn stream_error_restores_hidden_status_indicator() { let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(None).await; From b17148f13a2e261d36884ab3fa105d235426bf5a Mon Sep 17 00:00:00 2001 From: pakrym-oai Date: Sat, 21 Feb 2026 12:08:04 -0800 Subject: [PATCH 7/8] Prefer v2 websockets if available (#12428) And also cleanup settings flow to avoid reading many separate flags. --------- Co-authored-by: Codex --- codex-rs/core/src/client.rs | 136 ++++++++++-------- codex-rs/core/src/codex.rs | 20 +-- codex-rs/core/src/lib.rs | 2 + codex-rs/core/tests/responses_headers.rs | 10 +- codex-rs/core/tests/suite/client.rs | 4 +- .../core/tests/suite/client_websockets.rs | 60 +++++++- 6 files changed, 153 insertions(+), 79 deletions(-) diff --git a/codex-rs/core/src/client.rs b/codex-rs/core/src/client.rs index 6f33910ce7..5239841041 100644 --- a/codex-rs/core/src/client.rs +++ b/codex-rs/core/src/client.rs @@ -92,6 +92,7 @@ use crate::auth::RefreshTokenError; use crate::client_common::Prompt; use crate::client_common::ResponseEvent; use crate::client_common::ResponseStream; +use crate::config::Config; use crate::default_client::build_reqwest_client; use crate::error::CodexErr; use crate::error::Result; @@ -107,6 +108,28 @@ pub const X_CODEX_TURN_METADATA_HEADER: &str = "x-codex-turn-metadata"; pub const X_RESPONSESAPI_INCLUDE_TIMING_METRICS_HEADER: &str = "x-responsesapi-include-timing-metrics"; const RESPONSES_WEBSOCKETS_V2_BETA_HEADER_VALUE: &str = "responses_websockets=2026-02-06"; + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum ResponsesWebsocketVersion { + V1, + V2, +} + +pub fn ws_version_from_features(config: &Config) -> Option { + match ( + config + .features + .enabled(crate::features::Feature::ResponsesWebsockets), + config + .features + .enabled(crate::features::Feature::ResponsesWebsocketsV2), + ) { + (_, true) => Some(ResponsesWebsocketVersion::V2), + (true, false) => Some(ResponsesWebsocketVersion::V1), + (false, false) => None, + } +} + /// Session-scoped state shared by all [`ModelClient`] clones. /// /// This is intentionally kept minimal so `ModelClient` does not need to hold a full `Config`. Most @@ -118,8 +141,7 @@ struct ModelClientState { provider: ModelProviderInfo, session_source: SessionSource, model_verbosity: Option, - enable_responses_websockets: bool, - enable_responses_websockets_v2: bool, + responses_websocket_version: Option, enable_request_compression: bool, include_timing_metrics: bool, beta_features_header: Option, @@ -209,14 +231,11 @@ impl ModelClient { provider: ModelProviderInfo, session_source: SessionSource, model_verbosity: Option, - enable_responses_websockets: bool, - enable_responses_websockets_v2: bool, + responses_websocket_version: Option, enable_request_compression: bool, include_timing_metrics: bool, beta_features_header: Option, ) -> Self { - let enable_responses_websockets = - enable_responses_websockets || enable_responses_websockets_v2; Self { state: Arc::new(ModelClientState { auth_manager, @@ -224,8 +243,7 @@ impl ModelClient { provider, session_source, model_verbosity, - enable_responses_websockets, - enable_responses_websockets_v2, + responses_websocket_version, enable_request_compression, include_timing_metrics, beta_features_header, @@ -367,26 +385,25 @@ impl ModelClient { request_telemetry } - /// Returns whether this session is configured to use Responses-over-WebSocket. + /// Returns the active Responses-over-WebSocket version for this session. /// /// This combines provider capability and feature gating; both must be true for websocket paths /// to be eligible. - pub fn responses_websocket_enabled(&self, model_info: &ModelInfo) -> bool { - self.state.provider.supports_websockets - && (self.state.enable_responses_websockets - || self.state.enable_responses_websockets_v2 - || model_info.prefer_websockets) - } - - fn responses_websockets_v2_enabled(&self) -> bool { - self.state.enable_responses_websockets_v2 - } - - /// Returns whether websocket transport has been permanently disabled for this session. /// - /// Once set by fallback activation, subsequent turns must stay on HTTP transport. - fn websockets_disabled(&self) -> bool { - self.state.disable_websockets.load(Ordering::Relaxed) + /// If websockets are only enabled via model preference (no explicit feature flag), default to + /// v1 behavior. + pub fn active_ws_version(&self, model_info: &ModelInfo) -> Option { + if !self.state.provider.supports_websockets + || self.state.disable_websockets.load(Ordering::Relaxed) + { + return None; + } + + match self.state.responses_websocket_version { + Some(version) => Some(version), + None if model_info.prefer_websockets => Some(ResponsesWebsocketVersion::V1), + None => None, + } } /// Returns auth + provider configuration resolved from the current session auth state. @@ -419,10 +436,12 @@ impl ModelClient { otel_manager: &OtelManager, api_provider: codex_api::Provider, api_auth: CoreAuthProvider, + ws_version: ResponsesWebsocketVersion, turn_state: Option>>, turn_metadata_header: Option<&str>, ) -> std::result::Result { - let headers = self.build_websocket_headers(turn_state.as_ref(), turn_metadata_header); + let headers = + self.build_websocket_headers(ws_version, turn_state.as_ref(), turn_metadata_header); let websocket_telemetry = ModelClientSession::build_websocket_telemetry(otel_manager); ApiWebSocketResponsesClient::new(api_provider, api_auth) .connect( @@ -440,6 +459,7 @@ impl ModelClient { /// replayed on reconnect within the same turn. fn build_websocket_headers( &self, + ws_version: ResponsesWebsocketVersion, turn_state: Option<&Arc>>, turn_metadata_header: Option<&str>, ) -> ApiHeaderMap { @@ -452,10 +472,9 @@ impl ModelClient { headers.extend(build_conversation_headers(Some( self.state.conversation_id.to_string(), ))); - let responses_websockets_beta_header = if self.responses_websockets_v2_enabled() { - RESPONSES_WEBSOCKETS_V2_BETA_HEADER_VALUE - } else { - OPENAI_BETA_RESPONSES_WEBSOCKETS + let responses_websockets_beta_header = match ws_version { + ResponsesWebsocketVersion::V2 => RESPONSES_WEBSOCKETS_V2_BETA_HEADER_VALUE, + ResponsesWebsocketVersion::V1 => OPENAI_BETA_RESPONSES_WEBSOCKETS, }; headers.insert( OPENAI_BETA_HEADER, @@ -628,35 +647,39 @@ impl ModelClientSession { &mut self, payload: ResponseCreateWsRequest, request: &ResponsesApiRequest, + ws_version: ResponsesWebsocketVersion, ) -> ResponsesWsRequest { let Some(last_response) = self.get_last_response() else { return ResponsesWsRequest::ResponseCreate(payload); }; - let responses_websockets_v2_enabled = self.client.responses_websockets_v2_enabled(); - if !responses_websockets_v2_enabled && !last_response.can_append { - trace!("incremental request failed, can't append"); + let Some(append_items) = self.get_incremental_items(request, Some(&last_response)) else { return ResponsesWsRequest::ResponseCreate(payload); - } - let incremental_items = self.get_incremental_items(request, Some(&last_response)); - if let Some(append_items) = incremental_items { - if responses_websockets_v2_enabled && !last_response.response_id.is_empty() { - let payload = ResponseCreateWsRequest { + }; + + match ws_version { + ResponsesWebsocketVersion::V2 => { + if last_response.response_id.is_empty() { + trace!("incremental request failed, no previous response id"); + return ResponsesWsRequest::ResponseCreate(payload); + } + + ResponsesWsRequest::ResponseCreate(ResponseCreateWsRequest { previous_response_id: Some(last_response.response_id), input: append_items, ..payload - }; - return ResponsesWsRequest::ResponseCreate(payload); + }) } - - if !responses_websockets_v2_enabled { - return ResponsesWsRequest::ResponseAppend(ResponseAppendWsRequest { + ResponsesWebsocketVersion::V1 => { + if !last_response.can_append { + trace!("incremental request failed, can't append"); + return ResponsesWsRequest::ResponseCreate(payload); + } + ResponsesWsRequest::ResponseAppend(ResponseAppendWsRequest { input: append_items, client_metadata: payload.client_metadata, - }); + }) } } - - ResponsesWsRequest::ResponseCreate(payload) } /// Opportunistically warms a websocket for this turn-scoped client session. @@ -667,10 +690,9 @@ impl ModelClientSession { otel_manager: &OtelManager, model_info: &ModelInfo, ) -> std::result::Result<(), ApiError> { - if !self.client.responses_websocket_enabled(model_info) || self.client.websockets_disabled() - { + let Some(ws_version) = self.client.active_ws_version(model_info) else { return Ok(()); - } + }; if self.connection.is_some() { return Ok(()); } @@ -687,6 +709,7 @@ impl ModelClientSession { otel_manager, client_setup.api_provider, client_setup.api_auth, + ws_version, Some(Arc::clone(&self.turn_state)), None, ) @@ -701,6 +724,7 @@ impl ModelClientSession { otel_manager: &OtelManager, api_provider: codex_api::Provider, api_auth: CoreAuthProvider, + ws_version: ResponsesWebsocketVersion, turn_metadata_header: Option<&str>, options: &ApiResponsesOptions, ) -> std::result::Result<&ApiWebSocketConnection, ApiError> { @@ -722,6 +746,7 @@ impl ModelClientSession { otel_manager, api_provider, api_auth, + ws_version, Some(turn_state), turn_metadata_header, ) @@ -818,6 +843,7 @@ impl ModelClientSession { &mut self, prompt: &Prompt, model_info: &ModelInfo, + ws_version: ResponsesWebsocketVersion, otel_manager: &OtelManager, effort: Option, summary: ReasoningSummaryConfig, @@ -850,6 +876,7 @@ impl ModelClientSession { otel_manager, client_setup.api_provider, client_setup.api_auth, + ws_version, turn_metadata_header, &options, ) @@ -870,7 +897,7 @@ impl ModelClientSession { Err(err) => return Err(map_api_error(err)), } - let ws_request = self.prepare_websocket_request(ws_payload, &request); + let ws_request = self.prepare_websocket_request(ws_payload, &request, ws_version); let stream_result = self .connection @@ -928,14 +955,12 @@ impl ModelClientSession { let wire_api = self.client.state.provider.wire_api; match wire_api { WireApi::Responses => { - let websocket_enabled = self.client.responses_websocket_enabled(model_info) - && !self.client.websockets_disabled(); - - if websocket_enabled { + if let Some(ws_version) = self.client.active_ws_version(model_info) { match self .stream_responses_websocket( prompt, model_info, + ws_version, otel_manager, effort, summary, @@ -974,7 +999,7 @@ impl ModelClientSession { otel_manager: &OtelManager, model_info: &ModelInfo, ) -> bool { - let websocket_enabled = self.client.responses_websocket_enabled(model_info); + let websocket_enabled = self.client.active_ws_version(model_info).is_some(); let activated = self.activate_http_fallback(websocket_enabled); if activated { warn!("falling back to HTTP"); @@ -1224,8 +1249,7 @@ mod tests { provider, session_source, None, - false, - false, + None, false, false, None, diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index 6c9c9a678f..55a8062187 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -45,6 +45,7 @@ use crate::terminal; use crate::truncate::TruncationPolicy; use crate::turn_metadata::TurnMetadataState; use crate::util::error_or_panic; +use crate::ws_version_from_features; use async_channel::Receiver; use async_channel::Sender; use codex_hooks::HookEvent; @@ -1335,9 +1336,7 @@ impl Session { session_configuration.provider.clone(), session_configuration.session_source.clone(), config.model_verbosity, - config.features.enabled(Feature::ResponsesWebsockets) - || config.features.enabled(Feature::ResponsesWebsocketsV2), - config.features.enabled(Feature::ResponsesWebsocketsV2), + ws_version_from_features(config.as_ref()), config.features.enabled(Feature::EnableRequestCompression), config.features.enabled(Feature::RuntimeMetrics), Self::build_model_client_beta_features_header(config.as_ref()), @@ -5198,10 +5197,11 @@ async fn run_sampling_request( // transient reconnect messages. In debug builds, keep full visibility for diagnosis. let report_error = retries > 1 || cfg!(debug_assertions) - || !sess + || sess .services .model_client - .responses_websocket_enabled(&turn_context.model_info); + .active_ws_version(&turn_context.model_info) + .is_none(); if report_error { // Surface retry information to any UI/front‑end so the @@ -7844,10 +7844,7 @@ mod tests { session_configuration.provider.clone(), session_configuration.session_source.clone(), config.model_verbosity, - model_info.prefer_websockets - || config.features.enabled(Feature::ResponsesWebsockets) - || config.features.enabled(Feature::ResponsesWebsocketsV2), - config.features.enabled(Feature::ResponsesWebsocketsV2), + ws_version_from_features(config.as_ref()), config.features.enabled(Feature::EnableRequestCompression), config.features.enabled(Feature::RuntimeMetrics), Session::build_model_client_beta_features_header(config.as_ref()), @@ -8000,10 +7997,7 @@ mod tests { session_configuration.provider.clone(), session_configuration.session_source.clone(), config.model_verbosity, - model_info.prefer_websockets - || config.features.enabled(Feature::ResponsesWebsockets) - || config.features.enabled(Feature::ResponsesWebsocketsV2), - config.features.enabled(Feature::ResponsesWebsocketsV2), + ws_version_from_features(config.as_ref()), config.features.enabled(Feature::EnableRequestCompression), config.features.enabled(Feature::RuntimeMetrics), Session::build_model_client_beta_features_header(config.as_ref()), diff --git a/codex-rs/core/src/lib.rs b/codex-rs/core/src/lib.rs index a0ad85a421..e40905e314 100644 --- a/codex-rs/core/src/lib.rs +++ b/codex-rs/core/src/lib.rs @@ -157,6 +157,8 @@ pub use zsh_exec_bridge::maybe_run_zsh_exec_wrapper_mode; pub use client::ModelClient; pub use client::ModelClientSession; +pub use client::ResponsesWebsocketVersion; +pub use client::ws_version_from_features; pub use client_common::Prompt; pub use client_common::REVIEW_PROMPT; pub use client_common::ResponseEvent; diff --git a/codex-rs/core/tests/responses_headers.rs b/codex-rs/core/tests/responses_headers.rs index b392081eeb..126dc2c288 100644 --- a/codex-rs/core/tests/responses_headers.rs +++ b/codex-rs/core/tests/responses_headers.rs @@ -6,6 +6,7 @@ use codex_core::ModelClient; use codex_core::ModelProviderInfo; use codex_core::Prompt; use codex_core::ResponseEvent; +use codex_core::ResponsesWebsocketVersion; use codex_core::WireApi; use codex_otel::OtelManager; use codex_otel::TelemetryAuthMode; @@ -91,8 +92,7 @@ async fn responses_stream_includes_subagent_header_on_review() { provider.clone(), session_source, config.model_verbosity, - false, - false, + None::, false, false, None, @@ -197,8 +197,7 @@ async fn responses_stream_includes_subagent_header_on_other() { provider.clone(), session_source, config.model_verbosity, - false, - false, + None::, false, false, None, @@ -302,8 +301,7 @@ async fn responses_respects_model_info_overrides_from_config() { provider.clone(), session_source, config.model_verbosity, - false, - false, + None::, false, false, None, diff --git a/codex-rs/core/tests/suite/client.rs b/codex-rs/core/tests/suite/client.rs index 77a2ba33e0..5fc94a033e 100644 --- a/codex-rs/core/tests/suite/client.rs +++ b/codex-rs/core/tests/suite/client.rs @@ -4,6 +4,7 @@ use codex_core::ModelProviderInfo; use codex_core::NewThread; use codex_core::Prompt; use codex_core::ResponseEvent; +use codex_core::ResponsesWebsocketVersion; use codex_core::ThreadManager; use codex_core::WireApi; use codex_core::auth::AuthCredentialsStoreMode; @@ -1353,8 +1354,7 @@ async fn azure_responses_request_includes_store_and_reasoning_ids() { provider.clone(), SessionSource::Exec, config.model_verbosity, - false, - false, + None::, false, false, None, diff --git a/codex-rs/core/tests/suite/client_websockets.rs b/codex-rs/core/tests/suite/client_websockets.rs index 54e197e568..d8b1940d15 100755 --- a/codex-rs/core/tests/suite/client_websockets.rs +++ b/codex-rs/core/tests/suite/client_websockets.rs @@ -8,6 +8,7 @@ use codex_core::ResponseEvent; use codex_core::WireApi; use codex_core::X_RESPONSESAPI_INCLUDE_TIMING_METRICS_HEADER; use codex_core::features::Feature; +use codex_core::ws_version_from_features; use codex_otel::OtelManager; use codex_otel::TelemetryAuthMode; use codex_otel::metrics::MetricsClient; @@ -319,6 +320,62 @@ async fn responses_websocket_v2_requests_use_v2_when_model_prefers_websockets() server.shutdown().await; } +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn responses_websocket_v2_wins_when_both_features_enabled() { + skip_if_no_network!(); + + let server = start_websocket_server(vec![vec![ + vec![ + ev_response_created("resp-1"), + ev_assistant_message("msg-1", "assistant output"), + ev_done_with_id("resp-1"), + ], + vec![ev_response_created("resp-2"), ev_completed("resp-2")], + ]]) + .await; + + let harness = websocket_harness_with_options(&server, false, true, true, false).await; + let mut client_session = harness.client.new_session(); + let prompt_one = prompt_with_input(vec![message_item("hello")]); + let prompt_two = prompt_with_input(vec![ + message_item("hello"), + assistant_message_item("msg-1", "assistant output"), + message_item("second"), + ]); + + stream_until_complete(&mut client_session, &harness, &prompt_one).await; + stream_until_complete(&mut client_session, &harness, &prompt_two).await; + + let connection = server.single_connection(); + assert_eq!(connection.len(), 2); + let second = connection.get(1).expect("missing request").body_json(); + assert_eq!(second["type"].as_str(), Some("response.create")); + assert_eq!(second["previous_response_id"].as_str(), Some("resp-1")); + assert_eq!( + second["input"], + serde_json::to_value(&prompt_two.input[2..]).unwrap() + ); + + let handshake = server.single_handshake(); + let openai_beta_header = handshake + .header(OPENAI_BETA_HEADER) + .expect("missing OpenAI-Beta header"); + assert!( + openai_beta_header + .split(',') + .map(str::trim) + .any(|value| value == WS_V2_BETA_HEADER_VALUE) + ); + assert!( + !openai_beta_header + .split(',') + .map(str::trim) + .any(|value| value == OPENAI_BETA_RESPONSES_WEBSOCKETS) + ); + + server.shutdown().await; +} + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] #[traced_test] async fn responses_websocket_emits_websocket_telemetry_events() { @@ -1251,8 +1308,7 @@ async fn websocket_harness_with_options( provider.clone(), SessionSource::Exec, config.model_verbosity, - websocket_enabled, - websocket_v2_enabled, + ws_version_from_features(&config), false, runtime_metrics_enabled, None, From 9902581c6bc00dcf8508cd7ca9e8c97504f22b4b Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Sat, 21 Feb 2026 13:05:44 -0800 Subject: [PATCH 8/8] fix: update approval_policy and sandbox_policy in TurnContext to be Constrained --- codex-rs/core/src/apply_patch.rs | 4 +- codex-rs/core/src/codex.rs | 58 +++++++++++-------- codex-rs/core/src/context_manager/updates.rs | 8 ++- codex-rs/core/src/mcp/skill_dependencies.rs | 4 +- codex-rs/core/src/mcp_tool_call.rs | 4 +- .../core/src/tools/handlers/apply_patch.rs | 16 ++++- .../core/src/tools/handlers/multi_agents.rs | 11 ++-- codex-rs/core/src/tools/handlers/shell.rs | 16 +++-- .../core/src/tools/handlers/unified_exec.rs | 4 +- codex-rs/core/src/tools/js_repl/mod.rs | 32 +++++++--- codex-rs/core/src/tools/network_approval.rs | 2 +- codex-rs/core/src/unified_exec/mod.rs | 8 ++- .../core/src/unified_exec/process_manager.rs | 6 +- 13 files changed, 112 insertions(+), 61 deletions(-) diff --git a/codex-rs/core/src/apply_patch.rs b/codex-rs/core/src/apply_patch.rs index 714d9c40fe..0b9cca1c9d 100644 --- a/codex-rs/core/src/apply_patch.rs +++ b/codex-rs/core/src/apply_patch.rs @@ -38,8 +38,8 @@ pub(crate) async fn apply_patch( ) -> InternalApplyPatchInvocation { match assess_patch_safety( &action, - turn_context.approval_policy, - &turn_context.sandbox_policy, + turn_context.approval_policy.value(), + turn_context.sandbox_policy.get(), &turn_context.cwd, turn_context.windows_sandbox_level, ) { diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index 55a8062187..832592d406 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -558,8 +558,8 @@ pub(crate) struct TurnContext { pub(crate) user_instructions: Option, pub(crate) collaboration_mode: CollaborationMode, pub(crate) personality: Option, - pub(crate) approval_policy: AskForApproval, - pub(crate) sandbox_policy: SandboxPolicy, + pub(crate) approval_policy: Constrained, + pub(crate) sandbox_policy: Constrained, pub(crate) network: Option, pub(crate) windows_sandbox_level: WindowsSandboxLevel, pub(crate) shell_environment_policy: ShellEnvironmentPolicy, @@ -640,7 +640,7 @@ impl TurnContext { user_instructions: self.user_instructions.clone(), collaboration_mode, personality: self.personality, - approval_policy: self.approval_policy, + approval_policy: self.approval_policy.clone(), sandbox_policy: self.sandbox_policy.clone(), network: self.network.clone(), windows_sandbox_level: self.windows_sandbox_level, @@ -674,8 +674,8 @@ impl TurnContext { TurnContextItem { turn_id: Some(self.sub_id.clone()), cwd: self.cwd.clone(), - approval_policy: self.approval_policy, - sandbox_policy: self.sandbox_policy.clone(), + approval_policy: self.approval_policy.value(), + sandbox_policy: self.sandbox_policy.get().clone(), network: self.turn_context_network_item(), model: self.model_info.slug.clone(), personality: self.personality, @@ -980,8 +980,8 @@ impl Session { user_instructions: session_configuration.user_instructions.clone(), collaboration_mode: session_configuration.collaboration_mode.clone(), personality: session_configuration.personality, - approval_policy: session_configuration.approval_policy.value(), - sandbox_policy: session_configuration.sandbox_policy.get().clone(), + approval_policy: session_configuration.approval_policy.clone(), + sandbox_policy: session_configuration.sandbox_policy.clone(), network, windows_sandbox_level: session_configuration.windows_sandbox_level, shell_environment_policy: per_turn_config.permissions.shell_environment_policy.clone(), @@ -2713,8 +2713,8 @@ impl Session { let shell = self.user_shell(); items.push( DeveloperInstructions::from_policy( - &turn_context.sandbox_policy, - turn_context.approval_policy, + turn_context.sandbox_policy.get(), + turn_context.approval_policy.value(), self.services.exec_policy.current().as_ref(), &turn_context.cwd, ) @@ -3227,7 +3227,7 @@ impl Session { ); let auth_statuses = compute_auth_statuses(mcp_servers.iter(), store_mode).await; let sandbox_state = SandboxState { - sandbox_policy: turn_context.sandbox_policy.clone(), + sandbox_policy: turn_context.sandbox_policy.get().clone(), codex_linux_sandbox_exe: turn_context.codex_linux_sandbox_exe.clone(), sandbox_cwd: turn_context.cwd.clone(), use_linux_sandbox_bwrap: turn_context.features.enabled(Feature::UseLinuxSandboxBwrap), @@ -4340,7 +4340,7 @@ async fn spawn_review_thread( let turn_metadata_state = Arc::new(TurnMetadataState::new( review_turn_id.clone(), parent_turn_context.cwd.clone(), - &parent_turn_context.sandbox_policy, + parent_turn_context.sandbox_policy.get(), parent_turn_context.windows_sandbox_level, parent_turn_context .features @@ -4365,7 +4365,7 @@ async fn spawn_review_thread( compact_prompt: parent_turn_context.compact_prompt.clone(), collaboration_mode: parent_turn_context.collaboration_mode.clone(), personality: parent_turn_context.personality, - approval_policy: parent_turn_context.approval_policy, + approval_policy: parent_turn_context.approval_policy.clone(), sandbox_policy: parent_turn_context.sandbox_policy.clone(), network: parent_turn_context.network.clone(), windows_sandbox_level: parent_turn_context.windows_sandbox_level, @@ -5720,8 +5720,8 @@ async fn try_run_sampling_request( feedback_tags!( model = turn_context.model_info.slug.clone(), - approval_policy = turn_context.approval_policy, - sandbox_policy = turn_context.sandbox_policy, + approval_policy = turn_context.approval_policy.value(), + sandbox_policy = turn_context.sandbox_policy.get(), effort = turn_context.reasoning_effort, auth_mode = sess.services.auth_manager.auth_mode(), features = sess.features.enabled_features(), @@ -6652,8 +6652,8 @@ mod tests { let previous_context_item = TurnContextItem { turn_id: Some(turn_context.sub_id.clone()), cwd: turn_context.cwd.clone(), - approval_policy: turn_context.approval_policy, - sandbox_policy: turn_context.sandbox_policy.clone(), + approval_policy: turn_context.approval_policy.value(), + sandbox_policy: turn_context.sandbox_policy.get().clone(), network: None, model: previous_model.to_string(), personality: turn_context.personality, @@ -6689,8 +6689,8 @@ mod tests { let mut previous_context_item = TurnContextItem { turn_id: Some(turn_context.sub_id.clone()), cwd: turn_context.cwd.clone(), - approval_policy: turn_context.approval_policy, - sandbox_policy: turn_context.sandbox_policy.clone(), + approval_policy: turn_context.approval_policy.value(), + sandbox_policy: turn_context.sandbox_policy.get().clone(), network: None, model: previous_model.to_string(), personality: turn_context.personality, @@ -7051,8 +7051,8 @@ mod tests { let previous_context_item = TurnContextItem { turn_id: Some(turn_context.sub_id.clone()), cwd: turn_context.cwd.clone(), - approval_policy: turn_context.approval_policy, - sandbox_policy: turn_context.sandbox_policy.clone(), + approval_policy: turn_context.approval_policy.value(), + sandbox_policy: turn_context.sandbox_policy.get().clone(), network: None, model: previous_model.to_string(), personality: turn_context.personality, @@ -8809,7 +8809,10 @@ mod tests { let (session, mut turn_context_raw) = make_session_and_context().await; // Ensure policy is NOT OnRequest so the early rejection path triggers - turn_context_raw.approval_policy = AskForApproval::OnFailure; + turn_context_raw + .approval_policy + .set(AskForApproval::OnFailure) + .expect("test setup should allow updating approval policy"); let session = Arc::new(session); let mut turn_context = Arc::new(turn_context_raw); @@ -8883,7 +8886,7 @@ mod tests { let expected = format!( "approval policy is {policy:?}; reject command — you should not ask for escalated permissions if the approval policy is {policy:?}", - policy = turn_context.approval_policy + policy = turn_context.approval_policy.value() ); pretty_assertions::assert_eq!(output, expected); @@ -8892,7 +8895,9 @@ mod tests { // Force DangerFullAccess to avoid platform sandbox dependencies in tests. Arc::get_mut(&mut turn_context) .expect("unique turn context Arc") - .sandbox_policy = SandboxPolicy::DangerFullAccess; + .sandbox_policy + .set(SandboxPolicy::DangerFullAccess) + .expect("test setup should allow updating sandbox policy"); let resp2 = handler .handle(ToolInvocation { @@ -8946,7 +8951,10 @@ mod tests { use crate::turn_diff_tracker::TurnDiffTracker; let (session, mut turn_context_raw) = make_session_and_context().await; - turn_context_raw.approval_policy = AskForApproval::OnFailure; + turn_context_raw + .approval_policy + .set(AskForApproval::OnFailure) + .expect("test setup should allow updating approval policy"); let session = Arc::new(session); let turn_context = Arc::new(turn_context_raw); let tracker = Arc::new(tokio::sync::Mutex::new(TurnDiffTracker::new())); @@ -8976,7 +8984,7 @@ mod tests { let expected = format!( "approval policy is {policy:?}; reject command — you cannot ask for escalated permissions if the approval policy is {policy:?}", - policy = turn_context.approval_policy + policy = turn_context.approval_policy.value() ); pretty_assertions::assert_eq!(output, expected); diff --git a/codex-rs/core/src/context_manager/updates.rs b/codex-rs/core/src/context_manager/updates.rs index cdb53ff0c4..87a31031b3 100644 --- a/codex-rs/core/src/context_manager/updates.rs +++ b/codex-rs/core/src/context_manager/updates.rs @@ -31,14 +31,16 @@ fn build_permissions_update_item( exec_policy: &Policy, ) -> Option { let prev = previous?; - if prev.sandbox_policy == next.sandbox_policy && prev.approval_policy == next.approval_policy { + if prev.sandbox_policy == *next.sandbox_policy.get() + && prev.approval_policy == next.approval_policy.value() + { return None; } Some( DeveloperInstructions::from_policy( - &next.sandbox_policy, - next.approval_policy, + next.sandbox_policy.get(), + next.approval_policy.value(), exec_policy, &next.cwd, ) diff --git a/codex-rs/core/src/mcp/skill_dependencies.rs b/codex-rs/core/src/mcp/skill_dependencies.rs index e8b2d4f7b0..f2138cc293 100644 --- a/codex-rs/core/src/mcp/skill_dependencies.rs +++ b/codex-rs/core/src/mcp/skill_dependencies.rs @@ -32,9 +32,9 @@ const MCP_DEPENDENCY_OPTION_INSTALL: &str = "Install"; const MCP_DEPENDENCY_OPTION_SKIP: &str = "Continue anyway"; fn is_full_access_mode(turn_context: &TurnContext) -> bool { - matches!(turn_context.approval_policy, AskForApproval::Never) + matches!(turn_context.approval_policy.value(), AskForApproval::Never) && matches!( - turn_context.sandbox_policy, + turn_context.sandbox_policy.get(), SandboxPolicy::DangerFullAccess | SandboxPolicy::ExternalSandbox { .. } ) } diff --git a/codex-rs/core/src/mcp_tool_call.rs b/codex-rs/core/src/mcp_tool_call.rs index f194ee79a6..ce260773d2 100644 --- a/codex-rs/core/src/mcp_tool_call.rs +++ b/codex-rs/core/src/mcp_tool_call.rs @@ -401,9 +401,9 @@ async fn maybe_request_mcp_tool_approval( } fn is_full_access_mode(turn_context: &TurnContext) -> bool { - matches!(turn_context.approval_policy, AskForApproval::Never) + matches!(turn_context.approval_policy.value(), AskForApproval::Never) && matches!( - turn_context.sandbox_policy, + turn_context.sandbox_policy.get(), SandboxPolicy::DangerFullAccess | SandboxPolicy::ExternalSandbox { .. } ) } diff --git a/codex-rs/core/src/tools/handlers/apply_patch.rs b/codex-rs/core/src/tools/handlers/apply_patch.rs index 8f14fccedf..3ef79f7aa0 100644 --- a/codex-rs/core/src/tools/handlers/apply_patch.rs +++ b/codex-rs/core/src/tools/handlers/apply_patch.rs @@ -145,7 +145,13 @@ impl ToolHandler for ApplyPatchHandler { tool_name: tool_name.to_string(), }; let out = orchestrator - .run(&mut runtime, &req, &tool_ctx, &turn, turn.approval_policy) + .run( + &mut runtime, + &req, + &tool_ctx, + &turn, + turn.approval_policy.value(), + ) .await .map(|result| result.output); let event_ctx = ToolEventCtx::new( @@ -235,7 +241,13 @@ pub(crate) async fn intercept_apply_patch( tool_name: tool_name.to_string(), }; let out = orchestrator - .run(&mut runtime, &req, &tool_ctx, turn, turn.approval_policy) + .run( + &mut runtime, + &req, + &tool_ctx, + turn, + turn.approval_policy.value(), + ) .await .map(|result| result.output); let event_ctx = diff --git a/codex-rs/core/src/tools/handlers/multi_agents.rs b/codex-rs/core/src/tools/handlers/multi_agents.rs index b0c4cc4126..455a209526 100644 --- a/codex-rs/core/src/tools/handlers/multi_agents.rs +++ b/codex-rs/core/src/tools/handlers/multi_agents.rs @@ -919,7 +919,7 @@ fn build_agent_shared_config( config .permissions .sandbox_policy - .set(turn.sandbox_policy.clone()) + .set(turn.sandbox_policy.get().clone()) .map_err(|err| { FunctionCallError::RespondToModel(format!("sandbox_policy is invalid: {err}")) })?; @@ -1913,10 +1913,13 @@ mod tests { let temp_dir = tempfile::tempdir().expect("temp dir"); turn.cwd = temp_dir.path().to_path_buf(); turn.codex_linux_sandbox_exe = Some(PathBuf::from("/bin/echo")); - turn.sandbox_policy = pick_allowed_sandbox_policy( + let sandbox_policy = pick_allowed_sandbox_policy( &turn.config.permissions.sandbox_policy, turn.config.permissions.sandbox_policy.get().clone(), ); + turn.sandbox_policy + .set(sandbox_policy) + .expect("sandbox policy set"); let config = build_agent_spawn_config(&base_instructions, &turn, 0).expect("spawn config"); let mut expected = (*turn.config).clone(); @@ -1938,7 +1941,7 @@ mod tests { expected .permissions .sandbox_policy - .set(turn.sandbox_policy) + .set(turn.sandbox_policy.get().clone()) .expect("sandbox policy set"); assert_eq!(config, expected); } @@ -1987,7 +1990,7 @@ mod tests { expected .permissions .sandbox_policy - .set(turn.sandbox_policy) + .set(turn.sandbox_policy.get().clone()) .expect("sandbox policy set"); assert_eq!(config, expected); } diff --git a/codex-rs/core/src/tools/handlers/shell.rs b/codex-rs/core/src/tools/handlers/shell.rs index f040b4ba64..4cdbf454c4 100644 --- a/codex-rs/core/src/tools/handlers/shell.rs +++ b/codex-rs/core/src/tools/handlers/shell.rs @@ -281,11 +281,11 @@ impl ShellHandler { .sandbox_permissions .requires_escalated_permissions() && !matches!( - turn.approval_policy, + turn.approval_policy.value(), codex_protocol::protocol::AskForApproval::OnRequest ) { - let approval_policy = turn.approval_policy; + let approval_policy = turn.approval_policy.value(); return Err(FunctionCallError::RespondToModel(format!( "approval policy is {approval_policy:?}; reject command — you should not ask for escalated permissions if the approval policy is {approval_policy:?}" ))); @@ -322,8 +322,8 @@ impl ShellHandler { .exec_policy .create_exec_approval_requirement_for_command(ExecApprovalRequest { command: &exec_params.command, - approval_policy: turn.approval_policy, - sandbox_policy: &turn.sandbox_policy, + approval_policy: turn.approval_policy.value(), + sandbox_policy: turn.sandbox_policy.get(), sandbox_permissions: exec_params.sandbox_permissions, prefix_rule, }) @@ -349,7 +349,13 @@ impl ShellHandler { tool_name, }; let out = orchestrator - .run(&mut runtime, &req, &tool_ctx, &turn, turn.approval_policy) + .run( + &mut runtime, + &req, + &tool_ctx, + &turn, + turn.approval_policy.value(), + ) .await .map(|result| result.output); let event_ctx = ToolEventCtx::new(session.as_ref(), turn.as_ref(), &call_id, None); diff --git a/codex-rs/core/src/tools/handlers/unified_exec.rs b/codex-rs/core/src/tools/handlers/unified_exec.rs index 62da64a877..c7d57731e3 100644 --- a/codex-rs/core/src/tools/handlers/unified_exec.rs +++ b/codex-rs/core/src/tools/handlers/unified_exec.rs @@ -152,11 +152,11 @@ impl ToolHandler for UnifiedExecHandler { if sandbox_permissions.requires_escalated_permissions() && !matches!( - context.turn.approval_policy, + context.turn.approval_policy.value(), codex_protocol::protocol::AskForApproval::OnRequest ) { - let approval_policy = context.turn.approval_policy; + let approval_policy = context.turn.approval_policy.value(); manager.release_process_id(&process_id).await; return Err(FunctionCallError::RespondToModel(format!( "approval policy is {approval_policy:?}; reject command — you cannot ask for escalated permissions if the approval policy is {approval_policy:?}" diff --git a/codex-rs/core/src/tools/js_repl/mod.rs b/codex-rs/core/src/tools/js_repl/mod.rs index 5b7b539bb3..79195d97f6 100644 --- a/codex-rs/core/src/tools/js_repl/mod.rs +++ b/codex-rs/core/src/tools/js_repl/mod.rs @@ -1800,8 +1800,12 @@ mod tests { } let (session, mut turn) = make_session_and_context().await; - turn.approval_policy = AskForApproval::Never; - turn.sandbox_policy = SandboxPolicy::DangerFullAccess; + turn.approval_policy + .set(AskForApproval::Never) + .expect("test setup should allow updating approval policy"); + turn.sandbox_policy + .set(SandboxPolicy::DangerFullAccess) + .expect("test setup should allow updating sandbox policy"); let session = Arc::new(session); let turn = Arc::new(turn); @@ -1843,8 +1847,12 @@ mod tests { } let (session, mut turn) = make_session_and_context().await; - turn.approval_policy = AskForApproval::Never; - turn.sandbox_policy = SandboxPolicy::DangerFullAccess; + turn.approval_policy + .set(AskForApproval::Never) + .expect("test setup should allow updating approval policy"); + turn.sandbox_policy + .set(SandboxPolicy::DangerFullAccess) + .expect("test setup should allow updating sandbox policy"); let session = Arc::new(session); let turn = Arc::new(turn); @@ -1891,8 +1899,12 @@ try { } let (session, mut turn) = make_session_and_context().await; - turn.approval_policy = AskForApproval::Never; - turn.sandbox_policy = SandboxPolicy::DangerFullAccess; + turn.approval_policy + .set(AskForApproval::Never) + .expect("test setup should allow updating approval policy"); + turn.sandbox_policy + .set(SandboxPolicy::DangerFullAccess) + .expect("test setup should allow updating sandbox policy"); let session = Arc::new(session); let turn = Arc::new(turn); @@ -1941,8 +1953,12 @@ console.log("cell-complete"); { return Ok(()); } - turn.approval_policy = AskForApproval::Never; - turn.sandbox_policy = SandboxPolicy::DangerFullAccess; + turn.approval_policy + .set(AskForApproval::Never) + .expect("test setup should allow updating approval policy"); + turn.sandbox_policy + .set(SandboxPolicy::DangerFullAccess) + .expect("test setup should allow updating sandbox policy"); let session = Arc::new(session); let turn = Arc::new(turn); diff --git a/codex-rs/core/src/tools/network_approval.rs b/codex-rs/core/src/tools/network_approval.rs index 4a893f74e9..0d4f2f3078 100644 --- a/codex-rs/core/src/tools/network_approval.rs +++ b/codex-rs/core/src/tools/network_approval.rs @@ -299,7 +299,7 @@ impl NetworkApprovalService { .await; return NetworkDecision::deny(REASON_NOT_ALLOWED); }; - if !allows_network_prompt(turn_context.approval_policy) { + if !allows_network_prompt(turn_context.approval_policy.value()) { pending.set_decision(PendingApprovalDecision::Deny).await; let mut pending_approvals = self.pending_host_approvals.lock().await; pending_approvals.remove(&key); diff --git a/codex-rs/core/src/unified_exec/mod.rs b/codex-rs/core/src/unified_exec/mod.rs index a586572dd1..62269a96d6 100644 --- a/codex-rs/core/src/unified_exec/mod.rs +++ b/codex-rs/core/src/unified_exec/mod.rs @@ -193,8 +193,12 @@ mod tests { async fn test_session_and_turn() -> (Arc, Arc) { let (session, mut turn) = make_session_and_context().await; - turn.approval_policy = AskForApproval::Never; - turn.sandbox_policy = SandboxPolicy::DangerFullAccess; + turn.approval_policy + .set(AskForApproval::Never) + .expect("test setup should allow updating approval policy"); + turn.sandbox_policy + .set(SandboxPolicy::DangerFullAccess) + .expect("test setup should allow updating sandbox policy"); (Arc::new(session), Arc::new(turn)) } diff --git a/codex-rs/core/src/unified_exec/process_manager.rs b/codex-rs/core/src/unified_exec/process_manager.rs index 6945316469..68c4f95ceb 100644 --- a/codex-rs/core/src/unified_exec/process_manager.rs +++ b/codex-rs/core/src/unified_exec/process_manager.rs @@ -576,8 +576,8 @@ impl UnifiedExecProcessManager { .exec_policy .create_exec_approval_requirement_for_command(ExecApprovalRequest { command: &request.command, - approval_policy: context.turn.approval_policy, - sandbox_policy: &context.turn.sandbox_policy, + approval_policy: context.turn.approval_policy.value(), + sandbox_policy: context.turn.sandbox_policy.get(), sandbox_permissions: request.sandbox_permissions, prefix_rule: request.prefix_rule.clone(), }) @@ -605,7 +605,7 @@ impl UnifiedExecProcessManager { &req, &tool_ctx, context.turn.as_ref(), - context.turn.approval_policy, + context.turn.approval_policy.value(), ) .await .map(|result| (result.output, result.deferred_network_approval))