fix: use constant-time comparison for Shopify and HubSpot webhook signatures - #4449
Conversation
…natures - Replace plain !== with crypto.timingSafeEqual in both handlers - Add length check before comparison (same pattern as Intercom handler) - Mitigates timing side-channel attacks (CWE-208) Fixes #4415 Co-authored-by: Kiran K <kiran@dub.co>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (3)
Included review availability: Your plan provides up to 10 included reviews per hour; 7 remain after this review. 📝 WalkthroughWalkthroughThe change adds a shared constant-time comparison helper. Webhook handlers, OAuth token endpoints, and unsubscribe-token verification now use it for secret and signature checks. Shopify and Dub webhook handlers also use ChangesConstant-time verification
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to Webhook and secret comparisons now use a shared timing-safe helper, preserving invalid-signature rejection behavior while reducing timing side-channel exposure. No merge-blocking risk is identified. Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches 💡 1📝 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 |
- Created timingSafeCompare utility in lib/webhook/timing-safe-compare.ts - Updated all webhook signature verifications to use the shared utility: - Shopify webhook handler - HubSpot webhook handler - Intercom webhook handler - Slack signature verification - Dub webhook handler - Updated OAuth token handlers to use constant-time comparison: - exchange-code-for-token (client_secret verification) - refresh-access-token (client_secret verification) This ensures consistent constant-time comparison across the codebase and prevents timing side-channel attacks (CWE-208). Co-authored-by: Kiran K <kiran@dub.co>
|
@coderabbitai full review |
✅ Action performedFull review finished. |
What does this PR do?
Extracts constant-time comparison logic into a shared utility function and applies it consistently across all webhook signature verifications and OAuth token handlers to prevent timing side-channel attacks.
Changes
New Shared Utility
apps/web/lib/webhook/timing-safe-compare.ts: Created a reusabletimingSafeCompare()function that:crypto.timingSafeEqualUpdated Webhook Handlers
All webhook signature verifications now use the shared utility:
apps/web/app/(ee)/api/shopify/integration/webhook/route.ts)apps/web/app/(ee)/api/hubspot/webhook/route.ts)apps/web/app/(ee)/api/intercom/webhook/verify-webhook-signature.ts)apps/web/lib/integrations/slack/verify-request.ts)apps/web/app/api/dub/webhook/route.ts)Updated OAuth Handlers
OAuth client secret comparisons now use constant-time comparison:
apps/web/app/api/oauth/token/exchange-code-for-token.tsapps/web/app/api/oauth/token/refresh-access-token.tsSecurity Impact
Mitigates timing side-channel attacks (CWE-208) by ensuring all security-sensitive string comparisons take constant time regardless of where the strings differ. While timing attacks against HMAC comparisons are difficult to exploit over the network due to jitter, this change:
Code Quality Benefits
Fixes #4415
Slack Thread
Summary by CodeRabbit