Repository navigation
Harden client retry path, refresh-scope grant, and tidy tool/OAuth internals - #6
Conversation
Bound total call time against a hostile/misbehaving upstream: every honored delay is clamped to MAX_BACKOFF_MS, so a large Retry-After can no longer stall a request past NETBIRD_TIMEOUT_MS. Decompose request<T> into fetchOnce/decideRetry/backoff — one retry decision, explicit throw-on-exhausted-retries, and the redundant catch-block clearTimeout removed. Test drives a 429 with Retry-After: 86400 and asserts the cap.
exchangeRefreshToken now rejects any requested scope not in the originally granted set (InvalidScopeError), per RFC 6749 §6; no-scope and subset refreshes still succeed. exchangeAuthorizationCode is unchanged (its scopes come from the stored code). Tests cover all three.
Single private register<Args>() owns the guard wrap and the lone 'as unknown as ToolCallback<Args>' cast; registerRead/Mutation/Delete become cast-free config builders. Behavior unchanged.
The decision object is already structurally LoginPageParams
(OAuthChallenge/OAuthLoginError = { kind } & LoginPrefill), so drop the
hand-rebuilt six-field prefill in sendPage. Behavior-preserving.
|
Warning Review limit reached
Next review available in: 2 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (6)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
1 participant
Four small, independent hardening/cleanup changes to the MCP server.
Cap honored
Retry-Afterand decompose the client retry loopThe NetBird API client honored an upstream
Retry-Afterverbatim, and the backoff sleep ran outside the request abort/timeout guard — so a misbehaving or compromised (but allowlisted) upstream returning e.g.Retry-After: 86400could stall a call ~24h, well pastNETBIRD_TIMEOUT_MS. Every honored delay is now clamped to a namedMAX_BACKOFF_MS, so total call time stays bounded regardless of what the upstream sends. The same change decomposesrequest<T>()intofetchOnce/decideRetry/backoff: the retry decision lives in one place, the terminal throw-on-exhausted-retries is explicit rather than fall-through, and a redundantclearTimeoutis removed.Reject scope widening on the OAuth refresh grant
exchangeRefreshTokenhonored whatever scopes the client requested, with no check against the originally granted set — a client could widen scope on refresh, which RFC 6749 §6 forbids. It now rejects any requested scope outside the originally granted set withInvalidScopeError; no-scope and subset refreshes still succeed. The authorization-code exchange is unchanged (its scopes come from the stored code and are not client-controllable).Quarantine the tool-registration type cast
The tool registry repeated the same
as unknown as ToolCallback<Args>cast andregisterToolskeleton across three functions. A single privateregister<Args>()primitive now owns theguardwrap and the lone cast;registerRead/registerMutation/registerDeletebecome cast-free config builders. Behavior is unchanged.Simplify the OAuth login render path
The login decision object is already structurally a
LoginPageParams(OAuthChallenge/OAuthLoginErrorare{ kind } & LoginPrefill), sosendPageno longer hand-rebuilds the six prefill fields — it passes the decision straight torenderLoginPage. Behavior-preserving.Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.