mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
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`
This commit is contained in:
committed by
GitHub
Unverified
parent
aee1fe2659
commit
a8db4af5c3
@@ -1015,13 +1015,11 @@ impl PluginRequestProcessor {
|
||||
) -> Result<PluginUninstallResponse, JSONRPCErrorError> {
|
||||
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],
|
||||
|
||||
@@ -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(())
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user