mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
[codex] Fix long proxy socket paths (#26553)
## Summary - avoid generating host proxy bridge Unix socket paths that exceed Linux's `sockaddr_un.sun_path` limit - fall back from a long `$CODEX_HOME/tmp` path to the system temp directory, then `/tmp` - add focused unit coverage for short and overlong parent paths ## Root cause With a sufficiently long `CODEX_HOME`, the generated `proxy-route-*.sock` path exceeds Linux's 107-byte pathname limit. The host bridge child exits before writing its readiness byte, so the parent reports the indirect error `failed to prepare host proxy routing bridge: failed to fill whole buffer`. ## Validation - reproduced the original error with a long `CODEX_HOME` using `codex-cli 0.138.0-alpha.4` - `cargo clippy -p codex-linux-sandbox --all-targets` - `just fix -p codex-linux-sandbox` - `just fmt` The Linux-only unit test could not execute locally: the arm64 Docker build was repeatedly OOM-killed by `rustc` while compiling an unrelated `codex-app-server-protocol` dependency, before reaching the test. --------- Co-authored-by: Codex <noreply@openai.com>
This commit is contained in:
committed by
GitHub
Unverified
parent
5e62c735b2
commit
a14a73b54a
@@ -14,6 +14,7 @@ use std::net::SocketAddr;
|
||||
use std::net::TcpListener;
|
||||
use std::net::TcpStream;
|
||||
use std::os::fd::FromRawFd;
|
||||
use std::os::unix::ffi::OsStrExt;
|
||||
use std::os::unix::fs::DirBuilderExt;
|
||||
use std::os::unix::fs::PermissionsExt;
|
||||
use std::os::unix::net::UnixListener;
|
||||
@@ -43,6 +44,8 @@ const PROXY_ENV_KEYS: &[&str] = &[
|
||||
const PROXY_SOCKET_DIR_PREFIX: &str = "codex-linux-sandbox-proxy-";
|
||||
const HOST_BRIDGE_READY: u8 = 1;
|
||||
const LOOPBACK_INTERFACE_NAME: &[u8] = b"lo";
|
||||
// Linux sockaddr_un.sun_path allows 108 bytes, including the trailing NUL.
|
||||
const UNIX_SOCKET_PATH_MAX_BYTES: usize = 107;
|
||||
|
||||
#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)]
|
||||
pub(crate) struct ProxyRouteSpec {
|
||||
@@ -278,8 +281,9 @@ fn rewrite_proxy_env_value(proxy_url: &str, local_port: u16) -> Option<String> {
|
||||
fn create_proxy_socket_dir() -> io::Result<PathBuf> {
|
||||
let temp_dir = proxy_socket_parent_dir();
|
||||
let pid = std::process::id();
|
||||
let uid = unsafe { libc::geteuid() };
|
||||
for attempt in 0..128 {
|
||||
let candidate = temp_dir.join(format!("{PROXY_SOCKET_DIR_PREFIX}{pid}-{attempt}"));
|
||||
let candidate = temp_dir.join(format!("{PROXY_SOCKET_DIR_PREFIX}{pid}-{uid}-{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();
|
||||
@@ -302,11 +306,29 @@ fn create_proxy_socket_dir() -> io::Result<PathBuf> {
|
||||
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() {
|
||||
if proxy_socket_paths_fit(candidate.as_path())
|
||||
&& ensure_private_proxy_socket_parent_dir(candidate.as_path()).is_ok()
|
||||
{
|
||||
return candidate;
|
||||
}
|
||||
}
|
||||
std::env::temp_dir()
|
||||
let temp_dir = std::env::temp_dir();
|
||||
if proxy_socket_paths_fit(temp_dir.as_path()) {
|
||||
temp_dir
|
||||
} else {
|
||||
PathBuf::from("/tmp")
|
||||
}
|
||||
}
|
||||
|
||||
fn proxy_socket_paths_fit(parent: &Path) -> bool {
|
||||
let socket_path = parent
|
||||
.join(format!(
|
||||
"{PROXY_SOCKET_DIR_PREFIX}{}-{}-127",
|
||||
u32::MAX,
|
||||
libc::uid_t::MAX
|
||||
))
|
||||
.join(format!("proxy-route-{}.sock", usize::MAX));
|
||||
socket_path.as_os_str().as_bytes().len() <= UNIX_SOCKET_PATH_MAX_BYTES
|
||||
}
|
||||
|
||||
fn ensure_private_proxy_socket_parent_dir(path: &Path) -> io::Result<()> {
|
||||
@@ -661,6 +683,7 @@ mod tests {
|
||||
use super::parse_loopback_proxy_endpoint;
|
||||
use super::parse_proxy_socket_dir_owner_pid;
|
||||
use super::plan_proxy_routes;
|
||||
use super::proxy_socket_paths_fit;
|
||||
use super::rewrite_proxy_env_value;
|
||||
use pretty_assertions::assert_eq;
|
||||
use std::collections::HashMap;
|
||||
@@ -735,6 +758,18 @@ mod tests {
|
||||
assert_eq!(default_proxy_port("socks5h"), 1080);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn proxy_socket_paths_enforce_linux_path_limit() {
|
||||
assert_eq!(
|
||||
proxy_socket_paths_fit(PathBuf::from("/tmp").as_path()),
|
||||
true
|
||||
);
|
||||
assert_eq!(
|
||||
proxy_socket_paths_fit(PathBuf::from(format!("/tmp/{}", "a".repeat(96))).as_path()),
|
||||
false
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn cleanup_proxy_socket_dir_removes_bridge_artifacts() {
|
||||
let root = tempfile::tempdir().expect("tempdir should create");
|
||||
@@ -770,6 +805,10 @@ mod tests {
|
||||
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-1234-1000-0"),
|
||||
Some(1234)
|
||||
);
|
||||
assert_eq!(
|
||||
parse_proxy_socket_dir_owner_pid("codex-linux-sandbox-proxy-x"),
|
||||
None
|
||||
|
||||
Reference in New Issue
Block a user