[codex] close remaining WIF review gaps [ci changed_files]

This commit is contained in:
Cooper Gamble
2026-06-17 17:00:15 +00:00
parent e1cbb802cc
commit f7c59e93d6
12 changed files with 441 additions and 56 deletions
@@ -230,11 +230,7 @@ impl McpRequestProcessor {
.await
}
};
let auth = self
.auth_manager
.auth()
.await
.map_err(|err| internal_error(format!("failed to resolve auth: {err}")))?;
let auth = self.auth_manager.auth_cached();
let environment_manager = self.thread_manager.environment_manager();
// This status path has no turn-selected environment. Use config cwd
// as the local stdio fallback; named environment stdio MCPs must
@@ -1015,11 +1015,14 @@ impl PluginRequestProcessor {
let config = self.load_latest_config(config_cwd).await?;
let plugins_input = config.plugins_config_input();
let auth = self
.auth_manager
.auth()
.await
.map_err(|err| internal_error(format!("failed to resolve auth: {err}")))?;
let auth = match &read_source {
Ok(_) => self.auth_manager.auth_cached(),
Err(_) => self
.auth_manager
.auth()
.await
.map_err(|err| internal_error(format!("failed to resolve auth: {err}")))?,
};
plugins_manager.set_auth_mode(auth.as_ref().map(CodexAuth::api_auth_mode));
let plugin = match read_source {
@@ -1466,11 +1469,7 @@ impl PluginRequestProcessor {
};
let config_cwd = marketplace_path.as_path().parent().map(Path::to_path_buf);
let config = self.load_latest_config(config_cwd.clone()).await?;
let auth = self
.auth_manager
.auth()
.await
.map_err(|err| internal_error(format!("failed to resolve auth: {err}")))?;
let auth = self.auth_manager.auth_cached();
if !self
.workspace_codex_plugins_enabled(&config, auth.as_ref())
+3
View File
@@ -8,6 +8,7 @@ mod models_cache;
mod responses;
mod rollout;
mod test_app_server;
mod workload_identity;
pub use analytics_server::start_analytics_events_server;
pub use auth_fixtures::ChatGptAuthFixture;
@@ -47,6 +48,8 @@ use serde::de::DeserializeOwned;
pub use test_app_server::DEFAULT_CLIENT_NAME;
pub use test_app_server::DISABLE_PLUGIN_STARTUP_TASKS_ARG;
pub use test_app_server::TestAppServer;
pub use workload_identity::ExpiringWorkloadIdentityFixture;
pub use workload_identity::configure_expiring_workload_identity;
pub fn to_response<T: DeserializeOwned>(response: JSONRPCResponse) -> anyhow::Result<T> {
let value = serde_json::to_value(response.result)?;
@@ -0,0 +1,91 @@
use std::path::Path;
use std::path::PathBuf;
use std::time::Duration;
use anyhow::Result;
use wiremock::Mock;
use wiremock::MockServer;
use wiremock::ResponseTemplate;
use wiremock::matchers::header;
use wiremock::matchers::method;
use wiremock::matchers::path;
use crate::ChatGptIdTokenClaims;
use crate::encode_id_token;
pub struct ExpiringWorkloadIdentityFixture {
token_path: PathBuf,
}
impl ExpiringWorkloadIdentityFixture {
pub async fn remove_and_wait_for_expiry(self) -> Result<()> {
tokio::fs::remove_file(self.token_path).await?;
tokio::time::sleep(Duration::from_secs(3)).await;
Ok(())
}
}
pub async fn configure_expiring_workload_identity(
codex_home: &Path,
server: &MockServer,
) -> Result<ExpiringWorkloadIdentityFixture> {
let token_path = codex_home.join("projected-workload-token");
tokio::fs::write(&token_path, "external-subject-token\n").await?;
let token_path_toml = serde_json::to_string(token_path.to_string_lossy().as_ref())?;
let config_path = codex_home.join("config.toml");
let mut config = std::fs::read_to_string(&config_path)?;
config.push_str(&format!(
r#"
[workload_identity]
identity_provider_id = "idp_test"
identity_provider_mapping_id = "idpm_test"
audience = "api://codex-test"
token_url = "{}/oauth/token"
[workload_identity.credential_source]
type = "file"
path = {token_path_toml}
"#,
server.uri(),
));
std::fs::write(config_path, config)?;
let access_token = encode_id_token(
&ChatGptIdTokenClaims::new()
.email("workload@example.com")
.plan_type("enterprise")
.chatgpt_user_id("user_test")
.chatgpt_account_id("workspace_test"),
)?;
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": 2,
"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 },
})))
.mount(server)
.await;
Ok(ExpiringWorkloadIdentityFixture { token_path })
}
@@ -6,6 +6,7 @@ use std::time::Duration;
use anyhow::Result;
use app_test_support::TestAppServer;
use app_test_support::configure_expiring_workload_identity;
use app_test_support::create_mock_responses_server_sequence_unchecked;
use app_test_support::to_response;
use app_test_support::write_mock_responses_config_toml;
@@ -114,6 +115,68 @@ url = "{mcp_server_url}/mcp"
Ok(())
}
#[tokio::test]
async fn mcp_server_status_list_succeeds_when_workload_identity_is_unavailable() -> Result<()> {
let server = create_mock_responses_server_sequence_unchecked(Vec::new()).await;
let (mcp_server_url, mcp_server_handle) = start_mcp_server("local.status").await?;
let codex_home = TempDir::new()?;
write_mock_responses_config_toml(
codex_home.path(),
&server.uri(),
&BTreeMap::new(),
/*auto_compact_limit*/ 1024,
/*requires_openai_auth*/ None,
"mock_provider",
"compact",
)?;
let config_path = codex_home.path().join("config.toml");
let mut config_toml = std::fs::read_to_string(&config_path)?;
config_toml.insert_str(
0,
&format!("chatgpt_base_url = \"{}/backend-api\"\n", server.uri()),
);
config_toml.push_str(&format!(
r#"
[mcp_servers.local]
url = "{mcp_server_url}/mcp"
"#
));
std::fs::write(config_path, config_toml)?;
let workload_identity =
configure_expiring_workload_identity(codex_home.path(), &server).await?;
let mut mcp =
TestAppServer::new_with_env(codex_home.path(), &[("OPENAI_API_KEY", None)]).await?;
timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??;
workload_identity.remove_and_wait_for_expiry().await?;
let request_id = mcp
.send_list_mcp_server_status_request(ListMcpServerStatusParams {
cursor: None,
limit: None,
detail: None,
thread_id: None,
})
.await?;
let response = timeout(
DEFAULT_READ_TIMEOUT,
mcp.read_stream_until_response_message(RequestId::Integer(request_id)),
)
.await??;
let response: ListMcpServerStatusResponse = to_response(response)?;
let local_status = response
.data
.iter()
.find(|status| status.name == "local")
.expect("local MCP status should be returned");
assert!(local_status.tools.contains_key("local.status"));
server.verify().await;
mcp_server_handle.abort();
let _ = mcp_server_handle.await;
Ok(())
}
#[tokio::test]
async fn mcp_server_status_list_uses_thread_project_local_config() -> Result<()> {
let server = create_mock_responses_server_sequence_unchecked(Vec::new()).await;
@@ -10,6 +10,7 @@ use anyhow::bail;
use app_test_support::ChatGptAuthFixture;
use app_test_support::DEFAULT_CLIENT_NAME;
use app_test_support::TestAppServer;
use app_test_support::configure_expiring_workload_identity;
use app_test_support::start_analytics_events_server;
use app_test_support::to_response;
use app_test_support::write_chatgpt_auth;
@@ -666,6 +667,66 @@ async fn plugin_install_returns_invalid_request_for_missing_marketplace_file() -
Ok(())
}
#[tokio::test]
async fn plugin_install_local_succeeds_when_workload_identity_is_unavailable() -> Result<()> {
let codex_home = TempDir::new()?;
let server = MockServer::start().await;
std::fs::write(
codex_home.path().join("config.toml"),
format!(
r#"chatgpt_base_url = "{}/backend-api"
[features]
plugins = true
"#,
server.uri()
),
)?;
let workload_identity =
configure_expiring_workload_identity(codex_home.path(), &server).await?;
let repo_root = TempDir::new()?;
write_plugin_marketplace(
repo_root.path(),
"debug",
"sample-plugin",
"./sample-plugin",
/*install_policy*/ None,
/*auth_policy*/ None,
)?;
write_plugin_source(repo_root.path(), "sample-plugin", &[])?;
let marketplace_path =
AbsolutePathBuf::try_from(repo_root.path().join(".agents/plugins/marketplace.json"))?;
let mut mcp =
TestAppServer::new_with_env(codex_home.path(), &[("OPENAI_API_KEY", None)]).await?;
timeout(DEFAULT_TIMEOUT, mcp.initialize()).await??;
workload_identity.remove_and_wait_for_expiry().await?;
let request_id = mcp
.send_plugin_install_request(PluginInstallParams {
marketplace_path: Some(marketplace_path),
remote_marketplace_name: None,
plugin_name: "sample-plugin".to_string(),
})
.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.apps_needing_auth, Vec::<AppSummary>::new());
assert!(
codex_home
.path()
.join("plugins/cache/debug/sample-plugin/local/.codex-plugin/plugin.json")
.is_file()
);
server.verify().await;
Ok(())
}
#[tokio::test]
async fn plugin_install_returns_invalid_request_for_not_available_plugin() -> Result<()> {
let codex_home = TempDir::new()?;
@@ -5,6 +5,7 @@ use std::time::Duration;
use anyhow::Result;
use app_test_support::ChatGptAuthFixture;
use app_test_support::TestAppServer;
use app_test_support::configure_expiring_workload_identity;
use app_test_support::to_response;
use app_test_support::write_chatgpt_auth;
use axum::Json;
@@ -926,6 +927,59 @@ enabled = true
Ok(())
}
#[tokio::test]
async fn plugin_read_local_succeeds_when_workload_identity_is_unavailable() -> Result<()> {
let codex_home = TempDir::new()?;
let server = MockServer::start().await;
let repo_root = TempDir::new()?;
write_plugin_marketplace(
repo_root.path(),
"debug",
"sample-plugin",
"./sample-plugin",
)?;
write_plugin_source(repo_root.path(), "sample-plugin", &[])?;
std::fs::write(
codex_home.path().join("config.toml"),
format!(
r#"chatgpt_base_url = "{}/backend-api"
[features]
plugins = true
"#,
server.uri()
),
)?;
let workload_identity =
configure_expiring_workload_identity(codex_home.path(), &server).await?;
let marketplace_path =
AbsolutePathBuf::try_from(repo_root.path().join(".agents/plugins/marketplace.json"))?;
let mut mcp =
TestAppServer::new_with_env(codex_home.path(), &[("OPENAI_API_KEY", None)]).await?;
timeout(DEFAULT_TIMEOUT, mcp.initialize()).await??;
workload_identity.remove_and_wait_for_expiry().await?;
let request_id = mcp
.send_plugin_read_request(PluginReadParams {
marketplace_path: Some(marketplace_path),
remote_marketplace_name: None,
plugin_name: "sample-plugin".to_string(),
})
.await?;
let response: JSONRPCResponse = timeout(
DEFAULT_TIMEOUT,
mcp.read_stream_until_response_message(RequestId::Integer(request_id)),
)
.await??;
let response: PluginReadResponse = to_response(response)?;
assert_eq!(response.plugin.summary.id, "sample-plugin@debug");
assert_eq!(response.plugin.summary.name, "sample-plugin");
server.verify().await;
Ok(())
}
#[tokio::test]
async fn plugin_read_returns_share_context_for_shared_local_plugin() -> Result<()> {
let codex_home = TempDir::new()?;
+113 -3
View File
@@ -145,10 +145,35 @@ async fn build_uploaded_argument_value(
.to_abs_path()
.map_err(|error| contextualize_error(error.to_string()))?;
let resolved_path = native_environment_cwd.join(file_path);
let path_uri = PathUri::from_abs_path(&resolved_path);
let file_system_policy = turn_context.file_system_sandbox_policy();
if !file_system_policy
.can_read_path_with_cwd(resolved_path.as_path(), native_environment_cwd.as_path())
{
return Err(contextualize_error(
"path is not readable under the current sandbox policy".to_string(),
));
}
let requested_path_uri = PathUri::from_abs_path(&resolved_path);
let fs = turn_environment.environment.get_filesystem();
// Upload preprocessing runs in the Codex process, so enforce the turn's
// read policy before opening either the requested path or a symlink target.
let canonical_path_uri = fs
.canonicalize(&requested_path_uri, /*sandbox*/ None)
.await
.map_err(|error| contextualize_error(error.to_string()))?;
let canonical_path = canonical_path_uri
.to_abs_path()
.map_err(|error| contextualize_error(error.to_string()))?;
if !file_system_policy
.can_read_path_with_cwd(canonical_path.as_path(), native_environment_cwd.as_path())
{
return Err(contextualize_error(
"path is not readable under the current sandbox policy".to_string(),
));
}
let metadata = fs
.get_metadata(&path_uri, /*sandbox*/ None)
.get_metadata(&canonical_path_uri, /*sandbox*/ None)
.await
.map_err(|error| contextualize_error(error.to_string()))?;
if !metadata.is_file {
@@ -166,7 +191,7 @@ async fn build_uploaded_argument_value(
)));
}
let contents = fs
.read_file_stream(&path_uri, /*sandbox*/ None)
.read_file_stream(&canonical_path_uri, /*sandbox*/ None)
.await
.map_err(|error| contextualize_error(error.to_string()))?;
let file_name = resolved_path
@@ -199,6 +224,11 @@ mod tests {
use super::*;
use crate::session::tests::make_session_and_context;
use crate::session::turn_context::TurnEnvironment;
use codex_protocol::models::PermissionProfile;
use codex_protocol::permissions::FileSystemAccessMode;
use codex_protocol::permissions::FileSystemPath;
use codex_protocol::permissions::FileSystemSandboxEntry;
use codex_protocol::permissions::FileSystemSandboxPolicy;
use codex_utils_absolute_path::AbsolutePathBuf;
use codex_utils_path_uri::PathUri;
use pretty_assertions::assert_eq;
@@ -233,6 +263,25 @@ mod tests {
);
}
fn deny_file_read(turn_context: &mut TurnContext, path: AbsolutePathBuf) {
let mut file_system_policy = turn_context.file_system_sandbox_policy();
file_system_policy.preserve_deny_read_restrictions_from(
&FileSystemSandboxPolicy::restricted(vec![FileSystemSandboxEntry {
path: FileSystemPath::Path { path: path.clone() },
access: FileSystemAccessMode::Deny,
}]),
);
turn_context.permission_profile =
PermissionProfile::from_runtime_permissions_with_enforcement(
turn_context.permission_profile.enforcement(),
&file_system_policy,
turn_context.network_sandbox_policy(),
);
Arc::make_mut(&mut turn_context.config)
.workload_identity_credential_deny_paths
.push(path);
}
#[tokio::test]
async fn openai_file_argument_rewrite_requires_declared_file_params() {
let (session, turn_context) = make_session_and_context().await;
@@ -358,6 +407,67 @@ mod tests {
assert!(error.contains(&(OPENAI_FILE_UPLOAD_LIMIT_BYTES + 1).to_string()));
}
#[tokio::test]
async fn build_uploaded_argument_value_rejects_wif_credential_path() {
let (_, mut turn_context) = make_session_and_context().await;
let auth = CodexAuth::create_dummy_chatgpt_auth_for_testing();
let dir = tempdir().expect("temp dir");
let credential_path = dir.path().join("subject-token");
tokio::fs::write(&credential_path, b"external.subject.token")
.await
.expect("write credential");
set_primary_environment_cwd(&mut turn_context, dir.path());
deny_file_read(
&mut turn_context,
AbsolutePathBuf::try_from(credential_path).expect("absolute credential path"),
);
let error = build_uploaded_argument_value(
&turn_context,
Some(&auth),
"file",
/*index*/ None,
"subject-token",
)
.await
.expect_err("WIF credential should not be uploaded");
assert!(error.contains("not readable under the current sandbox policy"));
assert!(!error.contains("external.subject.token"));
}
#[cfg(unix)]
#[tokio::test]
async fn build_uploaded_argument_value_rejects_symlink_to_wif_credential() {
let (_, mut turn_context) = make_session_and_context().await;
let auth = CodexAuth::create_dummy_chatgpt_auth_for_testing();
let dir = tempdir().expect("temp dir");
let credential_path = dir.path().join("subject-token");
let upload_path = dir.path().join("report.csv");
tokio::fs::write(&credential_path, b"external.subject.token")
.await
.expect("write credential");
std::os::unix::fs::symlink(&credential_path, &upload_path).expect("create symlink");
set_primary_environment_cwd(&mut turn_context, dir.path());
deny_file_read(
&mut turn_context,
AbsolutePathBuf::try_from(credential_path.clone()).expect("absolute credential path"),
);
let error = build_uploaded_argument_value(
&turn_context,
Some(&auth),
"file",
/*index*/ None,
"report.csv",
)
.await
.expect_err("symlinked WIF credential should not be uploaded");
assert!(error.contains("not readable under the current sandbox policy"));
assert!(!error.contains(credential_path.to_string_lossy().as_ref()));
}
#[tokio::test]
async fn rewrite_argument_value_for_openai_files_rewrites_scalar_path() {
use wiremock::Mock;
@@ -191,12 +191,12 @@ impl RequestPluginInstallHandler {
.as_ref()
.is_some_and(|response| response.action == ElicitationAction::Accept);
let auth = session.services.auth_manager.auth().await.map_err(|err| {
FunctionCallError::Fatal(format!(
"failed to resolve auth after plugin installation: {err}"
))
})?;
let completed = if user_confirmed {
let auth = session.services.auth_manager.auth().await.map_err(|err| {
FunctionCallError::Fatal(format!(
"failed to resolve auth after plugin installation: {err}"
))
})?;
verify_request_plugin_install_completed(&session, &turn, &tool, auth.as_ref()).await
} else {
false
@@ -0,0 +1,31 @@
use super::*;
use pretty_assertions::assert_eq;
#[tokio::test]
async fn reads_and_rereads_projected_token() -> anyhow::Result<()> {
let temp_dir = tempfile::tempdir()?;
let path = temp_dir.path().join("azure-token");
tokio::fs::write(&path, "first.azure.token\n").await?;
let source = AzureSubjectTokenProvider::new(Some(path.clone()));
assert_eq!(
source.subject_token().await?,
SubjectToken::jwt("first.azure.token", "azure")?
);
tokio::fs::write(path, "second.azure.token\n").await?;
assert_eq!(
source.subject_token().await?,
SubjectToken::jwt("second.azure.token", "azure")?
);
Ok(())
}
#[tokio::test]
async fn rejects_relative_token_path() {
let source = AzureSubjectTokenProvider::new(Some(PathBuf::from("azure-token")));
assert!(matches!(
source.subject_token().await,
Err(SubjectTokenError::InvalidConfiguration { provider: "azure" })
));
}
+2 -33
View File
@@ -39,36 +39,5 @@ impl SubjectTokenProvider for AzureSubjectTokenProvider {
}
#[cfg(test)]
mod tests {
use super::*;
use pretty_assertions::assert_eq;
#[tokio::test]
async fn reads_and_rereads_projected_token() -> anyhow::Result<()> {
let temp_dir = tempfile::tempdir()?;
let path = temp_dir.path().join("azure-token");
tokio::fs::write(&path, "first.azure.token\n").await?;
let source = AzureSubjectTokenProvider::new(Some(path.clone()));
assert_eq!(
source.subject_token().await?,
SubjectToken::jwt("first.azure.token", "azure")?
);
tokio::fs::write(path, "second.azure.token\n").await?;
assert_eq!(
source.subject_token().await?,
SubjectToken::jwt("second.azure.token", "azure")?
);
Ok(())
}
#[tokio::test]
async fn rejects_relative_token_path() {
let source = AzureSubjectTokenProvider::new(Some(PathBuf::from("azure-token")));
assert!(matches!(
source.subject_token().await,
Err(SubjectTokenError::InvalidConfiguration { provider: "azure" })
));
}
}
#[path = "azure_tests.rs"]
mod tests;
@@ -163,6 +163,7 @@ async fn proactive_refresh_failure_uses_token_until_expiry() -> anyhow::Result<(
cache.token.as_mut().expect("cached token").refresh_at = Instant::now();
}
server.verify().await;
server.reset().await;
Mock::given(method("POST"))
.and(path("/oauth/token"))
@@ -173,6 +174,7 @@ async fn proactive_refresh_failure_uses_token_until_expiry() -> anyhow::Result<(
assert_eq!(client.resolve().await?, token);
assert_eq!(client.resolve().await?, token);
server.verify().await;
Ok(())
}
@@ -194,6 +196,7 @@ async fn proactive_refresh_failure_fails_after_cached_token_expires() -> anyhow:
cached.expires_at = Instant::now();
}
server.verify().await;
server.reset().await;
Mock::given(method("POST"))
.and(path("/oauth/token"))
@@ -204,6 +207,7 @@ async fn proactive_refresh_failure_fails_after_cached_token_expires() -> anyhow:
assert!(client.resolve().await.is_err());
assert!(client.resolve().await.is_err());
server.verify().await;
Ok(())
}
@@ -219,6 +223,7 @@ async fn forced_refresh_does_not_reuse_a_rejected_token() -> anyhow::Result<()>
let client = test_client(&server).await?;
client.resolve().await?;
server.verify().await;
server.reset().await;
Mock::given(method("POST"))
.and(path("/oauth/token"))
@@ -230,6 +235,7 @@ async fn forced_refresh_does_not_reuse_a_rejected_token() -> anyhow::Result<()>
assert!(client.refresh().await.is_err());
assert!(client.cache.lock().expect("cache lock").token.is_none());
assert!(client.resolve().await.is_err());
server.verify().await;
Ok(())
}
@@ -245,6 +251,7 @@ async fn refresh_rejects_a_changed_principal() -> anyhow::Result<()> {
let client = test_client(&server).await?;
client.resolve().await?;
server.verify().await;
server.reset().await;
Mock::given(method("POST"))
.and(path("/oauth/token"))
@@ -265,6 +272,7 @@ async fn refresh_rejects_a_changed_principal() -> anyhow::Result<()> {
Err(WorkloadIdentityError::PrincipalChanged)
));
assert!(client.cache.lock().expect("cache lock").token.is_none());
server.verify().await;
Ok(())
}