mirror of
https://github.com/openai/codex.git
synced 2026-05-29 23:40:29 +00:00
Validate host-owned Codex Apps MCP endpoint
This commit is contained in:
@@ -36,6 +36,8 @@ use codex_protocol::protocol::McpListToolsResponseEvent;
|
||||
use rmcp::model::ReadResourceRequestParams;
|
||||
use rmcp::model::ReadResourceResult;
|
||||
use serde_json::Value;
|
||||
use url::Host;
|
||||
use url::Url;
|
||||
|
||||
use crate::codex_apps::codex_apps_tools_cache_key;
|
||||
use crate::connection_manager::McpConnectionManager;
|
||||
@@ -205,10 +207,15 @@ pub fn with_codex_apps_mcp(
|
||||
config: &McpConfig,
|
||||
) -> HashMap<String, McpServerConfig> {
|
||||
if config.apps_enabled && auth.is_some_and(CodexAuth::uses_codex_backend) {
|
||||
servers.insert(
|
||||
CODEX_APPS_MCP_SERVER_NAME.to_string(),
|
||||
codex_apps_mcp_server_config(config),
|
||||
);
|
||||
match codex_apps_mcp_server_config(config) {
|
||||
Ok(server_config) => {
|
||||
servers.insert(CODEX_APPS_MCP_SERVER_NAME.to_string(), server_config);
|
||||
}
|
||||
Err(err) => {
|
||||
tracing::warn!("Skipping host-owned Codex Apps MCP server: {err}");
|
||||
servers.remove(CODEX_APPS_MCP_SERVER_NAME);
|
||||
}
|
||||
}
|
||||
} else {
|
||||
servers.remove(CODEX_APPS_MCP_SERVER_NAME);
|
||||
}
|
||||
@@ -353,11 +360,9 @@ pub async fn collect_mcp_snapshot_from_manager(
|
||||
.await
|
||||
}
|
||||
|
||||
pub(crate) fn codex_apps_mcp_url(config: &McpConfig) -> String {
|
||||
codex_apps_mcp_url_for_base_url(
|
||||
&config.chatgpt_base_url,
|
||||
config.apps_mcp_path_override.as_deref(),
|
||||
)
|
||||
#[cfg(test)]
|
||||
pub(crate) fn codex_apps_mcp_url(config: &McpConfig) -> Result<String, String> {
|
||||
host_owned_codex_apps_mcp_endpoint(config).map(|endpoint| endpoint.url.0)
|
||||
}
|
||||
|
||||
/// The Responses API requires tool names to match `^[a-zA-Z0-9_-]+$`.
|
||||
@@ -389,19 +394,47 @@ fn codex_apps_mcp_bearer_token_env_var() -> Option<String> {
|
||||
}
|
||||
}
|
||||
|
||||
fn normalize_codex_apps_base_url(base_url: &str) -> String {
|
||||
let mut base_url = base_url.trim_end_matches('/').to_string();
|
||||
if (base_url.starts_with("https://chatgpt.com")
|
||||
|| base_url.starts_with("https://chat.openai.com"))
|
||||
&& !base_url.contains("/backend-api")
|
||||
#[derive(Debug, Clone, PartialEq, Eq)]
|
||||
struct HostOwnedCodexAppsMcpEndpoint {
|
||||
url: TrustedCodexAppsMcpUrl,
|
||||
provenance: McpServerProvenance,
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, PartialEq, Eq)]
|
||||
struct TrustedCodexAppsMcpUrl(String);
|
||||
|
||||
fn host_owned_codex_apps_mcp_endpoint(
|
||||
config: &McpConfig,
|
||||
) -> Result<HostOwnedCodexAppsMcpEndpoint, String> {
|
||||
// HostOwnedCodexApps gates first-party connector behavior, including
|
||||
// privileged file upload handling. Keep the trusted URL check and the
|
||||
// provenance grant together so a config-derived URL cannot receive the
|
||||
// host-owned marker without passing this audit point.
|
||||
let url = codex_apps_mcp_url_for_base_url(
|
||||
&config.chatgpt_base_url,
|
||||
config.apps_mcp_path_override.as_deref(),
|
||||
)?;
|
||||
Ok(HostOwnedCodexAppsMcpEndpoint {
|
||||
url,
|
||||
provenance: McpServerProvenance::HostOwnedCodexApps,
|
||||
})
|
||||
}
|
||||
|
||||
fn codex_apps_mcp_url_for_base_url(
|
||||
base_url: &str,
|
||||
apps_mcp_path_override: Option<&str>,
|
||||
) -> Result<TrustedCodexAppsMcpUrl, String> {
|
||||
let base_url = base_url.trim_end_matches('/');
|
||||
let parsed_base_url = Url::parse(base_url)
|
||||
.map_err(|err| format!("invalid Codex Apps MCP base URL `{base_url}`: {err}"))?;
|
||||
validate_codex_apps_mcp_base_url(&parsed_base_url, base_url)?;
|
||||
|
||||
let mut base_url = base_url.to_string();
|
||||
if is_allowed_codex_apps_chatgpt_host(&parsed_base_url.host())
|
||||
&& !parsed_base_url.path().contains("/backend-api")
|
||||
{
|
||||
base_url = format!("{base_url}/backend-api");
|
||||
}
|
||||
base_url
|
||||
}
|
||||
|
||||
fn codex_apps_mcp_url_for_base_url(base_url: &str, apps_mcp_path_override: Option<&str>) -> String {
|
||||
let base_url = normalize_codex_apps_base_url(base_url);
|
||||
let (base_url, default_path) = if base_url.contains("/backend-api") {
|
||||
(base_url, "wham/apps")
|
||||
} else if base_url.contains("/api/codex") {
|
||||
@@ -412,13 +445,63 @@ fn codex_apps_mcp_url_for_base_url(base_url: &str, apps_mcp_path_override: Optio
|
||||
let path = apps_mcp_path_override
|
||||
.unwrap_or(default_path)
|
||||
.trim_start_matches('/');
|
||||
format!("{base_url}/{path}")
|
||||
Ok(TrustedCodexAppsMcpUrl(format!("{base_url}/{path}")))
|
||||
}
|
||||
|
||||
fn codex_apps_mcp_server_config(config: &McpConfig) -> McpServerConfig {
|
||||
let url = codex_apps_mcp_url(config);
|
||||
fn validate_codex_apps_mcp_base_url(url: &Url, original_base_url: &str) -> Result<(), String> {
|
||||
if !url.username().is_empty()
|
||||
|| url.password().is_some()
|
||||
|| url.query().is_some()
|
||||
|| url.fragment().is_some()
|
||||
{
|
||||
return Err(format!(
|
||||
"invalid Codex Apps MCP base URL `{original_base_url}`; expected a URL without credentials, query, or fragment"
|
||||
));
|
||||
}
|
||||
|
||||
McpServerConfig {
|
||||
let scheme = url.scheme();
|
||||
let host = url.host();
|
||||
let valid_first_party_url =
|
||||
scheme == "https" && url.port().is_none() && is_allowed_codex_apps_chatgpt_host(&host);
|
||||
let valid_local_url = cfg!(debug_assertions)
|
||||
&& matches!(scheme, "http" | "https")
|
||||
&& is_localhost_for_codex_apps(&host);
|
||||
if valid_first_party_url || valid_local_url {
|
||||
Ok(())
|
||||
} else {
|
||||
Err(format!(
|
||||
"invalid Codex Apps MCP base URL `{original_base_url}`; expected an HTTPS URL for chatgpt.com, chat.openai.com, or chatgpt-staging.com"
|
||||
))
|
||||
}
|
||||
}
|
||||
|
||||
fn is_allowed_codex_apps_chatgpt_host(host: &Option<Host<&str>>) -> bool {
|
||||
let Some(Host::Domain(host)) = host else {
|
||||
return false;
|
||||
};
|
||||
let host = *host;
|
||||
|
||||
host == "chatgpt.com"
|
||||
|| host == "chat.openai.com"
|
||||
|| host == "chatgpt-staging.com"
|
||||
|| host.ends_with(".chatgpt.com")
|
||||
|| host.ends_with(".chatgpt-staging.com")
|
||||
}
|
||||
|
||||
fn is_localhost_for_codex_apps(host: &Option<Host<&str>>) -> bool {
|
||||
match host {
|
||||
Some(Host::Domain(host)) => *host == "localhost",
|
||||
Some(Host::Ipv4(ip)) => ip.is_loopback(),
|
||||
Some(Host::Ipv6(ip)) => ip.is_loopback(),
|
||||
_ => false,
|
||||
}
|
||||
}
|
||||
|
||||
fn codex_apps_mcp_server_config(config: &McpConfig) -> Result<McpServerConfig, String> {
|
||||
let endpoint = host_owned_codex_apps_mcp_endpoint(config)?;
|
||||
let TrustedCodexAppsMcpUrl(url) = endpoint.url;
|
||||
|
||||
Ok(McpServerConfig {
|
||||
transport: McpServerTransportConfig::StreamableHttp {
|
||||
url,
|
||||
bearer_token_env_var: codex_apps_mcp_bearer_token_env_var(),
|
||||
@@ -430,7 +513,7 @@ fn codex_apps_mcp_server_config(config: &McpConfig) -> McpServerConfig {
|
||||
required: false,
|
||||
supports_parallel_tool_calls: false,
|
||||
disabled_reason: None,
|
||||
provenance: McpServerProvenance::HostOwnedCodexApps,
|
||||
provenance: endpoint.provenance,
|
||||
startup_timeout_sec: Some(Duration::from_secs(30)),
|
||||
tool_timeout_sec: None,
|
||||
default_tools_approval_mode: None,
|
||||
@@ -439,7 +522,7 @@ fn codex_apps_mcp_server_config(config: &McpConfig) -> McpServerConfig {
|
||||
scopes: None,
|
||||
oauth_resource: None,
|
||||
tools: HashMap::new(),
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
fn protocol_tool_from_rmcp_tool(name: &str, tool: &rmcp::model::Tool) -> Option<Tool> {
|
||||
|
||||
@@ -170,45 +170,73 @@ fn codex_apps_mcp_url_for_base_url_keeps_existing_paths() {
|
||||
codex_apps_mcp_url_for_base_url(
|
||||
"https://chatgpt.com/backend-api",
|
||||
/*apps_mcp_path_override*/ None,
|
||||
),
|
||||
"https://chatgpt.com/backend-api/wham/apps"
|
||||
)
|
||||
.expect("trusted ChatGPT URL should build"),
|
||||
TrustedCodexAppsMcpUrl("https://chatgpt.com/backend-api/wham/apps".to_string())
|
||||
);
|
||||
assert_eq!(
|
||||
codex_apps_mcp_url_for_base_url(
|
||||
"https://chat.openai.com",
|
||||
/*apps_mcp_path_override*/ None,
|
||||
),
|
||||
"https://chat.openai.com/backend-api/wham/apps"
|
||||
)
|
||||
.expect("trusted legacy ChatGPT URL should build"),
|
||||
TrustedCodexAppsMcpUrl("https://chat.openai.com/backend-api/wham/apps".to_string())
|
||||
);
|
||||
assert_eq!(
|
||||
codex_apps_mcp_url_for_base_url(
|
||||
"http://localhost:8080/api/codex",
|
||||
/*apps_mcp_path_override*/ None,
|
||||
),
|
||||
"http://localhost:8080/api/codex/apps"
|
||||
)
|
||||
.expect("local debug URL should build"),
|
||||
TrustedCodexAppsMcpUrl("http://localhost:8080/api/codex/apps".to_string())
|
||||
);
|
||||
assert_eq!(
|
||||
codex_apps_mcp_url_for_base_url(
|
||||
"http://localhost:8080",
|
||||
/*apps_mcp_path_override*/ None,
|
||||
),
|
||||
"http://localhost:8080/api/codex/apps"
|
||||
)
|
||||
.expect("local debug URL should build"),
|
||||
TrustedCodexAppsMcpUrl("http://localhost:8080/api/codex/apps".to_string())
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn codex_apps_mcp_url_for_base_url_rejects_untrusted_urls() {
|
||||
for base_url in [
|
||||
"http://chatgpt.com/backend-api",
|
||||
"https://example.com/backend-api",
|
||||
"https://chatgpt.com.evil.example/backend-api",
|
||||
"https://evilchatgpt.com/backend-api",
|
||||
"https://foo.chat.openai.com/backend-api",
|
||||
"https://chatgpt.com:4443/backend-api",
|
||||
"https://user:pass@chatgpt.com/backend-api",
|
||||
"https://chatgpt.com/backend-api?token=secret",
|
||||
] {
|
||||
let err = codex_apps_mcp_url_for_base_url(base_url, /*apps_mcp_path_override*/ None)
|
||||
.expect_err("untrusted URL should be rejected");
|
||||
|
||||
assert!(
|
||||
err.starts_with("invalid Codex Apps MCP base URL"),
|
||||
"unexpected error: {err}"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn codex_apps_mcp_url_uses_legacy_codex_apps_path() {
|
||||
let config = test_mcp_config(PathBuf::from("/tmp"));
|
||||
let codex_home = tempfile::tempdir().expect("tempdir");
|
||||
let config = test_mcp_config(codex_home.path().to_path_buf());
|
||||
|
||||
assert_eq!(
|
||||
codex_apps_mcp_url(&config),
|
||||
codex_apps_mcp_url(&config).expect("trusted ChatGPT URL should build"),
|
||||
"https://chatgpt.com/backend-api/wham/apps"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn codex_apps_server_config_uses_legacy_codex_apps_path() {
|
||||
let mut config = test_mcp_config(PathBuf::from("/tmp"));
|
||||
let codex_home = tempfile::tempdir().expect("tempdir");
|
||||
let mut config = test_mcp_config(codex_home.path().to_path_buf());
|
||||
let auth = CodexAuth::create_dummy_chatgpt_auth_for_testing();
|
||||
|
||||
let mut servers = with_codex_apps_mcp(HashMap::new(), /*auth*/ None, &config);
|
||||
@@ -230,17 +258,35 @@ fn codex_apps_server_config_uses_legacy_codex_apps_path() {
|
||||
|
||||
#[test]
|
||||
fn codex_apps_server_config_is_marked_host_owned() {
|
||||
let config = test_mcp_config(PathBuf::from("/tmp"));
|
||||
let codex_home = tempfile::tempdir().expect("tempdir");
|
||||
let config = test_mcp_config(codex_home.path().to_path_buf());
|
||||
|
||||
assert_eq!(
|
||||
codex_apps_mcp_server_config(&config).provenance,
|
||||
codex_apps_mcp_server_config(&config)
|
||||
.expect("trusted ChatGPT URL should build")
|
||||
.provenance,
|
||||
McpServerProvenance::HostOwnedCodexApps
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn host_owned_codex_apps_endpoint_pairs_trusted_url_with_provenance() {
|
||||
let codex_home = tempfile::tempdir().expect("tempdir");
|
||||
let config = test_mcp_config(codex_home.path().to_path_buf());
|
||||
|
||||
assert_eq!(
|
||||
host_owned_codex_apps_mcp_endpoint(&config).expect("trusted ChatGPT URL should build"),
|
||||
HostOwnedCodexAppsMcpEndpoint {
|
||||
url: TrustedCodexAppsMcpUrl("https://chatgpt.com/backend-api/wham/apps".to_string(),),
|
||||
provenance: McpServerProvenance::HostOwnedCodexApps,
|
||||
}
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn codex_apps_server_config_uses_configured_apps_mcp_path_override() {
|
||||
let mut config = test_mcp_config(PathBuf::from("/tmp"));
|
||||
let codex_home = tempfile::tempdir().expect("tempdir");
|
||||
let mut config = test_mcp_config(codex_home.path().to_path_buf());
|
||||
config.apps_mcp_path_override = Some("/custom/mcp".to_string());
|
||||
config.apps_enabled = true;
|
||||
let auth = CodexAuth::create_dummy_chatgpt_auth_for_testing();
|
||||
@@ -257,6 +303,19 @@ fn codex_apps_server_config_uses_configured_apps_mcp_path_override() {
|
||||
assert_eq!(url, "https://chatgpt.com/backend-api/custom/mcp");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn with_codex_apps_mcp_does_not_trust_untrusted_base_url() {
|
||||
let codex_home = tempfile::tempdir().expect("tempdir");
|
||||
let mut config = test_mcp_config(codex_home.path().to_path_buf());
|
||||
config.apps_enabled = true;
|
||||
config.chatgpt_base_url = "https://chatgpt.com.evil.example/backend-api".to_string();
|
||||
let auth = CodexAuth::create_dummy_chatgpt_auth_for_testing();
|
||||
|
||||
let servers = with_codex_apps_mcp(HashMap::new(), Some(&auth), &config);
|
||||
|
||||
assert!(!servers.contains_key(CODEX_APPS_MCP_SERVER_NAME));
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn effective_mcp_servers_preserve_user_servers_and_add_codex_apps() {
|
||||
let codex_home = tempfile::tempdir().expect("tempdir");
|
||||
|
||||
Reference in New Issue
Block a user