From 0439f3bf87b610d6f8814b263a8692fbd540b503 Mon Sep 17 00:00:00 2001 From: Rogee Date: Fri, 5 Jun 2026 00:35:30 +0800 Subject: [PATCH] feat(webhook): require line ingress signatures --- docs/CHATWOOT_PARITY_DEVELOPMENT_PLAN.md | 5 +- internal/handler/webhook/line_webhook.go | 4 +- .../handler/webhook/webhook_lookup_test.go | 49 +++++++++++++++++++ 3 files changed, 55 insertions(+), 3 deletions(-) diff --git a/docs/CHATWOOT_PARITY_DEVELOPMENT_PLAN.md b/docs/CHATWOOT_PARITY_DEVELOPMENT_PLAN.md index 66cbe2de..dc867cb2 100644 --- a/docs/CHATWOOT_PARITY_DEVELOPMENT_PLAN.md +++ b/docs/CHATWOOT_PARITY_DEVELOPMENT_PLAN.md @@ -65,6 +65,7 @@ This ledger records the committed parity checkpoints that future slices should b | `06b999b feat(webhook): dispatch persisted provider events` | Wired webhook incoming persistence and status updates into the existing `channel.Dispatcher` fan-out boundary. | Focused webhook tests passed; full `go test ./...` passed. | Continue P6.7 review with broader provider fixture assertions. | | `55295dd test(webhook): cover provider ingress persistence fixtures` | Added provider-specific webhook persistence fixture assertions for LINE, Twilio SMS, WhatsApp, Instagram, and TikTok, extending the existing Telegram fixture. | Focused webhook tests passed; full `go test ./...` passed. | Continue P6.7 review with signature edge fixtures and final provider Done/Review classification. | | `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. | +| Working tree | 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. | ## Next Slice Contract @@ -87,6 +88,7 @@ Current N1/N2 implementation checkpoint: - Wired the same boundary into `channel.Dispatcher` so incoming webhooks emit `contact.created`, `conversation.created/opened/updated`, `message.created/incoming`, and `message.status_updated` events for automation, CSAT, bot rules, notifications, and future async workers. - Added provider-specific persistence fixtures for Telegram, LINE, Twilio SMS, WhatsApp, Instagram, and TikTok. These tests assert durable `messages` rows by provider source ID instead of only checking webhook `200 OK` acknowledgements. - Added TikTok webhook signature verification to match `reference/chatwoot/app/controllers/webhooks/tiktok_controller.rb`: `Tiktok-Signature` must include `t=,s=`, the HMAC is `sha256(TIKTOK_APP_SECRET, ".")`, 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. ## Immediate Execution Queue @@ -510,7 +512,7 @@ Webhook ingress subtracking: | ID | Provider/path | Chatwoot reference | Current Go gap | Done when | Status | | --- | --- | --- | --- | --- | --- | | 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 dispatches/acks like Chatwoot. | Review | +| 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.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. | Review | @@ -609,3 +611,4 @@ Verification milestone gates: - 2026-06-05: Wired P6.7 incoming persistence into the existing dispatcher fan-out boundary. Newly persisted webhook contacts, conversations, messages, and message status updates now emit `ChannelEvent`s through `DispatchAsync`'s current sync fallback, keeping automation, bot rule, CSAT, and notification listeners reachable from provider webhooks. Added listener-based regression coverage for incoming and status events. Focused webhook tests and full `go test ./...` passed. - 2026-06-05: Broadened P6.7 provider webhook persistence fixtures. LINE, Twilio SMS, WhatsApp, Instagram, and TikTok webhook tests now assert persisted incoming `messages` by provider source ID, matching the existing Telegram persistence fixture and reducing the remaining provider-review surface to signature edge cases and final unsupported-provider classification. Focused webhook tests and full `go test ./...` passed. - 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. diff --git a/internal/handler/webhook/line_webhook.go b/internal/handler/webhook/line_webhook.go index 2819cf8e..aa33e166 100644 --- a/internal/handler/webhook/line_webhook.go +++ b/internal/handler/webhook/line_webhook.go @@ -79,8 +79,8 @@ func (h *LineWebhookHandler) HandleLineWebhook(c *gin.Context) { } signature := c.GetHeader("X-Line-Signature") - if channelSecret != "" && signature != "" { - if !h.service.VerifySignature(channelSecret, string(body), signature) { + if channelSecret != "" { + if signature == "" || h.service == nil || !h.service.VerifySignature(channelSecret, string(body), signature) { applogger.L().Warnf("LINE webhook: invalid signature for inbox=%d", inbox.ID) c.JSON(http.StatusUnauthorized, gin.H{"error": "invalid signature"}) return diff --git a/internal/handler/webhook/webhook_lookup_test.go b/internal/handler/webhook/webhook_lookup_test.go index 9559ca2c..9c2d8608 100644 --- a/internal/handler/webhook/webhook_lookup_test.go +++ b/internal/handler/webhook/webhook_lookup_test.go @@ -192,6 +192,12 @@ func shopifyHMAC(secret string, body []byte) string { return base64.StdEncoding.EncodeToString(mac.Sum(nil)) } +func lineSignature(secret string, body []byte) string { + mac := hmac.New(sha256.New, []byte(secret)) + mac.Write(body) + return base64.StdEncoding.EncodeToString(mac.Sum(nil)) +} + func metaSignature(secret string, body []byte) string { mac := hmac.New(sha256.New, []byte(secret)) mac.Write(body) @@ -306,6 +312,10 @@ func TestLineWebhookPersistsIncomingMessage(t *testing.T) { gin.SetMode(gin.TestMode) db := newWebhookLookupTestDB(t) inbox := seedWebhookInbox(t, db, "line") + inbox.ChannelConfig = `{"channel_secret":"line-secret"}` + if err := db.Save(&inbox).Error; err != nil { + t.Fatalf("update line inbox config: %v", err) + } channelRecord := channelmodel.ChannelLINE{AccountID: 1, InboxID: inbox.ID, ChannelID: "line-channel-1", Name: "LINE OA"} if err := db.Create(&channelRecord).Error; err != nil { t.Fatalf("create line channel: %v", err) @@ -320,6 +330,7 @@ func TestLineWebhookPersistsIncomingMessage(t *testing.T) { body := []byte(`{"destination":"line-channel-1","events":[{"type":"message","replyToken":"reply-1","timestamp":1710000000000,"source":{"type":"user","userId":"line-user-1"},"message":{"type":"text","id":"line-msg-1","text":"hello line"}}]}`) req := httptest.NewRequest(http.MethodPost, "/webhooks/line/line-channel-1", bytes.NewReader(body)) req.Header.Set("Content-Type", "application/json") + req.Header.Set("X-Line-Signature", lineSignature("line-secret", body)) w := httptest.NewRecorder() r.ServeHTTP(w, req) @@ -330,6 +341,44 @@ func TestLineWebhookPersistsIncomingMessage(t *testing.T) { assertPersistedMessage(t, db, inbox.ID, "line-msg-1", "hello line") } +func TestLineWebhookRejectsMissingSignatureWhenSecretConfigured(t *testing.T) { + gin.SetMode(gin.TestMode) + db := newWebhookLookupTestDB(t) + inbox := seedWebhookInbox(t, db, "line") + inbox.ChannelConfig = `{"channel_secret":"line-secret"}` + if err := db.Save(&inbox).Error; err != nil { + t.Fatalf("update line inbox config: %v", err) + } + channelRecord := channelmodel.ChannelLINE{AccountID: 1, InboxID: inbox.ID, ChannelID: "line-channel-1", Name: "LINE OA"} + if err := db.Create(&channelRecord).Error; err != nil { + t.Fatalf("create line channel: %v", err) + } + + lineRepo := linechannel.NewRepository(db) + lineService := linechannel.NewLineService(lineRepo) + linePipeline := linechannel.NewIncomingProcessor(lineService) + h := NewLineWebhookHandler(nil, linePipeline, lineService, db) + r := gin.New() + r.POST("/webhooks/line/:line_channel_id", h.HandleLineWebhook) + body := []byte(`{"destination":"line-channel-1","events":[{"type":"message","replyToken":"reply-1","timestamp":1710000000000,"source":{"type":"user","userId":"line-user-1"},"message":{"type":"text","id":"line-msg-missing-sig","text":"hello line"}}]}`) + req := httptest.NewRequest(http.MethodPost, "/webhooks/line/line-channel-1", bytes.NewReader(body)) + req.Header.Set("Content-Type", "application/json") + 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, "line-msg-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 TestTwilioWebhookLookupInboxByPhoneNumber(t *testing.T) { db := newWebhookLookupTestDB(t) inbox := seedWebhookInbox(t, db, "twilio_sms")