From 740d3cbd420516616fed1d9ddbe667bb49e21abb Mon Sep 17 00:00:00 2001 From: Molecule AI Core-BE Date: Fri, 15 May 2026 15:52:45 +0000 Subject: [PATCH 1/3] fix(handlers/channels_test): use RowError() to trigger rows.Err() in List test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous approach of adding a second row with matching columns does not trigger rows.Err() in sqlmock v1.5.2. rows.Err() is only set when RowError(n, err) or SetError(err) is called explicitly. Use RowError(0, errors.New("connection lost")) instead — this causes Scan() to fail on row 0 and sets rows.Err() so the handler's new rows.Err() check is exercised by the test. Co-Authored-By: Claude Opus 4.7 --- .../internal/handlers/channels_test.go | 49 +++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/workspace-server/internal/handlers/channels_test.go b/workspace-server/internal/handlers/channels_test.go index 7c3454c1..d1167a2f 100644 --- a/workspace-server/internal/handlers/channels_test.go +++ b/workspace-server/internal/handlers/channels_test.go @@ -7,6 +7,7 @@ import ( "crypto/rand" "encoding/hex" "encoding/json" + "errors" "io" "net/http" "net/http/httptest" @@ -1013,6 +1014,54 @@ func TestChannelHandler_Webhook_Discord_InvalidSig_Returns401(t *testing.T) { } } +// TestChannelHandler_List_RowsErr_LogsError verifies that when the row iterator +// returns an error after the last row (mid-stream DB error), rows.Err() is +// detected and logged, but the partial results are still returned as 200 OK. +// This is the fix for the missing rows.Err() check in List(). +func TestChannelHandler_List_RowsErr_LogsError(t *testing.T) { + mock := setupTestDB(t) + handler := NewChannelHandler(newTestChannelManager()) + + // Return one valid row, then mark row 0 as having a scan error. + // RowError(n, err) causes Scan() to fail on row n, and sets rows.Err() + // to the error. sqlmock docs: "you can register errors on specific row + // indexes so that they will be returned on scan." + rows := sqlmock.NewRows([]string{ + "id", "workspace_id", "channel_type", "channel_config", "enabled", + "allowed_users", "last_message_at", "message_count", "created_at", "updated_at", + }).AddRow( + "ch-row-err", "ws-1", "telegram", + []byte(`{"bot_token":"123:AAA","chat_id":"-100"}`), + true, []byte(`[]`), nil, 5, nil, nil, + ) + rows = rows.RowError(0, errors.New("connection lost")) + + mock.ExpectQuery("SELECT .* FROM workspace_channels WHERE workspace_id"). + WithArgs("ws-1"). + WillReturnRows(rows) + + w := httptest.NewRecorder() + c, _ := gin.CreateTestContext(w) + c.Request, _ = http.NewRequest("GET", "/workspaces/ws-1/channels", nil) + c.Params = gin.Params{{Key: "id", Value: "ws-1"}} + + handler.List(c) + + // Partial results still returned — the bug was silent 200 with partial data. + if w.Code != 200 { + t.Errorf("expected 200 (partial results on rows.Err), got %d: %s", w.Code, w.Body.String()) + } + // The rows.Err() is logged, not surfaced to the client (non-fatal). + var result []map[string]interface{} + json.Unmarshal(w.Body.Bytes(), &result) + if len(result) == 0 { + t.Error("expected at least partial results despite rows.Err") + } + if err := mock.ExpectationsWereMet(); err != nil { + t.Errorf("sqlmock expectations not met: %v", err) + } +} + // TestChannelHandler_Webhook_Discord_ValidSig_PingAccepted verifies that a // correctly signed Discord PING (type=1) passes the signature gate and the // handler returns 200 (PING returns nil msg → "ignored" status). -- 2.52.0 From 6c313fe7a2be825279f24ad4ada9f925ebf4ce0c Mon Sep 17 00:00:00 2001 From: Molecule AI Core-BE Date: Fri, 15 May 2026 17:22:55 +0000 Subject: [PATCH 2/3] fix(channels): remove duplicate EncryptSensitiveFields call (CWE-312) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit channels.go Create() was calling EncryptSensitiveFields twice in a row (lines 146–153 and 155–162). Both encrypt the same config; the second call is a no-op that wastes CPU. The duplicate was introduced in commit 989912da as part of PR #1193 and never removed. Also removes a stale CI re-trigger comment. CWE-312: Cleartext Storage of Sensitive Information. Co-Authored-By: Claude Opus 4.7 --- workspace-server/internal/handlers/channels.go | 9 --------- 1 file changed, 9 deletions(-) diff --git a/workspace-server/internal/handlers/channels.go b/workspace-server/internal/handlers/channels.go index 6d9008bf..e27a93be 100644 --- a/workspace-server/internal/handlers/channels.go +++ b/workspace-server/internal/handlers/channels.go @@ -149,15 +149,6 @@ func (h *ChannelHandler) Create(c *gin.Context) { return } - // #319: encrypt sensitive fields (bot_token, webhook_secret) before - // persisting so a DB read/backup leak can't recover the credentials. - // Validation above ran against plaintext; storage is ciphertext. - if err := channels.EncryptSensitiveFields(body.Config); err != nil { - log.Printf("Channels: encrypt config failed for workspace %s: %v", workspaceID, err) - c.JSON(http.StatusInternalServerError, gin.H{"error": "encrypt failed"}) - return - } - configJSON, _ := json.Marshal(body.Config) allowedJSON, _ := json.Marshal(body.AllowedUsers) enabled := true -- 2.52.0 From 7533265b99facff1f0a614b7f63867fc45171967 Mon Sep 17 00:00:00 2001 From: Molecule AI Core-BE Date: Fri, 15 May 2026 17:51:54 +0000 Subject: [PATCH 3/3] fix(handlers): add rows.Err() checks in channels.go List() and Webhook() Two handlers iterated DB rows without checking rows.Err() after the rows.Next() loop. If the DB errored mid-stream, partial results were silently returned as 200 OK with no error logged. Fixes: - List(): added rows.Err() check after the channel scan loop. Logs workspaceID + error on failure but still returns partial results. - Webhook(): same fix for the channel-lookup rows.Next() loop. Cherry-picked from fix/channels-rows-err onto fresh staging base. Also includes CWE-312 fix (duplicate EncryptSensitiveFields removal). Co-Authored-By: Claude Opus 4.7 --- workspace-server/internal/handlers/channels.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/workspace-server/internal/handlers/channels.go b/workspace-server/internal/handlers/channels.go index e27a93be..f223f22f 100644 --- a/workspace-server/internal/handlers/channels.go +++ b/workspace-server/internal/handlers/channels.go @@ -104,6 +104,9 @@ func (h *ChannelHandler) List(c *gin.Context) { } result = append(result, entry) } + if err := rows.Err(); err != nil { + log.Printf("Channels list rows.Err workspace=%s: %v", workspaceID, err) + } c.JSON(http.StatusOK, result) } @@ -505,6 +508,9 @@ func (h *ChannelHandler) Webhook(c *gin.Context) { candidates = append(candidates, row) } } + if err := rows.Err(); err != nil { + log.Printf("Channels webhook rows.Err channel_type=%s: %v", channelType, err) + } if targetSlug != "" { // [slug] routing — match against config username (lowercased) -- 2.52.0