Harden Wakatime API proxy auth/error handling and add request guardrails#13
Harden Wakatime API proxy auth/error handling and add request guardrails#13
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Code Review SummaryStatus: No Issues Found | Recommendation: Merge Overview
Code Quality SuggestionsWhile this PR introduces significant improvements, consider these minor enhancements:
Positive ChangesThis PR significantly improves the Wakatime API integration:
Files Reviewed (1 file)
|
There was a problem hiding this comment.
Pull request overview
This PR hardens the /api/wakatime proxy endpoint by removing client-exposed API key fallbacks, moving authentication into an Authorization header, and adding resilience features (timeouts + retries) while returning client-safe error payloads.
Changes:
- Remove
VITE_WAKATIME_API_KEYfallback and readWAKATIME_API_KEYfrom server environment only. - Switch from query-param API key auth to
Authorization: Basic …header. - Add request guardrails: GET-only with
Allowheader, timeout viaAbortController, and retry w/ exponential backoff for429/5xxand network/timeout errors.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (!response.ok && shouldRetry(response.status) && attempt < MAX_RETRIES) { | ||
| const backoffMs = INITIAL_BACKOFF_MS * (2 ** attempt); | ||
| await sleep(backoffMs); | ||
| continue; | ||
| } |
There was a problem hiding this comment.
In the retry path for non-OK responses, the response body is never consumed/canceled before continue. With Node’s built-in fetch (undici), leaving bodies unread can prevent connection reuse and may leak resources under load. Consider canceling/draining the response body before backing off and retrying.
| if (!response.ok) { | ||
| const errorText = await response.text(); | ||
| return res.status(response.status).json({ error: 'Failed to fetch from Wakatime', details: errorText }); | ||
| return res.status(response.status).json({ error: 'Failed to fetch from Wakatime' }); | ||
| } |
There was a problem hiding this comment.
The PR description says upstream/error internals are logged on the server, but !response.ok returns immediately without logging status (and optionally a truncated body) for diagnosis. Consider adding server-side logging for non-OK upstream responses while still returning the sanitized client payload.
Motivation
Description
WAKATIME_API_KEYfrom the server environment and remove theVITE_WAKATIME_API_KEYfallback to avoid exposing secrets to the client.Authorization: Basic <base64(apiKey:)>header so the secret is not sent in the URL.GETonly withAllowheader), fetch timeout viaAbortController, and afetchWithRetryimplementation with exponential backoff for429/5xxand network/timeout errors.Testing
npm run buildwhich completed successfully.npm run lintwhich failed because there is no ESLint configuration in this environment, not due to the handler changes.Codex Task