fix(tui): enrich auth errors with request context
Harvested from PR #2792 by @mvanhorn. Reported by @Hmbown in #2665.
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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 [
|
||||
|
||||
@@ -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,
|
||||
)
|
||||
|
||||
Reference in New Issue
Block a user