mirror of
https://github.com/openai/codex.git
synced 2026-08-23 13:09:46 +00:00
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
This commit is contained in:
@@ -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,
|
||||
|
||||
@@ -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();
|
||||
|
||||
@@ -164,7 +164,6 @@ impl SandboxedFileSystem {
|
||||
recursive: Some(options.recursive),
|
||||
follow_symlinks: (!options.follow_symlinks).then_some(false),
|
||||
sandbox: None,
|
||||
private: None,
|
||||
}),
|
||||
)
|
||||
.await?
|
||||
|
||||
@@ -169,32 +169,6 @@ impl FileSystemHandler {
|
||||
&self,
|
||||
params: FsCreateDirectoryParams,
|
||||
) -> Result<FsCreateDirectoryResponse, JSONRPCErrorError> {
|
||||
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");
|
||||
|
||||
Reference in New Issue
Block a user