From daba925fc8a3dfcaff8df6b53f78feb151001bb9 Mon Sep 17 00:00:00 2001 From: Chris Farhood Date: Mon, 4 May 2026 03:06:57 +0000 Subject: [PATCH] fix(GRO-982): address all QA blocking failures MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - #7: Extract validateTelnyxSignature in sms.ts as standalone exported fn, reuse in TelnyxProvider.validateWebhookSignature and telnyx.ts route - #1: Replace uuid v4 import with crypto.randomUUID() (built-in, no dep) - #2: Remove updatedAt from messages update in handleMessageFinalized (no such column exists) - #3: Fix test import path ../../ → ../../../ for telnyx route import - #4: validateTelnyxSignature accepts string | undefined | null to match Hono c.req.header() return type - #5&6: Add null guards for .returning() results in findOrCreateConversation and upsertMessage - #8: Remove dead buildFindOrCreateConversationParams function - #9: Remove unused imports (messageDirectionEnum, messageStatusEnum, resolveBusinessIdByMessagingNumber in test) - #10: Wrap upsertMessage insert in try/catch; unique violation returns {isNew: false} instead of crashing - #11: Add EOF newlines to all modified files Co-Authored-By: Paperclip --- apps/api/src/routes/webhooks/telnyx.ts | 31 +--------- .../messaging/__tests__/inbound.test.ts | 11 ++-- apps/api/src/services/messaging/inbound.ts | 59 ++++++++++--------- apps/api/src/services/sms.ts | 57 +++++++++--------- 4 files changed, 71 insertions(+), 87 deletions(-) diff --git a/apps/api/src/routes/webhooks/telnyx.ts b/apps/api/src/routes/webhooks/telnyx.ts index 723663d..369461d 100644 --- a/apps/api/src/routes/webhooks/telnyx.ts +++ b/apps/api/src/routes/webhooks/telnyx.ts @@ -1,40 +1,13 @@ import { Hono } from "hono"; -import { createHmac } from "crypto"; +import { validateTelnyxSignature } from "../../services/sms.js"; import { handleMessageReceived, handleMessageFinalized, - resolveBusinessIdByMessagingNumber, TelnyxMessageReceivedPayload, } from "../../services/messaging/inbound.js"; export const telnyxWebhooksRouter = new Hono(); -function validateTelnyxSignature(rawBody: string, signature: string | null): boolean { - if (!signature) return false; - const secret = process.env.TELNYX_WEBHOOK_SECRET; - if (!secret) return false; - - try { - const hmac = createHmac("sha256", secret); - const expected = `sha256=${hmac.update(rawBody).digest("hex")}`; - - const sigBuf = Buffer.from(signature); - const expBuf = Buffer.from(expected); - - if (sigBuf.length !== expBuf.length) return false; - - let diff = 0; - for (let i = 0; i < sigBuf.length; i++) { - const sigByte = sigBuf[i] ?? 0; - const expByte = expBuf[i] ?? 0; - diff |= sigByte ^ expByte; - } - return diff === 0; - } catch { - return false; - } -} - telnyxWebhooksRouter.post("/messaging", async (c) => { const signature = c.req.header("telnyx-signature"); @@ -83,4 +56,4 @@ telnyxWebhooksRouter.post("/messaging", async (c) => { } return c.json({ received: true }); -}); \ No newline at end of file +}); diff --git a/apps/api/src/services/messaging/__tests__/inbound.test.ts b/apps/api/src/services/messaging/__tests__/inbound.test.ts index aab629d..482545d 100644 --- a/apps/api/src/services/messaging/__tests__/inbound.test.ts +++ b/apps/api/src/services/messaging/__tests__/inbound.test.ts @@ -2,7 +2,6 @@ import { describe, it, expect, vi, beforeEach } from "vitest"; import { findOrCreateConversation, upsertMessage, - resolveBusinessIdByMessagingNumber, handleMessageReceived, handleMessageFinalized, TelnyxMessageReceivedPayload, @@ -53,8 +52,12 @@ const makePayload = ( }); describe("signature validation via route", () => { + beforeEach(() => { + vi.resetModules(); + }); + it("returns 401 when telnyx-signature header is missing", async () => { - const { telnyxWebhooksRouter } = await import("../../routes/webhooks/telnyx.js"); + const { telnyxWebhooksRouter } = await import("../../../routes/webhooks/telnyx.js"); const payload = JSON.stringify(makePayload("message.received", "msg-123", "+1555111", "+1555222")); const req = new Request("http://localhost/api/webhooks/telnyx/messaging", { method: "POST", @@ -67,7 +70,7 @@ describe("signature validation via route", () => { it("returns 401 when signature does not match", async () => { process.env.TELNYX_WEBHOOK_SECRET = "test-secret"; - const { telnyxWebhooksRouter } = await import("../../routes/webhooks/telnyx.js"); + const { telnyxWebhooksRouter } = await import("../../../routes/webhooks/telnyx.js"); const payload = JSON.stringify(makePayload("message.received", "msg-123", "+1555111", "+1555222")); const req = new Request("http://localhost/api/webhooks/telnyx/messaging", { method: "POST", @@ -273,4 +276,4 @@ describe("handleMessageFinalized", () => { const result = await handleMessageFinalized(payload); expect(result?.newStatus).toBe("delivered"); }); -}); \ No newline at end of file +}); diff --git a/apps/api/src/services/messaging/inbound.ts b/apps/api/src/services/messaging/inbound.ts index d6d683e..f64a02a 100644 --- a/apps/api/src/services/messaging/inbound.ts +++ b/apps/api/src/services/messaging/inbound.ts @@ -1,6 +1,4 @@ import { getDb, conversations, messages, businessSettings, eq, and, sql } from "@groombook/db"; -import { messageDirectionEnum, messageStatusEnum } from "@groombook/db"; -import { v4 as uuidv4 } from "uuid"; export interface TelnyxMessageReceivedPayload { data: { @@ -20,14 +18,6 @@ export interface TelnyxMessageReceivedPayload { }; } -function buildFindOrCreateConversationParams(businessId: string, clientPhone: string, businessNumber: string) { - return { - businessId, - externalNumber: clientPhone, - businessNumber, - }; -} - export async function findOrCreateConversation( businessId: string, clientPhone: string, @@ -57,12 +47,12 @@ export async function findOrCreateConversation( .where(eq(businessSettings.id, businessId)) .limit(1); - const clientId = business?.primaryClientId ?? uuidv4(); + const clientId = business?.primaryClientId ?? crypto.randomUUID(); const [created] = await db .insert(conversations) .values({ - id: uuidv4(), + id: crypto.randomUUID(), businessId, clientId, channel: "sms", @@ -73,6 +63,8 @@ export async function findOrCreateConversation( }) .returning({ id: conversations.id, clientId: conversations.clientId }); + if (!created) throw new Error("Failed to create conversation"); + return { id: created.id, clientId: created.clientId }; } @@ -96,20 +88,33 @@ export async function upsertMessage( return { id: existing.id, isNew: false }; } - const [inserted] = await db - .insert(messages) - .values({ - id: uuidv4(), - conversationId, - direction, - body, - status, - providerMessageId, - sentByStaffId: sentByStaffId ?? null, - }) - .returning({ id: messages.id }); + try { + const [inserted] = await db + .insert(messages) + .values({ + id: crypto.randomUUID(), + conversationId, + direction, + body, + status, + providerMessageId, + sentByStaffId: sentByStaffId ?? null, + }) + .returning({ id: messages.id }); - return { id: inserted.id, isNew: true }; + if (!inserted) throw new Error("Failed to insert message"); + return { id: inserted.id, isNew: true }; + } catch (err) { + if (err instanceof Error && err.message.includes("unique")) { + const [existing] = await db + .select({ id: messages.id }) + .from(messages) + .where(eq(messages.providerMessageId, providerMessageId)) + .limit(1); + if (existing) return { id: existing.id, isNew: false }; + } + throw err; + } } export async function resolveBusinessIdByMessagingNumber(toNumber: string): Promise { @@ -179,9 +184,9 @@ export async function handleMessageFinalized(payload: TelnyxMessageReceivedPaylo if (newStatus !== existing.status) { await db .update(messages) - .set({ status: newStatus, deliveredAt: new Date(), updatedAt: new Date() }) + .set({ status: newStatus, deliveredAt: new Date() }) .where(eq(messages.id, existing.id)); } return { messageId: existing.id, newStatus }; -} \ No newline at end of file +} diff --git a/apps/api/src/services/sms.ts b/apps/api/src/services/sms.ts index 5be4009..209e5e2 100644 --- a/apps/api/src/services/sms.ts +++ b/apps/api/src/services/sms.ts @@ -32,6 +32,35 @@ function isE164(phone: string): boolean { return /^\+[1-9]\d{7,14}$/.test(phone); } +export function validateTelnyxSignature( + rawBody: string, + signature: string | undefined | null +): boolean { + if (!signature) return false; + const secret = process.env.TELNYX_WEBHOOK_SECRET; + if (!secret) return false; + + try { + const hmac = createHmac("sha256", secret); + const expected = `sha256=${hmac.update(rawBody).digest("hex")}`; + + const sigBuf = Buffer.from(signature); + const expBuf = Buffer.from(expected); + + if (sigBuf.length !== expBuf.length) return false; + + let diff = 0; + for (let i = 0; i < sigBuf.length; i++) { + const sigByte = sigBuf[i] ?? 0; + const expByte = expBuf[i] ?? 0; + diff |= sigByte ^ expByte; + } + return diff === 0; + } catch { + return false; + } +} + export async function sendSms( to: string, body: string, @@ -74,33 +103,7 @@ export class TelnyxProvider implements SmsProvider { } validateWebhookSignature(req: Request): boolean { - const secret = process.env.TELNYX_WEBHOOK_SECRET; - if (!secret) return false; - - const signature = req.headers.get("telnyx-signature"); - if (!signature) return false; - - const payload = JSON.stringify(req.body); - - try { - const hmac = createHmac("sha256", secret); - const expected = `sha256=${hmac.update(payload).digest("hex")}`; - - const sigBuf = Buffer.from(signature); - const expBuf = Buffer.from(expected); - - if (sigBuf.length !== expBuf.length) return false; - - let diff = 0; - for (let i = 0; i < sigBuf.length; i++) { - const sigByte = sigBuf[i] ?? 0; - const expByte = expBuf[i] ?? 0; - diff |= sigByte ^ expByte; - } - return diff === 0; - } catch { - return false; - } + return validateTelnyxSignature(JSON.stringify(req.body), req.headers.get("telnyx-signature")); } }