-
Notifications
You must be signed in to change notification settings - Fork 192
security(webhook): add HMAC signature verification for Sendblue webhooks #25
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
1ed57c4
d80f530
810e86a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,5 @@ | ||
| import express from "express"; | ||
| import { createHmac, timingSafeEqual } from "node:crypto"; | ||
| import { api } from "../convex/_generated/api.js"; | ||
| import { convex } from "./convex-client.js"; | ||
| import { handleUserMessage } from "./interaction-agent.js"; | ||
|
|
@@ -8,6 +9,141 @@ import { validateImageHeader, MAX_IMAGE_BYTES, type ImageMediaType } from "./ima | |
| const API_BASE = "https://api.sendblue.com/api"; | ||
| const MAX_CHUNK = 2900; | ||
|
|
||
| // NOTE: As of 2026-04, Sendblue echoes the raw secret in `sb-signing-secret` | ||
| // rather than sending an HMAC digest. The HMAC verification path below exists | ||
| // for forward-compatibility if Sendblue (or a replacement provider) adds | ||
| // proper HMAC-SHA256 payload signing in the future. | ||
| const SIGNATURE_HEADERS = [ | ||
| "x-sendblue-signature", | ||
| "signature", | ||
| "x-webhook-signature", | ||
| ]; | ||
|
|
||
| // Shared-secret carriers. Sendblue currently puts the raw signing secret in | ||
| // `sb-signing-secret`; the generic alternate covers proxied upstreams that | ||
| // rename the header. Use only on trusted transport (TLS). | ||
| const SHARED_SECRET_HEADERS = [ | ||
| "sb-signing-secret", | ||
| "x-webhook-secret", | ||
| ]; | ||
|
|
||
| function bufferEquals(a: Buffer, b: Buffer): boolean { | ||
| if (a.length !== b.length) return false; | ||
| return timingSafeEqual(a, b); | ||
| } | ||
|
|
||
| function decodeSignature(value: string): Buffer | null { | ||
| const trimmed = value.trim().replace(/^sha256=/i, ""); | ||
| // Hex-encoded: 64 hex chars = 32 bytes | ||
| if (/^[0-9a-f]+$/i.test(trimmed) && trimmed.length === 64) { | ||
| return Buffer.from(trimmed, "hex"); | ||
| } | ||
| // Base64-encoded: decode and verify exactly 32 bytes | ||
| if (/^[A-Za-z0-9+/=_-]+$/.test(trimmed)) { | ||
| const normalized = trimmed.replace(/-/g, "+").replace(/_/g, "/"); | ||
| const decoded = Buffer.from(normalized, "base64"); | ||
| if (decoded.length === 32) return decoded; | ||
| return null; | ||
| } | ||
|
|
||
| return null; | ||
| } | ||
|
|
||
| function verifyHmac(rawBody: Buffer, secret: string, signatureHeader: string): boolean { | ||
| const expected = createHmac("sha256", secret).update(rawBody).digest(); | ||
| const provided = decodeSignature(signatureHeader); | ||
| if (!provided) return false; | ||
| return bufferEquals(expected, provided); | ||
| } | ||
|
|
||
| function verifySharedSecret(provided: string, secret: string): boolean { | ||
| // HMAC both sides so the comparison is always fixed-length (32 bytes) and | ||
| // does not leak the real secret's length via an early length-mismatch | ||
| // return path. | ||
| const a = createHmac("sha256", "webhook-verify").update(provided).digest(); | ||
| const b = createHmac("sha256", "webhook-verify").update(secret).digest(); | ||
| return timingSafeEqual(a, b); | ||
| } | ||
|
|
||
| function clientIp(req: express.Request): string { | ||
| // Use req.ip which respects Express's `trust proxy` setting. | ||
| // When trust proxy is not configured, req.ip returns the socket address | ||
| // and ignores X-Forwarded-For, preventing log spoofing. | ||
| return req.ip ?? req.socket.remoteAddress ?? "unknown"; | ||
| } | ||
|
greptile-apps[bot] marked this conversation as resolved.
|
||
|
|
||
| function verifyWebhookRequest( | ||
| req: express.Request, | ||
| ): express.RequestHandler | true { | ||
| const secret = process.env.SENDBLUE_SIGNING_SECRET; | ||
| if (!secret) { | ||
| // Graceful degradation — startup already warned. Allow through. | ||
| return true; | ||
| } | ||
|
|
||
| const rawBody = (req as express.Request & { rawBody?: Buffer }).rawBody; | ||
|
|
||
| for (const name of SIGNATURE_HEADERS) { | ||
| const header = req.headers[name]; | ||
| const value = Array.isArray(header) ? header[0] : header; | ||
| if (typeof value === "string" && value.length > 0) { | ||
| if (!rawBody) { | ||
| // HMAC header is present but no raw body to verify against — refuse. | ||
| return (_req, res) => { | ||
| res.status(400).json({ error: "bad request" }); | ||
| }; | ||
| } | ||
| if (verifyHmac(rawBody, secret, value)) return true; | ||
| const ip = clientIp(req); | ||
| console.warn( | ||
| `[security] sendblue webhook signature verification FAILED (header=${name}, ip=${ip})`, | ||
| ); | ||
| return (_req, res) => { | ||
| res.status(401).json({ error: "unauthorized" }); | ||
| }; | ||
| } | ||
| } | ||
|
|
||
| // Shared-secret path. Sendblue's actual delivery uses `sb-signing-secret`; | ||
| // we also accept a generic alternate for proxied upstreams. Header-only — | ||
| // query parameters leak into access logs, proxy logs, and ngrok inspection. | ||
| for (const name of SHARED_SECRET_HEADERS) { | ||
| const header = req.headers[name]; | ||
| const value = Array.isArray(header) ? header[0] : header; | ||
| if (typeof value === "string" && value.length > 0) { | ||
| if (verifySharedSecret(value, secret)) return true; | ||
| const ip = clientIp(req); | ||
| console.warn( | ||
| `[security] sendblue webhook shared-secret verification FAILED (header=${name}, ip=${ip})`, | ||
| ); | ||
| return (_req, res) => { | ||
| res.status(401).json({ error: "unauthorized" }); | ||
| }; | ||
| } | ||
| } | ||
|
|
||
| const ip = clientIp(req); | ||
| console.warn( | ||
| `[security] sendblue webhook missing signature (ip=${ip})`, | ||
| ); | ||
| return (_req, res) => { | ||
| res.status(401).json({ error: "unauthorized" }); | ||
| }; | ||
| } | ||
|
|
||
| function sendblueWebhookAuth( | ||
| req: express.Request, | ||
| res: express.Response, | ||
| next: express.NextFunction, | ||
| ): void { | ||
| const result = verifyWebhookRequest(req); | ||
| if (result === true) { | ||
| next(); | ||
| return; | ||
| } | ||
| result(req, res, next); | ||
| } | ||
|
|
||
| function stripMarkdown(text: string): string { | ||
| return text | ||
| .replace(/```[\s\S]*?```/g, (m) => m.replace(/```\w*\n?|```/g, "")) | ||
|
|
@@ -202,6 +338,9 @@ export async function ingestSendblueImage( | |
| export function createSendblueRouter(): express.Router { | ||
| const router = express.Router(); | ||
|
|
||
| router.post("/webhook", sendblueWebhookAuth, async (req, res) => { | ||
| const { content, from_number, is_outbound, message_handle } = req.body ?? {}; | ||
| if (is_outbound || !content || !from_number) { | ||
|
Comment on lines
+341
to
+343
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
The three added lines open a new The fix is to replace the old handler wholesale: add |
||
| router.post("/webhook", async (req, res) => { | ||
| const { content, from_number, is_outbound, message_handle, media_url, media_urls } = | ||
| req.body ?? {}; | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
express.jsonplaced before Composio'sexpress.raw, breaking Composio HMACThe new
express.jsonmiddleware (lines 58-66) is mounted before the Composio-specificexpress.rawat line 77. Express body-parsers setreq._body = trueon first parse and skip on all subsequent parsers, so the stream is fully consumed at line 58. When the Composio route handler runs,Buffer.isBuffer(req.body)atcomposio-routes.ts:190returnsfalse(the body is now a JS object, not a Buffer),rawBodybecomes"", and every Composio webhook request fails HMAC verification with a rejected payload.The fix is to add the
verifyhook to the existingexpress.jsoncall at line 78 (after the Composio raw handler) rather than inserting a second, globally-ordered one before it.