Add webhook verification functionality for secure payload validation #7103
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
🦋 Changeset detectedLatest commit: 9aea6f4 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 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 |
WalkthroughThis update introduces webhook verification to the thirdweb package by adding a Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Server
participant Webhook
Client->>Server: Sends webhook request (payload, headers)
Server->>Webhook: Calls parse(payload, headers, secret, tolerance?)
Webhook->>Server: Validates signature & timestamp, parses payload
alt Valid request & supported version
Webhook-->>Server: Returns parsed payload
Server-->>Client: Processes webhook
else Invalid signature/timestamp/version
Webhook-->>Server: Throws error
Server-->>Client: Responds with error
end
Suggested labels
📜 Recent review detailsConfiguration used: CodeRabbit UI ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (9)
✅ Files skipped from review due to trivial changes (3)
🚧 Files skipped from review as they are similar to previous changes (6)
⏰ Context from checks skipped due to timeout of 90000ms (8)
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
How to use the Graphite Merge QueueAdd either label to this PR to merge it via the merge queue:
You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has enabled the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. This stack of pull requests is managed by Graphite. Learn more about stacking. |
jnsdls
force-pushed
the
Add_webhook_verification_functionality_for_secure_payload_validation
branch
from
09b2a7e to
4884440
Compare
![graphite-app[bot]](https://avatars.githubusercontent.com/in/158384?s=60&v=4){kind=small}
jnsdls
force-pushed
the
Add_webhook_verification_functionality_for_secure_payload_validation
branch
from
4884440 to
e7c744f
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 1
🧹 Nitpick comments (2)
packages/thirdweb/src/bridge/Webhook.test.ts (1)
127-127: Remove debug console.log statement.This debug log statement should be removed before merging the code.
- console.log("Payload string:", v1PayloadString); // Debug logpackages/thirdweb/src/bridge/Webhook.ts (1)
76-82: Consider adding payload structure validation.After parsing the JSON, it would be beneficial to validate that the payload structure matches the expected structure for the given version, especially for critical fields. This would provide earlier and clearer error messages for malformed payloads.
🛑 Comments failed to post (1)
packages/thirdweb/src/bridge/Webhook.ts (1)
85-89: 🛠️ Refactor suggestion
Add explicit version validation to ensure only version 2 is accepted.
Currently, the code rejects version 1 payloads but doesn't explicitly verify that the version is 2. This could potentially allow payloads with other version numbers to be processed without proper validation.
// Check version after successful JSON parsing if (parsedPayload.version === 1) { throw new Error( "Invalid webhook payload: version 1 is no longer supported, please upgrade to webhook version 2.", ); } + // Ensure only version 2 is accepted + if (parsedPayload.version !== 2) { + throw new Error( + `Invalid webhook payload: unsupported version ${parsedPayload.version}. Only version 2 is supported.` + ); + }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.// Check version after successful JSON parsing if (parsedPayload.version === 1) { throw new Error( "Invalid webhook payload: version 1 is no longer supported, please upgrade to webhook version 2.", ); } // Ensure only version 2 is accepted if (parsedPayload.version !== 2) { throw new Error( `Invalid webhook payload: unsupported version ${parsedPayload.version}. Only version 2 is supported.` ); }🤖 Prompt for AI Agents
In packages/thirdweb/src/bridge/Webhook.ts around lines 85 to 89, the code only rejects version 1 payloads but does not explicitly check that the payload version is exactly 2. Update the validation logic to explicitly verify that parsedPayload.version equals 2, and throw an error for any other version values to ensure only version 2 payloads are accepted.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
packages/thirdweb/src/bridge/Webhook.ts (2)
30-33: Consider case-insensitive header matching for more robust header handling.The current implementation checks for specific header names with exact casing, but HTTP headers are case-insensitive according to the spec. Using lowercase for all header names would make this more robust against variations in header casing from different environments or proxies.
- const receivedSignature = - headers["x-payload-signature"] || headers["x-pay-signature"]; - const receivedTimestamp = - headers["x-timestamp"] || headers["x-pay-timestamp"]; + // Normalize headers to lowercase for case-insensitive matching + const normalizedHeaders = Object.fromEntries( + Object.entries(headers).map(([key, value]) => [key.toLowerCase(), value]) + ); + const receivedSignature = + normalizedHeaders["x-payload-signature"] || normalizedHeaders["x-pay-signature"]; + const receivedTimestamp = + normalizedHeaders["x-timestamp"] || normalizedHeaders["x-pay-timestamp"];
1-92: Add function to generate webhook signatures for testing and validation.Consider adding a companion function that generates webhook signatures using the same algorithm. This would be valuable for users who need to test their webhook handlers or validate the signature generation process.
/** * Generates a signature for an outgoing webhook payload. * This is useful for testing webhook handlers. * * @param payload - The payload to sign * @param secret - The webhook secret to sign with * @returns The signature and timestamp that can be used in headers */ export async function generateSignature( payload: string, secret: string, ): Promise<{ signature: string; timestamp: string }> { const timestamp = Math.floor(Date.now() / 1000).toString(); const encoder = new TextEncoder(); const key = await crypto.subtle.importKey( "raw", encoder.encode(secret), { name: "HMAC", hash: "SHA-256" }, false, ["sign"], ); const signature = await crypto.subtle.sign( "HMAC", key, encoder.encode(`${timestamp}.${payload}`), ); // Convert the signature to hex string const hexSignature = Array.from(new Uint8Array(signature)) .map((b) => b.toString(16).padStart(2, "0")) .join(""); return { signature: hexSignature, timestamp, }; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.changeset/webhook-verification.md(1 hunks)packages/thirdweb/src/bridge/Webhook.test.ts(1 hunks)packages/thirdweb/src/bridge/Webhook.ts(1 hunks)packages/thirdweb/src/bridge/index.ts(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/thirdweb/src/bridge/Webhook.test.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- .changeset/webhook-verification.md
- packages/thirdweb/src/bridge/index.ts
🧰 Additional context used
🧬 Code Graph Analysis (1)
packages/thirdweb/src/bridge/Webhook.ts (2)
packages/thirdweb/src/bridge/index.ts (2)
parse(9-9)WebhookPayload(22-22)apps/dashboard/next.config.ts (1)
headers(129-143)
⏰ Context from checks skipped due to timeout of 90000ms (7)
- GitHub Check: Analyze (javascript)
- GitHub Check: E2E Tests (pnpm, esbuild)
- GitHub Check: E2E Tests (pnpm, webpack)
- GitHub Check: Unit Tests
- GitHub Check: E2E Tests (pnpm, vite)
- GitHub Check: Size
- GitHub Check: Lint Packages
🔇 Additional comments (3)
packages/thirdweb/src/bridge/Webhook.ts (3)
39-48: Excellent timestamp validation implementation.The timestamp validation logic is well-implemented with proper tolerance handling and clear error messaging. Using
Math.abs()correctly handles both future and past timestamps, which is important for webhook validation across systems with clock skew.
94-119: Well-structured type definition with strong typing.The
WebhookPayloadtype is well-designed using a discriminated union based on version. The version 2 payload has proper typing for all fields, including using template literals for Ethereum addresses (0x${string}), which helps prevent address format errors.
50-74:✅ Verification successful
Robust signature verification using Web Crypto API.
The signature verification implementation is secure and follows best practices by using the Web Crypto API with HMAC-SHA256. The hex conversion is correctly implemented, and the signature comparison uses strict equality.
Note that the Web Crypto API may not be available in all Node.js environments. Please verify your target environments support this API or consider adding a fallback implementation:
🏁 Script executed:
#!/bin/bash # Check if this code is intended to run in Node.js versions that might not support Web Crypto API fd -e json "package.json" --exec grep -l "\"engines\"" {} \; | xargs catLength of output: 27662
Signature verification via Web Crypto API is safe on supported environments
The HMAC-SHA256 signature check is implemented correctly and securely using the Web Crypto API. Since our
packages/thirdwebpackage.json declares“engines”: { "node": ">=18" }and all browsers support Web Crypto, the globalcrypto.subtleAPI is guaranteed in all targeted runtimes—no fallback is required.
- File: packages/thirdweb/src/bridge/Webhook.ts (lines 50–74)
size-limit report 📦
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #7103 +/- ##
==========================================
+ Coverage 55.62% 55.67% +0.05%
==========================================
Files 902 904 +2
Lines 58190 58328 +138
Branches 4098 4120 +22
==========================================
+ Hits 32369 32475 +106
- Misses 25716 25748 +32
Partials 105 105
🚀 New features to boost your workflow:
|
jnsdls
force-pushed
the
Add_webhook_verification_functionality_for_secure_payload_validation
branch
from
e7c744f to
bbe5dfc
Compare
jnsdls
force-pushed
the
Add_webhook_verification_functionality_for_secure_payload_validation
branch
from
3824d8d to
c5e2b36
Compare
github-actions
bot
added
Dashboard
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
jnsdls
force-pushed
the
Add_webhook_verification_functionality_for_secure_payload_validation
branch
from
c5e2b36 to
a8a346a
Compare
jnsdls
force-pushed
the
Add_webhook_verification_functionality_for_secure_payload_validation
branch
from
a8a346a to
3c5e0c1
Compare
jnsdls
force-pushed
the
Add_webhook_verification_functionality_for_secure_payload_validation
branch
from
3c5e0c1 to
9aea6f4
Compare
jnsdls
deleted the
Add_webhook_verification_functionality_for_secure_payload_validation
branch
PR-Codex overview
This PR primarily updates the
zoddependency to version3.25.24across multiple packages and introduces a new webhook verification feature with associated tests.Detailed summary
zodfrom3.24.3to3.25.24in severalpackage.jsonfiles.packages/thirdweb/.size-limit.json.changeset/webhook-verification.md.Webhook.parsefunction for verifying webhook signatures.Webhook.parsefunction inpackages/thirdweb/src/bridge/Webhook.test.ts.Summary by CodeRabbit
New Features
Bug Fixes
Tests