fix(sdk): add fetch timeouts to TokenVerifier JWKS and revocation requests#36
fix(sdk): add fetch timeouts to TokenVerifier JWKS and revocation requests#36khaliqgant merged 1 commit intomainfrom
Conversation
…uests The JWKS and revocation fetches in TokenVerifier had no AbortSignal / timeout, so any slow or unresponsive relayauth endpoint could stall callers indefinitely. After cloud migrated relayfile tokens to RS256, every relayfile request verifies via JWKS, so a JWKS hang propagates everywhere. Add an AbortSignal.timeout() to both fetches, defaulting to 5000ms each, plumbed through as optional `jwksTimeoutMs` and `revocationTimeoutMs` on VerifyOptions. New unit tests cover: - JWKS fetch aborts within the configured window when upstream stalls - Default JWKS timeout is applied when no override is given (signal is still attached) - Revocation fetch aborts within the configured window
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 37409e41ba
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| signal: AbortSignal.timeout( | ||
| normalizeTimeoutMs(this.options?.jwksTimeoutMs, DEFAULT_JWKS_TIMEOUT_MS), | ||
| ), |
There was a problem hiding this comment.
This unconditionally calls AbortSignal.timeout(...), which throws in runtimes where fetch exists but the static timeout helper is not implemented (for example older browser/edge runtimes or polyfilled environments). In that case verification fails before any network request is made, so valid tokens are rejected with jwks_fetch_failed (and the same regression appears in #checkRevocation), which is a behavior break from the previous implementation that worked without this API. Add a feature check/fallback (e.g., AbortController + setTimeout, or omit the signal when timeout helpers are unavailable).
Useful? React with 👍 / 👎.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| jwksTimeoutMs?: number; | ||
| revocationTimeoutMs?: number; |
There was a problem hiding this comment.
🟡 New timeout options silently dropped by AI adapter's #buildVerifyOptions
The PR adds jwksTimeoutMs and revocationTimeoutMs to the exported VerifyOptions interface, but packages/ai/src/adapter.ts:507-527 (#buildVerifyOptions) explicitly enumerates which fields to forward to TokenVerifier and does not include the new fields. Since AdapterOptions extends Partial<VerifyOptions> (packages/ai/src/types.ts:29), TypeScript allows users to pass jwksTimeoutMs/revocationTimeoutMs through the adapter config, but at runtime these values are silently dropped — the verifier always uses the 5000ms defaults. Additionally, areVerifyOptionsEqual at packages/ai/src/adapter.ts:661-674 doesn't compare the new fields, so changing timeout options on a live adapter won't trigger verifier re-creation.
Prompt for agents
The new jwksTimeoutMs and revocationTimeoutMs fields were added to VerifyOptions in packages/sdk/typescript/src/verify.ts, but the downstream consumer in packages/ai/src/adapter.ts was not updated. Two places need changes:
1. #buildVerifyOptions() at packages/ai/src/adapter.ts:507-527 — add jwksTimeoutMs and revocationTimeoutMs to both the destructuring and the return object so they are forwarded to TokenVerifier.
2. areVerifyOptionsEqual() at packages/ai/src/adapter.ts:661-674 — add comparisons for previous.jwksTimeoutMs === next.jwksTimeoutMs and previous.revocationTimeoutMs === next.revocationTimeoutMs so that changing these options triggers verifier re-creation.
Was this helpful? React with 👍 or 👎 to provide feedback.
1 participant
Summary
@relayauth/sdkTokenVerifiermade its two HTTP fetches with noAbortSignal/ timeout, so any slow or unresponsive relayauth endpoint would stall callers indefinitely. After cloud#329 migrated relayfile tokens from HS256 to RS256, every relayfile request verifies via JWKS — so JWKS hangs propagate everywhere.This PR:
packages/sdk/typescript/src/verify.ts) withAbortSignal.timeout()(default 5000ms).AbortSignal.timeout()(default 5000ms).VerifyOptions.jwksTimeoutMsandVerifyOptions.revocationTimeoutMs. Non-positive / non-finite values fall back to the default.packages/sdk/typescript/src/__tests__/verify-timeout.test.tswith three cases:No version bumps in this PR — Khaliq is handling the publish step.
Test plan
cd packages/sdk/typescript && npm test— 141/141 pass (was 138 before, +3 new timeout tests).npm run typecheck— clean.RelayAuthError(jwks_fetch_failed/revocation_check_failed).Notes
RelayAuthErrorthat a network failure would. Callers that already handlejwks_fetch_failed/revocation_check_failedget the timeout case for free.AbortSignal.timeoutin Node uses an unref'd timer, so a naive test that relies solely on the abort signal can let the process exit before the timer fires. The new tests pair the abort signal with a refedsetTimeoutin the mock (cleared on abort) so the loop is held open and the test reliably asserts timing.🤖 Generated with Claude Code