History

fix(cliproxy): refine Codex model resolution and credential validation

- Adjusted `resolveConfigCodexStyleKey` to include `validateIndexCredentials` for stricter credential checks. - Enhanced logic in `buildCodexConfigModels` to handle empty model lists more effectively. - Standardized credential matching with helper functions for improved maintainability.

Luis Pater committed 3 months agov7.2.107

Commit b4d94d51 parent 1c1d8ef2 files · +208 −17Browse files

Unified · Split

2 files
  1. sdk/cliproxy/service_codex_models_test.go +188 −0
  2. sdk/cliproxy/service_models.go +20 −17
sdk/cliproxy/service_codex_models_test.go+188−0
@@ -0,0 +1,188 @@
1+package cliproxy
2+
3+import (
4+ "context"
5+ "fmt"
6+ "testing"
7+
8+ internalconfig "github.com/router-for-me/CLIProxyAPI/v7/internal/config"
9+ internalregistry "github.com/router-for-me/CLIProxyAPI/v7/internal/registry"
10+ coreauth "github.com/router-for-me/CLIProxyAPI/v7/sdk/cliproxy/auth"
11+ "github.com/router-for-me/CLIProxyAPI/v7/sdk/config"
12+)
13+
14+func TestRegisterModelsForAuthCodexAPIKeyModels(t *testing.T) {
15+ defaultModels := internalregistry.GetCodexProModels()
16+ if len(defaultModels) == 0 {
17+ t.Fatal("expected Codex Pro default models")
18+ }
19+
20+ excludedModelID := defaultModels[0].ID
21+ tests := []struct {
22+ name string
23+ entry config.CodexKey
24+ wantIDs map[string]struct{}
25+ }{
26+ {
27+ name: "defaults without configuration",
28+ entry: config.CodexKey{APIKey: "default-key"},
29+ wantIDs: codexModelIDSet(defaultModels),
30+ },
31+ {
32+ name: "explicit configuration replaces defaults",
33+ entry: config.CodexKey{
34+ APIKey: "configured-key",
35+ Models: []internalconfig.CodexModel{{
36+ Name: "upstream-codex", Alias: "configured-codex",
37+ }},
38+ },
39+ wantIDs: map[string]struct{}{"configured-codex": {}},
40+ },
41+ {
42+ name: "exclusions apply to defaults",
43+ entry: config.CodexKey{
44+ APIKey: "excluded-key",
45+ ExcludedModels: []string{excludedModelID},
46+ },
47+ wantIDs: codexModelIDSet(defaultModels[1:]),
48+ },
49+ }
50+
51+ for index := range tests {
52+ testCase := tests[index]
53+ t.Run(testCase.name, func(t *testing.T) {
54+ authID := fmt.Sprintf("codex-api-key-models-%d", index)
55+ modelRegistry := internalregistry.GetGlobalRegistry()
56+ modelRegistry.UnregisterClient(authID)
57+ t.Cleanup(func() { modelRegistry.UnregisterClient(authID) })
58+
59+ service := &Service{cfg: &config.Config{CodexKey: []config.CodexKey{testCase.entry}}}
60+ auth := &coreauth.Auth{
61+ ID: authID,
62+ Provider: "codex",
63+ Status: coreauth.StatusActive,
64+ Attributes: map[string]string{
65+ coreauth.AttributeAPIKey: testCase.entry.APIKey,
66+ coreauth.AttributeConfigIndex: "0",
67+ coreauth.AttributeSource: "config:codex:test",
68+ },
69+ }
70+
71+ service.registerModelsForAuth(context.Background(), auth)
72+ gotIDs := codexModelIDSet(modelRegistry.GetModelsForClient(authID))
73+ if len(gotIDs) != len(testCase.wantIDs) {
74+ t.Fatalf("registered model IDs = %#v, want %#v", gotIDs, testCase.wantIDs)
75+ }
76+ for modelID := range testCase.wantIDs {
77+ if _, ok := gotIDs[modelID]; !ok {
78+ t.Errorf("missing registered model %q", modelID)
79+ }
80+ }
81+ })
82+ }
83+}
84+
85+func TestRegisterModelsForAuthCodexAPIKeyDefaultRequiresConfigMatch(t *testing.T) {
86+ defaultIDs := codexModelIDSet(internalregistry.GetCodexProModels())
87+ tests := []struct {
88+ name string
89+ config config.Config
90+ attributes map[string]string
91+ wantIDs map[string]struct{}
92+ }{
93+ {
94+ name: "valid index with unmatched API key",
95+ config: config.Config{CodexKey: []config.CodexKey{{
96+ APIKey: "configured-key",
97+ }}},
98+ attributes: map[string]string{
99+ coreauth.AttributeAPIKey: "stale-key",
100+ coreauth.AttributeConfigIndex: "0",
101+ coreauth.AttributeSource: "config:codex:stale",
102+ },
103+ wantIDs: map[string]struct{}{},
104+ },
105+ {
106+ name: "valid index with unmatched base URL",
107+ config: config.Config{CodexKey: []config.CodexKey{{
108+ APIKey: "configured-key", BaseURL: "https://new.example.com",
109+ }}},
110+ attributes: map[string]string{
111+ coreauth.AttributeAPIKey: "configured-key",
112+ coreauth.AttributeConfigIndex: "0",
113+ coreauth.AttributeSource: "config:codex:stale",
114+ "base_url": "https://old.example.com",
115+ },
116+ wantIDs: map[string]struct{}{},
117+ },
118+ {
119+ name: "stale index falls back to matching credentials",
120+ config: config.Config{CodexKey: []config.CodexKey{
121+ {
122+ APIKey: "wrong-key",
123+ Models: []internalconfig.CodexModel{{Name: "wrong-model"}},
124+ },
125+ {APIKey: "configured-key"},
126+ }},
127+ attributes: map[string]string{
128+ coreauth.AttributeAPIKey: "configured-key",
129+ coreauth.AttributeConfigIndex: "0",
130+ coreauth.AttributeSource: "config:codex:stale",
131+ },
132+ wantIDs: defaultIDs,
133+ },
134+ {
135+ name: "API key ignores OAuth plan type",
136+ config: config.Config{CodexKey: []config.CodexKey{{
137+ APIKey: "configured-key",
138+ }}},
139+ attributes: map[string]string{
140+ coreauth.AttributeAPIKey: "configured-key",
141+ coreauth.AttributeConfigIndex: "0",
142+ coreauth.AttributeSource: "config:codex:test",
143+ "plan_type": "free",
144+ },
145+ wantIDs: defaultIDs,
146+ },
147+ }
148+
149+ for index := range tests {
150+ testCase := tests[index]
151+ t.Run(testCase.name, func(t *testing.T) {
152+ authID := fmt.Sprintf("codex-api-key-config-match-%d", index)
153+ modelRegistry := internalregistry.GetGlobalRegistry()
154+ modelRegistry.UnregisterClient(authID)
155+ modelRegistry.RegisterClient(authID, "codex", []*internalregistry.ModelInfo{{ID: "stale-model"}})
156+ t.Cleanup(func() { modelRegistry.UnregisterClient(authID) })
157+
158+ service := &Service{cfg: &testCase.config}
159+ auth := &coreauth.Auth{
160+ ID: authID,
161+ Provider: "codex",
162+ Status: coreauth.StatusActive,
163+ Attributes: testCase.attributes,
164+ }
165+
166+ service.registerModelsForAuth(context.Background(), auth)
167+ gotIDs := codexModelIDSet(modelRegistry.GetModelsForClient(authID))
168+ if len(gotIDs) != len(testCase.wantIDs) {
169+ t.Fatalf("registered model IDs = %#v, want %#v", gotIDs, testCase.wantIDs)
170+ }
171+ for modelID := range testCase.wantIDs {
172+ if _, ok := gotIDs[modelID]; !ok {
173+ t.Errorf("missing registered model %q", modelID)
174+ }
175+ }
176+ })
177+ }
178+}
179+
180+func codexModelIDSet(models []*internalregistry.ModelInfo) map[string]struct{} {
181+ ids := make(map[string]struct{}, len(models))
182+ for _, model := range models {
183+ if model != nil && model.ID != "" {
184+ ids[model.ID] = struct{}{}
185+ }
186+ }
187+ return ids
188+}
sdk/cliproxy/service_models.go+20−17
@@ -117,10 +117,11 @@ func (s *Service) registerModelsForAuthWithCache(ctx context.Context, a *coreaut · 116 unmodified lines
117117 case "codex":
118118 if authKind == "apikey" {
119119 if entry := s.resolveConfigCodexKey(a); entry != nil {
120− models = buildCodexConfigModels(entry)
120+ models = registry.GetCodexProModels()
121+ if len(entry.Models) > 0 {
122+ models = buildCodexConfigModels(entry)
123+ }
121124 excluded = entry.ExcludedModels
122− } else {
123− models = nil
124125 }
125126 models = applyExcludedModels(models, excluded)
126127 break
@@ -486,39 +487,41 @@ func (s *Service) resolveConfigCodexKey(auth *coreauth.Auth) *config.CodexKey { · 359 unmodified lines
486487 if s == nil || s.cfg == nil {
487488 return nil
488489 }
489− return resolveConfigCodexStyleKey(auth, s.cfg.CodexKey)
490+ return resolveConfigCodexStyleKey(auth, s.cfg.CodexKey, true)
490491 }
491492
492493 func (s *Service) resolveConfigXAIKey(auth *coreauth.Auth) *config.XAIKey {
493494 if s == nil || s.cfg == nil {
494495 return nil
495496 }
496− return resolveConfigCodexStyleKey(auth, s.cfg.XAIKey)
497+ return resolveConfigCodexStyleKey(auth, s.cfg.XAIKey, false)
497498 }
498499
499−func resolveConfigCodexStyleKey(auth *coreauth.Auth, entries []config.CodexKey) *config.CodexKey {
500+func resolveConfigCodexStyleKey(auth *coreauth.Auth, entries []config.CodexKey, validateIndexCredentials bool) *config.CodexKey {
500501 if auth == nil {
501502 return nil
502503 }
503− if entry := configEntryForAuthIndex(auth, entries); entry != nil {
504− return entry
505− }
506504 var attrKey, attrBase string
507505 if auth.Attributes != nil {
508506 attrKey = strings.TrimSpace(auth.Attributes["api_key"])
509507 attrBase = strings.TrimSpace(auth.Attributes["base_url"])
510508 }
511− for i := range entries {
512− entry := &entries[i]
509+ matchesCredentials := func(entry *config.CodexKey) bool {
510+ if entry == nil {
511+ return false
512+ }
513513 cfgKey := strings.TrimSpace(entry.APIKey)
514514 cfgBase := strings.TrimSpace(entry.BaseURL)
515− if attrKey != "" && strings.EqualFold(cfgKey, attrKey) {
516− if cfgBase == "" || strings.EqualFold(cfgBase, attrBase) {
517− return entry
518− }
519− continue
515+ if attrKey != "" {
516+ return strings.EqualFold(cfgKey, attrKey) && (cfgBase == "" || strings.EqualFold(cfgBase, attrBase))
520517 }
521− if attrKey == "" && attrBase != "" && strings.EqualFold(cfgBase, attrBase) {
518+ return attrBase != "" && strings.EqualFold(cfgBase, attrBase)
519+ }
520+ if entry := configEntryForAuthIndex(auth, entries); entry != nil && (!validateIndexCredentials || matchesCredentials(entry)) {
521+ return entry
522+ }
523+ for i := range entries {
524+ if entry := &entries[i]; matchesCredentials(entry) {
522525 return entry
523526 }
524527 }