Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 4 additions & 12 deletions internal/api/handlers/management/auth_files.go
Original file line number Diff line number Diff line change
Expand Up @@ -928,24 +928,16 @@ func (h *Handler) disableAuth(ctx context.Context, id string) {
if id == "" {
return
}
if auth, ok := h.authManager.GetByID(id); ok {
auth.Disabled = true
auth.Status = coreauth.StatusDisabled
auth.StatusMessage = "removed via management API"
auth.UpdatedAt = time.Now()
_, _ = h.authManager.Update(ctx, auth)
if auth, ok := h.authManager.GetByID(id); ok && auth != nil {
_ = h.authManager.Remove(ctx, auth.ID)
return
}
authID := h.authIDForPath(id)
if authID == "" {
return
}
if auth, ok := h.authManager.GetByID(authID); ok {
auth.Disabled = true
auth.Status = coreauth.StatusDisabled
auth.StatusMessage = "removed via management API"
auth.UpdatedAt = time.Now()
_, _ = h.authManager.Update(ctx, auth)
if _, ok := h.authManager.GetByID(authID); ok {
_ = h.authManager.Remove(ctx, authID)
}
}

Expand Down
70 changes: 70 additions & 0 deletions internal/api/handlers/management/auth_files_delete_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"os"
"path/filepath"
"testing"
"time"

"github.com/gin-gonic/gin"
"github.com/router-for-me/CLIProxyAPI/v6/internal/config"
Expand Down Expand Up @@ -127,3 +128,72 @@ func TestDeleteAuthFile_FallbackToAuthDirPath(t *testing.T) {
t.Fatalf("expected auth file to be removed from auth dir, stat err: %v", errStat)
}
}

func TestDeleteAuthFile_AllowsReaddingSameNameWithoutStaleState(t *testing.T) {
t.Setenv("MANAGEMENT_PASSWORD", "")
gin.SetMode(gin.TestMode)

authDir := t.TempDir()
fileName := "codex-readd.json"
filePath := filepath.Join(authDir, fileName)
fileData := []byte(`{"type":"codex","email":"[email protected]"}`)
if errWrite := os.WriteFile(filePath, fileData, 0o600); errWrite != nil {
t.Fatalf("failed to write auth file: %v", errWrite)
}

store := &memoryAuthStore{}
manager := coreauth.NewManager(store, nil, nil)
authID := fileName
if _, errRegister := manager.Register(context.Background(), &coreauth.Auth{
ID: authID,
FileName: fileName,
Provider: "codex",
Metadata: map[string]any{"type": "codex", "email": "[email protected]"},
Attributes: map[string]string{
"path": filePath,
},
ModelStates: map[string]*coreauth.ModelState{
"gpt-5.3-codex": {
Status: coreauth.StatusError,
Unavailable: true,
NextRetryAfter: time.Now().Add(30 * time.Minute),
},
},
}); errRegister != nil {
t.Fatalf("register auth: %v", errRegister)
}

h := NewHandlerWithoutConfigFilePath(&config.Config{AuthDir: authDir}, manager)
h.tokenStore = store

deleteRec := httptest.NewRecorder()
deleteCtx, _ := gin.CreateTestContext(deleteRec)
deleteReq := httptest.NewRequest(http.MethodDelete, "/v0/management/auth-files?name="+url.QueryEscape(fileName), nil)
deleteCtx.Request = deleteReq
h.DeleteAuthFile(deleteCtx)

if deleteRec.Code != http.StatusOK {
t.Fatalf("expected delete status %d, got %d with body %s", http.StatusOK, deleteRec.Code, deleteRec.Body.String())
}
if _, ok := manager.GetByID(authID); ok {
t.Fatal("expected auth to be fully removed from manager")
}

if errWrite := os.WriteFile(filePath, fileData, 0o600); errWrite != nil {
t.Fatalf("failed to recreate auth file: %v", errWrite)
}
if errRegister := h.registerAuthFromFile(context.Background(), filePath, fileData); errRegister != nil {
t.Fatalf("re-register auth from file: %v", errRegister)
}

updated, ok := manager.GetByID(authID)
if !ok || updated == nil {
t.Fatal("expected re-added auth to exist")
}
if updated.Disabled || updated.Status == coreauth.StatusDisabled {
t.Fatalf("expected re-added auth to be active, got disabled=%v status=%s", updated.Disabled, updated.Status)
}
if len(updated.ModelStates) != 0 {
t.Fatalf("expected stale model state to be cleared, got %#v", updated.ModelStates)
}
}
34 changes: 34 additions & 0 deletions sdk/cliproxy/auth/conductor.go
Original file line number Diff line number Diff line change
Expand Up @@ -481,6 +481,40 @@ func (m *Manager) Update(ctx context.Context, auth *Auth) (*Auth, error) {
return auth.Clone(), nil
}

// Remove deletes an auth entry from runtime state and persistence.
func (m *Manager) Remove(ctx context.Context, id string) error {
id = strings.TrimSpace(id)
if m == nil || id == "" {
return nil
}

var removed *Auth
m.mu.Lock()
if existing, ok := m.auths[id]; ok && existing != nil {
removed = existing.Clone()
}
delete(m.auths, id)
cfg, _ := m.runtimeConfig.Load().(*internalconfig.Config)
if cfg == nil {
cfg = &internalconfig.Config{}
}
m.rebuildAPIKeyModelAliasLocked(cfg)
m.mu.Unlock()

if m.store == nil || shouldSkipPersist(ctx) {
return nil
}
if removed != nil && removed.Attributes != nil {
if v := strings.ToLower(strings.TrimSpace(removed.Attributes["runtime_only"])); v == "true" {
return nil
}
}
if err := m.store.Delete(ctx, id); err != nil {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Delete persisted auth via record path instead of ID

Calling m.store.Delete(ctx, id) here assumes auth IDs are valid delete keys, but file-backed auth IDs can contain path separators (e.g. nested IDs like team/user.json). In FileTokenStore.resolveDeletePath, any ID containing a separator is treated as a direct filesystem path rather than being rooted under AuthDir, so removal can target the wrong location and leave the actual auth file undeleted (or delete an unrelated relative path) when an auth is removed and later reloaded. This regression is introduced by the new Remove persistence path and is triggered whenever removed auth IDs are not simple basename IDs.

Useful? React with 👍 / 👎.

return err
}
return nil
}

// Load resets manager state from the backing store.
func (m *Manager) Load(ctx context.Context) error {
m.mu.Lock()
Expand Down
52 changes: 52 additions & 0 deletions sdk/cliproxy/auth/conductor_update_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package auth
import (
"context"
"testing"
"time"
)

func TestManager_Update_PreservesModelStates(t *testing.T) {
Expand Down Expand Up @@ -47,3 +48,54 @@ func TestManager_Update_PreservesModelStates(t *testing.T) {
t.Fatalf("expected BackoffLevel to be %d, got %d", backoffLevel, state.Quota.BackoffLevel)
}
}

func TestManager_Remove_DropsStaleModelStatesForRecreatedAuth(t *testing.T) {
m := NewManager(nil, nil, nil)
model := "test-model"

if _, errRegister := m.Register(context.Background(), &Auth{
ID: "auth-1",
Provider: "codex",
Metadata: map[string]any{"type": "codex"},
ModelStates: map[string]*ModelState{
model: {
Status: StatusError,
Unavailable: true,
NextRetryAfter: time.Now().Add(30 * time.Minute),
},
},
}); errRegister != nil {
t.Fatalf("register auth: %v", errRegister)
}

if errRemove := m.Remove(context.Background(), "auth-1"); errRemove != nil {
t.Fatalf("remove auth: %v", errRemove)
}
if _, ok := m.GetByID("auth-1"); ok {
t.Fatal("expected auth to be removed")
}

if _, errRegister := m.Register(context.Background(), &Auth{
ID: "auth-1",
Provider: "codex",
Metadata: map[string]any{"type": "codex"},
}); errRegister != nil {
t.Fatalf("re-register auth: %v", errRegister)
}

updated, ok := m.GetByID("auth-1")
if !ok || updated == nil {
t.Fatalf("expected recreated auth to be present")
}
if len(updated.ModelStates) != 0 {
t.Fatalf("expected stale ModelStates to be cleared, got %#v", updated.ModelStates)
}

available, errAvailable := getAvailableAuths([]*Auth{updated}, "codex", model, time.Now())
if errAvailable != nil {
t.Fatalf("getAvailableAuths returned error: %v", errAvailable)
}
if len(available) != 1 || available[0] == nil || available[0].ID != "auth-1" {
t.Fatalf("expected recreated auth to be selectable, got %#v", available)
}
}
11 changes: 2 additions & 9 deletions sdk/cliproxy/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -322,15 +322,8 @@ func (s *Service) applyCoreAuthRemoval(ctx context.Context, id string) {
return
}
GlobalModelRegistry().UnregisterClient(id)
if existing, ok := s.coreManager.GetByID(id); ok && existing != nil {
existing.Disabled = true
existing.Status = coreauth.StatusDisabled
if _, err := s.coreManager.Update(ctx, existing); err != nil {
log.Errorf("failed to disable auth %s: %v", id, err)
}
if strings.EqualFold(strings.TrimSpace(existing.Provider), "codex") {
s.ensureExecutorsForAuth(existing)
}
if err := s.coreManager.Remove(ctx, id); err != nil {
log.Errorf("failed to remove auth %s: %v", id, err)
}
}

Expand Down