mirror of
https://tangled.org/evan.jarrett.net/at-container-registry
synced 2026-08-29 04:06:58 +00:00
auth: bound the PDS and hold HTTP clients on the token path
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) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
cecb8879de
commit
e6959e6dc6
@@ -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)
|
||||
}
|
||||
|
||||
@@ -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",
|
||||
|
||||
+5
-1
@@ -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),
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user