From 4a0131c062cdb0a2a3c084e05a4bbfa733d5b792 Mon Sep 17 00:00:00 2001 From: Kenny Date: Sat, 5 Sep 2026 15:17:06 +0000 Subject: [PATCH] fix(meta): unify credential minting and preserve auth lifecycle --- internal/api/handlers/management/api_tools.go | 212 +++--------------- .../api/handlers/management/api_tools_test.go | 44 +++- internal/auth/meta/meta.go | 59 ++--- internal/auth/meta/meta_auth_test.go | 62 ++++- internal/runtime/executor/meta_executor.go | 39 +--- .../runtime/executor/meta_executor_test.go | 177 ++++++++++++--- sdk/cliproxy/auth/conductor_execution.go | 38 +++- sdk/cliproxy/auth/conductor_lifecycle.go | 14 +- sdk/cliproxy/auth/metadata_merge.go | 6 + sdk/cliproxy/auth/metadata_merge_test.go | 15 ++ 10 files changed, 391 insertions(+), 275 deletions(-) diff --git a/internal/api/handlers/management/api_tools.go b/internal/api/handlers/management/api_tools.go index 4a8c8d354..9e9b5dd0e 100644 --- a/internal/api/handlers/management/api_tools.go +++ b/internal/api/handlers/management/api_tools.go @@ -11,12 +11,11 @@ import ( "time" "github.com/gin-gonic/gin" - metaauth "github.com/router-for-me/CLIProxyAPI/v7/internal/auth/meta" "github.com/router-for-me/CLIProxyAPI/v7/internal/config" + "github.com/router-for-me/CLIProxyAPI/v7/internal/runtime/executor" coreauth "github.com/router-for-me/CLIProxyAPI/v7/sdk/cliproxy/auth" "github.com/router-for-me/CLIProxyAPI/v7/sdk/proxyutil" log "github.com/sirupsen/logrus" - "golang.org/x/sync/singleflight" ) const defaultAPICallTimeout = 60 * time.Second @@ -360,8 +359,6 @@ func (h *Handler) refreshAntigravityOAuthAccessToken(ctx context.Context, auth * return strings.TrimSpace(tokenResp.AccessToken), nil } -var metaManagementMintGroup singleflight.Group - func metaTokenFromAuth(auth *coreauth.Auth) string { if auth == nil { return "" @@ -385,6 +382,29 @@ func metaTokenFromAuth(auth *coreauth.Auth) string { return "" } +// metaManagementPreparer applies the tool's proxy override only to acquisition. +// The saved credential retains its configured proxy. +type metaManagementPreparer struct { + executor *executor.MetaExecutor + proxyURL string +} + +func (p metaManagementPreparer) ShouldPrepareRequestAuth(auth *coreauth.Auth) bool { + return p.executor.ShouldPrepareRequestAuth(auth) +} + +func (p metaManagementPreparer) PrepareRequestAuth(ctx context.Context, auth *coreauth.Auth) (*coreauth.Auth, error) { + proxyURL := auth.ProxyURL + if strings.TrimSpace(p.proxyURL) != "" { + auth.ProxyURL = p.proxyURL + } + updated, err := p.executor.PrepareRequestAuth(ctx, auth) + if updated != nil { + updated.ProxyURL = proxyURL + } + return updated, err +} + func (h *Handler) resolveMetaToken(ctx context.Context, auth *coreauth.Auth, requestProxyURL string) (string, error) { if ctx == nil { ctx = context.Background() @@ -392,183 +412,25 @@ func (h *Handler) resolveMetaToken(ctx context.Context, auth *coreauth.Auth, req if auth == nil { return "", nil } - if tok := metaTokenFromAuth(auth); tok != "" { - return tok, nil + if token := metaTokenFromAuth(auth); token != "" { + return token, nil } - - var dcaToken string - if !coreauth.IsConfigAPIKeyAuth(auth) { - if auth.Metadata != nil { - if d, ok := auth.Metadata["dca_token"].(string); ok && strings.TrimSpace(d) != "" { - dcaToken = strings.TrimSpace(d) - } else if t, ok := auth.Metadata["access_token"].(string); ok && strings.HasPrefix(strings.TrimSpace(t), "dca:") { - dcaToken = strings.TrimSpace(t) - } - } - if dcaToken == "" && auth.Attributes != nil { - if d := strings.TrimSpace(auth.Attributes["dca_token"]); d != "" { - dcaToken = d - } else if t := strings.TrimSpace(auth.Attributes["access_token"]); strings.HasPrefix(t, "dca:") { - dcaToken = t - } - } + var cfg *config.Config + if h != nil { + cfg = h.cfg } - - if dcaToken == "" { + preparer := metaManagementPreparer{executor: executor.NewMetaExecutor(cfg), proxyURL: requestProxyURL} + if !preparer.ShouldPrepareRequestAuth(auth) { return "", nil } - - flightKey := firstNonEmptyString(&auth.ID, &dcaToken) - - mintRes, errMint, _ := metaManagementMintGroup.Do(flightKey, func() (any, error) { - if h != nil && h.authManager != nil { - var latest *coreauth.Auth - if auth.ID != "" { - if a, ok := h.authManager.GetByID(auth.ID); ok && a != nil { - latest = a - } - } - if latest == nil && auth.Index != "" { - latest = h.authByIndex(auth.Index) - } - if latest != nil { - if k := metaTokenFromAuth(latest); k != "" { - return k, nil - } - } - } - - proxyURL := firstNonEmptyString(&requestProxyURL, &auth.ProxyURL) - var cfg *config.Config - if h != nil { - cfg = h.cfg - } - authSvc := metaauth.NewMetaAuthWithProxyURL(cfg, proxyURL) - minted, err := authSvc.MintAPIKey(ctx, dcaToken) - if err != nil { - return "", fmt.Errorf("meta token mint failed: %w", err) - } - if minted == nil || minted.APIKey == "" { - return "", fmt.Errorf("meta token mint returned empty key") - } - - base := auth.Clone() - baseURL := "" - if auth.Attributes != nil { - baseURL = strings.TrimSpace(auth.Attributes["base_url"]) - } - if baseURL == "" { - baseURL = stringValue(auth.Metadata, "base_url") - } - if baseURL == "" { - baseURL = stringValue(auth.Metadata, "api_base_url") - } - if baseURL == "" { - baseURL = metaauth.DefaultAPIBaseURL - } - if mintedURL := strings.TrimSpace(minted.BaseURL); mintedURL != "" { - baseURL = mintedURL - } - if auth.Metadata == nil { - auth.Metadata = make(map[string]any) - } - auth.Metadata["base_url"] = baseURL - auth.Metadata["api_key"] = minted.APIKey - auth.Metadata["access_token"] = minted.APIKey - auth.Metadata["dca_token"] = dcaToken - delete(auth.Metadata, "expired") - if minted.UserEmail != "" { - auth.Metadata["email"] = minted.UserEmail - } - if minted.UserFullName != "" { - auth.Metadata["name"] = minted.UserFullName - } - now := time.Now() - nowStr := now.Format(time.RFC3339) - auth.LastRefreshedAt = now - auth.UpdatedAt = now - auth.Metadata["last_refresh"] = nowStr - - if auth.Attributes == nil { - auth.Attributes = make(map[string]string) - } - auth.Attributes["base_url"] = baseURL - auth.Attributes["api_key"] = minted.APIKey - auth.Attributes["access_token"] = minted.APIKey - - storage := &metaauth.MetaTokenStorage{Type: "meta", AuthKind: "oauth"} - if existing, ok := auth.Storage.(*metaauth.MetaTokenStorage); ok && existing != nil { - copyStorage := *existing - storage = ©Storage - } - storage.APIKey = minted.APIKey - storage.AccessToken = minted.APIKey - storage.DCAToken = dcaToken - storage.Expired = "" - storage.BaseURL = baseURL - storage.LastRefresh = nowStr - storage.Metadata = auth.Metadata - if minted.UserEmail != "" { - storage.Email = minted.UserEmail - } - if minted.UserFullName != "" { - storage.Name = minted.UserFullName - } - auth.Storage = storage - - // Use the configured backend; direct file writes bypass remote token stores. - if !coreauth.IsConfigAPIKeyAuth(auth) { - store := h.tokenStoreWithBaseDir() - if store == nil { - return "", fmt.Errorf("meta token store unavailable") - } - if _, errSave := store.Save(ctx, auth); errSave != nil { - return "", fmt.Errorf("persist meta token: %w", errSave) - } - } - if h != nil && h.authManager != nil { - if _, errUpdate := h.authManager.UpdateRefreshedAuth(coreauth.WithSkipPersist(ctx), base, auth); errUpdate != nil { - return "", fmt.Errorf("update meta auth: %w", errUpdate) - } - } - - return minted.APIKey, nil - }) - if errMint != nil { - return "", errMint + if h == nil || h.authManager == nil || auth.ID == "" { + return "", fmt.Errorf("meta token mint requires a registered credential") } - - key, _ := mintRes.(string) - if h != nil && h.authManager != nil { - var latest *coreauth.Auth - if auth.ID != "" { - if a, ok := h.authManager.GetByID(auth.ID); ok && a != nil { - latest = a - } - } - if latest == nil && auth.Index != "" { - latest = h.authByIndex(auth.Index) - } - if latest != nil { - if auth.Metadata == nil { - auth.Metadata = make(map[string]any) - } - for k, v := range latest.Metadata { - auth.Metadata[k] = v - } - if auth.Attributes == nil { - auth.Attributes = make(map[string]string) - } - for k, v := range latest.Attributes { - auth.Attributes[k] = v - } - auth.Storage = latest.Storage - auth.LastRefreshedAt = latest.LastRefreshedAt - auth.UpdatedAt = latest.UpdatedAt - } + updated, err := h.authManager.PrepareRequestAuth(ctx, preparer, auth) + if err != nil { + return "", err } - - return key, nil + return metaTokenFromAuth(updated), nil } func antigravityTokenNeedsRefresh(metadata map[string]any) bool { diff --git a/internal/api/handlers/management/api_tools_test.go b/internal/api/handlers/management/api_tools_test.go index dabf1070e..2a8a22246 100644 --- a/internal/api/handlers/management/api_tools_test.go +++ b/internal/api/handlers/management/api_tools_test.go @@ -6,7 +6,6 @@ import ( "errors" "net/http" "net/http/httptest" - "runtime" "strings" "sync" "sync/atomic" @@ -14,6 +13,7 @@ import ( "github.com/gin-gonic/gin" "github.com/router-for-me/CLIProxyAPI/v7/internal/config" + "github.com/router-for-me/CLIProxyAPI/v7/internal/runtime/executor" coreauth "github.com/router-for-me/CLIProxyAPI/v7/sdk/cliproxy/auth" sdkconfig "github.com/router-for-me/CLIProxyAPI/v7/sdk/config" ) @@ -524,7 +524,15 @@ func TestResolveMetaToken_ConcurrentSingleflight(t *testing.T) { entryWg.Done() <-ready inFlight.Done() - token, errToken := h.resolveTokenForAuth(context.Background(), h.authByIndex(auth.Index), "") + var token string + var errToken error + if i%2 == 0 { + prepared, err := manager.PrepareRequestAuth(context.Background(), executor.NewMetaExecutor(h.cfg), auth.Clone()) + errToken = err + token = metaTokenFromAuth(prepared) + } else { + token, errToken = h.resolveTokenForAuth(context.Background(), auth.Clone(), "") + } if errToken != nil { t.Errorf("resolveTokenForAuth error: %v", errToken) } @@ -539,9 +547,6 @@ func TestResolveMetaToken_ConcurrentSingleflight(t *testing.T) { <-serverStarted inFlight.Wait() - for i := 0; i < 50; i++ { - runtime.Gosched() - } close(releaseServer) doneWg.Wait() @@ -556,3 +561,32 @@ func TestResolveMetaToken_ConcurrentSingleflight(t *testing.T) { t.Error("live manager did not retain minted key in attributes") } } + +func TestResolveMetaTokenUsesRequestProxyWithoutSavingOverride(t *testing.T) { + proxy := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Header.Get("Authorization") != "Bearer dca:proxy-test" { + t.Error("mint request did not carry the DCA token") + } + _, _ = w.Write([]byte(`{"api_key":"LLM|proxied"}`)) + })) + defer proxy.Close() + t.Setenv("META_MINT_URL", "http://meta.invalid/key") + store := &memoryAuthStore{} + manager := coreauth.NewManager(store, nil, nil) + auth, err := manager.Register(coreauth.WithSkipPersist(context.Background()), &coreauth.Auth{ + ID: "meta-proxy.json", Provider: "meta", ProxyURL: "http://127.0.0.1:1", + Metadata: map[string]any{"dca_token": "dca:proxy-test"}, + }) + if err != nil { + t.Fatal(err) + } + h := &Handler{cfg: &config.Config{}, authManager: manager, tokenStore: store} + token, err := h.resolveMetaToken(context.Background(), auth.Clone(), proxy.URL) + if err != nil || token != "LLM|proxied" { + t.Fatalf("resolve via request proxy: token=%q, err=%v", token, err) + } + live, _ := manager.GetByID(auth.ID) + if live.ProxyURL != auth.ProxyURL { + t.Fatal("request proxy override changed the credential's configured proxy") + } +} diff --git a/internal/auth/meta/meta.go b/internal/auth/meta/meta.go index b441f66a9..3eb355419 100644 --- a/internal/auth/meta/meta.go +++ b/internal/auth/meta/meta.go @@ -173,43 +173,44 @@ func (ts *MetaTokenStorage) SaveTokenToFile(authFilePath string) error { if ts.Name != "" { data["name"] = ts.Name } - if raw, errRead := os.ReadFile(authFilePath); errRead == nil { - var existing map[string]any - if errJSON := json.Unmarshal(raw, &existing); errJSON == nil { - for k, v := range existing { - if _, isCred := metaCredentialFields[k]; isCred { - continue - } - if _, exists := data[k]; !exists { - data[k] = v - } - } + // Managed saves supply the current metadata, including deliberate deletions. + // Only login callers without a metadata snapshot inherit settings from disk. + metadata := ts.Metadata + if metadata == nil { + if raw, errRead := os.ReadFile(authFilePath); errRead == nil { + _ = json.Unmarshal(raw, &metadata) } } - for k, v := range ts.Metadata { + for k, v := range metadata { if _, isCred := metaCredentialFields[k]; isCred { continue } - if _, exists := data[k]; !exists { + // Supplied settings take precedence over storage's older snapshot. + if _, exists := data[k]; !exists || ts.Metadata != nil { data[k] = v } } - file, err := os.Create(authFilePath) + raw, err := json.MarshalIndent(data, "", " ") + if err != nil { + return fmt.Errorf("meta token storage: encode token file: %w", err) + } + file, err := os.CreateTemp(filepath.Dir(authFilePath), ".meta-token-*") if err != nil { return fmt.Errorf("meta token storage: create token file: %w", err) } - defer func() { - if errClose := file.Close(); errClose != nil { - log.Errorf("meta token storage: close token file error: %v", errClose) - } - }() - - encoder := json.NewEncoder(file) - encoder.SetIndent("", " ") - if err = encoder.Encode(data); err != nil { - return fmt.Errorf("meta token storage: write token file: %w", err) + defer func() { _ = os.Remove(file.Name()) }() + if _, errWrite := file.Write(append(raw, '\n')); errWrite != nil { + _ = file.Close() + return fmt.Errorf("meta token storage: write token file: %w", errWrite) } + if errClose := file.Close(); errClose != nil { + return fmt.Errorf("meta token storage: close token file: %w", errClose) + } + if errRename := os.Rename(file.Name(), authFilePath); errRename != nil { + return fmt.Errorf("meta token storage: replace token file: %w", errRename) + } + return nil } @@ -536,7 +537,8 @@ func (a *MetaAuth) CreateTokenStorage(bundle *MetaAuthBundle) *MetaTokenStorage } } -// CredentialFileName derives a deterministic, collision-free file name for the credentials. +// CredentialFileName keeps a readable account name and hashes the original identity +// so distinct emails that sanitize identically cannot overwrite each other. func CredentialFileName(email, sub string) string { clean := strings.TrimSpace(email) if clean != "" { @@ -546,7 +548,12 @@ func CredentialFileName(email, sub string) string { } return '_' }, clean) - return fmt.Sprintf("meta-%s.json", sanitized) + // Leave room for the prefix, identity hash and extension on common filesystems. + if len(sanitized) > 120 { + sanitized = sanitized[:120] + } + hash := sha256.Sum256([]byte(clean)) + return fmt.Sprintf("meta-%s-%s.json", sanitized, hex.EncodeToString(hash[:8])) } cleanSub := strings.TrimSpace(sub) if cleanSub != "" { diff --git a/internal/auth/meta/meta_auth_test.go b/internal/auth/meta/meta_auth_test.go index 3faceccd8..56c7f665d 100644 --- a/internal/auth/meta/meta_auth_test.go +++ b/internal/auth/meta/meta_auth_test.go @@ -7,6 +7,7 @@ import ( "net/http/httptest" "os" "path/filepath" + "strings" "testing" "github.com/router-for-me/CLIProxyAPI/v7/internal/config" @@ -144,7 +145,7 @@ func TestMetaAuth_WaitForAuthorization(t *testing.T) { } func TestCredentialFileName(t *testing.T) { - if got := CredentialFileName("user@example.com", ""); got != "meta-user_example.com.json" { + if got := CredentialFileName("user@example.com", ""); !strings.HasPrefix(got, "meta-user_example.com-") || !strings.HasSuffix(got, ".json") { t.Errorf("unexpected fileName: %s", got) } if got := CredentialFileName("", "12345"); got == "" || !filepath.IsLocal(got) { @@ -298,3 +299,62 @@ func TestCreateTokenStorageUsesMintedBaseURL(t *testing.T) { }) } } + +func TestCredentialFileNameSeparatesAccounts(t *testing.T) { + first := CredentialFileName("alice+work@example.com", "dca:first") + second := CredentialFileName("alice_work@example.com", "dca:second") + if first == second { + t.Fatal("distinct accounts overwrite the same file") + } + if first != CredentialFileName("alice+work@example.com", "dca:replacement") { + t.Fatal("re-login changed account file") + } + long := CredentialFileName(strings.Repeat("a", 300)+"@example.com", "") + if len(long) > 255 || !filepath.IsLocal(long) { + t.Fatal("filename cannot be saved on common filesystems") + } +} + +func TestSaveTokenToFileUsesCurrentSettings(t *testing.T) { + path := filepath.Join(t.TempDir(), "meta.json") + if err := os.WriteFile(path, []byte(`{"disabled":false,"priority":1,"notes":"removed"}`), 0600); err != nil { + t.Fatal(err) + } + storage := &MetaTokenStorage{AccessToken: "LLM|key", APIKey: "LLM|key", Metadata: map[string]any{"disabled": true, "priority": float64(2)}} + if err := storage.SaveTokenToFile(path); err != nil { + t.Fatal(err) + } + raw, err := os.ReadFile(path) + if err != nil { + t.Fatal(err) + } + var saved map[string]any + if err := json.Unmarshal(raw, &saved); err != nil { + t.Fatal(err) + } + if saved["disabled"] != true || saved["priority"] != float64(2) { + t.Fatalf("restored old settings: %s", raw) + } + if _, exists := saved["notes"]; exists { + t.Fatal("restored a deleted setting") + } +} + +func TestSaveTokenToFilePreservesFileOnEncodingFailure(t *testing.T) { + path := filepath.Join(t.TempDir(), "meta.json") + initial := `{"access_token":"LLM|previous"}` + if err := os.WriteFile(path, []byte(initial), 0600); err != nil { + t.Fatal(err) + } + storage := &MetaTokenStorage{AccessToken: "LLM|new", Metadata: map[string]any{"invalid": make(chan int)}} + if err := storage.SaveTokenToFile(path); err == nil { + t.Fatal("expected encoding failure") + } + raw, err := os.ReadFile(path) + if err != nil { + t.Fatal(err) + } + if string(raw) != initial { + t.Fatalf("failed save corrupted existing credential: %q", raw) + } +} diff --git a/internal/runtime/executor/meta_executor.go b/internal/runtime/executor/meta_executor.go index 6f81d9e72..4fb0dadfa 100644 --- a/internal/runtime/executor/meta_executor.go +++ b/internal/runtime/executor/meta_executor.go @@ -208,46 +208,27 @@ func (e *MetaExecutor) Refresh(ctx context.Context, auth *cliproxyauth.Auth) (*c auth.Attributes["api_key"] = minted.APIKey auth.Attributes["access_token"] = minted.APIKey - var storage *metaauth.MetaTokenStorage - if ms, ok := auth.Storage.(*metaauth.MetaTokenStorage); ok && ms != nil { - storage = ms + // Auth.Clone does not clone Storage. Keep the candidate isolated from live + // login storage; file-loaded records already persist through Metadata. + if existing, ok := auth.Storage.(*metaauth.MetaTokenStorage); ok && existing != nil { + storage := *existing storage.APIKey = minted.APIKey storage.AccessToken = minted.APIKey storage.DCAToken = dcaToken storage.Expired = "" + storage.BaseURL = baseURL + storage.LastRefresh = nowStr + storage.Metadata = auth.Metadata if minted.UserEmail != "" { storage.Email = minted.UserEmail } if minted.UserFullName != "" { storage.Name = minted.UserFullName } - storage.LastRefresh = nowStr - } else { - storage = &metaauth.MetaTokenStorage{ - Type: "meta", - AuthKind: "oauth", - AccessToken: minted.APIKey, - APIKey: minted.APIKey, - DCAToken: dcaToken, - Email: minted.UserEmail, - Name: minted.UserFullName, - LastRefresh: nowStr, - Metadata: auth.Metadata, - } - auth.Storage = storage - } - - storage.BaseURL = baseURL - - filePath := strings.TrimSpace(auth.Attributes[cliproxyauth.AttributePath]) - if filePath == "" { - filePath = strings.TrimSpace(auth.FileName) - } - if filePath != "" { - if errSave := storage.SaveTokenToFile(filePath); errSave != nil { - log.Warnf("meta executor: failed to persist refreshed token to %s: %v", filePath, errSave) - } + auth.Storage = &storage } + // Only the manager may accept and persist the candidate. + auth.LastRefreshedAt = time.Now() return auth, nil } diff --git a/internal/runtime/executor/meta_executor_test.go b/internal/runtime/executor/meta_executor_test.go index 7d36809d5..50393ff7d 100644 --- a/internal/runtime/executor/meta_executor_test.go +++ b/internal/runtime/executor/meta_executor_test.go @@ -8,14 +8,15 @@ import ( "net/http/httptest" "os" "path/filepath" - "runtime" "sync" "sync/atomic" "testing" "time" + metaauth "github.com/router-for-me/CLIProxyAPI/v7/internal/auth/meta" "github.com/router-for-me/CLIProxyAPI/v7/internal/config" "github.com/router-for-me/CLIProxyAPI/v7/internal/registry" + sdkauth "github.com/router-for-me/CLIProxyAPI/v7/sdk/auth" cliproxyauth "github.com/router-for-me/CLIProxyAPI/v7/sdk/cliproxy/auth" cliproxyexecutor "github.com/router-for-me/CLIProxyAPI/v7/sdk/cliproxy/executor" sdktranslator "github.com/router-for-me/CLIProxyAPI/v7/sdk/translator" @@ -198,7 +199,7 @@ func mapToJSON(m map[string]any) string { return string(b) } -func TestMetaExecutor_Refresh_DCA_MintAndPersist(t *testing.T) { +func TestMetaExecutor_Refresh_RequiresManagerAcceptance(t *testing.T) { tempDir := t.TempDir() authFilePath := filepath.Join(tempDir, "meta-test.json") initialContent := `{"type":"meta","auth_kind":"oauth","access_token":"dca:initial-dca","dca_token":"dca:initial-dca","expired":"2020-01-01T00:00:00Z","request-retry":3}` @@ -223,6 +224,8 @@ func TestMetaExecutor_Refresh_DCA_MintAndPersist(t *testing.T) { t.Setenv("META_MINT_URL", server.URL+"/key") auth := &cliproxyauth.Auth{ + ID: "meta-test.json", + FileName: "meta-test.json", Provider: "meta", Attributes: map[string]string{ cliproxyauth.AttributePath: authFilePath, @@ -233,8 +236,17 @@ func TestMetaExecutor_Refresh_DCA_MintAndPersist(t *testing.T) { }, } + store := sdkauth.NewFileTokenStore() + store.SetBaseDir(tempDir) + manager := cliproxyauth.NewManager(store, nil, nil) + auth.Metadata["request-retry"] = float64(3) + auth.Storage = &metaauth.MetaTokenStorage{AccessToken: "dca:initial-dca", DCAToken: "dca:initial-dca", Expired: "2020-01-01T00:00:00Z"} + auth, errRegister := manager.Register(cliproxyauth.WithSkipPersist(context.Background()), auth) + if errRegister != nil { + t.Fatal(errRegister) + } exec := NewMetaExecutor(&config.Config{}) - refreshed, err := exec.Refresh(context.Background(), auth) + refreshed, err := exec.Refresh(context.Background(), auth.Clone()) if err != nil { t.Fatalf("exec.Refresh error: %v", err) } @@ -249,6 +261,20 @@ func TestMetaExecutor_Refresh_DCA_MintAndPersist(t *testing.T) { t.Errorf("expected expired to be removed from metadata after minting") } + unchanged, errRead := os.ReadFile(authFilePath) + if errRead != nil { + t.Fatal(errRead) + } + if string(unchanged) != initialContent { + t.Fatal("executor wrote candidate before manager acceptance") + } + live, _ := manager.GetByID(auth.ID) + if live.Storage.(*metaauth.MetaTokenStorage).AccessToken != "dca:initial-dca" { + t.Fatal("refresh mutated live token storage") + } + if _, err := manager.UpdateRefreshedAuth(context.Background(), auth, refreshed); err != nil { + t.Fatal(err) + } // Verify durable file persistence on disk diskBytes, errRead := os.ReadFile(authFilePath) if errRead != nil { @@ -265,12 +291,12 @@ func TestMetaExecutor_Refresh_DCA_MintAndPersist(t *testing.T) { t.Errorf("expected persisted access_token LLM|persisted-minted-key, got %v", diskData["access_token"]) } // Verify existing properties preserved - if diskData["request-retry"] != float64(3) { - t.Errorf("expected preserved request-retry: 3, got %v", diskData["request-retry"]) + if diskData["request_retry"] != float64(3) { + t.Errorf("expected preserved request-retry: 3, got %v", diskData["request_retry"]) } } -func TestMetaExecutor_Refresh_SingleflightAndMultiAccount(t *testing.T) { +func TestMetaExecutor_PrepareConcurrentAccounts(t *testing.T) { var count1, count2 int64 serverStarted := make(chan struct{}) @@ -312,10 +338,15 @@ func TestMetaExecutor_Refresh_SingleflightAndMultiAccount(t *testing.T) { // 10 concurrent refreshes for account 1 var wg sync.WaitGroup auth1 := &cliproxyauth.Auth{ + ID: "meta-acct1", Provider: "meta", Metadata: map[string]any{"dca_token": "dca:acct1"}, } + manager := cliproxyauth.NewManager(nil, nil, nil) + if _, err := manager.Register(context.Background(), auth1); err != nil { + t.Fatal(err) + } const goroutines = 10 var entryWg sync.WaitGroup entryWg.Add(goroutines) @@ -330,7 +361,7 @@ func TestMetaExecutor_Refresh_SingleflightAndMultiAccount(t *testing.T) { entryWg.Done() <-ready inFlight.Done() - res, err := exec.Refresh(context.Background(), auth1.Clone()) + res, err := manager.PrepareRequestAuth(context.Background(), exec, auth1.Clone()) if err != nil { t.Errorf("Refresh acct1 error: %v", err) } @@ -348,12 +379,12 @@ func TestMetaExecutor_Refresh_SingleflightAndMultiAccount(t *testing.T) { <-serverStarted inFlight.Wait() - // Allow pending goroutines to enter singleflight.Do while the server holds the in-flight request. - for i := 0; i < 50; i++ { - runtime.Gosched() + // A second account must complete while the first account's mint is blocked. + auth2 := &cliproxyauth.Auth{ID: "meta-acct2", Provider: "meta", Metadata: map[string]any{"dca_token": "dca:acct2"}} + if _, err := manager.Register(context.Background(), auth2); err != nil { + t.Fatal(err) } - - // Release the HTTP server handler to complete the single in-flight mint. + res2, err2 := manager.PrepareRequestAuth(context.Background(), exec, auth2.Clone()) close(releaseServer) wg.Wait() @@ -361,12 +392,6 @@ func TestMetaExecutor_Refresh_SingleflightAndMultiAccount(t *testing.T) { t.Errorf("expected exactly 1 singleflight mint request for acct1, got %d", totalMint1) } - // Account 2 refreshes independently - auth2 := &cliproxyauth.Auth{ - Provider: "meta", - Metadata: map[string]any{"dca_token": "dca:acct2"}, - } - res2, err2 := exec.Refresh(context.Background(), auth2.Clone()) if err2 != nil { t.Fatalf("Refresh acct2 error: %v", err2) } @@ -504,17 +529,13 @@ func TestMetaExecutorRefreshUsesMintedBaseURL(t *testing.T) { if got, _ := metaCreds(updated); got != tc.want { t.Errorf("request base URL = %q, want %q", got, tc.want) } - raw, err := os.ReadFile(path) - if err != nil { - t.Fatal(err) + if updated.Metadata["base_url"] != tc.want { + t.Errorf("candidate base URL = %v, want %q", updated.Metadata["base_url"], tc.want) } - var saved map[string]any - if err = json.Unmarshal(raw, &saved); err != nil { - t.Fatal(err) - } - if saved["base_url"] != tc.want { - t.Errorf("saved base URL = %v, want %q", saved["base_url"], tc.want) + if _, err := os.Stat(path); !os.IsNotExist(err) { + t.Fatalf("refresh created a file: %v", err) } + }) } } @@ -616,7 +637,7 @@ func TestMetaExecutor_RequestAuthPreparer(t *testing.T) { } authDCAOnly := &cliproxyauth.Auth{ - ID: "meta-prep-test", + ID: "meta-prep-test.json", Provider: "meta", Metadata: map[string]any{ "dca_token": "dca:valid", @@ -630,7 +651,9 @@ func TestMetaExecutor_RequestAuthPreparer(t *testing.T) { t.Errorf("ShouldPrepareRequestAuth should be true when only dca_token is present") } - manager := cliproxyauth.NewManager(nil, nil, nil) + store := sdkauth.NewFileTokenStore() + store.SetBaseDir(t.TempDir()) + manager := cliproxyauth.NewManager(store, nil, nil) manager.RegisterExecutor(exec) if _, err := manager.Register(context.Background(), authDCAOnly); err != nil { t.Fatal(err) @@ -662,6 +685,14 @@ func TestMetaExecutor_RequestAuthPreparer(t *testing.T) { t.Errorf("expected stored api_key 'LLM|prepared-key', got %q", key) } + reloaded, err := store.List(context.Background()) + if err != nil || len(reloaded) != 1 { + t.Fatalf("reload auth: %v, records=%d", err, len(reloaded)) + } + if reloaded[0].Metadata["api_key"] != "LLM|prepared-key" { + t.Fatal("minted key was not persisted for restart") + } + _, err = manager.Execute(context.Background(), []string{"meta"}, req, cliproxyexecutor.Options{SourceFormat: sdktranslator.FromString("openai")}) if err != nil { t.Fatalf("second execute failed: %v", err) @@ -670,3 +701,91 @@ func TestMetaExecutor_RequestAuthPreparer(t *testing.T) { t.Fatalf("mint count after second request = %d, want 1", mints.Load()) } } + +func TestMetaMintRejectsRemovedOrReloadedCredential(t *testing.T) { + for _, prepare := range []bool{false, true} { + for _, action := range []string{"remove", "reload"} { + name := action + "/refresh" + if prepare { + name = action + "/prepare" + } + t.Run(name, func(t *testing.T) { + started := make(chan struct{}) + release := make(chan struct{}) + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + close(started) + <-release + _, _ = w.Write([]byte(`{"api_key":"LLM|obsolete"}`)) + })) + defer server.Close() + var releaseOnce sync.Once + defer releaseOnce.Do(func() { close(release) }) + t.Setenv("META_MINT_URL", server.URL) + dir := t.TempDir() + store := sdkauth.NewFileTokenStore() + store.SetBaseDir(dir) + manager := cliproxyauth.NewManager(store, nil, nil) + storage := &metaauth.MetaTokenStorage{AccessToken: "dca:initial", DCAToken: "dca:initial"} + auth, err := manager.Register(context.Background(), &cliproxyauth.Auth{ID: "meta-lifecycle.json", Provider: "meta", Storage: storage, Metadata: map[string]any{"type": "meta", "access_token": "dca:initial", "dca_token": "dca:initial"}}) + if err != nil { + t.Fatal(err) + } + exec := NewMetaExecutor(nil) + done := make(chan error, 1) + go func() { + if prepare { + _, err := manager.PrepareRequestAuth(context.Background(), exec, auth.Clone()) + done <- err + return + } + updated, err := exec.Refresh(context.Background(), auth.Clone()) + if err == nil { + _, err = manager.UpdateRefreshedAuth(context.Background(), auth, updated) + } + done <- err + }() + <-started + path := filepath.Join(dir, auth.ID) + if action == "remove" { + manager.Remove(context.Background(), auth.ID) + if err := store.Delete(context.Background(), auth.ID); err != nil { + t.Fatal(err) + } + } else { + _, err := manager.Register(context.Background(), &cliproxyauth.Auth{ID: auth.ID, Provider: "meta", Metadata: map[string]any{"type": "meta", "api_key": "LLM|reloaded", "access_token": "LLM|reloaded", "dca_token": "dca:reloaded"}}) + if err != nil { + t.Fatal(err) + } + } + releaseOnce.Do(func() { close(release) }) + err = <-done + if (prepare || action == "reload") && err == nil { + t.Fatal("obsolete mint was accepted") + } + if storage.AccessToken != "dca:initial" { + t.Fatal("obsolete mint changed shared login storage") + } + raw, errRead := os.ReadFile(path) + if action == "remove" { + if !os.IsNotExist(errRead) { + t.Fatalf("removed auth was recreated: %s, %v", raw, errRead) + } + if _, ok := manager.GetByID(auth.ID); ok { + t.Fatal("removed auth was reinstalled") + } + } else { + if errRead != nil { + t.Fatal(errRead) + } + var saved map[string]any + if err := json.Unmarshal(raw, &saved); err != nil { + t.Fatal(err) + } + if saved["api_key"] != "LLM|reloaded" { + t.Fatalf("obsolete mint overwrote reload: %s", raw) + } + } + }) + } + } +} diff --git a/sdk/cliproxy/auth/conductor_execution.go b/sdk/cliproxy/auth/conductor_execution.go index 2be0a2e71..3b4b250d1 100644 --- a/sdk/cliproxy/auth/conductor_execution.go +++ b/sdk/cliproxy/auth/conductor_execution.go @@ -1387,7 +1387,17 @@ func (m *Manager) prepareRequestAuth(ctx context.Context, executor ProviderExecu return auth, nil } preparer, ok := executor.(RequestAuthPreparer) - if !ok || preparer == nil || !preparer.ShouldPrepareRequestAuth(auth) { + if !ok { + return auth, nil + } + + return m.PrepareRequestAuth(ctx, preparer, auth) +} + +// PrepareRequestAuth prepares a registered credential using the same serialization +// and lifecycle checks as normal request execution. Management tools use this path too. +func (m *Manager) PrepareRequestAuth(ctx context.Context, preparer RequestAuthPreparer, auth *Auth) (*Auth, error) { + if m == nil || preparer == nil || auth == nil || !preparer.ShouldPrepareRequestAuth(auth) { return auth, nil } @@ -1396,21 +1406,28 @@ func (m *Manager) prepareRequestAuth(ctx context.Context, executor ProviderExecu return preparer.PrepareRequestAuth(ctx, auth.Clone()) } - lockValue, _ := m.requestPrepareLocks.LoadOrStore(id, &requestAuthPrepareLock{}) - lock, ok := lockValue.(*requestAuthPrepareLock) - if !ok || lock == nil { - return preparer.PrepareRequestAuth(ctx, auth.Clone()) + var prepareMu *sync.Mutex + if strings.EqualFold(strings.TrimSpace(auth.Provider), "meta") { + // Meta also mints on 401 recovery. Serialize both paths per credential. + lockValue, _ := m.refreshLocks.LoadOrStore(id, &authRefreshLock{}) + prepareMu = &lockValue.(*authRefreshLock).mu + } else { + lockValue, _ := m.requestPrepareLocks.LoadOrStore(id, &requestAuthPrepareLock{}) + prepareMu = &lockValue.(*requestAuthPrepareLock).mu } - - lock.mu.Lock() - defer lock.mu.Unlock() + prepareMu.Lock() + defer prepareMu.Unlock() target := auth.Clone() m.mu.RLock() - if current := m.auths[id]; current != nil { + current := m.auths[id] + if current != nil { target = current.Clone() } m.mu.RUnlock() + if current == nil && strings.EqualFold(strings.TrimSpace(auth.Provider), "meta") { + return nil, fmt.Errorf("prepare meta auth: credential no longer registered") + } if !preparer.ShouldPrepareRequestAuth(target) { return target, nil @@ -1432,6 +1449,9 @@ func (m *Manager) prepareRequestAuth(ctx context.Context, executor ProviderExecu if saved != nil { return saved, nil } + if strings.EqualFold(strings.TrimSpace(auth.Provider), "meta") { + return nil, fmt.Errorf("prepare meta auth: credential removed during mint") + } return target, nil } diff --git a/sdk/cliproxy/auth/conductor_lifecycle.go b/sdk/cliproxy/auth/conductor_lifecycle.go index c1125419b..bd3bde600 100644 --- a/sdk/cliproxy/auth/conductor_lifecycle.go +++ b/sdk/cliproxy/auth/conductor_lifecycle.go @@ -224,6 +224,16 @@ func (m *Manager) updateInternal(ctx context.Context, base, auth *Auth, mode upd cooldownStateChanged = clearCooldownStateForAuth(auth, now) || cooldownStateChanged } auth.EnsureIndex() + // A minted Meta key must reach the configured store before requests can use it. + // Keep the epoch check, save and installation together so a concurrent reload + // or removal cannot let an obsolete mint overwrite the credential on disk. + persistMetaMint := (mode == updateModePrepare || mode == updateModeRefresh) && strings.EqualFold(strings.TrimSpace(auth.Provider), "meta") + if persistMetaMint { + if errPersist := m.persist(ctx, auth); errPersist != nil { + m.mu.Unlock() + return nil, fmt.Errorf("persist meta auth: %w", errPersist) + } + } authClone := auth.Clone() m.auths[auth.ID] = authClone m.mu.Unlock() @@ -234,7 +244,9 @@ func (m *Manager) updateInternal(ctx context.Context, base, auth *Auth, mode upd m.scheduler.upsertAuth(authClone.Clone()) } m.queueRefreshReschedule(auth.ID) - _ = m.persist(ctx, auth) + if !persistMetaMint { + _ = m.persist(ctx, auth) + } m.hook.OnAuthUpdated(ctx, auth.Clone()) if cooldownStateChanged { m.persistCooldownStates(context.Background()) diff --git a/sdk/cliproxy/auth/metadata_merge.go b/sdk/cliproxy/auth/metadata_merge.go index ddcb770b0..310b6ac4d 100644 --- a/sdk/cliproxy/auth/metadata_merge.go +++ b/sdk/cliproxy/auth/metadata_merge.go @@ -32,6 +32,12 @@ func MergeExistingAuthMetadata(target *Auth, existingMap map[string]any) { if IsAuthTokenPayloadKey(k) { continue } + if strings.EqualFold(strings.TrimSpace(target.Provider), "meta") { + switch CanonicalCredentialMetadataKey(k) { + case "api_key", "dca_token", "dca_expired", "dca_expires_at": + continue + } + } if _, exists := target.Metadata[k]; !exists { target.Metadata[k] = v } diff --git a/sdk/cliproxy/auth/metadata_merge_test.go b/sdk/cliproxy/auth/metadata_merge_test.go index 33e4fe630..c9c089d00 100644 --- a/sdk/cliproxy/auth/metadata_merge_test.go +++ b/sdk/cliproxy/auth/metadata_merge_test.go @@ -404,3 +404,18 @@ func TestMergeRefreshedAuth(t *testing.T) { } }) } + +func TestMergeExistingAuthMetadataMetaDoesNotRestoreOldKey(t *testing.T) { + // A new device login can succeed while API key minting fails. + // Its DCA credential must not inherit the previous login's API key. + auth := &Auth{Provider: "meta", Metadata: map[string]any{"access_token": "dca:new", "dca_token": "dca:new"}} + MergeExistingAuthMetadata(auth, map[string]any{"api_key": "LLM|old", "dca_expired": "old expiry", "dca_expires_at": 42, "priority": 3}) + for _, key := range []string{"api_key", "dca_expired", "dca_expires_at"} { + if _, exists := auth.Metadata[key]; exists { + t.Fatalf("restored old Meta credential field %s", key) + } + } + if auth.Metadata["priority"] != 3 || auth.Metadata["dca_token"] != "dca:new" { + t.Fatalf("incorrect merged metadata: %#v", auth.Metadata) + } +}