From 18ecee3e460fd18f27728d5f4fefdc064ba2f26e Mon Sep 17 00:00:00 2001 From: Rogee Date: Sat, 15 Aug 2026 01:15:04 +0800 Subject: [PATCH] H-116: close visitor payload trust boundaries (#19) * H-116: close visitor payload trust boundaries * H-129: unblock SQLite backend tests * H-129: remove stale last-seen response assertions --------- Co-authored-by: Rogee --- backend/internal/channel/listener.go | 4 +- .../internal/channel/webhook_listener_test.go | 28 ++++++++ .../internal/handler/widget/widget_handler.go | 8 +-- .../handler/widget/widget_handler_test.go | 23 +++++++ backend/internal/handler/ws/hub.go | 22 +++++-- backend/internal/handler/ws/ws_test.go | 55 ++++++++++++++++ .../internal/service/agent_bot_listener.go | 3 +- .../service/agent_bot_listener_test.go | 18 ++++- backend/internal/webhookutil/sanitize.go | 66 +++++++++++++++++++ 9 files changed, 211 insertions(+), 16 deletions(-) create mode 100644 backend/internal/webhookutil/sanitize.go diff --git a/backend/internal/channel/listener.go b/backend/internal/channel/listener.go index 182e5483..64ed07f1 100644 --- a/backend/internal/channel/listener.go +++ b/backend/internal/channel/listener.go @@ -435,10 +435,10 @@ func webhookEventPayload(event *ChannelEvent) map[string]interface{} { "account_id": event.AccountID, "inbox_id": event.InboxID, "timestamp": event.Timestamp, - "data": event.Data, + "data": webhookutil.SanitizeOutbound(event.Data), } if changed := normalizedChangedAttributes(event.Data["changed_attributes"]); len(changed) > 0 { - payload["changed_attributes"] = changed + payload["changed_attributes"] = webhookutil.SanitizeOutbound(changed) } return payload } diff --git a/backend/internal/channel/webhook_listener_test.go b/backend/internal/channel/webhook_listener_test.go index 6508eeb2..009d449c 100644 --- a/backend/internal/channel/webhook_listener_test.go +++ b/backend/internal/channel/webhook_listener_test.go @@ -17,6 +17,7 @@ import ( "github.com/google/uuid" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "gorm.io/datatypes" "gorm.io/driver/sqlite" "gorm.io/gorm" "gorm.io/gorm/logger" @@ -45,6 +46,33 @@ func newWebhookListenerTestDB(t *testing.T) *gorm.DB { return db } +func TestWebhookEventPayloadOmitsAIIdentity(t *testing.T) { + senderID := uint(7) + botID := uint(8) + payload := webhookEventPayload(&ChannelEvent{ + Type: EventMessageCreated, + Data: map[string]interface{}{ + "message": &model.Message{ + SenderID: &senderID, + SenderType: string(model.SenderTypeAgentBot), + AITakeoverExited: true, + AdditionalAttributes: datatypes.JSON([]byte(`{"agent_name":"Captain","sender_name":"Captain"}`)), + }, + "conversation": &model.Conversation{AssigneeAgentBotID: &botID, AdditionalAttributes: datatypes.JSON([]byte(`{"swt_baidu_agent_name":"Captain"}`))}, + "performer": map[string]interface{}{"name": "Captain", "type": "AgentBot"}, + "changed_attributes": map[string]interface{}{ + "assignee_agent_bot_id": []interface{}{nil, float64(botID)}, + }, + }, + }) + + raw, err := json.Marshal(payload) + require.NoError(t, err) + for _, marker := range []string{"AgentBot", "sender_id", "sender_type", "assignee_agent_bot_id", "ai_takeover_exited", "agent_name", "sender_name", "swt_baidu_agent_name", "performer", "Captain"} { + assert.NotContains(t, string(raw), marker) + } +} + func TestWebhookListenerDeliversAPIInboxWebhook(t *testing.T) { db := newWebhookListenerTestDB(t) secret := "api-webhook-secret" diff --git a/backend/internal/handler/widget/widget_handler.go b/backend/internal/handler/widget/widget_handler.go index ef414fe6..659a4149 100644 --- a/backend/internal/handler/widget/widget_handler.go +++ b/backend/internal/handler/widget/widget_handler.go @@ -11,6 +11,7 @@ import ( "github.com/gochat/gochat/internal/model" "github.com/gochat/gochat/internal/service" + "github.com/gochat/gochat/internal/webhookutil" wspkg "github.com/gochat/gochat/internal/ws" ) @@ -1323,7 +1324,7 @@ func widgetMessagePayload(message model.Message, conversationID uint) gin.H { "conversation_id": conversationID, "message_type": message.MessageType, "content_type": message.ContentType, - "content_attributes": message.ContentAttributes, + "content_attributes": webhookutil.SanitizeOutboundJSON(message.ContentAttributes), "created_at": message.CreatedAt.Unix(), "private": message.Private, "source_id": message.SourceID, @@ -1513,13 +1514,10 @@ func publicMessagePayload(message model.Message, conversation model.Conversation "content": message.Content, "message_type": message.MessageType, "content_type": message.ContentType, - "content_attributes": message.ContentAttributes, + "content_attributes": webhookutil.SanitizeOutboundJSON(message.ContentAttributes), "created_at": message.CreatedAt.Unix(), "conversation_id": publicDisplayID(conversation), } - if message.SenderID != nil { - payload["sender"] = gin.H{"id": *message.SenderID, "type": message.SenderType} - } return payload } diff --git a/backend/internal/handler/widget/widget_handler_test.go b/backend/internal/handler/widget/widget_handler_test.go index 836efe4b..f854a187 100644 --- a/backend/internal/handler/widget/widget_handler_test.go +++ b/backend/internal/handler/widget/widget_handler_test.go @@ -50,6 +50,29 @@ type recordingWidgetEventPublisher struct { payload interface{} } +func TestPublicMessagePayloadSanitizesIdentity(t *testing.T) { + senderID := uint(9) + message := model.Message{ + SenderID: &senderID, + SenderType: string(model.SenderTypeAgentBot), + Content: "same reply", + ContentAttributes: datatypes.JSON(`[{"business":"keep","nested":{"sender_name":"Captain","ai_takeover_active":true}}]`), + } + + publicPayload := publicMessagePayload(message, model.Conversation{Base: model.Base{ID: 1}}) + assert.NotContains(t, publicPayload, "sender") + assert.Equal(t, "same reply", publicPayload["content"]) + assert.Equal(t, []any{map[string]any{"business": "keep", "nested": map[string]any{}}}, publicPayload["content_attributes"]) + message.SenderType = string(model.SenderTypeUser) + assert.Equal(t, publicPayload, publicMessagePayload(message, model.Conversation{Base: model.Base{ID: 1}})) + + widgetPayload := widgetMessagePayload(message, 1) + assert.Equal(t, publicPayload["content_attributes"], widgetPayload["content_attributes"]) + + message.ContentAttributes = datatypes.JSON(`{"sender_name":`) + assert.Empty(t, publicMessagePayload(message, model.Conversation{})["content_attributes"]) +} + func (p *recordingWidgetEventPublisher) PublishWidgetEvent(accountID uint, pubsubToken string, eventType string, payload interface{}) { p.accountID = accountID p.pubsubToken = pubsubToken diff --git a/backend/internal/handler/ws/hub.go b/backend/internal/handler/ws/hub.go index 882ac9f1..9fc50cc3 100644 --- a/backend/internal/handler/ws/hub.go +++ b/backend/internal/handler/ws/hub.go @@ -14,6 +14,7 @@ import ( "github.com/gorilla/websocket" + "github.com/gochat/gochat/internal/webhookutil" wspkg "github.com/gochat/gochat/internal/ws" "github.com/gochat/gochat/pkg/logger" ) @@ -282,7 +283,7 @@ func (h *Hub) SendToAccount(accountID uint, data []byte) { if clientIDs, ok := h.rooms[roomName]; ok { for clientID := range clientIDs { if client, ok := h.clients[clientID]; ok { - msg := wrapActionCableMessage(client.Identifier, data) + msg := wrapActionCableMessage(client.Identifier, eventDataForClient(client, data)) if msg == nil { continue } @@ -306,7 +307,7 @@ func (h *Hub) SendToAccountConversation(accountID uint, conversationID uint, dat if clientIDs, ok := h.rooms[roomName]; ok { for clientID := range clientIDs { if client, ok := h.clients[clientID]; ok { - msg := wrapActionCableMessage(client.Identifier, data) + msg := wrapActionCableMessage(client.Identifier, eventDataForClient(client, data)) if msg == nil { continue } @@ -320,6 +321,17 @@ func (h *Hub) SendToAccountConversation(accountID uint, conversationID uint, dat } } +func eventDataForClient(client *Client, data []byte) []byte { + if !client.IsContact { + return data + } + payload, err := json.Marshal(webhookutil.SanitizeOutboundJSON(data)) + if err != nil { + return []byte(`{}`) + } + return payload +} + // SendToRoom sends a message to all clients in a named room. func (h *Hub) SendToRoom(room string, data []byte) { h.mu.RLock() @@ -328,7 +340,7 @@ func (h *Hub) SendToRoom(room string, data []byte) { if clientIDs, ok := h.rooms[room]; ok { for clientID := range clientIDs { if client, ok := h.clients[clientID]; ok { - msg := wrapActionCableMessage(client.Identifier, data) + msg := wrapActionCableMessage(client.Identifier, eventDataForClient(client, data)) if msg == nil { continue } @@ -348,7 +360,7 @@ func (h *Hub) SendToClient(clientID string, data []byte) { defer h.mu.RUnlock() if client, ok := h.clients[clientID]; ok { - msg := wrapActionCableMessage(client.Identifier, data) + msg := wrapActionCableMessage(client.Identifier, eventDataForClient(client, data)) if msg == nil { return } @@ -624,4 +636,4 @@ func conversationRoomName(accountID uint, conversationID uint) string { func pubsubTokenRoomName(token string) string { return fmt.Sprintf("pubsub_token_%s", token) -} \ No newline at end of file +} diff --git a/backend/internal/handler/ws/ws_test.go b/backend/internal/handler/ws/ws_test.go index d8493771..93f5af76 100644 --- a/backend/internal/handler/ws/ws_test.go +++ b/backend/internal/handler/ws/ws_test.go @@ -255,6 +255,61 @@ func TestHub_SendToAccount(t *testing.T) { } } +func TestHub_SendToAccountSanitizesVisitorIdentity(t *testing.T) { + hub := NewHubSimple() + agent := NewClient(1, 10, nil, hub) + agent.Identifier = `{"channel":"AccountChannel","account_id":10}` + hub.Register(agent) + visitor := NewClient(2, 10, nil, hub) + visitor.IsContact = true + visitor.Identifier = `{"channel":"AccountChannel","account_id":10}` + hub.Register(visitor) + + hub.SendToAccount(10, []byte(`{"event":"message.created","data":{"content":"same reply","sender_type":"AgentBot","sender_id":7,"ai_takeover_active":true,"additional_attributes":{"agent_name":"Captain"}}}`)) + + var agentMessage, visitorMessage []byte + select { + case agentMessage = <-agent.Send: + default: + t.Fatal("expected agent event") + } + select { + case visitorMessage = <-visitor.Send: + default: + t.Fatal("expected visitor event") + } + assert.Contains(t, string(agentMessage), "AgentBot") + assert.NotContains(t, string(visitorMessage), "AgentBot") + assert.NotContains(t, string(visitorMessage), "sender_id") + assert.NotContains(t, string(visitorMessage), "sender_type") + assert.NotContains(t, string(visitorMessage), "ai_takeover_active") + assert.NotContains(t, string(visitorMessage), "agent_name") + assert.NotContains(t, string(visitorMessage), "Captain") +} + +func TestHub_VisitorRoomAndClientDeliverySanitizeIdentity(t *testing.T) { + hub := NewHubSimple() + visitor := NewClient(2, 10, nil, hub) + visitor.IsContact = true + visitor.Identifier = `{"channel":"RoomChannel"}` + hub.Register(visitor) + hub.subscribeClient(visitor.ID, "visitor-room") + visitor.SubscribedRooms["visitor-room"] = true + data := []byte(`{"event":"message.created","data":{"sender_type":"Captain::Assistant","sender_name":"Captain"}}`) + + hub.SendToRoom("visitor-room", data) + hub.SendToClient(visitor.ID, data) + for range 2 { + select { + case message := <-visitor.Send: + assert.NotContains(t, string(message), "Captain") + assert.NotContains(t, string(message), "sender_type") + default: + t.Fatal("expected sanitized visitor event") + } + } +} + func TestHub_SendToAccount_NoClients(t *testing.T) { hub := NewHubSimple() hub.SendToAccount(10, []byte(`{"event":"test"}`)) // should not panic diff --git a/backend/internal/service/agent_bot_listener.go b/backend/internal/service/agent_bot_listener.go index d69807b2..62a66531 100644 --- a/backend/internal/service/agent_bot_listener.go +++ b/backend/internal/service/agent_bot_listener.go @@ -294,7 +294,8 @@ func agentBotWebhookPayload(eventType string, accountID uint, inboxID uint, data if _, ok := payload["inbox_id"]; !ok { payload["inbox_id"] = inboxID } - return payload + sanitized, _ := webhookutil.SanitizeOutbound(payload).(map[string]interface{}) + return sanitized } // pushToBot sends the webhook payload to a single bot's outgoing_url with Chatwoot webhook headers. diff --git a/backend/internal/service/agent_bot_listener_test.go b/backend/internal/service/agent_bot_listener_test.go index 2df047d8..c04d2557 100644 --- a/backend/internal/service/agent_bot_listener_test.go +++ b/backend/internal/service/agent_bot_listener_test.go @@ -106,10 +106,15 @@ func TestAgentBotListenerSendsChatwootStyleTopLevelPayload(t *testing.T) { listener := NewAgentBotListener(repository.NewAgentBotInboxRepo(db), repository.NewAgentBotRepo(db), nil, nil) err := listener.HandleEvent(context.Background(), "message_created", account.ID, inbox.ID, map[string]interface{}{ - "id": float64(123), - "content": "hello bot", + "id": float64(123), + "content": "hello bot", + "sender_id": float64(7), + "sender_type": "AgentBot", + "ai_takeover_active": true, + "content_attributes": []interface{}{map[string]interface{}{"business": "keep", "sender_name": "Captain"}}, "conversation": map[string]interface{}{ - "id": float64(456), + "id": float64(456), + "assignee_agent_bot_id": float64(8), }, }) require.NoError(t, err) @@ -122,6 +127,13 @@ func TestAgentBotListenerSendsChatwootStyleTopLevelPayload(t *testing.T) { assert.Equal(t, "hello bot", received["content"]) assert.NotContains(t, received, "data") assert.NotContains(t, received, "timestamp") + assert.NotContains(t, string(receivedBody), "AgentBot") + assert.NotContains(t, string(receivedBody), "Captain") + assert.NotContains(t, string(receivedBody), "sender_") + assert.NotContains(t, string(receivedBody), "ai_takeover_") + assert.NotContains(t, string(receivedBody), "assignee_agent_bot_id") + attributes := received["content_attributes"].([]interface{}) + assert.Equal(t, map[string]interface{}{"business": "keep"}, attributes[0]) assert.NotEmpty(t, receivedDelivery) assert.Regexp(t, `^\d+$`, receivedTimestamp) mac := hmac.New(sha256.New, []byte("secret")) diff --git a/backend/internal/webhookutil/sanitize.go b/backend/internal/webhookutil/sanitize.go new file mode 100644 index 00000000..679fe0c6 --- /dev/null +++ b/backend/internal/webhookutil/sanitize.go @@ -0,0 +1,66 @@ +package webhookutil + +import ( + "encoding/json" + "strings" +) + +var privateOutboundFields = map[string]struct{}{ + "assignee": {}, + "assignee_agent_bot_id": {}, + "agent_bot_id": {}, + "agent_name": {}, + "assistant_id": {}, + "assistant_name": {}, + "bot_type": {}, + "captain_assistant_id": {}, + "performer": {}, + "swt_baidu_agent_name": {}, + "swt_baidu_conversation_type": {}, +} + +// SanitizeOutbound removes internal sender and AI identity from an external payload. +func SanitizeOutbound(value any) any { + raw, err := json.Marshal(value) + if err != nil { + return map[string]any{} + } + return SanitizeOutboundJSON(raw) +} + +// SanitizeOutboundJSON decodes and sanitizes an external JSON object or array. +func SanitizeOutboundJSON(raw []byte) any { + var value any + if len(raw) == 0 || json.Unmarshal(raw, &value) != nil { + return map[string]any{} + } + switch value.(type) { + case map[string]any, []any: + return sanitizeOutboundValue(value) + default: + return map[string]any{} + } +} + +func sanitizeOutboundValue(value any) any { + switch value := value.(type) { + case map[string]any: + clean := make(map[string]any, len(value)) + for key, nested := range value { + _, private := privateOutboundFields[key] + if private || strings.HasPrefix(key, "sender") || strings.HasPrefix(key, "ai_takeover_") { + continue + } + clean[key] = sanitizeOutboundValue(nested) + } + return clean + case []any: + clean := make([]any, len(value)) + for i, nested := range value { + clean[i] = sanitizeOutboundValue(nested) + } + return clean + default: + return value + } +}