fix: harden auth profile storage

codex-rs/login/src/auth/auth_tests.rs

@@ -795,7 +795,37 @@ fn remove_access_token_env_var() -> EnvVarGuard {
 
 #[tokio::test]
 #[serial(codex_auth_env)]
-async fn load_auth_reads_access_token_from_env() {
+async fn load_auth_ignores_access_token_env_when_env_auth_disabled() {
+    let codex_home = tempdir().unwrap();
+    let expected_record = agent_identity_record(WORKSPACE_ID_ALLOWED);
+    let agent_identity =
+        signed_agent_identity_jwt(&expected_record, json!(expected_record.plan_type))
+            .expect("signed agent identity");
+    let _access_token_guard = EnvVarGuard::set(CODEX_ACCESS_TOKEN_ENV_VAR, &agent_identity);
+
+    let server = MockServer::start().await;
+    let chatgpt_base_url = format!("{}/backend-api", server.uri());
+    let _authapi_guard =
+        EnvVarGuard::set("CODEX_AGENT_IDENTITY_AUTHAPI_BASE_URL", &chatgpt_base_url);
+    let auth = super::load_auth(
+        codex_home.path(),
+        /*enable_codex_api_key_env*/ false,
+        AuthCredentialsStoreMode::File,
+        Some(&chatgpt_base_url),
+    )
+    .await
+    .expect("auth load should succeed");
+
+    assert_eq!(auth, None);
+    assert!(
+        !get_auth_file(codex_home.path()).exists(),
+        "env auth should not write auth.json"
+    );
+}
+
+#[tokio::test]
+#[serial(codex_auth_env)]
+async fn load_auth_reads_access_token_from_env_when_env_auth_enabled() {
     let codex_home = tempdir().unwrap();
     let expected_record = agent_identity_record(WORKSPACE_ID_ALLOWED);
     let agent_identity =
@@ -823,7 +853,7 @@ async fn load_auth_reads_access_token_from_env() {
         EnvVarGuard::set("CODEX_AGENT_IDENTITY_AUTHAPI_BASE_URL", &chatgpt_base_url);
     let auth = super::load_auth(
         codex_home.path(),
-        /*enable_codex_api_key_env*/ false,
+        /*enable_codex_api_key_env*/ true,
         AuthCredentialsStoreMode::File,
         Some(&chatgpt_base_url),
     )
@@ -845,7 +875,29 @@ async fn load_auth_reads_access_token_from_env() {
 
 #[tokio::test]
 #[serial(codex_auth_env)]
-async fn load_auth_reads_personal_access_token_from_env() {
+async fn load_auth_ignores_personal_access_token_env_when_env_auth_disabled() {
+    let codex_home = tempdir().unwrap();
+    let _access_token_guard = EnvVarGuard::set(CODEX_ACCESS_TOKEN_ENV_VAR, "at-env-test");
+
+    let auth = super::load_auth(
+        codex_home.path(),
+        /*enable_codex_api_key_env*/ false,
+        AuthCredentialsStoreMode::File,
+        /*chatgpt_base_url*/ None,
+    )
+    .await
+    .expect("auth load should succeed");
+
+    assert_eq!(auth, None);
+    assert!(
+        !get_auth_file(codex_home.path()).exists(),
+        "env auth should not write auth.json"
+    );
+}
+
+#[tokio::test]
+#[serial(codex_auth_env)]
+async fn load_auth_reads_personal_access_token_from_env_when_env_auth_enabled() {
     let codex_home = tempdir().unwrap();
     let server = MockServer::start().await;
     Mock::given(method("GET"))
@@ -867,7 +919,7 @@ async fn load_auth_reads_personal_access_token_from_env() {
     ] {
         let auth = super::load_auth(
             codex_home.path(),
-            /*enable_codex_api_key_env*/ false,
+            /*enable_codex_api_key_env*/ true,
             auth_credentials_store_mode,
             /*chatgpt_base_url*/ None,
         )
@@ -917,7 +969,7 @@ async fn personal_access_token_does_not_offer_unauthorized_recovery() {
     let manager = Arc::new(
         AuthManager::new(
             codex_home.path().to_path_buf(),
-            /*enable_codex_api_key_env*/ false,
+            /*enable_codex_api_key_env*/ true,
             AuthCredentialsStoreMode::File,
             /*chatgpt_base_url*/ None,
         )

codex-rs/login/src/auth/manager.rs

@@ -828,7 +828,7 @@ async fn load_auth(
         return Ok(Some(auth));
     }
 
-    if let Some(access_token) = read_codex_access_token_from_env() {
+    if enable_codex_api_key_env && let Some(access_token) = read_codex_access_token_from_env() {
         return match classify_codex_access_token(&access_token) {
             CodexAccessToken::PersonalAccessToken(access_token) => {
                 CodexAuth::from_personal_access_token(access_token)

codex-rs/login/src/auth_profiles.rs

@@ -124,25 +124,105 @@ pub fn load_auth_profiles(codex_home: &Path) -> io::Result<AuthProfilesFile> {
 pub fn save_auth_profiles(codex_home: &Path, profiles: &AuthProfilesFile) -> io::Result<()> {
     fs::create_dir_all(codex_home)?;
     let path = profile_metadata_path(codex_home);
-    let tmp_path = path.with_extension("json.tmp");
     let raw = serde_json::to_string_pretty(profiles).map_err(io::Error::other)?;
-    let mut options = OpenOptions::new();
-    options.write(true).create(true).truncate(true);
-
-    #[cfg(unix)]
-    {
-        use std::os::unix::fs::OpenOptionsExt;
-        options.mode(0o600);
-    }
-
-    let mut file = options.open(&tmp_path)?;
+    let (tmp_path, mut file) = create_metadata_tmp_file(&path)?;
     #[cfg(unix)]
     {
         use std::os::unix::fs::PermissionsExt;
         file.set_permissions(fs::Permissions::from_mode(0o600))?;
     }
     file.write_all(raw.as_bytes())?;
-    fs::rename(tmp_path, path)?;
+    file.flush()?;
+    drop(file);
+    if let Err(err) = replace_metadata_file(&tmp_path, &path) {
+        let _ = fs::remove_file(&tmp_path);
+        return Err(err);
+    }
+    Ok(())
+}
+
+fn create_metadata_tmp_file(path: &Path) -> io::Result<(PathBuf, fs::File)> {
+    let file_name = path
+        .file_name()
+        .and_then(|name| name.to_str())
+        .unwrap_or(PROFILE_METADATA_FILE);
+    let parent = path.parent().unwrap_or_else(|| Path::new("."));
+    for attempt in 0..100 {
+        let tmp_path = parent.join(format!(
+            ".{file_name}.{}.{}.tmp",
+            std::process::id(),
+            attempt
+        ));
+        let mut options = OpenOptions::new();
+        options.write(true).create_new(true);
+        #[cfg(unix)]
+        {
+            use std::os::unix::fs::OpenOptionsExt;
+            options.mode(0o600);
+        }
+        match options.open(&tmp_path) {
+            Ok(file) => return Ok((tmp_path, file)),
+            Err(err) if err.kind() == io::ErrorKind::AlreadyExists => continue,
+            Err(err) => return Err(err),
+        }
+    }
+    Err(io::Error::new(
+        io::ErrorKind::AlreadyExists,
+        format!(
+            "failed to allocate temporary metadata path for {}",
+            path.display()
+        ),
+    ))
+}
+
+fn replace_metadata_file(src: &Path, dst: &Path) -> io::Result<()> {
+    #[cfg(not(windows))]
+    {
+        fs::rename(src, dst)
+    }
+    #[cfg(windows)]
+    {
+        match replace_file_windows(src, dst) {
+            Ok(()) => Ok(()),
+            Err(err) if err.kind() == io::ErrorKind::NotFound => fs::rename(src, dst),
+            Err(err) => Err(err),
+        }
+    }
+}
+
+#[cfg(windows)]
+fn replace_file_windows(src: &Path, dst: &Path) -> io::Result<()> {
+    use std::ffi::c_void;
+    use std::os::windows::ffi::OsStrExt;
+
+    const REPLACEFILE_IGNORE_MERGE_ERRORS: u32 = 0x0000_0002;
+
+    unsafe extern "system" {
+        fn ReplaceFileW(
+            lp_replaced_file_name: *const u16,
+            lp_replacement_file_name: *const u16,
+            lp_backup_file_name: *const u16,
+            dw_replace_flags: u32,
+            lp_exclude: *mut c_void,
+            lp_reserved: *mut c_void,
+        ) -> i32;
+    }
+
+    let dst_wide: Vec<u16> = dst.as_os_str().encode_wide().chain(Some(0)).collect();
+    let src_wide: Vec<u16> = src.as_os_str().encode_wide().chain(Some(0)).collect();
+    let replaced = unsafe {
+        ReplaceFileW(
+            dst_wide.as_ptr(),
+            src_wide.as_ptr(),
+            std::ptr::null(),
+            REPLACEFILE_IGNORE_MERGE_ERRORS,
+            std::ptr::null_mut(),
+            std::ptr::null_mut(),
+        )
+    };
+    if replaced == 0 {
+        return Err(io::Error::last_os_error());
+    }
     Ok(())
 }
 
@@ -252,6 +332,27 @@ mod tests {
         assert_eq!(profiles[0].metadata.priming_enabled, Some(true));
     }
 
+    #[test]
+    fn save_profiles_replaces_existing_metadata() {
+        let temp = TempDir::new().expect("tempdir");
+
+        upsert_auth_profile(temp.path(), "work", |metadata| {
+            metadata.email = Some("work@example.com".to_string());
+        })
+        .expect("first upsert");
+        upsert_auth_profile(temp.path(), "work", |metadata| {
+            metadata.email = Some("updated@example.com".to_string());
+        })
+        .expect("second upsert");
+
+        let profiles = load_auth_profiles(temp.path()).expect("load profiles");
+        assert_eq!(profiles.profiles.len(), 1);
+        assert_eq!(
+            profiles.profiles["work"].email.as_deref(),
+            Some("updated@example.com")
+        );
+    }
+
     #[cfg(unix)]
     #[test]
     fn saved_profiles_metadata_is_private() {
@@ -275,7 +376,9 @@ mod tests {
         use std::os::unix::fs::PermissionsExt;
 
         let temp = TempDir::new().expect("tempdir");
-        let tmp_path = profile_metadata_path(temp.path()).with_extension("json.tmp");
+        let tmp_path = temp
+            .path()
+            .join(format!(".auth-profiles.json.{}.0.tmp", std::process::id()));
         fs::write(&tmp_path, "{}").expect("write stale tmp");
         fs::set_permissions(&tmp_path, fs::Permissions::from_mode(0o644))
             .expect("make stale tmp permissive");
@@ -288,6 +391,7 @@ mod tests {
             .mode()
             & 0o777;
         assert_eq!(mode, 0o600);
+        assert!(tmp_path.exists());
     }
 
     #[test]

codex-rs/login/tests/suite/logout.rs

@@ -6,6 +6,7 @@ use codex_config::types::AuthCredentialsStoreMode;
 use codex_login::AuthDotJson;
 use codex_login::AuthManager;
 use codex_login::CLIENT_ID;
+use codex_login::CODEX_ACCESS_TOKEN_ENV_VAR;
 use codex_login::REVOKE_TOKEN_URL_OVERRIDE_ENV_VAR;
 use codex_login::logout_with_revoke;
 use codex_login::save_auth;
@@ -113,6 +114,56 @@ async fn logout_with_revoke_removes_auth_when_revoke_fails() -> Result<()> {
     Ok(())
 }
 
+#[serial_test::serial(logout_revoke)]
+#[tokio::test]
+async fn logout_with_revoke_ignores_access_token_env() -> Result<()> {
+    skip_if_no_network!(Ok(()));
+
+    let server = MockServer::start().await;
+    Mock::given(method("POST"))
+        .and(path("/oauth/revoke"))
+        .respond_with(ResponseTemplate::new(200).set_body_json(json!({
+            "message": "success"
+        })))
+        .expect(1)
+        .mount(&server)
+        .await;
+    let _revoke_guard = EnvGuard::set(
+        REVOKE_TOKEN_URL_OVERRIDE_ENV_VAR,
+        format!("{}/oauth/revoke", server.uri()),
+    );
+    let _access_token_guard = EnvGuard::set(CODEX_ACCESS_TOKEN_ENV_VAR, "at-env-test".to_string());
+
+    let codex_home = TempDir::new()?;
+    save_auth(
+        codex_home.path(),
+        &chatgpt_auth_with_refresh_token("profile-refresh-token"),
+        AuthCredentialsStoreMode::File,
+    )?;
+
+    let removed = logout_with_revoke(codex_home.path(), AuthCredentialsStoreMode::File).await?;
+
+    assert!(removed);
+    assert!(!codex_home.path().join("auth.json").exists());
+    let requests = server
+        .received_requests()
+        .await
+        .context("failed to fetch revoke requests")?;
+    assert_eq!(requests.len(), 1);
+    assert_eq!(
+        requests[0]
+            .body_json::<Value>()
+            .context("revoke request should be JSON")?,
+        json!({
+            "token": "profile-refresh-token",
+            "token_type_hint": "refresh_token",
+            "client_id": CLIENT_ID,
+        })
+    );
+    server.verify().await;
+    Ok(())
+}
+
 #[serial_test::serial(logout_revoke)]
 #[tokio::test]
 async fn auth_manager_logout_with_revoke_uses_cached_auth() -> Result<()> {