From 23e74d4d55945dabcabf36c9ff80a6a81b7a25bb Mon Sep 17 00:00:00 2001 From: rawdaymx Date: Sat, 19 Sep 2026 11:23:19 -0700 Subject: [PATCH 1/2] fix(comment-automation): validate flowId exists before saving a reply MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit None of CommentAutomation's 8 write methods (Messenger, Instagram, Threads, TikTok create/update) validated that privateReply.value or publicReply.value pointed at a real flow when their reply type is "flow" — AutomatedResponse (Keywords) already does this via flowService.exists before saving. A stale or mistyped flowId saves silently and the automation looks active in the UI, but every trigger fails at delivery time with a bare "FlowVersion not found" (worker's detectFlowVersion, apps/worker/src/lib/db.ts) — never surfaced back to the user who configured it. detectFlowVersion scopes its lookup to the triggering conversation's own workspaceId in all cases, so a foreign flowId can never execute cross-tenant — this is a validation gap, not a tenant-isolation defect. Adds assertFlowReplyExists, mirroring the check Keywords already runs, wired into all 8 write methods for privateReply and (where the channel lets the caller set it) publicReply. --- .../comment-automation-write-methods.test.ts | 162 ++++++++++++++++++ .../src/comment-automation/service.ts | 99 ++++++++++- 2 files changed, 260 insertions(+), 1 deletion(-) diff --git a/packages/business/__tests__/comment-automation-write-methods.test.ts b/packages/business/__tests__/comment-automation-write-methods.test.ts index 1b4dc24ce..64ea54353 100644 --- a/packages/business/__tests__/comment-automation-write-methods.test.ts +++ b/packages/business/__tests__/comment-automation-write-methods.test.ts @@ -8,6 +8,7 @@ const mocks = vi.hoisted(() => ({ update: vi.fn(), delete: vi.fn(), assertDeletable: vi.fn(), + flowExists: vi.fn(), })) vi.mock("@chatbotx.io/database/client", () => ({ @@ -66,6 +67,10 @@ vi.mock("../src/template/installed-resource.service", () => ({ assertDeletable: mocks.assertDeletable, })) +vi.mock("../src/flow/service", () => ({ + flowService: { exists: mocks.flowExists }, +})) + const { commentAutomationService } = await import( "../src/comment-automation/service" ) @@ -74,6 +79,7 @@ beforeEach(() => { vi.clearAllMocks() mocks.findFirst.mockResolvedValue(undefined) mocks.assertDeletable.mockResolvedValue(undefined) + mocks.flowExists.mockResolvedValue(true) }) describe("commentAutomationService — type-scoped writes", () => { @@ -223,4 +229,160 @@ describe("commentAutomationService — type-scoped writes", () => { }), ) }) + + test("createMessenger rejects a privateReply flow that does not exist in this workspace", async () => { + mocks.flowExists.mockResolvedValue(false) + + await expect( + commentAutomationService.createMessenger({ + workspaceId: "1", + data: { + name: "hello", + privateReply: { type: "flow", value: "flow-from-another-space" }, + }, + }), + ).rejects.toMatchObject({ field: "privateReply" }) + + expect(mocks.flowExists).toHaveBeenCalledWith( + "1", + "flow-from-another-space", + undefined, + ) + expect(mocks.insert).not.toHaveBeenCalled() + }) + + test("createMessenger rejects a publicReply flow that does not exist in this workspace", async () => { + mocks.flowExists.mockResolvedValue(false) + + await expect( + commentAutomationService.createMessenger({ + workspaceId: "1", + data: { + name: "hello", + publicReply: { type: "flow", value: "flow-from-another-space" }, + }, + }), + ).rejects.toMatchObject({ field: "publicReply" }) + + expect(mocks.insert).not.toHaveBeenCalled() + }) + + test("createMessenger inserts when both reply flows exist in this workspace", async () => { + mocks.flowExists.mockResolvedValue(true) + const returning = vi.fn().mockResolvedValue([{ id: "id-1" }]) + const values = vi.fn(() => ({ returning })) + mocks.insert.mockReturnValue({ values }) + + await commentAutomationService.createMessenger({ + workspaceId: "1", + data: { + name: "hello", + privateReply: { type: "flow", value: "flow-1" }, + }, + }) + + expect(mocks.flowExists).toHaveBeenCalledWith("1", "flow-1", undefined) + expect(values).toHaveBeenCalled() + }) + + test("createMessenger never calls flowService when neither reply is a flow", async () => { + const returning = vi.fn().mockResolvedValue([{ id: "id-1" }]) + const values = vi.fn(() => ({ returning })) + mocks.insert.mockReturnValue({ values }) + + await commentAutomationService.createMessenger({ + workspaceId: "1", + data: { name: "hello", privateReply: { type: "text", value: "hi" } }, + }) + + expect(mocks.flowExists).not.toHaveBeenCalled() + }) + + test("updateInstagram rejects a privateReply flow from another workspace", async () => { + mocks.findFirst.mockResolvedValue({ id: "9", type: "instagram" }) + mocks.flowExists.mockResolvedValue(false) + + await expect( + commentAutomationService.updateInstagram( + { workspaceId: "1", id: "9" }, + { privateReply: { type: "flow", value: "flow-from-another-space" } }, + ), + ).rejects.toMatchObject({ field: "privateReply" }) + + expect(mocks.update).not.toHaveBeenCalled() + }) + + test("createThreadsAutomation rejects a publicReply flow from another workspace", async () => { + mocks.flowExists.mockResolvedValue(false) + + await expect( + commentAutomationService.createThreadsAutomation({ + workspaceId: "1", + data: { + name: "hello", + post: { type: "all", value: [] }, + publicReply: { type: "flow", value: "flow-from-another-space" }, + includeKeywords: { type: "all", value: [] }, + excludeKeywords: [], + options: { + replyToNewContactsOnly: false, + replyOncePerUserPerPost: false, + replyToUsersWhoCommentedOnOtherPosts: true, + ignoreCommentReplies: true, + }, + replyAfter: { type: "immediately", value: 0 }, + }, + }), + ).rejects.toMatchObject({ field: "publicReply" }) + + expect(mocks.flowExists).toHaveBeenCalledWith( + "1", + "flow-from-another-space", + expect.anything(), + ) + expect(mocks.insert).not.toHaveBeenCalled() + }) + + test("updateThreadsAutomation rejects a publicReply flow from another workspace", async () => { + mocks.flowExists.mockResolvedValue(false) + + await expect( + commentAutomationService.updateThreadsAutomation({ + workspaceId: "1", + id: "9", + data: { + publicReply: { type: "flow", value: "flow-from-another-space" }, + }, + }), + ).rejects.toMatchObject({ field: "publicReply" }) + + expect(mocks.update).not.toHaveBeenCalled() + }) + + test("createTiktokAutomation rejects a publicReply flow from another workspace", async () => { + mocks.flowExists.mockResolvedValue(false) + + await expect( + commentAutomationService.createTiktokAutomation({ + workspaceId: "1", + data: { + name: "hello", + post: { type: "all", value: [] }, + publicReply: { type: "flow", value: "flow-from-another-space" }, + includeKeywords: { type: "all", value: [] }, + excludeKeywords: [], + options: { + replyToNewContactsOnly: false, + replyOncePerUserPerPost: false, + likeUserComment: false, + replyToUsersWhoCommentedOnOtherPosts: true, + ignoreCommentReplies: true, + }, + replyAfter: { type: "immediately", value: 0 }, + }, + }), + ).rejects.toMatchObject({ field: "publicReply" }) + + expect(mocks.insert).not.toHaveBeenCalled() + }) }) diff --git a/packages/business/src/comment-automation/service.ts b/packages/business/src/comment-automation/service.ts index 81f5ae6cb..147b2ab8f 100644 --- a/packages/business/src/comment-automation/service.ts +++ b/packages/business/src/comment-automation/service.ts @@ -30,7 +30,8 @@ import { import { createId } from "@chatbotx.io/utils" import { formatInTimeZone } from "date-fns-tz" import { BaseService } from "../base.service" -import { notFoundException } from "../errors" +import { notFoundException, validationException } from "../errors" +import { flowService } from "../flow/service" import { resolveFolderIdFilter } from "../lib/folder-filter" import { assertDeletable } from "../template/installed-resource.service" @@ -512,10 +513,52 @@ class CommentAutomationService extends BaseService { return { ...data, publicReply: normalizeReplyTexts(data.publicReply) } } + /** + * `AutomatedResponse` (Keywords) validates a `flowId` against + * `flowService.exists` before saving it (`automated-response/service.ts`); + * this table never did, for either reply field or any of its 8 write + * methods. The worker itself scopes the lookup to the triggering + * conversation's own workspace (`detectFlowVersion`, + * `apps/worker/src/lib/db.ts`) for BOTH `privateReply` + * (`private-reply.ts`) and `publicReply` (`public-reply.ts`) — a foreign + * flowId can never run cross-tenant, it just fails at delivery time with a + * bare `FlowVersion not found`, on an automation that otherwise looks + * active. Catching it at write time turns a silent dead automation into an + * immediate, actionable error. Structurally typed so it accepts every + * reply shape this table's channels use — Messenger/Instagram's + * `CommentReply`, and Threads/TikTok's channel-specific reply type (both + * fix `privateReply` to `{type: "none"}` and never let the caller set it, + * so only their `publicReply` needs covering). + */ + private async assertFlowReplyExists( + workspaceId: string, + field: "privateReply" | "publicReply", + reply: { type: string; value: string | null } | null | undefined, + tx?: DatabaseClient, + ): Promise { + if (reply?.type !== "flow" || !reply.value) { + return + } + const exists = await flowService.exists(workspaceId, reply.value, tx) + if (!exists) { + throw validationException(field, "Flow not found") + } + } + async createMessenger(input: { workspaceId: string data: FbCommentAutomationWriteData }): Promise { + await this.assertFlowReplyExists( + input.workspaceId, + "privateReply", + input.data.privateReply, + ) + await this.assertFlowReplyExists( + input.workspaceId, + "publicReply", + input.data.publicReply, + ) const [created] = await db .insert(commentAutomationModel) .values({ @@ -533,6 +576,16 @@ class CommentAutomationService extends BaseService { data: Partial, ): Promise { await this.findMessengerOrFail(ctx) + await this.assertFlowReplyExists( + ctx.workspaceId, + "privateReply", + data.privateReply, + ) + await this.assertFlowReplyExists( + ctx.workspaceId, + "publicReply", + data.publicReply, + ) const [updated] = await db .update(commentAutomationModel) @@ -619,6 +672,16 @@ class CommentAutomationService extends BaseService { type: IgCommentAutomationType data: FbCommentAutomationWriteData }): Promise { + await this.assertFlowReplyExists( + input.workspaceId, + "privateReply", + input.data.privateReply, + ) + await this.assertFlowReplyExists( + input.workspaceId, + "publicReply", + input.data.publicReply, + ) const [created] = await db .insert(commentAutomationModel) .values({ @@ -636,6 +699,16 @@ class CommentAutomationService extends BaseService { data: Partial, ): Promise { await this.findInstagramOrFail(ctx) + await this.assertFlowReplyExists( + ctx.workspaceId, + "privateReply", + data.privateReply, + ) + await this.assertFlowReplyExists( + ctx.workspaceId, + "publicReply", + data.publicReply, + ) const [updated] = await db .update(commentAutomationModel) @@ -735,6 +808,12 @@ class CommentAutomationService extends BaseService { tx?: DatabaseClient }) { const { workspaceId, data, tx = db } = props + await this.assertFlowReplyExists( + workspaceId, + "publicReply", + data.publicReply, + tx, + ) const [record] = await tx .insert(commentAutomationModel) .values({ @@ -764,6 +843,12 @@ class CommentAutomationService extends BaseService { tx?: DatabaseClient }) { const { workspaceId, id, data, tx = db } = props + await this.assertFlowReplyExists( + workspaceId, + "publicReply", + data.publicReply, + tx, + ) const values: Record = {} if (data.name !== undefined) { @@ -876,6 +961,12 @@ class CommentAutomationService extends BaseService { tx?: DatabaseClient }) { const { workspaceId, data, tx = db } = props + await this.assertFlowReplyExists( + workspaceId, + "publicReply", + data.publicReply, + tx, + ) const [record] = await tx .insert(commentAutomationModel) .values({ @@ -905,6 +996,12 @@ class CommentAutomationService extends BaseService { tx?: DatabaseClient }) { const { workspaceId, id, data, tx = db } = props + await this.assertFlowReplyExists( + workspaceId, + "publicReply", + data.publicReply, + tx, + ) const values: Record = {} if (data.name !== undefined) { From 6319a1825cdcc86a81041c3150145ed166363231 Mon Sep 17 00:00:00 2001 From: rawdaymx Date: Sat, 19 Sep 2026 11:56:36 -0700 Subject: [PATCH 2/2] test(comment-automation): stub flowService in pre-existing CRUD suites service.ts now calls flowService.exists() before saving a flow reply (previous commit). Importing the real flowService transitively pulls in botFieldService -> the full contact-filter query builder, which these two suites fully replace @chatbotx.io/database/partials and @chatbotx.io/database/schema without, so the real chain broke both with "No X export is defined on the mock". Neither suite is testing flow validation, so flowService.exists is stubbed to always resolve true instead of chasing the transitive mock surface deeper. --- .../business/__tests__/comment-automation.list.test.ts | 7 +++++++ .../business/__tests__/comment-automation.service.test.ts | 7 +++++++ 2 files changed, 14 insertions(+) diff --git a/packages/business/__tests__/comment-automation.list.test.ts b/packages/business/__tests__/comment-automation.list.test.ts index f4a821ea4..056e78853 100644 --- a/packages/business/__tests__/comment-automation.list.test.ts +++ b/packages/business/__tests__/comment-automation.list.test.ts @@ -30,6 +30,13 @@ vi.mock("@chatbotx.io/database/partials", () => ({ rootFolderId: "0", })) +// service.ts now calls flowService.exists() to validate a flow reply; this +// suite isn't about that check, so stub it to always resolve true and avoid +// pulling in flowService's own (unrelated) dependency chain. +vi.mock("../src/flow/service", () => ({ + flowService: { exists: vi.fn().mockResolvedValue(true) }, +})) + vi.mock("@chatbotx.io/database/schema", () => ({ contactInboxModel: {}, commentAutomationModel: { name: "commentAutomation.name" }, diff --git a/packages/business/__tests__/comment-automation.service.test.ts b/packages/business/__tests__/comment-automation.service.test.ts index 71b375a33..c59e69d16 100644 --- a/packages/business/__tests__/comment-automation.service.test.ts +++ b/packages/business/__tests__/comment-automation.service.test.ts @@ -51,6 +51,13 @@ vi.mock("@chatbotx.io/database/partials", () => ({ }, })) +// service.ts now calls flowService.exists() to validate a flow reply; this +// suite isn't about that check, so stub it to always resolve true and avoid +// pulling in flowService's own (unrelated) dependency chain. +vi.mock("../src/flow/service", () => ({ + flowService: { exists: vi.fn().mockResolvedValue(true) }, +})) + vi.mock("@chatbotx.io/database/schema", () => ({ contactInboxModel: { contactId: "ContactInbox.contactId" }, commentAutomationModel: {