mirror of
https://github.com/openai/codex.git
synced 2026-11-12 21:20:34 +00:00
Limit MCP product attribution to configured REPL-backed calls (#60697)
## Why MCP request metadata could forward a client-supplied `product_sku` to unrelated servers and connectors. ## What changed Remove `product_sku` from turn metadata before forwarding it. For Node REPL-backed servers and the browser connector, populate it from the turn's `apps_mcp_product_sku` configuration, mapping unrecognized values to `other` and omitting empty or missing values. ## Testing Add regression coverage for configured REPL and browser attribution, unrelated servers and connectors, unrecognized or absent configuration, and spoofed client metadata. GitOrigin-RevId: bc24062526048ea1c1621099e609053cf167602f
This commit is contained in:
@@ -1456,16 +1456,27 @@ fn build_mcp_tool_call_request_meta(
|
||||
serde_json::Value::String(call_id.to_string()),
|
||||
);
|
||||
|
||||
if let Some(turn_metadata) = step_context
|
||||
if let Some(serde_json::Value::Object(mut turn_metadata)) = step_context
|
||||
.turn
|
||||
.turn_metadata_state
|
||||
.current_meta_value_for_mcp_request(ExecutionMetadata::from_settings(
|
||||
&step_context.settings,
|
||||
))
|
||||
{
|
||||
// Attribute trusted browser/REPL telemetry from this turn's configuration,
|
||||
// without forwarding internal product identity to unrelated MCP servers.
|
||||
turn_metadata.remove("product_sku");
|
||||
if is_node_repl_backed_connector(
|
||||
server,
|
||||
metadata.and_then(|metadata| metadata.connector_id.as_deref()),
|
||||
) && let Some(sku) = codex_otel::bounded_product_sku(
|
||||
step_context.turn.config.apps_mcp_product_sku.as_deref(),
|
||||
) {
|
||||
turn_metadata.insert("product_sku".to_string(), serde_json::json!(sku));
|
||||
}
|
||||
request_meta.insert(
|
||||
crate::X_CODEX_TURN_METADATA_HEADER.to_string(),
|
||||
turn_metadata,
|
||||
serde_json::Value::Object(turn_metadata),
|
||||
);
|
||||
}
|
||||
|
||||
|
||||
@@ -1155,6 +1155,65 @@ fn truncate_mcp_tool_result_for_event_bounds_large_error() {
|
||||
assert!(got.contains("truncated"));
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn mcp_tool_call_request_meta_attributes_only_repl_calls_to_configured_product() {
|
||||
for (server, connector, configured, expected) in [
|
||||
("node_repl", None, Some("codex"), Some("codex")),
|
||||
(
|
||||
"codex_apps",
|
||||
Some("connector_openai_browser"),
|
||||
Some("codex"),
|
||||
Some("codex"),
|
||||
),
|
||||
("custom_server", None, Some("codex"), None),
|
||||
(
|
||||
"codex_apps",
|
||||
Some("unrelated_connector"),
|
||||
Some("codex"),
|
||||
None,
|
||||
),
|
||||
(
|
||||
"node_repl",
|
||||
None,
|
||||
Some("private-unrecognized-product"),
|
||||
Some("other"),
|
||||
),
|
||||
("node_repl", None, Some(""), None),
|
||||
("node_repl", None, None, None),
|
||||
] {
|
||||
let (_, mut turn_context) = make_session_and_context().await;
|
||||
Arc::make_mut(&mut turn_context.config).apps_mcp_product_sku =
|
||||
configured.map(str::to_owned);
|
||||
turn_context
|
||||
.turn_metadata_state
|
||||
.update_responsesapi_client_metadata(ResponsesApiClientMetadataUpdate {
|
||||
replacement: Some(HashMap::from([(
|
||||
"product_sku".to_string(),
|
||||
"spoofed".to_string(),
|
||||
)])),
|
||||
..Default::default()
|
||||
});
|
||||
let step_context = StepContext::for_test(Arc::new(turn_context));
|
||||
let metadata = approval_metadata(
|
||||
connector, /*connector_name*/ None, /*connector_description*/ None,
|
||||
/*tool_title*/ None, /*tool_description*/ None,
|
||||
);
|
||||
let meta = build_mcp_tool_call_request_meta(
|
||||
&step_context,
|
||||
server,
|
||||
"call-product",
|
||||
Some(&metadata),
|
||||
/*is_host_owned_apps*/ false,
|
||||
)
|
||||
.expect("request metadata");
|
||||
assert_eq!(
|
||||
meta[crate::X_CODEX_TURN_METADATA_HEADER].get("product_sku"),
|
||||
expected.map(serde_json::Value::from).as_ref(),
|
||||
"{server}, {connector:?}, {configured:?}",
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn mcp_tool_call_request_meta_includes_turn_metadata_for_custom_server() {
|
||||
let (_, turn_context) = make_session_and_context().await;
|
||||
|
||||
Reference in New Issue
Block a user