Http - verify hmac signature #14717
Http - verify hmac signature #14717
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ |
WalkthroughThe changes introduce a new module for validating HMAC signatures in incoming HTTP webhook requests, encapsulated in the file Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (2)
components/http/actions/verify-hmac-signature/verify-hmac-signature.mjs (2)
5-5: Consider starting with version 1.0.0 for this security-critical componentFollowing semantic versioning, it's recommended to start with version 1.0.0 for security-critical components to indicate a stable and production-ready implementation.
- version: "0.0.1", + version: "1.0.0",
66-66: Enhance success loggingAdd more context to the success summary for better audit trails.
- $.export("$summary", "HTTP request successfully authenticated"); + $.export("$summary", `HTTP request authenticated successfully at ${new Date().toISOString()}`);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
components/http/actions/verify-hmac-signature/verify-hmac-signature.mjs(1 hunks)components/http/package.json(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- components/http/package.json
🔇 Additional comments (1)
components/http/actions/verify-hmac-signature/verify-hmac-signature.mjs (1)
1-68: Verify integration points and usage patterns
Let's verify how this component is being used across the codebase to ensure secure implementation.
✅ Verification successful
Based on the search results, I can now provide a final response:
HMAC verification implementation is secure and properly isolated
The implementation follows security best practices:
- Uses Node's native
cryptomodule for HMAC operations - Implements timing-safe comparison to prevent timing attacks
- Properly handles signature length validation
- No hardcoded secrets found in the codebase
The component is self-contained and other implementations of HMAC verification in the codebase (found in cats, cloud_convert, workbooks_crm, etc.) use their own isolated implementations appropriate for their specific use cases.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for proper usage of the HMAC verification component
# Check for direct imports of this component
rg -l "verify-hmac-signature"
# Look for HTTP trigger configurations
rg -A 5 "return_response: true" components/
# Check for potential hardcoded secrets
rg -g '!{*.test.*,*.spec.*}' -e 'createHmac.*sha256.*["\'].*["\']'
Length of output: 265
Script:
#!/bin/bash
# Let's try a more comprehensive verification approach
# Check for any references or imports of this component
rg -l -i "hmac.*signature|verify.*hmac"
# Look for HTTP trigger configurations with different patterns
rg "interface.http" components/
# Check for HMAC-related implementations
ast-grep --pattern 'createHmac("sha256", $_)'
# Look for workflow configurations that might use this component
fd -e json . components/ -x cat {} \; | grep -i "verify.*hmac"
Length of output: 33159
| if (!valid) { | ||
| if (this.customResponse) { | ||
| await $.respond({ | ||
| status: 401, | ||
| headers: {}, | ||
| body: "Invalid credentials", | ||
| }); | ||
| } | ||
| return $.flow.exit("Invalid credentials"); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add security logging and secure headers
For security monitoring and hardening:
- Add logging for validation failures
- Include security headers in the response
- Consider rate limiting for failed attempts
if (!valid) {
+ console.error("HMAC validation failed", {
+ timestamp: new Date().toISOString(),
+ ip: $.event.client_ip,
+ });
+
if (this.customResponse) {
await $.respond({
status: 401,
- headers: {},
+ headers: {
+ "Content-Type": "text/plain",
+ "X-Content-Type-Options": "nosniff",
+ "Cache-Control": "no-store",
+ },
body: "Invalid credentials",
});
}
return $.flow.exit("Invalid credentials");
}📝 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.
| if (!valid) { | |
| if (this.customResponse) { | |
| await $.respond({ | |
| status: 401, | |
| headers: {}, | |
| body: "Invalid credentials", | |
| }); | |
| } | |
| return $.flow.exit("Invalid credentials"); | |
| } | |
| if (!valid) { | |
| console.error("HMAC validation failed", { | |
| timestamp: new Date().toISOString(), | |
| ip: $.event.client_ip, | |
| }); | |
| if (this.customResponse) { | |
| await $.respond({ | |
| status: 401, | |
| headers: { | |
| "Content-Type": "text/plain", | |
| "X-Content-Type-Options": "nosniff", | |
| "Cache-Control": "no-store", | |
| }, | |
| body: "Invalid credentials", | |
| }); | |
| } | |
| return $.flow.exit("Invalid credentials"); | |
| } |
| signature: { | ||
| type: "string", | ||
| label: "Signature", | ||
| description: "The HMAC signature received from the incoming webhook, typically provided in a specific HTTP header", | ||
| }, | ||
| bodyRaw: { | ||
| type: "string", | ||
| label: "Raw Body", | ||
| description: "The raw request body received from the webhook caller, provided as a string without any parsing or modifications", | ||
| }, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add input validation for signature and bodyRaw props
For security-critical components, it's important to validate inputs before processing. Consider adding:
- Minimum length requirement for the signature
- Maximum length limits for both signature and bodyRaw to prevent DoS
- Format validation for the signature (hex string)
signature: {
type: "string",
label: "Signature",
description: "The HMAC signature received from the incoming webhook, typically provided in a specific HTTP header",
+ validate: (value) => {
+ if (!/^[0-9a-f]{64}$/i.test(value)) {
+ throw new Error("Signature must be a 64-character hexadecimal string");
+ }
+ },
},
bodyRaw: {
type: "string",
label: "Raw Body",
description: "The raw request body received from the webhook caller, provided as a string without any parsing or modifications",
+ validate: (value) => {
+ if (value.length > 1048576) { // 1MB limit
+ throw new Error("Request body exceeds maximum size of 1MB");
+ }
+ },
},📝 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.
| signature: { | |
| type: "string", | |
| label: "Signature", | |
| description: "The HMAC signature received from the incoming webhook, typically provided in a specific HTTP header", | |
| }, | |
| bodyRaw: { | |
| type: "string", | |
| label: "Raw Body", | |
| description: "The raw request body received from the webhook caller, provided as a string without any parsing or modifications", | |
| }, | |
| signature: { | |
| type: "string", | |
| label: "Signature", | |
| description: "The HMAC signature received from the incoming webhook, typically provided in a specific HTTP header", | |
| validate: (value) => { | |
| if (!/^[0-9a-f]{64}$/i.test(value)) { | |
| throw new Error("Signature must be a 64-character hexadecimal string"); | |
| } | |
| }, | |
| }, | |
| bodyRaw: { | |
| type: "string", | |
| label: "Raw Body", | |
| description: "The raw request body received from the webhook caller, provided as a string without any parsing or modifications", | |
| validate: (value) => { | |
| if (value.length > 1048576) { // 1MB limit | |
| throw new Error("Request body exceeds maximum size of 1MB"); | |
| } | |
| }, | |
| }, |
| _checkHmac(secret, signature, bodyRaw) { | ||
| const expectedSignature = crypto | ||
| .createHmac("sha256", secret) | ||
| .update(bodyRaw, "utf8") | ||
| .digest(); | ||
|
|
||
| const signatureBuffer = Buffer.from(signature, "hex"); | ||
| if (signatureBuffer.length !== expectedSignature.length) { | ||
| return false; | ||
| } | ||
| return crypto.timingSafeEqual(signatureBuffer, expectedSignature); | ||
| }, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance input validation and add JSDoc documentation
While the core HMAC validation is secure, there are some improvements needed:
- Add explicit null/empty input validation
- Document the expected secret key format and encoding
- Add JSDoc for better maintainability
+ /**
+ * Validates an HMAC signature against a request body
+ * @param {string} secret - The secret key (UTF-8 encoded)
+ * @param {string} signature - Hexadecimal signature string
+ * @param {string} bodyRaw - Raw request body
+ * @returns {boolean} True if signature is valid
+ * @throws {Error} If inputs are null/empty or improperly formatted
+ */
_checkHmac(secret, signature, bodyRaw) {
+ if (!secret || !signature || !bodyRaw) {
+ throw new Error("Missing required parameters");
+ }
+
const expectedSignature = crypto
.createHmac("sha256", secret)
.update(bodyRaw, "utf8")
.digest();
const signatureBuffer = Buffer.from(signature, "hex");
if (signatureBuffer.length !== expectedSignature.length) {
return false;
}
return crypto.timingSafeEqual(signatureBuffer, expectedSignature);
},📝 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.
| _checkHmac(secret, signature, bodyRaw) { | |
| const expectedSignature = crypto | |
| .createHmac("sha256", secret) | |
| .update(bodyRaw, "utf8") | |
| .digest(); | |
| const signatureBuffer = Buffer.from(signature, "hex"); | |
| if (signatureBuffer.length !== expectedSignature.length) { | |
| return false; | |
| } | |
| return crypto.timingSafeEqual(signatureBuffer, expectedSignature); | |
| }, | |
| /** | |
| * Validates an HMAC signature against a request body | |
| * @param {string} secret - The secret key (UTF-8 encoded) | |
| * @param {string} signature - Hexadecimal signature string | |
| * @param {string} bodyRaw - Raw request body | |
| * @returns {boolean} True if signature is valid | |
| * @throws {Error} If inputs are null/empty or improperly formatted | |
| */ | |
| _checkHmac(secret, signature, bodyRaw) { | |
| if (!secret || !signature || !bodyRaw) { | |
| throw new Error("Missing required parameters"); | |
| } | |
| const expectedSignature = crypto | |
| .createHmac("sha256", secret) | |
| .update(bodyRaw, "utf8") | |
| .digest(); | |
| const signatureBuffer = Buffer.from(signature, "hex"); | |
| if (signatureBuffer.length !== expectedSignature.length) { | |
| return false; | |
| } | |
| return crypto.timingSafeEqual(signatureBuffer, expectedSignature); | |
| }, |
WHY
Summary by CodeRabbit
New Features
Updates
@pipedream/httppackage to 0.5.0.