Skip to content

Commit 0f6dc71

Browse files
authored
Merge pull request #231 from onuibeblessing2019-hash/fix/webhook-signature-verification-24
fix(payments): verify HMAC signature on delivery-confirmation webhook
2 parents 0d392c5 + 8a3b682 commit 0f6dc71

3 files changed

Lines changed: 281 additions & 1 deletion

File tree

Lines changed: 241 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,241 @@
1+
import { createHmac } from "node:crypto";
2+
import { EventEmitter } from "node:events";
3+
import type { IncomingMessage, ServerResponse } from "node:http";
4+
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
5+
import { handleDeliveryConfirmationWebhook } from "../escrow/autoSettlement.js";
6+
import { registerRoutes } from "./routes.js";
7+
import type { Route } from "@delegolabs/utils";
8+
9+
// Issue #24/#445 — POST /webhooks/delivery-confirmation accepted delivery
10+
// confirmations with no signature verification at all: anyone who could
11+
// reach it could forge a webhook and trigger escrow release. These tests
12+
// cover the HMAC verification added to close that gap.
13+
14+
vi.mock("../escrow/autoSettlement.js", async () => {
15+
const actual = await vi.importActual<typeof import("../escrow/autoSettlement.js")>(
16+
"../escrow/autoSettlement.js"
17+
);
18+
return {
19+
...actual,
20+
handleDeliveryConfirmationWebhook: vi.fn(),
21+
};
22+
});
23+
24+
const SECRET = "test-webhook-secret";
25+
26+
type MockResponse = ServerResponse & { statusCode: number; body: string };
27+
28+
function createMockReq(body: string, headers: Record<string, string> = {}): IncomingMessage {
29+
const req = new EventEmitter() as unknown as IncomingMessage;
30+
req.headers = { "content-type": "application/json", ...headers };
31+
process.nextTick(() => {
32+
req.emit("data", Buffer.from(body));
33+
req.emit("end");
34+
});
35+
return req;
36+
}
37+
38+
function createMockRes(): MockResponse {
39+
const res = {
40+
statusCode: 0,
41+
body: "",
42+
writeHead(status: number) {
43+
this.statusCode = status;
44+
},
45+
end(body?: string) {
46+
if (body !== undefined) this.body = body;
47+
},
48+
};
49+
return res as MockResponse;
50+
}
51+
52+
function sign(body: string, secret = SECRET): string {
53+
return createHmac("sha256", secret).update(body, "utf8").digest("hex");
54+
}
55+
56+
function findRoute(): Route {
57+
const route = registerRoutes().find(
58+
(r) => r.method === "POST" && r.pattern.test("/webhooks/delivery-confirmation")
59+
);
60+
if (!route) throw new Error("/webhooks/delivery-confirmation route not registered");
61+
return route;
62+
}
63+
64+
function payload(overrides: Record<string, unknown> = {}): string {
65+
return JSON.stringify({
66+
webhookId: "wh_1",
67+
orderId: "order-1",
68+
escrowId: "escrow-1",
69+
escrowContractId: "CCONTRACTID000000000000000000000000000000000000000000000000",
70+
callerAddress: "GCALLERADDRESS0000000000000000000000000000000000000000000",
71+
confirmedAt: new Date().toISOString(),
72+
...overrides,
73+
});
74+
}
75+
76+
describe("POST /webhooks/delivery-confirmation", () => {
77+
beforeEach(() => {
78+
process.env.ESCROW_WEBHOOK_SECRET = SECRET;
79+
vi.mocked(handleDeliveryConfirmationWebhook).mockReset();
80+
});
81+
82+
afterEach(() => {
83+
vi.restoreAllMocks();
84+
delete process.env.ESCROW_WEBHOOK_SECRET;
85+
});
86+
87+
it("returns 401 and never invokes the handler when the signature is invalid", async () => {
88+
const route = findRoute();
89+
const body = payload();
90+
const req = createMockReq(body, { "x-signature": "0".repeat(64) });
91+
const res = createMockRes();
92+
93+
await route.handler(req, res, {});
94+
95+
expect(res.statusCode).toBe(401);
96+
expect(JSON.parse(res.body).error.code).toBe("UNAUTHORIZED");
97+
expect(handleDeliveryConfirmationWebhook).not.toHaveBeenCalled();
98+
});
99+
100+
it("returns 401 and never invokes the handler when the signature header is missing", async () => {
101+
const route = findRoute();
102+
const body = payload();
103+
const req = createMockReq(body);
104+
const res = createMockRes();
105+
106+
await route.handler(req, res, {});
107+
108+
expect(res.statusCode).toBe(401);
109+
expect(handleDeliveryConfirmationWebhook).not.toHaveBeenCalled();
110+
});
111+
112+
it("returns 401 when the signature was computed over a different body (tampered payload)", async () => {
113+
const route = findRoute();
114+
const originalBody = payload();
115+
const tamperedBody = payload({ callerAddress: "GATTACKER00000000000000000000000000000000000000000000000" });
116+
// Signature is valid for originalBody, but the request carries tamperedBody.
117+
const req = createMockReq(tamperedBody, { "x-signature": sign(originalBody) });
118+
const res = createMockRes();
119+
120+
await route.handler(req, res, {});
121+
122+
expect(res.statusCode).toBe(401);
123+
expect(handleDeliveryConfirmationWebhook).not.toHaveBeenCalled();
124+
});
125+
126+
it("returns 503 and never invokes the handler when the webhook secret is not configured", async () => {
127+
delete process.env.ESCROW_WEBHOOK_SECRET;
128+
const route = findRoute();
129+
const body = payload();
130+
const req = createMockReq(body, { "x-signature": sign(body) });
131+
const res = createMockRes();
132+
133+
await route.handler(req, res, {});
134+
135+
expect(res.statusCode).toBe(503);
136+
expect(JSON.parse(res.body).error.code).toBe("CONFIG_ERROR");
137+
expect(handleDeliveryConfirmationWebhook).not.toHaveBeenCalled();
138+
});
139+
140+
it("processes a validly signed request and releases escrow", async () => {
141+
vi.mocked(handleDeliveryConfirmationWebhook).mockResolvedValue({
142+
webhookId: "wh_1",
143+
orderId: "order-1",
144+
escrowId: "escrow-1",
145+
status: "released",
146+
});
147+
148+
const route = findRoute();
149+
const body = payload();
150+
const req = createMockReq(body, { "x-signature": sign(body) });
151+
const res = createMockRes();
152+
153+
await route.handler(req, res, {});
154+
155+
expect(res.statusCode).toBe(200);
156+
const parsed = JSON.parse(res.body);
157+
expect(parsed.data.status).toBe("released");
158+
expect(handleDeliveryConfirmationWebhook).toHaveBeenCalledWith(
159+
expect.objectContaining({ webhookId: "wh_1", orderId: "order-1", escrowId: "escrow-1" })
160+
);
161+
});
162+
163+
it("accepts the X-Webhook-Signature header name as documented in the issue", async () => {
164+
vi.mocked(handleDeliveryConfirmationWebhook).mockResolvedValue({
165+
webhookId: "wh_1",
166+
orderId: "order-1",
167+
escrowId: "escrow-1",
168+
status: "released",
169+
});
170+
171+
const route = findRoute();
172+
const body = payload();
173+
const req = createMockReq(body, { "x-webhook-signature": sign(body) });
174+
const res = createMockRes();
175+
176+
await route.handler(req, res, {});
177+
178+
expect(res.statusCode).toBe(200);
179+
});
180+
181+
it("accepts the sha256=<hex> prefixed signature style", async () => {
182+
vi.mocked(handleDeliveryConfirmationWebhook).mockResolvedValue({
183+
webhookId: "wh_1",
184+
orderId: "order-1",
185+
escrowId: "escrow-1",
186+
status: "released",
187+
});
188+
189+
const route = findRoute();
190+
const body = payload();
191+
const req = createMockReq(body, { "x-hub-signature-256": `sha256=${sign(body)}` });
192+
const res = createMockRes();
193+
194+
await route.handler(req, res, {});
195+
196+
expect(res.statusCode).toBe(200);
197+
});
198+
199+
it("returns 502 when the handler reports a failed release", async () => {
200+
vi.mocked(handleDeliveryConfirmationWebhook).mockResolvedValue({
201+
webhookId: "wh_1",
202+
orderId: "order-1",
203+
escrowId: "escrow-1",
204+
status: "failed",
205+
reason: "Release transaction failed",
206+
});
207+
208+
const route = findRoute();
209+
const body = payload();
210+
const req = createMockReq(body, { "x-signature": sign(body) });
211+
const res = createMockRes();
212+
213+
await route.handler(req, res, {});
214+
215+
expect(res.statusCode).toBe(502);
216+
});
217+
218+
it("returns 400 when required fields are missing, after a valid signature", async () => {
219+
const route = findRoute();
220+
const body = JSON.stringify({ webhookId: "wh_1" });
221+
const req = createMockReq(body, { "x-signature": sign(body) });
222+
const res = createMockRes();
223+
224+
await route.handler(req, res, {});
225+
226+
expect(res.statusCode).toBe(400);
227+
expect(handleDeliveryConfirmationWebhook).not.toHaveBeenCalled();
228+
});
229+
230+
it("returns 400 for invalid JSON, after a valid signature over the raw bytes", async () => {
231+
const route = findRoute();
232+
const body = "{not valid json";
233+
const req = createMockReq(body, { "x-signature": sign(body) });
234+
const res = createMockRes();
235+
236+
await route.handler(req, res, {});
237+
238+
expect(res.statusCode).toBe(400);
239+
expect(handleDeliveryConfirmationWebhook).not.toHaveBeenCalled();
240+
});
241+
});

apps/backend/payments/src/routes.ts

Lines changed: 39 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -316,9 +316,47 @@ export function registerRoutes(): Route[] {
316316
}),
317317

318318
// Issue #363 — delivery-confirmation webhook auto-triggers escrow release.
319+
//
320+
// Issue #24/#445 — this endpoint accepted the webhook with no signature
321+
// verification at all: anyone who could reach it could forge a delivery
322+
// confirmation and trigger escrow release. Verified the same way as
323+
// /escrow/:escrowId/delivery-confirmed (issue #45) — HMAC-SHA256 over the
324+
// raw body, constant-time comparison, via hmac.ts — reusing that route's
325+
// ESCROW_WEBHOOK_SECRET since both endpoints sit in the same trust
326+
// domain (a delivery-confirmation webhook driving escrow release).
319327
route("POST", "/webhooks/delivery-confirmation", async (req, res) => {
320328
try {
321-
const body = await readJsonBody(req);
329+
const rawBody = await readRawBody(req);
330+
331+
const secret = getWebhookSecret();
332+
if (!secret) {
333+
json(res, 503, {
334+
data: null,
335+
error: { code: "CONFIG_ERROR", message: "ESCROW_WEBHOOK_SECRET is not configured" },
336+
});
337+
return;
338+
}
339+
340+
const signatureHeaderRaw =
341+
req.headers[WEBHOOK_SIGNATURE_HEADER] ?? req.headers["x-webhook-signature"] ?? req.headers["x-hub-signature-256"];
342+
const signatureHeader = Array.isArray(signatureHeaderRaw) ? signatureHeaderRaw[0] : signatureHeaderRaw;
343+
344+
if (!verifyWebhookSignature(rawBody, signatureHeader, secret)) {
345+
json(res, 401, {
346+
data: null,
347+
error: { code: "UNAUTHORIZED", message: "Invalid or missing webhook signature" },
348+
});
349+
return;
350+
}
351+
352+
let body: Record<string, unknown>;
353+
try {
354+
body = rawBody ? (JSON.parse(rawBody) as Record<string, unknown>) : {};
355+
} catch {
356+
sendValidationError(res, { code: "VALIDATION_ERROR", message: "Invalid JSON body" });
357+
return;
358+
}
359+
322360
const { webhookId, orderId, escrowId, escrowContractId, callerAddress, confirmedAt } = body;
323361

324362
if (

packages/utils/package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
"name": "@delegolabs/utils",
33
"version": "0.0.1",
44
"description": "Shared utility functions",
5+
"type": "module",
56
"main": "./dist/index.js",
67
"types": "./dist/index.d.ts",
78
"repository": {

0 commit comments

Comments
 (0)