mirror of
https://github.com/farion1231/cc-switch.git
synced 2026-08-03 19:12:04 +08:00
fix(proxy/gemini): narrow URL normalization + guard empty OAuth access_token
P2a — Preserve opaque relay URLs that contain `/v1/models/` prefixes.
`should_normalize_gemini_full_url` previously flagged any full URL whose
path merely contained `/v1beta/models/` or `/v1/models/` as a structured
Gemini endpoint, forcing rewrite to `.../v1beta/models/{model}:method`.
This silently dropped legitimate relay route segments (e.g.
`https://relay.example/v1/models/invoke` → `.../v1beta/models/...:generateContent`,
losing `/invoke`) and sent traffic to the wrong upstream path.
Replace the bare `contains(...)` checks with
`matches_structured_gemini_models_path`, which requires the
`/models/` segment to be followed by a canonical Gemini method call
(`*:generateContent` or `*:streamGenerateContent`). The
`matches_bare_gemini_models_path` helper is generalized (and renamed) to
handle both `/v1beta/models/` and `/v1/models/` alongside the original
bare `/models/` shape.
P2b — Reject empty Gemini OAuth access_tokens before they reach the
bearer header.
`GeminiAdapter::parse_oauth_credentials` accepts refresh-token-only JSON
(and surfaces `{"access_token": "", ...}` for expired credentials) with
`access_token` defaulting to `""`. The Claude adapter's GeminiCli branch
then called `AuthInfo::with_access_token(key, creds.access_token)`
unconditionally, so the bearer-header builder at
`AuthStrategy::GoogleOAuth` resolved to `Authorization: Bearer ` — a
deterministic 401 from upstream.
CC Switch does not currently exchange the refresh_token for a fresh
access_token (`OAuthCredentials::needs_refresh` / `can_refresh` are
annotated `#[allow(dead_code)]`). Until that exists, only attach
`access_token` when it is non-empty; fall back to plain GoogleOAuth
strategy with the raw key and log a warn pointing users at
`~/.gemini/oauth_creds.json` so the failure mode is observable.
Tests:
- gemini_url.rs: three new regressions — opaque `/v1/models/invoke`,
opaque `/v1beta/models/route`, and the positive counter-case where a
structured `/v1/models/...:generateContent` path still normalizes.
- claude.rs: three new `test_extract_auth_gemini_cli_*` tests covering
refresh-only JSON, empty-string access_token JSON, and the valid-JSON
pass-through.
All 839 lib tests pass; cargo fmt + clippy -D warnings clean.
Addresses Codex review P2 findings on #1918.
This commit is contained in:
@@ -63,9 +63,14 @@ fn should_normalize_gemini_full_url(base_url: &str) -> bool {
|
|||||||
}
|
}
|
||||||
|
|
||||||
let path = path.trim_end_matches('/');
|
let path = path.trim_end_matches('/');
|
||||||
path.contains("/v1beta/models/")
|
// Only classify a path as a "structured Gemini endpoint" — safe to
|
||||||
|| path.contains("/v1/models/")
|
// rewrite to /v1beta/models/{model}:method — when the `/models/`
|
||||||
|| matches_bare_gemini_models_path(path)
|
// segment is actually followed by a Gemini `*:generateContent` /
|
||||||
|
// `*:streamGenerateContent` method call. Bare `path.contains("/v1/models/")`
|
||||||
|
// would also match legitimate opaque relay routes like
|
||||||
|
// `/v1/models/invoke`, which are *not* Gemini-structured and must be
|
||||||
|
// preserved as-is.
|
||||||
|
matches_structured_gemini_models_path(path)
|
||||||
|| path.ends_with("/v1beta")
|
|| path.ends_with("/v1beta")
|
||||||
|| path.ends_with("/v1")
|
|| path.ends_with("/v1")
|
||||||
|| path.ends_with("/v1beta/models")
|
|| path.ends_with("/v1beta/models")
|
||||||
@@ -80,8 +85,6 @@ fn should_normalize_gemini_full_url(base_url: &str) -> bool {
|
|||||||
|| path.ends_with("/v1beta/openai/responses")
|
|| path.ends_with("/v1beta/openai/responses")
|
||||||
|| path.ends_with("/v1/openai/responses")
|
|| path.ends_with("/v1/openai/responses")
|
||||||
|| path.ends_with("/openai/responses")
|
|| path.ends_with("/openai/responses")
|
||||||
|| path.contains(":generateContent")
|
|
||||||
|| path.contains(":streamGenerateContent")
|
|
||||||
}
|
}
|
||||||
|
|
||||||
fn split_query(input: &str) -> (&str, Option<&str>) {
|
fn split_query(input: &str) -> (&str, Option<&str>) {
|
||||||
@@ -150,13 +153,25 @@ fn normalize_prefix(prefix: &str) -> String {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
fn matches_bare_gemini_models_path(path: &str) -> bool {
|
/// Returns true when `path` contains a `/models/` segment followed by a
|
||||||
if let Some(idx) = path.find("/models/") {
|
/// canonical Gemini method call (`*:generateContent` or
|
||||||
let after = &path[idx + "/models/".len()..];
|
/// `*:streamGenerateContent`). The `/models/` segment alone is not enough:
|
||||||
after.contains(":generateContent") || after.contains(":streamGenerateContent")
|
/// opaque relay routes such as `/v1/models/invoke` or `/custom/models/foo`
|
||||||
} else {
|
/// also contain `/models/` but are not Gemini-structured and must not be
|
||||||
false
|
/// rewritten.
|
||||||
|
fn matches_structured_gemini_models_path(path: &str) -> bool {
|
||||||
|
let mut cursor = path;
|
||||||
|
while let Some(idx) = cursor.find("/models/") {
|
||||||
|
let after = &cursor[idx + "/models/".len()..];
|
||||||
|
if after.contains(":generateContent") || after.contains(":streamGenerateContent") {
|
||||||
|
return true;
|
||||||
|
}
|
||||||
|
// Advance past this `/models/` occurrence so a later Gemini-style
|
||||||
|
// segment in the same path (unusual but cheap to handle) can still
|
||||||
|
// match.
|
||||||
|
cursor = &cursor[idx + "/models/".len()..];
|
||||||
}
|
}
|
||||||
|
false
|
||||||
}
|
}
|
||||||
|
|
||||||
fn merge_queries(base_query: Option<&str>, endpoint_query: Option<&str>) -> Option<String> {
|
fn merge_queries(base_query: Option<&str>, endpoint_query: Option<&str>) -> Option<String> {
|
||||||
@@ -265,4 +280,50 @@ mod tests {
|
|||||||
|
|
||||||
assert_eq!(url, "https://relay.example/custom/models/invoke?alt=sse");
|
assert_eq!(url, "https://relay.example/custom/models/invoke?alt=sse");
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Regression: a relay whose fixed path starts with `/v1/models/` is an
|
||||||
|
/// opaque route, not a Gemini-structured endpoint. The previous
|
||||||
|
/// heuristic matched any `contains("/v1/models/")` and rewrote it to
|
||||||
|
/// `/v1beta/models/{model}:generateContent`, dropping the relay's own
|
||||||
|
/// route component (`/invoke`) and sending traffic to the wrong place.
|
||||||
|
#[test]
|
||||||
|
fn preserves_opaque_full_url_with_v1_models_prefix() {
|
||||||
|
let url = resolve_gemini_native_url(
|
||||||
|
"https://relay.example/v1/models/invoke",
|
||||||
|
"/v1beta/models/gemini-2.5-flash:streamGenerateContent?alt=sse",
|
||||||
|
true,
|
||||||
|
);
|
||||||
|
|
||||||
|
assert_eq!(url, "https://relay.example/v1/models/invoke?alt=sse");
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Same regression, `/v1beta/models/` variant — relays may use Google's
|
||||||
|
/// path layout defensively for routing while still being opaque.
|
||||||
|
#[test]
|
||||||
|
fn preserves_opaque_full_url_with_v1beta_models_prefix() {
|
||||||
|
let url = resolve_gemini_native_url(
|
||||||
|
"https://relay.example/v1beta/models/route",
|
||||||
|
"/v1beta/models/gemini-2.5-flash:streamGenerateContent?alt=sse",
|
||||||
|
true,
|
||||||
|
);
|
||||||
|
|
||||||
|
assert_eq!(url, "https://relay.example/v1beta/models/route?alt=sse");
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Counter-case: a full URL that *does* carry a genuine Gemini method
|
||||||
|
/// segment (`:generateContent`) under `/v1/models/` should still be
|
||||||
|
/// recognized as structured and normalized to the requested model.
|
||||||
|
#[test]
|
||||||
|
fn normalizes_structured_v1_models_path_with_method_segment() {
|
||||||
|
let url = resolve_gemini_native_url(
|
||||||
|
"https://relay.example/v1/models/gemini-2.5-pro:generateContent",
|
||||||
|
"/v1beta/models/gemini-2.5-flash:streamGenerateContent?alt=sse",
|
||||||
|
true,
|
||||||
|
);
|
||||||
|
|
||||||
|
assert_eq!(
|
||||||
|
url,
|
||||||
|
"https://relay.example/v1beta/models/gemini-2.5-flash:streamGenerateContent?alt=sse"
|
||||||
|
);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -427,12 +427,32 @@ impl ProviderAdapter for ClaudeAdapter {
|
|||||||
|
|
||||||
match provider_type {
|
match provider_type {
|
||||||
ProviderType::GeminiCli => {
|
ProviderType::GeminiCli => {
|
||||||
if let Some(creds) =
|
// Parse stored OAuth JSON and only attach access_token when
|
||||||
super::gemini::GeminiAdapter::new().parse_oauth_credentials(&key)
|
// it's actually usable. `parse_oauth_credentials` accepts
|
||||||
{
|
// refresh-token-only JSON (which is legitimate before the
|
||||||
Some(AuthInfo::with_access_token(key, creds.access_token))
|
// first refresh) and also surfaces `{"access_token": "", ...}`
|
||||||
} else {
|
// for expired credentials. In both cases we would otherwise
|
||||||
Some(AuthInfo::new(key, AuthStrategy::GoogleOAuth))
|
// send `Authorization: Bearer ` to upstream and get a 401.
|
||||||
|
//
|
||||||
|
// CC Switch does not currently exchange the refresh_token for
|
||||||
|
// a fresh access_token. Until that path exists, degrade to
|
||||||
|
// plain GoogleOAuth strategy (which still sends the raw key
|
||||||
|
// as a fallback) and log loudly so users know to refresh
|
||||||
|
// their `~/.gemini/oauth_creds.json`.
|
||||||
|
match super::gemini::GeminiAdapter::new().parse_oauth_credentials(&key) {
|
||||||
|
Some(creds) if !creds.access_token.is_empty() => {
|
||||||
|
Some(AuthInfo::with_access_token(key, creds.access_token))
|
||||||
|
}
|
||||||
|
Some(_) => {
|
||||||
|
log::warn!(
|
||||||
|
"[Gemini OAuth] access_token missing or empty for provider `{}`; \
|
||||||
|
bearer auth will likely fail with 401. Refresh \
|
||||||
|
~/.gemini/oauth_creds.json via the gemini CLI to obtain a new token.",
|
||||||
|
provider.id
|
||||||
|
);
|
||||||
|
Some(AuthInfo::new(key, AuthStrategy::GoogleOAuth))
|
||||||
|
}
|
||||||
|
None => Some(AuthInfo::new(key, AuthStrategy::GoogleOAuth)),
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
ProviderType::Gemini => Some(AuthInfo::new(key, AuthStrategy::Google)),
|
ProviderType::Gemini => Some(AuthInfo::new(key, AuthStrategy::Google)),
|
||||||
@@ -752,6 +772,92 @@ mod tests {
|
|||||||
assert_eq!(auth.strategy, AuthStrategy::ClaudeAuth);
|
assert_eq!(auth.strategy, AuthStrategy::ClaudeAuth);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Regression: a Gemini OAuth credential JSON that carries only a
|
||||||
|
/// refresh_token (no active access_token) must not be surfaced as an
|
||||||
|
/// `AuthInfo` whose bearer would be empty. Without the guard, downstream
|
||||||
|
/// header injection produces `Authorization: Bearer ` and a deterministic
|
||||||
|
/// 401 from upstream.
|
||||||
|
#[test]
|
||||||
|
fn test_extract_auth_gemini_cli_refresh_only_json_does_not_expose_empty_bearer() {
|
||||||
|
let adapter = ClaudeAdapter::new();
|
||||||
|
let refresh_only_json =
|
||||||
|
r#"{"refresh_token":"rt-abc","client_id":"cid","client_secret":"cs"}"#;
|
||||||
|
let provider = create_provider_with_meta(
|
||||||
|
json!({
|
||||||
|
"env": {
|
||||||
|
"ANTHROPIC_BASE_URL": "https://generativelanguage.googleapis.com",
|
||||||
|
"ANTHROPIC_API_KEY": refresh_only_json
|
||||||
|
}
|
||||||
|
}),
|
||||||
|
ProviderMeta {
|
||||||
|
api_format: Some("gemini_native".to_string()),
|
||||||
|
..Default::default()
|
||||||
|
},
|
||||||
|
);
|
||||||
|
|
||||||
|
let auth = adapter.extract_auth(&provider).unwrap();
|
||||||
|
// access_token must not be surfaced as `Some("")` — the OAuth header
|
||||||
|
// builder uses `access_token.as_ref().unwrap_or(&api_key)`, so a
|
||||||
|
// `Some("")` would win over the raw key and emit `Bearer `.
|
||||||
|
assert!(
|
||||||
|
auth.access_token.as_deref().is_none_or(|t| !t.is_empty()),
|
||||||
|
"empty access_token leaked into AuthInfo"
|
||||||
|
);
|
||||||
|
assert_eq!(auth.strategy, AuthStrategy::GoogleOAuth);
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Companion case: a JSON credential with an empty-string `access_token`
|
||||||
|
/// field (the shape an expired credential can take after partial writes)
|
||||||
|
/// must degrade the same way.
|
||||||
|
#[test]
|
||||||
|
fn test_extract_auth_gemini_cli_empty_access_token_degrades_to_raw_key() {
|
||||||
|
let adapter = ClaudeAdapter::new();
|
||||||
|
let expired_json = r#"{"access_token":"","refresh_token":"rt-abc","client_id":"cid","client_secret":"cs"}"#;
|
||||||
|
let provider = create_provider_with_meta(
|
||||||
|
json!({
|
||||||
|
"env": {
|
||||||
|
"ANTHROPIC_BASE_URL": "https://generativelanguage.googleapis.com",
|
||||||
|
"ANTHROPIC_API_KEY": expired_json
|
||||||
|
}
|
||||||
|
}),
|
||||||
|
ProviderMeta {
|
||||||
|
api_format: Some("gemini_native".to_string()),
|
||||||
|
..Default::default()
|
||||||
|
},
|
||||||
|
);
|
||||||
|
|
||||||
|
let auth = adapter.extract_auth(&provider).unwrap();
|
||||||
|
assert!(
|
||||||
|
auth.access_token.as_deref().is_none_or(|t| !t.is_empty()),
|
||||||
|
"empty access_token leaked into AuthInfo"
|
||||||
|
);
|
||||||
|
assert_eq!(auth.strategy, AuthStrategy::GoogleOAuth);
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Counter-case: a well-formed JSON credential with a non-empty
|
||||||
|
/// access_token must still flow through the OAuth path unchanged.
|
||||||
|
#[test]
|
||||||
|
fn test_extract_auth_gemini_cli_valid_json_keeps_access_token() {
|
||||||
|
let adapter = ClaudeAdapter::new();
|
||||||
|
let valid_json = r#"{"access_token":"ya29.valid","refresh_token":"rt"}"#;
|
||||||
|
let provider = create_provider_with_meta(
|
||||||
|
json!({
|
||||||
|
"env": {
|
||||||
|
"ANTHROPIC_BASE_URL": "https://generativelanguage.googleapis.com",
|
||||||
|
"ANTHROPIC_API_KEY": valid_json
|
||||||
|
}
|
||||||
|
}),
|
||||||
|
ProviderMeta {
|
||||||
|
api_format: Some("gemini_native".to_string()),
|
||||||
|
..Default::default()
|
||||||
|
},
|
||||||
|
);
|
||||||
|
|
||||||
|
let auth = adapter.extract_auth(&provider).unwrap();
|
||||||
|
assert_eq!(auth.access_token.as_deref(), Some("ya29.valid"));
|
||||||
|
assert_eq!(auth.strategy, AuthStrategy::GoogleOAuth);
|
||||||
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn test_provider_type_detection() {
|
fn test_provider_type_detection() {
|
||||||
let adapter = ClaudeAdapter::new();
|
let adapter = ClaudeAdapter::new();
|
||||||
|
|||||||
Reference in New Issue
Block a user