Skip to content

Comment automations never validate a reply's flow id — Keywords does, comment triggers don't, so a bad reference silently dead-ends at delivery time #1228

Description

@rawdaymx

Summary

When a Keyword automation's reply is set to a flow, automated-response/service.ts validates
the flowId before saving: flowService.exists(workspaceId, flowId, tx)
(automated-response/service.ts:158-172), rejecting the write with a validationException if
that flow doesn't exist in the caller's own workspace. The comment-automation service
(packages/business/src/comment-automation/service.ts) has the exact same shape of field on BOTH
its reply fields — privateReply/publicReply: { type: "flow", value: <flowId> } — but none of
its eight write methods (Messenger/Instagram create+update, Threads create+update, TikTok
create+update) validate either one.

privateReply.type === "flow" is the "Private reply = Flow" feature our earlier report #1063
covered. publicReply.type === "flow" is a separate, equally real path: the worker's public-reply
handler (public-reply.ts) dispatches a flow the same way when that field is set to "flow", in
every channel that has one.

We checked whether this is a tenant-isolation problem (a foreign flow id actually running) before
writing this up — it is not (see Environment/Evidence below): the worker scopes flow lookup to
the triggering conversation's own workspace for both reply mechanisms, so a foreign or
nonexistent flow id just fails at send time with a bare FlowVersion not found, on an automation
that otherwise looks active and that the operator has no way to know is broken until a comment
tries to trigger it.

Environment

Static defect — read against upstream/main at commit 3871b5818, remeasured immediately before
filing this (it had moved from 62e9fe323, mid-investigation, via #1215 which renamed this
service's file and added a fifth channel with the same gap — accounted for below).

File Line(s)
packages/business/src/automated-response/service.ts 158-172 (the validation Keywords has)
packages/business/src/comment-automation/service.ts all 8 write methods
apps/worker/src/lib/db.ts 50-81 (detectFlowVersion, the workspace-scoped lookup)
apps/worker/src/integration/handlers/comment-automation/private-reply.ts privateReply dispatch
apps/worker/src/integration/handlers/comment-automation/public-reply.ts publicReply dispatch

How to see it

  1. automated-response/service.ts:158-172: if (flowId) { const exists = await flowService.exists(workspaceId, flowId, tx); if (!exists) throw validationException(...) }.
  2. comment-automation/service.ts: none of createMessenger, updateMessenger,
    createInstagram, updateInstagram, createThreadsAutomation, updateThreadsAutomation,
    createTiktokAutomation, updateTiktokAutomation read privateReply.type/.value or
    publicReply.type/.value before writing.
  3. Each write method inserts/updates the caller-supplied reply fields verbatim.
  4. On the read side, apps/worker/src/lib/db.ts:50 (detectFlowVersion) resolves the flow with
    workspaceId: conversation.workspaceId — the contact's workspace, not whatever workspace
    the comment automation happens to belong to — so a wrong flow id can never execute
    cross-tenant; it throws SdkException("FlowVersion not found").
  5. Both dispatch sites (private-reply.ts for privateReply, public-reply.ts for
    publicReply) funnel into the same sendFlow job → runFlowNodedetectFlowVersion path.

Expected (by analogy with Keywords): saving a comment automation with a "flow"-typed reply and
a flow id that doesn't exist in that workspace is rejected immediately with a clear validation
error.
Actual: the write succeeds unconditionally. The first sign anything is wrong is a queue-level
FlowVersion not found exception the next time a comment actually triggers it — nothing in the
builder UI flags the automation as broken before then.

Second run: confirming the worker can't be tricked into cross-tenant execution, for both reply fields

Read detectFlowVersion end to end: every path threads workspaceId: conversation.workspaceId
into flowVersionService.findByIdForWorkspace/flowService.findActiveById, both of which use
that workspaceId in their actual database WHERE clause — confirmed by reading each service
method, not assumed from the parameter name. conversation.workspaceId itself comes from a real
database row (detectConversationAndContactInbox), not anything a job payload could forge. We
also checked for a second flow-dispatch path that might skip this scoping: public-reply.ts
dispatches through the identical sendFlow/detectFlowVersion mechanism, so the same guarantee
holds there too. When upstream/main moved during this investigation (#1215, adding TikTok), we
re-diffed apps/worker/src/lib/db.ts and confirmed zero changes, and re-diffed both dispatch
handlers to confirm neither the sendFlow call nor the .type branch changed — only type renames
and TikTok-specific plumbing elsewhere in those files. This is what let us rule out a
tenant-isolation defect, twice, and report this as an ordinary validation gap instead.

Where it comes from

// packages/business/src/comment-automation/service.ts (createMessenger, unabridged apart from formatting)
async createMessenger(input: {
  workspaceId: string
  data: FbCommentAutomationWriteData
}): Promise<CommentAutomationModel> {
  const [created] = await db
    .insert(commentAutomationModel)
    .values({
      id: createId(),
      workspaceId: input.workspaceId,
      type: commentAutomationTypes.enum.messenger,
      ...this.withNormalizedReplies(input.data),
    })
    .returning()
  return created
}

No validation step exists between receiving input.data and inserting it. The other seven write
methods follow the same shape (Threads and TikTok fix privateReply to {type: "none"} and
never let the caller set it, so only their publicReply needs covering).

Why we think this is a defect rather than the intended design

We looked for a reason comment automations might deliberately skip this check that Keywords has
— no test, comment, or PR distinguishes the two, including in #1215, which added an eighth write
method (TikTok) with the identical gap rather than closing it. The two features (Keywords'
AutomatedResponse and comment triggers' CommentAutomation) share the same "reply with this
flow" concept and the same downstream worker contract; only one of the two write paths enforces
it.

Suggested fix

A private helper mirroring Keywords' check, structurally typed so it accepts either reply field
on any of this table's channel-specific reply shapes, called from all eight write methods:

private async assertFlowReplyExists(
  workspaceId: string,
  field: "privateReply" | "publicReply",
  reply: { type: string; value: string | null } | null | undefined,
  tx?: DatabaseClient,
): Promise<void> {
  if (reply?.type !== "flow" || !reply.value) {
    return
  }
  const exists = await flowService.exists(workspaceId, reply.value, tx)
  if (!exists) {
    throw validationException(field, "Flow not found")
  }
}

Full patch (imports flowService and validationException, wires the helper into all eight
write methods, threading tx through on the Threads/TikTok methods that already receive one) and
a test file (7 new cases across both reply fields and Messenger/Instagram/Threads/TikTok) are
attached, verified red→green against this commit.

What we did not verify

  • We did not run the full worker test suite; we traced detectFlowVersion and both dispatch
    handlers by reading the code, and ran only the business package's own targeted test file plus
    tsc --noEmit.
  • We did not check every other write path that might touch this table (e.g. a template
    installer, if one exists for comment automations) — we covered the eight methods reachable from
    the builder actions and the public/private APIs, matching what withNormalizedReplies's own
    doc comment says every write funnels through.

Related issues — none of these describes it

Searched "comment automation flow validation", "privateReply flow", and "FlowVersion not found" — no results for any. Not related to our own #1063 (that was about which conversation a
flow's state lands in, not about validating the flow reference itself) or #1186 (the
commentAnchor delivery bug) — adjacent area, different defect.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions