mirror of
https://github.com/openai/codex.git
synced 2026-11-09 20:50:33 +00:00
## What changed - Make `ExecutorFileSystem::walk` a required backend operation instead of providing a fallback built from directory reads and metadata requests. - Implement bounded local walks on a blocking task with cancellation, symlink cycle detection, deterministic ordering, error collection, and response-size limits. - Have remote filesystems use the server's walk operation directly. ## Testing - Cover local and remote handling of invalid roots and limits, directory symlinks, non-UTF-8 names, cancellation, sandbox contexts, and response budgets. GitOrigin-RevId: 7499bf05080c3f9965a5eb7ffd593de604d62c2a
443 lines
15 KiB
Rust
443 lines
15 KiB
Rust
use super::DEFAULT_MCP_CONFIG_FILE;
|
|
use super::ExecutorPluginMcpProviderError;
|
|
use super::load_from_file_system;
|
|
use codex_config::McpServerConfig;
|
|
use codex_config::McpServerTransportConfig;
|
|
use codex_exec_server::CopyOptions;
|
|
use codex_exec_server::CreateDirectoryOptions;
|
|
use codex_exec_server::ExecutorFileSystem;
|
|
use codex_exec_server::ExecutorFileSystemFuture;
|
|
use codex_exec_server::FileMetadata;
|
|
use codex_exec_server::FileSystemReadStream;
|
|
use codex_exec_server::FileSystemResult;
|
|
use codex_exec_server::FileSystemSandboxContext;
|
|
use codex_exec_server::GetMetadataOptions;
|
|
use codex_exec_server::ReadDirectoryEntry;
|
|
use codex_exec_server::ReadFileOptions;
|
|
use codex_exec_server::RemoveOptions;
|
|
use codex_exec_server::WalkOptions;
|
|
use codex_exec_server::WalkOutcome;
|
|
use codex_exec_server::WriteFileOptions;
|
|
use codex_plugin::ResolvedPlugin;
|
|
use codex_plugin::manifest::PluginManifest;
|
|
use codex_plugin::manifest::PluginManifestMcpServers;
|
|
use codex_plugin::manifest::PluginManifestPaths;
|
|
use codex_utils_absolute_path::AbsolutePathBuf;
|
|
use codex_utils_path_uri::LegacyAppPathString;
|
|
use codex_utils_path_uri::PathUri;
|
|
use pretty_assertions::assert_eq;
|
|
use std::collections::HashMap;
|
|
use std::io;
|
|
use std::sync::Mutex;
|
|
|
|
const MCP_CONFIG_CONTENTS: &str = r#"{
|
|
"mcpServers": {
|
|
"demo": {"command": "demo-mcp", "environment_id": "local"},
|
|
"hosted": {"url": "https://example.com/mcp"}
|
|
}
|
|
}"#;
|
|
|
|
struct SyntheticExecutorFileSystem {
|
|
config_path: AbsolutePathBuf,
|
|
config_contents: Option<&'static str>,
|
|
reads: Mutex<Vec<AbsolutePathBuf>>,
|
|
}
|
|
|
|
impl SyntheticExecutorFileSystem {
|
|
fn unsupported<T>() -> FileSystemResult<T> {
|
|
Err(io::Error::new(
|
|
io::ErrorKind::Unsupported,
|
|
"operation is not used by executor MCP provider tests",
|
|
))
|
|
}
|
|
}
|
|
|
|
impl ExecutorFileSystem for SyntheticExecutorFileSystem {
|
|
fn canonicalize<'a>(
|
|
&'a self,
|
|
_path: &'a PathUri,
|
|
_sandbox: Option<&'a FileSystemSandboxContext>,
|
|
) -> ExecutorFileSystemFuture<'a, PathUri> {
|
|
Box::pin(async { Self::unsupported() })
|
|
}
|
|
|
|
fn read_file<'a>(
|
|
&'a self,
|
|
path: &'a PathUri,
|
|
_options: ReadFileOptions,
|
|
_sandbox: Option<&'a FileSystemSandboxContext>,
|
|
) -> ExecutorFileSystemFuture<'a, Vec<u8>> {
|
|
Box::pin(async move {
|
|
let path = path.to_abs_path()?;
|
|
self.reads
|
|
.lock()
|
|
.unwrap_or_else(std::sync::PoisonError::into_inner)
|
|
.push(path.clone());
|
|
if path != self.config_path {
|
|
return Err(io::Error::new(io::ErrorKind::NotFound, "not found"));
|
|
}
|
|
self.config_contents
|
|
.map(|contents| contents.as_bytes().to_vec())
|
|
.ok_or_else(|| io::Error::new(io::ErrorKind::NotFound, "not found"))
|
|
})
|
|
}
|
|
|
|
fn read_file_stream<'a>(
|
|
&'a self,
|
|
_path: &'a PathUri,
|
|
_sandbox: Option<&'a FileSystemSandboxContext>,
|
|
) -> ExecutorFileSystemFuture<'a, FileSystemReadStream> {
|
|
Box::pin(async { Self::unsupported() })
|
|
}
|
|
|
|
fn write_file<'a>(
|
|
&'a self,
|
|
_path: &'a PathUri,
|
|
_contents: Vec<u8>,
|
|
_options: WriteFileOptions,
|
|
_sandbox: Option<&'a FileSystemSandboxContext>,
|
|
) -> ExecutorFileSystemFuture<'a, ()> {
|
|
Box::pin(async { Self::unsupported() })
|
|
}
|
|
|
|
fn create_directory<'a>(
|
|
&'a self,
|
|
_path: &'a PathUri,
|
|
_options: CreateDirectoryOptions,
|
|
_sandbox: Option<&'a FileSystemSandboxContext>,
|
|
) -> ExecutorFileSystemFuture<'a, ()> {
|
|
Box::pin(async { Self::unsupported() })
|
|
}
|
|
|
|
fn get_metadata<'a>(
|
|
&'a self,
|
|
_path: &'a PathUri,
|
|
_options: GetMetadataOptions,
|
|
_sandbox: Option<&'a FileSystemSandboxContext>,
|
|
) -> ExecutorFileSystemFuture<'a, FileMetadata> {
|
|
Box::pin(async { Self::unsupported() })
|
|
}
|
|
|
|
fn read_directory<'a>(
|
|
&'a self,
|
|
_path: &'a PathUri,
|
|
_sandbox: Option<&'a FileSystemSandboxContext>,
|
|
) -> ExecutorFileSystemFuture<'a, Vec<ReadDirectoryEntry>> {
|
|
Box::pin(async { Self::unsupported() })
|
|
}
|
|
|
|
fn walk<'a>(
|
|
&'a self,
|
|
_path: &'a PathUri,
|
|
_options: WalkOptions,
|
|
_sandbox: Option<&'a FileSystemSandboxContext>,
|
|
) -> ExecutorFileSystemFuture<'a, WalkOutcome> {
|
|
Box::pin(async { Self::unsupported() })
|
|
}
|
|
|
|
fn remove<'a>(
|
|
&'a self,
|
|
_path: &'a PathUri,
|
|
_options: RemoveOptions,
|
|
_sandbox: Option<&'a FileSystemSandboxContext>,
|
|
) -> ExecutorFileSystemFuture<'a, ()> {
|
|
Box::pin(async { Self::unsupported() })
|
|
}
|
|
|
|
fn copy<'a>(
|
|
&'a self,
|
|
_source_path: &'a PathUri,
|
|
_destination_path: &'a PathUri,
|
|
_options: CopyOptions,
|
|
_sandbox: Option<&'a FileSystemSandboxContext>,
|
|
) -> ExecutorFileSystemFuture<'a, ()> {
|
|
Box::pin(async { Self::unsupported() })
|
|
}
|
|
}
|
|
|
|
#[tokio::test]
|
|
async fn reads_declared_config_only_through_executor_file_system() {
|
|
let temp_dir = tempfile::tempdir().expect("tempdir");
|
|
let plugin_root =
|
|
AbsolutePathBuf::from_absolute_path_checked(temp_dir.path().join("executor-only-plugin"))
|
|
.expect("absolute plugin root");
|
|
assert!(!plugin_root.as_path().exists());
|
|
let config_path = plugin_root.join("config/mcp.json");
|
|
let plugin = resolved_plugin(
|
|
&plugin_root,
|
|
Some(PluginManifestMcpServers::Path(config_path.clone())),
|
|
);
|
|
let file_system = SyntheticExecutorFileSystem {
|
|
config_path: config_path.clone(),
|
|
config_contents: Some(MCP_CONFIG_CONTENTS),
|
|
reads: Mutex::new(Vec::new()),
|
|
};
|
|
|
|
let plugin_root_uri = PathUri::from_abs_path(&plugin_root);
|
|
let servers = load_from_file_system(&plugin, &plugin_root_uri, &file_system)
|
|
.await
|
|
.expect("load executor MCP config");
|
|
|
|
assert_eq!(
|
|
servers,
|
|
vec![
|
|
(
|
|
"demo".to_string(),
|
|
McpServerConfig {
|
|
auth: Default::default(),
|
|
transport: McpServerTransportConfig::Stdio {
|
|
command: "demo-mcp".to_string(),
|
|
args: Vec::new(),
|
|
env: None,
|
|
env_vars: Vec::new(),
|
|
cwd: Some(LegacyAppPathString::from_path(plugin_root.as_path())),
|
|
},
|
|
environment_id: "executor-test".to_string(),
|
|
enabled: true,
|
|
required: false,
|
|
supports_parallel_tool_calls: false,
|
|
omit_tools_from: None,
|
|
disabled_reason: None,
|
|
startup_timeout_sec: None,
|
|
tool_timeout_sec: None,
|
|
default_tools_approval_mode: None,
|
|
enabled_tools: None,
|
|
disabled_tools: None,
|
|
scopes: None,
|
|
oauth: None,
|
|
oauth_resource: None,
|
|
tools: HashMap::new(),
|
|
},
|
|
),
|
|
(
|
|
"hosted".to_string(),
|
|
McpServerConfig {
|
|
auth: Default::default(),
|
|
transport: McpServerTransportConfig::StreamableHttp {
|
|
url: "https://example.com/mcp".to_string(),
|
|
bearer_token_env_var: None,
|
|
http_headers: None,
|
|
env_http_headers: None,
|
|
http_headers_helper: None,
|
|
},
|
|
environment_id: "executor-test".to_string(),
|
|
enabled: true,
|
|
required: false,
|
|
supports_parallel_tool_calls: false,
|
|
omit_tools_from: None,
|
|
disabled_reason: None,
|
|
startup_timeout_sec: None,
|
|
tool_timeout_sec: None,
|
|
default_tools_approval_mode: None,
|
|
enabled_tools: None,
|
|
disabled_tools: None,
|
|
scopes: None,
|
|
oauth: None,
|
|
oauth_resource: None,
|
|
tools: HashMap::new(),
|
|
},
|
|
),
|
|
]
|
|
);
|
|
assert_eq!(reads(&file_system), vec![config_path]);
|
|
}
|
|
|
|
#[tokio::test]
|
|
async fn reads_manifest_object_config_without_executor_file_system_access() {
|
|
let temp_dir = tempfile::tempdir().expect("tempdir");
|
|
let plugin_root = AbsolutePathBuf::from_absolute_path_checked(temp_dir.path().join("plugin"))
|
|
.expect("absolute plugin root");
|
|
let config_path = plugin_root.join(DEFAULT_MCP_CONFIG_FILE);
|
|
let plugin = resolved_plugin(
|
|
&plugin_root,
|
|
Some(PluginManifestMcpServers::Object(
|
|
r#"{"counter":{"command":"counter-mcp","environment_id":"local"}}"#.to_string(),
|
|
)),
|
|
);
|
|
let file_system = SyntheticExecutorFileSystem {
|
|
config_path,
|
|
config_contents: None,
|
|
reads: Mutex::new(Vec::new()),
|
|
};
|
|
|
|
let plugin_root_uri = PathUri::from_abs_path(&plugin_root);
|
|
let servers = load_from_file_system(&plugin, &plugin_root_uri, &file_system)
|
|
.await
|
|
.expect("load manifest object executor MCP config");
|
|
|
|
assert_eq!(
|
|
servers,
|
|
vec![(
|
|
"counter".to_string(),
|
|
McpServerConfig {
|
|
auth: Default::default(),
|
|
transport: McpServerTransportConfig::Stdio {
|
|
command: "counter-mcp".to_string(),
|
|
args: Vec::new(),
|
|
env: None,
|
|
env_vars: Vec::new(),
|
|
cwd: Some(LegacyAppPathString::from_path(plugin_root.as_path())),
|
|
},
|
|
environment_id: "executor-test".to_string(),
|
|
enabled: true,
|
|
required: false,
|
|
supports_parallel_tool_calls: false,
|
|
omit_tools_from: None,
|
|
disabled_reason: None,
|
|
startup_timeout_sec: None,
|
|
tool_timeout_sec: None,
|
|
default_tools_approval_mode: None,
|
|
enabled_tools: None,
|
|
disabled_tools: None,
|
|
scopes: None,
|
|
oauth: None,
|
|
oauth_resource: None,
|
|
tools: HashMap::new(),
|
|
},
|
|
)]
|
|
);
|
|
assert_eq!(reads(&file_system), Vec::new());
|
|
}
|
|
|
|
#[tokio::test]
|
|
async fn missing_default_config_is_empty() {
|
|
let temp_dir = tempfile::tempdir().expect("tempdir");
|
|
let plugin_root = AbsolutePathBuf::from_absolute_path_checked(temp_dir.path().join("plugin"))
|
|
.expect("absolute plugin root");
|
|
let config_path = plugin_root.join(DEFAULT_MCP_CONFIG_FILE);
|
|
let plugin = resolved_plugin(&plugin_root, /*mcp_servers*/ None);
|
|
let file_system = SyntheticExecutorFileSystem {
|
|
config_path: config_path.clone(),
|
|
config_contents: None,
|
|
reads: Mutex::new(Vec::new()),
|
|
};
|
|
|
|
let plugin_root_uri = PathUri::from_abs_path(&plugin_root);
|
|
let servers = load_from_file_system(&plugin, &plugin_root_uri, &file_system)
|
|
.await
|
|
.expect("missing default config should be ignored");
|
|
|
|
assert_eq!(servers, Vec::new());
|
|
assert_eq!(reads(&file_system), vec![config_path]);
|
|
}
|
|
|
|
#[tokio::test]
|
|
async fn malformed_declared_config_is_an_error() {
|
|
let temp_dir = tempfile::tempdir().expect("tempdir");
|
|
let plugin_root = AbsolutePathBuf::from_absolute_path_checked(temp_dir.path().join("plugin"))
|
|
.expect("absolute plugin root");
|
|
let config_path = plugin_root.join("mcp.json");
|
|
let plugin = resolved_plugin(
|
|
&plugin_root,
|
|
Some(PluginManifestMcpServers::Path(config_path.clone())),
|
|
);
|
|
let file_system = SyntheticExecutorFileSystem {
|
|
config_path: config_path.clone(),
|
|
config_contents: Some("{not-json"),
|
|
reads: Mutex::new(Vec::new()),
|
|
};
|
|
|
|
let plugin_root_uri = PathUri::from_abs_path(&plugin_root);
|
|
let err = load_from_file_system(&plugin, &plugin_root_uri, &file_system)
|
|
.await
|
|
.expect_err("malformed declared config should fail");
|
|
|
|
let ExecutorPluginMcpProviderError::ParseConfig {
|
|
plugin_id,
|
|
path,
|
|
source: _,
|
|
} = err
|
|
else {
|
|
panic!("expected parse error");
|
|
};
|
|
assert_eq!(
|
|
(plugin_id, path),
|
|
(
|
|
"selected-root".to_string(),
|
|
PathUri::from_abs_path(&config_path)
|
|
)
|
|
);
|
|
assert_eq!(reads(&file_system), vec![config_path]);
|
|
}
|
|
|
|
#[tokio::test]
|
|
async fn malformed_manifest_object_config_reports_actual_manifest_path() {
|
|
let temp_dir = tempfile::tempdir().expect("tempdir");
|
|
let plugin_root = AbsolutePathBuf::from_absolute_path_checked(temp_dir.path().join("plugin"))
|
|
.expect("absolute plugin root");
|
|
let plugin = resolved_plugin(
|
|
&plugin_root,
|
|
Some(PluginManifestMcpServers::Object("{not-json".to_string())),
|
|
);
|
|
let file_system = SyntheticExecutorFileSystem {
|
|
config_path: plugin_root.join(DEFAULT_MCP_CONFIG_FILE),
|
|
config_contents: None,
|
|
reads: Mutex::new(Vec::new()),
|
|
};
|
|
|
|
let plugin_root_uri = PathUri::from_abs_path(&plugin_root);
|
|
let err = load_from_file_system(&plugin, &plugin_root_uri, &file_system)
|
|
.await
|
|
.expect_err("malformed manifest object config should fail");
|
|
|
|
let ExecutorPluginMcpProviderError::ParseConfig {
|
|
plugin_id,
|
|
path,
|
|
source: _,
|
|
} = err
|
|
else {
|
|
panic!("expected parse error");
|
|
};
|
|
assert_eq!(
|
|
(plugin_id, path),
|
|
(
|
|
"selected-root".to_string(),
|
|
PathUri::from_abs_path(&plugin_root.join(".claude-plugin/plugin.json"))
|
|
)
|
|
);
|
|
assert_eq!(reads(&file_system), Vec::new());
|
|
}
|
|
|
|
fn resolved_plugin(
|
|
plugin_root: &AbsolutePathBuf,
|
|
mcp_servers: Option<PluginManifestMcpServers<AbsolutePathBuf>>,
|
|
) -> ResolvedPlugin {
|
|
let plugin_root_uri = PathUri::from_abs_path(plugin_root);
|
|
let mcp_servers = mcp_servers.map(|mcp_servers| match mcp_servers {
|
|
PluginManifestMcpServers::Path(path) => {
|
|
PluginManifestMcpServers::Path(PathUri::from_abs_path(&path))
|
|
}
|
|
PluginManifestMcpServers::Object(config) => PluginManifestMcpServers::Object(config),
|
|
});
|
|
ResolvedPlugin::from_environment(
|
|
"selected-root".to_string(),
|
|
"executor-test".to_string(),
|
|
plugin_root_uri.clone(),
|
|
plugin_root_uri
|
|
.join(".claude-plugin/plugin.json")
|
|
.expect("manifest URI"),
|
|
PluginManifest {
|
|
name: "demo-plugin".to_string(),
|
|
version: None,
|
|
description: None,
|
|
keywords: Vec::new(),
|
|
paths: PluginManifestPaths {
|
|
skills: Vec::new(),
|
|
mcp_servers,
|
|
apps: None,
|
|
hooks: None,
|
|
},
|
|
interface: None,
|
|
},
|
|
)
|
|
.expect("valid plugin descriptor")
|
|
}
|
|
|
|
fn reads(file_system: &SyntheticExecutorFileSystem) -> Vec<AbsolutePathBuf> {
|
|
file_system
|
|
.reads
|
|
.lock()
|
|
.unwrap_or_else(std::sync::PoisonError::into_inner)
|
|
.clone()
|
|
}
|