Address direct MCP egress review feedback

App-server callers can supply arbitrary MCP request metadata. The direct egress policy covered the snake_case connector identity fields but omitted the camelCase variants used by app-server surfaces, allowing those reserved identities to bypass the boundary. Strip the three canonical camelCase variants and document the filtering contract for app-server clients.\n\nCover the policy at the real MCP wire boundary: configured direct servers keep benign caller metadata and negotiated sandbox state while dropping thread, turn, trace, plugin, and connector identity; hosted Codex Apps calls continue to receive authoritative thread and turn metadata.
This commit is contained in:
Max Stoiber
2026-07-07 09:26:46 -07:00
parent c8bbae079f
commit 0a451911c3
4 changed files with 64 additions and 2 deletions
+1 -1
View File
@@ -235,7 +235,7 @@ Example with notification opt-out:
- `config/mcpServer/reload` — reload MCP server config from disk and queue a refresh for loaded threads (applied on each thread's next active turn); returns `{}`. Use this after editing `config.toml` without restarting the server.
- `mcpServerStatus/list` — enumerate configured MCP servers with their tools, auth status, server info, plus resources/resource templates for `full` detail; supports optional `threadId` and cursor+limit pagination. If `threadId` is omitted, the server reads from the latest global config directly. If `detail` is omitted, the server defaults to `full`.
- `mcpServer/resource/read` — read a resource from a configured MCP server by optional `threadId`, `server`, and `uri`, returning text/blob resource `contents`. If `threadId` is omitted, the server reads from the latest MCP config directly.
- `mcpServer/tool/call` — call a tool on a thread's configured MCP server by `threadId`, `server`, `tool`, optional `arguments`, and optional `_meta`, returning the MCP tool result.
- `mcpServer/tool/call` — call a tool on a thread's configured MCP server by `threadId`, `server`, `tool`, optional `arguments`, and optional `_meta`, returning the MCP tool result. For direct MCP servers, app-server forwards caller-supplied `_meta` except private OpenAI/Codex identity, routing, workspace, turn, tracing, plugin, and connector fields, plus keys in the reserved `openai/`, `x-openai-`, `_openai_`, `codex/`, `x-codex-`, `_codex_`, and `codex_` namespaces. Codex does not inject `threadId` for direct calls. Host-owned servers retain their existing metadata behavior.
- `windowsSandbox/setupStart` — start Windows sandbox setup for the selected mode (`elevated` or `unelevated`); accepts an optional absolute `cwd` to target setup for a specific workspace, returns `{ started: true }` immediately, and later emits `windowsSandbox/setupCompleted`.
- `feedback/upload` — submit a feedback report (classification + optional reason/logs, conversation_id, and optional `extraLogFiles` attachments array); returns the tracking thread id.
- `config/read` — fetch the effective config on disk after resolving config layering, including opaque `desktop` values stored in `config.toml`.
+3
View File
@@ -91,8 +91,11 @@ fn is_reserved_direct_mcp_meta_key(key: &str) -> bool {
| "plugin_id"
| "pluginId"
| "connector_id"
| "connectorId"
| "connector_name"
| "connectorName"
| "connector_display_name"
| "connectorDisplayName"
| "connector_description"
| "connectorDescription"
| "connected_account_email"
+56 -1
View File
@@ -869,7 +869,7 @@ async fn local_stdio_server_uses_runtime_fallback_cwd_when_config_omits_cwd() ->
}
#[tokio::test(flavor = "multi_thread", worker_threads = 1)]
async fn stdio_mcp_tool_call_includes_sandbox_state_meta() -> anyhow::Result<()> {
async fn direct_mcp_tool_call_omits_host_metadata_but_keeps_sandbox_state() -> anyhow::Result<()> {
// TODO(anp): Remove after packaging a Windows stdio test server for Wine exec.
skip_if_wine_exec!(
Ok(()),
@@ -937,6 +937,18 @@ async fn stdio_mcp_tool_call_includes_sandbox_state_meta() -> anyhow::Result<()>
let meta = output_json
.as_object()
.expect("sandbox_meta should return metadata object");
assert!(
!meta.contains_key("threadId"),
"direct MCP calls should not include thread identity: {meta:?}"
);
assert!(
!meta.contains_key("x-codex-turn-metadata"),
"direct MCP calls should not include turn metadata: {meta:?}"
);
assert!(
!meta.contains_key("codex_bridge_mcp_call_id"),
"direct MCP calls should not include rollout correlation: {meta:?}"
);
let sandbox_meta = meta
.get(MCP_SANDBOX_STATE_META_CAPABILITY)
@@ -952,6 +964,49 @@ async fn stdio_mcp_tool_call_includes_sandbox_state_meta() -> anyhow::Result<()>
}
);
let caller_meta_result = fixture
.codex
.call_mcp_tool(
server_name,
"sandbox_meta",
/*arguments*/ None,
Some(json!({
"threadId": "caller-thread",
"x-codex-turn-metadata": {"model": "caller-model"},
"codex_bridge_mcp_call_id": "caller-trace",
"connector_id": "snake-id",
"connectorId": "camel-id",
"connector_name": "Snake Name",
"connectorName": "Camel Name",
"connector_display_name": "Snake Display Name",
"connectorDisplayName": "Camel Display Name",
"thirdPartyHint": "preserved",
})),
)
.await?;
let caller_meta = caller_meta_result
.structured_content
.as_ref()
.and_then(Value::as_object)
.expect("sandbox_meta should return caller metadata");
assert_eq!(caller_meta.get("thirdPartyHint"), Some(&json!("preserved")));
for reserved_key in [
"threadId",
"x-codex-turn-metadata",
"codex_bridge_mcp_call_id",
"connector_id",
"connectorId",
"connector_name",
"connectorName",
"connector_display_name",
"connectorDisplayName",
] {
assert!(
!caller_meta.contains_key(reserved_key),
"direct MCP calls should strip {reserved_key}: {caller_meta:?}"
);
}
server.verify().await;
Ok(())
+4
View File
@@ -605,6 +605,10 @@ async fn tool_search_returns_deferred_tools_without_follow_up_tool_injection() -
let apps_tool_call = recorded_apps_tool_call_by_call_id(&server, "calendar-call-1").await;
assert_eq!(
apps_tool_call.pointer("/params/_meta/threadId"),
Some(&json!(test.session_configured.thread_id.to_string()))
);
assert_eq!(
apps_tool_call.pointer("/params/_meta/_codex_apps"),
Some(&json!({