auth: evict app-password tokens a PDS reports stale, not just on 401

An expired app-password token could wedge an account permanently. The 401
branch clears the cached token, but some PDSes report the same condition as
400 with an atproto error name in the body, which fell through to the generic
non-200 branch. That clears only the derived service token, so the dead
bearer token stayed in the cache and every subsequent request replayed it.

Observed on one account against at.hexlab.foo: 16,110 of these errors and
4,254 retryable 503s over 33 hours, with no recovery path. The cache is
in-memory, so it only cleared on process restart.

Now the non-200 branch classifies the atproto error name and evicts on the
ones that mean the presented token is unusable, matching what the 401 branch
already does. For app-passwords that is the equivalent of a refresh: the next
authentication re-mints via createSession.

Deliberately not routed through oauth.IsSessionInvalidError, which excludes
ExpiredToken on purpose — there it would delete a recoverable OAuth session
and sign the user out everywhere, whereas here the only thing discarded is a
cache entry that will be repopulated.

Not addressed here: the failure still surfaces as a 503, which is retryable
and so keeps clients looping. Returning 401 with the re-auth hint would be
the better signal, but it spans the token handler and is a separate change.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

pkg/auth/servicetoken.go

@@ -31,6 +31,23 @@ func atprotoErrorName(body []byte) string {
 	return e.Error
 }
 
+// isStaleBearerToken reports whether an atproto XRPC error name means the
+// bearer token presented was rejected as expired or otherwise unusable, and so
+// should be discarded rather than replayed.
+//
+// This is scoped to the app-password path, where the cached access token is the
+// only credential state and dropping it simply forces a re-mint on the next
+// authentication. Do not reuse this for OAuth sessions: ExpiredToken there is
+// recoverable by refreshing, and treating it as fatal would sign users out.
+func isStaleBearerToken(name string) bool {
+	switch name {
+	case "ExpiredToken", "InvalidToken", "ExpiredSession", "InvalidSession":
+		return true
+	default:
+		return false
+	}
+}
+
 // getErrorHint provides context-specific troubleshooting hints based on API error type
 func getErrorHint(apiErr *atclient.APIError) string {
 	switch apiErr.Name {
@@ -365,6 +382,30 @@ func GetOrFetchServiceTokenWithAppPassword(
 		// Service auth failed
 		bodyBytes, _ := io.ReadAll(resp.Body)
 		InvalidateServiceToken(did, holdDID)
+
+		// Some PDSes report a stale bearer token as 400 + an atproto error name
+		// rather than 401, so the status check above misses it. Without this the
+		// expired token stays in the cache, every subsequent request replays it,
+		// and the user is wedged until the process restarts: one account
+		// generated 16,110 of these and 4,254 retryable 503s over 33 hours.
+		//
+		// Dropping the cached token is the app-password equivalent of a refresh
+		// — the next authentication re-mints it via createSession. Note this is
+		// deliberately not routed through oauth.IsSessionInvalidError, which
+		// excludes ExpiredToken on purpose: there it would delete a recoverable
+		// OAuth session, whereas here the only thing discarded is a cache entry.
+		if name := atprotoErrorName(bodyBytes); isStaleBearerToken(name) {
+			GetGlobalTokenCache().Delete(did)
+			slog.Warn("App-password token reported stale by PDS, evicting from cache",
+				"component", "token/servicetoken",
+				"did", did,
+				"holdDID", holdDID,
+				"statusCode", resp.StatusCode,
+				"atprotoError", name,
+				"hint", "next authentication will re-mint; if it persists the user must re-authenticate")
+			return "", fmt.Errorf("app-password token stale (%s): re-authentication required", name)
+		}
+
 		slog.Error("Service token request returned non-200 status (app-password)",
 			"component", "token/servicetoken",
 			"did", did,

pkg/auth/servicetoken_test.go

@@ -5,6 +5,7 @@ import (
 	"fmt"
 	"net/http"
 	"net/http/httptest"
+	"strings"
 	"sync"
 	"testing"
 	"time"
@@ -217,3 +218,89 @@ func TestGetOrFetchServiceToken_InvalidGrantDeletesSession(t *testing.T) {
 		t.Errorf("expected UI session invalidation for %s, got: %v", did, uiStore.deleted)
 	}
 }
+
+func TestIsStaleBearerToken(t *testing.T) {
+	stale := []string{"ExpiredToken", "InvalidToken", "ExpiredSession", "InvalidSession"}
+	for _, name := range stale {
+		if !isStaleBearerToken(name) {
+			t.Errorf("isStaleBearerToken(%q) = false, want true", name)
+		}
+	}
+	// Anything else must not discard the credential — an unrelated server-side
+	// failure should not force the user to re-authenticate.
+	for _, name := range []string{"", "InvalidRequest", "RateLimitExceeded", "InternalServerError"} {
+		if isStaleBearerToken(name) {
+			t.Errorf("isStaleBearerToken(%q) = true, want false", name)
+		}
+	}
+}
+
+// TestAppPasswordServiceToken_EvictsOnStaleToken is the regression test for a
+// wedged-account loop: a PDS reporting an expired bearer token as 400 with an
+// atproto error name (rather than 401) left the dead token in the cache, so
+// every subsequent request replayed it. One account produced 16,110 such errors
+// and 4,254 retryable 503s over 33 hours, recovering only on process restart.
+func TestAppPasswordServiceToken_EvictsOnStaleToken(t *testing.T) {
+	tests := []struct {
+		name        string
+		status      int
+		body        string
+		wantEvicted bool
+	}{
+		{
+			name:        "400 with ExpiredToken evicts",
+			status:      http.StatusBadRequest,
+			body:        `{"error":"ExpiredToken","message":"Token has expired"}`,
+			wantEvicted: true,
+		},
+		{
+			name:        "401 evicts (pre-existing path)",
+			status:      http.StatusUnauthorized,
+			body:        `{"error":"AuthMissing"}`,
+			wantEvicted: true,
+		},
+		{
+			name:        "400 with an unrelated error keeps the token",
+			status:      http.StatusBadRequest,
+			body:        `{"error":"InvalidRequest","message":"bad aud"}`,
+			wantEvicted: false,
+		},
+		{
+			name:        "500 keeps the token",
+			status:      http.StatusInternalServerError,
+			body:        `{"error":"InternalServerError"}`,
+			wantEvicted: false,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			pds := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+				w.Header().Set("Content-Type", "application/json")
+				w.WriteHeader(tt.status)
+				_, _ = w.Write([]byte(tt.body))
+			}))
+			defer pds.Close()
+
+			did := "did:plc:" + strings.ToLower(strings.ReplaceAll(t.Name(), "/", ""))
+			holdDID := "did:web:hold.example.com"
+
+			GetGlobalTokenCache().Set(did, "stale-access-token", time.Hour)
+			t.Cleanup(func() { GetGlobalTokenCache().Delete(did) })
+
+			if _, err := GetOrFetchServiceTokenWithAppPassword(
+				context.Background(), did, holdDID, pds.URL,
+			); err == nil {
+				t.Fatal("expected an error from a non-200 PDS response")
+			}
+
+			_, stillCached := GetGlobalTokenCache().Get(did)
+			if tt.wantEvicted && stillCached {
+				t.Error("expected the stale app-password token to be evicted, but it is still cached")
+			}
+			if !tt.wantEvicted && !stillCached {
+				t.Error("token was evicted on an unrelated failure; the user is forced to re-authenticate needlessly")
+			}
+		})
+	}
+}