From 8c828b18d631c0b5efb2ba66bc6fa151fff9da6c Mon Sep 17 00:00:00 2001 From: rhan-oai Date: Thu, 20 Aug 2026 16:15:44 +0000 Subject: [PATCH] Remove private executor directory creation (#39736) ## What changed - Create remote plugin metrics directories through the standard executor filesystem API. - Remove the `private` directory-creation protocol option and its platform-specific handling. - Update the executor temporary-directory documentation to describe child-visible sidecars without an owner-private guarantee. GitOrigin-RevId: 9a8532403a3ad2bf998281735be0b668893918c9 --- .../src/plugin_metrics_sidecar.rs | 15 ++- codex-rs/exec-server-protocol/src/protocol.rs | 5 +- codex-rs/exec-server/src/environment.rs | 23 ---- .../exec-server/src/remote_file_system.rs | 1 - .../exec-server/src/sandboxed_file_system.rs | 1 - .../src/server/file_system_handler.rs | 118 ------------------ 6 files changed, 13 insertions(+), 150 deletions(-) diff --git a/codex-rs/core-plugins/src/plugin_metrics_sidecar.rs b/codex-rs/core-plugins/src/plugin_metrics_sidecar.rs index 7474c0a659..c1302f9e68 100644 --- a/codex-rs/core-plugins/src/plugin_metrics_sidecar.rs +++ b/codex-rs/core-plugins/src/plugin_metrics_sidecar.rs @@ -1,5 +1,6 @@ use crate::ResolvedPluginMetricsOperation; use codex_analytics::PluginMeasurementRow; +use codex_exec_server::CreateDirectoryOptions; use codex_exec_server::Environment; use codex_exec_server::ExecutorFileSystem; use codex_exec_server::FileSystemReadStream; @@ -142,12 +143,20 @@ impl PluginMetricsSidecar { .join(&format!("codex-plugin-metrics-{execution_id}")) .ok()?; let absolute_output_dir = directory_path.to_abs_path().ok()?; - environment - .create_private_directory(&directory_path) + let filesystem = environment.get_filesystem(); + filesystem + .create_directory( + &directory_path, + CreateDirectoryOptions { + recursive: false, + follow_symlinks: true, + }, + /*sandbox*/ None, + ) .await .ok()?; let directory = RemotePluginMetricsDirectory { - filesystem: environment.get_filesystem(), + filesystem, path: directory_path, }; let output_path = directory.path.join("measurements.json").ok()?; diff --git a/codex-rs/exec-server-protocol/src/protocol.rs b/codex-rs/exec-server-protocol/src/protocol.rs index 8095b662d6..016283e290 100644 --- a/codex-rs/exec-server-protocol/src/protocol.rs +++ b/codex-rs/exec-server-protocol/src/protocol.rs @@ -97,7 +97,7 @@ pub struct EnvironmentInfo { /// On Windows, a command's `TEMP` or `TMP` overrides take precedence. #[serde(default, skip_serializing_if = "Option::is_none")] pub temporary_directories: Option>, - /// Executor-native temporary directory for private, child-visible sidecars. + /// Executor-native temporary directory for child-visible sidecars. #[serde(default, skip_serializing_if = "Option::is_none")] pub temp_dir: Option, /// Optional executor features that clients must gate before sending newer request fields. @@ -433,9 +433,6 @@ pub struct FsCreateDirectoryParams { #[serde(default, skip_serializing_if = "Option::is_none")] pub follow_symlinks: Option, pub sandbox: Option, - /// Atomically restrict a newly created, non-recursive directory to its owner. - #[serde(default, skip_serializing_if = "Option::is_none")] - pub private: Option, } #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] diff --git a/codex-rs/exec-server/src/environment.rs b/codex-rs/exec-server/src/environment.rs index 1b93c7e47c..20ada8d8fb 100644 --- a/codex-rs/exec-server/src/environment.rs +++ b/codex-rs/exec-server/src/environment.rs @@ -38,11 +38,9 @@ use crate::local_file_system::LocalFileSystem; use crate::local_process::LocalProcess; use crate::process::ExecBackend; use crate::protocol::EnvironmentInfo; -use crate::protocol::FsCreateDirectoryParams; use crate::remote::NoiseRendezvousEnvironmentConfig; use crate::remote_file_system::RemoteFileSystem; use crate::remote_process::RemoteProcess; -use codex_utils_path_uri::PathUri; use tokio::sync::watch; use tokio_util::task::AbortOnDropHandle; use tracing::Instrument; @@ -906,27 +904,6 @@ impl Environment { } } - /// Atomically creates an owner-private directory on a remote executor. - pub async fn create_private_directory(&self, path: &PathUri) -> Result<(), ExecServerError> { - let Some(client) = &self.remote_client else { - return Err(ExecServerError::Protocol( - "private executor directory creation requires a remote environment".to_string(), - )); - }; - client - .get() - .await? - .fs_create_directory(FsCreateDirectoryParams { - path: path.clone(), - recursive: Some(false), - follow_symlinks: None, - sandbox: None, - private: Some(true), - }) - .await?; - Ok(()) - } - /// Discovers plugin and skill manifests through the environment's high-level discovery API. pub async fn discover_capability_roots( &self, diff --git a/codex-rs/exec-server/src/remote_file_system.rs b/codex-rs/exec-server/src/remote_file_system.rs index 7d624cefda..a2242ca668 100644 --- a/codex-rs/exec-server/src/remote_file_system.rs +++ b/codex-rs/exec-server/src/remote_file_system.rs @@ -146,7 +146,6 @@ impl RemoteFileSystem { recursive: Some(options.recursive), follow_symlinks: (!options.follow_symlinks).then_some(false), sandbox: remote_sandbox_context(sandbox), - private: None, }) .await; self.metadata_requests.lock().await.clear(); diff --git a/codex-rs/exec-server/src/sandboxed_file_system.rs b/codex-rs/exec-server/src/sandboxed_file_system.rs index ce70fe9a75..571deda1bb 100644 --- a/codex-rs/exec-server/src/sandboxed_file_system.rs +++ b/codex-rs/exec-server/src/sandboxed_file_system.rs @@ -164,7 +164,6 @@ impl SandboxedFileSystem { recursive: Some(options.recursive), follow_symlinks: (!options.follow_symlinks).then_some(false), sandbox: None, - private: None, }), ) .await? diff --git a/codex-rs/exec-server/src/server/file_system_handler.rs b/codex-rs/exec-server/src/server/file_system_handler.rs index e781558f94..fd5eaaea2d 100644 --- a/codex-rs/exec-server/src/server/file_system_handler.rs +++ b/codex-rs/exec-server/src/server/file_system_handler.rs @@ -169,32 +169,6 @@ impl FileSystemHandler { &self, params: FsCreateDirectoryParams, ) -> Result { - if params.private.unwrap_or(false) { - if params.follow_symlinks == Some(false) { - return Err(invalid_request( - "private directories do not support followSymlinks=false".to_string(), - )); - } - if params.recursive.unwrap_or(false) || params.sandbox.is_some() { - return Err(invalid_request( - "private directories must be non-recursive and unsandboxed".to_string(), - )); - } - #[cfg(unix)] - { - let path = params.path.to_abs_path().map_err(map_fs_error)?; - let mut builder = tokio::fs::DirBuilder::new(); - builder.mode(0o700); - builder.create(path.as_path()).await.map_err(map_fs_error)?; - return Ok(FsCreateDirectoryResponse {}); - } - #[cfg(not(unix))] - { - return Err(invalid_request( - "owner-private directories are unsupported on this platform".to_string(), - )); - } - } let recursive = params.recursive.unwrap_or(true); self.file_system .create_directory( @@ -344,9 +318,6 @@ fn map_fs_error(err: io::Error) -> JSONRPCErrorError { #[cfg(test)] mod tests { - #[cfg(unix)] - use std::os::unix::fs::PermissionsExt; - use codex_protocol::protocol::NetworkAccess; use codex_protocol::protocol::SandboxPolicy; use codex_utils_path_uri::PathUri; @@ -357,95 +328,6 @@ mod tests { use crate::protocol::FsReadFileParams; use crate::protocol::FsWriteFileParams; - #[cfg(unix)] - #[tokio::test] - async fn private_directories_are_created_with_owner_only_permissions() { - let temp_dir = tempfile::tempdir().expect("tempdir"); - let runtime_paths = ExecServerRuntimePaths::new( - std::env::current_exe().expect("current exe"), - /*codex_linux_sandbox_exe*/ None, - ) - .expect("runtime paths"); - let handler = FileSystemHandler::new(runtime_paths); - let directory = temp_dir.path().join("private-metrics"); - handler - .create_directory(FsCreateDirectoryParams { - path: PathUri::from_host_native_path(&directory).expect("directory URI"), - follow_symlinks: None, - recursive: Some(false), - sandbox: None, - private: Some(true), - }) - .await - .expect("create private directory"); - - assert_eq!( - std::fs::metadata(directory) - .expect("directory metadata") - .permissions() - .mode() - & 0o777, - 0o700 - ); - } - - #[cfg(unix)] - #[tokio::test] - async fn private_directories_reject_no_follow_before_resolving_the_path() { - let temp_dir = tempfile::tempdir().expect("tempdir"); - let runtime_paths = ExecServerRuntimePaths::new( - std::env::current_exe().expect("current exe"), - /*codex_linux_sandbox_exe*/ None, - ) - .expect("runtime paths"); - let handler = FileSystemHandler::new(runtime_paths); - let target = temp_dir.path().join("target"); - std::fs::create_dir(&target).expect("create target"); - let alias = temp_dir.path().join("alias"); - std::os::unix::fs::symlink(&target, &alias).expect("create alias"); - let directory = alias.join("private-metrics"); - - let error = handler - .create_directory(FsCreateDirectoryParams { - path: PathUri::from_host_native_path(&directory).expect("directory URI"), - follow_symlinks: Some(false), - recursive: Some(false), - sandbox: None, - private: Some(true), - }) - .await - .expect_err("strict private directory request must fail closed"); - - assert!(error.message.contains("followSymlinks=false")); - assert!(!target.join("private-metrics").exists()); - } - - #[cfg(windows)] - #[tokio::test] - async fn private_directories_are_rejected_when_owner_only_permissions_are_unsupported() { - let temp_dir = tempfile::tempdir().expect("tempdir"); - let runtime_paths = ExecServerRuntimePaths::new( - std::env::current_exe().expect("current exe"), - /*codex_linux_sandbox_exe*/ None, - ) - .expect("runtime paths"); - let handler = FileSystemHandler::new(runtime_paths); - let directory = temp_dir.path().join("private-metrics"); - let error = handler - .create_directory(FsCreateDirectoryParams { - path: PathUri::from_host_native_path(&directory).expect("directory URI"), - follow_symlinks: None, - recursive: Some(false), - sandbox: None, - private: Some(true), - }) - .await - .expect_err("private directories must fail closed"); - - assert!(error.message.contains("owner-private directories")); - assert!(!directory.exists()); - } - #[tokio::test] async fn no_platform_sandbox_policies_do_not_require_configured_sandbox_helper() { let temp_dir = tempfile::tempdir().expect("tempdir");