From b5701f416b64d9c2af6ad3d36c3ed68d8bfa746d Mon Sep 17 00:00:00 2001 From: Luis Pater Date: Sun, 15 Mar 2026 02:48:54 +0800 Subject: [PATCH] Fixed: #2102 fix(auth): ensure unique auth index for shared API keys across providers and credential identities --- .../api/handlers/management/api_tools_test.go | 55 +++++++++++++++ sdk/cliproxy/auth/types.go | 70 +++++++++++++++---- sdk/cliproxy/auth/types_test.go | 63 +++++++++++++++++ 3 files changed, 174 insertions(+), 14 deletions(-) diff --git a/internal/api/handlers/management/api_tools_test.go b/internal/api/handlers/management/api_tools_test.go index 5b0c6369..6ed98c6e 100644 --- a/internal/api/handlers/management/api_tools_test.go +++ b/internal/api/handlers/management/api_tools_test.go @@ -1,6 +1,7 @@ package management import ( + "context" "net/http" "testing" @@ -56,3 +57,57 @@ func TestAPICallTransportInvalidAuthFallsBackToGlobalProxy(t *testing.T) { t.Fatalf("proxy URL = %v, want http://global-proxy.example.com:8080", proxyURL) } } + +func TestAuthByIndexDistinguishesSharedAPIKeysAcrossProviders(t *testing.T) { + t.Parallel() + + manager := coreauth.NewManager(nil, nil, nil) + geminiAuth := &coreauth.Auth{ + ID: "gemini:apikey:123", + Provider: "gemini", + Attributes: map[string]string{ + "api_key": "shared-key", + }, + } + compatAuth := &coreauth.Auth{ + ID: "openai-compatibility:bohe:456", + Provider: "bohe", + Label: "bohe", + Attributes: map[string]string{ + "api_key": "shared-key", + "compat_name": "bohe", + "provider_key": "bohe", + }, + } + + if _, errRegister := manager.Register(context.Background(), geminiAuth); errRegister != nil { + t.Fatalf("register gemini auth: %v", errRegister) + } + if _, errRegister := manager.Register(context.Background(), compatAuth); errRegister != nil { + t.Fatalf("register compat auth: %v", errRegister) + } + + geminiIndex := geminiAuth.EnsureIndex() + compatIndex := compatAuth.EnsureIndex() + if geminiIndex == compatIndex { + t.Fatalf("shared api key produced duplicate auth_index %q", geminiIndex) + } + + h := &Handler{authManager: manager} + + gotGemini := h.authByIndex(geminiIndex) + if gotGemini == nil { + t.Fatal("expected gemini auth by index") + } + if gotGemini.ID != geminiAuth.ID { + t.Fatalf("authByIndex(gemini) returned %q, want %q", gotGemini.ID, geminiAuth.ID) + } + + gotCompat := h.authByIndex(compatIndex) + if gotCompat == nil { + t.Fatal("expected compat auth by index") + } + if gotCompat.ID != compatAuth.ID { + t.Fatalf("authByIndex(compat) returned %q, want %q", gotCompat.ID, compatAuth.ID) + } +} diff --git a/sdk/cliproxy/auth/types.go b/sdk/cliproxy/auth/types.go index 0bfaf11a..8390b051 100644 --- a/sdk/cliproxy/auth/types.go +++ b/sdk/cliproxy/auth/types.go @@ -162,7 +162,60 @@ func stableAuthIndex(seed string) string { return hex.EncodeToString(sum[:8]) } -// EnsureIndex returns a stable index derived from the auth file name or API key. +func (a *Auth) indexSeed() string { + if a == nil { + return "" + } + + if fileName := strings.TrimSpace(a.FileName); fileName != "" { + return "file:" + fileName + } + + providerKey := strings.ToLower(strings.TrimSpace(a.Provider)) + compatName := "" + baseURL := "" + apiKey := "" + source := "" + if a.Attributes != nil { + if value := strings.TrimSpace(a.Attributes["provider_key"]); value != "" { + providerKey = strings.ToLower(value) + } + compatName = strings.ToLower(strings.TrimSpace(a.Attributes["compat_name"])) + baseURL = strings.TrimSpace(a.Attributes["base_url"]) + apiKey = strings.TrimSpace(a.Attributes["api_key"]) + source = strings.TrimSpace(a.Attributes["source"]) + } + + proxyURL := strings.TrimSpace(a.ProxyURL) + hasCredentialIdentity := compatName != "" || baseURL != "" || proxyURL != "" || apiKey != "" || source != "" + if providerKey != "" && hasCredentialIdentity { + parts := []string{"provider=" + providerKey} + if compatName != "" { + parts = append(parts, "compat="+compatName) + } + if baseURL != "" { + parts = append(parts, "base="+baseURL) + } + if proxyURL != "" { + parts = append(parts, "proxy="+proxyURL) + } + if apiKey != "" { + parts = append(parts, "api_key="+apiKey) + } + if source != "" { + parts = append(parts, "source="+source) + } + return "config:" + strings.Join(parts, "\x00") + } + + if id := strings.TrimSpace(a.ID); id != "" { + return "id:" + id + } + + return "" +} + +// EnsureIndex returns a stable index derived from the auth file name or credential identity. func (a *Auth) EnsureIndex() string { if a == nil { return "" @@ -171,20 +224,9 @@ func (a *Auth) EnsureIndex() string { return a.Index } - seed := strings.TrimSpace(a.FileName) - if seed != "" { - seed = "file:" + seed - } else if a.Attributes != nil { - if apiKey := strings.TrimSpace(a.Attributes["api_key"]); apiKey != "" { - seed = "api_key:" + apiKey - } - } + seed := a.indexSeed() if seed == "" { - if id := strings.TrimSpace(a.ID); id != "" { - seed = "id:" + id - } else { - return "" - } + return "" } idx := stableAuthIndex(seed) diff --git a/sdk/cliproxy/auth/types_test.go b/sdk/cliproxy/auth/types_test.go index 8249b063..e7029385 100644 --- a/sdk/cliproxy/auth/types_test.go +++ b/sdk/cliproxy/auth/types_test.go @@ -33,3 +33,66 @@ func TestToolPrefixDisabled(t *testing.T) { t.Error("should return false when set to false") } } + +func TestEnsureIndexUsesCredentialIdentity(t *testing.T) { + t.Parallel() + + geminiAuth := &Auth{ + Provider: "gemini", + Attributes: map[string]string{ + "api_key": "shared-key", + "source": "config:gemini[abc123]", + }, + } + compatAuth := &Auth{ + Provider: "bohe", + Attributes: map[string]string{ + "api_key": "shared-key", + "compat_name": "bohe", + "provider_key": "bohe", + "source": "config:bohe[def456]", + }, + } + geminiAltBase := &Auth{ + Provider: "gemini", + Attributes: map[string]string{ + "api_key": "shared-key", + "base_url": "https://alt.example.com", + "source": "config:gemini[ghi789]", + }, + } + geminiDuplicate := &Auth{ + Provider: "gemini", + Attributes: map[string]string{ + "api_key": "shared-key", + "source": "config:gemini[abc123-1]", + }, + } + + geminiIndex := geminiAuth.EnsureIndex() + compatIndex := compatAuth.EnsureIndex() + altBaseIndex := geminiAltBase.EnsureIndex() + duplicateIndex := geminiDuplicate.EnsureIndex() + + if geminiIndex == "" { + t.Fatal("gemini index should not be empty") + } + if compatIndex == "" { + t.Fatal("compat index should not be empty") + } + if altBaseIndex == "" { + t.Fatal("alt base index should not be empty") + } + if duplicateIndex == "" { + t.Fatal("duplicate index should not be empty") + } + if geminiIndex == compatIndex { + t.Fatalf("shared api key produced duplicate auth_index %q", geminiIndex) + } + if geminiIndex == altBaseIndex { + t.Fatalf("same provider/key with different base_url produced duplicate auth_index %q", geminiIndex) + } + if geminiIndex == duplicateIndex { + t.Fatalf("duplicate config entries should be separated by source-derived seed, got %q", geminiIndex) + } +}