mirror of
https://tangled.org/evan.jarrett.net/at-container-registry
synced 2026-09-02 08:16:57 +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"
|
"io"
|
||||||
"net/http"
|
"net/http"
|
||||||
"strings"
|
"strings"
|
||||||
|
"time"
|
||||||
|
|
||||||
"github.com/bluesky-social/indigo/atproto/syntax"
|
"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.
|
// 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
|
// 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,
|
// 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)
|
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 {
|
if err != nil {
|
||||||
return "", fmt.Errorf("failed to fetch hold DID from %s: %w", holdURL, err)
|
return "", fmt.Errorf("failed to fetch hold DID from %s: %w", holdURL, err)
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -297,6 +297,14 @@ func GetOrFetchServiceToken(
|
|||||||
return serviceToken, nil
|
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.
|
// GetOrFetchServiceTokenWithAppPassword gets a service token using app-password Bearer authentication.
|
||||||
// Used when auth method is app_password instead of OAuth.
|
// Used when auth method is app_password instead of OAuth.
|
||||||
func GetOrFetchServiceTokenWithAppPassword(
|
func GetOrFetchServiceTokenWithAppPassword(
|
||||||
@@ -353,8 +361,8 @@ func GetOrFetchServiceTokenWithAppPassword(
|
|||||||
// Set Bearer token authentication (app-password)
|
// Set Bearer token authentication (app-password)
|
||||||
req.Header.Set("Authorization", "Bearer "+accessToken)
|
req.Header.Set("Authorization", "Bearer "+accessToken)
|
||||||
|
|
||||||
// Make request with standard HTTP client
|
// Make request with a bounded HTTP client (see appPasswordServiceAuthClient)
|
||||||
resp, err := http.DefaultClient.Do(req)
|
resp, err := appPasswordServiceAuthClient.Do(req)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
InvalidateServiceToken(did, holdDID)
|
InvalidateServiceToken(did, holdDID)
|
||||||
slog.Error("App-password service token request failed",
|
slog.Error("App-password service token request failed",
|
||||||
|
|||||||
+5
-1
@@ -49,7 +49,11 @@ type SessionValidator struct {
|
|||||||
// NewSessionValidator creates a new ATProto session validator
|
// NewSessionValidator creates a new ATProto session validator
|
||||||
func NewSessionValidator() *SessionValidator {
|
func NewSessionValidator() *SessionValidator {
|
||||||
return &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),
|
cache: make(map[string]*CachedSession),
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user