auth: remove contexts under a single locked Modify

5306712→main·

toothbrush·1mo ago·1 file·+30 added/-28

Review finding: the logout-failsafe rework split context removal into a Load and a separate Modify, reopening the read/write lock gap the contexts package warns about — a concurrent auth use in the window could retarget which context "current" means mid-logout. Selection, keyring deletion, and entry removal now all run inside one locked Modify; a failed credential delete aborts the callback, which discards the entry change and preserves the retry-friendly failure mode.

Co-Authored-By: Claude Fable 5 noreply@anthropic.com

Sessions

02cd9554697cView transcript

Changes

1

• cmd/entire/cli/auth

• Mcontext_store.go+30/-28

12 unmodified lines

// contexts.json entry, clearing current_context. It is a no-op (returns nil)
// when there is no current context. Used by logout.
func RemoveCurrentContext() error {
    f, err := contexts.Load(userdirs.Config())
    if err != nil {
        if err := removeContextLocked(func(f *contexts.File) *contexts.Context {
            return f.Find(f.CurrentContext)
        }); err != nil {
            return fmt.Errorf("remove current context: %w", err)
        }
        current := f.Find(f.CurrentContext)
        if current == nil {
            return nil
        }
        return RemoveContext(current.Name)
    }
    return nil
}

// RemoveContext deletes the named context's keyring tokens, then its
// contexts.json entry. A missing context is a no-op. Used by logout and
// `logout --all-contexts`. File.Delete clears current_context when name was
// the active one, so removing the current context this way also logs it out.
func RemoveContext(name string) error {
    if err := removeContextLocked(func(f *contexts.File) *contexts.Context {
        return f.Find(name)
    }); err != nil {
        return fmt.Errorf("remove context %q: %w", name, err)
    }
    return nil
}

// removeContextLocked deletes the context selected by pick — keyring slots
// first, then the contexts.json entry — inside a single locked Modify, so
// selection, credential deletion, and entry removal can't interleave with a
// concurrent `auth use` or login. A nil pick result is a no-op.

// Credential deletion comes first and is part of the success contract:
// removing the entry and then failing the keyring delete would report
// "Logged out." while the long-lived refresh token survives on the machine,
// mintable by any keyring-capable process. The inverse partial failure
// (slots gone, entry left) is benign — the context reads as not logged in
// mintable by any keyring-capable process. A delete error aborts the Modify,
// leaving the entry intact for a retry. The inverse partial failure (slots
// deleted, entry write fails) is benign — the context reads as not logged in
// and a retried logout no-ops the deletes.
func RemoveContext(name string) error {
    f, err := contexts.Load(userdirs.Config())
    if err != nil {
        return fmt.Errorf("remove context %q: %w", name, err)
    }
    c := f.Find(name)
    if c == nil {
        return nil
    }
    if err := deleteContextKeychain(c.KeychainService, c.Handle); err != nil {
        return fmt.Errorf("remove credentials for %q: %w", name, err)
    }
    if err := contexts.Modify(userdirs.Config(), func(f *contexts.File) (bool, error) {
        if f.Find(name) == nil {
            return false, nil
        }
        f.Delete(name)
        if err := deleteContextKeychain(c.KeychainService, c.Handle); err != nil {
            return false, fmt.Errorf("remove credentials for %q: %w", c.Name, err)
        }
        f.Delete(c.Name)
        return true, nil
    }); err != nil {
        return fmt.Errorf("remove context %q: %w", name, err)
    }
    return nil
}
// deleteContextKeychain removes a context's keyring slots. A missing entry is a no-op.