From e6959e6dc682a47c786869fd4fc9f4ceba333de9 Mon Sep 17 00:00:00 2001 From: Evan Jarrett Date: Sun, 9 Aug 2026 20:52:13 -0500 Subject: [PATCH] auth: bound the PDS and hold HTTP clients on the token path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit createSession, the app-password getServiceAuth call, and the hold's /.well-known/atproto-did resolution all used http.DefaultClient, which has no timeout. All three run on the /auth/token path, so a slow or unreachable PDS or hold could hold the request open indefinitely, well past Docker's own token-fetch timeout. Give each a bounded client: 15s for createSession, 10s for the app-password getServiceAuth, 10s for the hold DID fetch. All three are safe to cut off — the two GETs are idempotent, and a timed-out createSession only orphans an unused server-side session. The OAuth refresh path deliberately keeps no overall timeout: its POSTs run through refreshDetachTransport and must not be cancelled mid-rotation, which would strand a rotated refresh token. Note holdDIDResolveClient is package-level in pkg/atproto, so the 10s cap applies to every ResolveHoldDID caller, including the GC, Jetstream backfill and hold-health background workers, not only the token path. That is intended (none of them want an unbounded fetch either), but it is a wider blast radius than the token path alone. This bounds three hops, not the whole request: the OAuth getServiceAuth GET and identity resolution are still unbounded, so /auth/token is not yet fully time-boxed. Co-Authored-By: Claude Opus 5 (1M context) --- pkg/atproto/resolver.go | 9 ++++++++- pkg/auth/servicetoken.go | 12 ++++++++++-- pkg/auth/session.go | 6 +++++- 3 files changed, 23 insertions(+), 4 deletions(-) diff --git a/pkg/atproto/resolver.go b/pkg/atproto/resolver.go index 3112495..c36a4fa 100644 --- a/pkg/atproto/resolver.go +++ b/pkg/atproto/resolver.go @@ -6,10 +6,17 @@ import ( "io" "net/http" "strings" + "time" "github.com/bluesky-social/indigo/atproto/syntax" ) +// holdDIDResolveClient bounds the /.well-known/atproto-did fetch so an +// unreachable or slow hold can't stall /auth/token (this resolution runs on the +// hot gate + fetch path) past Docker's token-fetch timeout. It's a plain +// idempotent GET, so a hard timeout is safe. +var holdDIDResolveClient = &http.Client{Timeout: 10 * time.Second} + // ResolveHoldURL converts a hold identifier (DID or URL) to an HTTP/HTTPS URL. // For DIDs (both did:web and did:plc), resolves via the indigo identity directory // which caches results (24h TTL). Prefers the #atcr_hold service endpoint, @@ -61,7 +68,7 @@ func ResolveHoldDID(ctx context.Context, holdIdentifier string) (string, error) return "", fmt.Errorf("failed to create request for hold DID resolution: %w", err) } - resp, err := http.DefaultClient.Do(req) + resp, err := holdDIDResolveClient.Do(req) if err != nil { return "", fmt.Errorf("failed to fetch hold DID from %s: %w", holdURL, err) } diff --git a/pkg/auth/servicetoken.go b/pkg/auth/servicetoken.go index 207e629..27b55c3 100644 --- a/pkg/auth/servicetoken.go +++ b/pkg/auth/servicetoken.go @@ -297,6 +297,14 @@ func GetOrFetchServiceToken( return serviceToken, nil } +// appPasswordServiceAuthClient bounds the app-password getServiceAuth call so a +// slow/unreachable PDS can't stall /auth/token past Docker's token-fetch +// timeout. It's an idempotent GET with a Bearer token — no rotated credential to +// strand — so a hard timeout is safe. (The OAuth flow above deliberately does +// NOT get an overall timeout here: its refresh POSTs run through +// refreshDetachTransport, which must not be cancelled mid-rotation.) +var appPasswordServiceAuthClient = &http.Client{Timeout: 10 * time.Second} + // GetOrFetchServiceTokenWithAppPassword gets a service token using app-password Bearer authentication. // Used when auth method is app_password instead of OAuth. func GetOrFetchServiceTokenWithAppPassword( @@ -353,8 +361,8 @@ func GetOrFetchServiceTokenWithAppPassword( // Set Bearer token authentication (app-password) req.Header.Set("Authorization", "Bearer "+accessToken) - // Make request with standard HTTP client - resp, err := http.DefaultClient.Do(req) + // Make request with a bounded HTTP client (see appPasswordServiceAuthClient) + resp, err := appPasswordServiceAuthClient.Do(req) if err != nil { InvalidateServiceToken(did, holdDID) slog.Error("App-password service token request failed", diff --git a/pkg/auth/session.go b/pkg/auth/session.go index 3a9a8bb..1ee4a29 100644 --- a/pkg/auth/session.go +++ b/pkg/auth/session.go @@ -49,7 +49,11 @@ type SessionValidator struct { // NewSessionValidator creates a new ATProto session validator func NewSessionValidator() *SessionValidator { return &SessionValidator{ - httpClient: &http.Client{}, + // Bounded so a slow/unreachable PDS createSession can't stall /auth/token + // past Docker's own token-fetch timeout. createSession is non-idempotent + // only in that it mints a fresh session, so a timeout here just orphans an + // unused session server-side — no rotated-token to strand. + httpClient: &http.Client{Timeout: 15 * time.Second}, cache: make(map[string]*CachedSession), } }