[connectors] Track host-owned Codex Apps MCP provenance

[mzeng-openai]

Why

MCP server names are routing labels, not a trustworthy ownership boundary. Connector-gateway behavior should only apply to the built-in Codex Apps MCP instance created by the host, not to an MCP server that merely has the same name or declares the same capabilities.

How provenance is determined

McpServerProvenance is host-assigned and runtime-only. MCP servers loaded from user/plugin config deserialize as UserConfigured; the only path that marks a server HostOwnedCodexApps is the host path that injects Codex Apps MCP at codex_apps using the ChatGPT backend /wham/apps endpoint. Because provenance is not serialized, cached Codex Apps tools are restamped as host-owned when loaded.

What changed

• Add runtime-only McpServerProvenance and mark the host-injected Apps MCP as HostOwnedCodexApps.
• Carry that provenance through managed MCP clients and emitted ToolInfo.
• Switch connector-gateway behavior from raw server-name checks to provenance checks across auth/cache setup, connector discovery, tool exposure, and MCP tool-call approval handling.
• Add regression coverage proving that a spoofed codex_apps server name is not treated as host-owned.

Testing

• cargo test -p codex-mcp
• cargo test -p codex-core mcp_tool_call

[chatgpt-codex-connector]

Codex Review: Something went wrong. Try again later by commenting “@codex review”.

An unknown error occurred

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[viyatb-oai]

Flagging one security concern from Viyat's security review automation.

The cached Codex Apps tool path appears to drop host-owned provenance. ToolInfo.server_provenance is skipped during serde, so when cached tools are loaded they come back with the default UserConfigured provenance. load_cached_codex_apps_tools then returns those tools without re-stamping them as HostOwnedCodexApps.

That matters because the exposure filtering treats host-owned Codex Apps tools differently: connector accessibility/enabled checks only apply when is_host_owned_codex_apps() is true. A cached app tool can therefore be model/tool-search visible as a normal MCP tool even when its connector should be disabled or inaccessible. The actual tool call path may recover once the live server is connected, but the model-visible surface is already wider than intended.

The fix I'd suggest: after deserializing the cache, set every cached Codex Apps ToolInfo.server_provenance = McpServerProvenance::HostOwnedCodexApps before running exposure filtering. Longer term, consider a cache DTO that does not serialize ToolInfo directly and converts back into ToolInfo with explicit provenance. A small regression should load a cached app tool for a disabled/inaccessible connector and assert build_mcp_tool_exposure does not expose it.

[mzeng-openai]

Good catch. Cached Codex Apps tools now get restamped as HostOwnedCodexApps at the cache boundary, and I added a regression for the startup-cache load. The current manager path was already stamping before exposure, but this makes the invariant local so future direct cache reads cannot miss it.

[bolinfest]

Can/do we want to impose any restrictions on MCP server names read from config.toml so that "internal" MCP servers cannot run the risk of name collisions with user-defined ones?

You could also use https://doc.rust-lang.org/std/collections/struct.HashMap.html#method.entry and ensure your insert() is not and overwrite and if so error?

[bolinfest]

Should this get moved to mod_tests.rs instead?

[bolinfest]

This is adding a lot of code to a file that is already fairly large: I would put this in a separate file (and move any tests to a parallel file, as appropriate).

[bolinfest]

You really want something like this to ensure the integrity of the String value:

mod trusted_codex_apps_mcp_url {
    #[derive(Debug, Clone, PartialEq, Eq)]
    pub struct TrustedCodexAppsMcpUrl(String);

    impl TrustedCodexAppsMcpUrl {
        pub fn new(url: String) -> Option<Self> {
            if is_trusted_codex_apps_mcp_url(&url) {
                Some(Self(url))
            } else {
                None
            }
        }

        pub fn as_str(&self) -> &str {
            &self.0
        }
    }

    fn is_trusted_codex_apps_mcp_url(url: &str) -> bool {
        // your validation logic here
        url.starts_with("https://")
    }
}

pub use trusted_codex_apps_mcp_url::TrustedCodexAppsMcpUrl;

All the more reason to move this to a separate file to restrict the public API!