Add API key console#553
Closed
antoncoding wants to merge 3 commits into
Closed
Conversation
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces an API Key Console feature to Monarch, allowing users to generate and test API keys by signing a wallet proof. It adds the /api-keys page, the backend routes POST /api/api-keys and POST /api/api-keys/test, and updates documentation and dependencies. Feedback on these changes highlights several opportunities to improve robustness, including handling null JSON bodies to prevent runtime crashes, wrapping external fetch calls in try/catch blocks for graceful error handling, providing a fallback for crypto.randomUUID in non-secure contexts, and relaxing signature validation to support smart contract wallets.
Comment on lines
+88
to
+93
| let body: CreateApiKeyRequestBody; | ||
| try { | ||
| body = (await request.json()) as CreateApiKeyRequestBody; | ||
| } catch { | ||
| return { error: 'Invalid JSON body.' }; | ||
| } |
There was a problem hiding this comment.
If the request body is null (which is valid JSON), request.json() will return null. Accessing properties on body afterwards will throw a TypeError and crash the route handler. It is safer to verify that body is a non-null object before proceeding.
Suggested change
| let body: CreateApiKeyRequestBody; | |
| try { | |
| body = (await request.json()) as CreateApiKeyRequestBody; | |
| } catch { | |
| return { error: 'Invalid JSON body.' }; | |
| } | |
| let body: CreateApiKeyRequestBody; | |
| try { | |
| body = (await request.json()) as CreateApiKeyRequestBody; | |
| if (!body || typeof body !== 'object') { | |
| return { error: 'Invalid JSON body.' }; | |
| } | |
| } catch { | |
| return { error: 'Invalid JSON body.' }; | |
| } |
Comment on lines
+53
to
+58
| let body: TestApiKeyRequestBody; | ||
| try { | ||
| body = (await request.json()) as TestApiKeyRequestBody; | ||
| } catch { | ||
| return { error: 'Invalid JSON body.' }; | ||
| } |
There was a problem hiding this comment.
If the request body is null, request.json() will return null. Accessing body.apiKey will then throw a TypeError. We should ensure body is a valid object before accessing its properties.
let body: TestApiKeyRequestBody;
try {
body = (await request.json()) as TestApiKeyRequestBody;
if (!body || typeof body !== 'object') {
return { error: 'Invalid JSON body.' };
}
} catch {
return { error: 'Invalid JSON body.' };
}
Comment on lines
+130
to
+149
| const response = await fetch(adminEndpoint, { | ||
| method: 'POST', | ||
| headers: { | ||
| Authorization: `Bearer ${adminToken}`, | ||
| 'Content-Type': 'application/json', | ||
| }, | ||
| body: JSON.stringify({ | ||
| name, | ||
| environment: 'live', | ||
| scopes: ['data.read', 'indexer.query'], | ||
| tier: 'free', | ||
| rateLimitTier: 'free', | ||
| metadata: { | ||
| ownerAddress: address, | ||
| origin, | ||
| signedAt: issuedAt, | ||
| createdBy: 'monarch-api-key-console', | ||
| }, | ||
| }), | ||
| }); |
There was a problem hiding this comment.
The fetch call to the external admin endpoint is not wrapped in a try/catch block. If the gateway is temporarily down or a network error occurs, fetch will throw an unhandled exception, causing a 500 error. Wrapping it in a try/catch allows returning a clean 502 Bad Gateway response.
let response: Response;
try {
response = await fetch(adminEndpoint, {
method: 'POST',
headers: {
Authorization: `Bearer ${adminToken}`,
'Content-Type': 'application/json',
},
body: JSON.stringify({
name,
environment: 'live',
scopes: ['data.read', 'indexer.query'],
tier: 'free',
rateLimitTier: 'free',
metadata: {
ownerAddress: address,
origin,
signedAt: issuedAt,
createdBy: 'monarch-api-key-console',
},
}),
});
} catch (error) {
return NextResponse.json({ error: 'Failed to connect to the API gateway.' }, { status: 502 });
}
Comment on lines
+22
to
+29
| const response = await fetch(TEST_ENDPOINT, { | ||
| method: 'GET', | ||
| headers: { | ||
| Accept: 'application/json', | ||
| 'X-API-Key': apiKey.value, | ||
| }, | ||
| cache: 'no-store', | ||
| }); |
There was a problem hiding this comment.
The fetch call to the test endpoint is not wrapped in a try/catch block. If the endpoint is down or a network error occurs, it will throw an unhandled exception. Wrapping it in a try/catch ensures a graceful 502 response.
Suggested change
| const response = await fetch(TEST_ENDPOINT, { | |
| method: 'GET', | |
| headers: { | |
| Accept: 'application/json', | |
| 'X-API-Key': apiKey.value, | |
| }, | |
| cache: 'no-store', | |
| }); | |
| let response: Response; | |
| try { | |
| response = await fetch(TEST_ENDPOINT, { | |
| method: 'GET', | |
| headers: { | |
| Accept: 'application/json', | |
| 'X-API-Key': apiKey.value, | |
| }, | |
| cache: 'no-store', | |
| }); | |
| } catch (error) { | |
| return NextResponse.json( | |
| { | |
| ok: false, | |
| status: 502, | |
| error: 'Failed to connect to the test endpoint.', | |
| }, | |
| { status: 502 } | |
| ); | |
| } |
| wallet: normalizedAddress, | ||
| origin: window.location.origin, | ||
| issuedAt: new Date().toISOString(), | ||
| nonce: crypto.randomUUID(), |
There was a problem hiding this comment.
crypto.randomUUID is only available in secure contexts (HTTPS or localhost). If the app is accessed over HTTP in non-localhost environments (e.g., local network testing or staging), this will throw a TypeError and crash the key generation flow. Providing a simple fallback prevents this.
Suggested change
| nonce: crypto.randomUUID(), | |
| nonce: typeof crypto.randomUUID === 'function' ? crypto.randomUUID() : Math.random().toString(36).slice(2) + Math.random().toString(36).slice(2), |
Comment on lines
+104
to
+106
| if (!/^0x[0-9a-fA-F]{130}$/.test(signature)) { | ||
| return { error: 'Invalid signature format.' }; | ||
| } |
There was a problem hiding this comment.
The strict regex check /^0x[0-9a-fA-F]{130}$/ limits signatures to exactly 65 bytes, which blocks smart contract wallets (like Safe) that use ERC-1271 signatures of arbitrary lengths. Relaxing this regex to allow any valid hex string and passing a publicClient to verifyMessage will enable support for smart contract wallets, which is highly recommended for a DeFi dashboard.
Suggested change
| if (!/^0x[0-9a-fA-F]{130}$/.test(signature)) { | |
| return { error: 'Invalid signature format.' }; | |
| } | |
| if (!/^0x[0-9a-fA-F]+$/.test(signature)) { | |
| return { error: 'Invalid signature format.' }; | |
| } |
📝 WalkthroughWalkthroughAdds a complete API key management system allowing users to generate keys by signing a wallet message, with server-side validation and EIP-191 signature verification. Includes a console UI, navigation integration, hook rename for clarity, and widespread formatting improvements. ChangesAPI Key Management Feature
Hook Rename and Security
Configuration, Dependencies, and Documentation
Code Formatting and Style Consolidation
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
coderabbitai
Bot
added
feature request
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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/api/api-keys/route.ts`:
- Around line 54-74: parsedMessage.nonce is only validated but not consumed,
allowing replay of the same signed payload to create multiple keys; before
calling createGatewayApiKey, check a server-side nonce store (or DB table) keyed
by {address, nonce} and atomically mark the nonce as used (or insert a unique
{address, nonce} row) so subsequent requests with the same nonce are rejected,
or alternatively modify createGatewayApiKey to be idempotent on {address, nonce}
by returning the existing key if the pair already exists; ensure this
check/insert is performed after verifyMessage succeeds and before
createGatewayApiKey is invoked to prevent race conditions.
- Around line 129-151: The admin fetch to adminEndpoint (creating the POST with
Authorization, body and casting to AdminCreateApiKeyResponse) can hang or throw;
wrap the fetch in an AbortController-based timeout and put the whole network
call and response.json() inside a try/catch so any fetch/network/timeout error
is caught and you return a JSON failure payload instead of letting the route
throw; specifically, create an AbortController, set a timeout to call
controller.abort(), pass controller.signal to fetch, await response and
response.json() inside the try block, and in the catch return a consistent error
object (or set a failure body) so callers always receive JSON even on network
errors or timeouts.
In `@app/api/api-keys/test/route.ts`:
- Around line 22-39: The route's fetch to TEST_ENDPOINT (in
app/api/api-keys/test/route.ts) is unbounded and not wrapped in a try/catch, so
network/timeouts will throw and produce unhandled 500s; update the handler that
calls fetch(TEST_ENDPOINT, ...) to perform the request inside a try/catch and
use an AbortController timeout (or equivalent) to bound the call, then on any
fetch/network/timeout error return a stable JSON response via NextResponse.json
with ok: false, a suitable status (e.g., 502/504), and an explanatory error
message instead of letting the exception bubble; keep references to
TEST_ENDPOINT, MarketMetricsResponse, and the existing NextResponse.json
response shape so consumer parsing remains consistent.
In `@src/features/markets/components/table/market-table-utils.tsx`:
- Line 55: Update the external anchor(s) that currently set rel="noopener" (in
src/features/markets/components/table/market-table-utils.tsx) to include
noreferrer as well so they read rel="noopener noreferrer"; search for
occurrences of rel="noopener" or anchors with target="_blank" (e.g., the link
rendering in the market table util helper) and add "noreferrer" to the rel
attribute to prevent referrer leakage and ensure noopener protection.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: c8c31df5-d77a-4824-bb35-d44bbb2a9c03
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (50)
.env.local.exampleapp/api-keys/page.tsxapp/api/api-keys/route.tsapp/api/api-keys/test/route.tsapp/fonts.tsbiome.jsoncdocs/TECHNICAL_OVERVIEW.mddocs/VALIDATIONS.mdpackage.jsonsrc/components/layout/header/Navbar.tsxsrc/components/layout/header/NavbarMobile.tsxsrc/components/shared/account-actions-popover.tsxsrc/components/shared/account-identity.tsxsrc/components/shared/transaction-identity.tsxsrc/contexts/VaultRegistryContext.tsxsrc/data-sources/monarch-api/markets.tssrc/data-sources/monarch-api/vaults.tssrc/data-sources/morpho-api/vault-share-price-history.tssrc/data-sources/morpho-api/vaults.tssrc/features/api-keys/api-key-console-view.tsxsrc/features/autovault/components/vault-detail/vault-market-allocations.tsxsrc/features/autovault/components/vault-identity.tsxsrc/features/markets/components/market-details-block.tsxsrc/features/markets/components/markets-table-same-loan.tsxsrc/features/markets/components/table/market-table-body.tsxsrc/features/markets/components/table/market-table-utils.tsxsrc/features/markets/components/table/markets-table.tsxsrc/features/position-detail/components/report-range-picker.tsxsrc/features/position-detail/hooks/usePositionDetailData.tssrc/features/positions/components/allocation-cell.tsxsrc/features/positions/components/markets-filter-compact.tsxsrc/features/positions/components/portfolio-analytics-banner.tsxsrc/features/positions/components/user-vaults-table.tsxsrc/features/vault/components/vault-market-allocations-table.tsxsrc/features/vault/components/vault-share-price-chart.tsxsrc/features/vault/vault-view.tsxsrc/hooks/queries/useMarketV2SupplyingVaultsQuery.tssrc/hooks/use4626VaultAPR.tssrc/hooks/useEffectiveTrustedVaults.tssrc/hooks/useFilteredMarkets.tssrc/hooks/useTrustedVaultMetadata.tssrc/hooks/useUserPositions.tssrc/hooks/useVaultAllocations.tssrc/hooks/useVaultSharePriceHistory.tssrc/modals/leverage/components/add-collateral-and-leverage.tsxsrc/modals/settings/monarch-settings/details/TrustedVaultsDetail.tsxsrc/modals/vault/vault-withdraw-modal.tsxsrc/stores/usePositionsFilters.tssrc/utils/apiKeyRequest.tssrc/utils/portfolio.ts
Comment on lines
+54
to
+74
| if (!/^[A-Za-z0-9-]{16,80}$/.test(parsedMessage.nonce)) { | ||
| return NextResponse.json({ error: 'Invalid signature nonce.' }, { status: 400 }); | ||
| } | ||
|
|
||
| const signatureValid = await verifyMessage({ | ||
| address, | ||
| message: body.message, | ||
| signature: body.signature as `0x${string}`, | ||
| }); | ||
|
|
||
| if (!signatureValid) { | ||
| return NextResponse.json({ error: 'Invalid wallet signature.' }, { status: 401 }); | ||
| } | ||
|
|
||
| const adminResponse = await createGatewayApiKey({ | ||
| adminToken, | ||
| address, | ||
| name: body.name, | ||
| origin: parsedMessage.origin, | ||
| issuedAt: parsedMessage.issuedAt, | ||
| }); |
There was a problem hiding this comment.
Consume the nonce after first use.
parsedMessage.nonce is only shape-checked here. The same signed payload can be replayed for the full TTL and create multiple keys, so one captured signature becomes a reusable create token. Reject reused nonces server-side, or make the admin write idempotent on {address, nonce} before calling upstream.
🤖 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/api-keys/route.ts` around lines 54 - 74, parsedMessage.nonce is only
validated but not consumed, allowing replay of the same signed payload to create
multiple keys; before calling createGatewayApiKey, check a server-side nonce
store (or DB table) keyed by {address, nonce} and atomically mark the nonce as
used (or insert a unique {address, nonce} row) so subsequent requests with the
same nonce are rejected, or alternatively modify createGatewayApiKey to be
idempotent on {address, nonce} by returning the existing key if the pair already
exists; ensure this check/insert is performed after verifyMessage succeeds and
before createGatewayApiKey is invoked to prevent race conditions.
Comment on lines
+22
to
+39
| const response = await fetch(TEST_ENDPOINT, { | ||
| method: 'GET', | ||
| headers: { | ||
| Accept: 'application/json', | ||
| 'X-API-Key': apiKey.value, | ||
| }, | ||
| cache: 'no-store', | ||
| }); | ||
|
|
||
| const body = (await response.json().catch(() => ({}))) as MarketMetricsResponse; | ||
| if (!response.ok) { | ||
| return NextResponse.json( | ||
| { | ||
| ok: false, | ||
| status: response.status, | ||
| error: typeof body.error === 'string' ? body.error : 'API key test failed.', | ||
| }, | ||
| { status: response.status }, |
There was a problem hiding this comment.
Bound and catch the test fetch.
This upstream call has no timeout or network error handling, so transient gateway failures will bubble out as unhandled 500s. Wrap it and return a stable JSON error from this route instead.
🤖 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/api-keys/test/route.ts` around lines 22 - 39, The route's fetch to
TEST_ENDPOINT (in app/api/api-keys/test/route.ts) is unbounded and not wrapped
in a try/catch, so network/timeouts will throw and produce unhandled 500s;
update the handler that calls fetch(TEST_ENDPOINT, ...) to perform the request
inside a try/catch and use an AbortController timeout (or equivalent) to bound
the call, then on any fetch/network/timeout error return a stable JSON response
via NextResponse.json with ok: false, a suitable status (e.g., 502/504), and an
explanatory error message instead of letting the exception bubble; keep
references to TEST_ENDPOINT, MarketMetricsResponse, and the existing
NextResponse.json response shape so consumer parsing remains consistent.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/features/api-keys/api-key-console-view.tsx (1)
100-105:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winHandle clipboard failures.
navigator.clipboard.writeTextcan reject when clipboard access is blocked or unavailable. Right now the copy button can throw an unhandled client error.Proposed fix
const handleCopy = async () => { if (!createdKey) return; - await navigator.clipboard.writeText(createdKey.apiKey); - setCopied(true); + try { + await navigator.clipboard.writeText(createdKey.apiKey); + setCopied(true); + } catch { + setCopied(false); + setCreationError('Copy failed. Please copy the key manually.'); + } };🤖 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 `@src/features/api-keys/api-key-console-view.tsx` around lines 100 - 105, The handleCopy function can throw if navigator.clipboard.writeText rejects; wrap the await call in a try/catch around navigator.clipboard.writeText(createdKey.apiKey) inside handleCopy, handle the error by logging it (console.error or processLogger), setCopied(false) or set an error state to show the user a failure message, and optionally fall back to a document.execCommand('copy') or selecting a temporary textarea to copy; ensure you still return early if !createdKey and reference the handleCopy function, createdKey, and setCopied when implementing the fix.app/api/api-keys/route.ts (1)
225-231:⚠️ Potential issue | 🟠 Major | ⚡ Quick winMark the API key response as non-cacheable.
This payload contains the one-time secret, but the success response does not send
Cache-Control: no-store. Add it here so browsers and intermediaries do not retain the key.Proposed fix
return NextResponse.json( { apiKey: body.apiKey, key: body.key, }, - { status: 201 }, + { + status: 201, + headers: { 'Cache-Control': 'no-store' }, + }, );🤖 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/api-keys/route.ts` around lines 225 - 231, The response that returns the one-time secret (using NextResponse.json with payload containing body.apiKey and body.key) must be marked non-cacheable; update the NextResponse.json call to include headers: { "Cache-Control": "no-store" } so browsers and intermediaries do not retain the key (i.e., NextResponse.json({...}, { status: 201, headers: { "Cache-Control": "no-store" } })). Ensure you modify the response creation in route.ts where NextResponse.json is used to return the API key.
🤖 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.
Outside diff comments:
In `@app/api/api-keys/route.ts`:
- Around line 225-231: The response that returns the one-time secret (using
NextResponse.json with payload containing body.apiKey and body.key) must be
marked non-cacheable; update the NextResponse.json call to include headers: {
"Cache-Control": "no-store" } so browsers and intermediaries do not retain the
key (i.e., NextResponse.json({...}, { status: 201, headers: { "Cache-Control":
"no-store" } })). Ensure you modify the response creation in route.ts where
NextResponse.json is used to return the API key.
In `@src/features/api-keys/api-key-console-view.tsx`:
- Around line 100-105: The handleCopy function can throw if
navigator.clipboard.writeText rejects; wrap the await call in a try/catch around
navigator.clipboard.writeText(createdKey.apiKey) inside handleCopy, handle the
error by logging it (console.error or processLogger), setCopied(false) or set an
error state to show the user a failure message, and optionally fall back to a
document.execCommand('copy') or selecting a temporary textarea to copy; ensure
you still return early if !createdKey and reference the handleCopy function,
createdKey, and setCopied when implementing the fix.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 25df52c6-6b71-436c-975e-9969922013ff
📒 Files selected for processing (6)
app/api/api-keys/route.tsdocs/TECHNICAL_OVERVIEW.mdsrc/features/api-keys/api-key-console-view.tsxsrc/features/markets/components/market-details-block.tsxsrc/features/markets/components/table/market-table-utils.tsxsrc/utils/apiKeyRequest.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- src/features/markets/components/table/market-table-utils.tsx
- src/features/markets/components/market-details-block.tsx
Owner
Author
|
Closing this messy branch in favor of the clean API-console-only PR: #554 |
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
1 participant
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.
Summary
/api-keys.Verification
npx ultracite fixnpx ultracite checkpnpm lint:checkpnpm typecheckpnpm buildgit diff --checknpm run typecheckinmonarch-data-gateway-workerSummary by CodeRabbit
New Features
Improvements
Documentation