fix(codex): clear stale official auth API key#3904
Conversation
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
This PR refines provider-switching behavior for the “official” Codex provider to avoid unintentionally persisting a stale OPENAI_API_KEY while keeping existing OAuth tokens intact.
Changes:
- Routes official-provider writes through a dedicated
write_codex_official_live_for_providerhelper. - Adds logic to strip a stale
OPENAI_API_KEYfrom existingauth.jsonwhen switching to official with empty auth. - Introduces a serial test with a temp home-dir guard to validate the stale-key removal behavior.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| impl TestHomeGuard { | ||
| fn new() -> Self { | ||
| let dir = tempfile::tempdir().expect("create temp home"); | ||
| let old_test_home = std::env::var_os("CC_SWITCH_TEST_HOME"); | ||
| std::env::set_var("CC_SWITCH_TEST_HOME", dir.path()); | ||
| Self { | ||
| _dir: dir, | ||
| old_test_home, | ||
| } | ||
| } | ||
| } | ||
|
|
||
| impl Drop for TestHomeGuard { | ||
| fn drop(&mut self) { | ||
| match &self.old_test_home { | ||
| Some(value) => std::env::set_var("CC_SWITCH_TEST_HOME", value), | ||
| None => std::env::remove_var("CC_SWITCH_TEST_HOME"), | ||
| } | ||
| } | ||
| } |
| fn write_codex_official_live_for_provider( | ||
| auth: &Value, | ||
| config_text: Option<&str>, | ||
| ) -> Result<(), AppError> { | ||
| if codex_auth_has_login_material(auth) { | ||
| return write_codex_live_atomic(auth, config_text); | ||
| } | ||
|
|
||
| let auth_path = get_codex_auth_path(); | ||
| if auth_path.exists() { | ||
| let mut live_auth: Value = read_json_file(&auth_path)?; | ||
| if let Some(live_auth_obj) = live_auth.as_object_mut() { | ||
| if live_auth_obj.remove("OPENAI_API_KEY").is_some() { | ||
| return write_codex_live_atomic(&live_auth, config_text); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| write_codex_live_config_atomic(config_text) | ||
| } |
|
cc @farion1231 because this PR fixes a small Codex official auth persistence bug and is currently blocked waiting for maintainer-side workflow/review action. The change is intentionally narrow: when the OpenAI Official Codex provider is saved with empty auth, it removes only stale |
Switching to an official Codex provider with empty auth was removing the OPENAI_API_KEY entry even when it was already null, breaking the existing integration test that expects a null key to be preserved. Guard the removal so it only triggers for a real (non-null) stale key.
Summary / 概述
Fixes a Codex OpenAI Official provider save path where clearing
OPENAI_API_KEYfrom the auth JSON editor did not remove the stale key from live~/.codex/auth.json.The official-provider branch previously skipped writing
auth.jsonwhen the submitted auth had no login material. That made an intentional clear a no-op for the live auth file, so reopening the active provider read the old key back from live settings.This change handles official providers separately:
OPENAI_API_KEY.Related Issue / 关联 Issue
Fixes #3903
Screenshots / 截图
N/A; backend persistence fix.
Checklist / 检查清单
pnpm typecheckpasses / 通过 TypeScript 类型检查 (not run; Rust-only change)pnpm format:checkpasses / 通过代码格式检查 (not run; Rust-only change)cargo clippypasses (if Rust code changed) / 通过 Clippy 检查（如修改了 Rust 代码）Validation
cargo fmt --checkcargo test official_provider_empty_auth_clears_stale_live_api_key_without_dropping_oauthcargo test codex_config::tests::cargo clippy --all-targetsNote:
cargo clippy --all-targetsexits successfully. It still reports three pre-existingfield_reassign_with_defaultwarnings in unrelated modules (claude_desktop_config.rsandservices/proxy.rs).