fix(shared): decode JWT v2 permission masks exactly - #9381
Conversation
🦋 Changeset detectedLatest commit: 9dd9f76 The changes in this PR will be included in the next version bump. This PR includes changesets to release 23 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
@clerk/astro
@clerk/backend
@clerk/chrome-extension
@clerk/clerk-js
@clerk/electron
@clerk/electron-passkeys
@clerk/eslint-plugin
@clerk/expo
@clerk/expo-google-signin
@clerk/expo-passkeys
@clerk/express
@clerk/fastify
@clerk/hono
@clerk/localizations
@clerk/nextjs
@clerk/nuxt
@clerk/react
@clerk/react-router
@clerk/shared
@clerk/tanstack-react-start
@clerk/testing
@clerk/ui
@clerk/upgrade
@clerk/vue
commit: |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Organization UI (inherited) Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🔗 Linked repositories identifiedCodeRabbit considers these linked repositories for cross-repo context during reviews:
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThe JWT payload parser now converts decimal permission masks through custom decimal division. This avoids precision loss for values beyond JavaScript’s safe integer range. Invalid masks produce empty bit arrays. Tests cover large masks, valid safe masks, out-of-range bits, malformed masks, and fail-closed authorization checks. A patch changeset was added for Estimated code review effort: 3 (Moderate) | ~20 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Comment |
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 `@packages/shared/src/jwtPayloadParser.ts`:
- Around line 52-54: Update buildOrgPermissions and the featurePermissionMap
handling around decimalToBinaryBits so bit indexes greater than or equal to
permissions.length are discarded before permission names are resolved. Preserve
valid permission mappings and add a regression test covering the 54-item list
with mask 18014398509481984, ensuring no org:feature:undefined permission is
produced.
🪄 Autofix
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 YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: 1868a535-5352-4646-8435-325176767171
📒 Files selected for processing (3)
.changeset/exact-jwt-permission-masks.mdpackages/shared/src/__tests__/jwtPayloadParser.spec.tspackages/shared/src/jwtPayloadParser.ts
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
clerk/clerk_go(manual)clerk/dashboard(manual)clerk/accounts(manual)clerk/backoffice(manual)clerk/clerk(manual)clerk/clerk-docs(manual)clerk/cloudflare-workers(manual)clerk/cli(auto-detected)clerk/clerk-ios(auto-detected)clerk/clerk-android(auto-detected)
API Changes Report
Summary
No API Changes DetectedAll packages have stable APIs with no detected changes. Report generated by Break Check Last ran on |
Description
JWT v2 organization permission masks were parsed through JavaScript numbers, which could lose precision above the safe integer limit and cause
auth().has()to incorrectly grant or deny permissions.Decode permission masks directly from their decimal strings with
decimalToBinaryBits. This avoids relying onBigInt, which may be incompatible with ClerkJS's legacy browser bundle, while preserving the existing bit ordering for normal masks. Malformed and negative masks now fail closed.Type of change