From e179485a0ea2668547b268ab4ea8f48444b8de7f Mon Sep 17 00:00:00 2001 From: "Molecule AI Dev Engineer A (Kimi)" Date: Tue, 26 May 2026 11:40:39 +0000 Subject: [PATCH 1/3] fix(channels,messagestore): log json.Unmarshal errors instead of silently dropping them Four production sites were ignoring json.Unmarshal return values: - channels.go List+Webhook: corrupt JSON rows would produce empty config/allowed_users without any signal. - manager.go FetchWorkspaceChannelContext: empty config would fall through to DecryptSensitiveFields failure, masking the root cause. - messagestore extractFilesFromResponse: _ = json.Unmarshal discarded parse errors on the probe wrapper. All four now log the error at the point of failure so operators can spot data-corruption or schema-drift incidents. --- workspace-server/internal/channels/manager.go | 5 ++++- .../internal/handlers/channels.go | 20 +++++++++++++++---- .../internal/messagestore/postgres_store.go | 5 ++++- 3 files changed, 24 insertions(+), 6 deletions(-) diff --git a/workspace-server/internal/channels/manager.go b/workspace-server/internal/channels/manager.go index 3de98c912..4e74c677b 100644 --- a/workspace-server/internal/channels/manager.go +++ b/workspace-server/internal/channels/manager.go @@ -515,7 +515,10 @@ func (m *Manager) FetchWorkspaceChannelContext(ctx context.Context, workspaceID return "" } var config map[string]interface{} - json.Unmarshal(configJSON, &config) + if err := json.Unmarshal(configJSON, &config); err != nil { + log.Printf("ChannelManager: unmarshal config: %v", err) + return "" + } if err := DecryptSensitiveFields(config); err != nil { return "" } diff --git a/workspace-server/internal/handlers/channels.go b/workspace-server/internal/handlers/channels.go index b7167720e..b6ae79e3e 100644 --- a/workspace-server/internal/handlers/channels.go +++ b/workspace-server/internal/handlers/channels.go @@ -67,7 +67,10 @@ func (h *ChannelHandler) List(c *gin.Context) { } var config map[string]interface{} - json.Unmarshal(configJSON, &config) + if err := json.Unmarshal(configJSON, &config); err != nil { + log.Printf("Channels: unmarshal config for channel %s: %v", id, err) + continue + } // #319: decrypt sensitive fields first so the mask operates on // plaintext (first-4 / last-4 of the real token, not the ciphertext // prefix). Decrypt errors are logged but non-fatal — List must keep @@ -86,7 +89,10 @@ func (h *ChannelHandler) List(c *gin.Context) { } var allowed []string - json.Unmarshal(allowedJSON, &allowed) + if err := json.Unmarshal(allowedJSON, &allowed); err != nil { + log.Printf("Channels: unmarshal allowed_users for channel %s: %v", id, err) + continue + } entry := map[string]interface{}{ "id": id, @@ -499,8 +505,14 @@ func (h *ChannelHandler) Webhook(c *gin.Context) { if err := rows.Scan(&row.ID, &row.WorkspaceID, &row.ChannelType, &configJSON, &row.Enabled, &allowedJSON); err != nil { continue } - json.Unmarshal(configJSON, &row.Config) - json.Unmarshal(allowedJSON, &row.AllowedUsers) + if err := json.Unmarshal(configJSON, &row.Config); err != nil { + log.Printf("Channels: unmarshal config for webhook row %s: %v", row.ID, err) + continue + } + if err := json.Unmarshal(allowedJSON, &row.AllowedUsers); err != nil { + log.Printf("Channels: unmarshal allowed_users for webhook row %s: %v", row.ID, err) + continue + } if err := channels.DecryptSensitiveFields(row.Config); err != nil { log.Printf("Channels: decrypt webhook row %s: %v", row.ID, err) continue diff --git a/workspace-server/internal/messagestore/postgres_store.go b/workspace-server/internal/messagestore/postgres_store.go index 67987569c..cf1ef7ad5 100644 --- a/workspace-server/internal/messagestore/postgres_store.go +++ b/workspace-server/internal/messagestore/postgres_store.go @@ -15,6 +15,7 @@ import ( "context" "database/sql" "encoding/json" + "log" "path" "strings" "time" @@ -391,7 +392,9 @@ func extractFilesFromResponse(body json.RawMessage) []ChatAttachment { var probe struct { Result json.RawMessage `json:"result"` } - _ = json.Unmarshal(body, &probe) + if err := json.Unmarshal(body, &probe); err != nil { + log.Printf("messagestore: unmarshal probe body: %v", err) + } feed := body if len(probe.Result) > 0 { trimmed := bytesTrimSpace(probe.Result) -- 2.52.0 From 212842bc3f797245694c80c7054fe2ea7afad695 Mon Sep 17 00:00:00 2001 From: "Molecule AI Dev Engineer A (Kimi)" Date: Tue, 26 May 2026 11:44:35 +0000 Subject: [PATCH 2/3] fix(channels,messagestore): preserve previous flow after logging json.Unmarshal errors Addresses review feedback on PR #1899: the stated scope is observability/ no behavior change, but the initial diff changed control flow on unmarshal failures (continue/return instead of proceeding with zero values). This revision keeps the error logging but restores the previous flow so that List/Webhook proceed with nil config/allowedUsers and FetchWorkspaceChannelContext proceeds to DecryptSensitiveFields, exactly as the bare `_ = json.Unmarshal` did before. Refs PR #1899 Co-Authored-By: Claude Opus 4.7 --- workspace-server/internal/channels/manager.go | 1 - workspace-server/internal/handlers/channels.go | 4 ---- 2 files changed, 5 deletions(-) diff --git a/workspace-server/internal/channels/manager.go b/workspace-server/internal/channels/manager.go index 4e74c677b..f84816742 100644 --- a/workspace-server/internal/channels/manager.go +++ b/workspace-server/internal/channels/manager.go @@ -517,7 +517,6 @@ func (m *Manager) FetchWorkspaceChannelContext(ctx context.Context, workspaceID var config map[string]interface{} if err := json.Unmarshal(configJSON, &config); err != nil { log.Printf("ChannelManager: unmarshal config: %v", err) - return "" } if err := DecryptSensitiveFields(config); err != nil { return "" diff --git a/workspace-server/internal/handlers/channels.go b/workspace-server/internal/handlers/channels.go index b6ae79e3e..fea222055 100644 --- a/workspace-server/internal/handlers/channels.go +++ b/workspace-server/internal/handlers/channels.go @@ -69,7 +69,6 @@ func (h *ChannelHandler) List(c *gin.Context) { var config map[string]interface{} if err := json.Unmarshal(configJSON, &config); err != nil { log.Printf("Channels: unmarshal config for channel %s: %v", id, err) - continue } // #319: decrypt sensitive fields first so the mask operates on // plaintext (first-4 / last-4 of the real token, not the ciphertext @@ -91,7 +90,6 @@ func (h *ChannelHandler) List(c *gin.Context) { var allowed []string if err := json.Unmarshal(allowedJSON, &allowed); err != nil { log.Printf("Channels: unmarshal allowed_users for channel %s: %v", id, err) - continue } entry := map[string]interface{}{ @@ -507,11 +505,9 @@ func (h *ChannelHandler) Webhook(c *gin.Context) { } if err := json.Unmarshal(configJSON, &row.Config); err != nil { log.Printf("Channels: unmarshal config for webhook row %s: %v", row.ID, err) - continue } if err := json.Unmarshal(allowedJSON, &row.AllowedUsers); err != nil { log.Printf("Channels: unmarshal allowed_users for webhook row %s: %v", row.ID, err) - continue } if err := channels.DecryptSensitiveFields(row.Config); err != nil { log.Printf("Channels: decrypt webhook row %s: %v", row.ID, err) -- 2.52.0 From 62cbf57cb2a90a5d5d6f8f8aae1d3f1f91221cb3 Mon Sep 17 00:00:00 2001 From: "Molecule AI Dev Engineer A (Kimi)" Date: Tue, 26 May 2026 12:15:18 +0000 Subject: [PATCH 3/3] fix(channels): add rows.Err() and scan error logging in FetchWorkspaceChannelContext The FetchWorkspaceChannelContext function in manager.go was silently dropping two error paths: 1. When rows.Next() returned false due to an iteration error (not just no rows), we returned "" without logging the underlying DB error. 2. When rows.Scan failed, we returned "" without logging what went wrong, making channel-context debugging harder. This change adds log.Printf for both paths while preserving the existing fail-soft behavior (return empty string so cron prompts proceed without Slack ambient context). Co-Authored-By: Claude Opus 4.7 --- workspace-server/internal/channels/manager.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/workspace-server/internal/channels/manager.go b/workspace-server/internal/channels/manager.go index f84816742..5a30f09ce 100644 --- a/workspace-server/internal/channels/manager.go +++ b/workspace-server/internal/channels/manager.go @@ -508,10 +508,14 @@ func (m *Manager) FetchWorkspaceChannelContext(ctx context.Context, workspaceID } defer rows.Close() if !rows.Next() { + if err := rows.Err(); err != nil { + log.Printf("ChannelManager: FetchWorkspaceChannelContext rows error for %s: %v", workspaceID, err) + } return "" } var configJSON []byte - if rows.Scan(&configJSON) != nil { + if err := rows.Scan(&configJSON); err != nil { + log.Printf("ChannelManager: FetchWorkspaceChannelContext scan error for %s: %v", workspaceID, err) return "" } var config map[string]interface{} -- 2.52.0