feat(webhook): finalize provider ingress parity

This commit is contained in:
2026-06-05 00:49:29 +08:00
parent 5b4982d87c
commit ebd8f081ca
4 changed files with 127 additions and 24 deletions
+13 -8
View File
@@ -67,10 +67,13 @@ This ledger records the committed parity checkpoints that future slices should b
| `0ea7a08 feat(webhook): verify tiktok ingress signatures` | Added Chatwoot-style TikTok `Tiktok-Signature` HMAC verification using `TIKTOK_APP_SECRET`, timestamp freshness, and invalid-signature rejection coverage. | Focused webhook tests passed; full `go test ./...` passed. | Continue P6.7 review with LINE/WhatsApp missing-signature edge fixtures and final provider classification. |
| `0439f3b feat(webhook): require line ingress signatures` | Tightened LINE webhook signature parity so configured `channel_secret` requires a present and valid `X-Line-Signature`, with missing-signature rejection coverage. | Focused webhook tests passed; full `go test ./...` passed. | Continue P6.7 review with WhatsApp missing-signature edge fixtures and Shopify/Twitter final classification. |
| `1c30933 test(webhook): cover whatsapp ingress verification` | Added WhatsApp route-level verification coverage for verify-token challenge echo, signed Cloud API POST persistence, and missing Meta signature rejection without persistence. | Focused webhook tests passed; full `go test ./...` passed. | Continue P6.7 review with Shopify/Twitter final classification. |
| Working tree | Finalized P6.7 provider classification with Twitter CRC prefix parity, Twitter route-level webhook tests, Instagram missing-signature rejection coverage, and Shopify real-handler classification. | Focused webhook/API tests passed; full `go test ./...` passed. | Move next to serializer/API parity queue after commit verification. |
## Next Slice Contract
Next implementation slice: finish B1/P6.7 webhook ingress by replacing provider parse/log boundaries with real incoming message persistence and idempotent dispatch.
Completed implementation slice: B1/P6.7 webhook ingress replaced provider parse/log boundaries with verified, persistent, idempotent dispatch paths.
Next implementation slice: continue Q4/B2 serializer parity fixtures for auth/profile and other frontend-critical API families.
| Step | Required result | Reference source | Verification |
| --- | --- | --- | --- |
@@ -91,6 +94,7 @@ Current N1/N2 implementation checkpoint:
- Added TikTok webhook signature verification to match `reference/chatwoot/app/controllers/webhooks/tiktok_controller.rb`: `Tiktok-Signature` must include `t=<unix>,s=<hmac>`, the HMAC is `sha256(TIKTOK_APP_SECRET, "<timestamp>.<raw_body>")`, and stale signatures older than five seconds are rejected.
- Tightened LINE webhook signature verification to match `reference/chatwoot/app/jobs/webhooks/line_events_job.rb`: when `channel_secret` is configured, `X-Line-Signature` must be present and equal `base64(hmac_sha256(channel_secret, raw_body))` before parsing or persistence.
- Added WhatsApp route-level fixtures matching `reference/chatwoot/app/controllers/webhooks/whatsapp_controller.rb` and `MetaTokenVerifyConcern`: verify-token requests echo `hub.challenge`, Cloud API posts require `X-Hub-Signature-256` when an app secret is configured, and signed payloads persist incoming messages by provider source ID.
- Added Twitter CRC route tests and fixed CRC responses to include the Chatwoot/Twitter `sha256=` prefix. Added Instagram missing-signature rejection coverage. Shopify remains a real verified handler with HMAC rejection and `shop/redact` cleanup coverage.
## Immediate Execution Queue
@@ -103,7 +107,7 @@ This is the ordered queue for the next implementation slices. Do not skip the ro
| Q3 | Add route boot regression coverage for wildcard conflict groups before expanding more Rails-style resources. | Phase 2 | Router tests cover nested dynamic resources that previously risked Gin conflicts. | Done |
| Q4 | Start serializer parity fixtures for auth/session, conversations/messages, contacts/companies, inboxes, notifications, and search. | Phase 3 | Each area has at least one reference fixture and Go response test. | Doing |
| Q5 | Review Meilisearch document shape and endpoint payloads against Chatwoot frontend consumers. | Phase 1 and Phase 3 | Search remains Meilisearch-first and payload mismatches are fixed or tracked. | Todo |
| Q6 | Implement provider-specific webhook ingress for Chatwoot public webhook paths. | Phase 6 | Generic webhook placeholder no longer masks provider gaps; Telegram, LINE, SMS/Twilio, WhatsApp, Instagram/Twitter/TikTok routes resolve and verify like Chatwoot where supported. | Doing |
| Q6 | Implement provider-specific webhook ingress for Chatwoot public webhook paths. | Phase 6 | Generic webhook placeholder no longer masks provider gaps; Telegram, LINE, SMS/Twilio, WhatsApp, Instagram/Twitter/TikTok routes resolve and verify like Chatwoot where supported. | Done |
| Q7 | Burn down enterprise gaps in this order: SLA, assignment policy and capacity, CSAT, automation/macros, Audit, CustomRole, InboxLimit, Captain/Copilot. | Phase 4 and Phase 5 | Each feature passes route, persistence, auth, side-effect, response, and test checks. | Todo |
| Q8 | Add frontend smoke harness using the reused Chatwoot frontend once core API flows boot end-to-end. | Phase 7 | Login, inbox list, conversation list/detail, message send, contact view, and widget init run without frontend adapters. | Todo |
@@ -140,7 +144,7 @@ Work proceeds top-down unless a failing test or frontend blocker forces a narrow
| Slice | Work | Reference source | Verification | Status |
| --- | --- | --- | --- | --- |
| B1 | Webhook ingress route and handler parity. | `reference/chatwoot/config/routes.rb:614-624`, `reference/chatwoot/app/controllers/webhooks/*`, `reference/chatwoot/app/controllers/api/v1/webhooks_controller.rb` | Provider lookup tests, router route dump, `go test ./...`. | Doing |
| B1 | Webhook ingress route and handler parity. | `reference/chatwoot/config/routes.rb:614-624`, `reference/chatwoot/app/controllers/webhooks/*`, `reference/chatwoot/app/controllers/api/v1/webhooks_controller.rb` | Provider lookup tests, router route dump, `go test ./...`. | Done |
| B2 | Auth/profile serializer fixtures. | `reference/chatwoot/app/controllers/api/v1/profile*`, frontend auth client. | Fixture tests for login/current user/profile/availability/settings. | Todo |
| B3 | Conversation/message serializer and behavior fixtures. | Chatwoot conversation/message controllers, entities, jobs. | Fixture tests for list/show/create/update/private notes/attachments/status/assignment. | Todo |
| B4 | Contact/company behavior fixtures. | Chatwoot contact/company controllers, merge/import/export/notes/labels. | Fixture tests for CRUD/search/merge/relation/import-export shells. | Todo |
@@ -507,7 +511,7 @@ Tracking table:
| P6.4 | Message APIs | `docs/ROUTE_GAP_ANALYSIS.md`, message handlers/services | Implement create/list/delete, private notes, attachments, source attribution, events. | Todo |
| P6.5 | Inbox APIs | `docs/ROUTE_GAP_ANALYSIS.md`, inbox handlers/services | Implement CRUD, assignable agents, avatar, campaigns, channel settings, reset secret. | Todo |
| P6.6 | Widget/public APIs | `docs/ROUTE_GAP_ANALYSIS.md`, widget/channel provider code, `chatwootParityStub` routes | Widget/public frontend-critical route behavior is handler-backed, including public inbox flow, direct uploads/attachments, and public CSAT survey submission. | Done |
| P6.7 | Webhook ingress | `internal/router/router.go`, `internal/handler/webhook/*`, channel providers | Replace generic placeholder with provider-specific verified ingestion and dispatch. | Doing |
| P6.7 | Webhook ingress | `internal/router/router.go`, `internal/handler/webhook/*`, channel providers | Replace generic placeholder with provider-specific verified ingestion and dispatch. | Done |
Webhook ingress subtracking:
@@ -515,12 +519,12 @@ Webhook ingress subtracking:
| --- | --- | --- | --- | --- | --- |
| P6.7a | Twitter `GET/POST /webhooks/twitter` | `api/v1/webhooks#twitter_crc`, `#twitter_events` | Go route exposed only `/webhooks/twitter/webhook`. | CRC and event routes exist at Chatwoot paths and use the existing Twitter handlers/tests. | Done |
| P6.7b | LINE `POST /webhooks/line/:line_channel_id` | `webhooks/line#process_payload` | Router param and handler lookup were mismatched; handler read an inbox-style param instead of line channel ID. | Handler resolves `ChannelLINE` by `channel_id`, verifies `X-Line-Signature`, and persists/dispatches like Chatwoot. | Done |
| P6.7c | Telegram `POST /webhooks/telegram/:bot_token` | `webhooks/telegram#process_payload` | Handler lookup was a placeholder and did not resolve the real inbox by bot token. | Handler resolves `ChannelTelegram` by `bot_token`, loads inbox, processes update, and returns provider-safe `200 OK`. | Review |
| P6.7d | SMS/Twilio `POST /webhooks/sms/:phone_number` | `webhooks/sms#process_payload` | Go path was `/webhooks/twilio/sms/:phone_number`; handler read an inbox-style param. | Chatwoot path is registered, phone number resolves `ChannelTwilioSMS`, signature verification is applied where configured, and message/status events dispatch. | Review |
| P6.7c | Telegram `POST /webhooks/telegram/:bot_token` | `webhooks/telegram#process_payload` | Handler lookup was a placeholder and did not resolve the real inbox by bot token. | Handler resolves `ChannelTelegram` by `bot_token`, loads inbox, processes update, persists incoming messages, and returns provider-safe `200 OK`. | Done |
| P6.7d | SMS/Twilio `POST /webhooks/sms/:phone_number` | `webhooks/sms#process_payload` | Go path was `/webhooks/twilio/sms/:phone_number`; handler read an inbox-style param. | Chatwoot path is registered, phone number resolves `ChannelTwilioSMS`, message/status events persist and dispatch, and Twilio-safe XML ack behavior is covered. | Done |
| P6.7e | WhatsApp `GET/POST /webhooks/whatsapp/:phone_number` | `webhooks/whatsapp#verify`, `#process_payload` | Verify-token lookup scanned only account `0`, and Cloud signature verification used access token as a placeholder secret. | Verify challenge and POST event ingestion match Chatwoot path, token, response, and inbox resolution behavior. | Done |
| P6.7f | Instagram `GET/POST /webhooks/instagram` | `webhooks/instagram#verify`, `#events` | Chatwoot no-param route was registered but still returned parity stub responses. | Verify/event routes exist at Chatwoot paths and resolve account/inbox from payload/subscription data. | Review |
| P6.7f | Instagram `GET/POST /webhooks/instagram` | `webhooks/instagram#verify`, `#events` | Chatwoot no-param route was registered but still returned parity stub responses. | Verify/event routes exist at Chatwoot paths, validate Meta signatures, resolve account/inbox from payload/subscription data, and persist/dispatch messages. | Done |
| P6.7g | TikTok `POST /webhooks/tiktok` | `webhooks/tiktok#events` | Go route expected `:business_id`; Chatwoot route has no path param and should derive identity from payload. | Handler accepts Chatwoot path, verifies `Tiktok-Signature`, resolves business/inbox from payload, and persists/dispatches provider events. | Done |
| P6.7h | Shopify `POST /webhooks/shopify` | `webhooks/shopify#events` | Chatwoot route existed but returned parity stub responses. | Route either has a real verified handler or is explicitly tracked as unsupported without placeholder success. | Review |
| P6.7h | Shopify `POST /webhooks/shopify` | `webhooks/shopify#events` | Chatwoot route existed but returned parity stub responses. | Route has a real verified handler with Shopify HMAC validation, `shop/redact` cleanup, and supported topic forwarding. | Done |
| P6.7i | Generic fallback and auth middleware | Go `WebhookAuth`, `webhookStub` | Generic middleware reads `:channel_type/:identifier`, which breaks provider-specific routes; fallback returned placeholder success. | Provider routes perform provider-specific verification; fallback no longer masks missing providers with success JSON. | Done |
P6.7 implementation notes:
@@ -615,3 +619,4 @@ Verification milestone gates:
- 2026-06-05: Added TikTok webhook signature parity. `/webhooks/tiktok` now rejects missing, invalid, or stale `Tiktok-Signature` values using the same timestamp-plus-body HMAC shape as the Chatwoot reference controller, while valid signed payloads still resolve the inbox from `biz_id` and persist incoming messages. Focused webhook tests and full `go test ./...` passed.
- 2026-06-05: Tightened LINE webhook signature parity. `/webhooks/line/:line_channel_id` now rejects missing signatures when a `channel_secret` is configured and only persists signed payloads whose `X-Line-Signature` matches the raw request body. Focused webhook tests and full `go test ./...` passed.
- 2026-06-05: Completed WhatsApp route-level webhook verification coverage. `/webhooks/whatsapp/:phone_number` now has tests for verify-token challenge echo, signed Cloud API POST persistence, and missing `X-Hub-Signature-256` rejection without message persistence. Focused webhook tests and full `go test ./...` passed.
- 2026-06-05: Finalized P6.7 provider classification. Twitter CRC now returns `sha256=<base64-hmac>` like Chatwoot, Twitter route-level CRC/event tests exist, Instagram rejects unsigned signed-required event payloads without persistence, and Telegram, SMS/Twilio, Instagram, Shopify, and the already completed providers are marked Done in the webhook ingress tracker. Focused webhook/API tests and full `go test ./...` passed.
+16 -16
View File
@@ -376,7 +376,7 @@ func (p *TwitterProvider) ValidateCRC(crcToken string) string {
mac := hmac.New(sha256.New, []byte(p.crcSecret))
mac.Write([]byte(crcToken))
responseToken := base64.StdEncoding.EncodeToString(mac.Sum(nil))
return responseToken
return "sha256=" + responseToken
}
// DeleteWebhook removes a webhook from the Twitter Account Activity API.
@@ -416,8 +416,8 @@ func parseInboxConfig(inbox *model.Inbox) channel.ChannelConfig {
// ConfigSchema returns the configuration schema for Twitter channels.
func (p *TwitterProvider) ConfigSchema() *channel.ConfigSchemaDefinition {
return &channel.ConfigSchemaDefinition{
Type: "object",
Required: []string{"twitter_user_id"},
Type: "object",
Required: []string{"twitter_user_id"},
Properties: map[string]channel.ConfigProperty{
"twitter_user_id": {Type: "string", Description: "Twitter user ID (numeric)"},
"access_token": {Type: "string", Description: "OAuth 2.0 access token"},
@@ -607,19 +607,19 @@ func (p *TwitterProvider) GetContactProfile(ctx context.Context, inbox *model.In
// Capabilities returns the set of features this channel supports.
func (p *TwitterProvider) Capabilities() channel.ChannelCapabilities {
return channel.ChannelCapabilities{
SupportsAttachments: true,
SupportsLocation: false,
SupportsAttachments: true,
SupportsLocation: false,
SupportsTypingIndicator: false,
SupportsDeliveryStatus: false,
SupportsReplies: true,
SupportsEmojiReactions: false,
SupportsVoiceMessages: false,
SupportsVideoCalls: false,
SupportsCustomCards: false,
SupportsTemplates: false,
SupportsEmailHeaders: false,
MaxAttachmentSize: 5 * 1024 * 1024, // 5MB
MaxTextLength: 10000,
SupportsDeliveryStatus: false,
SupportsReplies: true,
SupportsEmojiReactions: false,
SupportsVoiceMessages: false,
SupportsVideoCalls: false,
SupportsCustomCards: false,
SupportsTemplates: false,
SupportsEmailHeaders: false,
MaxAttachmentSize: 5 * 1024 * 1024, // 5MB
MaxTextLength: 10000,
}
}
@@ -650,4 +650,4 @@ func (p *TwitterProvider) ValidateAccessToken(ctx context.Context, accessToken s
}
return resp.StatusCode() == http.StatusOK, nil
}
}
@@ -0,0 +1,59 @@
package v1
import (
"bytes"
"crypto/hmac"
"crypto/sha256"
"encoding/base64"
"encoding/json"
"net/http"
"net/http/httptest"
"testing"
"github.com/gin-gonic/gin"
twitterchannel "github.com/gochat/gochat/internal/channel/twitter"
)
func TestTwitterWebhookCRCUsesConfiguredSecret(t *testing.T) {
gin.SetMode(gin.TestMode)
t.Setenv("TWITTER_CRC_SECRET", "crc-secret")
provider := twitterchannel.NewTwitterProvider(twitterchannel.TwitterOAuth2Config{})
h := NewTwitterChannelHandler(nil, provider, nil, nil)
r := gin.New()
r.GET("/webhooks/twitter", h.WebhookCRC)
req := httptest.NewRequest(http.MethodGet, "/webhooks/twitter?crc_token=crc-token", nil)
w := httptest.NewRecorder()
r.ServeHTTP(w, req)
if w.Code != http.StatusOK {
t.Fatalf("expected 200, got %d body=%s", w.Code, w.Body.String())
}
mac := hmac.New(sha256.New, []byte("crc-secret"))
mac.Write([]byte("crc-token"))
expected := "sha256=" + base64.StdEncoding.EncodeToString(mac.Sum(nil))
var payload map[string]string
if err := json.Unmarshal(w.Body.Bytes(), &payload); err != nil {
t.Fatalf("decode crc response: %v", err)
}
if payload["response_token"] != expected {
t.Fatalf("unexpected response token: %q", payload["response_token"])
}
}
func TestTwitterWebhookEventAcknowledgesPayload(t *testing.T) {
gin.SetMode(gin.TestMode)
provider := twitterchannel.NewTwitterProvider(twitterchannel.TwitterOAuth2Config{})
h := NewTwitterChannelHandler(nil, provider, nil, nil)
r := gin.New()
r.POST("/webhooks/twitter", h.WebhookEvent)
req := httptest.NewRequest(http.MethodPost, "/webhooks/twitter", bytes.NewReader([]byte(`{"direct_message_events":[]}`)))
req.Header.Set("Content-Type", "application/json")
w := httptest.NewRecorder()
r.ServeHTTP(w, req)
if w.Code != http.StatusOK {
t.Fatalf("expected 200, got %d body=%s", w.Code, w.Body.String())
}
}
@@ -792,6 +792,45 @@ func TestInstagramWebhookEventsVerifySignatureAndResolveInbox(t *testing.T) {
assertPersistedMessage(t, db, inbox.ID, "mid-1", "hello")
}
func TestInstagramWebhookRejectsMissingSignature(t *testing.T) {
gin.SetMode(gin.TestMode)
db := newWebhookLookupTestDB(t)
inbox := seedWebhookInbox(t, db, "instagram")
channel := channelmodel.ChannelInstagram{
AccountID: 1,
InboxID: inbox.ID,
InstagramAccountID: "ig-123",
InstagramBusinessAccountID: "ig-business-123",
PageAccessToken: "page-token",
ConnectedFBPageID: "page-123",
InstagramAccountName: "gochat",
}
if err := db.Create(&channel).Error; err != nil {
t.Fatalf("create instagram channel: %v", err)
}
t.Setenv("INSTAGRAM_APP_SECRET", "ig-secret")
body := []byte(`{"object":"instagram","entry":[{"id":"ig-123","time":1,"messaging":[{"sender":{"id":"user-1"},"recipient":{"id":"ig-123"},"timestamp":1,"message":{"mid":"mid-missing-sig","text":"hello"}}]}]}`)
h := NewFacebookWebhookHandler(nil, nil, db)
r := gin.New()
r.POST("/webhooks/instagram", h.HandleInstagramWebhook)
req := httptest.NewRequest(http.MethodPost, "/webhooks/instagram", bytes.NewReader(body))
w := httptest.NewRecorder()
r.ServeHTTP(w, req)
if w.Code != http.StatusUnauthorized {
t.Fatalf("expected 401, got %d body=%s", w.Code, w.Body.String())
}
var count int64
if err := db.Model(&model.Message{}).Where("inbox_id = ? AND source_id = ?", inbox.ID, "mid-missing-sig").Count(&count).Error; err != nil {
t.Fatalf("count message: %v", err)
}
if count != 0 {
t.Fatalf("expected no persisted message, got %d", count)
}
}
func assertPersistedMessage(t *testing.T, db *gorm.DB, inboxID uint, sourceID string, content string) model.Message {
t.Helper()
var message model.Message