openai/codex
1
0
Fork 0
mirror of https://github.com/openai/codex.git synced 2026-10-24 18:18:41 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix MCP file transfer security boundaries

Browse Source
This commit is contained in:
Soheil Norouzi
2026-06-15 19:32:09 +00:00
parent 555f4bcebb
commit 341486baef
8 changed files with 561 additions and 274 deletions
Download Patch File Download Diff File Expand all files Collapse all files
codex-rs/codex-mcp/src/connection_manager.rs
+1 -1
View File
@@ -817,7 +817,7 @@ impl McpConnectionManager {
let params = serde_json::to_value(params).context("failed to serialize file request")?;
let result = client
.client
.send_custom_request(method, Some(params), client.tool_timeout)
.send_custom_request_with_timeout(method, Some(params), client.tool_timeout)
.await
.with_context(|| format!("MCP file request `{method}` failed for `{server}`"))?;
let ServerResult::CustomResult(result) = result else {
codex-rs/core/src/mcp_file_transfer.rs
+58 -248
View File
@@ -2,32 +2,30 @@
use std::collections::HashMap;
use std::path::PathBuf;
use std::time::Duration;
use base64::Engine;
use base64::engine::general_purpose::STANDARD;
use codex_mcp::FileInputSpec;
use codex_mcp::PrepareUploadParams;
use codex_protocol::mcp::CallToolResult;
use futures::StreamExt;
use reqwest::Method;
use reqwest::header::ACCEPT;
use reqwest::header::CONTENT_LENGTH;
use reqwest::header::HeaderMap;
use reqwest::header::HeaderValue;
use reqwest::header::USER_AGENT;
use reqwest::redirect::Policy;
use serde_json::Value as JsonValue;
use tokio::io::AsyncReadExt;
use tokio::io::AsyncWriteExt;
use url::Url;
use crate::session::session::Session;
use crate::session::turn_context::TurnContext;
const DEFAULT_MAX_FILE_BYTES: u64 = 50 * 1024 * 1024;
const TRANSFER_CONNECT_TIMEOUT: Duration = Duration::from_secs(10);
const TRANSFER_TIMEOUT: Duration = Duration::from_secs(5 * 60);
use self::http::download_transfer_file;
#[cfg(test)]
use self::http::is_disallowed_transfer_address;
#[cfg(test)]
use self::http::is_trusted_direct_transfer_host;
use self::http::put_transfer_file;
#[cfg(test)]
use self::http::validated_transfer_descriptor;
mod http;
#[tracing::instrument(
name = "mcp_file_transfer.adapt_input",
@@ -114,6 +112,7 @@ pub(crate) async fn materialize_mcp_file_outputs(
);
let output_path = unique_output_path(&output_dir, &name).await;
let size = download_transfer_file(
sess,
&download.transfer,
&output_path,
download
@@ -243,62 +242,6 @@ async fn unique_output_path(output_dir: &std::path::Path, name: &str) -> PathBuf
output_dir.join(format!("{}-{name}", uuid::Uuid::new_v4()))
}
async fn download_transfer_file(
transfer: &codex_mcp::FileTransferDescriptor,
output_path: &std::path::Path,
max_size: u64,
) -> Result<u64, String> {
let url = validated_transfer_descriptor(transfer, "GET")?;
let response = transfer_client()?
.get(url)
.send()
.await
.map_err(|_| "MCP download transfer request failed".to_string())?;
let status = response.status();
let response = response
.error_for_status()
.map_err(|_| format!("MCP download transfer returned HTTP {status}"))?;
if response
.content_length()
.is_some_and(|size| size > max_size)
{
return Err(format!("MCP download exceeds the {max_size}-byte limit"));
}
let temporary_path = output_path.with_extension("part");
let result = async {
let mut output = tokio::fs::File::create(&temporary_path)
.await
.map_err(|error| format!("failed to create MCP download: {error}"))?;
let mut size = 0_u64;
let mut stream = response.bytes_stream();
while let Some(chunk) = stream.next().await {
let chunk = chunk.map_err(|error| format!("failed to read MCP download: {error}"))?;
size = size.saturating_add(chunk.len() as u64);
if size > max_size {
return Err(format!("MCP download exceeds the {max_size}-byte limit"));
}
output
.write_all(&chunk)
.await
.map_err(|error| format!("failed to write MCP download: {error}"))?;
}
output
.flush()
.await
.map_err(|error| format!("failed to flush MCP download: {error}"))?;
drop(output);
tokio::fs::rename(&temporary_path, output_path)
.await
.map_err(|error| format!("failed to finalize MCP download: {error}"))?;
Ok(size)
}
.await;
if result.is_err() {
let _ = tokio::fs::remove_file(&temporary_path).await;
}
result
}
async fn rewrite_file_value(
sess: &Session,
turn_context: &TurnContext,
@@ -335,25 +278,45 @@ async fn rewrite_single_file(
spec.path
));
}
let path = resolve_file_path(turn_context, file_ref)?;
let metadata = tokio::fs::metadata(&path)
let Some(turn_environment) = turn_context.environments.primary() else {
return Err("no primary turn environment is available".to_string());
};
let path = resolve_file_path(turn_environment.cwd_uri(), file_ref)?;
let path_display = path.to_url().to_string();
let sandbox = turn_context.file_system_sandbox_context(
/*additional_permissions*/ None,
turn_environment.cwd_uri(),
);
let fs = turn_environment.environment.get_filesystem();
let metadata = fs
.get_metadata(&path, Some(&sandbox))
.await
.map_err(|error| format!("failed to read `{}`: {error}", path.display()))?;
if !metadata.is_file() {
return Err(format!("`{}` is not a regular file", path.display()));
.map_err(|error| format!("failed to read `{path_display}`: {error}"))?;
if !metadata.is_file {
return Err(format!("`{path_display}` is not a regular file"));
}
let max_size = spec
.max_size
.unwrap_or(DEFAULT_MAX_FILE_BYTES)
.min(DEFAULT_MAX_FILE_BYTES);
if metadata.len() > max_size {
if metadata.size > max_size {
return Err(format!(
"file `{}` is {} bytes, exceeding the {max_size}-byte limit",
path.display(),
metadata.len()
"file `{path_display}` is {} bytes, exceeding the {max_size}-byte limit",
metadata.size
));
}
let mime_type = mime_guess::from_path(&path)
let bytes = fs
.read_file(&path, Some(&sandbox))
.await
.map_err(|error| format!("failed to read `{path_display}`: {error}"))?;
let size = bytes.len() as u64;
if size > max_size {
return Err(format!(
"file `{path_display}` exceeds the {max_size}-byte limit"
));
}
let name = path.basename().unwrap_or_else(|| "upload".to_string());
let mime_type = mime_guess::from_path(&name)
.first_raw()
.unwrap_or("application/octet-stream");
if !spec.accepts.is_empty()
@@ -363,8 +326,7 @@ async fn rewrite_single_file(
.any(|accept| mime_matches(accept, mime_type))
{
return Err(format!(
"file `{}` has MIME type `{mime_type}`, which is not accepted by `{}`",
path.display(),
"file `{path_display}` has MIME type `{mime_type}`, which is not accepted by `{}`",
spec.path
));
}
@@ -376,27 +338,22 @@ async fn rewrite_single_file(
if spec.accepts_upload() && capabilities.prepare_upload && capabilities.complete_upload {
tracing::debug!(
transfer_mode = "upload",
size_bucket = file_size_bucket(metadata.len()),
size_bucket = file_size_bucket(size),
"adapting MCP file input"
);
let name = path
.file_name()
.and_then(|name| name.to_str())
.unwrap_or("upload")
.to_string();
let prepared = manager
.prepare_file_upload(
server,
PrepareUploadParams {
name,
mime_type: mime_type.to_string(),
size: metadata.len(),
size,
},
)
.await
.map_err(|error| format!("failed to prepare MCP file upload: {error:#}"))?;
validate_mcp_file_uri(&prepared.file.uri)?;
put_transfer_file(&prepared.transfer, &path, max_size).await?;
put_transfer_file(sess, &prepared.transfer, bytes, max_size).await?;
let completed = manager
.complete_file_upload(server, prepared.file.uri)
.await
@@ -407,23 +364,9 @@ async fn rewrite_single_file(
if spec.accepts_inline() {
tracing::debug!(
transfer_mode = "inline",
size_bucket = file_size_bucket(metadata.len()),
size_bucket = file_size_bucket(size),
"adapting MCP file input"
);
let file = tokio::fs::File::open(&path)
.await
.map_err(|error| format!("failed to open `{}`: {error}", path.display()))?;
let mut bytes = Vec::with_capacity(metadata.len() as usize);
file.take(max_size.saturating_add(1))
.read_to_end(&mut bytes)
.await
.map_err(|error| format!("failed to read `{}`: {error}", path.display()))?;
if bytes.len() as u64 > max_size {
return Err(format!(
"file `{}` exceeds the {max_size}-byte limit",
path.display()
));
}
return Ok(JsonValue::String(format!(
"data:{mime_type};base64,{}",
STANDARD.encode(bytes)
@@ -469,153 +412,20 @@ fn file_size_bucket(size: u64) -> &'static str {
}
}
fn resolve_file_path(turn_context: &TurnContext, file_ref: &str) -> Result<PathBuf, String> {
fn resolve_file_path(
cwd: &codex_utils_path_uri::PathUri,
file_ref: &str,
) -> Result<codex_utils_path_uri::PathUri, String> {
if file_ref.starts_with("file:") {
return Url::parse(file_ref)
.map_err(|error| format!("invalid file URI: {error}"))?
.to_file_path()
.map_err(|()| "file URI does not identify a local path".to_string());
return codex_utils_path_uri::PathUri::parse(file_ref)
.map_err(|error| format!("invalid file URI: {error}"));
}
#[allow(deprecated)]
Ok(turn_context
.resolve_path(Some(file_ref.to_string()))
.to_path_buf())
}
async fn put_transfer_file(
transfer: &codex_mcp::FileTransferDescriptor,
path: &std::path::Path,
max_size: u64,
) -> Result<(), String> {
let url = validated_upload_transfer_descriptor(transfer)?;
let method = Method::from_bytes(transfer.method.as_bytes())
.map_err(|error| format!("invalid MCP transfer method: {error}"))?;
let size = tokio::fs::metadata(path)
.await
.map_err(|error| format!("failed to inspect MCP upload: {error}"))?
.len();
if size > max_size {
return Err(format!("MCP upload exceeds the {max_size}-byte limit"));
if std::path::Path::new(file_ref).is_absolute() {
return codex_utils_path_uri::PathUri::from_path(file_ref)
.map_err(|error| format!("invalid absolute file path: {error}"));
}
let file = tokio::fs::File::open(path)
.await
.map_err(|error| format!("failed to open MCP upload: {error}"))?;
let stream = futures::stream::try_unfold((file, 0_u64), move |(mut file, sent)| async move {
let mut buffer = vec![0_u8; 64 * 1024];
let read = file.read(&mut buffer).await?;
if read == 0 {
return Ok(None);
}
buffer.truncate(read);
let sent = sent.saturating_add(read as u64);
if sent > max_size {
return Err(std::io::Error::other("MCP upload exceeded its size limit"));
}
Ok(Some((buffer, (file, sent))))
});
let azure_blob_upload = url.host_str().is_some_and(|host| {
host.ends_with(".blob.core.windows.net") || host.ends_with(".oaiusercontent.com")
});
let mut request = transfer_client()?
.request(method, url)
.header(CONTENT_LENGTH, size)
.body(reqwest::Body::wrap_stream(stream));
if azure_blob_upload {
request = request.header("x-ms-blob-type", "BlockBlob");
}
let response = request
.send()
.await
.map_err(|_| "MCP upload transfer request failed".to_string())?;
let status = response.status();
response
.error_for_status()
.map_err(|_| format!("MCP upload transfer returned HTTP {status}"))?;
Ok(())
}
fn validated_transfer_url(url: &str) -> Result<Url, String> {
let url = Url::parse(url).map_err(|error| format!("invalid MCP transfer URL: {error}"))?;
let local_http = cfg!(test)
&& url.scheme() == "http"
&& url
.host_str()
.is_some_and(|host| host == "localhost" || host == "127.0.0.1" || host == "::1");
if url.scheme() != "https" && !local_http {
return Err("MCP transfer URL must use HTTPS".to_string());
}
if !url.username().is_empty() || url.password().is_some() {
return Err("MCP transfer URL must not contain credentials".to_string());
}
if let Some(url::Host::Ipv4(address)) = url.host()
&& (address.is_private()
|| address.is_link_local()
|| address.is_loopback()
|| address.is_unspecified())
&& !local_http
{
return Err("MCP transfer URL must not target a private address".to_string());
}
if let Some(url::Host::Ipv6(address)) = url.host()
&& (address.is_loopback()
|| address.is_unspecified()
|| (address.segments()[0] & 0xffc0) == 0xfe80)
&& !local_http
{
return Err("MCP transfer URL must not target a private address".to_string());
}
Ok(url)
}
fn validated_transfer_descriptor(
transfer: &codex_mcp::FileTransferDescriptor,
expected_method: &str,
) -> Result<Url, String> {
if transfer
.transport
.as_deref()
.is_some_and(|value| value != "https")
{
return Err("MCP transfer transport must be HTTPS".to_string());
}
if transfer.method != expected_method {
return Err(format!("MCP transfer method must be {expected_method}"));
}
if let Some(expires_at) = transfer.expires_at.as_deref() {
let expires_at = chrono::DateTime::parse_from_rfc3339(expires_at)
.map_err(|error| format!("invalid MCP transfer expiry: {error}"))?;
if expires_at <= chrono::Utc::now() {
return Err("MCP transfer descriptor has expired".to_string());
}
}
validated_transfer_url(&transfer.url)
}
fn validated_upload_transfer_descriptor(
transfer: &codex_mcp::FileTransferDescriptor,
) -> Result<Url, String> {
if !matches!(transfer.method.as_str(), "PUT" | "POST") {
return Err("MCP upload transfer method must be PUT or POST".to_string());
}
validated_transfer_descriptor(transfer, &transfer.method)
}
fn transfer_client() -> Result<reqwest::Client, String> {
let mut headers = HeaderMap::new();
headers.insert(ACCEPT, HeaderValue::from_static("*/*"));
headers.insert(
USER_AGENT,
HeaderValue::from_static("Mozilla/5.0 Codex MCP File Transfer"),
);
let client = reqwest::Client::builder()
.default_headers(headers)
.connect_timeout(TRANSFER_CONNECT_TIMEOUT)
.timeout(TRANSFER_TIMEOUT)
.no_proxy()
.redirect(Policy::none())
.build()
.map_err(|error| format!("failed to build MCP transfer client: {error}"))?;
Ok(client)
cwd.join(file_ref)
.map_err(|error| format!("invalid file path `{file_ref}`: {error}"))
}
#[cfg(test)]
codex-rs/core/src/mcp_file_transfer/http.rs
+301
View File
@@ -0,0 +1,301 @@
use std::net::IpAddr;
use std::time::Duration;
use codex_network_proxy::NetworkProxy;
use futures::StreamExt;
use reqwest::Method;
use reqwest::header::ACCEPT;
use reqwest::header::CONTENT_LENGTH;
use reqwest::header::HeaderMap;
use reqwest::header::HeaderValue;
use reqwest::header::USER_AGENT;
use reqwest::redirect::Policy;
use tokio::io::AsyncWriteExt;
use url::Url;
use crate::session::session::Session;
const TRANSFER_CONNECT_TIMEOUT: Duration = Duration::from_secs(10);
const TRANSFER_TIMEOUT: Duration = Duration::from_secs(5 * 60);
pub(super) async fn download_transfer_file(
sess: &Session,
transfer: &codex_mcp::FileTransferDescriptor,
output_path: &std::path::Path,
max_size: u64,
) -> Result<u64, String> {
let url = validated_transfer_descriptor(transfer, "GET")?;
let response = transfer_client(sess, &url)
.await?
.get(url)
.send()
.await
.map_err(|_| "MCP download transfer request failed".to_string())?;
let status = response.status();
let response = response
.error_for_status()
.map_err(|_| format!("MCP download transfer returned HTTP {status}"))?;
if response
.content_length()
.is_some_and(|size| size > max_size)
{
return Err(format!("MCP download exceeds the {max_size}-byte limit"));
}
let temporary_path = output_path.with_extension("part");
let result = async {
let mut output = tokio::fs::File::create(&temporary_path)
.await
.map_err(|error| format!("failed to create MCP download: {error}"))?;
let mut size = 0_u64;
let mut stream = response.bytes_stream();
while let Some(chunk) = stream.next().await {
let chunk = chunk.map_err(|error| format!("failed to read MCP download: {error}"))?;
size = size.saturating_add(chunk.len() as u64);
if size > max_size {
return Err(format!("MCP download exceeds the {max_size}-byte limit"));
}
output
.write_all(&chunk)
.await
.map_err(|error| format!("failed to write MCP download: {error}"))?;
}
output
.flush()
.await
.map_err(|error| format!("failed to flush MCP download: {error}"))?;
drop(output);
tokio::fs::rename(&temporary_path, output_path)
.await
.map_err(|error| format!("failed to finalize MCP download: {error}"))?;
Ok(size)
}
.await;
if result.is_err() {
let _ = tokio::fs::remove_file(&temporary_path).await;
}
result
}
pub(super) async fn put_transfer_file(
sess: &Session,
transfer: &codex_mcp::FileTransferDescriptor,
bytes: Vec<u8>,
max_size: u64,
) -> Result<(), String> {
let url = validated_upload_transfer_descriptor(transfer)?;
let method = Method::from_bytes(transfer.method.as_bytes())
.map_err(|error| format!("invalid MCP transfer method: {error}"))?;
let size = bytes.len() as u64;
if size > max_size {
return Err(format!("MCP upload exceeds the {max_size}-byte limit"));
}
let stream = futures::stream::once(async move { Ok::<_, std::io::Error>(bytes) });
let azure_blob_upload = url.host_str().is_some_and(|host| {
host.ends_with(".blob.core.windows.net") || host.ends_with(".oaiusercontent.com")
});
let mut request = transfer_client(sess, &url)
.await?
.request(method, url)
.header(CONTENT_LENGTH, size)
.body(reqwest::Body::wrap_stream(stream));
if azure_blob_upload {
request = request.header("x-ms-blob-type", "BlockBlob");
}
let response = request
.send()
.await
.map_err(|_| "MCP upload transfer request failed".to_string())?;
let status = response.status();
response
.error_for_status()
.map_err(|_| format!("MCP upload transfer returned HTTP {status}"))?;
Ok(())
}
pub(super) fn validated_transfer_descriptor(
transfer: &codex_mcp::FileTransferDescriptor,
expected_method: &str,
) -> Result<Url, String> {
if transfer
.transport
.as_deref()
.is_some_and(|value| value != "https")
{
return Err("MCP transfer transport must be HTTPS".to_string());
}
if transfer.method != expected_method {
return Err(format!("MCP transfer method must be {expected_method}"));
}
if let Some(expires_at) = transfer.expires_at.as_deref() {
let expires_at = chrono::DateTime::parse_from_rfc3339(expires_at)
.map_err(|error| format!("invalid MCP transfer expiry: {error}"))?;
if expires_at <= chrono::Utc::now() {
return Err("MCP transfer descriptor has expired".to_string());
}
}
validated_transfer_url(&transfer.url)
}
fn validated_upload_transfer_descriptor(
transfer: &codex_mcp::FileTransferDescriptor,
) -> Result<Url, String> {
if !matches!(transfer.method.as_str(), "PUT" | "POST") {
return Err("MCP upload transfer method must be PUT or POST".to_string());
}
validated_transfer_descriptor(transfer, &transfer.method)
}
fn validated_transfer_url(url: &str) -> Result<Url, String> {
let url = Url::parse(url).map_err(|error| format!("invalid MCP transfer URL: {error}"))?;
let local_http = is_local_test_url(&url);
if url.scheme() != "https" && !local_http {
return Err("MCP transfer URL must use HTTPS".to_string());
}
if !url.username().is_empty() || url.password().is_some() {
return Err("MCP transfer URL must not contain credentials".to_string());
}
if url
.host()
.and_then(|host| match host {
url::Host::Ipv4(address) => Some(IpAddr::V4(address)),
url::Host::Ipv6(address) => Some(IpAddr::V6(address)),
url::Host::Domain(_) => None,
})
.is_some_and(is_disallowed_transfer_address)
&& !local_http
{
return Err("MCP transfer URL must not target a private address".to_string());
}
Ok(url)
}
async fn transfer_client(sess: &Session, url: &Url) -> Result<reqwest::Client, String> {
let network = sess
.services
.network_proxy
.load_full()
.map_or(TransferNetwork::Direct, |started| {
TransferNetwork::Managed(started.proxy())
});
build_transfer_client(url, network).await
}
enum TransferNetwork {
Managed(NetworkProxy),
Direct,
}
async fn build_transfer_client(
url: &Url,
network: TransferNetwork,
) -> Result<reqwest::Client, String> {
let mut headers = HeaderMap::new();
headers.insert(ACCEPT, HeaderValue::from_static("*/*"));
headers.insert(
USER_AGENT,
HeaderValue::from_static("Mozilla/5.0 Codex MCP File Transfer"),
);
let mut builder = reqwest::Client::builder()
.default_headers(headers)
.connect_timeout(TRANSFER_CONNECT_TIMEOUT)
.timeout(TRANSFER_TIMEOUT)
.no_proxy()
.redirect(Policy::none());
match network {
TransferNetwork::Managed(proxy) => {
let proxy_url = format!("http://{}", proxy.http_addr());
let proxy = reqwest::Proxy::all(&proxy_url)
.map_err(|error| format!("failed to configure MCP transfer proxy: {error}"))?;
builder = builder.proxy(proxy);
}
TransferNetwork::Direct => {
builder = pin_public_transfer_addresses(builder, url).await?;
}
}
builder
.build()
.map_err(|error| format!("failed to build MCP transfer client: {error}"))
}
async fn pin_public_transfer_addresses(
mut builder: reqwest::ClientBuilder,
url: &Url,
) -> Result<reqwest::ClientBuilder, String> {
let Some(host) = url.host_str() else {
return Err("MCP transfer URL must contain a host".to_string());
};
if is_local_test_url(url) {
return Ok(builder);
}
if !is_trusted_direct_transfer_host(host) {
return Err(
"MCP transfer URL requires the managed network proxy or a trusted transfer host"
.to_string(),
);
}
let port = url
.port_or_known_default()
.ok_or_else(|| "MCP transfer URL must contain a valid port".to_string())?;
let addresses = tokio::net::lookup_host((host, port))
.await
.map_err(|_| "failed to resolve MCP transfer host".to_string())?
.collect::<Vec<_>>();
if addresses.is_empty() {
return Err("MCP transfer host did not resolve".to_string());
}
if addresses
.iter()
.any(|address| is_disallowed_transfer_address(address.ip()))
{
return Err("MCP transfer URL must not target a private address".to_string());
}
if matches!(url.host(), Some(url::Host::Domain(_))) {
builder = builder.resolve_to_addrs(host, &addresses);
}
Ok(builder)
}
pub(super) fn is_trusted_direct_transfer_host(host: &str) -> bool {
let host = host.trim_end_matches('.').to_ascii_lowercase();
host.ends_with(".blob.core.windows.net") || host.ends_with(".oaiusercontent.com")
}
fn is_local_test_url(url: &Url) -> bool {
cfg!(test)
&& url.scheme() == "http"
&& url
.host_str()
.is_some_and(|host| host == "localhost" || host == "127.0.0.1" || host == "::1")
}
pub(super) fn is_disallowed_transfer_address(address: IpAddr) -> bool {
match address {
IpAddr::V4(address) => {
let octets = address.octets();
address.is_private()
|| address.is_link_local()
|| address.is_loopback()
|| address.is_unspecified()
|| address.is_multicast()
|| address.is_broadcast()
|| address.is_documentation()
|| octets[0] == 0
|| (octets[0] == 100 && (64..=127).contains(&octets[1]))
|| (octets[0] == 192 && octets[1] == 0 && octets[2] == 0)
|| (octets[0] == 198 && (18..=19).contains(&octets[1]))
|| octets[0] >= 240
}
IpAddr::V6(address) => {
let segments = address.segments();
address.is_loopback()
|| address.is_unspecified()
|| address.is_multicast()
|| (segments[0] & 0xfe00) == 0xfc00
|| (segments[0] & 0xffc0) == 0xfe80
|| (segments[0] == 0x2001 && segments[1] == 0x0db8)
|| address
.to_ipv4_mapped()
.is_some_and(|address| is_disallowed_transfer_address(IpAddr::V4(address)))
}
}
}
codex-rs/core/src/mcp_file_transfer_tests.rs
+64 -22
View File
@@ -30,16 +30,18 @@ fn model_file_ref_accepts_string_and_blob_like_values() {
#[tokio::test]
async fn transfer_rejects_non_https_remote_urls() {
let (session, _) = crate::session::tests::make_session_and_context().await;
assert_eq!(
put_transfer_file(
&session,
&codex_mcp::FileTransferDescriptor {
transport: Some("https".to_string()),
method: "PUT".to_string(),
url: "http://example.com/upload".to_string(),
expires_at: None,
},
std::path::Path::new("unused"),
0,
Vec::new(),
/*max_size*/ 0,
)
.await
.expect_err("remote HTTP must be rejected"),
@@ -122,9 +124,51 @@ fn validates_opaque_mcp_file_uris() {
);
}
#[test]
fn rejects_non_public_transfer_addresses() {
for address in [
"127.0.0.1",
"10.0.0.1",
"100.64.0.1",
"169.254.169.254",
"192.168.0.1",
"198.18.0.1",
"224.0.0.1",
"::1",
"fc00::1",
"fe80::1",
"ff02::1",
"::ffff:127.0.0.1",
] {
assert!(is_disallowed_transfer_address(
address.parse().expect("IP address")
));
}
for address in ["1.1.1.1", "8.8.8.8", "2606:4700:4700::1111"] {
assert!(!is_disallowed_transfer_address(
address.parse().expect("IP address")
));
}
}
#[test]
fn direct_transfers_require_trusted_storage_hosts() {
assert!(is_trusted_direct_transfer_host(
"account.blob.core.windows.net"
));
assert!(is_trusted_direct_transfer_host("files.oaiusercontent.com"));
assert!(is_trusted_direct_transfer_host("FILES.OAIUSERCONTENT.COM."));
assert!(!is_trusted_direct_transfer_host("oaiusercontent.com"));
assert!(!is_trusted_direct_transfer_host(
"files.oaiusercontent.com.attacker.example"
));
assert!(!is_trusted_direct_transfer_host("example.com"));
}
#[tokio::test]
async fn inline_rewrite_turns_a_local_path_into_a_data_uri() {
let (session, turn_context) = crate::session::tests::make_session_and_context().await;
let (session, mut turn_context) = crate::session::tests::make_session_and_context().await;
turn_context.permission_profile = codex_protocol::models::PermissionProfile::Disabled;
let directory = tempfile::tempdir().expect("temp dir");
let path = directory.path().join("report.txt");
tokio::fs::write(&path, b"hello")
@@ -159,6 +203,7 @@ async fn inline_rewrite_turns_a_local_path_into_a_data_uri() {
#[tokio::test]
async fn upload_transfer_streams_the_exact_file_bytes() {
let (session, _) = crate::session::tests::make_session_and_context().await;
let server = MockServer::start().await;
Mock::given(method("PUT"))
.and(path("/upload"))
@@ -168,21 +213,16 @@ async fn upload_transfer_streams_the_exact_file_bytes() {
.expect(1)
.mount(&server)
.await;
let directory = tempdir().expect("temp dir");
let file = directory.path().join("upload.txt");
tokio::fs::write(&file, b"stream me")
.await
.expect("write upload");
put_transfer_file(
&session,
&codex_mcp::FileTransferDescriptor {
transport: Some("https".to_string()),
method: "PUT".to_string(),
url: format!("{}/upload", server.uri()),
expires_at: None,
},
&file,
32,
b"stream me".to_vec(),
/*max_size*/ 32,
)
.await
.expect("upload succeeds");
@@ -190,6 +230,7 @@ async fn upload_transfer_streams_the_exact_file_bytes() {
#[tokio::test]
async fn upload_transfer_accepts_post_descriptors() {
let (session, _) = crate::session::tests::make_session_and_context().await;
let server = MockServer::start().await;
Mock::given(method("POST"))
.and(path("/upload"))
@@ -199,21 +240,16 @@ async fn upload_transfer_accepts_post_descriptors() {
.expect(1)
.mount(&server)
.await;
let directory = tempdir().expect("temp dir");
let file = directory.path().join("upload.txt");
tokio::fs::write(&file, b"stream me")
.await
.expect("write upload");
put_transfer_file(
&session,
&codex_mcp::FileTransferDescriptor {
transport: None,
method: "POST".to_string(),
url: format!("{}/upload", server.uri()),
expires_at: None,
},
&file,
32,
b"stream me".to_vec(),
/*max_size*/ 32,
)
.await
.expect("upload succeeds");
@@ -221,6 +257,7 @@ async fn upload_transfer_accepts_post_descriptors() {
#[tokio::test]
async fn download_transfer_materializes_exact_bytes() {
let (session, _) = crate::session::tests::make_session_and_context().await;
let server = MockServer::start().await;
Mock::given(method("GET"))
.and(path("/download"))
@@ -232,6 +269,7 @@ async fn download_transfer_materializes_exact_bytes() {
let output = directory.path().join("download.txt");
let size = download_transfer_file(
&session,
&codex_mcp::FileTransferDescriptor {
transport: Some("https".to_string()),
method: "GET".to_string(),
@@ -239,7 +277,7 @@ async fn download_transfer_materializes_exact_bytes() {
expires_at: None,
},
&output,
32,
/*max_size*/ 32,
)
.await
.expect("download succeeds");
@@ -253,6 +291,7 @@ async fn download_transfer_materializes_exact_bytes() {
#[tokio::test]
async fn transfer_errors_do_not_expose_signed_urls() {
let (session, _) = crate::session::tests::make_session_and_context().await;
let server = MockServer::start().await;
Mock::given(method("GET"))
.and(path("/download"))
@@ -264,6 +303,7 @@ async fn transfer_errors_do_not_expose_signed_urls() {
let secret = "sensitive-signature";
let error = download_transfer_file(
&session,
&codex_mcp::FileTransferDescriptor {
transport: None,
method: "GET".to_string(),
@@ -271,7 +311,7 @@ async fn transfer_errors_do_not_expose_signed_urls() {
expires_at: None,
},
&output,
32,
/*max_size*/ 32,
)
.await
.expect_err("failed transfer");
@@ -285,6 +325,7 @@ async fn transfer_errors_do_not_expose_signed_urls() {
#[tokio::test]
async fn failed_download_removes_partial_file() {
let (session, _) = crate::session::tests::make_session_and_context().await;
let server = MockServer::start().await;
Mock::given(method("GET"))
.and(path("/download"))
@@ -295,6 +336,7 @@ async fn failed_download_removes_partial_file() {
let output = directory.path().join("download.txt");
let error = download_transfer_file(
&session,
&codex_mcp::FileTransferDescriptor {
transport: None,
method: "GET".to_string(),
@@ -302,7 +344,7 @@ async fn failed_download_removes_partial_file() {
expires_at: None,
},
&output,
32,
/*max_size*/ 32,
)
.await
.expect_err("oversized transfer");
codex-rs/core/src/tools/runtimes/mod_tests.rs
-1
View File
@@ -26,7 +26,6 @@ use codex_sandboxing::SandboxType;
use codex_utils_absolute_path::AbsolutePathBuf;
use codex_utils_path_uri::PathUri;
use core_test_support::PathBufExt;
use core_test_support::PathExt;
use pretty_assertions::assert_eq;
use std::path::PathBuf;
use std::process::Command;
codex-rs/core/tests/suite/rmcp_client.rs
+119
View File
@@ -32,6 +32,7 @@ use codex_mcp::MCP_SANDBOX_STATE_META_CAPABILITY;
use codex_models_manager::manager::RefreshStrategy;
use codex_protocol::config_types::ReasoningSummary;
use codex_protocol::models::ManagedFileSystemPermissions;
use codex_protocol::models::PermissionProfile;
use codex_protocol::openai_models::ConfigShellToolType;
use codex_protocol::openai_models::InputModality;
@@ -40,6 +41,10 @@ use codex_protocol::openai_models::ModelVisibility;
use codex_protocol::openai_models::ModelsResponse;
use codex_protocol::openai_models::ReasoningEffortPreset;
use codex_protocol::openai_models::TruncationPolicyConfig;
use codex_protocol::permissions::FileSystemAccessMode;
use codex_protocol::permissions::FileSystemPath;
use codex_protocol::permissions::FileSystemSandboxEntry;
use codex_protocol::permissions::NetworkSandboxPolicy;
use codex_protocol::protocol::AskForApproval;
use codex_protocol::protocol::EventMsg;
use codex_protocol::protocol::McpInvocation;
@@ -675,6 +680,120 @@ async fn stdio_server_adapts_inline_mcp_file_input() -> anyhow::Result<()> {
Ok(())
}
#[tokio::test(flavor = "multi_thread", worker_threads = 1)]
async fn stdio_server_rejects_mcp_file_input_denied_by_turn_sandbox() -> anyhow::Result<()> {
let model_server = responses::start_mock_server().await;
let files = tempdir()?;
let denied_path = files.path().join("denied.txt");
let call_marker = files.path().join("tool-called");
fs::write(&denied_path, b"must not leave the sandbox")?;
let call_id = "file-call-denied";
let server_name = "rmcp_files_denied";
let namespace = format!("mcp__{server_name}");
let arguments = serde_json::to_string(&json!({"file": denied_path}))?;
responses::mount_sse_sequence(
&model_server,
vec![
responses::sse(vec![
responses::ev_response_created("resp-1"),
responses::ev_function_call_with_namespace(
call_id,
&namespace,
"file_round_trip",
&arguments,
),
responses::ev_completed("resp-1"),
]),
responses::sse(vec![
responses::ev_assistant_message("msg-1", "The file read was denied."),
responses::ev_completed("resp-2"),
]),
],
)
.await;
let rmcp_test_server_bin = stdio_server_bin()?;
let call_marker_for_config = call_marker.clone();
let fixture = test_codex()
.with_config(move |config| {
config
.features
.enable(Feature::McpFileTransfer)
.expect("test feature should enable");
insert_mcp_server(
config,
server_name,
stdio_transport(
rmcp_test_server_bin,
Some(HashMap::from([
("MCP_TEST_FILE_INLINE".to_string(), "1".to_string()),
(
"MCP_TEST_FILE_CALL_MARKER".to_string(),
call_marker_for_config.display().to_string(),
),
])),
Vec::new(),
),
TestMcpServerOptions::default(),
);
})
.build(&model_server)
.await?;
wait_for_mcp_server(&fixture.codex, server_name).await?;
let permission_profile = PermissionProfile::Managed {
file_system: ManagedFileSystemPermissions::Restricted {
entries: vec![
FileSystemSandboxEntry {
path: FileSystemPath::Special {
value: codex_protocol::permissions::FileSystemSpecialPath::Root,
},
access: FileSystemAccessMode::Read,
},
FileSystemSandboxEntry {
path: FileSystemPath::Path {
path: denied_path.as_path().try_into()?,
},
access: FileSystemAccessMode::Deny,
},
],
glob_scan_max_depth: None,
},
network: NetworkSandboxPolicy::Restricted,
};
fixture
.codex
.submit(user_turn_with_permission_profile(
&fixture,
"send the denied local file",
fixture.session_configured.model.clone(),
permission_profile,
))
.await?;
let end_event = wait_for_event(&fixture.codex, |event| {
matches!(event, EventMsg::McpToolCallEnd(_))
})
.await;
let EventMsg::McpToolCallEnd(end) = end_event else {
unreachable!("event guard guarantees McpToolCallEnd");
};
let error = end.result.expect_err("sandbox-denied file should fail");
assert!(
error.contains("failed to read"),
"unexpected error: {error}"
);
assert!(
!call_marker.exists(),
"MCP server must not receive file input"
);
wait_for_event(&fixture.codex, |event| {
matches!(event, EventMsg::TurnComplete(_))
})
.await;
Ok(())
}
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn shutdown_cancels_startup_prewarm_waiting_for_mcp_startup() -> anyhow::Result<()> {
skip_if_no_network!(Ok(()));
codex-rs/rmcp-client/src/bin/test_stdio_server.rs
+9 -2
View File
@@ -45,6 +45,7 @@ const MEMO_CONTENT: &str = "This is a sample MCP resource served by the rmcp tes
const SANDBOX_STATE_META_CAPABILITY: &str = "codex/sandbox-state-meta";
const SMALL_PNG_BASE64: &str = "iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR4nGP4z8DwHwAFAAH/iZk9HQAAAABJRU5ErkJggg==";
const FILE_INLINE_ENV: &str = "MCP_TEST_FILE_INLINE";
const FILE_CALL_MARKER_ENV: &str = "MCP_TEST_FILE_CALL_MARKER";
pub fn stdio() -> (tokio::io::Stdin, tokio::io::Stdout) {
(tokio::io::stdin(), tokio::io::stdout())
@@ -189,11 +190,13 @@ impl TestToolServer {
"additionalProperties": false
}))
.expect("file round-trip tool schema should deserialize");
Tool::new(
let mut tool = Tool::new(
Cow::Borrowed("file_round_trip"),
Cow::Borrowed("Accept an uploaded file and return a downloadable file."),
Arc::new(schema),
)
);
tool.annotations = Some(ToolAnnotations::new().read_only(true));
tool
}
fn sync_tool() -> Tool {
@@ -517,6 +520,10 @@ impl ServerHandler for TestToolServer {
) -> Result<CallToolResult, McpError> {
match request.name.as_ref() {
"file_round_trip" => {
if let Ok(marker) = std::env::var(FILE_CALL_MARKER_ENV) {
std::fs::write(marker, b"called")
.map_err(|err| McpError::internal_error(err.to_string(), None))?;
}
let args = Self::parse_call_args::<FileRoundTripArgs>(&request, "file_round_trip")?;
if args.file != "data:text/plain;base64,dXBsb2FkIG1l" {
return Err(McpError::invalid_params(
codex-rs/rmcp-client/src/rmcp_client.rs
+9
View File
@@ -669,6 +669,15 @@ impl RmcpClient {
&self,
method: &str,
params: Option<serde_json::Value>,
) -> Result<ServerResult> {
self.send_custom_request_with_timeout(method, params, /*timeout*/ None)
.await
}
pub async fn send_custom_request_with_timeout(
&self,
method: &str,
params: Option<serde_json::Value>,
timeout: Option<Duration>,
) -> Result<ServerResult> {
self.refresh_oauth_if_needed().await;