feat(channels): add LINE, Viber, and Meta Threads messaging channels - #3
Conversation
- REST API clients (internal/line, internal/viber, internal/threads) with webhook
signature verification (HMAC-SHA256 base64 for LINE, hex for Viber, sha256 for Meta)
- Inbound services mapping external identity, conversation dedupe, and customer
message persistence per channel
- Async outbox delivery via ChannelMessageOutbox with batch 20 / max retry 5 /
linear backoff, wired into cron and message service hooks
- Third-party webhook endpoints under /api/third/{line,viber,threads} with
Meta-style hub.challenge verification for Threads
- Viber conversation_started welcome message response support
- Threads replies anchor to the latest customer media id for two-step publish
- Dashboard channel create/edit forms, icons, and i18n for en-US, zh-CN, vi-VN
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_0ff4200c-cc2f-4c75-9cff-2ee275efed2a) |
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Team Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces support for three new messaging channels: LINE, Viber, and Meta Threads, including webhook handlers, inbound/outbound services, database configurations, and frontend UI components. The review feedback highlights several critical issues that need to be addressed: a security vulnerability in the Threads webhook verification when channelID is empty; a threading bug in Threads inbound processing where reply IDs are incorrectly used as customer external IDs; potential race conditions in the LINE, Viber, and Threads outbound dispatchers that could cause duplicate messages; and a frontend issue where editing channels silently deletes configuration fields like avatarUrl and welcomeMessage from the database.
| if mode == "subscribe" { | ||
| if channelID != "" { | ||
| channel := services.ChannelService.Take("channel_id = ? AND channel_type = ? AND status = ?", channelID, enums.ChannelTypeThreads, enums.StatusOk) | ||
| if channel != nil { | ||
| if cfg, err := services.ChannelService.ParseThreadsChannelConfig(channel.ConfigJSON); err == nil && cfg != nil { | ||
| if cfg.WebhookVerifyToken != "" && cfg.WebhookVerifyToken != token { | ||
| ctx.String(http.StatusForbidden, "Verification token mismatch") | ||
| return | ||
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
If channelID is empty, the verification token check is skipped entirely, and the challenge is returned with a 200 OK status. This allows anyone to verify a subscription without a valid token if they call the endpoint without a channel_id parameter.
To fix this security issue, you should fall back to the default Threads channel when channelID is empty (similar to how it is handled in ThreadsPostWebhook), and enforce token verification.
if mode == "subscribe" {
var channel *models.Channel
if channelID != "" {
channel = services.ChannelService.Take("channel_id = ? AND channel_type = ? AND status = ?", channelID, enums.ChannelTypeThreads, enums.StatusOk)
} else {
channel = services.ChannelService.Take("channel_type = ? AND status = ?", enums.ChannelTypeThreads, enums.StatusOk)
}
if channel != nil {
if cfg, err := services.ChannelService.ParseThreadsChannelConfig(channel.ConfigJSON); err == nil && cfg != nil {
if cfg.WebhookVerifyToken != "" && cfg.WebhookVerifyToken != token {
ctx.String(http.StatusForbidden, "Verification token mismatch")
return
}
}
} else {
ctx.String(http.StatusNotFound, "Channel not found")
return
}
ctx.String(http.StatusOK, challenge)
return
}| externalID := strings.TrimSpace(value.ID) | ||
| if externalID == "" { | ||
| externalID = strings.TrimSpace(value.MediaID) | ||
| } | ||
| if externalID == "" { | ||
| return nil | ||
| } | ||
|
|
||
| name := strings.TrimSpace(value.Username) | ||
| if name == "" { | ||
| name = fmt.Sprintf("Threads User %s", externalID) | ||
| } | ||
|
|
||
| externalUser := openidentity.ExternalUser{ | ||
| ExternalSource: enums.ExternalSourceThreads, | ||
| ExternalID: externalID, | ||
| ExternalName: name, | ||
| } |
There was a problem hiding this comment.
Using value.ID (the reply/media ID) as the customer's ExternalID is a critical bug. Since value.ID is unique for every single reply, every new message from the same user will be treated as a completely new customer, creating a new CustomerIdentity and a new Conversation. This breaks conversation threading and history.
Instead, you should use the customer's unique identifier (such as value.Username if a unique user ID is not provided by the Threads webhook) as the ExternalID so that all replies from the same user are correctly grouped into the same conversation.
externalID := strings.TrimSpace(value.Username)
if externalID == "" {
externalID = strings.TrimSpace(value.ID)
}
if externalID == "" {
return nil
}
name := externalID
externalUser := openidentity.ExternalUser{
ExternalSource: enums.ExternalSourceThreads,
ExternalID: externalID,
ExternalName: name,
}| // Verify customer identity | ||
| identity := repositories.CustomerIdentityRepository.FindOne(sqls.DB(), sqls.NewCnd(). | ||
| Eq("external_source", enums.ExternalSourceThreads). | ||
| Eq("external_id", "reply_9001")) |
There was a problem hiding this comment.
Since the customer's ExternalID should be mapped to their unique Username (e.g., "threads_customer") rather than the reply ID ("reply_9001"), this assertion needs to be updated to match the correct external_id and keep the tests green.
| // Verify customer identity | |
| identity := repositories.CustomerIdentityRepository.FindOne(sqls.DB(), sqls.NewCnd(). | |
| Eq("external_source", enums.ExternalSourceThreads). | |
| Eq("external_id", "reply_9001")) | |
| // Verify customer identity | |
| identity := repositories.CustomerIdentityRepository.FindOne(sqls.DB(), sqls.NewCnd(). | |
| Eq("external_source", enums.ExternalSourceThreads). | |
| Eq("external_id", "threads_customer")) |
| if err := ChannelMessageOutboxService.Updates(outbox.ID, map[string]any{ | ||
| "send_status": string(enums.ChannelMessageOutboxStatusSending), | ||
| "updated_at": time.Now(), | ||
| }); err != nil { | ||
| return err | ||
| } |
There was a problem hiding this comment.
There is a race condition here. Since DispatchPendingOutbox is triggered asynchronously on every message enqueue, multiple goroutines can run concurrently. Both goroutines can fetch the same pending outbox items via ListPending before either has updated their status to sending. As a result, both will pass the SendStatus check and call client.PushMessage, causing duplicate messages to be sent to the customer.
To prevent this, you should use optimistic locking when updating the status to sending (e.g., UPDATE ... WHERE id = ? AND send_status = 'pending') and verify that the update actually affected a row before proceeding to send the message.
| if err := ChannelMessageOutboxService.Updates(outbox.ID, map[string]any{ | ||
| "send_status": string(enums.ChannelMessageOutboxStatusSending), | ||
| "updated_at": time.Now(), | ||
| }); err != nil { | ||
| return err | ||
| } |
There was a problem hiding this comment.
There is a race condition here. Since DispatchPendingOutbox is triggered asynchronously on every message enqueue, multiple goroutines can run concurrently. Both goroutines can fetch the same pending outbox items via ListPending before either has updated their status to sending. As a result, both will pass the SendStatus check and call client.SendTextMessage, causing duplicate messages to be sent to the customer.
To prevent this, you should use optimistic locking when updating the status to sending (e.g., UPDATE ... WHERE id = ? AND send_status = 'pending') and verify that the update actually affected a row before proceeding to send the message.
| if err := ChannelMessageOutboxService.Updates(outbox.ID, map[string]any{ | ||
| "send_status": string(enums.ChannelMessageOutboxStatusSending), | ||
| "updated_at": time.Now(), | ||
| }); err != nil { | ||
| return err | ||
| } |
There was a problem hiding this comment.
There is a race condition here. Since DispatchPendingOutbox is triggered asynchronously on every message enqueue, multiple goroutines can run concurrently. Both goroutines can fetch the same pending outbox items via ListPending before either has updated their status to sending. As a result, both will pass the SendStatus check and call client.PublishTextReply, causing duplicate messages to be sent to the customer.
To prevent this, you should use optimistic locking when updating the status to sending (e.g., UPDATE ... WHERE id = ? AND send_status = 'pending') and verify that the update actually affected a row before proceeding to send the message.
| : channelType === "viber" | ||
| ? JSON.stringify({ | ||
| authToken: form.viberAuthToken.trim(), | ||
| botName: form.viberBotName.trim(), | ||
| }) |
There was a problem hiding this comment.
The Viber channel configuration contains fields like avatarUrl and welcomeMessage on the backend, but these are completely missing from the frontend form state (EditForm), createEmptyForm, and buildPayload.
If a Viber channel has an avatarUrl or welcomeMessage configured (e.g., via API or default settings), editing and saving the channel in the UI will silently overwrite and delete these fields from the database because they are not included in the serialized JSON payload.
Please ensure that all configuration fields (including welcomeMessage for LINE and Threads) are either preserved during updates or added to the form UI so they are not lost.
- Degrade image/attachment messages to signed URL text on outbound (matches TikTok pattern); outbox enqueue now accepts image/attachment so agent media no longer silently drops for the new channels - Threads webhook signature check is fail-closed once AppSecret is configured - Use the stable @username as the Threads customer identity to prevent conversation fragmentation per reply (media id stays the dedupe key) - Fall back to the channel name as the Viber sender name when botName is unset - LINE: send the configured welcome message on follow events - Round-trip all channel config fields through the edit dialog (welcome message, avatar URL, webhook secret) so saving no longer drops values set outside the form - Show the auto-generated Threads webhook verify token read-only in the form - Distinct icon for Threads channels
1 participant
Summary
Adds three new omnichannel messaging channels following the existing channel integration architecture (models -> repositories -> services -> handlers -> builders):
X-Line-SignatureHMAC-SHA256 (base64) webhook verificationsend_messageAPI,X-Viber-Content-SignatureHMAC-SHA256 (hex) verification,conversation_startedwelcome message responsereply_to_idanchored to the latest customer media id), Meta-stylehub.challengeGET verification +X-Hub-Signature-256POST verification, supports both documented webhook envelope shapesEach channel ships: REST client + tests, inbound webhook service + tests (identity mapping, conversation dedupe, message persistence), async outbox delivery (batch 20 / max retry 5 / linear backoff) wired into cron and message service hooks, third-party webhook routes under
/api/third/{line,viber,threads}, dashboard create/edit forms with icons, and i18n for en-US / zh-CN / vi-VN.Also backfills the shared
ChannelIconcomponent with missing icons for whatsapp / slack / x / tiktok.Not included (deliberate)
channel_oauth_handlerpattern.Test plan
go build ./...go test ./...- all packages pass (pre-existing unrelated failureTestBuildLightweightTicketininternal/buildersreproduces on a clean baseline; verified viagit stash -u)internal/line,internal/viber,internal/threadsclient + signature tests;TestLineInboundAndOutbound,TestViberInboundAndOutbound,TestThreadsInboundAndOutboundservice testspnpm typecheckpnpm lint- no new issues (7 pre-existing errors in unrelated files)task enumsregenerated (ExternalSource.Threadsadded to generated enums)