Skip to content

Commit 6138063

Browse files
authored
Strip connector provenance metadata from custom MCP tools (#19875)
# Summary This prevents non-codex_apps MCP servers from spoofing connector provenance metadata.
1 parent ccec84b commit 6138063

1 file changed

Lines changed: 152 additions & 11 deletions

File tree

codex-rs/codex-mcp/src/rmcp_client.rs

Lines changed: 152 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,7 @@ use rmcp::model::FormElicitationCapability;
5959
use rmcp::model::Implementation;
6060
use rmcp::model::InitializeRequestParams;
6161
use rmcp::model::ProtocolVersion;
62+
use rmcp::model::Tool as RmcpTool;
6263
use tokio_util::sync::CancellationToken;
6364
use tracing::warn;
6465

@@ -72,6 +73,14 @@ pub(crate) const MCP_TOOLS_FETCH_UNCACHED_DURATION_METRIC: &str =
7273
pub(crate) const DEFAULT_STARTUP_TIMEOUT: Duration = Duration::from_secs(30);
7374
pub(crate) const DEFAULT_TOOL_TIMEOUT: Duration = Duration::from_secs(120);
7475

76+
const UNTRUSTED_CONNECTOR_META_KEYS: &[&str] = &[
77+
"connector_id",
78+
"connector_name",
79+
"connector_display_name",
80+
"connector_description",
81+
"connectorDescription",
82+
];
83+
7584
#[derive(Clone)]
7685
pub(crate) struct ManagedClient {
7786
pub(crate) client: Arc<RmcpClient>,
@@ -335,19 +344,23 @@ pub(crate) async fn list_tools_for_client_uncached(
335344
.tools
336345
.into_iter()
337346
.map(|tool| {
347+
let mut tool_def = tool.tool;
348+
let (connector_id, connector_name, connector_description) =
349+
sanitize_tool_connector_metadata(
350+
server_name,
351+
&mut tool_def,
352+
tool.connector_id,
353+
tool.connector_name,
354+
tool.connector_description,
355+
);
338356
let callable_name = normalize_codex_apps_callable_name(
339357
server_name,
340-
&tool.tool.name,
341-
tool.connector_id.as_deref(),
342-
tool.connector_name.as_deref(),
358+
&tool_def.name,
359+
connector_id.as_deref(),
360+
connector_name.as_deref(),
343361
);
344-
let callable_namespace = normalize_codex_apps_callable_namespace(
345-
server_name,
346-
tool.connector_name.as_deref(),
347-
);
348-
let connector_name = tool.connector_name;
349-
let connector_description = tool.connector_description;
350-
let mut tool_def = tool.tool;
362+
let callable_namespace =
363+
normalize_codex_apps_callable_namespace(server_name, connector_name.as_deref());
351364
if let Some(title) = tool_def.title.as_deref() {
352365
let normalized_title =
353366
normalize_codex_apps_tool_title(server_name, connector_name.as_deref(), title);
@@ -361,7 +374,7 @@ pub(crate) async fn list_tools_for_client_uncached(
361374
callable_namespace,
362375
server_instructions: server_instructions.map(str::to_string),
363376
tool: tool_def,
364-
connector_id: tool.connector_id,
377+
connector_id,
365378
connector_name,
366379
plugin_display_names: Vec::new(),
367380
connector_description,
@@ -374,6 +387,31 @@ pub(crate) async fn list_tools_for_client_uncached(
374387
Ok(tools)
375388
}
376389

390+
fn sanitize_tool_connector_metadata(
391+
server_name: &str,
392+
tool: &mut RmcpTool,
393+
connector_id: Option<String>,
394+
connector_name: Option<String>,
395+
connector_description: Option<String>,
396+
) -> (Option<String>, Option<String>, Option<String>) {
397+
if server_name == CODEX_APPS_MCP_SERVER_NAME {
398+
return (connector_id, connector_name, connector_description);
399+
}
400+
401+
strip_untrusted_connector_meta(tool);
402+
(None, None, None)
403+
}
404+
405+
fn strip_untrusted_connector_meta(tool: &mut RmcpTool) {
406+
if let Some(meta) = tool.meta.as_mut() {
407+
meta.retain(|key, _| !is_untrusted_connector_meta_key(key));
408+
}
409+
}
410+
411+
fn is_untrusted_connector_meta_key(key: &str) -> bool {
412+
UNTRUSTED_CONNECTOR_META_KEYS.contains(&key)
413+
}
414+
377415
fn resolve_bearer_token(
378416
server_name: &str,
379417
bearer_token_env_var: Option<&str>,
@@ -604,3 +642,106 @@ async fn make_rmcp_client(
604642
}
605643
}
606644
}
645+
646+
#[cfg(test)]
647+
mod tests {
648+
use super::*;
649+
use rmcp::model::JsonObject;
650+
use rmcp::model::Meta;
651+
652+
fn tool_with_connector_meta() -> RmcpTool {
653+
RmcpTool {
654+
name: "capture_file_upload".to_string().into(),
655+
title: None,
656+
description: Some("test tool".to_string().into()),
657+
input_schema: Arc::new(JsonObject::default()),
658+
output_schema: None,
659+
annotations: None,
660+
execution: None,
661+
icons: None,
662+
meta: Some(Meta(
663+
serde_json::json!({
664+
"connector_id": "connector_gmail",
665+
"connector_name": "Gmail",
666+
"connector_display_name": "Gmail",
667+
"connector_description": "Mail connector",
668+
"connectorDescription": "Mail connector",
669+
"connectorFutureField": "future connector metadata",
670+
"CONNECTOR_UPPERCASE": "uppercase connector metadata",
671+
"openai/fileParams": ["file"],
672+
"custom": "kept"
673+
})
674+
.as_object()
675+
.expect("object")
676+
.clone(),
677+
)),
678+
}
679+
}
680+
681+
#[test]
682+
fn custom_mcp_connector_metadata_is_stripped() {
683+
let mut tool = tool_with_connector_meta();
684+
685+
let (connector_id, connector_name, connector_description) =
686+
sanitize_tool_connector_metadata(
687+
"minimaltest",
688+
&mut tool,
689+
Some("connector_gmail".to_string()),
690+
Some("Gmail".to_string()),
691+
Some("Mail connector".to_string()),
692+
);
693+
694+
assert_eq!(connector_id, None);
695+
assert_eq!(connector_name, None);
696+
assert_eq!(connector_description, None);
697+
698+
let meta = tool.meta.as_ref().expect("meta");
699+
for key in [
700+
"connector_id",
701+
"connector_name",
702+
"connector_display_name",
703+
"connector_description",
704+
"connectorDescription",
705+
] {
706+
assert!(!meta.0.contains_key(key), "{key} should be stripped");
707+
}
708+
assert!(meta.0.contains_key("connectorFutureField"));
709+
assert!(meta.0.contains_key("CONNECTOR_UPPERCASE"));
710+
assert!(meta.0.contains_key("openai/fileParams"));
711+
assert_eq!(
712+
meta.0.get("custom").and_then(|value| value.as_str()),
713+
Some("kept")
714+
);
715+
}
716+
717+
#[test]
718+
fn codex_apps_connector_metadata_is_preserved() {
719+
let mut tool = tool_with_connector_meta();
720+
721+
let (connector_id, connector_name, connector_description) =
722+
sanitize_tool_connector_metadata(
723+
CODEX_APPS_MCP_SERVER_NAME,
724+
&mut tool,
725+
Some("connector_gmail".to_string()),
726+
Some("Gmail".to_string()),
727+
Some("Mail connector".to_string()),
728+
);
729+
730+
assert_eq!(connector_id.as_deref(), Some("connector_gmail"));
731+
assert_eq!(connector_name.as_deref(), Some("Gmail"));
732+
assert_eq!(connector_description.as_deref(), Some("Mail connector"));
733+
734+
let meta = tool.meta.as_ref().expect("meta");
735+
for key in [
736+
"connector_id",
737+
"connector_name",
738+
"connector_display_name",
739+
"connector_description",
740+
"connectorDescription",
741+
"connectorFutureField",
742+
"CONNECTOR_UPPERCASE",
743+
] {
744+
assert!(meta.0.contains_key(key), "{key} should be preserved");
745+
}
746+
}
747+
}

0 commit comments

Comments
 (0)