Merge pull request #2795 from Hmbown/codex/harvest-2792-auth-context

fix(tui): enrich auth errors with request context
This commit is contained in:
Hunter Bown
2026-06-05 08:17:20 -07:00
committed by GitHub
4 changed files with 402 additions and 9 deletions
+4
View File
@@ -161,6 +161,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
### Fixed
- Authentication failures now include redacted request context such as provider,
base URL authority, model, key source, key type, and key fingerprint, making
stale provider, endpoint, or API-key state diagnosable without exposing the
secret (#2665, #2792). Thanks @mvanhorn for the implementation.
- Browser-opening actions now compile on non-desktop targets by delegating the
unsupported-platform error to the shared URL opener instead of hiding the TUI
wrapper behind a narrower macOS/Linux/Windows cfg. Thanks @ci4ic4 for the
+4
View File
@@ -161,6 +161,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
### Fixed
- Authentication failures now include redacted request context such as provider,
base URL authority, model, key source, key type, and key fingerprint, making
stale provider, endpoint, or API-key state diagnosable without exposing the
secret (#2665, #2792). Thanks @mvanhorn for the implementation.
- Browser-opening actions now compile on non-desktop targets by delegating the
unsupported-platform error to the shared URL opener instead of hiding the TUI
wrapper behind a narrower macOS/Linux/Windows cfg. Thanks @ci4ic4 for the
+31 -2
View File
@@ -234,12 +234,12 @@ impl From<LlmError> for ErrorEnvelope {
"llm_timeout",
format!("Request timed out after {duration:?}"),
),
LlmError::AuthenticationError(message) => Self::new(
LlmError::AuthenticationError(auth) => Self::new(
ErrorCategory::Authentication,
ErrorSeverity::Critical,
false,
"llm_auth_error",
message,
auth.to_user_message(),
),
LlmError::AuthorizationError(message) => Self::new(
ErrorCategory::Authorization,
@@ -566,6 +566,35 @@ mod tests {
}
}
#[test]
fn llm_auth_error_envelope_renders_context_without_secret() {
let api_key = "tp-secret-token-plan-value";
let env = ErrorEnvelope::from(LlmError::from_http_response_with_request_context(
401,
&format!("Invalid API Key: {api_key}"),
Some("Xiaomi MiMo"),
Some("https://token-plan-sgp.xiaomimimo.com/v1"),
Some("mimo-v2.5"),
Some("env"),
Some(api_key),
));
assert_eq!(env.category, ErrorCategory::Authentication);
assert_eq!(env.severity, ErrorSeverity::Critical);
assert!(!env.recoverable);
assert!(env.message.contains("provider: Xiaomi MiMo"));
assert!(
env.message
.contains("base URL authority: token-plan-sgp.xiaomimimo.com")
);
assert!(env.message.contains("model: mimo-v2.5"));
assert!(env.message.contains("key source: env"));
assert!(env.message.contains("key fingerprint: tp-... (len=26)"));
assert!(env.message.contains("key type: Xiaomi MiMo Token Plan key"));
assert!(!env.message.contains(api_key));
assert!(!env.message.contains("secret-token-plan-value"));
}
#[test]
fn authorization_catches_forbidden_and_denied() {
for msg in [
+363 -7
View File
@@ -82,6 +82,194 @@ pub trait RetryConfigurable {
fn set_retry_config(&mut self, config: RetryConfig);
}
// === Authentication diagnostics ===
#[derive(Debug, Clone, PartialEq, Eq, Default)]
pub struct AuthenticationErrorContext {
pub provider: Option<String>,
pub base_url_authority: Option<String>,
pub model: Option<String>,
pub key_source: Option<String>,
pub key_fingerprint: Option<String>,
pub key_kind: Option<String>,
}
impl AuthenticationErrorContext {
#[must_use]
pub fn new(
provider: &str,
base_url: &str,
model: &str,
key_source: &str,
api_key: &str,
) -> Self {
Self::from_parts(
Some(provider),
Some(base_url),
Some(model),
Some(key_source),
Some(api_key),
)
}
#[must_use]
pub fn from_parts(
provider: Option<&str>,
base_url: Option<&str>,
model: Option<&str>,
key_source: Option<&str>,
api_key: Option<&str>,
) -> Self {
let api_key = api_key.and_then(non_empty_trimmed);
Self {
provider: provider.and_then(non_empty_trimmed).map(str::to_string),
base_url_authority: base_url.and_then(base_url_authority),
model: model.and_then(non_empty_trimmed).map(str::to_string),
key_source: key_source.and_then(non_empty_trimmed).map(str::to_string),
key_fingerprint: api_key.map(redacted_key_fingerprint),
key_kind: api_key.map(classify_api_key_prefix).map(str::to_string),
}
}
fn is_empty(&self) -> bool {
self.provider.is_none()
&& self.base_url_authority.is_none()
&& self.model.is_none()
&& self.key_source.is_none()
&& self.key_fingerprint.is_none()
&& self.key_kind.is_none()
}
fn detail_segments(&self) -> Vec<String> {
let mut segments = Vec::new();
if let Some(provider) = self.provider.as_deref() {
segments.push(format!("provider: {provider}"));
}
if let Some(authority) = self.base_url_authority.as_deref() {
segments.push(format!("base URL authority: {authority}"));
}
if let Some(model) = self.model.as_deref() {
segments.push(format!("model: {model}"));
}
if let Some(source) = self.key_source.as_deref() {
segments.push(format!("key source: {source}"));
}
if let Some(fingerprint) = self.key_fingerprint.as_deref() {
segments.push(format!("key fingerprint: {fingerprint}"));
}
if let Some(kind) = self.key_kind.as_deref() {
segments.push(format!("key type: {kind}"));
}
segments
}
}
#[derive(Debug, Clone, PartialEq, Eq)]
pub struct AuthenticationErrorDetail {
message: String,
context: Option<AuthenticationErrorContext>,
}
impl AuthenticationErrorDetail {
#[must_use]
pub fn new(message: impl Into<String>) -> Self {
Self {
message: message.into(),
context: None,
}
}
#[must_use]
pub fn with_context(
message: impl Into<String>,
context: Option<AuthenticationErrorContext>,
) -> Self {
let context = context.filter(|context| !context.is_empty());
Self {
message: message.into(),
context,
}
}
#[must_use]
pub fn message(&self) -> &str {
&self.message
}
#[must_use]
pub fn to_user_message(&self) -> String {
let Some(context) = self.context.as_ref() else {
return self.message.clone();
};
let segments = context.detail_segments();
if segments.is_empty() {
self.message.clone()
} else {
format!("{} ({})", self.message, segments.join(", "))
}
}
}
impl From<String> for AuthenticationErrorDetail {
fn from(message: String) -> Self {
Self::new(message)
}
}
impl From<&str> for AuthenticationErrorDetail {
fn from(message: &str) -> Self {
Self::new(message)
}
}
#[must_use]
pub fn classify_api_key_prefix(api_key: &str) -> &'static str {
if api_key.starts_with("tp-") {
"Xiaomi MiMo Token Plan key"
} else {
"API key"
}
}
fn non_empty_trimmed(value: &str) -> Option<&str> {
let value = value.trim();
if value.is_empty() { None } else { Some(value) }
}
fn base_url_authority(base_url: &str) -> Option<String> {
let base_url = non_empty_trimmed(base_url)?;
let without_scheme = base_url
.split_once("://")
.map_or(base_url, |(_, rest)| rest);
let authority = without_scheme.split('/').next().unwrap_or(without_scheme);
let authority = authority
.rsplit_once('@')
.map_or(authority, |(_, authority)| authority);
non_empty_trimmed(authority).map(str::to_string)
}
fn redacted_key_fingerprint(api_key: &str) -> String {
let api_key = api_key.trim();
let len = api_key.chars().count();
match public_key_prefix(api_key) {
Some(prefix) => format!("{prefix}... (len={len})"),
None => format!("unprefixed (len={len})"),
}
}
fn public_key_prefix(api_key: &str) -> Option<&str> {
["tp-", "sk-", "hf_", "hf-", "ak-", "rk-"]
.into_iter()
.find(|prefix| api_key.starts_with(prefix))
}
fn redact_api_key_from_message(message: &str, api_key: Option<&str>) -> String {
let Some(api_key) = api_key.and_then(non_empty_trimmed) else {
return message.to_string();
};
message.replace(api_key, "[redacted API key]")
}
// === LlmError - Classified Error Types ===
/// Classified LLM errors with retryability information.
@@ -107,8 +295,8 @@ pub enum LlmError {
/// Request timed out
Timeout(Duration),
/// Authentication failed (HTTP 401, 403)
AuthenticationError(String),
/// Authentication failed (HTTP 401, selected HTTP 403)
AuthenticationError(AuthenticationErrorDetail),
/// Authorization or provider-side blocking failed (HTTP 403)
AuthorizationError(String),
@@ -141,7 +329,9 @@ impl std::fmt::Display for LlmError {
}
LlmError::NetworkError(msg) => write!(f, "Network error: {msg}"),
LlmError::Timeout(d) => write!(f, "Request timed out after {d:?}"),
LlmError::AuthenticationError(msg) => write!(f, "Authentication failed: {msg}"),
LlmError::AuthenticationError(auth) => {
write!(f, "Authentication failed: {}", auth.to_user_message())
}
LlmError::AuthorizationError(msg) => write!(f, "Authorization failed: {msg}"),
LlmError::InvalidRequest { status, message } => {
write!(f, "Invalid request ({status}): {message}")
@@ -203,10 +393,10 @@ impl LlmError {
message: body.to_string(),
retry_after: None,
},
401 => LlmError::AuthenticationError(body.to_string()),
401 => Self::authentication_error(body),
403 => {
if looks_like_authentication_failure(body) {
LlmError::AuthenticationError(body.to_string())
Self::authentication_error(body)
} else {
LlmError::AuthorizationError(body.to_string())
}
@@ -262,6 +452,62 @@ impl LlmError {
}
}
#[must_use]
pub fn authentication_error(message: impl Into<String>) -> Self {
LlmError::AuthenticationError(AuthenticationErrorDetail::new(message))
}
#[must_use]
pub fn authentication_error_with_context(
message: impl Into<String>,
context: Option<AuthenticationErrorContext>,
) -> Self {
LlmError::AuthenticationError(AuthenticationErrorDetail::with_context(message, context))
}
/// Constructs an `LlmError` from HTTP response data plus request context
/// that is safe to display when authentication fails.
#[must_use]
pub fn from_http_response_with_request_context(
status: u16,
body: &str,
provider: Option<&str>,
base_url: Option<&str>,
model: Option<&str>,
key_source: Option<&str>,
api_key: Option<&str>,
) -> Self {
let body = redact_api_key_from_message(body, api_key);
let context =
AuthenticationErrorContext::from_parts(provider, base_url, model, key_source, api_key);
Self::from_http_response_with_auth_context(status, &body, Some(context))
}
/// Constructs an `LlmError` from HTTP status code and response body, with
/// optional structured details for authentication failures.
///
/// The `body` passed here must already be safe for user display. Prefer
/// [`Self::from_http_response_with_request_context`] when the raw API key is
/// available so the response body can be redacted before rendering.
#[must_use]
pub fn from_http_response_with_auth_context(
status: u16,
body: &str,
auth_context: Option<AuthenticationErrorContext>,
) -> Self {
match status {
401 => Self::authentication_error_with_context(body, auth_context),
403 => {
if looks_like_authentication_failure(body) {
Self::authentication_error_with_context(body, auth_context)
} else {
LlmError::AuthorizationError(body.to_string())
}
}
_ => Self::from_http_response(status, body),
}
}
/// Constructs an `LlmError` from HTTP status code, body, and optional Retry-After header.
pub fn from_http_response_with_retry_after(
status: u16,
@@ -898,6 +1144,13 @@ mod tests {
);
}
fn auth_user_message(error: LlmError) -> String {
match error {
LlmError::AuthenticationError(auth) => auth.to_user_message(),
other => panic!("expected authentication error, got {other}"),
}
}
#[test]
fn test_retry_config_defaults() {
let config = RetryConfig::default();
@@ -1014,7 +1267,7 @@ mod tests {
assert!(LlmError::Timeout(Duration::from_secs(30)).is_retryable());
// Non-retryable errors
assert!(!LlmError::AuthenticationError("invalid key".to_string()).is_retryable());
assert!(!LlmError::authentication_error("invalid key").is_retryable());
assert!(!LlmError::AuthorizationError("blocked".to_string()).is_retryable());
assert!(
!LlmError::InvalidRequest {
@@ -1071,6 +1324,109 @@ mod tests {
assert!(matches!(err, LlmError::InvalidRequest { status: 400, .. }));
}
#[test]
fn auth_error_with_context_includes_provider_authority_model_and_key_source() {
let err = LlmError::from_http_response_with_request_context(
401,
"Invalid API Key",
Some("Xiaomi MiMo"),
Some("https://token-plan-sgp.xiaomimimo.com/v1"),
Some("mimo-v2.5"),
Some("env"),
Some("tp-secret-token-plan-value"),
);
let message = auth_user_message(err);
assert!(message.contains("Invalid API Key"));
assert!(message.contains("provider: Xiaomi MiMo"));
assert!(message.contains("base URL authority: token-plan-sgp.xiaomimimo.com"));
assert!(message.contains("model: mimo-v2.5"));
assert!(message.contains("key source: env"));
assert!(message.contains("key fingerprint: tp-... (len=26)"));
}
#[test]
fn auth_error_redacts_full_api_key_from_body_and_context() {
let api_key = "tp-secret-token-plan-value";
let err = LlmError::from_http_response_with_request_context(
401,
&format!("Invalid API Key: {api_key}"),
Some("Xiaomi MiMo"),
Some("https://token-plan-sgp.xiaomimimo.com/v1"),
Some("mimo-v2.5"),
Some("config-file"),
Some(api_key),
);
let message = auth_user_message(err);
assert!(!message.contains(api_key));
assert!(!message.contains("secret-token-plan-value"));
assert!(message.contains("[redacted API key]"));
assert!(message.contains("key fingerprint: tp-... (len=26)"));
}
#[test]
fn auth_error_classifies_xiaomi_token_plan_key_prefix() {
let token_plan = AuthenticationErrorContext::from_parts(
None,
None,
None,
Some("session"),
Some("tp-secret-token-plan-value"),
);
let generic = AuthenticationErrorContext::from_parts(
None,
None,
None,
Some("session"),
Some("sk-other"),
);
let unprefixed = AuthenticationErrorContext::from_parts(
None,
None,
None,
Some("session"),
Some("plainsecretvalue"),
);
assert_eq!(
token_plan.key_kind.as_deref(),
Some("Xiaomi MiMo Token Plan key")
);
assert_eq!(generic.key_kind.as_deref(), Some("API key"));
assert_eq!(unprefixed.key_kind.as_deref(), Some("API key"));
assert_eq!(
unprefixed.key_fingerprint.as_deref(),
Some("unprefixed (len=16)")
);
}
#[test]
fn authorization_403_is_not_reclassified_by_auth_context() {
let err = LlmError::from_http_response_with_request_context(
403,
"forbidden",
Some("Arcee AI"),
Some("https://api.arcee.ai/v1"),
Some("auto"),
Some("env"),
Some("sk-arcee-secret"),
);
assert!(matches!(err, LlmError::AuthorizationError(_)));
}
#[test]
fn auth_error_without_context_preserves_bare_message() {
let err = LlmError::from_http_response_with_auth_context(
401,
"Invalid API Key",
Some(AuthenticationErrorContext::default()),
);
assert_eq!(auth_user_message(err), "Invalid API Key");
}
#[test]
fn cloudflare_html_error_is_summarized_without_raw_markup() {
let body = r#"<!DOCTYPE html><html><head><title>Access Denied</title><style>
@@ -1247,7 +1603,7 @@ mod tests {
&config,
|| {
call_count += 1;
async { Err(LlmError::AuthenticationError("bad key".to_string())) }
async { Err(LlmError::authentication_error("bad key")) }
},
None,
)