Skip to content

feat(whatsapp): store inbound media, close the OAuth loop, fail closed on signatures - #7

Merged
JOY (JOY) merged 2 commits into
devfrom
feat/whatsapp-media-oauth-and-rebrand
Sep 12, 2026
Merged

JOY (JOY) merged 2 commits into
devfrom
feat/whatsapp-media-oauth-and-rebrand

Conversation

@JOY

@JOY JOY (JOY) commented Sep 12, 2026 •

Copy link
Copy Markdown

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, 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 fell through to if text == "" { continue } and vanished — the customer's turn never reached an agent or the AI.

  • internal/whatsapp/types.go decodes the full inbound message surface.
  • internal/whatsapp/client.go gains GetMediaMetadata and DownloadMedia. 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.
  • Media uploads as a first-class asset, so it renders in the workbench and reaches the AI agent.
  • A download failure or an upload-policy rejection degrades the message to text carrying its caption, rather than losing it.
  • Location, contact cards, button/list replies and reactions become readable content. Unsupported types are recorded with their type name.

Caption handling note: normalizeMessageContent replaces an image/attachment message's content with the stored asset file name and rebuilds the payload canonically. A caption therefore cannot ride in content or payload, 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

verifyWhatsAppSignature returned true for any header without the sha256= 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 401 without 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_SECRET or a per-channel appSecret: the WhatsApp webhook stops accepting messages until one is configured. The rejection is logged at error level with the remedy, and .env.example now documents the requirement.

3. The OAuth flow could not complete

The Connect button opened /api/dashboard/channel/whatsapp_oauth_url in 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 omitted response_type=code, without which Meta returns a token fragment the server never sees, and fell back to a fabricated app id 123456789012345 that produced a Meta error page looking like our bug.

  • POST /api/dashboard/channel/whatsapp_oauth_callback exchanges the code, inspects it with debug_token, discovers reachable WABAs and sender numbers, and saves onto the target channel while preserving its existing webhook verify token.
  • Discovery is best effort. An Embedded Signup business token often cannot enumerate the portfolio and Meta answers with an empty list or code 200 rather than a failure, so partial results surface as localized warnings instead of discarding a usable token.
  • The dialog opens Meta in a popup; a new /dashboard/channels/whatsapp-callback landing page performs the exchange and posts the result back with the origin pinned.
  • The dialog 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 with a copy action.

READMEs

Rewritten for Crove Desk and corrected against what the tree actually does:

  • Compose starts qdrant and the application and reads its DSN from DATABASE_URL; it has not started a mysql service, so the documented topology could not be brought up as written.
  • PostgreSQL is a supported engine alongside SQLite and MySQL.
  • Doc links point at docs/ in this repository instead of agent-desk.huabei.pro.
  • Image name, task list, project structure and the 16-channel inventory match docker-compose.yml, Taskfile.yml, the tree and enums/wxwork_kf.go.
  • Fork attribution and the upstream sync model are stated.

.env.example documented four WHATSAPP_* 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-US and zh-CN now read Crove Desk (vi-VN already did). The widget SDK's public AgentDesk* globals are deliberately unchanged — they are the integration contract for every site that has already embedded the widget.

Verification

  • go build ./... clean; go vet clean on every changed package.
  • go test ./... full suite green, re-run after rebasing onto dev.
  • New tests: signature table test, unsigned/invalid/valid webhook delivery, missing-app-secret rejection, structured inbound types, media stored as asset, media-failure fallback, and five OAuth connect cases including discovery failure and wrong-channel-type rejection.
  • cd web && pnpm typecheck clean.
  • npx eslint clean on all five new or changed frontend files.
  • Both locale files carry the same 12 new keys; all three frontend catalogs parse and carry the same 26 WhatsApp keys.
  • git diff --check clean; all 25 paths referenced by the READMEs verified to exist.

Known limitations

  • pnpm lint fails repo-wide on pre-existing react-hooks/set-state-in-effect and 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 lint continue-on-error for exactly this). Nothing added here contributes to that count.
  • No browser verification of the Channels dialog or the callback page was performed; the OAuth flow was not exercised against Meta's live endpoints.
  • WhatsApp outbound still has no template message support, so business-initiated conversations outside the 24-hour window are not possible, and the webhook statuses field is not consumed, so delivery and read receipts are not reflected.
  • response.ChannelResponse returns configJson verbatim, which includes access tokens and app secrets, to any dashboard caller holding channel.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 set META_APP_SECRET or a per-channel appSecret will 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, required redirect_uri). The Channels UI uses a popup + callback page and postMessage to prefill credentials; the dialog shows the per-channel webhook URL with copy.

Client & tests: Extends the Graph whatsapp client (media download, OAuth, discovery APIs) and adds focused Go tests plus dashboard API/types and i18n strings. READMEs, CHANGELOG, backlog, and .env.example are updated for Crove Desk and correct WhatsApp env/channel configuration.

Reviewed by Cursor Bugbot for commit c63a35b. Configure here.

…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.
@coderabbitai

coderabbitai Bot commented Sep 12, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: e94b7e37-9288-4be4-9863-ab89f3173f5e

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@cursor

cursor Bot commented Sep 12, 2026

Copy link
Copy Markdown

Bugbot couldn't run - usage limit reached

Bugbot 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)

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 == "" {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

security-medium medium

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)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

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)

@JOY
JOY (JOY) merged commit 7addccc into dev Sep 12, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant