mirror of
https://github.com/openai/codex.git
synced 2026-11-07 20:30:33 +00:00
Fix authentication for git-attribution settings requests (#60035)
## Why Git-attribution settings requests need to honor runtime authentication for their destination. Repeated authorization failures after a credential refresh must also let world-state contribution finish. ## What changed - Use the thread's `ProviderAuthContext` credentials and runtime authentication when available. - Bind the settings client's network policy to the authenticated account and select authentication through the runtime-aware backend client. - Stop policy resolution after an unsuccessful settings lookup or timeout instead of restarting it when credentials changed during recovery. ## Testing Add app-server coverage for `dual_header` and `assertion_only` authentication with default and custom destinations, including attribution instructions in model input. Add a regression test verifying that repeated `401` responses trigger only one credential refresh and omit attribution instructions. GitOrigin-RevId: 4824bde7d9ec11c241f1caebc754d203dbb0480c
This commit is contained in:
Generated
+3
@@ -3645,8 +3645,11 @@ version = "0.0.0"
|
||||
dependencies = [
|
||||
"codex-backend-client",
|
||||
"codex-extension-api",
|
||||
"codex-features",
|
||||
"codex-http-client",
|
||||
"codex-login",
|
||||
"codex-model-provider",
|
||||
"codex-runtime-agent-instance",
|
||||
"serde_json",
|
||||
"tokio",
|
||||
"wiremock",
|
||||
|
||||
@@ -64,6 +64,155 @@ enum LegacyAttribution {
|
||||
UnlinkedPullRequest,
|
||||
}
|
||||
|
||||
#[test_case("dual_header", false; "dual_header")]
|
||||
#[test_case("assertion_only", false; "assertion_only")]
|
||||
#[test_case("dual_header", true; "dual_header_custom_destination")]
|
||||
#[test_case("assertion_only", true; "assertion_only_custom_destination")]
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn git_attribution_runtime_auth_uses_request_destination(
|
||||
mode: &str,
|
||||
custom_destination: bool,
|
||||
) -> Result<()> {
|
||||
use base64::Engine;
|
||||
use core_test_support::runtime_auth::PROBE_SERVER;
|
||||
use core_test_support::runtime_auth::run_harness_through_proxy;
|
||||
|
||||
skip_if_no_network!(Ok(()));
|
||||
let token = format!(
|
||||
"e30.{}.c2ln",
|
||||
base64::engine::general_purpose::URL_SAFE_NO_PAD.encode(serde_json::to_vec(&json!({
|
||||
"exp": chrono::Utc::now().timestamp() + 3600,
|
||||
"session_id": "session-a", "iss": "https://auth.openai.com",
|
||||
"https://api.openai.com/auth": {
|
||||
"chatgpt_user_id": "user-123", "chatgpt_account_id": "workspace-enabled",
|
||||
},
|
||||
}))?)
|
||||
);
|
||||
if let Ok(server_url) = std::env::var(PROBE_SERVER) {
|
||||
let home = TempDir::new()?;
|
||||
let base_url = if custom_destination {
|
||||
format!("{server_url}/backend-api")
|
||||
} else {
|
||||
"https://chatgpt.com/backend-api".into()
|
||||
};
|
||||
write_mock_responses_config_toml_with_chatgpt_base_url(
|
||||
home.path(),
|
||||
&server_url,
|
||||
&base_url,
|
||||
)?;
|
||||
let config_path = home.path().join("config.toml");
|
||||
let config = std::fs::read_to_string(&config_path)?;
|
||||
std::fs::write(
|
||||
config_path,
|
||||
format!(
|
||||
"{config}\n[features.runtime_agent_instances]\nenabled = {}\nmode = {mode:?}\n",
|
||||
!custom_destination
|
||||
),
|
||||
)?;
|
||||
write_chatgpt_auth(
|
||||
home.path(),
|
||||
ChatGptAuthFixture::new(&token)
|
||||
.account_id("workspace-enabled")
|
||||
.chatgpt_user_id("user-123")
|
||||
.plan_type("enterprise"),
|
||||
AuthCredentialsStoreMode::File,
|
||||
)?;
|
||||
let mut app = TestAppServer::builder()
|
||||
.with_codex_home(home.path())
|
||||
.with_env_overrides(&[("OPENAI_API_KEY", None), ("CODEX_ACCESS_TOKEN", None)])
|
||||
.build_initialized_with_timeout(DEFAULT_READ_TIMEOUT)
|
||||
.await?;
|
||||
let id = app
|
||||
.send_thread_start_request_with_auto_env(ThreadStartParams {
|
||||
// Extensions retain the harness URL, even when a thread overrides it.
|
||||
config: Some(HashMap::from([
|
||||
(
|
||||
"chatgpt_base_url".into(),
|
||||
json!("https://chatgpt.com/backend-api"),
|
||||
),
|
||||
(
|
||||
"features.runtime_agent_instances".into(),
|
||||
json!({"enabled": true, "mode": mode}),
|
||||
),
|
||||
])),
|
||||
..Default::default()
|
||||
})
|
||||
.await?;
|
||||
let ThreadStartResponse { thread, .. } = read_response(&mut app, id).await?;
|
||||
run_turn(&mut app, &thread.id, "Hello").await?;
|
||||
return Ok(());
|
||||
}
|
||||
|
||||
let server = responses::start_mock_server().await;
|
||||
let response_mock = responses::mount_sse_sequence(
|
||||
&server,
|
||||
vec![create_final_assistant_message_sse_response("Done")?],
|
||||
)
|
||||
.await;
|
||||
Mock::given(method("GET"))
|
||||
.and(path("/backend-api/wham/config/bundle"))
|
||||
.respond_with(ResponseTemplate::new(200).set_body_json(json!({})))
|
||||
.mount(&server)
|
||||
.await;
|
||||
Mock::given(method("POST"))
|
||||
.and(path("/agent/register"))
|
||||
.respond_with(ResponseTemplate::new(200).set_body_json(json!({
|
||||
"runtime_agent_instance_id": "runtime-a", "agent_host_id": "host-a",
|
||||
"actor": {"kind": "chatgpt", "user_id": "user-123", "account_id": "workspace-enabled"},
|
||||
})))
|
||||
.expect(1)
|
||||
.mount(&server)
|
||||
.await;
|
||||
Mock::given(method("GET"))
|
||||
.and(path("/backend-api/wham/settings/user"))
|
||||
.respond_with(ResponseTemplate::new(200).set_body_json(json!({
|
||||
"commit_attribution_enabled": true,
|
||||
})))
|
||||
.expect(1)
|
||||
.mount(&server)
|
||||
.await;
|
||||
let case = if custom_destination {
|
||||
format!("{mode}_custom_destination")
|
||||
} else {
|
||||
mode.into()
|
||||
};
|
||||
run_harness_through_proxy(
|
||||
&server,
|
||||
&format!("suite::v2::git_attribution::git_attribution_runtime_auth_uses_request_destination::{case}"),
|
||||
/*realtime_target*/ None,
|
||||
)
|
||||
.await?;
|
||||
|
||||
let requests = server
|
||||
.received_requests()
|
||||
.await
|
||||
.context("captured requests")?;
|
||||
let settings = requests
|
||||
.iter()
|
||||
.find(|request| request.url.path() == "/backend-api/wham/settings/user")
|
||||
.context("git-attribution settings request")?;
|
||||
let authorization = settings.headers["authorization"].to_str()?;
|
||||
if custom_destination || mode == "dual_header" {
|
||||
assert!(authorization.starts_with("Bearer e30."));
|
||||
} else {
|
||||
assert!(authorization.starts_with("AgentAssertion 1."));
|
||||
}
|
||||
assert_eq!(
|
||||
settings.headers.contains_key("x-openai-agent-assertion"),
|
||||
!custom_destination && mode == "dual_header",
|
||||
);
|
||||
let model_requests = response_mock.requests();
|
||||
assert_eq!(model_requests.len(), 1);
|
||||
assert!(
|
||||
model_requests[0]
|
||||
.message_input_texts("developer")
|
||||
.join("\n")
|
||||
.contains(COMMIT_ATTRIBUTION)
|
||||
);
|
||||
server.verify().await;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn git_attribution_follows_authenticated_workspace_policy() -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
|
||||
@@ -15,8 +15,11 @@ workspace = true
|
||||
[dependencies]
|
||||
codex-backend-client = { workspace = true }
|
||||
codex-extension-api = { workspace = true }
|
||||
codex-features = { workspace = true }
|
||||
codex-http-client = { workspace = true }
|
||||
codex-login = { workspace = true }
|
||||
codex-model-provider = { workspace = true }
|
||||
codex-runtime-agent-instance = { workspace = true }
|
||||
serde_json = { workspace = true }
|
||||
tokio = { workspace = true, features = ["time"] }
|
||||
|
||||
|
||||
@@ -78,6 +78,7 @@ async fn policy_resolution_recovers_after_unauthorized() {
|
||||
&auth_manager,
|
||||
&format!("{}/backend-api", server.uri()),
|
||||
&http_client_factory(),
|
||||
/*runtime_auth*/ None,
|
||||
)
|
||||
.await
|
||||
.expect("policy resolution should not time out")
|
||||
@@ -121,10 +122,15 @@ async fn policy_resolution_retries_after_auth_refresh() {
|
||||
let auth_manager = auth_manager.clone();
|
||||
let base_url = format!("{}/backend-api", server.uri());
|
||||
async move {
|
||||
resolve_attribution_policy(&auth_manager, &base_url, &http_client_factory())
|
||||
.await
|
||||
.ok()
|
||||
.flatten()
|
||||
resolve_attribution_policy(
|
||||
&auth_manager,
|
||||
&base_url,
|
||||
&http_client_factory(),
|
||||
/*runtime_auth*/ None,
|
||||
)
|
||||
.await
|
||||
.ok()
|
||||
.flatten()
|
||||
}
|
||||
});
|
||||
|
||||
@@ -142,3 +148,89 @@ async fn policy_resolution_retries_after_auth_refresh() {
|
||||
);
|
||||
server.verify().await;
|
||||
}
|
||||
|
||||
struct RotatingExternalAuth(AtomicUsize);
|
||||
|
||||
impl ExternalAuth for RotatingExternalAuth {
|
||||
fn resolve(&self) -> ExternalAuthFuture<'_, CodexAuth> {
|
||||
Box::pin(async {
|
||||
let generation = self.0.load(Ordering::SeqCst);
|
||||
CodexAuth::from_external_chatgpt_tokens(
|
||||
&format!("e30.e30.signature-{generation}"),
|
||||
"workspace-123",
|
||||
Some("enterprise"),
|
||||
)
|
||||
})
|
||||
}
|
||||
|
||||
fn refresh(&self, _context: ExternalAuthRefreshContext) -> ExternalAuthFuture<'_, CodexAuth> {
|
||||
self.0.fetch_add(/*val*/ 1, Ordering::SeqCst);
|
||||
self.resolve()
|
||||
}
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn world_state_stops_after_rejected_refresh() {
|
||||
use codex_extension_api::ContextContributor;
|
||||
use codex_extension_api::ExtensionData;
|
||||
use codex_extension_api::PreviousWorldStateSection;
|
||||
use codex_extension_api::WorldStateContributionInput;
|
||||
|
||||
let server = MockServer::start().await;
|
||||
Mock::given(method("GET"))
|
||||
.and(path("/backend-api/wham/settings/user"))
|
||||
.respond_with(ResponseTemplate::new(401))
|
||||
.expect(2)
|
||||
.mount(&server)
|
||||
.await;
|
||||
let source = Arc::new(RotatingExternalAuth(AtomicUsize::new(/*v*/ 0)));
|
||||
let auth_manager = enterprise_auth_manager();
|
||||
auth_manager
|
||||
.set_external_auth(source.clone())
|
||||
.await
|
||||
.expect("external credentials should load");
|
||||
let extension = super::GitAttributionExtension {
|
||||
auth_manager,
|
||||
base_url: format!("{}/backend-api", server.uri()),
|
||||
http_client_factory: http_client_factory(),
|
||||
};
|
||||
let model_info = serde_json::from_value(serde_json::json!({
|
||||
"slug": "test-model",
|
||||
"display_name": "Test model",
|
||||
"supported_reasoning_levels": [],
|
||||
"shell_type": "shell_command",
|
||||
"visibility": "list",
|
||||
"supported_in_api": true,
|
||||
"priority": 0,
|
||||
"support_verbosity": false,
|
||||
"truncation_policy": {"mode": "bytes", "limit": 10000},
|
||||
"experimental_supported_tools": []
|
||||
}))
|
||||
.expect("model metadata should parse");
|
||||
let store = ExtensionData::new("test");
|
||||
let sections = tokio::time::timeout(
|
||||
Duration::from_secs(2),
|
||||
extension.contribute_world_state(WorldStateContributionInput {
|
||||
thread_id: Default::default(),
|
||||
turn_id: "turn",
|
||||
model_info: &model_info,
|
||||
environments: &[],
|
||||
extension_metrics: None,
|
||||
session_store: &store,
|
||||
thread_store: &store,
|
||||
turn_store: &store,
|
||||
step_store: &store,
|
||||
previous_world_state: None,
|
||||
}),
|
||||
)
|
||||
.await
|
||||
.expect("an optional settings failure must not keep refreshing credentials");
|
||||
assert_eq!(source.0.load(Ordering::SeqCst), 1);
|
||||
assert!(sections.iter().all(|section| {
|
||||
section
|
||||
.render_diff(PreviousWorldStateSection::Absent)
|
||||
.1
|
||||
.is_none()
|
||||
}));
|
||||
server.verify().await;
|
||||
}
|
||||
|
||||
@@ -11,6 +11,7 @@ use codex_extension_api::WorldStateContributionInput;
|
||||
use codex_extension_api::WorldStateSectionContribution;
|
||||
use codex_http_client::HttpClientFactory;
|
||||
use codex_login::AuthManager;
|
||||
use codex_model_provider::ProviderAuthContext;
|
||||
|
||||
use crate::policy::GitAttributionPolicy;
|
||||
use crate::policy::GitAttributionRetry;
|
||||
@@ -35,8 +36,15 @@ impl ContextContributor for GitAttributionExtension {
|
||||
input: WorldStateContributionInput<'a>,
|
||||
) -> ExtensionFuture<'a, Vec<WorldStateSectionContribution>> {
|
||||
Box::pin(async move {
|
||||
let auth_context = input.thread_store.get::<ProviderAuthContext>();
|
||||
let auth_manager = auth_context
|
||||
.as_ref()
|
||||
.map_or(&self.auth_manager, |context| &context.credentials);
|
||||
let runtime_auth = auth_context
|
||||
.as_ref()
|
||||
.and_then(|context| context.runtime_auth.as_ref());
|
||||
let enabled = loop {
|
||||
let current_auth_generation = auth_generation(self.auth_manager.as_ref());
|
||||
let current_auth_generation = auth_generation(auth_manager.as_ref());
|
||||
let policy = match cached_attribution_policy(
|
||||
input.thread_store,
|
||||
input.turn_store,
|
||||
@@ -51,9 +59,10 @@ impl ContextContributor for GitAttributionExtension {
|
||||
}
|
||||
None => {
|
||||
match resolve_attribution_policy(
|
||||
&self.auth_manager,
|
||||
auth_manager,
|
||||
&self.base_url,
|
||||
&self.http_client_factory,
|
||||
runtime_auth,
|
||||
)
|
||||
.await
|
||||
{
|
||||
@@ -62,30 +71,26 @@ impl ContextContributor for GitAttributionExtension {
|
||||
policy
|
||||
}
|
||||
Ok(None) => {
|
||||
let policy = GitAttributionPolicy {
|
||||
input.turn_store.insert(GitAttributionPolicy {
|
||||
auth_generation: current_auth_generation,
|
||||
enabled: false,
|
||||
};
|
||||
input.turn_store.insert(policy.clone());
|
||||
policy
|
||||
});
|
||||
break false;
|
||||
}
|
||||
Err(_) => {
|
||||
let auth_generation = auth_generation(self.auth_manager.as_ref());
|
||||
let auth_generation = auth_generation(auth_manager.as_ref());
|
||||
if auth_generation == current_auth_generation {
|
||||
input.thread_store.insert(GitAttributionRetry {
|
||||
auth_generation,
|
||||
retry_at: Instant::now() + POLICY_RETRY_DELAY,
|
||||
});
|
||||
}
|
||||
GitAttributionPolicy {
|
||||
auth_generation: current_auth_generation,
|
||||
enabled: false,
|
||||
}
|
||||
break false;
|
||||
}
|
||||
}
|
||||
}
|
||||
};
|
||||
if policy.auth_generation == auth_generation(self.auth_manager.as_ref()) {
|
||||
if policy.auth_generation == auth_generation(auth_manager.as_ref()) {
|
||||
break policy.enabled;
|
||||
}
|
||||
};
|
||||
|
||||
@@ -4,8 +4,10 @@ use std::time::Instant;
|
||||
|
||||
use codex_backend_client::Client as BackendClient;
|
||||
use codex_extension_api::ExtensionData;
|
||||
use codex_features::RuntimeAuthMode;
|
||||
use codex_http_client::HttpClientFactory;
|
||||
use codex_login::AuthManager;
|
||||
use codex_runtime_agent_instance::RuntimeAuthManager;
|
||||
use tokio::time::timeout;
|
||||
|
||||
#[derive(Clone, Debug)]
|
||||
@@ -53,6 +55,7 @@ pub(super) async fn resolve_attribution_policy(
|
||||
auth_manager: &Arc<AuthManager>,
|
||||
base_url: &str,
|
||||
http_client_factory: &HttpClientFactory,
|
||||
runtime_auth: Option<&(Arc<RuntimeAuthManager>, RuntimeAuthMode)>,
|
||||
) -> Result<Option<GitAttributionPolicy>, tokio::time::error::Elapsed> {
|
||||
timeout(POLICY_RESOLUTION_TIMEOUT, async {
|
||||
let mut recovery_generation = auth_generation(auth_manager);
|
||||
@@ -63,15 +66,28 @@ pub(super) async fn resolve_attribution_policy(
|
||||
auth_recovery = auth_manager.unauthorized_recovery();
|
||||
recovery_generation = auth_generation_at_start;
|
||||
}
|
||||
let auth = auth_manager.auth().await;
|
||||
let auth = auth_manager.auth_with_http_client_factory().await;
|
||||
if auth_generation(auth_manager) != auth_generation_at_start {
|
||||
continue;
|
||||
}
|
||||
let enabled = match auth {
|
||||
Some(auth) if auth.uses_codex_backend() => {
|
||||
let client =
|
||||
BackendClient::from_auth(base_url, &auth, http_client_factory.clone());
|
||||
let settings = client.get_user_settings().await;
|
||||
Some((auth, captured_factory)) if auth.uses_codex_backend() => {
|
||||
let policy = http_client_factory
|
||||
.network_policy()
|
||||
.clone()
|
||||
.with_account_binding(captured_factory.network_policy())?;
|
||||
let client = BackendClient::from_auth_with_runtime(
|
||||
base_url,
|
||||
&auth,
|
||||
http_client_factory.clone().with_network_policy(policy),
|
||||
runtime_auth,
|
||||
/*headers*/ &Default::default(),
|
||||
)
|
||||
.await;
|
||||
if auth_generation(auth_manager) != auth_generation_at_start {
|
||||
continue;
|
||||
}
|
||||
let settings = client.ok()?.get_user_settings().await;
|
||||
if auth_generation(auth_manager) != auth_generation_at_start {
|
||||
continue;
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user