tokenstore: bound OS keyring calls with a timeout · Entire

tokenstore: bound OS keyring calls with a timeout

613cdaf→main

The keyring-timeout shim deleted in 37de13a6d only ever wrapped the legacy auth.Store; the surviving tokenstore keyringStore called keyring.Get/Set/Delete with no deadline, even on main. The legacy store's dual-write was the only thing that surfaced a hung keyring (via its 5s timeout) during login — with it gone, login/logout/status can block forever on a headless Linux box with no Secret Service daemon, a suppressed Keychain prompt, or a stuck Credential Manager.

Move the timeout down to where the raw keyring calls now live: every keyringStore op runs in a goroutine bounded by ENTIRE_KEYRING_TIMEOUT (default 5s). The inner error — including ErrNotFound — propagates unchanged on the fast path; only the timeout branch wraps, naming the platform's keyring backend and the override env var. The file backend is left untouched: it can't hang on a daemon.

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

Sessions

589410b3edffView transcript

Changes

3

package tokenstore

import (
    "context"
    "fmt"
    "os"
    "runtime"
    "time"
)

// defaultKeyringTimeout caps how long every OS keyring call may take.
// The underlying keyring API (Secret Service on Linux, Keychain on
// macOS, Credential Manager on Windows) can block indefinitely when no
// provider is reachable — a headless SSH/container/WSL session, a
// suppressed Keychain prompt, a stuck Credential Manager — and that
// freezes the CLI. 5s is comfortably longer than any healthy
// round-trip while still surfacing the hang to the user quickly.
const defaultKeyringTimeout = 5 * time.Second

// keyringTimeoutEnvVar overrides defaultKeyringTimeout. Accepts any
// time.ParseDuration string; invalid or non-positive values fall back
// to the default. Useful on slow keyrings or to extend the wait on a
// system where the secret service is just sluggish.
const keyringTimeoutEnvVar = "ENTIRE_KEYRING_TIMEOUT"

func keyringTimeout() time.Duration {
    v := os.Getenv(keyringTimeoutEnvVar)
    if v == "" {
        return defaultKeyringTimeout
    }
    d, err := time.ParseDuration(v)
    if err != nil || d <= 0 {
        return defaultKeyringTimeout
    }
    return d
}

// callKeyringWithTimeout runs fn in a goroutine and returns its result,
// or a descriptive error if the configured keyring timeout elapses
// first. The goroutine continues running — a blocked D-Bus syscall
// can't be cancelled from Go — and its eventual result is discarded.
// The buffered result channel keeps the goroutine from leaking forever
// waiting to publish into a receiver that's already gone. fn's own
// error (including ErrNotFound) propagates unchanged on the fast path;
// only the timeout branch wraps.
func callKeyringWithTimeout(op string, fn func() (string, error)) (string, error) {
    ctx, cancel := context.WithTimeout(context.Background(), keyringTimeout())
    defer cancel()

type result struct {
        val string
        err error
    }
    ch := make(chan result, 1)
    go func() {
        v, err := fn()
        ch <- result{val: v, err: err}
    }()
    select {
    case r := <-ch:
        return r.val, r.err
    case <-ctx.Done():
        return "", fmt.Errorf(
            "%s timed out: OS keyring (%s) appears unavailable; set %s to a longer duration to wait further: %w",
            op, keyringProviderName(), keyringTimeoutEnvVar, ctx.Err(),
        )
    }
}

// keyringProviderName returns the human name of the OS keyring backend
// for the current platform, so the timeout error can point the user at
// the specific service that's likely stuck (Keychain on macOS,
// Credential Manager on Windows, Secret Service on Linux/BSD). The
// fallback for unrecognised GOOS is the generic "OS keyring".
func keyringProviderName() string {
    switch runtime.GOOS {
    case "darwin":
        return "macOS Keychain"
    case "windows":
        return "Windows Credential Manager"
    case "linux", "freebsd", "openbsd", "netbsd", "dragonfly":
        return "Secret Service (D-Bus)"
    default:
        return "OS keyring"
    }
}

package tokenstore

import (
    "context"
    "errors"
    "strings"
    "testing"
    "time"
)

func TestCallKeyringWithTimeout_ReturnsValueWhenFast(t *testing.T) {
    t.Parallel()

got, err := callKeyringWithTimeout("get", func() (string, error) {
        return "token", nil
    })
    if err != nil {
        t.Fatalf("unexpected error: %v", err)
    }
    if got != "token" {
        t.Fatalf("got = %q, want %q", got, "token")
    }
}

func TestCallKeyringWithTimeout_PropagatesInnerError(t *testing.T) {
    t.Parallel()

sentinel := errors.New("backend exploded")
    _, err := callKeyringWithTimeout("get", func() (string, error) {
        return "", sentinel
    })
    if !errors.Is(err, sentinel) {
        t.Fatalf("got %v, want %v wrapped", err, sentinel)
    }
}

// A missing-credential error must reach the caller unchanged so the
// errors.Is(err, ErrNotFound) checks scattered through the auth package
// keep working through the timeout wrapper.
func TestCallKeyringWithTimeout_PropagatesNotFound(t *testing.T) {
    t.Parallel()

_, err := callKeyringWithTimeout("get", func() (string, error) {
        return "", ErrNotFound
    })
    if !errors.Is(err, ErrNotFound) {
        t.Fatalf("got %v, want ErrNotFound", err)
    }
}

func TestCallKeyringWithTimeout_DeadlineExceeded(t *testing.T) {
    t.Setenv(keyringTimeoutEnvVar, "50ms")

start := time.Now()
    _, err := callKeyringWithTimeout("get", func() (string, error) {
        time.Sleep(5 * time.Second)
        return "should not be returned", nil
    })
    elapsed := time.Since(start)

if err == nil {
        t.Fatal("expected timeout error, got nil")
    }
    if !errors.Is(err, context.DeadlineExceeded) {
        t.Fatalf("want DeadlineExceeded wrapped, got %v", err)
    }
    if elapsed > 2*time.Second {
        t.Fatalf("call did not return promptly after timeout: elapsed=%s", elapsed)
    }

msg := err.Error()
    for _, want := range []string{"get", "OS keyring", keyringTimeoutEnvVar} {
        if !strings.Contains(msg, want) {
            t.Errorf("timeout error %q missing %q", msg, want)
        }
    }
}

func TestKeyringTimeout_DefaultWhenUnset(t *testing.T) {
    t.Setenv(keyringTimeoutEnvVar, "")

if got := keyringTimeout(); got != defaultKeyringTimeout {
        t.Fatalf("got %v, want default %v", got, defaultKeyringTimeout)
    }
}

func TestKeyringTimeout_HonoursEnvOverride(t *testing.T) {
    t.Setenv(keyringTimeoutEnvVar, "150ms")

if got := keyringTimeout(); got != 150*time.Millisecond {
        t.Fatalf("got %v, want 150ms", got)
    }
}

func TestKeyringTimeout_IgnoresInvalidEnvValue(t *testing.T) {
    t.Setenv(keyringTimeoutEnvVar, "not-a-duration")

if got := keyringTimeout(); got != defaultKeyringTimeout {
        t.Fatalf("got %v, want default %v", got, defaultKeyringTimeout)
    }
}

func TestKeyringTimeout_IgnoresNonPositiveValue(t *testing.T) {
    t.Setenv(keyringTimeoutEnvVar, "0s")

if got := keyringTimeout(); got != defaultKeyringTimeout {
        t.Fatalf("got %v, want default %v", got, defaultKeyringTimeout)
    }
}

// keyringProviderName must always name something the timeout error can
// point at, including on unrecognised platforms.
func TestKeyringProviderName_NonEmpty(t *testing.T) {
    t.Parallel()

if keyringProviderName() == "" {
        t.Fatal("keyringProviderName returned empty string")
    }
}

// keyringStore delegates to the OS keyring.
// keyringStore delegates to the OS keyring. Every call is bounded by
// callKeyringWithTimeout: the underlying provider (Secret Service,
// Keychain, Credential Manager) can block indefinitely when no daemon
// is reachable, and an unbounded keyring call freezes the whole CLI.
type keyringStore struct{}

func (keyringStore) Get(service, user string) (string, error) {
    //nolint:wrapcheck // thin wrapper, callers handle errors
    return keyring.Get(service, user)
    // keyring.ErrNotFound propagates unchanged; only a timeout wraps.
    return callKeyringWithTimeout("get", func() (string, error) {
        return keyring.Get(service, user)
    })
}

func (keyringStore) Set(service, user, password string) error {
    //nolint:wrapcheck // thin wrapper, callers handle errors
    return keyring.Set(service, user, password)
    _, err := callKeyringWithTimeout("set", func() (string, error) {
        return "", keyring.Set(service, user, password)
    })
    return err
}

func (keyringStore) Delete(service, user string) error {
    //nolint:wrapcheck // thin wrapper, callers handle errors
    return keyring.Delete(service, user)
    _, err := callKeyringWithTimeout("delete", func() (string, error) {
        return "", keyring.Delete(service, user)
    })
    return err
}

Minternal/entireclient/tokenstore/tokenstore.go+16/-7