feat(whatsapp): store inbound media, close the OAuth loop, fail closed on signatures - #7
Conversation
…d on signatures
Three defects made the WhatsApp channel unusable in production.
Inbound media was dropped. The webhook decoder only understood text, image and
document, and even those were reduced to a caption string; audio, voice, video,
sticker, location, contact cards, button and list replies and reactions all hit
`if text == "" { continue }` and vanished. Media ids now resolve through the
Graph API, download server-side under a size cap, and upload as assets so they
render in the workbench and reach the AI agent. A download or upload-policy
failure degrades the message to text with its caption instead of losing the
customer's turn. Structured types become readable content so the agent can see
what the customer did.
Signature verification failed open. A missing app secret, a missing header, or a
header without the `sha256=` prefix all passed, so anyone who learned a webhook
URL could write into a conversation and trigger paid AI replies. Verification
now requires a configured Meta App Secret and a matching HMAC, and the handler
answers 401 without echoing the reason. This is breaking for deployments that
never configured a secret.
The OAuth button opened a raw JSON endpoint in a new tab. There was no callback
to exchange the code, so the flow could never complete and operators pasted
credentials by hand. POST /api/dashboard/channel/whatsapp_oauth_callback now
exchanges the code, inspects the token, discovers the reachable WABAs and sender
numbers, and saves them onto the target channel while preserving its webhook
verify token. Discovery is best effort because an Embedded Signup business token
often cannot enumerate the portfolio; partial results surface as localized
warnings rather than discarding a usable token. The dialog opens Meta in a popup
and prefills the form from the result. The authorization URL also gains
response_type=code, without which Meta returns a fragment the server never sees,
and loses a fabricated app id fallback that produced a confusing Meta error page.
The dialog also advertised /api/third/whatsapp/webhook, which verification
rejects because it requires a bound channel id, so the documented URL never
worked. It now shows the real per-channel URL.
web/messages carries the brand rename to Crove Desk alongside the new WhatsApp
strings; en-US and zh-CN still read "Agent Desk". The widget SDK's public
AgentDesk* globals are untouched because renaming them would break every site
that has already integrated it.
Both READMEs were still the upstream AgentDesk text, so a reader landing on this repository was told the wrong product name, pointed at a documentation site that is not this project's, and given setup instructions that no longer match the tree. Corrected against what the code actually does: - The compose file starts qdrant and the application, and reads its DSN from DATABASE_URL; it has not started a mysql service. The documented three-service topology could not be brought up as written. - PostgreSQL is a supported engine alongside SQLite and MySQL; bootstrap/db.go opens it for "postgres", "postgresql" or "pg", and normalizeLoadedConfig infers the engine from the DSN when it was left at the sqlite default. - The authorization URL was missing response_type=code, and the connect flow had no callback, so the "1-Click" claim described something that could not run. - Documentation links pointed at agent-desk.huabei.pro. They now point at docs/ in this repository, which is where the material actually lives. - The docker image is crove-desk:latest, matching docker-compose.yml, not mlogclub/agent-desk. - The task list, project structure and channel inventory are brought in line with Taskfile.yml, the tree, and enums/wxwork_kf.go. .env.example documented WHATSAPP_ACCESS_TOKEN, WHATSAPP_PHONE_NUMBER_ID, WHATSAPP_WABA_ID and WHATSAPP_VERIFY_TOKEN. Nothing in the codebase reads any of them; those credentials live on the channel. Replaced with the two Meta app variables that are actually bound, and a note that META_APP_SECRET is now required for the webhook to accept anything. The product backlog listed WhatsApp as "Under Consideration" while the adapter was shipping. Marked shipped, with the template-message and delivery-receipt gaps called out separately so the entry does not overclaim. The Frill publisher script carries the same entry and is updated to match, but was not run. The widget SDK's public AgentDesk* globals keep their names. They are the integration contract for every site that has already embedded the widget, and renaming them would break those sites silently. The e2e workspace-switcher selector matched the old brand names and is widened rather than replaced, so it keeps working against a configured company name.
|
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: Advanced Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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_334457f6-1aa1-457c-bd49-7a20907bead1) |
There was a problem hiding this comment.
Code Review
This pull request transitions the project branding from AgentDesk to Crove Desk and implements a robust WhatsApp Business API integration. Key additions include secure webhook signature verification that fails closed, server-side downloading and asset storage of inbound media, and support for structured message types like locations, contacts, and reactions. Additionally, an Embedded Signup and OAuth connect flow has been introduced to streamline channel setup. The review feedback highlights two important improvements: lowercasing the expected signature hex string to avoid verification failures from proxy normalization, and adding a fallback to http.DefaultClient to prevent a potential nil pointer dereference panic when c.httpClient is not initialized.
| return false | ||
| } | ||
| expected := strings.TrimSpace(signature[len(prefix):]) | ||
| if expected == "" { |
There was a problem hiding this comment.
To ensure robust signature verification, it is recommended to lowercase the expected signature before comparison. While Meta typically sends lowercase hex signatures, some proxies, API gateways, or middleware might normalize hex strings to uppercase, which would cause verification to fail.
\texpected := strings.ToLower(strings.TrimSpace(signature[len(prefix):]))| } | ||
| req.Header.Set("Authorization", "Bearer "+c.accessToken) | ||
|
|
||
| res, err := c.httpClient.Do(req) |
There was a problem hiding this comment.
If c.httpClient is nil (for example, if the client is constructed directly or SetHTTPClient is not called), calling c.httpClient.Do(req) will panic with a nil pointer dereference. It is safer to use a fallback to http.DefaultClient when c.httpClient is nil.
\tclient := c.httpClient\n\tif client == nil {\n\t\tclient = http.DefaultClient\n\t}\n\tres, err := client.Do(req)
What this does
Closes three defects that made the WhatsApp channel unusable in production, and rebrands the READMEs, which were still the upstream AgentDesk text.
1. Inbound media was dropped
The webhook decoder only understood
text,imageanddocument, and even those were reduced to a caption string. Audio, voice, video, sticker, location, contact cards, button and list replies and reactions all fell through toif text == "" { continue }and vanished — the customer's turn never reached an agent or the AI.internal/whatsapp/types.godecodes the full inbound message surface.internal/whatsapp/client.gogainsGetMediaMetadataandDownloadMedia. Media ids resolve through the Graph API and download server-side under a size cap, because Meta's lookaside URL is short-lived and only answers a request carrying our token — storing the URL on the message would give agents a broken image.Caption handling note:
normalizeMessageContentreplaces an image/attachment message's content with the stored asset file name and rebuilds the payload canonically. A caption therefore cannot ride incontentorpayload, and sending it as a second message would trigger a second AI reply. So for media with no sender-supplied file name the caption becomes the file name. A document that has both a real file name and a caption loses the caption — recorded under Known issues.2. Signature verification failed open
verifyWhatsAppSignaturereturnedtruefor any header without thesha256=prefix, and the whole check was skipped when the app secret or the header was empty. Anyone who learned a webhook URL could write into a customer conversation, trigger paid AI replies and burn message quota.Verification now requires a configured Meta App Secret and a matching HMAC, and the handler answers
401without echoing the reason so the endpoint cannot be used to probe which secret a channel expects.Warning
Breaking for a deployment that never set
META_APP_SECRETor a per-channelappSecret: the WhatsApp webhook stops accepting messages until one is configured. The rejection is logged at error level with the remedy, and.env.examplenow documents the requirement.3. The OAuth flow could not complete
The Connect button opened
/api/dashboard/channel/whatsapp_oauth_urlin a new tab — a JSON endpoint, not an authorization page. There was no callback anywhere, so the code was never exchanged and operators pasted credentials by hand. The URL also omittedresponse_type=code, without which Meta returns a token fragment the server never sees, and fell back to a fabricated app id123456789012345that produced a Meta error page looking like our bug.POST /api/dashboard/channel/whatsapp_oauth_callbackexchanges the code, inspects it withdebug_token, discovers reachable WABAs and sender numbers, and saves onto the target channel while preserving its existing webhook verify token./dashboard/channels/whatsapp-callbacklanding page performs the exchange and posts the result back with the origin pinned./api/third/whatsapp/webhook, which verification rejects because it requires a bound channel id, so the documented URL never worked. It now shows the real per-channel URL with a copy action.READMEs
Rewritten for Crove Desk and corrected against what the tree actually does:
qdrantand the application and reads its DSN fromDATABASE_URL; it has not started amysqlservice, so the documented topology could not be brought up as written.docs/in this repository instead ofagent-desk.huabei.pro.docker-compose.yml,Taskfile.yml, the tree andenums/wxwork_kf.go..env.exampledocumented fourWHATSAPP_*variables that nothing in the codebase reads; replaced with the two Meta app variables that are actually bound. The backlog listed WhatsApp as "Under Consideration" while it was shipping; marked shipped with the remaining gaps called out, and the Frill publisher script updated to match but not run.Brand strings in
en-USandzh-CNnow readCrove Desk(vi-VNalready did). The widget SDK's publicAgentDesk*globals are deliberately unchanged — they are the integration contract for every site that has already embedded the widget.Verification
go build ./...clean;go vetclean on every changed package.go test ./...full suite green, re-run after rebasing ontodev.cd web && pnpm typecheckclean.npx eslintclean on all five new or changed frontend files.git diff --checkclean; all 25 paths referenced by the READMEs verified to exist.Known limitations
pnpm lintfails repo-wide on pre-existingreact-hooks/set-state-in-effectand ref-access errors (the CI job added in ci: run CI on push to dev and main, widen coverage, and fix the release skill for this fork #6 marks lintcontinue-on-errorfor exactly this). Nothing added here contributes to that count.statusesfield is not consumed, so delivery and read receipts are not reflected.response.ChannelResponsereturnsconfigJsonverbatim, which includes access tokens and app secrets, to any dashboard caller holdingchannel.view. Pre-existing and out of scope here, but worth its own issue.Note
High Risk
Fail-closed webhook verification is a breaking change for unconfigured secrets, and the PR touches auth-sensitive inbound webhooks plus OAuth credential exchange and persistence on channels.
Overview
This PR hardens WhatsApp production use and finishes the Meta connect path, alongside Crove Desk docs and UI branding updates.
Security (breaking): WhatsApp webhook handling now requires a Meta App Secret and a valid
X-Hub-Signature-256. Missing secret, missing header, or malformed signatures are rejected with 401 (no detail on auth failures). Deployments that never setMETA_APP_SECRETor a per-channelappSecretwill stop receiving inbound WhatsApp until configured.Inbound messaging: The inbound service decodes many more webhook types (media, location, contacts, interactive replies, reactions, etc.), downloads and stores inbound media as assets (with text fallback when download/upload fails), and processes messages per batch without dropping the whole webhook on one failure.
OAuth: Adds
POST /api/dashboard/channel/whatsapp_oauth_callback,WhatsAppOAuthService(code exchange,debug_token, WABA/phone discovery, optional persist to channel), fixes the OAuth URL (no fake app id,response_type=code, requiredredirect_uri). The Channels UI uses a popup + callback page andpostMessageto prefill credentials; the dialog shows the per-channel webhook URL with copy.Client & tests: Extends the Graph
whatsappclient (media download, OAuth, discovery APIs) and adds focused Go tests plus dashboard API/types and i18n strings. READMEs, CHANGELOG, backlog, and.env.exampleare updated for Crove Desk and correct WhatsApp env/channel configuration.Reviewed by Cursor Bugbot for commit c63a35b. Configure here.