fix(security): tighten provider credential log hygiene
- remove as_deref credential routing path in provider factory - avoid raw provider error text in warmup/retry failure summaries - keep retry telemetry while reducing secret propagation risk
This commit is contained in:
parent
a1bb72767a
commit
5d131a8903
2 changed files with 32 additions and 13 deletions
|
|
@ -105,11 +105,11 @@ pub async fn api_error(provider: &str, response: reqwest::Response) -> anyhow::E
|
||||||
/// For Anthropic, the provider-specific env var is `ANTHROPIC_OAUTH_TOKEN` (for setup-tokens)
|
/// For Anthropic, the provider-specific env var is `ANTHROPIC_OAUTH_TOKEN` (for setup-tokens)
|
||||||
/// followed by `ANTHROPIC_API_KEY` (for regular API keys).
|
/// followed by `ANTHROPIC_API_KEY` (for regular API keys).
|
||||||
fn resolve_provider_credential(name: &str, credential_override: Option<&str>) -> Option<String> {
|
fn resolve_provider_credential(name: &str, credential_override: Option<&str>) -> Option<String> {
|
||||||
if let Some(credential_value) = credential_override
|
if let Some(raw_override) = credential_override {
|
||||||
.map(str::trim)
|
let trimmed_override = raw_override.trim();
|
||||||
.filter(|value| !value.is_empty())
|
if !trimmed_override.is_empty() {
|
||||||
{
|
return Some(trimmed_override.to_owned());
|
||||||
return Some(credential_value.to_string());
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
let provider_env_candidates: Vec<&str> = match name {
|
let provider_env_candidates: Vec<&str> = match name {
|
||||||
|
|
@ -402,11 +402,16 @@ pub fn create_routed_provider(
|
||||||
// Create each provider (with its own resilience wrapper)
|
// Create each provider (with its own resilience wrapper)
|
||||||
let mut providers: Vec<(String, Box<dyn Provider>)> = Vec::new();
|
let mut providers: Vec<(String, Box<dyn Provider>)> = Vec::new();
|
||||||
for name in &needed {
|
for name in &needed {
|
||||||
let key = model_routes
|
let routed_credential = model_routes
|
||||||
.iter()
|
.iter()
|
||||||
.find(|r| &r.provider == name)
|
.find(|r| &r.provider == name)
|
||||||
.and_then(|r| r.api_key.as_deref())
|
.and_then(|r| {
|
||||||
.or(api_key);
|
r.api_key.as_ref().and_then(|raw_key| {
|
||||||
|
let trimmed_key = raw_key.trim();
|
||||||
|
(!trimmed_key.is_empty()).then_some(trimmed_key)
|
||||||
|
})
|
||||||
|
});
|
||||||
|
let key = routed_credential.or(api_key);
|
||||||
// Only use api_url for the primary provider
|
// Only use api_url for the primary provider
|
||||||
let url = if name == primary_name { api_url } else { None };
|
let url = if name == primary_name { api_url } else { None };
|
||||||
match create_resilient_provider(name, key, url, reliability) {
|
match create_resilient_provider(name, key, url, reliability) {
|
||||||
|
|
@ -451,7 +456,7 @@ mod tests {
|
||||||
#[test]
|
#[test]
|
||||||
fn resolve_provider_credential_prefers_explicit_argument() {
|
fn resolve_provider_credential_prefers_explicit_argument() {
|
||||||
let resolved = resolve_provider_credential("openrouter", Some(" explicit-key "));
|
let resolved = resolve_provider_credential("openrouter", Some(" explicit-key "));
|
||||||
assert_eq!(resolved.as_deref(), Some("explicit-key"));
|
assert_eq!(resolved, Some("explicit-key".to_string()));
|
||||||
}
|
}
|
||||||
|
|
||||||
// ── Primary providers ────────────────────────────────────
|
// ── Primary providers ────────────────────────────────────
|
||||||
|
|
|
||||||
|
|
@ -144,8 +144,8 @@ impl Provider for ReliableProvider {
|
||||||
async fn warmup(&self) -> anyhow::Result<()> {
|
async fn warmup(&self) -> anyhow::Result<()> {
|
||||||
for (name, provider) in &self.providers {
|
for (name, provider) in &self.providers {
|
||||||
tracing::info!(provider = name, "Warming up provider connection pool");
|
tracing::info!(provider = name, "Warming up provider connection pool");
|
||||||
if let Err(e) = provider.warmup().await {
|
if provider.warmup().await.is_err() {
|
||||||
tracing::warn!(provider = name, "Warmup failed (non-fatal): {e}");
|
tracing::warn!(provider = name, "Warmup failed (non-fatal)");
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
Ok(())
|
Ok(())
|
||||||
|
|
@ -186,8 +186,15 @@ impl Provider for ReliableProvider {
|
||||||
let non_retryable = is_non_retryable(&e);
|
let non_retryable = is_non_retryable(&e);
|
||||||
let rate_limited = is_rate_limited(&e);
|
let rate_limited = is_rate_limited(&e);
|
||||||
|
|
||||||
|
let failure_reason = if rate_limited {
|
||||||
|
"rate_limited"
|
||||||
|
} else if non_retryable {
|
||||||
|
"non_retryable"
|
||||||
|
} else {
|
||||||
|
"retryable"
|
||||||
|
};
|
||||||
failures.push(format!(
|
failures.push(format!(
|
||||||
"{provider_name}/{current_model} attempt {}/{}: {e}",
|
"{provider_name}/{current_model} attempt {}/{}: {failure_reason}",
|
||||||
attempt + 1,
|
attempt + 1,
|
||||||
self.max_retries + 1
|
self.max_retries + 1
|
||||||
));
|
));
|
||||||
|
|
@ -284,8 +291,15 @@ impl Provider for ReliableProvider {
|
||||||
let non_retryable = is_non_retryable(&e);
|
let non_retryable = is_non_retryable(&e);
|
||||||
let rate_limited = is_rate_limited(&e);
|
let rate_limited = is_rate_limited(&e);
|
||||||
|
|
||||||
|
let failure_reason = if rate_limited {
|
||||||
|
"rate_limited"
|
||||||
|
} else if non_retryable {
|
||||||
|
"non_retryable"
|
||||||
|
} else {
|
||||||
|
"retryable"
|
||||||
|
};
|
||||||
failures.push(format!(
|
failures.push(format!(
|
||||||
"{provider_name}/{current_model} attempt {}/{}: {e}",
|
"{provider_name}/{current_model} attempt {}/{}: {failure_reason}",
|
||||||
attempt + 1,
|
attempt + 1,
|
||||||
self.max_retries + 1
|
self.max_retries + 1
|
||||||
));
|
));
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue