[codex] Remove hardcoded app ID filters (#28947)

## Summary

- remove the duplicated originator-specific connector ID denylists
- stop filtering connector directory/accessibility results and
live/cached Codex Apps MCP tools by hardcoded connector ID
- remove the now-unused `codex-login` dependency from
`codex-utils-plugins`
- update regression coverage so formerly blocked connector IDs are
preserved

## Why

The client-side policy was duplicated across crates, used opaque IDs
without ownership or expiry information, and could drift between app
listing and MCP tool behavior. Server-provided visibility,
authorization, plugin discoverability, accessibility, enabled-state
handling, and consequential-tool approval templates remain unchanged.

## Validation

- `just fmt`
- `just bazel-lock-update`
- `just bazel-lock-check`
- `git diff --check`
- confirmed the final diff contains no hardcoded denylist symbols

A targeted `codex-mcp` test build spent an unusually long time in local
compilation/linking. Its first attempt exposed a test-only `PartialEq`
assertion issue, which was corrected. A follow-up non-linking `cargo
check -p codex-mcp --tests` was still running when this draft was
opened; CI should provide the complete Rust validation.
This commit is contained in:
Eric Ning
2026-06-18 13:29:01 -07:00
committed by GitHub
Unverified
parent d9dace8a59
commit 29eb434bc5
11 changed files with 81 additions and 209 deletions
-1
View File
@@ -4271,7 +4271,6 @@ name = "codex-utils-plugins"
version = "0.0.0"
dependencies = [
"codex-exec-server",
"codex-login",
"codex-utils-absolute-path",
"codex-utils-path-uri",
"serde",
@@ -365,10 +365,11 @@ connectors = false
#[tokio::test]
async fn list_apps_keeps_apps_with_app_only_tools_accessible() -> Result<()> {
let connector_id = "connector_2b0a9009c9c64bf9933a3dae3f2b1254";
let connectors = vec![AppInfo {
id: "beta".to_string(),
name: "Beta".to_string(),
description: Some("Beta connector".to_string()),
id: connector_id.to_string(),
name: "Formerly Blocked".to_string(),
description: Some("Formerly blocked connector".to_string()),
logo_url: None,
logo_url_dark: None,
distribution_channel: None,
@@ -380,7 +381,7 @@ async fn list_apps_keeps_apps_with_app_only_tools_accessible() -> Result<()> {
is_enabled: true,
plugin_display_names: Vec::new(),
}];
let mut app_only_tool = connector_tool("beta", "Beta App")?;
let mut app_only_tool = connector_tool(connector_id, "Formerly Blocked")?;
app_only_tool
.meta
.as_mut()
@@ -421,7 +422,7 @@ async fn list_apps_keeps_apps_with_app_only_tools_accessible() -> Result<()> {
let AppsListResponse { data, next_cursor } = to_response(response)?;
assert_eq!(data.len(), 1);
assert_eq!(data[0].id, "beta");
assert_eq!(data[0].id, connector_id);
assert!(data[0].is_accessible);
assert!(next_cursor.is_none());
@@ -1245,7 +1245,7 @@ async fn plugin_install_skips_mcp_oauth_for_chatgpt_dual_surface_plugin() -> Res
}
#[tokio::test]
async fn plugin_install_starts_mcp_oauth_when_only_plugin_apps_are_disallowed() -> Result<()> {
async fn plugin_install_starts_mcp_oauth_with_formerly_disallowed_plugin_app() -> Result<()> {
let (apps_server_url, apps_server_handle, _apps_server_control) =
start_apps_server(Vec::new(), Vec::new()).await?;
let oauth_server = MockServer::start().await;
@@ -1296,8 +1296,22 @@ async fn plugin_install_starts_mcp_oauth_when_only_plugin_apps_are_disallowed()
.await??;
let response: PluginInstallResponse = to_response(response)?;
assert_eq!(response.auth_policy, PluginAuthPolicy::OnInstall);
assert_eq!(response.apps_needing_auth, Vec::<AppSummary>::new());
assert_eq!(
response,
PluginInstallResponse {
auth_policy: PluginAuthPolicy::OnInstall,
apps_needing_auth: vec![AppSummary {
id: "asdk_app_6938a94a61d881918ef32cb999ff937c".to_string(),
name: "asdk_app_6938a94a61d881918ef32cb999ff937c".to_string(),
description: None,
install_url: Some(
"https://chatgpt.com/apps/asdk-app-6938a94a61d881918ef32cb999ff937c/asdk_app_6938a94a61d881918ef32cb999ff937c"
.to_string(),
),
category: None,
}],
}
);
assert!(oauth_discovery_request_count(&oauth_server).await > 0);
apps_server_handle.abort();
@@ -1461,7 +1475,7 @@ async fn plugin_install_skips_remote_mcp_oauth_for_bundled_same_name_app() -> Re
}
#[tokio::test]
async fn plugin_install_filters_disallowed_apps_needing_auth() -> Result<()> {
async fn plugin_install_includes_formerly_disallowed_apps_needing_auth() -> Result<()> {
let connectors = vec![AppInfo {
id: "alpha".to_string(),
name: "Alpha".to_string(),
@@ -1538,6 +1552,16 @@ async fn plugin_install_filters_disallowed_apps_needing_auth() -> Result<()> {
description: Some("Alpha connector".to_string()),
install_url: Some("https://chatgpt.com/apps/alpha/alpha".to_string()),
category: None,
},
AppSummary {
id: "asdk_app_6938a94a61d881918ef32cb999ff937c".to_string(),
name: "asdk_app_6938a94a61d881918ef32cb999ff937c".to_string(),
description: None,
install_url: Some(
"https://chatgpt.com/apps/asdk-app-6938a94a61d881918ef32cb999ff937c/asdk_app_6938a94a61d881918ef32cb999ff937c"
.to_string(),
),
category: None,
}],
}
);
+23 -22
View File
@@ -8,7 +8,6 @@ use codex_app_server_protocol::AppInfo;
use codex_connectors::ConnectorDirectoryCacheContext;
use codex_connectors::ConnectorDirectoryCacheKey;
use codex_connectors::DirectoryListResponse;
use codex_connectors::filter::filter_disallowed_connectors;
use codex_connectors::merge::merge_connectors;
use codex_connectors::merge::merge_plugin_connectors;
use codex_core::config::Config;
@@ -21,7 +20,6 @@ pub use codex_core::connectors::list_cached_accessible_connectors_from_mcp_tools
pub use codex_core::connectors::with_app_enabled_state;
use codex_login::AuthManager;
use codex_login::CodexAuth;
use codex_login::default_client::originator;
use codex_plugin::AppConnectorId;
const DIRECTORY_CONNECTORS_TIMEOUT: Duration = Duration::from_secs(60);
@@ -82,7 +80,10 @@ pub async fn list_cached_all_connectors(
let auth = connector_auth(config).await.ok()?;
let cache_context = connector_directory_cache_context(config, &auth);
let connectors = codex_connectors::cached_directory_connectors(&cache_context)?;
Some(merge_and_filter_plugin_connectors(connectors, plugin_apps))
Some(merge_directory_and_plugin_connectors(
connectors,
plugin_apps,
))
}
pub async fn list_all_connectors_with_options(
@@ -109,7 +110,10 @@ pub async fn list_all_connectors_with_options(
},
)
.await?;
Ok(merge_and_filter_plugin_connectors(connectors, plugin_apps))
Ok(merge_directory_and_plugin_connectors(
connectors,
plugin_apps,
))
}
fn connector_directory_cache_context(
@@ -127,17 +131,16 @@ fn connector_directory_cache_context(
)
}
fn merge_and_filter_plugin_connectors(
fn merge_directory_and_plugin_connectors(
connectors: Vec<AppInfo>,
plugin_apps: &[AppConnectorId],
) -> Vec<AppInfo> {
let connectors = merge_plugin_connectors(
merge_plugin_connectors(
connectors,
plugin_apps
.iter()
.map(|connector_id| connector_id.0.clone()),
);
filter_disallowed_connectors(connectors, originator().value.as_str())
)
}
pub fn connectors_for_plugin_apps(
@@ -150,11 +153,10 @@ pub fn connectors_for_plugin_apps(
.iter()
.map(|connector_id| connector_id.0.clone()),
);
let mut connectors_by_id =
filter_disallowed_connectors(connectors, originator().value.as_str())
.into_iter()
.map(|connector| (connector.id.clone(), connector))
.collect::<HashMap<_, _>>();
let mut connectors_by_id = connectors
.into_iter()
.map(|connector| (connector.id.clone(), connector))
.collect::<HashMap<_, _>>();
plugin_apps
.iter()
@@ -179,8 +181,7 @@ pub fn merge_connectors_with_accessible(
} else {
accessible_connectors
};
let merged = merge_connectors(connectors, accessible_connectors);
filter_disallowed_connectors(merged, originator().value.as_str())
merge_connectors(connectors, accessible_connectors)
}
#[cfg(test)]
@@ -269,13 +270,13 @@ mod tests {
}
#[test]
fn connectors_for_plugin_apps_filters_disallowed_plugin_apps() {
let connectors = connectors_for_plugin_apps(
Vec::new(),
&[AppConnectorId(
"asdk_app_6938a94a61d881918ef32cb999ff937c".to_string(),
)],
fn connectors_for_plugin_apps_preserves_formerly_disallowed_plugin_apps() {
let connector_id = "asdk_app_6938a94a61d881918ef32cb999ff937c";
let connectors =
connectors_for_plugin_apps(Vec::new(), &[AppConnectorId(connector_id.to_string())]);
assert_eq!(
connectors,
vec![merged_app(connector_id, /*is_accessible*/ false)]
);
assert_eq!(connectors, Vec::<AppInfo>::new());
}
}
+3 -16
View File
@@ -15,7 +15,6 @@ use crate::tools::ToolInfo;
use anyhow::Context;
use codex_login::CodexAuth;
use codex_protocol::mcp::McpServerInfo;
use codex_utils_plugins::mcp_connector::is_connector_id_allowed;
use codex_utils_plugins::mcp_connector::sanitize_name;
use serde::Deserialize;
use serde::Serialize;
@@ -220,7 +219,7 @@ pub(crate) fn load_cached_codex_apps_tools(
if cache.schema_version != CODEX_APPS_TOOLS_CACHE_SCHEMA_VERSION {
return CachedCodexAppsToolsLoad::Invalid;
}
CachedCodexAppsToolsLoad::Hit(filter_disallowed_codex_apps_tools(cache.tools))
CachedCodexAppsToolsLoad::Hit(cache.tools)
}
pub(crate) fn write_cached_codex_apps_tools(
@@ -233,10 +232,9 @@ pub(crate) fn write_cached_codex_apps_tools(
{
return;
}
let tools = filter_disallowed_codex_apps_tools(tools.to_vec());
let Ok(bytes) = serde_json::to_vec_pretty(&CodexAppsToolsDiskCache {
schema_version: CODEX_APPS_TOOLS_CACHE_SCHEMA_VERSION,
tools,
tools: tools.to_vec(),
}) else {
return;
};
@@ -279,17 +277,6 @@ fn write_cached_codex_apps_server_info(
Ok(())
}
pub(crate) fn filter_disallowed_codex_apps_tools(tools: Vec<ToolInfo>) -> Vec<ToolInfo> {
tools
.into_iter()
.filter(|tool| {
tool.connector_id
.as_deref()
.is_none_or(is_connector_id_allowed)
})
.collect()
}
#[derive(Debug, Clone, Serialize, Deserialize)]
struct CodexAppsToolsDiskCache {
schema_version: u8,
@@ -303,7 +290,7 @@ struct CodexAppsServerInfoDiskCache {
}
const CODEX_APPS_TOOLS_CACHE_DIR: &str = "cache/codex_apps_tools";
pub(crate) const CODEX_APPS_TOOLS_CACHE_SCHEMA_VERSION: u8 = 3;
pub(crate) const CODEX_APPS_TOOLS_CACHE_SCHEMA_VERSION: u8 = 4;
const CODEX_APPS_SERVER_INFO_CACHE_DIR: &str = "cache/codex_apps_server_info";
const CODEX_APPS_SERVER_INFO_CACHE_SCHEMA_VERSION: u8 = 1;
@@ -612,7 +612,7 @@ fn codex_apps_tools_cache_is_scoped_per_user() {
}
#[test]
fn codex_apps_tools_cache_filters_disallowed_connectors() {
fn codex_apps_tools_cache_preserves_formerly_disallowed_connectors() {
let codex_home = tempdir().expect("tempdir");
let cache_context = create_codex_apps_tools_cache_context(
codex_home.path().to_path_buf(),
@@ -622,13 +622,13 @@ fn codex_apps_tools_cache_filters_disallowed_connectors() {
let tools = vec![
create_test_tool_with_connector(
CODEX_APPS_MCP_SERVER_NAME,
"blocked_tool",
"formerly_blocked_tool",
"connector_2b0a9009c9c64bf9933a3dae3f2b1254",
Some("Blocked"),
Some("Formerly Blocked"),
),
create_test_tool_with_connector(
CODEX_APPS_MCP_SERVER_NAME,
"allowed_tool",
"calendar_tool",
"calendar",
Some("Calendar"),
),
@@ -637,9 +637,19 @@ fn codex_apps_tools_cache_filters_disallowed_connectors() {
write_cached_codex_apps_tools(&cache_context, &tools);
let cached = read_cached_codex_apps_tools(&cache_context).expect("cache entry exists for user");
assert_eq!(cached.len(), 1);
assert_eq!(cached[0].callable_name, "allowed_tool");
assert_eq!(cached[0].connector_id.as_deref(), Some("calendar"));
assert_eq!(
cached
.iter()
.map(|tool| (tool.callable_name.as_str(), tool.connector_id.as_deref()))
.collect::<Vec<_>>(),
vec![
(
"formerly_blocked_tool",
Some("connector_2b0a9009c9c64bf9933a3dae3f2b1254")
),
("calendar_tool", Some("calendar")),
]
);
}
#[test]
-4
View File
@@ -19,7 +19,6 @@ use std::time::Instant;
use crate::codex_apps::CachedCodexAppsToolsLoad;
use crate::codex_apps::CodexAppsToolsCacheContext;
use crate::codex_apps::filter_disallowed_codex_apps_tools;
use crate::codex_apps::load_cached_codex_apps_tools;
use crate::codex_apps::load_startup_cached_codex_apps_server_info;
use crate::codex_apps::load_startup_cached_codex_apps_tools_snapshot;
@@ -406,9 +405,6 @@ pub(crate) async fn list_tools_for_client_uncached(
}
})
.collect();
if server_name == CODEX_APPS_MCP_SERVER_NAME {
return Ok(filter_disallowed_codex_apps_tools(tools));
}
Ok(tools)
}
+1 -101
View File
@@ -6,7 +6,6 @@ pub fn filter_tool_suggest_discoverable_connectors(
directory_connectors: Vec<AppInfo>,
accessible_connectors: &[AppInfo],
discoverable_connector_ids: &HashSet<String>,
originator_value: &str,
) -> Vec<AppInfo> {
let accessible_connector_ids: HashSet<&str> = accessible_connectors
.iter()
@@ -14,7 +13,7 @@ pub fn filter_tool_suggest_discoverable_connectors(
.map(|connector| connector.id.as_str())
.collect();
let mut connectors = filter_disallowed_connectors(directory_connectors, originator_value)
let mut connectors = directory_connectors
.into_iter()
.filter(|connector| !accessible_connector_ids.contains(connector.id.as_str()))
.filter(|connector| discoverable_connector_ids.contains(connector.id.as_str()))
@@ -27,44 +26,6 @@ pub fn filter_tool_suggest_discoverable_connectors(
connectors
}
const DISALLOWED_CONNECTOR_IDS: &[&str] = &[
"asdk_app_6938a94a61d881918ef32cb999ff937c",
"connector_2b0a9009c9c64bf9933a3dae3f2b1254",
"connector_3f8d1a79f27c4c7ba1a897ab13bf37dc",
"connector_68de829bf7648191acd70a907364c67c",
"connector_68e004f14af881919eb50893d3d9f523",
"connector_69272cb413a081919685ec3c88d1744e",
];
const FIRST_PARTY_CHAT_DISALLOWED_CONNECTOR_IDS: &[&str] =
&["connector_0f9c9d4592e54d0a9a12b3f44a1e2010"];
pub fn filter_disallowed_connectors(
connectors: Vec<AppInfo>,
originator_value: &str,
) -> Vec<AppInfo> {
let first_party_chat_originator = is_first_party_chat_originator(originator_value);
connectors
.into_iter()
.filter(|connector| {
is_connector_id_allowed(connector.id.as_str(), first_party_chat_originator)
})
.collect()
}
fn is_first_party_chat_originator(originator_value: &str) -> bool {
originator_value == "codex_atlas" || originator_value == "codex_chatgpt_desktop"
}
fn is_connector_id_allowed(connector_id: &str, first_party_chat_originator: bool) -> bool {
let disallowed_connector_ids = if first_party_chat_originator {
FIRST_PARTY_CHAT_DISALLOWED_CONNECTOR_IDS
} else {
DISALLOWED_CONNECTOR_IDS
};
!disallowed_connector_ids.contains(&connector_id)
}
#[cfg(test)]
mod tests {
use super::*;
@@ -98,65 +59,6 @@ mod tests {
}
}
#[test]
fn filter_disallowed_connectors_allows_non_disallowed_connectors() {
let filtered =
filter_disallowed_connectors(vec![app("asdk_app_hidden"), app("alpha")], "codex_cli");
assert_eq!(filtered, vec![app("asdk_app_hidden"), app("alpha")]);
}
#[test]
fn filter_disallowed_connectors_allows_openai_prefix() {
let filtered = filter_disallowed_connectors(
vec![
app("connector_openai_foo"),
app("connector_openai_bar"),
app("gamma"),
],
"codex_cli",
);
assert_eq!(
filtered,
vec![
app("connector_openai_foo"),
app("connector_openai_bar"),
app("gamma")
]
);
}
#[test]
fn filter_disallowed_connectors_filters_disallowed_connector_ids() {
let filtered = filter_disallowed_connectors(
vec![
app("asdk_app_6938a94a61d881918ef32cb999ff937c"),
app("connector_3f8d1a79f27c4c7ba1a897ab13bf37dc"),
app("delta"),
],
"codex_cli",
);
assert_eq!(filtered, vec![app("delta")]);
}
#[test]
fn first_party_chat_originator_filters_target_connector_ids() {
let filtered = filter_disallowed_connectors(
vec![
app("connector_openai_foo"),
app("asdk_app_6938a94a61d881918ef32cb999ff937c"),
app("connector_0f9c9d4592e54d0a9a12b3f44a1e2010"),
],
"codex_atlas",
);
assert_eq!(
filtered,
vec![
app("connector_openai_foo"),
app("asdk_app_6938a94a61d881918ef32cb999ff937c")
]
);
}
#[test]
fn filter_tool_suggest_discoverable_connectors_keeps_only_plugin_backed_uninstalled_apps() {
let filtered = filter_tool_suggest_discoverable_connectors(
@@ -179,7 +81,6 @@ mod tests {
"connector_2128aebfecb84f64a069897515042a44".to_string(),
"connector_68df038e0ba48191908c8434991bbac2".to_string(),
]),
"codex_cli",
);
assert_eq!(
@@ -219,7 +120,6 @@ mod tests {
"connector_2128aebfecb84f64a069897515042a44".to_string(),
"connector_68df038e0ba48191908c8434991bbac2".to_string(),
]),
"codex_cli",
);
assert_eq!(filtered, Vec::<AppInfo>::new());
+3 -20
View File
@@ -31,7 +31,6 @@ use codex_core_plugins::PluginsManager;
use codex_features::Feature;
use codex_login::AuthManager;
use codex_login::CodexAuth;
use codex_login::default_client::originator;
use codex_mcp::CODEX_APPS_MCP_SERVER_NAME;
use codex_mcp::McpConnectionManager;
use codex_mcp::McpRuntimeContext;
@@ -112,7 +111,6 @@ pub(crate) async fn list_tool_suggest_discoverable_tools_with_auth(
directory_connectors,
accessible_connectors,
&connector_ids,
originator().value.as_str(),
)
.into_iter()
.map(DiscoverableTool::from);
@@ -143,12 +141,7 @@ pub async fn list_cached_accessible_connectors_from_mcp_tools(
return Some(Vec::new());
}
let cache_key = accessible_connectors_cache_key(config, auth.as_ref());
read_cached_accessible_connectors(&cache_key).map(|connectors| {
codex_connectors::filter::filter_disallowed_connectors(
connectors,
originator().value.as_str(),
)
})
read_cached_accessible_connectors(&cache_key)
}
pub(crate) fn refresh_accessible_connectors_cache_from_mcp_tools(
@@ -161,10 +154,7 @@ pub(crate) fn refresh_accessible_connectors_cache_from_mcp_tools(
}
let cache_key = accessible_connectors_cache_key(config, auth);
let accessible_connectors = codex_connectors::filter::filter_disallowed_connectors(
accessible_connectors_from_mcp_tools(mcp_tools),
originator().value.as_str(),
);
let accessible_connectors = accessible_connectors_from_mcp_tools(mcp_tools);
write_cached_accessible_connectors(cache_key, &accessible_connectors);
}
@@ -240,10 +230,6 @@ pub async fn list_accessible_connectors_from_mcp_tools_with_mcp_manager(
let tool_plugin_provenance = tool_plugin_provenance(&mcp_config);
if !force_refetch && let Some(cached_connectors) = read_cached_accessible_connectors(&cache_key)
{
let cached_connectors = codex_connectors::filter::filter_disallowed_connectors(
cached_connectors,
originator().value.as_str(),
);
let cached_connectors = with_app_plugin_sources(cached_connectors, &tool_plugin_provenance);
return Ok(AccessibleConnectorsStatus {
connectors: cached_connectors,
@@ -353,10 +339,7 @@ pub async fn list_accessible_connectors_from_mcp_tools_with_mcp_manager(
cancel_token.cancel();
}
let accessible_connectors = codex_connectors::filter::filter_disallowed_connectors(
accessible_connectors_from_mcp_tools(&tools),
originator().value.as_str(),
);
let accessible_connectors = accessible_connectors_from_mcp_tools(&tools);
if codex_apps_ready || !accessible_connectors.is_empty() {
write_cached_accessible_connectors(cache_key, &accessible_connectors);
}
-1
View File
@@ -15,7 +15,6 @@ workspace = true
[dependencies]
codex-exec-server = { workspace = true }
codex-login = { workspace = true }
codex-utils-absolute-path = { workspace = true }
codex-utils-path-uri = { workspace = true }
serde = { workspace = true, features = ["derive"] }
@@ -1,31 +1,3 @@
use codex_login::default_client::is_first_party_chat_originator;
use codex_login::default_client::originator;
const DISALLOWED_CONNECTOR_IDS: &[&str] = &[
"asdk_app_6938a94a61d881918ef32cb999ff937c",
"connector_2b0a9009c9c64bf9933a3dae3f2b1254",
"connector_3f8d1a79f27c4c7ba1a897ab13bf37dc",
"connector_68de829bf7648191acd70a907364c67c",
"connector_68e004f14af881919eb50893d3d9f523",
"connector_69272cb413a081919685ec3c88d1744e",
];
const FIRST_PARTY_CHAT_DISALLOWED_CONNECTOR_IDS: &[&str] =
&["connector_0f9c9d4592e54d0a9a12b3f44a1e2010"];
pub fn is_connector_id_allowed(connector_id: &str) -> bool {
is_connector_id_allowed_for_originator(connector_id, originator().value.as_str())
}
fn is_connector_id_allowed_for_originator(connector_id: &str, originator_value: &str) -> bool {
let disallowed_connector_ids = if is_first_party_chat_originator(originator_value) {
FIRST_PARTY_CHAT_DISALLOWED_CONNECTOR_IDS
} else {
DISALLOWED_CONNECTOR_IDS
};
!disallowed_connector_ids.contains(&connector_id)
}
pub fn sanitize_name(name: &str) -> String {
sanitize_slug(name).replace("-", "_")
}