fix: resolve fork-caused defects from code review audit - #4
Conversation
…ation - add discord and messenger channel and identity enums with generated frontend types - implement internal discord and messenger REST API clients - add discord and messenger inbound webhook services with customer identity resolution and rich media attachment support - add async outbox delivery services for discord and messenger with retry handling - register third webhook endpoints and 1-click oauth authorization handlers - update dashboard channels list and edit dialog with discord and messenger configurations - add full multilingual translation keys across en-US, vi-VN, and zh-CN
…s and system config support
…ress and auto-forwarding
…ect in conversation header and sidebar
…ation hierarchy provisioning
… inbound email routing
… and dashboard UI
…h quick take-it and unassign
…e.io for inbound forwarding
… active/mine tabs
…mbine omnichannel profiles
…s on customer list and details
- 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
- 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
docs/ pointed to huabeitech/agent-desk-docs (private, never initialized locally), so the Crove-internal documents in it were unversioned. Remove the gitlink and track them as regular files. qdrant/ was an empty, unused gitlink (compose uses the qdrant/qdrant Docker Hub image; code uses github.com/qdrant/go-client). With both entries gone, .gitmodules is deleted as well.
- Meta webhook GET verification (threads, messenger, whatsapp, instagram): require a configured channel and a constant-time verify-token match before echoing hub.challenge - an unbound echo let anyone confirm a webhook subscription they do not own - cron: drain X and TikTok outboxes, which relied solely on the fire-and-forget goroutine at enqueue - viber client: reject non-2xx responses instead of unmarshaling error bodies - email SMTP: use net.JoinHostPort so IPv6 literal hosts dial correctly (go vet) - frontend: replace the undefined --font-inter var with the loaded --font-geist-sans so font-family declarations stop being discarded; add the common and knowledge.status keys missing from en-US and zh-CN; drop the stale applyBranding call in the locale provider
Mark the 7 IDs fixed in the fix commit (SEC-11 plus three identical Meta-handler siblings, BUG-05, BUG-06, BUG-10, BUG-20, BUG-22, BUG-25) and retract BUG-02 (already fixed in 6a3b9c7), BUG-03 (guards are identical on read), BUG-07 (token input exists, disabled by design) and BUG-25 (narrowed: the locale/publicConfig effect self-heals the title).
|
Important Review skippedToo many files! This PR contains 140 files, which is 40 over the limit of 100. To get a review, reduce the PR to 100 files or fewer by splitting it into smaller PRs or changing its base branch. Upgrade to a paid plan to raise the limit. This review couldn't start because sufficient usage credits or metered capacity aren't available. Add credits or update usage-based reviews in the billing tab, then retry. ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (140)
You can disable this status message by setting the Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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_50476133-a381-4796-9524-aeb3a5e16fb1) |
![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 comprehensive support for multiple new communication channels (Discord, Facebook Messenger, Instagram, WhatsApp, Slack, X, TikTok, LINE, Viber, and Threads) into the Crove Desk platform. It includes backend API clients, inbound webhook handlers, outbound message queues, and frontend UI updates for channel configuration. Additionally, it implements a customer profile merging feature and updates the architecture to a conversational support model. My review identified a high-severity issue regarding the accidental removal of custom email delivery settings, as well as performance concerns related to N+1 database queries in conversation and customer builders. I also noted a minor formatting issue in the architecture documentation that conflicts with the project's new formatting rules.
I am having trouble creating individual review comments. Click here to see my feedback.
web/app/(dashboard)/dashboard/channels/_components/edit.tsx (414-424)
The logic for saving custom email delivery settings (SMTP, API keys) has been removed from the payload construction for email channels. This means any custom SMTP or ESP configuration will be lost upon saving the channel settings. Was this intentional? If custom delivery is still a supported feature, these fields should be included in the payload.
docs/OMNICHANNEL_CONVERSATIONAL_SUPPORT_REFACTOR.md (18)
The use of LaTeX math syntax $\rightarrow$ seems to violate the new formatting rule defined in .cursor/rules/user-interaction-preferences.mdc, which advises against it to prevent rendering issues. It would be better to use a standard Unicode character like → or ->.
internal/builders/conversation_builder.go (48-53)
This block introduces a potential N+1 query issue. The call to services.ChannelService.Get(item.ChannelID) is inside BuildConversationWithLocale, which is likely to be called within a loop when fetching a list of conversations. This would result in a separate database query for each conversation to get its channel details.
To optimize, consider fetching all required channels in a single query before the loop and then looking them up from a map, similar to the pattern used for identities in customer_builder.go.
internal/builders/customer_builder.go (97)
This line introduces a potential N+1 query problem. The call to services.CompanyService.Get(item.CompanyID) is inside a loop, which will likely result in a separate database query for each customer to fetch its company details. To improve performance, it would be better to fetch all required companies in a single query before this loop and then look them up from a map inside the loop.
…nancy claim - agent_loop_live_test.go: the hardcoded DOS.AI key fallback is replaced with t.Skip, matching dos_ai_live_test.go; the key itself must still be rotated and purged from git history (git filter-repo) - owner action - default_kb.go: the seeded FAQ no longer implies per-workspace data isolation; it now states that conversations, tickets, customers and knowledge are shared across workspaces in one deployment
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_9ad9fad2-ae1f-4300-882e-3f74558ec664) |
1 participant
What
Fixes for the fork-caused defects surfaced by the code review audit (
docs/CROVE_DESK_AUDIT.html). Also carries the submodule cleanup (9a25b41f) and the audit document itself.Fixes
hub.challenge— an unbound echo let anyone confirm a webhook subscription they do not ownnet.JoinHostPort— IPv6 literal hosts now dial correctly,go vetcleancommon.{actions,name,delete,edit,create,refresh,description}+knowledge.status--font-intervar with the loaded--font-geist-sans— the wholefont-familydeclarations were being discarded at computed-value timeapplyBrandingcall in the locale provider (the[locale, publicConfig]effect applies branding with fresh state)Retracted audit claims
6a3b9c7c(externalID = username)[locale, publicConfig]effect self-heals the titleVerification
go build ./internal/...·go vet(touched packages) ✓go test ./internal/handlers/third/...✓ ·go test ./internal/services/...✓pnpm typecheck✓ ·gofmt✓ ·git diff --check✓Not included (owner actions)
git filter-repo)Note
Medium Risk
Webhook verification and outbox cron changes reduce real exploit/reliability risk, but the PR also expands many inbound channel routes and Docker/DB defaults while numerous P0 audit items (auth, uploads, guest impersonation) remain unfixed.
Overview
This PR bundles audit-driven fork fixes with Crove Desk packaging/docs and a large omnichannel surface area (routes, OAuth URL endpoints, Discord client, customer API enrichments).
Security & reliability fixes tighten Meta-family webhook GET verification (configured channel + constant-time verify token before echoing
hub.challenge), add X/TikTok to the 5s outbox cron drain, make the Viber client fail on non-2xx responses, fix SMTP dialing vianet.JoinHostPort, remove the hardcoded DOS.AI key from the live agent test (skip when unset), and soften default KB copy so multi-tenancy is not overstated.Frontend/i18n adds missing
common.*/knowledge.statuskeys in en-US/zh-CN and replaces undefined--font-interwith--font-geist-sans; the i18n provider drops a staleapplyBrandingcall on locale change.Platform & ops shift defaults toward Supabase Postgres (
.env.example,docker-composewithout MySQL,crove-deskimage,.env+host.docker.internal), drop git submodules fordocs/qdrant, and expand.dockerignorefor monorepo builds.Product/engine registers many third-party channel webhooks and dashboard OAuth URL routes, introduces a Discord REST client, adds customer merge plus identity/channel fields on customer and conversation responses, and adds extensive in-repo architecture, audit register, backlog, and channel integration specs (plus Cursor interaction rules).
Reviewed by Cursor Bugbot for commit 95b32c6. Configure here.