Restack OAuth Auto-store drift diagnostics

This commit is contained in:
Steven Lee
2026-07-07 04:29:04 +00:00
parent 77ff788733
commit 14578ad4fc
7 changed files with 519 additions and 59 deletions

2
codex-rs/Cargo.lock generated
View File

@@ -3795,10 +3795,12 @@ dependencies = [
"codex-config",
"codex-exec-server",
"codex-keyring-store",
"codex-otel",
"codex-protocol",
"codex-secrets",
"codex-utils-cargo-bin",
"codex-utils-home-dir",
"codex-utils-path",
"codex-utils-path-uri",
"codex-utils-pty",
"futures",

View File

@@ -19,8 +19,10 @@ codex-api = { workspace = true }
codex-config = { workspace = true }
codex-exec-server = { workspace = true }
codex-keyring-store = { workspace = true }
codex-otel = { workspace = true }
codex-protocol = { workspace = true }
codex-secrets = { workspace = true }
codex-utils-path = { workspace = true }
codex-utils-path-uri = { workspace = true }
codex-utils-pty = { workspace = true }
codex-utils-home-dir = { workspace = true }

View File

@@ -18,6 +18,7 @@
mod refresh_lock;
mod refresh_transaction;
mod resolution_state;
mod resolved_store;
mod store_lock;
@@ -58,6 +59,8 @@ use std::time::SystemTime;
use std::time::UNIX_EPOCH;
use tracing::warn;
use self::resolution_state::StoreResolutionReason;
use self::resolution_state::record_store_resolution;
use self::store_lock::OAuthStore;
use self::store_lock::OAuthStoreLock;
use self::store_lock::OAuthStoreLockFailure;
@@ -282,21 +285,53 @@ fn save_oauth_tokens_with_keyring_store<K: KeyringStore + Clone + 'static>(
store_mode: OAuthCredentialsStoreMode,
keyring_backend_kind: AuthKeyringBackendKind,
) -> Result<()> {
match store_mode {
OAuthCredentialsStoreMode::Auto => save_oauth_tokens_with_keyring_with_fallback_to_file(
keyring_store,
keyring_backend_kind,
server_name,
tokens,
),
OAuthCredentialsStoreMode::File => save_oauth_tokens_to_file(tokens),
OAuthCredentialsStoreMode::Keyring => save_oauth_tokens_with_keyring_and_cleanup_file(
keyring_store,
keyring_backend_kind,
server_name,
tokens,
),
}
let (resolved_store, reason) = match store_mode {
OAuthCredentialsStoreMode::Auto => {
let resolved_store = save_oauth_tokens_with_keyring_with_fallback_to_file(
keyring_store,
keyring_backend_kind,
server_name,
tokens,
)?;
let reason = match resolved_store {
ResolvedOAuthCredentialStore::File => {
StoreResolutionReason::AutoSaveToFileAfterKeyringError
}
ResolvedOAuthCredentialStore::Keyring(_) => {
StoreResolutionReason::AutoSaveToKeyring
}
};
(resolved_store, reason)
}
OAuthCredentialsStoreMode::File => {
save_oauth_tokens_to_file(tokens)?;
(
ResolvedOAuthCredentialStore::File,
StoreResolutionReason::ConfiguredSave,
)
}
OAuthCredentialsStoreMode::Keyring => {
save_oauth_tokens_with_keyring_and_cleanup_file(
keyring_store,
keyring_backend_kind,
server_name,
tokens,
)?;
(
ResolvedOAuthCredentialStore::Keyring(keyring_backend_kind),
StoreResolutionReason::ConfiguredSave,
)
}
};
record_store_resolution(
server_name,
&tokens.url,
store_mode,
keyring_backend_kind,
resolved_store,
reason,
);
Ok(())
}
fn save_oauth_tokens_with_keyring<K: KeyringStore + Clone + 'static>(
@@ -399,14 +434,14 @@ fn save_oauth_tokens_with_keyring_with_fallback_to_file<K: KeyringStore + Clone
keyring_backend_kind: AuthKeyringBackendKind,
server_name: &str,
tokens: &StoredOAuthTokens,
) -> Result<()> {
) -> Result<ResolvedOAuthCredentialStore> {
match save_oauth_tokens_with_keyring_and_cleanup_file(
keyring_store,
keyring_backend_kind,
server_name,
tokens,
) {
Ok(()) => Ok(()),
Ok(()) => Ok(ResolvedOAuthCredentialStore::Keyring(keyring_backend_kind)),
// As on load, a store lock failure is a coordination failure rather than evidence that
// the keyring backend is unavailable. Falling back could leave a newer File token hidden
// behind a stale Secrets entry.
@@ -415,7 +450,8 @@ fn save_oauth_tokens_with_keyring_with_fallback_to_file<K: KeyringStore + Clone
let message = error.to_string();
warn!("falling back to file storage for OAuth tokens: {message}");
save_oauth_tokens_to_file(tokens)
.with_context(|| format!("failed to write OAuth tokens to keyring: {message}"))
.with_context(|| format!("failed to write OAuth tokens to keyring: {message}"))?;
Ok(ResolvedOAuthCredentialStore::File)
}
}
}

View File

@@ -0,0 +1,253 @@
//! Best-effort diagnostics for changes in MCP OAuth store resolution.
//!
//! This token-free state is observational, never authoritative. OAuth still resolves from config
//! and available credentials; failure to read, lock, or write this file must not affect credential
//! selection or an OAuth operation. The last observation intentionally survives logout so a later
//! login can report that `Auto` selected a different store.
use std::collections::BTreeMap;
use std::fs;
use std::io::ErrorKind;
use std::path::Path;
use std::time::Duration;
use anyhow::Context;
use anyhow::Result;
use codex_config::types::AuthKeyringBackendKind;
use codex_config::types::OAuthCredentialsStoreMode;
use codex_utils_home_dir::find_codex_home;
use codex_utils_path::write_atomically;
use serde::Deserialize;
use serde::Serialize;
use tracing::warn;
use super::ResolvedOAuthCredentialStore;
use super::compute_store_key;
use super::store_lock::OAuthStore;
use super::store_lock::OAuthStoreLock;
const RESOLUTION_STATE_FILENAME: &str = ".mcp-oauth-store-resolutions.json";
const RESOLUTION_STATE_VERSION: u32 = 1;
const RESOLUTION_STATE_LOCK_TIMEOUT: Duration = Duration::from_millis(/*millis*/ 250);
const RESOLUTION_CHANGED_METRIC: &str = "codex.mcp.oauth.store_resolution_changed";
#[derive(Debug, Clone, Copy)]
pub(super) enum StoreResolutionReason {
AutoLoadFromKeyring,
AutoLoadFromFileAfterMissingKeyring,
AutoLoadFromFileAfterKeyringError,
AutoSaveToKeyring,
AutoSaveToFileAfterKeyringError,
ConfiguredLoad,
ConfiguredSave,
}
impl StoreResolutionReason {
fn as_str(self) -> &'static str {
match self {
Self::AutoLoadFromKeyring => "auto_load_keyring",
Self::AutoLoadFromFileAfterMissingKeyring => "auto_load_file_keyring_missing",
Self::AutoLoadFromFileAfterKeyringError => "auto_load_file_keyring_error",
Self::AutoSaveToKeyring => "auto_save_keyring",
Self::AutoSaveToFileAfterKeyringError => "auto_save_file_keyring_error",
Self::ConfiguredLoad => "configured_load",
Self::ConfiguredSave => "configured_save",
}
}
}
#[derive(Debug, Clone, Copy, Serialize, Deserialize, PartialEq, Eq)]
#[serde(rename_all = "lowercase")]
enum ObservedStore {
File,
Keyring,
}
impl ObservedStore {
fn as_str(self) -> &'static str {
match self {
Self::File => "file",
Self::Keyring => "keyring",
}
}
}
impl From<ResolvedOAuthCredentialStore> for ObservedStore {
fn from(store: ResolvedOAuthCredentialStore) -> Self {
match store {
ResolvedOAuthCredentialStore::File => Self::File,
ResolvedOAuthCredentialStore::Keyring(_) => Self::Keyring,
}
}
}
#[derive(Debug, Clone, Copy, Serialize, Deserialize, PartialEq, Eq)]
struct StoreResolution {
store_mode: OAuthCredentialsStoreMode,
keyring_backend: AuthKeyringBackendKind,
resolved_store: ObservedStore,
}
#[derive(Debug, Serialize, Deserialize, PartialEq, Eq)]
struct ResolutionState {
#[serde(default = "resolution_state_version")]
version: u32,
#[serde(default)]
resolutions: BTreeMap<String, StoreResolution>,
}
impl Default for ResolutionState {
fn default() -> Self {
Self {
version: RESOLUTION_STATE_VERSION,
resolutions: BTreeMap::new(),
}
}
}
fn resolution_state_version() -> u32 {
RESOLUTION_STATE_VERSION
}
#[derive(Debug, PartialEq, Eq)]
struct StoreResolutionChange {
previous: ObservedStore,
current: ObservedStore,
}
pub(super) fn record_store_resolution(
server_name: &str,
url: &str,
store_mode: OAuthCredentialsStoreMode,
keyring_backend: AuthKeyringBackendKind,
resolved_store: ResolvedOAuthCredentialStore,
reason: StoreResolutionReason,
) {
let result = match find_codex_home() {
Ok(codex_home) => record_store_resolution_in(
&codex_home,
server_name,
url,
store_mode,
keyring_backend,
resolved_store,
),
Err(error) => Err(error.into()),
};
match result {
Ok(Some(change)) => {
warn!(
server_name,
previous_store = change.previous.as_str(),
resolved_store = change.current.as_str(),
resolution_reason = reason.as_str(),
"MCP OAuth Auto storage resolved differently than its previous use"
);
if let Some(metrics) = codex_otel::global() {
let _ = metrics.counter(
RESOLUTION_CHANGED_METRIC,
/*inc*/ 1,
&[
("previous_store", change.previous.as_str()),
("resolved_store", change.current.as_str()),
("reason", reason.as_str()),
],
);
}
}
Ok(None) => {}
Err(error) => {
warn!(
server_name,
resolution_reason = reason.as_str(),
error = %error,
"failed to update MCP OAuth store resolution diagnostics"
);
}
}
}
fn record_store_resolution_in(
codex_home: &Path,
server_name: &str,
url: &str,
store_mode: OAuthCredentialsStoreMode,
keyring_backend: AuthKeyringBackendKind,
resolved_store: ResolvedOAuthCredentialStore,
) -> Result<Option<StoreResolutionChange>> {
// This short, best-effort lock serializes only the diagnostic map. It must not make an OAuth
// operation wait behind the 60-second credential-store lock budget.
let _lock = OAuthStoreLock::acquire_in(
codex_home,
OAuthStore::ResolutionState,
RESOLUTION_STATE_LOCK_TIMEOUT,
)?;
let path = codex_home.join(RESOLUTION_STATE_FILENAME);
let mut state = read_resolution_state(&path)?;
anyhow::ensure!(
state.version == RESOLUTION_STATE_VERSION,
"unsupported MCP OAuth store resolution state version {}",
state.version
);
let store_key = compute_store_key(server_name, url)?;
let current = StoreResolution {
store_mode,
keyring_backend,
resolved_store: resolved_store.into(),
};
let previous = state.resolutions.get(&store_key).copied();
if previous == Some(current) {
return Ok(None);
}
state.resolutions.insert(store_key, current);
let serialized = serde_json::to_string(&state)
.context("failed to serialize MCP OAuth store resolution diagnostics")?;
write_atomically(&path, &serialized).with_context(|| {
format!(
"failed to write MCP OAuth store resolution diagnostics at {}",
path.display()
)
})?;
// Explicit modes and keyring-backend changes are intentional authority changes, so they reset
// the comparison baseline. Only repeated Auto resolution under one backend indicates the
// availability drift this diagnostic is meant to surface.
Ok(previous.and_then(|previous| {
(previous.store_mode == OAuthCredentialsStoreMode::Auto
&& current.store_mode == OAuthCredentialsStoreMode::Auto
&& previous.keyring_backend == current.keyring_backend
&& previous.resolved_store != current.resolved_store)
.then_some(StoreResolutionChange {
previous: previous.resolved_store,
current: current.resolved_store,
})
}))
}
fn read_resolution_state(path: &Path) -> Result<ResolutionState> {
let contents = match fs::read_to_string(path) {
Ok(contents) => contents,
Err(error) if error.kind() == ErrorKind::NotFound => return Ok(ResolutionState::default()),
Err(error) => {
return Err(error).with_context(|| {
format!(
"failed to read MCP OAuth store resolution diagnostics at {}",
path.display()
)
});
}
};
serde_json::from_str(&contents).with_context(|| {
format!(
"failed to parse MCP OAuth store resolution diagnostics at {}",
path.display()
)
})
}
#[cfg(test)]
#[path = "resolution_state_tests.rs"]
mod tests;

View File

@@ -0,0 +1,131 @@
use codex_config::types::AuthKeyringBackendKind;
use codex_config::types::OAuthCredentialsStoreMode;
use pretty_assertions::assert_eq;
use super::ObservedStore;
use super::ResolutionState;
use super::StoreResolution;
use super::StoreResolutionChange;
use super::record_store_resolution_in;
use crate::oauth::ResolvedOAuthCredentialStore;
use crate::oauth::compute_store_key;
#[test]
fn auto_resolution_change_is_reported_and_persisted() -> anyhow::Result<()> {
let codex_home = tempfile::tempdir()?;
let server_name = "test-server";
let url = "https://example.test/mcp";
assert_eq!(
record_store_resolution_in(
codex_home.path(),
server_name,
url,
OAuthCredentialsStoreMode::Auto,
AuthKeyringBackendKind::Direct,
ResolvedOAuthCredentialStore::Keyring(AuthKeyringBackendKind::Direct),
)?,
None
);
assert_eq!(
record_store_resolution_in(
codex_home.path(),
server_name,
url,
OAuthCredentialsStoreMode::Auto,
AuthKeyringBackendKind::Direct,
ResolvedOAuthCredentialStore::File,
)?,
Some(StoreResolutionChange {
previous: ObservedStore::Keyring,
current: ObservedStore::File,
})
);
let state: ResolutionState = serde_json::from_str(&std::fs::read_to_string(
codex_home.path().join(super::RESOLUTION_STATE_FILENAME),
)?)?;
assert_eq!(
state,
ResolutionState {
version: super::RESOLUTION_STATE_VERSION,
resolutions: [(
compute_store_key(server_name, url)?,
StoreResolution {
store_mode: OAuthCredentialsStoreMode::Auto,
keyring_backend: AuthKeyringBackendKind::Direct,
resolved_store: ObservedStore::File,
},
)]
.into(),
}
);
Ok(())
}
#[test]
fn intentional_configuration_changes_reset_the_auto_comparison() -> anyhow::Result<()> {
let codex_home = tempfile::tempdir()?;
let server_name = "test-server";
let url = "https://example.test/mcp";
assert_eq!(
record_store_resolution_in(
codex_home.path(),
server_name,
url,
OAuthCredentialsStoreMode::Auto,
AuthKeyringBackendKind::Direct,
ResolvedOAuthCredentialStore::Keyring(AuthKeyringBackendKind::Direct),
)?,
None
);
assert_eq!(
record_store_resolution_in(
codex_home.path(),
server_name,
url,
OAuthCredentialsStoreMode::File,
AuthKeyringBackendKind::Direct,
ResolvedOAuthCredentialStore::File,
)?,
None
);
assert_eq!(
record_store_resolution_in(
codex_home.path(),
server_name,
url,
OAuthCredentialsStoreMode::Auto,
AuthKeyringBackendKind::Direct,
ResolvedOAuthCredentialStore::File,
)?,
None
);
assert_eq!(
record_store_resolution_in(
codex_home.path(),
server_name,
url,
OAuthCredentialsStoreMode::Auto,
AuthKeyringBackendKind::Secrets,
ResolvedOAuthCredentialStore::Keyring(AuthKeyringBackendKind::Secrets),
)?,
None
);
assert_eq!(
record_store_resolution_in(
codex_home.path(),
server_name,
url,
OAuthCredentialsStoreMode::Auto,
AuthKeyringBackendKind::Secrets,
ResolvedOAuthCredentialStore::File,
)?,
Some(StoreResolutionChange {
previous: ObservedStore::Keyring,
current: ObservedStore::File,
})
);
Ok(())
}

View File

@@ -11,15 +11,17 @@ use super::OAuthKeyringLoadError;
use super::StoredOAuthTokens;
use super::load_oauth_tokens_from_file;
use super::load_oauth_tokens_from_keyring;
use super::resolution_state::StoreResolutionReason;
use super::resolution_state::record_store_resolution;
use super::save_oauth_tokens_to_file;
use super::save_oauth_tokens_with_keyring;
/// Concrete credential store resolved for one MCP OAuth client lifecycle.
///
/// This is intentionally not durable. `Auto` may resolve differently in a later process, but a
/// client that loaded credentials from one store must reread, refresh, persist, and remove only
/// through that store. A mid-lifecycle backend failure is unexpected and must return an error
/// rather than falling back to another possibly stale refresh token.
/// This selection is intentionally not persisted as credential authority. `Auto` may resolve
/// differently in a later process, but a client that loaded credentials from one store must
/// reread, refresh, persist, and remove only through that store. A separate best-effort diagnostic
/// record warns when later Auto resolution changes; it never influences this selection.
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub(crate) enum ResolvedOAuthCredentialStore {
File,
@@ -85,67 +87,98 @@ pub(crate) fn resolve_oauth_tokens_from_store_policy<K: KeyringStore + Clone + '
store_mode: OAuthCredentialsStoreMode,
keyring_backend_kind: AuthKeyringBackendKind,
) -> Result<Option<ResolvedOAuthTokens>> {
match store_mode {
let (resolved, reason) = match store_mode {
OAuthCredentialsStoreMode::Auto => {
// Auto remains keyring-first at lifecycle startup. The returned source is then pinned
// by the client transport recipe and OAuth persistor so retries, recovery, and
// refresh work cannot hot-switch stores.
// TODO(stevenlee): Different processes can still resolve Auto to different stores
// when keyring availability differs. Solving that safely requires durable backend
// selection or reconciliation of legacy entries and is intentionally outside this
// stack.
// Different processes can still resolve Auto differently when keyring availability
// changes. The token-free observation below makes that drift visible without turning
// the sidecar into another source of credential authority.
match load_oauth_tokens_from_keyring(
keyring_store,
keyring_backend_kind,
server_name,
url,
) {
Ok(Some(tokens)) => Ok(Some(ResolvedOAuthTokens {
tokens,
store: ResolvedOAuthCredentialStore::Keyring(keyring_backend_kind),
})),
Ok(None) => Ok(
Ok(Some(tokens)) => (
Some(ResolvedOAuthTokens {
tokens,
store: ResolvedOAuthCredentialStore::Keyring(keyring_backend_kind),
}),
StoreResolutionReason::AutoLoadFromKeyring,
),
Ok(None) => (
load_oauth_tokens_from_file(server_name, url)?.map(|tokens| {
ResolvedOAuthTokens {
tokens,
store: ResolvedOAuthCredentialStore::File,
}
}),
StoreResolutionReason::AutoLoadFromFileAfterMissingKeyring,
),
// Auto may fall back when the keyring backend is unavailable, but a Secrets
// aggregate-lock failure means authority may be changing. Consulting File in
// that state could replay credentials hidden behind a newer Secrets entry.
Err(OAuthKeyringLoadError::StoreLock(error)) => Err(error.into()),
Err(OAuthKeyringLoadError::StoreLock(error)) => {
return Err(error.into());
}
Err(error) => {
warn!("failed to read OAuth tokens from keyring: {error}");
Ok(load_oauth_tokens_from_file(server_name, url)
.with_context(|| {
format!("failed to read OAuth tokens from keyring: {error}")
})?
.map(|tokens| ResolvedOAuthTokens {
tokens,
store: ResolvedOAuthCredentialStore::File,
}))
(
load_oauth_tokens_from_file(server_name, url)
.with_context(|| {
format!("failed to read OAuth tokens from keyring: {error}")
})?
.map(|tokens| ResolvedOAuthTokens {
tokens,
store: ResolvedOAuthCredentialStore::File,
}),
StoreResolutionReason::AutoLoadFromFileAfterKeyringError,
)
}
}
}
OAuthCredentialsStoreMode::File => Ok(load_oauth_tokens_from_file(server_name, url)?.map(
|tokens| ResolvedOAuthTokens {
OAuthCredentialsStoreMode::File => (
load_oauth_tokens_from_file(server_name, url)?.map(|tokens| ResolvedOAuthTokens {
tokens,
store: ResolvedOAuthCredentialStore::File,
},
)),
OAuthCredentialsStoreMode::Keyring => Ok(load_oauth_tokens_from_keyring(
keyring_store,
keyring_backend_kind,
}),
StoreResolutionReason::ConfiguredLoad,
),
OAuthCredentialsStoreMode::Keyring => (
load_oauth_tokens_from_keyring(keyring_store, keyring_backend_kind, server_name, url)
.map_err(anyhow::Error::from)
.context("failed to read OAuth tokens from keyring")?
.map(|tokens| ResolvedOAuthTokens {
tokens,
store: ResolvedOAuthCredentialStore::Keyring(keyring_backend_kind),
}),
StoreResolutionReason::ConfiguredLoad,
),
};
// Explicit modes select their store even when no credential exists. Auto has no concrete
// selection until one store returns credentials or login successfully persists them.
let resolved_store = resolved
.as_ref()
.map(|resolved| resolved.store)
.or(match store_mode {
OAuthCredentialsStoreMode::Auto => None,
OAuthCredentialsStoreMode::File => Some(ResolvedOAuthCredentialStore::File),
OAuthCredentialsStoreMode::Keyring => {
Some(ResolvedOAuthCredentialStore::Keyring(keyring_backend_kind))
}
});
if let Some(resolved_store) = resolved_store {
record_store_resolution(
server_name,
url,
)
.map_err(anyhow::Error::from)
.context("failed to read OAuth tokens from keyring")?
.map(|tokens| ResolvedOAuthTokens {
tokens,
store: ResolvedOAuthCredentialStore::Keyring(keyring_backend_kind),
})),
store_mode,
keyring_backend_kind,
resolved_store,
reason,
);
}
Ok(resolved)
}

View File

@@ -1,8 +1,8 @@
//! Cross-process serialization for MCP OAuth stores shared by multiple credentials.
//! Cross-process serialization for MCP OAuth state shared by multiple credentials.
//!
//! File and Secrets each keep credentials for multiple MCP servers in one aggregate document.
//! Their lock therefore protects the complete read-modify-write operation. Direct keyring entries
//! are already stored independently per credential and do not use this lock.
//! File, Secrets, and the token-free resolution diagnostic each keep entries for multiple MCP
//! servers in one aggregate document. Their locks protect complete read-modify-write operations.
//! Direct keyring entries are already independent per credential and do not use this lock.
use std::fs;
use std::fs::File;
@@ -25,6 +25,7 @@ const LOCK_CONTENTION_EVENT_TARGET: &str = "codex_rmcp_client::oauth::store_lock
pub(super) enum OAuthStore {
File,
Secrets,
ResolutionState,
}
impl OAuthStore {
@@ -32,6 +33,7 @@ impl OAuthStore {
match self {
Self::File => "file-store.lock",
Self::Secrets => "secrets-store.lock",
Self::ResolutionState => "resolution-state.lock",
}
}
}
@@ -41,6 +43,7 @@ impl std::fmt::Display for OAuthStore {
match self {
Self::File => f.write_str("fallback file"),
Self::Secrets => f.write_str("encrypted secrets"),
Self::ResolutionState => f.write_str("store resolution diagnostics"),
}
}
}