feat(security): Move API Key to Cookies#243
Merged
Merged
Conversation
📝 WalkthroughWalkthroughAPI key handling now uses server-managed cookies instead of localStorage. The PR adds API-key create/delete routes, updates AuthContext and keystore UI flows, and changes shared server clients to read API keys from headers or cookies. ChangesAPI key storage migration to cookies
Estimated code review effort: 3 (Moderate) | ~30 minutes Sequence Diagram(s)sequenceDiagram
participant KeystorePage
participant AuthContext
participant ApiKeysRoute
participant BackendAPI
KeystorePage->>AuthContext: addKey({ key, label })
AuthContext->>ApiKeysRoute: POST /api/apikeys
ApiKeysRoute->>BackendAPI: verify key (X-API-KEY, cookies)
BackendAPI-->>ApiKeysRoute: verification result
ApiKeysRoute-->>AuthContext: ApiKeyMeta + Set-Cookie
AuthContext-->>KeystorePage: updated apiKeys state
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
github-actions
Bot
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Nitpick comments (1)
app/api/apikeys/route.ts (1)
40-48: 🔒 Security & Privacy | 🔵 TrivialNo rate limiting around key verification.
This endpoint acts as an oracle: repeated calls with different candidate keys reveal validity via the 200/401 response, enabling brute-force enumeration of valid API keys since there's no throttling visible in this route.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/api/apikeys/route.ts` around lines 40 - 48, Add rate limiting to the API key verification flow in the apikeys route so repeated verification attempts cannot be used as an oracle. Update the handler that calls apiClient for "/api/v1/apikeys/verify" to consult the existing throttling mechanism before making the backend request, and return an appropriate limited response when the caller exceeds the allowed attempts. Keep the fix localized to the route handler and its verification path so the 200/401 behavior is preserved for legitimate requests but abuse is blocked.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/`(main)/keystore/page.tsx:
- Around line 62-63: `handleDeleteKey` is still calling the key-removal flow
with an `id`, but the updated AuthContext `removeKey` now clears the single
cookie-backed key and takes no arguments. Update the delete handler in
`page.tsx` to stop passing the key id, and make sure the call site uses the
context-backed `removeKey` API consistently with the current `AuthContext`
contract.
In `@app/api/apikeys/route.ts`:
- Around line 22-24: The API key metadata flow is allowing an unbounded label to
be stored in the cookie, which can overflow browser cookie limits and break
client hydration. Update the `label` handling in `route.ts` and the
`COOKIE_KEYS.API_KEY_META` write path so `label` is capped to a reasonable
maximum before `JSON.stringify(meta)` is called, and make sure the same limit is
enforced anywhere the metadata is created or updated (including the save/update
logic referenced by the review).
- Around line 40-55: The verification flow in apikeys/route.ts is treating every
non-2xx response from apiClient in the same way, so backend 5xx outages are
being reported as an invalid key. Update the logic around the verifyRequest call
in the route handler to distinguish client-side auth failures from
server-side/backend failures by checking the returned status before mapping it
to a response: keep the 401 “invalid key” path only for auth-related statuses,
and return a backend/unavailable error for 5xx (or other non-auth) statuses,
while preserving the existing catch block for request/transport failures.
- Around line 11-68: The POST handler in apikeys/route.ts writes API key cookies
after successful verification without any CSRF/origin protection, so add an
Origin/Host check or CSRF token validation before calling setApiKeyCookies. Keep
the validation near the start of POST, before apiClient verification and cookie
writes, and reject requests with a 403 when the origin does not match the
expected site.
In `@app/components/keystore/KeysCard.tsx`:
- Around line 46-47: The security copy in KeysCard should not say the key is
stored server-side, since the raw key is actually placed in an HttpOnly cookie.
Update the user-facing message in the KeysCard component to accurately describe
the storage model while keeping the warning that the key cannot be viewed again
after it is added. Use the existing text block in KeysCard to adjust the copy
only, without changing the underlying key handling logic.
In `@app/lib/apiClient.ts`:
- Around line 6-12: The readCookieHeader helper in apiClient.ts currently calls
decodeURIComponent directly on request-controlled cookie values, which can throw
on malformed escapes and crash the route. Update readCookieHeader to safely
handle decoding failures by catching errors around the decode step (or otherwise
validating the value first) and treating bad cookie values as missing/invalid so
the function returns no usable cookie instead of throwing.
In `@app/lib/constants.ts`:
- Around line 27-35: The role and features cookie helpers still hardcode the
7-day max age instead of reusing the shared constant. Update the cookie-setting
logic in authCookie to use AUTH_COOKIE_MAX_AGE for both the role and features
cookies, and keep COOKIE_KEYS as the source for the cookie names so the lifetime
remains centralized.
In `@app/lib/context/AuthContext.tsx`:
- Around line 96-121: Route the API-key mutation calls in AuthContext through
clientFetch instead of direct fetch so shared 401 handling, AUTH_EXPIRED_EVENT
dispatching, and standardized error extraction are preserved. Update addKey and
removeKey to use clientFetch for the /api/apikeys POST and DELETE requests, and
ensure removeKey only clears api keys and dispatches kaapi-auth-changed after a
successful response. Keep the logic around setApiKeys, body.data validation, and
the kaapi-auth-changed event, but make success handling depend on clientFetch
returning without error.
- Around line 38-39: The API-key metadata parse in AuthContext.tsx should not
abort session hydration if COOKIE_KEYS.API_KEY_META is malformed. Update the
readClientCookie/JSON.parse(rawMeta) handling inside the AuthContext
initialization logic so parse failures are caught and ignored locally, allowing
the localStorage session restore that follows to still run. Keep the valid
setApiKeys path for well-formed metadata, but isolate it from the broader
hydration flow.
In `@app/lib/utils.ts`:
- Around line 72-80: The cookie parsing in readClientCookie can throw a URIError
when decodeURIComponent sees malformed client-controlled data. Update
readClientCookie to treat decoding as non-throwing by guarding the decode step
with error handling and returning undefined on decode failures, while keeping
the existing undefined return for missing cookies and server-side access.
---
Nitpick comments:
In `@app/api/apikeys/route.ts`:
- Around line 40-48: Add rate limiting to the API key verification flow in the
apikeys route so repeated verification attempts cannot be used as an oracle.
Update the handler that calls apiClient for "/api/v1/apikeys/verify" to consult
the existing throttling mechanism before making the backend request, and return
an appropriate limited response when the caller exceeds the allowed attempts.
Keep the fix localized to the route handler and its verification path so the
200/401 behavior is preserved for legitimate requests but abuse is blocked.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 772a3816-2fe4-4c7e-886e-22a6c28ff571
📒 Files selected for processing (15)
CLAUDE.mdapp/(main)/keystore/page.tsxapp/api/apikeys/route.tsapp/api/auth/logout/route.tsapp/api/evaluations/stt/runs/route.tsapp/components/keystore/AddKeyModal.tsxapp/components/keystore/KeysCard.tsxapp/lib/apiClient.tsapp/lib/authCookie.tsapp/lib/constants.tsapp/lib/context/AuthContext.tsxapp/lib/guardrailsClient.tsapp/lib/types/auth.tsapp/lib/types/credentials.tsapp/lib/utils.ts
💤 Files with no reviewable changes (1)
- app/components/keystore/AddKeyModal.tsx
vprashrex
approved these changes
Jul 3, 2026
| <InlineNotice> | ||
| Only one API key can be stored at a time. Delete this key to add a | ||
| different one. | ||
| different one. For your security, the key is stored server-side and |
Collaborator
There was a problem hiding this comment.
this extra information are not needed to tell
| export interface ApiKeyMeta { | ||
| id: string; | ||
| label: string; | ||
| provider: string; |
Collaborator
There was a problem hiding this comment.
why need to store provider in api key metadata ?
Collaborator
Author
There was a problem hiding this comment.
basically this is not needed for now, but if we add more provider later then it's use. Right now, this is not needed so dropping it.
Collaborator
There was a problem hiding this comment.
what type of provider u mean ?
Collaborator
Author
There was a problem hiding this comment.
So previously it's a dropdown and I'm thinking like future providers, grouped by the modalities this app already handles (text / STT / TTS / observability). But not needed for now I have removed this. Thanks for pointing this out.
Collaborator
|
Instead of storing the raw API key in the cookie, I'd recommend using a session_id or a signed JWT instead. That said, implementing a session-based flow for API key authentication adds a fair amount of complexity. If it's not a priority right now, we can take it up later or leave it as is for the time being. |
Collaborator
Author
I think for now, we can ship this current |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/components/keystore/AddKeyModal.tsx`:
- Around line 39-41: The AddKeyModal copy incorrectly says keys are “stored
securely server-side,” but the actual storage model is an HttpOnly cookie;
update the descriptive text in AddKeyModal to match the same cookie-based
wording used in KeysCard and remove any server-side persistence claim.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: b17080bb-f9cc-458f-86d0-5ea5926a18c2
📒 Files selected for processing (7)
app/(main)/keystore/page.tsxapp/api/apikeys/route.tsapp/components/keystore/AddKeyModal.tsxapp/components/keystore/KeysCard.tsxapp/lib/context/AuthContext.tsxapp/lib/types/auth.tsapp/lib/types/credentials.ts
💤 Files with no reviewable changes (2)
- app/api/apikeys/route.ts
- app/lib/types/credentials.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- app/lib/context/AuthContext.tsx
- app/lib/types/auth.ts
- app/(main)/keystore/page.tsx
|
🎉 This PR is included in version 0.3.0-main.8 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Issue
Closes #224
Summary
Checklist
Before submitting a pull request, please ensure that you mark these task.
npm run devandnpm run buildin the repository root and test.