Conversation
- Introduced a new function to resolve the JWT verification key from environment variables, supporting both raw PEM and base64 formats. - Updated the PrivyClient instantiation to conditionally include the jwtVerificationKey based on the resolved value.
|
@wisdom1016 is attempting to deploy a commit to the Recoupable Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughThe change refactors JWT verification key handling by introducing a Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 1✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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.
🧹 Nitpick comments (1)
lib/privy/client.ts (1)
12-15: Tighten PEM detection to avoid false positives.Line 12 and Line 15 use
includes("BEGIN"), which is too broad and can incorrectly treat non-PEM strings as PEM. Use a strict PEM header regex instead.As per coding guidelines, "KISS: Prefer simple, readable implementations over clever optimizations."Proposed diff
function resolveJwtVerificationKey(): string | undefined { const raw = process.env.PRIVY_JWT_VERIFICATION_KEY?.trim(); if (!raw) return undefined; - if (raw.includes("BEGIN")) return raw; + const isPem = (value: string) => /^-----BEGIN [A-Z0-9 ]+-----/m.test(value); + if (isPem(raw)) return raw; try { const decoded = Buffer.from(raw, "base64").toString("utf8"); - if (decoded.includes("BEGIN")) return decoded; + if (isPem(decoded)) return decoded; } catch { /* ignore */ } return raw; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/privy/client.ts` around lines 12 - 15, The PEM detection using raw.includes("BEGIN") and decoded.includes("BEGIN") is too broad; replace those checks with a strict PEM header/footer regex (e.g. /^-----BEGIN [A-Z ]+-----/m and corresponding END) and test raw (and the base64-decoded decoded) against that regex so only actual PEM blocks are accepted; update the checks around the Buffer.from(raw, "base64").toString("utf8") flow (the raw and decoded variables) to use regex.test(...) and return only when the regex matches.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@lib/privy/client.ts`:
- Around line 12-15: The PEM detection using raw.includes("BEGIN") and
decoded.includes("BEGIN") is too broad; replace those checks with a strict PEM
header/footer regex (e.g. /^-----BEGIN [A-Z ]+-----/m and corresponding END) and
test raw (and the base64-decoded decoded) against that regex so only actual PEM
blocks are accepted; update the checks around the Buffer.from(raw,
"base64").toString("utf8") flow (the raw and decoded variables) to use
regex.test(...) and return only when the regex matches.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 6fd0cef0-2431-4c8a-8da4-9b2802fc373b
📒 Files selected for processing (1)
lib/privy/client.ts
Summary
Pre-trial work for the API repo: sandbox setup endpoint and auth/config improvements.
Changes
lib/privy/client.tsupdated to support raw PEM, base64(PEM), or omitting the key (Privy SDK fetches JWKS). Fixes "Failed to verify authentication token" when PEM was incorrectly base64-decoded.Testing
TRIGGER_API_URLandTRIGGER_SECRET_KEYare set; without them it returns 500 with a clear env error (expected in local dev without Trigger.dev credentials).validateSetupSandboxBodyandsetupSandboxHandlerpass.Summary by CodeRabbit