mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
Use server app auth requirements for remote plugin install (#27085)
## Summary - request `includeAppsNeedingAuth=true` when installing remote plugins - return backend-provided `app_ids_needing_auth` from the remote install client - use those app IDs to populate `appsNeedingAuth` without refetching accessible apps, with fallback for older responses ## Testing - `just fmt` - `just test -p codex-app-server` - `just test -p codex-core-plugins` - real app-server install/uninstall check with Notion remote plugin - subagent review found no blocking issues
This commit is contained in:
committed by
GitHub
Unverified
parent
dffc4bf75d
commit
14660c22d1
@@ -1519,7 +1519,7 @@ impl PluginRequestProcessor {
|
||||
// Cache first so a backend install cannot succeed when local materialization fails.
|
||||
// If this backend call fails, the cache entry is harmless because remote installed state
|
||||
// is still backend-gated.
|
||||
codex_core_plugins::remote::install_remote_plugin(
|
||||
let install_result = codex_core_plugins::remote::install_remote_plugin(
|
||||
&remote_plugin_service_config,
|
||||
auth.as_ref(),
|
||||
&actual_remote_marketplace_name,
|
||||
@@ -1538,7 +1538,7 @@ impl PluginRequestProcessor {
|
||||
|
||||
let mut plugin_metadata =
|
||||
plugin_telemetry_metadata_from_root(&result.plugin_id, &result.installed_path).await;
|
||||
plugin_metadata.remote_plugin_id = Some(remote_plugin_id);
|
||||
plugin_metadata.remote_plugin_id = Some(remote_plugin_id.clone());
|
||||
self.analytics_events_client
|
||||
.track_plugin_installed(plugin_metadata);
|
||||
|
||||
@@ -1548,15 +1548,42 @@ impl PluginRequestProcessor {
|
||||
.await;
|
||||
}
|
||||
|
||||
let plugin_apps = load_plugin_apps(result.installed_path.as_path()).await;
|
||||
let apps_needing_auth = self
|
||||
.plugin_apps_needing_auth_for_install(
|
||||
&config,
|
||||
auth.as_ref().is_some_and(CodexAuth::is_chatgpt_auth),
|
||||
&result.plugin_id.as_key(),
|
||||
&plugin_apps,
|
||||
)
|
||||
.await;
|
||||
let is_chatgpt_auth = auth.as_ref().is_some_and(CodexAuth::is_chatgpt_auth);
|
||||
let apps_needing_auth =
|
||||
if let Some(app_ids_needing_auth) = install_result.app_ids_needing_auth {
|
||||
if app_ids_needing_auth.is_empty()
|
||||
|| !config.features.apps_enabled_for_auth(is_chatgpt_auth)
|
||||
{
|
||||
Vec::new()
|
||||
} else {
|
||||
let plugin_apps = app_ids_needing_auth
|
||||
.into_iter()
|
||||
.map(codex_plugin::AppConnectorId)
|
||||
.collect::<Vec<_>>();
|
||||
let all_connectors = connectors::list_cached_all_connectors(&config)
|
||||
.await
|
||||
.unwrap_or_default();
|
||||
connectors::connectors_for_plugin_apps(all_connectors, &plugin_apps)
|
||||
.into_iter()
|
||||
.map(|connector| AppSummary {
|
||||
id: connector.id,
|
||||
name: connector.name,
|
||||
description: connector.description,
|
||||
install_url: connector.install_url,
|
||||
needs_auth: true,
|
||||
})
|
||||
.collect()
|
||||
}
|
||||
} else {
|
||||
let plugin_apps = load_plugin_apps(result.installed_path.as_path()).await;
|
||||
self.plugin_apps_needing_auth_for_install(
|
||||
&config,
|
||||
is_chatgpt_auth,
|
||||
&result.plugin_id.as_key(),
|
||||
&plugin_apps,
|
||||
)
|
||||
.await
|
||||
};
|
||||
|
||||
Ok(PluginInstallResponse {
|
||||
auth_policy: remote_detail.summary.auth_policy,
|
||||
|
||||
@@ -288,6 +288,59 @@ async fn plugin_install_writes_remote_plugin_to_cloud_and_cache() -> Result<()>
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn plugin_install_uses_remote_apps_needing_auth_response() -> Result<()> {
|
||||
let codex_home = TempDir::new()?;
|
||||
let server = MockServer::start().await;
|
||||
let bundle_url = mount_remote_plugin_bundle(
|
||||
&server,
|
||||
/*status_code*/ 200,
|
||||
remote_plugin_bundle_tar_gz_bytes("linear")?,
|
||||
)
|
||||
.await;
|
||||
configure_remote_plugin_with_apps_test(codex_home.path(), &server)?;
|
||||
mount_remote_plugin_detail(&server, REMOTE_PLUGIN_ID, "1.2.3", Some(&bundle_url)).await;
|
||||
mount_empty_remote_installed_plugins(&server).await;
|
||||
mount_remote_plugin_install_with_apps_needing_auth(&server, REMOTE_PLUGIN_ID, &["alpha"]).await;
|
||||
|
||||
let mut mcp = TestAppServer::new_with_env(
|
||||
codex_home.path(),
|
||||
&[(TEST_ALLOW_HTTP_REMOTE_PLUGIN_BUNDLE_DOWNLOADS, Some("1"))],
|
||||
)
|
||||
.await?;
|
||||
timeout(DEFAULT_TIMEOUT, mcp.initialize()).await??;
|
||||
|
||||
let request_id = send_remote_plugin_install_request(&mut mcp, REMOTE_PLUGIN_ID).await?;
|
||||
let response: JSONRPCResponse = timeout(
|
||||
DEFAULT_TIMEOUT,
|
||||
mcp.read_stream_until_response_message(RequestId::Integer(request_id)),
|
||||
)
|
||||
.await??;
|
||||
let response: PluginInstallResponse = to_response(response)?;
|
||||
|
||||
assert_eq!(
|
||||
response,
|
||||
PluginInstallResponse {
|
||||
auth_policy: PluginAuthPolicy::OnUse,
|
||||
apps_needing_auth: vec![AppSummary {
|
||||
id: "alpha".to_string(),
|
||||
name: "alpha".to_string(),
|
||||
description: None,
|
||||
install_url: Some("https://chatgpt.com/apps/alpha/alpha".to_string()),
|
||||
needs_auth: true,
|
||||
}],
|
||||
}
|
||||
);
|
||||
wait_for_remote_plugin_request_count(
|
||||
&server,
|
||||
"GET",
|
||||
"/backend-api/connectors/directory/list",
|
||||
/*expected_count*/ 0,
|
||||
)
|
||||
.await?;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn plugin_install_rejects_missing_remote_bundle_url() -> Result<()> {
|
||||
let codex_home = TempDir::new()?;
|
||||
@@ -1398,6 +1451,34 @@ fn configure_remote_plugin_test(codex_home: &std::path::Path, server: &MockServe
|
||||
)
|
||||
}
|
||||
|
||||
fn configure_remote_plugin_with_apps_test(
|
||||
codex_home: &std::path::Path,
|
||||
server: &MockServer,
|
||||
) -> Result<()> {
|
||||
std::fs::write(
|
||||
codex_home.join("config.toml"),
|
||||
format!(
|
||||
r#"
|
||||
chatgpt_base_url = "{}/backend-api/"
|
||||
|
||||
[features]
|
||||
plugins = true
|
||||
remote_plugin = true
|
||||
connectors = true
|
||||
"#,
|
||||
server.uri()
|
||||
),
|
||||
)?;
|
||||
write_chatgpt_auth(
|
||||
codex_home,
|
||||
ChatGptAuthFixture::new("chatgpt-token")
|
||||
.account_id("account-123")
|
||||
.chatgpt_user_id("user-123")
|
||||
.chatgpt_account_id("account-123"),
|
||||
AuthCredentialsStoreMode::File,
|
||||
)
|
||||
}
|
||||
|
||||
async fn mount_remote_plugin_bundle(
|
||||
server: &MockServer,
|
||||
status_code: u16,
|
||||
@@ -1552,6 +1633,27 @@ async fn mount_remote_plugin_install(server: &MockServer, remote_plugin_id: &str
|
||||
.await;
|
||||
}
|
||||
|
||||
async fn mount_remote_plugin_install_with_apps_needing_auth(
|
||||
server: &MockServer,
|
||||
remote_plugin_id: &str,
|
||||
app_ids_needing_auth: &[&str],
|
||||
) {
|
||||
Mock::given(method("POST"))
|
||||
.and(path(format!(
|
||||
"/backend-api/ps/plugins/{remote_plugin_id}/install"
|
||||
)))
|
||||
.and(query_param("includeAppsNeedingAuth", "true"))
|
||||
.and(header("authorization", "Bearer chatgpt-token"))
|
||||
.and(header("chatgpt-account-id", "account-123"))
|
||||
.respond_with(ResponseTemplate::new(200).set_body_json(json!({
|
||||
"id": remote_plugin_id,
|
||||
"enabled": true,
|
||||
"app_ids_needing_auth": app_ids_needing_auth,
|
||||
})))
|
||||
.mount(server)
|
||||
.await;
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone)]
|
||||
struct CacheManifestExists {
|
||||
manifest_path: std::path::PathBuf,
|
||||
|
||||
@@ -550,6 +550,12 @@ struct RemotePluginInstalledResponse {
|
||||
struct RemotePluginMutationResponse {
|
||||
id: String,
|
||||
enabled: bool,
|
||||
app_ids_needing_auth: Option<Vec<String>>,
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, PartialEq, Eq)]
|
||||
pub struct RemotePluginInstallResult {
|
||||
pub app_ids_needing_auth: Option<Vec<String>>,
|
||||
}
|
||||
|
||||
pub async fn fetch_remote_marketplaces(
|
||||
@@ -1071,7 +1077,7 @@ pub async fn install_remote_plugin(
|
||||
auth: Option<&CodexAuth>,
|
||||
_marketplace_name: &str,
|
||||
plugin_id: &str,
|
||||
) -> Result<(), RemotePluginCatalogError> {
|
||||
) -> Result<RemotePluginInstallResult, RemotePluginCatalogError> {
|
||||
let auth = ensure_chatgpt_auth(auth)?;
|
||||
// Remote plugin IDs uniquely identify remote plugins, so the caller-provided
|
||||
// marketplace name is not validated before sending the install mutation.
|
||||
@@ -1079,7 +1085,12 @@ pub async fn install_remote_plugin(
|
||||
let base_url = config.chatgpt_base_url.trim_end_matches('/');
|
||||
let url = format!("{base_url}/ps/plugins/{plugin_id}/install");
|
||||
let client = build_reqwest_client();
|
||||
let request = authenticated_request(client.post(&url), auth)?;
|
||||
let request = authenticated_request(
|
||||
client
|
||||
.post(&url)
|
||||
.query(&[("includeAppsNeedingAuth", "true")]),
|
||||
auth,
|
||||
)?;
|
||||
let response: RemotePluginMutationResponse = send_and_decode(request, &url).await?;
|
||||
if response.id != plugin_id {
|
||||
return Err(RemotePluginCatalogError::UnexpectedPluginId {
|
||||
@@ -1095,7 +1106,9 @@ pub async fn install_remote_plugin(
|
||||
});
|
||||
}
|
||||
|
||||
Ok(())
|
||||
Ok(RemotePluginInstallResult {
|
||||
app_ids_needing_auth: response.app_ids_needing_auth,
|
||||
})
|
||||
}
|
||||
|
||||
pub async fn uninstall_remote_plugin(
|
||||
|
||||
Reference in New Issue
Block a user