mirror of
https://github.com/openai/codex.git
synced 2026-10-20 17:38:45 +00:00
[codex] address workload identity review findings [ci changed_files]
This commit is contained in:
@@ -17,6 +17,10 @@ name = "codex_app_server"
|
||||
path = "src/lib.rs"
|
||||
doctest = false
|
||||
|
||||
[features]
|
||||
default = ["workload-identity-all-providers"]
|
||||
workload-identity-all-providers = ["codex-login/workload-identity-all-providers"]
|
||||
|
||||
[lints]
|
||||
workspace = true
|
||||
|
||||
|
||||
@@ -207,6 +207,7 @@ pub(crate) struct MessageProcessor {
|
||||
thread_processor: ThreadRequestProcessor,
|
||||
turn_processor: TurnRequestProcessor,
|
||||
windows_sandbox_processor: WindowsSandboxRequestProcessor,
|
||||
owns_external_auth_refresh_bridge: bool,
|
||||
request_serialization_queues: RequestSerializationQueues,
|
||||
}
|
||||
|
||||
@@ -320,9 +321,10 @@ impl MessageProcessor {
|
||||
remote_control_handle,
|
||||
plugin_startup_tasks,
|
||||
} = args;
|
||||
auth_manager.set_external_auth_if_absent(Arc::new(ExternalAuthRefreshBridge {
|
||||
outgoing: outgoing.clone(),
|
||||
}));
|
||||
let owns_external_auth_refresh_bridge =
|
||||
auth_manager.set_external_auth_if_absent(Arc::new(ExternalAuthRefreshBridge {
|
||||
outgoing: outgoing.clone(),
|
||||
}));
|
||||
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,
|
||||
@@ -565,12 +567,15 @@ impl MessageProcessor {
|
||||
thread_processor,
|
||||
turn_processor,
|
||||
windows_sandbox_processor,
|
||||
owns_external_auth_refresh_bridge,
|
||||
request_serialization_queues: RequestSerializationQueues::default(),
|
||||
}
|
||||
}
|
||||
|
||||
pub(crate) fn clear_runtime_references(&self) {
|
||||
self.account_processor.clear_external_auth();
|
||||
if self.owns_external_auth_refresh_bridge {
|
||||
self.account_processor.clear_external_auth();
|
||||
}
|
||||
self.apps_processor.shutdown();
|
||||
self.models_refresh_worker.shutdown();
|
||||
self.skills_watcher.shutdown();
|
||||
|
||||
@@ -534,6 +534,10 @@ impl PluginRequestProcessor {
|
||||
marketplace_kinds.unwrap_or_else(|| vec![PluginListMarketplaceKind::Local]);
|
||||
let include_local = marketplace_kinds.contains(&PluginListMarketplaceKind::Local);
|
||||
let include_vertical = marketplace_kinds.contains(&PluginListMarketplaceKind::Vertical);
|
||||
let include_shared_with_me =
|
||||
marketplace_kinds.contains(&PluginListMarketplaceKind::SharedWithMe);
|
||||
let include_workspace_directory =
|
||||
marketplace_kinds.contains(&PluginListMarketplaceKind::WorkspaceDirectory);
|
||||
|
||||
let config = self.load_latest_config(/*fallback_cwd*/ None).await?;
|
||||
let empty_response = || PluginListResponse {
|
||||
@@ -544,11 +548,24 @@ 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 include_created_by_me_remote = marketplace_kinds
|
||||
.contains(&PluginListMarketplaceKind::CreatedByMeRemote)
|
||||
&& config.features.enabled(Feature::RemotePlugin);
|
||||
let include_global_remote =
|
||||
!explicit_marketplace_kinds && config.features.enabled(Feature::RemotePlugin);
|
||||
let requires_resolved_auth = include_vertical
|
||||
|| include_created_by_me_remote
|
||||
|| include_global_remote
|
||||
|| include_workspace_directory
|
||||
|| (include_shared_with_me && 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
|
||||
@@ -558,13 +575,6 @@ impl PluginRequestProcessor {
|
||||
let auth_mode = auth.as_ref().map(CodexAuth::api_auth_mode);
|
||||
plugins_manager.set_auth_mode(auth_mode);
|
||||
let plugins_input = config.plugins_config_input();
|
||||
let include_shared_with_me =
|
||||
marketplace_kinds.contains(&PluginListMarketplaceKind::SharedWithMe);
|
||||
let include_created_by_me_remote = marketplace_kinds
|
||||
.contains(&PluginListMarketplaceKind::CreatedByMeRemote)
|
||||
&& config.features.enabled(Feature::RemotePlugin);
|
||||
let include_global_remote =
|
||||
!explicit_marketplace_kinds && config.features.enabled(Feature::RemotePlugin);
|
||||
let use_remote_global_catalog =
|
||||
include_global_remote && auth_mode.is_some_and(AuthMode::uses_codex_backend);
|
||||
let remote_plugin_service_config = RemotePluginServiceConfig {
|
||||
@@ -681,7 +691,7 @@ impl PluginRequestProcessor {
|
||||
if include_created_by_me_remote {
|
||||
remote_sources.push(RemoteMarketplaceSource::CreatedByMeRemote);
|
||||
}
|
||||
if marketplace_kinds.contains(&PluginListMarketplaceKind::WorkspaceDirectory) {
|
||||
if include_workspace_directory {
|
||||
remote_sources.push(RemoteMarketplaceSource::WorkspaceDirectory);
|
||||
}
|
||||
if include_shared_with_me && config.features.enabled(Feature::PluginSharing) {
|
||||
|
||||
@@ -6,6 +6,8 @@ use app_test_support::ChatGptAuthFixture;
|
||||
use app_test_support::TestAppServer;
|
||||
use app_test_support::to_response;
|
||||
use app_test_support::write_chatgpt_auth;
|
||||
use base64::Engine;
|
||||
use base64::engine::general_purpose::URL_SAFE_NO_PAD;
|
||||
use codex_app_server_protocol::JSONRPCResponse;
|
||||
use codex_app_server_protocol::PluginAuthPolicy;
|
||||
use codex_app_server_protocol::PluginInstallPolicy;
|
||||
@@ -71,6 +73,52 @@ plugins = true
|
||||
)
|
||||
}
|
||||
|
||||
fn write_plugins_enabled_config_with_workload_identity(
|
||||
codex_home: &std::path::Path,
|
||||
server: &MockServer,
|
||||
token_path: &std::path::Path,
|
||||
) -> 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()));
|
||||
let token_url = toml::Value::String(format!("{}/oauth/token", server.uri()));
|
||||
std::fs::write(
|
||||
codex_home.join("config.toml"),
|
||||
format!(
|
||||
r#"chatgpt_base_url = {chatgpt_base_url}
|
||||
|
||||
[features]
|
||||
plugins = true
|
||||
remote_plugin = false
|
||||
|
||||
[workload_identity]
|
||||
identity_provider_id = "idp_test"
|
||||
identity_provider_mapping_id = "idpm_test"
|
||||
audience = "api://codex-test"
|
||||
token_url = {token_url}
|
||||
|
||||
[workload_identity.credential_source]
|
||||
type = "file"
|
||||
path = {token_path}
|
||||
"#,
|
||||
),
|
||||
)
|
||||
}
|
||||
|
||||
fn workload_identity_access_token() -> String {
|
||||
let payload = URL_SAFE_NO_PAD.encode(
|
||||
serde_json::json!({
|
||||
"email": "workload@example.com",
|
||||
"https://api.openai.com/auth": {
|
||||
"chatgpt_account_id": "workspace_test",
|
||||
"chatgpt_plan_type": "enterprise",
|
||||
"chatgpt_user_id": "user_test",
|
||||
}
|
||||
})
|
||||
.to_string(),
|
||||
);
|
||||
format!("header.{payload}.signature")
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn plugin_list_skips_invalid_marketplace_file_and_reports_error() -> Result<()> {
|
||||
let codex_home = TempDir::new()?;
|
||||
@@ -129,6 +177,56 @@ async fn plugin_list_skips_invalid_marketplace_file_and_reports_error() -> Resul
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn plugin_list_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_plugins_enabled_config_with_workload_identity(codex_home.path(), &server, &token_path)?;
|
||||
|
||||
Mock::given(method("POST"))
|
||||
.and(path("/oauth/token"))
|
||||
.respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({
|
||||
"access_token": workload_identity_access_token(),
|
||||
"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",
|
||||
})))
|
||||
.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;
|
||||
|
||||
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_list_request(PluginListParams {
|
||||
cwds: None,
|
||||
marketplace_kinds: Some(vec![PluginListMarketplaceKind::Local]),
|
||||
})
|
||||
.await?;
|
||||
let response: JSONRPCResponse = timeout(
|
||||
DEFAULT_TIMEOUT,
|
||||
mcp.read_stream_until_response_message(RequestId::Integer(request_id)),
|
||||
)
|
||||
.await??;
|
||||
let response: PluginListResponse = to_response(response)?;
|
||||
|
||||
assert!(response.marketplace_load_errors.is_empty());
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn plugin_installed_includes_installed_plugins_and_explicit_install_suggestions() -> Result<()>
|
||||
{
|
||||
|
||||
@@ -14,6 +14,10 @@ name = "codex_cli"
|
||||
path = "src/lib.rs"
|
||||
doctest = false
|
||||
|
||||
[features]
|
||||
default = ["workload-identity-all-providers"]
|
||||
workload-identity-all-providers = ["codex-login/workload-identity-all-providers"]
|
||||
|
||||
[lints]
|
||||
workspace = true
|
||||
|
||||
|
||||
@@ -810,6 +810,11 @@ pub struct Config {
|
||||
/// Workload identity federation configuration for external ChatGPT auth.
|
||||
pub workload_identity: Option<WorkloadIdentityConfig>,
|
||||
|
||||
/// Runtime-only paths added to the sandbox for workload credential isolation.
|
||||
/// These enforcement details must not be exposed in model-visible context.
|
||||
#[doc(hidden)]
|
||||
pub workload_identity_credential_deny_paths: Vec<AbsolutePathBuf>,
|
||||
|
||||
/// Definition for MCP servers that Codex can reach out to for tool calls.
|
||||
pub mcp_servers: Constrained<HashMap<String, McpServerConfig>>,
|
||||
|
||||
@@ -2347,13 +2352,11 @@ fn apply_managed_filesystem_constraints(
|
||||
|
||||
fn apply_workload_identity_filesystem_constraints(
|
||||
file_system_sandbox_policy: &mut FileSystemSandboxPolicy,
|
||||
workload_identity: &WorkloadIdentityConfig,
|
||||
credential_deny_paths: &[AbsolutePathBuf],
|
||||
) {
|
||||
let entries = workload_identity
|
||||
.credential_source
|
||||
.credential_file_paths()
|
||||
.into_iter()
|
||||
.flat_map(workload_identity_credential_deny_paths)
|
||||
let entries = credential_deny_paths
|
||||
.iter()
|
||||
.cloned()
|
||||
.map(|path| codex_protocol::permissions::FileSystemSandboxEntry {
|
||||
path: codex_protocol::permissions::FileSystemPath::Path { path },
|
||||
access: codex_protocol::permissions::FileSystemAccessMode::Deny,
|
||||
@@ -2367,7 +2370,18 @@ fn apply_workload_identity_filesystem_constraints(
|
||||
.preserve_deny_read_restrictions_from(&FileSystemSandboxPolicy::restricted(entries));
|
||||
}
|
||||
|
||||
fn workload_identity_credential_deny_paths(path: PathBuf) -> Vec<AbsolutePathBuf> {
|
||||
fn workload_identity_credential_deny_paths(
|
||||
workload_identity: &WorkloadIdentityConfig,
|
||||
) -> Vec<AbsolutePathBuf> {
|
||||
workload_identity
|
||||
.credential_source
|
||||
.credential_file_paths()
|
||||
.into_iter()
|
||||
.flat_map(workload_identity_credential_path_deny_paths)
|
||||
.collect()
|
||||
}
|
||||
|
||||
fn workload_identity_credential_path_deny_paths(path: PathBuf) -> Vec<AbsolutePathBuf> {
|
||||
let Ok(path) = AbsolutePathBuf::relative_to_current_dir(path) else {
|
||||
return Vec::new();
|
||||
};
|
||||
@@ -3615,7 +3629,9 @@ impl Config {
|
||||
filesystem_requirements,
|
||||
);
|
||||
}
|
||||
if let Some(workload_identity) = cfg.workload_identity.as_ref() {
|
||||
let workload_identity_credential_deny_paths = if let Some(workload_identity) =
|
||||
cfg.workload_identity.as_ref()
|
||||
{
|
||||
if cfg.forced_login_method == Some(ForcedLoginMethod::Api) {
|
||||
return Err(std::io::Error::new(
|
||||
std::io::ErrorKind::InvalidInput,
|
||||
@@ -3625,11 +3641,16 @@ impl Config {
|
||||
workload_identity.validate().map_err(|error| {
|
||||
std::io::Error::new(std::io::ErrorKind::InvalidInput, error.to_string())
|
||||
})?;
|
||||
let credential_deny_paths =
|
||||
workload_identity_credential_deny_paths(workload_identity);
|
||||
apply_workload_identity_filesystem_constraints(
|
||||
&mut effective_file_system_sandbox_policy,
|
||||
workload_identity,
|
||||
&credential_deny_paths,
|
||||
);
|
||||
}
|
||||
credential_deny_paths
|
||||
} else {
|
||||
Vec::new()
|
||||
};
|
||||
let effective_file_system_sandbox_policy = effective_file_system_sandbox_policy
|
||||
.with_additional_readable_roots(resolved_cwd.as_path(), &helper_readable_roots);
|
||||
let effective_permission_profile = PermissionProfile::from_runtime_permissions_with_enforcement(
|
||||
@@ -3694,6 +3715,7 @@ impl Config {
|
||||
env!("CARGO_PKG_VERSION"),
|
||||
),
|
||||
workload_identity: cfg.workload_identity.clone(),
|
||||
workload_identity_credential_deny_paths,
|
||||
mcp_servers,
|
||||
// The config.toml omits "_mode" because it's a config file. However, "_mode"
|
||||
// is important in code to differentiate the mode from the store implementation.
|
||||
|
||||
@@ -430,8 +430,9 @@ impl EnvironmentContext {
|
||||
Self::network_from_turn_context(turn_context),
|
||||
/*subagents*/ None,
|
||||
);
|
||||
let model_visible_permission_profile = turn_context.model_visible_permission_profile();
|
||||
context.filesystem = Some(FileSystemContext::from_permission_profile(
|
||||
&turn_context.permission_profile,
|
||||
&model_visible_permission_profile,
|
||||
&turn_context.config.effective_workspace_roots(),
|
||||
));
|
||||
context
|
||||
|
||||
@@ -183,6 +183,42 @@ fn serialize_environment_context_with_full_filesystem_profile() {
|
||||
assert_eq!(context.render(), expected);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn enforcement_only_paths_are_omitted_from_environment_context() {
|
||||
let credential_path = test_abs_path("/var/run/secrets/workload-token");
|
||||
let visible_denial = test_abs_path("/repo/private");
|
||||
let permission_profile = PermissionProfile::from_runtime_permissions(
|
||||
&FileSystemSandboxPolicy::restricted(vec![
|
||||
FileSystemSandboxEntry {
|
||||
path: FileSystemPath::Path {
|
||||
path: credential_path.clone(),
|
||||
},
|
||||
access: FileSystemAccessMode::Deny,
|
||||
},
|
||||
FileSystemSandboxEntry {
|
||||
path: FileSystemPath::Path {
|
||||
path: visible_denial.clone(),
|
||||
},
|
||||
access: FileSystemAccessMode::Deny,
|
||||
},
|
||||
]),
|
||||
NetworkSandboxPolicy::Restricted,
|
||||
);
|
||||
let model_visible_permission_profile =
|
||||
crate::session::turn_context::model_visible_permission_profile(
|
||||
&permission_profile,
|
||||
&[credential_path.clone()],
|
||||
);
|
||||
let context = FileSystemContext::from_permission_profile(
|
||||
&model_visible_permission_profile,
|
||||
&[test_abs_path("/repo")],
|
||||
)
|
||||
.render();
|
||||
|
||||
assert!(!context.contains(credential_path.to_string_lossy().as_ref()));
|
||||
assert!(context.contains(visible_denial.to_string_lossy().as_ref()));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn turn_context_item_filesystem_uses_workspace_roots_instead_of_cwd() {
|
||||
let repo = test_abs_path("/repo");
|
||||
|
||||
@@ -49,7 +49,8 @@ fn build_permissions_update_item(
|
||||
}
|
||||
|
||||
let prev = previous?;
|
||||
if prev.permission_profile() == next.permission_profile()
|
||||
let model_visible_permission_profile = next.model_visible_permission_profile();
|
||||
if prev.permission_profile() == model_visible_permission_profile
|
||||
&& prev.approval_policy == next.approval_policy.value()
|
||||
{
|
||||
return None;
|
||||
@@ -57,7 +58,7 @@ fn build_permissions_update_item(
|
||||
|
||||
Some(
|
||||
PermissionsInstructions::from_permission_profile(
|
||||
&next.permission_profile,
|
||||
&model_visible_permission_profile,
|
||||
next.approval_policy.value(),
|
||||
next.config.approvals_reviewer,
|
||||
exec_policy,
|
||||
|
||||
@@ -2892,9 +2892,10 @@ impl Session {
|
||||
developer_sections.push(model_switch_message);
|
||||
}
|
||||
if turn_context.config.include_permissions_instructions {
|
||||
let model_visible_permission_profile = turn_context.model_visible_permission_profile();
|
||||
developer_sections.push(
|
||||
PermissionsInstructions::from_permission_profile(
|
||||
&turn_context.permission_profile,
|
||||
&model_visible_permission_profile,
|
||||
turn_context.approval_policy.value(),
|
||||
turn_context.config.approvals_reviewer,
|
||||
self.services.exec_policy.current().as_ref(),
|
||||
|
||||
@@ -9,7 +9,10 @@ use codex_model_provider::create_model_provider;
|
||||
use codex_protocol::SessionId;
|
||||
use codex_protocol::ThreadId;
|
||||
use codex_protocol::models::AdditionalPermissionProfile;
|
||||
use codex_protocol::models::ManagedFileSystemPermissions;
|
||||
use codex_protocol::openai_models::ModelInfo;
|
||||
use codex_protocol::permissions::FileSystemAccessMode;
|
||||
use codex_protocol::permissions::FileSystemPath;
|
||||
use codex_protocol::protocol::MultiAgentVersion;
|
||||
use codex_protocol::protocol::TurnEnvironmentSelection;
|
||||
use codex_sandboxing::compatibility_sandbox_policy_for_permission_profile;
|
||||
@@ -40,6 +43,31 @@ impl TurnSkillsContext {
|
||||
|
||||
pub(crate) type ShellSnapshotTask = Shared<BoxFuture<'static, Option<Arc<ShellSnapshotFile>>>>;
|
||||
|
||||
pub(crate) fn model_visible_permission_profile(
|
||||
permission_profile: &PermissionProfile,
|
||||
enforcement_only_paths: &[AbsolutePathBuf],
|
||||
) -> PermissionProfile {
|
||||
let enforcement_only_paths = enforcement_only_paths
|
||||
.iter()
|
||||
.cloned()
|
||||
.collect::<HashSet<_>>();
|
||||
let mut permission_profile = permission_profile.clone();
|
||||
if let PermissionProfile::Managed {
|
||||
file_system: ManagedFileSystemPermissions::Restricted { entries, .. },
|
||||
..
|
||||
} = &mut permission_profile
|
||||
{
|
||||
entries.retain(|entry| {
|
||||
entry.access != FileSystemAccessMode::Deny
|
||||
|| !matches!(
|
||||
&entry.path,
|
||||
FileSystemPath::Path { path } if enforcement_only_paths.contains(path)
|
||||
)
|
||||
});
|
||||
}
|
||||
permission_profile
|
||||
}
|
||||
|
||||
#[derive(Clone)]
|
||||
pub(crate) struct TurnEnvironment {
|
||||
pub(crate) environment_id: String,
|
||||
@@ -154,6 +182,13 @@ impl TurnContext {
|
||||
self.permission_profile.clone()
|
||||
}
|
||||
|
||||
pub(crate) fn model_visible_permission_profile(&self) -> PermissionProfile {
|
||||
model_visible_permission_profile(
|
||||
&self.permission_profile,
|
||||
&self.config.workload_identity_credential_deny_paths,
|
||||
)
|
||||
}
|
||||
|
||||
pub(crate) fn file_system_sandbox_policy(&self) -> FileSystemSandboxPolicy {
|
||||
self.permission_profile.file_system_sandbox_policy()
|
||||
}
|
||||
@@ -367,7 +402,7 @@ impl TurnContext {
|
||||
timezone: self.timezone.clone(),
|
||||
approval_policy: self.approval_policy.value(),
|
||||
sandbox_policy: self.sandbox_policy(),
|
||||
permission_profile: Some(self.permission_profile()),
|
||||
permission_profile: Some(self.model_visible_permission_profile()),
|
||||
network: self.turn_context_network_item(),
|
||||
file_system_sandbox_policy: self.non_legacy_file_system_sandbox_policy(),
|
||||
model: self.model_info.slug.clone(),
|
||||
|
||||
@@ -30,6 +30,7 @@ use codex_extension_api::UserInstructionsProvider;
|
||||
use codex_extension_api::empty_extension_registry;
|
||||
use codex_features::Feature;
|
||||
use codex_home::CodexHomeUserInstructionsProvider;
|
||||
use codex_login::AuthManager;
|
||||
use codex_login::CodexAuth;
|
||||
use codex_model_provider_info::ModelProviderInfo;
|
||||
use codex_model_provider_info::built_in_model_providers;
|
||||
@@ -250,6 +251,7 @@ pub fn turn_permission_fields(
|
||||
pub struct TestCodexBuilder {
|
||||
config_mutators: Vec<Box<ConfigMutator>>,
|
||||
auth: CodexAuth,
|
||||
auth_manager_from_config: bool,
|
||||
pre_build_hooks: Vec<Box<PreBuildHook>>,
|
||||
workspace_setups: Vec<Box<WorkspaceSetup>>,
|
||||
home: Option<Arc<TempDir>>,
|
||||
@@ -274,6 +276,11 @@ impl TestCodexBuilder {
|
||||
self
|
||||
}
|
||||
|
||||
pub fn with_auth_manager_from_config(mut self) -> Self {
|
||||
self.auth_manager_from_config = true;
|
||||
self
|
||||
}
|
||||
|
||||
pub fn with_model(self, model: &str) -> Self {
|
||||
let new_model = model.to_string();
|
||||
self.with_config(move |config| {
|
||||
@@ -539,6 +546,11 @@ impl TestCodexBuilder {
|
||||
environment_manager: Arc<codex_exec_server::EnvironmentManager>,
|
||||
) -> anyhow::Result<TestCodex> {
|
||||
let auth = self.auth.clone();
|
||||
let configured_auth_manager = if self.auth_manager_from_config {
|
||||
Some(AuthManager::shared_from_config(&config, /*enable_codex_api_key_env*/ false).await)
|
||||
} else {
|
||||
None
|
||||
};
|
||||
let state_db = codex_core::init_state_db(&config).await;
|
||||
let thread_store = thread_store_from_config(&config, state_db.clone());
|
||||
let installation_id = resolve_installation_id(&config.codex_home).await?;
|
||||
@@ -550,7 +562,9 @@ impl TestCodexBuilder {
|
||||
});
|
||||
let thread_manager = ThreadManager::new(
|
||||
&config,
|
||||
codex_core::test_support::auth_manager_from_auth(auth.clone()),
|
||||
configured_auth_manager
|
||||
.clone()
|
||||
.unwrap_or_else(|| codex_core::test_support::auth_manager_from_auth(auth.clone())),
|
||||
SessionSource::Exec,
|
||||
Arc::clone(&environment_manager),
|
||||
Arc::clone(&self.extensions),
|
||||
@@ -566,7 +580,9 @@ impl TestCodexBuilder {
|
||||
|
||||
let new_conversation = match (resume_from, user_shell_override) {
|
||||
(Some(path), Some(user_shell_override)) => {
|
||||
let auth_manager = codex_core::test_support::auth_manager_from_auth(auth);
|
||||
let auth_manager = configured_auth_manager.clone().unwrap_or_else(|| {
|
||||
codex_core::test_support::auth_manager_from_auth(auth.clone())
|
||||
});
|
||||
Box::pin(
|
||||
codex_core::test_support::resume_thread_from_rollout_with_user_shell_override(
|
||||
thread_manager.as_ref(),
|
||||
@@ -579,7 +595,8 @@ impl TestCodexBuilder {
|
||||
.await?
|
||||
}
|
||||
(Some(path), None) => {
|
||||
let auth_manager = codex_core::test_support::auth_manager_from_auth(auth);
|
||||
let auth_manager = configured_auth_manager
|
||||
.unwrap_or_else(|| codex_core::test_support::auth_manager_from_auth(auth));
|
||||
Box::pin(thread_manager.resume_thread_from_rollout(
|
||||
config.clone(),
|
||||
path,
|
||||
@@ -1135,6 +1152,7 @@ pub fn test_codex() -> TestCodexBuilder {
|
||||
.expect("test config should allow Apps override");
|
||||
})],
|
||||
auth: CodexAuth::from_api_key("dummy"),
|
||||
auth_manager_from_config: false,
|
||||
pre_build_hooks: vec![],
|
||||
workspace_setups: vec![],
|
||||
home: None,
|
||||
|
||||
@@ -130,3 +130,4 @@ mod websocket_fallback;
|
||||
mod window_headers;
|
||||
#[cfg(target_os = "windows")]
|
||||
mod windows_sandbox;
|
||||
mod workload_identity;
|
||||
|
||||
@@ -0,0 +1,186 @@
|
||||
use std::path::PathBuf;
|
||||
|
||||
use anyhow::Result;
|
||||
use anyhow::bail;
|
||||
use base64::Engine;
|
||||
use codex_protocol::permissions::FileSystemAccessMode;
|
||||
use codex_protocol::permissions::FileSystemPath;
|
||||
use codex_workload_identity::CredentialSourceConfig;
|
||||
use codex_workload_identity::WorkloadIdentityConfig;
|
||||
use core_test_support::responses;
|
||||
use core_test_support::skip_if_no_network;
|
||||
use core_test_support::test_codex::TestCodexBuilder;
|
||||
use core_test_support::test_codex::test_codex;
|
||||
use pretty_assertions::assert_eq;
|
||||
use serde::Serialize;
|
||||
use serde_json::json;
|
||||
use tempfile::TempDir;
|
||||
use wiremock::Mock;
|
||||
use wiremock::MockServer;
|
||||
use wiremock::ResponseTemplate;
|
||||
use wiremock::matchers::method;
|
||||
use wiremock::matchers::path;
|
||||
|
||||
const ACCESS_TOKEN_TYPE: &str = "urn:ietf:params:oauth:token-type:access_token";
|
||||
|
||||
#[derive(Serialize)]
|
||||
struct TestConfigToml {
|
||||
workload_identity: WorkloadIdentityConfig,
|
||||
}
|
||||
|
||||
fn workload_identity_config_toml(server: &MockServer, token_path: PathBuf) -> Result<String> {
|
||||
Ok(toml::to_string(&TestConfigToml {
|
||||
workload_identity: WorkloadIdentityConfig {
|
||||
identity_provider_id: "idp_test".to_string(),
|
||||
identity_provider_mapping_id: "idpm_test".to_string(),
|
||||
audience: "api://codex-test".to_string(),
|
||||
token_url: format!("{}/oauth/token", server.uri()),
|
||||
credential_source: CredentialSourceConfig::File { path: token_path },
|
||||
},
|
||||
})?)
|
||||
}
|
||||
|
||||
fn test_codex_with_workload_identity(config_toml: String) -> TestCodexBuilder {
|
||||
test_codex()
|
||||
.with_auth_manager_from_config()
|
||||
.with_pre_build_hook(move |codex_home| {
|
||||
std::fs::write(codex_home.join("config.toml"), config_toml)
|
||||
.expect("write workload identity test config");
|
||||
})
|
||||
}
|
||||
|
||||
fn test_access_token() -> String {
|
||||
let payload = base64::engine::general_purpose::URL_SAFE_NO_PAD.encode(
|
||||
json!({
|
||||
"email": "workload@example.com",
|
||||
"https://api.openai.com/auth": {
|
||||
"chatgpt_account_id": "workspace_test",
|
||||
"chatgpt_plan_type": "enterprise",
|
||||
"chatgpt_user_id": "user_test",
|
||||
}
|
||||
})
|
||||
.to_string(),
|
||||
);
|
||||
format!("header.{payload}.signature")
|
||||
}
|
||||
|
||||
fn exchange_response(access_token: &str) -> ResponseTemplate {
|
||||
ResponseTemplate::new(200).set_body_json(json!({
|
||||
"access_token": access_token,
|
||||
"issued_token_type": ACCESS_TOKEN_TYPE,
|
||||
"token_type": "Bearer",
|
||||
"expires_in": 3600,
|
||||
"user_id": "user_test",
|
||||
"chatgpt_account_id": "workspace_test",
|
||||
"chatgpt_plan_type": "enterprise",
|
||||
}))
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn configured_workload_identity_authenticates_responses_turn() -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
|
||||
let server = MockServer::start().await;
|
||||
let credential_dir = TempDir::new()?;
|
||||
let token_path = credential_dir.path().join("subject-token");
|
||||
std::fs::write(&token_path, "external-subject-token\n")?;
|
||||
let access_token = test_access_token();
|
||||
Mock::given(method("POST"))
|
||||
.and(path("/oauth/token"))
|
||||
.respond_with(exchange_response(&access_token))
|
||||
.expect(1)
|
||||
.mount(&server)
|
||||
.await;
|
||||
let response_mock = responses::mount_sse_once(
|
||||
&server,
|
||||
responses::sse(vec![
|
||||
responses::ev_response_created("resp-1"),
|
||||
responses::ev_assistant_message("msg-1", "authenticated"),
|
||||
responses::ev_completed("resp-1"),
|
||||
]),
|
||||
)
|
||||
.await;
|
||||
|
||||
let config_toml = workload_identity_config_toml(&server, token_path.clone())?;
|
||||
let mut builder = test_codex_with_workload_identity(config_toml);
|
||||
let test = builder.build(&server).await?;
|
||||
test.submit_turn("authenticate with workload identity")
|
||||
.await?;
|
||||
|
||||
assert!(
|
||||
test.config
|
||||
.permissions
|
||||
.file_system_sandbox_policy()
|
||||
.entries
|
||||
.iter()
|
||||
.any(|entry| {
|
||||
entry.access == FileSystemAccessMode::Deny
|
||||
&& matches!(
|
||||
&entry.path,
|
||||
FileSystemPath::Path { path } if path.as_path() == token_path.as_path()
|
||||
)
|
||||
}),
|
||||
"the workload credential must remain denied by the runtime sandbox"
|
||||
);
|
||||
let request = response_mock.single_request();
|
||||
assert_eq!(
|
||||
request.header("authorization"),
|
||||
Some(format!("Bearer {access_token}"))
|
||||
);
|
||||
assert!(
|
||||
!request
|
||||
.body_json()
|
||||
.to_string()
|
||||
.contains(token_path.to_string_lossy().as_ref()),
|
||||
"the model-visible request must not reveal the workload credential path"
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn required_workload_identity_failure_blocks_session_startup() -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
|
||||
let server = MockServer::start().await;
|
||||
let credential_dir = TempDir::new()?;
|
||||
let missing_token_path = credential_dir.path().join("missing-subject-token");
|
||||
let config_toml = workload_identity_config_toml(&server, missing_token_path)?;
|
||||
let mut builder = test_codex_with_workload_identity(config_toml);
|
||||
|
||||
let error = match builder.build(&server).await {
|
||||
Ok(_) => bail!("required workload identity should block session startup"),
|
||||
Err(error) => error,
|
||||
};
|
||||
assert!(
|
||||
format!("{error:#}").contains("file credential source could not read"),
|
||||
"unexpected startup error: {error:#}"
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn unauthenticated_provider_skips_unavailable_workload_identity() -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
|
||||
let server = MockServer::start().await;
|
||||
let credential_dir = TempDir::new()?;
|
||||
let missing_token_path = credential_dir.path().join("missing-subject-token");
|
||||
let config_toml = workload_identity_config_toml(&server, missing_token_path)?;
|
||||
let response_mock = responses::mount_sse_once(
|
||||
&server,
|
||||
responses::sse(vec![
|
||||
responses::ev_response_created("resp-1"),
|
||||
responses::ev_assistant_message("msg-1", "unauthenticated"),
|
||||
responses::ev_completed("resp-1"),
|
||||
]),
|
||||
)
|
||||
.await;
|
||||
let mut builder = test_codex_with_workload_identity(config_toml).with_config(|config| {
|
||||
config.model_provider.requires_openai_auth = false;
|
||||
});
|
||||
let test = builder.build(&server).await?;
|
||||
test.submit_turn("use the unauthenticated provider").await?;
|
||||
|
||||
assert_eq!(response_mock.single_request().header("authorization"), None);
|
||||
Ok(())
|
||||
}
|
||||
@@ -568,7 +568,7 @@ struct TestExternalChatgptAuth {
|
||||
refresh_count: AtomicUsize,
|
||||
}
|
||||
|
||||
struct FailingExternalChatgptAuth;
|
||||
struct FailingExternalChatgptAuth(&'static str);
|
||||
|
||||
impl ExternalAuth for FailingExternalChatgptAuth {
|
||||
fn auth_mode(&self) -> AuthMode {
|
||||
@@ -580,14 +580,16 @@ impl ExternalAuth for FailingExternalChatgptAuth {
|
||||
}
|
||||
|
||||
fn resolve(&self) -> ExternalAuthFuture<'_, Option<ExternalAuthTokens>> {
|
||||
Box::pin(async { Err(std::io::Error::other("external token exchange failed")) })
|
||||
let message = self.0;
|
||||
Box::pin(async move { Err(std::io::Error::other(message)) })
|
||||
}
|
||||
|
||||
fn refresh(
|
||||
&self,
|
||||
_context: ExternalAuthRefreshContext,
|
||||
) -> ExternalAuthFuture<'_, ExternalAuthTokens> {
|
||||
Box::pin(async { Err(std::io::Error::other("external token exchange failed")) })
|
||||
let message = self.0;
|
||||
Box::pin(async move { Err(std::io::Error::other(message)) })
|
||||
}
|
||||
}
|
||||
|
||||
@@ -695,6 +697,11 @@ async fn external_chatgpt_auth_resolves_and_refreshes_without_writing_auth_file(
|
||||
manager.auth_cached().unwrap().get_token().unwrap(),
|
||||
refreshed_token
|
||||
);
|
||||
manager.reload().await;
|
||||
assert_eq!(
|
||||
manager.auth_cached().unwrap().get_token().unwrap(),
|
||||
refreshed_token
|
||||
);
|
||||
assert!(!get_auth_file(codex_home.path()).exists());
|
||||
}
|
||||
|
||||
@@ -756,7 +763,9 @@ async fn auth_result_preserves_external_auth_resolution_errors() {
|
||||
AuthKeyringBackendKind::default(),
|
||||
)
|
||||
.await;
|
||||
manager.set_external_auth(Arc::new(FailingExternalChatgptAuth));
|
||||
manager.set_external_auth(Arc::new(FailingExternalChatgptAuth(
|
||||
"external token exchange failed",
|
||||
)));
|
||||
|
||||
assert_eq!(manager.auth_mode(), Some(AuthMode::Chatgpt));
|
||||
assert_eq!(
|
||||
@@ -772,6 +781,35 @@ async fn auth_result_preserves_external_auth_resolution_errors() {
|
||||
assert!(manager.auth().await.is_err());
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn set_external_auth_if_absent_preserves_existing_provider() {
|
||||
let manager = AuthManager::shared(
|
||||
PathBuf::from("non-existent"),
|
||||
/*enable_codex_api_key_env*/ false,
|
||||
AuthCredentialsStoreMode::File,
|
||||
/*chatgpt_base_url*/ None,
|
||||
AuthKeyringBackendKind::default(),
|
||||
)
|
||||
.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",
|
||||
)))
|
||||
);
|
||||
|
||||
let error = manager
|
||||
.auth()
|
||||
.await
|
||||
.expect_err("the original external auth should remain installed");
|
||||
assert_eq!(error.to_string(), "original external auth");
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn external_bearer_only_auth_manager_uses_cached_provider_token() {
|
||||
let script = ProviderAuthScript::new(&["provider-token", "next-token"]).unwrap();
|
||||
|
||||
@@ -1879,12 +1879,14 @@ impl AuthManager {
|
||||
}
|
||||
}
|
||||
|
||||
pub fn set_external_auth_if_absent(&self, external_auth: Arc<dyn ExternalAuth>) {
|
||||
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()
|
||||
{
|
||||
*guard = Some(external_auth);
|
||||
return true;
|
||||
}
|
||||
false
|
||||
}
|
||||
|
||||
pub fn clear_external_auth(&self) {
|
||||
@@ -2258,6 +2260,20 @@ impl AuthManager {
|
||||
.external_chatgpt_auth(&refreshed)
|
||||
.await
|
||||
.map_err(RefreshTokenError::Transient)?;
|
||||
if !external_auth.requires_successful_resolution() {
|
||||
let chatgpt_metadata = refreshed.chatgpt_metadata().ok_or_else(|| {
|
||||
RefreshTokenError::Transient(std::io::Error::other(
|
||||
"external auth tokens are missing ChatGPT metadata",
|
||||
))
|
||||
})?;
|
||||
login_with_chatgpt_auth_tokens(
|
||||
&self.codex_home,
|
||||
&refreshed.access_token,
|
||||
&chatgpt_metadata.account_id,
|
||||
chatgpt_metadata.plan_type.as_deref(),
|
||||
)
|
||||
.map_err(RefreshTokenError::Transient)?;
|
||||
}
|
||||
self.set_cached_auth(Some(auth));
|
||||
Ok(())
|
||||
}
|
||||
|
||||
@@ -270,6 +270,7 @@ fn new_config(model: Option<String>, arg0_paths: Arg0DispatchPaths) -> anyhow::R
|
||||
forced_chatgpt_workspace_id: None,
|
||||
forced_login_method: None,
|
||||
workload_identity: None,
|
||||
workload_identity_credential_deny_paths: Vec::new(),
|
||||
web_search_mode: Constrained::allow_any(WebSearchMode::Disabled),
|
||||
web_search_config: None,
|
||||
experimental_request_user_input_enabled: true,
|
||||
|
||||
@@ -2,6 +2,11 @@ load("//:defs.bzl", "codex_rust_crate")
|
||||
|
||||
codex_rust_crate(
|
||||
name = "workload-identity-providers",
|
||||
crate_features = ["azure"],
|
||||
crate_features = [
|
||||
"aws",
|
||||
"azure",
|
||||
"github-actions",
|
||||
"spiffe",
|
||||
],
|
||||
crate_name = "codex_workload_identity_providers",
|
||||
)
|
||||
|
||||
Reference in New Issue
Block a user