mirror of
https://github.com/openai/codex.git
synced 2026-11-10 21:00:47 +00:00
[codex] preserve WIF isolation and local plugin flows [ci changed_files]
This commit is contained in:
@@ -207,7 +207,7 @@ pub(crate) struct MessageProcessor {
|
||||
thread_processor: ThreadRequestProcessor,
|
||||
turn_processor: TurnRequestProcessor,
|
||||
windows_sandbox_processor: WindowsSandboxRequestProcessor,
|
||||
owns_external_auth_refresh_bridge: bool,
|
||||
external_auth_refresh_bridge: Option<Arc<dyn ExternalAuth>>,
|
||||
request_serialization_queues: RequestSerializationQueues,
|
||||
}
|
||||
|
||||
@@ -321,10 +321,13 @@ impl MessageProcessor {
|
||||
remote_control_handle,
|
||||
plugin_startup_tasks,
|
||||
} = args;
|
||||
let owns_external_auth_refresh_bridge =
|
||||
auth_manager.set_external_auth_if_absent(Arc::new(ExternalAuthRefreshBridge {
|
||||
let external_auth_refresh_bridge: Arc<dyn ExternalAuth> =
|
||||
Arc::new(ExternalAuthRefreshBridge {
|
||||
outgoing: outgoing.clone(),
|
||||
}));
|
||||
});
|
||||
let external_auth_refresh_bridge = auth_manager
|
||||
.replace_non_required_external_auth(Arc::clone(&external_auth_refresh_bridge))
|
||||
.then_some(external_auth_refresh_bridge);
|
||||
let thread_state_manager = ThreadStateManager::new();
|
||||
// The thread store is intentionally process-scoped. Config reloads can
|
||||
// affect per-thread behavior, but they must not move newly started,
|
||||
@@ -567,14 +570,15 @@ impl MessageProcessor {
|
||||
thread_processor,
|
||||
turn_processor,
|
||||
windows_sandbox_processor,
|
||||
owns_external_auth_refresh_bridge,
|
||||
external_auth_refresh_bridge,
|
||||
request_serialization_queues: RequestSerializationQueues::default(),
|
||||
}
|
||||
}
|
||||
|
||||
pub(crate) fn clear_runtime_references(&self) {
|
||||
if self.owns_external_auth_refresh_bridge {
|
||||
self.account_processor.clear_external_auth();
|
||||
if let Some(external_auth_refresh_bridge) = &self.external_auth_refresh_bridge {
|
||||
self.account_processor
|
||||
.clear_external_auth_if(external_auth_refresh_bridge);
|
||||
}
|
||||
self.apps_processor.shutdown();
|
||||
self.models_refresh_worker.shutdown();
|
||||
|
||||
@@ -363,6 +363,7 @@ use codex_login::AuthManager;
|
||||
use codex_login::CodexAuth;
|
||||
use codex_login::ServerOptions as LoginServerOptions;
|
||||
use codex_login::ShutdownHandle;
|
||||
use codex_login::auth::ExternalAuth;
|
||||
use codex_login::auth::login_with_chatgpt_auth_tokens;
|
||||
use codex_login::complete_device_code_login;
|
||||
use codex_login::login_with_api_key;
|
||||
|
||||
@@ -158,11 +158,12 @@ impl AccountRequestProcessor {
|
||||
}
|
||||
}
|
||||
|
||||
pub(crate) fn clear_external_auth(&self) {
|
||||
self.auth_manager.clear_external_auth();
|
||||
self.thread_manager
|
||||
.plugins_manager()
|
||||
.set_auth_mode(self.auth_manager.get_api_auth_mode());
|
||||
pub(crate) fn clear_external_auth_if(&self, expected: &Arc<dyn ExternalAuth>) {
|
||||
if self.auth_manager.clear_external_auth_if(expected) {
|
||||
self.thread_manager
|
||||
.plugins_manager()
|
||||
.set_auth_mode(self.auth_manager.get_api_auth_mode());
|
||||
}
|
||||
}
|
||||
|
||||
fn current_account_updated_notification(&self) -> AccountUpdatedNotification {
|
||||
|
||||
@@ -809,11 +809,16 @@ impl PluginRequestProcessor {
|
||||
if !config.features.enabled(Feature::Plugins) {
|
||||
return Ok(empty_response());
|
||||
}
|
||||
let auth = self
|
||||
.auth_manager
|
||||
.auth()
|
||||
.await
|
||||
.map_err(|err| internal_error(format!("failed to resolve auth: {err}")))?;
|
||||
let requires_resolved_auth = config.features.enabled(Feature::RemotePlugin)
|
||||
|| config.features.enabled(Feature::PluginSharing);
|
||||
let auth = if requires_resolved_auth {
|
||||
self.auth_manager
|
||||
.auth()
|
||||
.await
|
||||
.map_err(|err| internal_error(format!("failed to resolve auth: {err}")))?
|
||||
} else {
|
||||
self.auth_manager.auth_cached()
|
||||
};
|
||||
if !self
|
||||
.workspace_codex_plugins_enabled(&config, auth.as_ref())
|
||||
.await
|
||||
|
||||
@@ -77,6 +77,7 @@ fn write_plugins_enabled_config_with_workload_identity(
|
||||
codex_home: &std::path::Path,
|
||||
server: &MockServer,
|
||||
token_path: &std::path::Path,
|
||||
extra_config: &str,
|
||||
) -> std::io::Result<()> {
|
||||
let token_path = toml::Value::String(token_path.to_string_lossy().into_owned());
|
||||
let chatgpt_base_url = toml::Value::String(format!("{}/backend-api", server.uri()));
|
||||
@@ -99,6 +100,8 @@ token_url = {token_url}
|
||||
[workload_identity.credential_source]
|
||||
type = "file"
|
||||
path = {token_path}
|
||||
|
||||
{extra_config}
|
||||
"#,
|
||||
),
|
||||
)
|
||||
@@ -183,7 +186,12 @@ async fn plugin_list_local_succeeds_when_workload_identity_becomes_unavailable()
|
||||
let server = MockServer::start().await;
|
||||
let token_path = codex_home.path().join("projected-workload-token");
|
||||
std::fs::write(&token_path, "external-subject-token\n")?;
|
||||
write_plugins_enabled_config_with_workload_identity(codex_home.path(), &server, &token_path)?;
|
||||
write_plugins_enabled_config_with_workload_identity(
|
||||
codex_home.path(),
|
||||
&server,
|
||||
&token_path,
|
||||
"",
|
||||
)?;
|
||||
|
||||
let access_token = workload_identity_access_token();
|
||||
Mock::given(method("POST"))
|
||||
@@ -238,6 +246,83 @@ async fn plugin_list_local_succeeds_when_workload_identity_becomes_unavailable()
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn plugin_installed_local_succeeds_when_workload_identity_becomes_unavailable() -> Result<()>
|
||||
{
|
||||
let codex_home = TempDir::new()?;
|
||||
let server = MockServer::start().await;
|
||||
let token_path = codex_home.path().join("projected-workload-token");
|
||||
std::fs::write(&token_path, "external-subject-token\n")?;
|
||||
write_openai_curated_marketplace(codex_home.path(), &["linear"])?;
|
||||
write_installed_plugin(&codex_home, "openai-curated", "linear")?;
|
||||
write_plugins_enabled_config_with_workload_identity(
|
||||
codex_home.path(),
|
||||
&server,
|
||||
&token_path,
|
||||
r#"[plugins."linear@openai-curated"]
|
||||
enabled = true"#,
|
||||
)?;
|
||||
|
||||
let access_token = workload_identity_access_token();
|
||||
Mock::given(method("POST"))
|
||||
.and(path("/oauth/token"))
|
||||
.respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({
|
||||
"access_token": access_token.clone(),
|
||||
"issued_token_type": "urn:ietf:params:oauth:token-type:access_token",
|
||||
"token_type": "Bearer",
|
||||
"expires_in": 3600,
|
||||
"user_id": "user_test",
|
||||
"chatgpt_account_id": "workspace_test",
|
||||
"chatgpt_plan_type": "enterprise",
|
||||
})))
|
||||
.expect(1)
|
||||
.mount(&server)
|
||||
.await;
|
||||
Mock::given(method("GET"))
|
||||
.and(path("/backend-api/wham/config/bundle"))
|
||||
.respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({})))
|
||||
.expect(1)
|
||||
.mount(&server)
|
||||
.await;
|
||||
Mock::given(method("GET"))
|
||||
.and(path("/backend-api/accounts/workspace_test/settings"))
|
||||
.and(header("authorization", format!("Bearer {access_token}")))
|
||||
.respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({
|
||||
"beta_settings": { "enable_plugins": true },
|
||||
})))
|
||||
.expect(1)
|
||||
.mount(&server)
|
||||
.await;
|
||||
|
||||
let mut mcp =
|
||||
TestAppServer::new_with_env(codex_home.path(), &[("OPENAI_API_KEY", None)]).await?;
|
||||
timeout(DEFAULT_TIMEOUT, mcp.initialize()).await??;
|
||||
std::fs::remove_file(token_path)?;
|
||||
|
||||
let request_id = mcp
|
||||
.send_plugin_installed_request(PluginInstalledParams {
|
||||
cwds: None,
|
||||
install_suggestion_plugin_names: None,
|
||||
})
|
||||
.await?;
|
||||
let response: JSONRPCResponse = timeout(
|
||||
DEFAULT_TIMEOUT,
|
||||
mcp.read_stream_until_response_message(RequestId::Integer(request_id)),
|
||||
)
|
||||
.await??;
|
||||
let response: PluginInstalledResponse = to_response(response)?;
|
||||
|
||||
assert!(response.marketplace_load_errors.is_empty());
|
||||
assert!(response.marketplaces.iter().any(|marketplace| {
|
||||
marketplace
|
||||
.plugins
|
||||
.iter()
|
||||
.any(|plugin| plugin.id == "linear@openai-curated" && plugin.installed)
|
||||
}));
|
||||
server.verify().await;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn plugin_installed_includes_installed_plugins_and_explicit_install_suggestions() -> Result<()>
|
||||
{
|
||||
|
||||
@@ -1034,9 +1034,14 @@ fn prune_redundant_unreadable_roots(
|
||||
retained.push((candidate, original));
|
||||
continue;
|
||||
}
|
||||
for (ancestor, _) in &retained {
|
||||
for (ancestor, ancestor_original) in &retained {
|
||||
if candidate == *ancestor {
|
||||
continue 'candidate;
|
||||
if original == *ancestor_original {
|
||||
continue 'candidate;
|
||||
}
|
||||
// Distinct symlink and canonical spellings need separate
|
||||
// mounts even when they resolve to the same inode.
|
||||
continue;
|
||||
}
|
||||
if !candidate.starts_with(ancestor) {
|
||||
continue;
|
||||
@@ -2687,6 +2692,25 @@ mod tests {
|
||||
);
|
||||
}
|
||||
|
||||
#[cfg(unix)]
|
||||
#[test]
|
||||
fn symlink_and_canonical_deny_spellings_are_both_retained() {
|
||||
let temp_dir = TempDir::new().expect("temp dir");
|
||||
let target = temp_dir.path().join("target-token");
|
||||
let symlink = temp_dir.path().join("projected-token");
|
||||
std::fs::write(&target, "secret").expect("write token");
|
||||
std::os::unix::fs::symlink(&target, &symlink).expect("create token symlink");
|
||||
|
||||
let retained = prune_redundant_unreadable_roots(
|
||||
vec![symlink.clone(), target.clone()],
|
||||
/*allowed_write_paths*/ &[],
|
||||
);
|
||||
|
||||
assert_eq!(retained.len(), 2);
|
||||
assert!(retained.contains(&symlink));
|
||||
assert!(retained.contains(&target));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn pruned_nested_deny_is_not_readded_as_a_read_only_mount() {
|
||||
let temp_dir = TempDir::new().expect("temp dir");
|
||||
|
||||
@@ -569,6 +569,7 @@ struct TestExternalChatgptAuth {
|
||||
}
|
||||
|
||||
struct FailingExternalChatgptAuth(&'static str);
|
||||
struct OptionalFailingExternalChatgptAuth(&'static str);
|
||||
|
||||
impl ExternalAuth for FailingExternalChatgptAuth {
|
||||
fn auth_mode(&self) -> AuthMode {
|
||||
@@ -593,6 +594,25 @@ impl ExternalAuth for FailingExternalChatgptAuth {
|
||||
}
|
||||
}
|
||||
|
||||
impl ExternalAuth for OptionalFailingExternalChatgptAuth {
|
||||
fn auth_mode(&self) -> AuthMode {
|
||||
AuthMode::Chatgpt
|
||||
}
|
||||
|
||||
fn resolve(&self) -> ExternalAuthFuture<'_, Option<ExternalAuthTokens>> {
|
||||
let message = self.0;
|
||||
Box::pin(async move { Err(std::io::Error::other(message)) })
|
||||
}
|
||||
|
||||
fn refresh(
|
||||
&self,
|
||||
_context: ExternalAuthRefreshContext,
|
||||
) -> ExternalAuthFuture<'_, ExternalAuthTokens> {
|
||||
let message = self.0;
|
||||
Box::pin(async move { Err(std::io::Error::other(message)) })
|
||||
}
|
||||
}
|
||||
|
||||
impl ExternalAuth for TestExternalChatgptAuth {
|
||||
fn auth_mode(&self) -> AuthMode {
|
||||
AuthMode::Chatgpt
|
||||
@@ -782,7 +802,7 @@ async fn auth_result_preserves_external_auth_resolution_errors() {
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn set_external_auth_if_absent_preserves_existing_provider() {
|
||||
async fn replace_non_required_external_auth_preserves_required_provider() {
|
||||
let manager = AuthManager::shared(
|
||||
PathBuf::from("non-existent"),
|
||||
/*enable_codex_api_key_env*/ false,
|
||||
@@ -792,22 +812,42 @@ async fn set_external_auth_if_absent_preserves_existing_provider() {
|
||||
)
|
||||
.await;
|
||||
|
||||
assert!(
|
||||
manager.set_external_auth_if_absent(Arc::new(FailingExternalChatgptAuth(
|
||||
"original external auth",
|
||||
)))
|
||||
);
|
||||
assert!(
|
||||
!manager.set_external_auth_if_absent(Arc::new(FailingExternalChatgptAuth(
|
||||
"replacement external auth",
|
||||
)))
|
||||
);
|
||||
manager.set_external_auth(Arc::new(FailingExternalChatgptAuth(
|
||||
"required external auth",
|
||||
)));
|
||||
let optional: Arc<dyn ExternalAuth> =
|
||||
Arc::new(OptionalFailingExternalChatgptAuth("optional external auth"));
|
||||
assert!(!manager.replace_non_required_external_auth(Arc::clone(&optional)));
|
||||
assert!(!manager.clear_external_auth_if(&optional));
|
||||
|
||||
let error = manager
|
||||
.auth()
|
||||
.await
|
||||
.expect_err("the original external auth should remain installed");
|
||||
assert_eq!(error.to_string(), "original external auth");
|
||||
.expect_err("the required external auth should remain installed");
|
||||
assert_eq!(error.to_string(), "required external auth");
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn non_required_external_auth_cleanup_is_registration_scoped() {
|
||||
let manager = AuthManager::shared(
|
||||
PathBuf::from("non-existent"),
|
||||
/*enable_codex_api_key_env*/ false,
|
||||
AuthCredentialsStoreMode::File,
|
||||
/*chatgpt_base_url*/ None,
|
||||
AuthKeyringBackendKind::default(),
|
||||
)
|
||||
.await;
|
||||
let first: Arc<dyn ExternalAuth> =
|
||||
Arc::new(OptionalFailingExternalChatgptAuth("first external auth"));
|
||||
let second: Arc<dyn ExternalAuth> =
|
||||
Arc::new(OptionalFailingExternalChatgptAuth("second external auth"));
|
||||
|
||||
assert!(manager.replace_non_required_external_auth(Arc::clone(&first)));
|
||||
assert!(manager.replace_non_required_external_auth(Arc::clone(&second)));
|
||||
assert!(!manager.clear_external_auth_if(&first));
|
||||
assert!(manager.has_external_auth());
|
||||
assert!(manager.clear_external_auth_if(&second));
|
||||
assert!(!manager.has_external_auth());
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
|
||||
@@ -1878,20 +1878,33 @@ impl AuthManager {
|
||||
}
|
||||
}
|
||||
|
||||
pub fn set_external_auth_if_absent(&self, external_auth: Arc<dyn ExternalAuth>) -> bool {
|
||||
if let Ok(mut guard) = self.external_auth.write()
|
||||
&& guard.is_none()
|
||||
{
|
||||
pub fn replace_non_required_external_auth(&self, external_auth: Arc<dyn ExternalAuth>) -> bool {
|
||||
if external_auth.requires_successful_resolution() {
|
||||
return false;
|
||||
}
|
||||
if let Ok(mut guard) = self.external_auth.write() {
|
||||
if guard
|
||||
.as_ref()
|
||||
.is_some_and(|current| current.requires_successful_resolution())
|
||||
{
|
||||
return false;
|
||||
}
|
||||
*guard = Some(external_auth);
|
||||
return true;
|
||||
}
|
||||
false
|
||||
}
|
||||
|
||||
pub fn clear_external_auth(&self) {
|
||||
if let Ok(mut guard) = self.external_auth.write() {
|
||||
pub fn clear_external_auth_if(&self, expected: &Arc<dyn ExternalAuth>) -> bool {
|
||||
if let Ok(mut guard) = self.external_auth.write()
|
||||
&& guard
|
||||
.as_ref()
|
||||
.is_some_and(|current| Arc::ptr_eq(current, expected))
|
||||
{
|
||||
*guard = None;
|
||||
return true;
|
||||
}
|
||||
false
|
||||
}
|
||||
|
||||
pub fn set_forced_chatgpt_workspace_id(&self, workspace_id: Option<Vec<String>>) {
|
||||
|
||||
Reference in New Issue
Block a user