mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
[codex] Gate plugin MCP servers by auth route (#27459)
## Context Some plugins expose both Apps and MCP servers. This PR moves auth-aware surface projection into `core-plugins::PluginsManager`, so callers get a consistent effective plugin view. Later PRs narrow the conflict rule and update listing/install paths. The high level goal of this PR is to set up the plumbing to conditionally filter App/MCP in the plugin manager layer. We start by removing MCP servers when using SIWC/Codex-backend auth, and removing Apps when using API-key-style auth. This PR is now stacked on #27652, which contains only the constructor plumbing for seeding `PluginsManager` with the current auth mode. ## Stack - PR1: #27652 seed plugin manager auth at construction. - PR2: #27459 route plugin surfaces by auth mode. - PR3: #27607 dedupe plugin MCP servers by App declaration name. - PR4: #27602 preserve plugin Apps in connector listings. - PR5: #27461 skip install-time plugin MCP OAuth for matching App routes. ## Summary - API-key/non-ChatGPT routes hide plugin Apps and keep plugin MCPs. - ChatGPT/SIWC with Apps enabled keeps plugin Apps and suppresses MCPs for dual-surface plugins. - MCP-only plugins stay available for ChatGPT/SIWC sessions. - Cached plugin load outcomes are re-projected when auth mode changes. ## Validation ```bash cargo test -p codex-core-plugins plugin_auth_projection cargo test -p codex-core list_tool_suggest_discoverable_plugins git diff --check ```
This commit is contained in:
committed by
GitHub
Unverified
parent
5c8136f48a
commit
5d7db08b61
@@ -207,9 +207,20 @@ fn featured_plugin_ids_cache_key(
|
||||
|
||||
fn project_plugin_load_outcome_for_auth(
|
||||
outcome: PluginLoadOutcome,
|
||||
_auth_mode: Option<AuthMode>,
|
||||
auth_mode: Option<AuthMode>,
|
||||
) -> PluginLoadOutcome {
|
||||
outcome
|
||||
let apps_route_available = auth_mode.is_some_and(AuthMode::uses_codex_backend);
|
||||
let mut plugins = outcome.plugins().to_vec();
|
||||
for plugin in &mut plugins {
|
||||
if apps_route_available {
|
||||
if plugin.is_active() && !plugin.apps.is_empty() {
|
||||
plugin.mcp_servers.clear();
|
||||
}
|
||||
} else {
|
||||
plugin.apps.clear();
|
||||
}
|
||||
}
|
||||
PluginLoadOutcome::from_plugins(plugins)
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, PartialEq, Eq)]
|
||||
|
||||
@@ -70,6 +70,187 @@ fn plugins_manager_tracks_auth_mode() {
|
||||
assert_eq!(manager_with_auth.auth_mode(), Some(AuthMode::Chatgpt));
|
||||
}
|
||||
|
||||
fn write_auth_projection_plugin(codex_home: &Path, name: &str, include_app: bool) {
|
||||
let plugin_root = codex_home
|
||||
.join("plugins/cache")
|
||||
.join("test")
|
||||
.join(name)
|
||||
.join("local");
|
||||
write_file(
|
||||
&plugin_root.join(".codex-plugin/plugin.json"),
|
||||
&format!(r#"{{"name":"{name}"}}"#),
|
||||
);
|
||||
write_file(
|
||||
&plugin_root.join(".mcp.json"),
|
||||
&format!(
|
||||
r#"{{
|
||||
"mcpServers": {{
|
||||
"{name}": {{
|
||||
"type": "stdio",
|
||||
"command": "{name}-mcp"
|
||||
}}
|
||||
}}
|
||||
}}"#
|
||||
),
|
||||
);
|
||||
if include_app {
|
||||
write_file(
|
||||
&plugin_root.join(".app.json"),
|
||||
&format!(r#"{{"apps":{{"{name}":{{"id":"connector_{name}"}}}}}}"#),
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
async fn auth_projection_config(codex_home: &Path) -> PluginsConfigInput {
|
||||
let config_toml = r#"[features]
|
||||
plugins = true
|
||||
|
||||
[plugins."sample@test"]
|
||||
enabled = true
|
||||
|
||||
[plugins."docs@test"]
|
||||
enabled = true
|
||||
"#
|
||||
.to_string();
|
||||
write_file(&codex_home.join(CONFIG_TOML_FILE), &config_toml);
|
||||
load_config(codex_home, codex_home).await
|
||||
}
|
||||
|
||||
fn sorted_effective_mcp_server_names(outcome: &PluginLoadOutcome) -> Vec<String> {
|
||||
let mut names = outcome
|
||||
.effective_mcp_servers()
|
||||
.keys()
|
||||
.cloned()
|
||||
.collect::<Vec<_>>();
|
||||
names.sort();
|
||||
names
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn plugin_auth_projection_hides_apps_without_chatgpt_auth() {
|
||||
let codex_home = TempDir::new().unwrap();
|
||||
write_auth_projection_plugin(codex_home.path(), "sample", /*include_app*/ true);
|
||||
write_auth_projection_plugin(codex_home.path(), "docs", /*include_app*/ false);
|
||||
let config = auth_projection_config(codex_home.path()).await;
|
||||
let manager = PluginsManager::new_with_options(
|
||||
codex_home.path().to_path_buf(),
|
||||
Some(Product::Codex),
|
||||
Some(AuthMode::ApiKey),
|
||||
);
|
||||
|
||||
let outcome = manager.plugins_for_config(&config).await;
|
||||
|
||||
assert!(outcome.effective_apps().is_empty());
|
||||
assert_eq!(
|
||||
sorted_effective_mcp_server_names(&outcome),
|
||||
vec!["docs".to_string(), "sample".to_string()]
|
||||
);
|
||||
let sample = outcome
|
||||
.capability_summaries()
|
||||
.iter()
|
||||
.find(|plugin| plugin.config_name == "sample@test")
|
||||
.expect("sample plugin summary should exist");
|
||||
assert_eq!(sample.mcp_server_names, vec!["sample".to_string()]);
|
||||
assert!(sample.app_connector_ids.is_empty());
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn plugin_auth_projection_hides_dual_surface_mcp_with_chatgpt_apps_route() {
|
||||
let codex_home = TempDir::new().unwrap();
|
||||
write_auth_projection_plugin(codex_home.path(), "sample", /*include_app*/ true);
|
||||
write_auth_projection_plugin(codex_home.path(), "docs", /*include_app*/ false);
|
||||
let config = auth_projection_config(codex_home.path()).await;
|
||||
let manager = PluginsManager::new_with_options(
|
||||
codex_home.path().to_path_buf(),
|
||||
Some(Product::Codex),
|
||||
Some(AuthMode::Chatgpt),
|
||||
);
|
||||
|
||||
let outcome = manager.plugins_for_config(&config).await;
|
||||
|
||||
assert_eq!(
|
||||
outcome.effective_apps(),
|
||||
vec![AppConnectorId("connector_sample".to_string())]
|
||||
);
|
||||
assert_eq!(
|
||||
sorted_effective_mcp_server_names(&outcome),
|
||||
vec!["docs".to_string()]
|
||||
);
|
||||
let sample = outcome
|
||||
.capability_summaries()
|
||||
.iter()
|
||||
.find(|plugin| plugin.config_name == "sample@test")
|
||||
.expect("sample plugin summary should exist");
|
||||
assert!(sample.mcp_server_names.is_empty());
|
||||
assert_eq!(
|
||||
sample.app_connector_ids,
|
||||
vec![AppConnectorId("connector_sample".to_string())]
|
||||
);
|
||||
let docs = outcome
|
||||
.capability_summaries()
|
||||
.iter()
|
||||
.find(|plugin| plugin.config_name == "docs@test")
|
||||
.expect("docs plugin summary should exist");
|
||||
assert_eq!(docs.mcp_server_names, vec!["docs".to_string()]);
|
||||
assert!(docs.app_connector_ids.is_empty());
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn plugin_auth_projection_hides_dual_surface_mcp_with_agent_identity_apps_route() {
|
||||
let codex_home = TempDir::new().unwrap();
|
||||
write_auth_projection_plugin(codex_home.path(), "sample", /*include_app*/ true);
|
||||
write_auth_projection_plugin(codex_home.path(), "docs", /*include_app*/ false);
|
||||
let config = auth_projection_config(codex_home.path()).await;
|
||||
let manager = PluginsManager::new_with_options(
|
||||
codex_home.path().to_path_buf(),
|
||||
Some(Product::Codex),
|
||||
Some(AuthMode::AgentIdentity),
|
||||
);
|
||||
|
||||
let outcome = manager.plugins_for_config(&config).await;
|
||||
|
||||
assert_eq!(
|
||||
outcome.effective_apps(),
|
||||
vec![AppConnectorId("connector_sample".to_string())]
|
||||
);
|
||||
assert_eq!(
|
||||
sorted_effective_mcp_server_names(&outcome),
|
||||
vec!["docs".to_string()]
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn plugin_auth_projection_reprojects_cached_outcome_when_auth_changes() {
|
||||
let codex_home = TempDir::new().unwrap();
|
||||
write_auth_projection_plugin(codex_home.path(), "sample", /*include_app*/ true);
|
||||
write_auth_projection_plugin(codex_home.path(), "docs", /*include_app*/ false);
|
||||
let config = auth_projection_config(codex_home.path()).await;
|
||||
let manager = PluginsManager::new_with_options(
|
||||
codex_home.path().to_path_buf(),
|
||||
Some(Product::Codex),
|
||||
Some(AuthMode::Chatgpt),
|
||||
);
|
||||
|
||||
let chatgpt_outcome = manager.plugins_for_config(&config).await;
|
||||
assert_eq!(
|
||||
sorted_effective_mcp_server_names(&chatgpt_outcome),
|
||||
vec!["docs".to_string()]
|
||||
);
|
||||
assert_eq!(
|
||||
chatgpt_outcome.effective_apps(),
|
||||
vec![AppConnectorId("connector_sample".to_string())]
|
||||
);
|
||||
|
||||
assert!(manager.set_auth_mode(Some(AuthMode::ApiKey)));
|
||||
let api_key_outcome = manager.plugins_for_config(&config).await;
|
||||
|
||||
assert_eq!(
|
||||
sorted_effective_mcp_server_names(&api_key_outcome),
|
||||
vec!["docs".to_string(), "sample".to_string()]
|
||||
);
|
||||
assert!(api_key_outcome.effective_apps().is_empty());
|
||||
}
|
||||
|
||||
fn write_plugin_with_version(
|
||||
root: &Path,
|
||||
dir_name: &str,
|
||||
@@ -145,10 +326,14 @@ fn plugin_config_toml(enabled: bool, plugins_feature_enabled: bool) -> String {
|
||||
toml::to_string(&Value::Table(root)).expect("plugin test config should serialize")
|
||||
}
|
||||
|
||||
async fn load_plugins_from_config(config_toml: &str, codex_home: &Path) -> PluginLoadOutcome {
|
||||
async fn load_plugins_from_config(
|
||||
config_toml: &str,
|
||||
codex_home: &Path,
|
||||
auth_mode: Option<AuthMode>,
|
||||
) -> PluginLoadOutcome {
|
||||
write_file(&codex_home.join(CONFIG_TOML_FILE), config_toml);
|
||||
let config = load_config(codex_home, codex_home).await;
|
||||
PluginsManager::new(codex_home.to_path_buf())
|
||||
PluginsManager::new_with_options(codex_home.to_path_buf(), Some(Product::Codex), auth_mode)
|
||||
.plugins_for_config(&config)
|
||||
.await
|
||||
}
|
||||
@@ -242,6 +427,7 @@ async fn load_plugins_loads_default_skills_and_mcp_servers() {
|
||||
let outcome = load_plugins_from_config(
|
||||
&plugin_config_toml(/*enabled*/ true, /*plugins_feature_enabled*/ true),
|
||||
codex_home.path(),
|
||||
/*auth_mode*/ None,
|
||||
)
|
||||
.await;
|
||||
|
||||
@@ -285,7 +471,7 @@ async fn load_plugins_loads_default_skills_and_mcp_servers() {
|
||||
tools: HashMap::new(),
|
||||
},
|
||||
)]),
|
||||
apps: vec![AppConnectorId("connector_example".to_string())],
|
||||
apps: Vec::new(),
|
||||
hook_sources: Vec::new(),
|
||||
hook_load_warnings: Vec::new(),
|
||||
error: None,
|
||||
@@ -299,7 +485,7 @@ async fn load_plugins_loads_default_skills_and_mcp_servers() {
|
||||
description: Some("Plugin that includes the sample MCP server and Skills".to_string(),),
|
||||
has_skills: true,
|
||||
mcp_server_names: vec!["sample".to_string()],
|
||||
app_connector_ids: vec![AppConnectorId("connector_example".to_string())],
|
||||
app_connector_ids: Vec::new(),
|
||||
}]
|
||||
);
|
||||
assert_eq!(
|
||||
@@ -307,10 +493,7 @@ async fn load_plugins_loads_default_skills_and_mcp_servers() {
|
||||
vec![plugin_root.join("skills").abs()]
|
||||
);
|
||||
assert_eq!(outcome.effective_mcp_servers().len(), 1);
|
||||
assert_eq!(
|
||||
outcome.effective_apps(),
|
||||
vec![AppConnectorId("connector_example".to_string())]
|
||||
);
|
||||
assert!(outcome.effective_apps().is_empty());
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
@@ -360,7 +543,8 @@ disabled_tools = ["delete"]
|
||||
approval_mode = "approve"
|
||||
"#;
|
||||
|
||||
let outcome = load_plugins_from_config(config_toml, codex_home.path()).await;
|
||||
let outcome =
|
||||
load_plugins_from_config(config_toml, codex_home.path(), /*auth_mode*/ None).await;
|
||||
let server = outcome.plugins()[0]
|
||||
.mcp_servers
|
||||
.get("sample")
|
||||
@@ -620,7 +804,8 @@ enabled = false
|
||||
[plugins."sample@test"]
|
||||
enabled = true
|
||||
"#;
|
||||
let outcome = load_plugins_from_config(config_toml, codex_home.path()).await;
|
||||
let outcome =
|
||||
load_plugins_from_config(config_toml, codex_home.path(), /*auth_mode*/ None).await;
|
||||
let skill_path = std::fs::canonicalize(skill_path)
|
||||
.expect("skill path should canonicalize")
|
||||
.abs();
|
||||
@@ -660,7 +845,8 @@ enabled = false
|
||||
[plugins."sample@test"]
|
||||
enabled = true
|
||||
"#;
|
||||
let outcome = load_plugins_from_config(config_toml, codex_home.path()).await;
|
||||
let outcome =
|
||||
load_plugins_from_config(config_toml, codex_home.path(), /*auth_mode*/ None).await;
|
||||
|
||||
assert!(outcome.plugins()[0].disabled_skill_paths.is_empty());
|
||||
assert!(outcome.plugins()[0].has_enabled_skills);
|
||||
@@ -745,6 +931,7 @@ async fn capability_summary_sanitizes_plugin_descriptions_to_one_line() {
|
||||
let outcome = load_plugins_from_config(
|
||||
&plugin_config_toml(/*enabled*/ true, /*plugins_feature_enabled*/ true),
|
||||
codex_home.path(),
|
||||
/*auth_mode*/ None,
|
||||
)
|
||||
.await;
|
||||
|
||||
@@ -784,6 +971,7 @@ async fn capability_summary_truncates_overlong_plugin_descriptions() {
|
||||
let outcome = load_plugins_from_config(
|
||||
&plugin_config_toml(/*enabled*/ true, /*plugins_feature_enabled*/ true),
|
||||
codex_home.path(),
|
||||
/*auth_mode*/ None,
|
||||
)
|
||||
.await;
|
||||
|
||||
@@ -868,6 +1056,7 @@ async fn load_plugins_uses_manifest_configured_component_paths() {
|
||||
let outcome = load_plugins_from_config(
|
||||
&plugin_config_toml(/*enabled*/ true, /*plugins_feature_enabled*/ true),
|
||||
codex_home.path(),
|
||||
/*auth_mode*/ None,
|
||||
)
|
||||
.await;
|
||||
|
||||
@@ -906,10 +1095,7 @@ async fn load_plugins_uses_manifest_configured_component_paths() {
|
||||
},
|
||||
)])
|
||||
);
|
||||
assert_eq!(
|
||||
outcome.plugins()[0].apps,
|
||||
vec![AppConnectorId("connector_custom".to_string())]
|
||||
);
|
||||
assert!(outcome.plugins()[0].apps.is_empty());
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
@@ -983,6 +1169,7 @@ async fn load_plugins_ignores_manifest_component_paths_without_dot_slash() {
|
||||
let outcome = load_plugins_from_config(
|
||||
&plugin_config_toml(/*enabled*/ true, /*plugins_feature_enabled*/ true),
|
||||
codex_home.path(),
|
||||
/*auth_mode*/ None,
|
||||
)
|
||||
.await;
|
||||
|
||||
@@ -1018,10 +1205,7 @@ async fn load_plugins_ignores_manifest_component_paths_without_dot_slash() {
|
||||
},
|
||||
)])
|
||||
);
|
||||
assert_eq!(
|
||||
outcome.plugins()[0].apps,
|
||||
vec![AppConnectorId("connector_default".to_string())]
|
||||
);
|
||||
assert!(outcome.plugins()[0].apps.is_empty());
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
@@ -1051,6 +1235,7 @@ async fn load_plugins_ignores_invalid_manifest_skills_shape() {
|
||||
let outcome = load_plugins_from_config(
|
||||
&plugin_config_toml(/*enabled*/ true, /*plugins_feature_enabled*/ true),
|
||||
codex_home.path(),
|
||||
/*auth_mode*/ None,
|
||||
)
|
||||
.await;
|
||||
|
||||
@@ -1090,6 +1275,7 @@ async fn load_plugins_preserves_disabled_plugins_without_effective_contributions
|
||||
/*enabled*/ false, /*plugins_feature_enabled*/ true,
|
||||
),
|
||||
codex_home.path(),
|
||||
/*auth_mode*/ None,
|
||||
)
|
||||
.await;
|
||||
|
||||
@@ -1162,6 +1348,7 @@ async fn effective_apps_dedupes_connector_ids_across_plugins() {
|
||||
let mut root = toml::map::Map::new();
|
||||
let mut features = toml::map::Map::new();
|
||||
features.insert("plugins".to_string(), Value::Boolean(true));
|
||||
features.insert("apps".to_string(), Value::Boolean(true));
|
||||
root.insert("features".to_string(), Value::Table(features));
|
||||
|
||||
let mut plugins = toml::map::Map::new();
|
||||
@@ -1178,7 +1365,8 @@ async fn effective_apps_dedupes_connector_ids_across_plugins() {
|
||||
let config_toml =
|
||||
toml::to_string(&Value::Table(root)).expect("plugin test config should serialize");
|
||||
|
||||
let outcome = load_plugins_from_config(&config_toml, codex_home.path()).await;
|
||||
let outcome =
|
||||
load_plugins_from_config(&config_toml, codex_home.path(), Some(AuthMode::Chatgpt)).await;
|
||||
|
||||
assert_eq!(
|
||||
outcome.effective_apps(),
|
||||
@@ -1221,6 +1409,7 @@ async fn effective_apps_preserves_app_config_order() {
|
||||
let outcome = load_plugins_from_config(
|
||||
&plugin_config_toml(/*enabled*/ true, /*plugins_feature_enabled*/ true),
|
||||
codex_home.path(),
|
||||
Some(AuthMode::Chatgpt),
|
||||
)
|
||||
.await;
|
||||
|
||||
@@ -1484,6 +1673,7 @@ async fn load_plugins_rejects_invalid_plugin_keys() {
|
||||
let outcome = load_plugins_from_config(
|
||||
&toml::to_string(&Value::Table(root)).expect("plugin test config should serialize"),
|
||||
codex_home.path(),
|
||||
/*auth_mode*/ None,
|
||||
)
|
||||
.await;
|
||||
|
||||
|
||||
Reference in New Issue
Block a user