fix(backend): Clock skew of 0 should not fall back#8359
Conversation
Because 0 is falsy the current code fell back to the default value when the clock skew was configured to 0. This changes the syntax to fall back on null-ish values which 0 is not.
🦋 Changeset detectedLatest commit: b6571c6 The changes in this PR will be included in the next version bump. This PR includes changesets to release 10 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/dev-cli
@clerk/expo
@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 Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughReplaced the previous Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/backend/src/jwt/verifyJwt.ts`:
- Line 134: The resolved clockSkew value currently uses nullish coalescing but
can remain NaN; before calling assertExpirationClaim, assertActivationClaim, and
assertIssuedAtClaim validate that clockSkew (the const clockSkew variable) is a
finite number (e.g., Number.isFinite(clockSkew)), and if not replace it with
DEFAULT_CLOCK_SKEW_IN_MS or throw a descriptive error; update the code around
the clockSkew assignment so callers passing NaN cannot bypass claim checks by
ensuring a finite skew is passed into
assertExpirationClaim/assertActivationClaim/assertIssuedAtClaim.
🪄 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 YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: 478f1c59-4092-4cbe-828b-319201b9cd77
📒 Files selected for processing (2)
.changeset/wacky-dryers-hammer.mdpackages/backend/src/jwt/verifyJwt.ts
jacekradko
left a comment
There was a problem hiding this comment.
Good enhancement, but we should add tests for this
Description
Because 0 is falsy the current code fell back to the default value when the clock skew was configured to 0. This changes the syntax to fall back on null-ish values which 0 is not.
Checklist
pnpm testruns as expected.pnpm buildruns as expected.Type of change