Add API key console#554
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Review limit reached
More reviews will be available in 33 minutes and 28 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis PR introduces a complete API key generation flow. Users sign a message containing wallet address, origin, and timestamp with their connected wallet. The server verifies the signature against chain-specific RPC endpoints, validates origin and timestamp freshness, then calls an external gateway to create the API key. The UI is accessible via the ChangesAPI Key Console Feature
🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
🚥 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 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 |
There was a problem hiding this comment.
Code Review
This pull request introduces an API Key Console feature, allowing users to generate API keys by signing a wallet message. The implementation includes a new /api-keys page, mobile and desktop navigation links, a backend Next.js route (POST /api/api-keys) for signature verification and admin gateway communication, and updated technical documentation. The review feedback highlights a critical security issue where validating the signed origin against the client-supplied Origin header could allow phishing sites to generate keys, recommending verification against the application's own host headers instead. Additionally, suggestions are made to handle potential runtime crashes by adding a fallback for navigator.clipboard in non-secure contexts, and to normalize CRLF line endings to prevent signature parsing failures.
| function getRequestOrigin(request: NextRequest): string | null { | ||
| const origin = request.headers.get('origin'); | ||
| if (origin) return origin.replace(/\/+$/, ''); | ||
|
|
||
| const host = request.headers.get('x-forwarded-host') ?? request.headers.get('host'); | ||
| if (!host) return null; | ||
|
|
||
| const protocol = request.headers.get('x-forwarded-proto') ?? 'https'; | ||
| return `${protocol}://${host}`.replace(/\/+$/, ''); | ||
| } |
There was a problem hiding this comment.
Using the client-supplied Origin header to validate the signed message's origin allows any external website (including phishing sites) to generate API keys using your admin token. A malicious site can prompt the user to sign a message with their own origin (e.g., https://phish-monarch.xyz), and then forward that signature to your API. Since the browser will send Origin: https://phish-monarch.xyz, the check will pass.
To prevent this, you should verify the signed origin against your own application's origin (derived from the host or x-forwarded-host headers), completely ignoring the client-supplied Origin header.
function getRequestOrigin(request: NextRequest): string | null {
const host = request.headers.get('x-forwarded-host') ?? request.headers.get('host');
if (!host) return null;
const protocol = request.headers.get('x-forwarded-proto') ?? 'https';
return `${protocol}://${host}`.replace(/\/+$/, '');
}| const handleCopy = async () => { | ||
| if (!createdKey) return; | ||
|
|
||
| await navigator.clipboard.writeText(createdKey.apiKey); | ||
| setCopied(true); | ||
| }; |
There was a problem hiding this comment.
In non-secure contexts (like HTTP local testing) or older browsers, navigator.clipboard can be undefined. Calling writeText directly on it will cause a runtime crash. Adding a safety check and a fallback using a temporary textarea ensures a robust copy experience across all environments.
const handleCopy = async () => {
if (!createdKey) return;
try {
if (typeof navigator !== 'undefined' && navigator.clipboard) {
await navigator.clipboard.writeText(createdKey.apiKey);
setCopied(true);
} else {
const textArea = document.createElement('textarea');
textArea.value = createdKey.apiKey;
textArea.style.position = 'fixed';
document.body.appendChild(textArea);
textArea.focus();
textArea.select();
document.execCommand('copy');
document.body.removeChild(textArea);
setCopied(true);
}
} catch (err) {
console.error('Failed to copy API key:', err);
}
};
| export function parseApiKeyRequestMessage(message: string): ApiKeyRequestMessage | null { | ||
| const lines = message.split('\n'); | ||
| if (lines[0] !== MESSAGE_TITLE) return null; |
There was a problem hiding this comment.
Some wallets, browser extensions, or operating systems (like Windows) normalize line endings to CRLF (\r\n) when displaying or transmitting signed messages. Splitting strictly by \n without normalizing first can cause signature parsing to fail because of trailing \r characters or mismatched line counts. Normalizing \r\n to \n first makes the parser much more robust.
| export function parseApiKeyRequestMessage(message: string): ApiKeyRequestMessage | null { | |
| const lines = message.split('\n'); | |
| if (lines[0] !== MESSAGE_TITLE) return null; | |
| export function parseApiKeyRequestMessage(message: string): ApiKeyRequestMessage | null { | |
| const normalizedMessage = message.replace(/\r\n/g, '\n'); | |
| const lines = normalizedMessage.split('\n'); | |
| if (lines[0] !== MESSAGE_TITLE) return null; |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/TECHNICAL_OVERVIEW.md (1)
444-444:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUpdate the Monarch GraphQL endpoint in the integrations table.
This row still points to
https://api.monarchlend.xyz/graphql, but the rest of this doc now useshttps://indexer.monarchlend.xyz/graphql. Please align this row too.🤖 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 `@docs/TECHNICAL_OVERVIEW.md` at line 444, Update the integrations table row for "Monarch GraphQL" so the endpoint URL matches the rest of the doc: replace `https://api.monarchlend.xyz/graphql` with `https://indexer.monarchlend.xyz/graphql` in the table row that contains "Monarch GraphQL" and its description of Autovault metadata, market live state, historical charts, market detail/activity, admin transactions.
🤖 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 89-121: The code validates parsedMessage.nonce format but doesn't
prevent replay—after verifyWalletSignature succeeds and before calling
createGatewayApiKey, atomically ensure the nonce is single-use: attempt to
insert or reserve parsedMessage.nonce (plus address/chainId) into a
datastore/table that enforces a unique constraint (e.g., via an
upsert/insert-if-not-exists or transaction) and only proceed to call
createGatewayApiKey when that insert succeeds; if the insert fails because the
nonce already exists, return a 409/400 error for reused nonce; handle race
conditions by relying on the DB uniqueness constraint and treat duplicate-key
errors as nonce replay.
- Around line 252-256: The verifyWalletSignature flow creates a viem public
client with createPublicClient using transport: http(rpcUrl) without a timeout;
update the transport creation in verifyWalletSignature (where createPublicClient
and http(...) are used) to pass a timeout option (e.g. http(rpcUrl, { timeout:
30000 })) so RPC calls from verifyMessage have a bounded request timeout (30s)
and won’t hang indefinitely.
---
Outside diff comments:
In `@docs/TECHNICAL_OVERVIEW.md`:
- Line 444: Update the integrations table row for "Monarch GraphQL" so the
endpoint URL matches the rest of the doc: replace
`https://api.monarchlend.xyz/graphql` with
`https://indexer.monarchlend.xyz/graphql` in the table row that contains
"Monarch GraphQL" and its description of Autovault metadata, market live state,
historical charts, market detail/activity, admin transactions.
🪄 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: 5f898182-da86-4953-a926-1fac580ca97f
📒 Files selected for processing (9)
.env.local.exampleapp/api-keys/page.tsxapp/api/api-keys/route.tsdocs/TECHNICAL_OVERVIEW.mdsrc/components/layout/header/Navbar.tsxsrc/components/layout/header/NavbarMobile.tsxsrc/features/api-keys/api-key-console-view.tsxsrc/utils/apiKeyRequest.tssrc/utils/external.ts
| if (!/^[A-Za-z0-9-]{16,80}$/.test(parsedMessage.nonce)) { | ||
| return NextResponse.json({ error: 'Invalid signature nonce.' }, { status: 400 }); | ||
| } | ||
|
|
||
| if (!isSupportedNetwork(parsedMessage.chainId)) { | ||
| return NextResponse.json({ error: 'Unsupported signature chain.' }, { status: 400 }); | ||
| } | ||
|
|
||
| let signatureValid: boolean; | ||
| try { | ||
| signatureValid = await verifyWalletSignature({ | ||
| address, | ||
| chainId: parsedMessage.chainId, | ||
| message: body.message, | ||
| signature: body.signature, | ||
| }); | ||
| } catch { | ||
| return NextResponse.json({ error: 'Failed to verify wallet signature.' }, { status: 502 }); | ||
| } | ||
|
|
||
| if (!signatureValid) { | ||
| return NextResponse.json({ error: 'Invalid wallet signature.' }, { status: 401 }); | ||
| } | ||
|
|
||
| const adminResponse = await createGatewayApiKey({ | ||
| adminToken, | ||
| address, | ||
| name: body.name, | ||
| chainId: parsedMessage.chainId, | ||
| origin: parsedMessage.origin, | ||
| issuedAt: parsedMessage.issuedAt, | ||
| nonce: parsedMessage.nonce, | ||
| }); |
There was a problem hiding this comment.
Block replay before minting the key.
This only checks that nonce looks valid. It never proves the nonce was unused before calling the non-idempotent admin endpoint, so the same signed payload can be replayed for the full 10-minute TTL to mint multiple keys.
🤖 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 89 - 121, The code validates
parsedMessage.nonce format but doesn't prevent replay—after
verifyWalletSignature succeeds and before calling createGatewayApiKey,
atomically ensure the nonce is single-use: attempt to insert or reserve
parsedMessage.nonce (plus address/chainId) into a datastore/table that enforces
a unique constraint (e.g., via an upsert/insert-if-not-exists or transaction)
and only proceed to call createGatewayApiKey when that insert succeeds; if the
insert fails because the nonce already exists, return a 409/400 error for reused
nonce; handle race conditions by relying on the DB uniqueness constraint and
treat duplicate-key errors as nonce replay.
| const rpcUrl = RPC_ENV_BY_CHAIN[chainId]?.trim() || undefined; | ||
| const client = createPublicClient({ | ||
| chain: VERIFICATION_CHAINS[chainId], | ||
| transport: http(rpcUrl), | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In viem v2.40.2, does http()support a timeout option forcreatePublicClient, and what is the correct way to configure a request timeout for verifyMessage RPC calls?
💡 Result:
Yes, in viem v2.40.2, the http transport supports a timeout option [1][2]. This option defines the timeout (in milliseconds) for HTTP requests and defaults to 10,000 ms [1][2]. To configure this, pass a configuration object as the second argument to the http function when defining the transport for your createPublicClient [1][3]: import { createPublicClient, http } from 'viem' import { mainnet } from 'viem/chains' const client = createPublicClient({ chain: mainnet, transport: http('https://your-rpc-url.com', { timeout: 60_000, // Timeout set to 60 seconds }), }) Because verifyMessage (when used as a public action) performs RPC calls—such as eth_call to a signature validator contract—it inherits the transport configuration of the public client [4][5]. Therefore, the correct way to configure a request timeout for verifyMessage is by setting the timeout option directly on the http transport used by the public client, as shown above. The verifyMessage action itself does not have a separate, direct timeout parameter in its function signature [4][6].
Citations:
- 1: https://viem.sh/docs/clients/transports/http
- 2: https://github.com/wevm/viem/blob/a59b5630/src/clients/transports/http.ts
- 3: https://viem-iyn6kgwt-wxqdoit.4everland.app/docs/clients/transports/http
- 4: https://viem.sh/docs/actions/public/verifyMessage
- 5: https://v1.viem.sh/docs/actions/public/verifyMessage.html
- 6: https://viem-iyn6kgwt-wxqdoit.4everland.app/docs/utilities/verifyMessage.html
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the referenced snippet area
sed -n '220,290p' app/api/api-keys/route.ts | cat -n
# Find verifyWalletSignature definition and usages
rg -n "verifyWalletSignature" app/api/api-keys/route.ts
# Show surrounding code where verifyWalletSignature is implemented/used
rg -n "verifyWalletSignature" app/api/api-keys/route.ts -n --context=8
# Find createPublicClient usage (in case there are multiple)
rg -n "createPublicClient\\(" app/api/api-keys/route.tsRepository: antoncoding/monarch
Length of output: 3554
Add an RPC timeout to signature verification
verifyWalletSignature() creates the viem client with transport: http(rpcUrl) but no explicit timeout, and verifyMessage() performs the RPC using that transport. In viem v2.40.2, set the request timeout on the transport via http(rpcUrl, { timeout: <ms> }) (e.g. 30s).
Current code:
const rpcUrl = RPC_ENV_BY_CHAIN[chainId]?.trim() || undefined;
const client = createPublicClient({
chain: VERIFICATION_CHAINS[chainId],
transport: http(rpcUrl),
});🤖 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 252 - 256, The verifyWalletSignature
flow creates a viem public client with createPublicClient using transport:
http(rpcUrl) without a timeout; update the transport creation in
verifyWalletSignature (where createPublicClient and http(...) are used) to pass
a timeout option (e.g. http(rpcUrl, { timeout: 30000 })) so RPC calls from
verifyMessage have a bounded request timeout (30s) and won’t hang indefinitely.
Summary
/api-keyswith the existing wallet connect control, wallet-signed key creation, one-time key display, and Docs link.POST /api/api-keysto verify signed wallet requests and create D1-backed gateway keys through the data gateway admin API.Verification
pnpm typecheckpnpm buildgit diff --checkNotes
npx ultracite fixor include lint/tooling cleanup in this PR.monarch-data-gateway-workerbranchcodex/wallet-api-key-creation.Summary by CodeRabbit
Release Notes
New Features
Documentation