From a8db4af5c370d4095c09815d24b90082551be96f Mon Sep 17 00:00:00 2001 From: xli-oai Date: Mon, 4 May 2026 16:28:13 -0700 Subject: [PATCH] Remove remote plugin uninstall prefix gate (#20722) ## Summary Remove the hardcoded remote plugin ID prefix allow-list from app-server uninstall routing. IDs that do not parse as local `plugin@marketplace` IDs now flow through the remote uninstall path, where the existing remote ID safety validation still rejects empty IDs, spaces, slashes, and other unsafe characters before URL/cache use. ## Why Plugin-service owns the backend remote plugin ID contract. Codex should not require remote IDs to start with the local hardcoded prefixes `plugins~`, `plugins_`, `app_`, `asdk_app_`, or `connector_`, because newer backend ID families could otherwise be rejected before plugin-service sees the request. ## Validation - `just fmt` - `cargo test -p codex-app-server plugin_uninstall` - `just fix -p codex-app-server` - `git diff --check` --- .../src/request_processors/plugins.rs | 17 +++-------------- .../tests/suite/v2/plugin_uninstall.rs | 8 ++++---- 2 files changed, 7 insertions(+), 18 deletions(-) diff --git a/codex-rs/app-server/src/request_processors/plugins.rs b/codex-rs/app-server/src/request_processors/plugins.rs index c4087bbf9..068dc04ca 100644 --- a/codex-rs/app-server/src/request_processors/plugins.rs +++ b/codex-rs/app-server/src/request_processors/plugins.rs @@ -1015,13 +1015,11 @@ impl PluginRequestProcessor { ) -> Result { let PluginUninstallParams { plugin_id } = params; if codex_plugin::PluginId::parse(&plugin_id).is_err() - && !is_valid_remote_uninstall_plugin_id(&plugin_id) + && !is_valid_remote_plugin_id(&plugin_id) { - return Err(invalid_request( - "invalid plugin id: expected a local plugin id in the form `plugin@marketplace` or a remote plugin id starting with `plugins~`, `plugins_`, `app_`, `asdk_app_`, or `connector_`", - )); + return Err(invalid_request("invalid remote plugin id")); } - if is_valid_remote_uninstall_plugin_id(&plugin_id) { + if is_valid_remote_plugin_id(&plugin_id) { return self.remote_plugin_uninstall_response(plugin_id).await; } let plugins_manager = self.thread_manager.plugins_manager(); @@ -1148,15 +1146,6 @@ impl PluginRequestProcessor { } } -fn is_valid_remote_uninstall_plugin_id(plugin_name: &str) -> bool { - is_valid_remote_plugin_id(plugin_name) - && (plugin_name.starts_with("plugins~") - || plugin_name.starts_with("plugins_") - || plugin_name.starts_with("app_") - || plugin_name.starts_with("asdk_app_") - || plugin_name.starts_with("connector_")) -} - async fn load_plugin_app_summaries( config: &Config, plugin_apps: &[codex_plugin::AppConnectorId], diff --git a/codex-rs/app-server/tests/suite/v2/plugin_uninstall.rs b/codex-rs/app-server/tests/suite/v2/plugin_uninstall.rs index 26d1e2f88..c47a6aac9 100644 --- a/codex-rs/app-server/tests/suite/v2/plugin_uninstall.rs +++ b/codex-rs/app-server/tests/suite/v2/plugin_uninstall.rs @@ -454,7 +454,7 @@ async fn plugin_uninstall_rejects_before_post_when_remote_detail_fetch_fails() - } #[tokio::test] -async fn plugin_uninstall_rejects_invalid_plugin_id_before_remote_path() -> Result<()> { +async fn plugin_uninstall_rejects_remote_plugin_id_with_spaces_before_network_call() -> Result<()> { let codex_home = TempDir::new()?; let server = MockServer::start().await; write_remote_plugin_catalog_config( @@ -477,7 +477,7 @@ async fn plugin_uninstall_rejects_invalid_plugin_id_before_remote_path() -> Resu .await??; assert_eq!(err.error.code, -32600); - assert!(err.error.message.contains("invalid plugin id")); + assert!(err.error.message.contains("invalid remote plugin id")); wait_for_remote_plugin_request_count( &server, "POST", @@ -512,7 +512,7 @@ async fn plugin_uninstall_rejects_invalid_remote_plugin_id_before_network_call() .await??; assert_eq!(err.error.code, -32600); - assert!(err.error.message.contains("invalid plugin id")); + assert!(err.error.message.contains("invalid remote plugin id")); wait_for_remote_plugin_request_count( &server, "POST", @@ -546,7 +546,7 @@ async fn plugin_uninstall_rejects_empty_remote_plugin_id() -> Result<()> { .await??; assert_eq!(err.error.code, -32600); - assert!(err.error.message.contains("invalid plugin id")); + assert!(err.error.message.contains("invalid remote plugin id")); Ok(()) }