From afc282c5568ff71e3df3efad9e7299856da78f05 Mon Sep 17 00:00:00 2001 From: Molecule AI Fullstack Engineer Date: Fri, 15 May 2026 04:18:09 +0000 Subject: [PATCH] fix(channels): remove duplicate EncryptSensitiveFields + log json.Marshal errors MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Create: remove the duplicate EncryptSensitiveFields call introduced during OFFSEC-010 conflict resolution (commit 58882381). The function is idempotent so impact was nil (wasted CPU), but the block was dead code. - Create: add error checks on json.Marshal for config and allowed_users (previously j, _ := json.Marshal(...) silently dropped marshal failures). - Update: add error checks on json.Marshal for config and allowed_users (same silent-discard pattern). Also renamed the EncryptSensitiveFields error variable to encErr to avoid shadowing the outer err used by db.DB.ExecContext. This PR complements #1109 (merge-queue: json.Unmarshal + rows.Err) and #1110 (duplicate EncryptSensitiveFields removal) — those two cover the same channels.go surface but neither adds json.Marshal error checks. Co-Authored-By: Claude Opus 4.7 --- .../internal/handlers/channels.go | 37 ++++++++++++------- 1 file changed, 24 insertions(+), 13 deletions(-) diff --git a/workspace-server/internal/handlers/channels.go b/workspace-server/internal/handlers/channels.go index 6d9008bf5..ca5e9890b 100644 --- a/workspace-server/internal/handlers/channels.go +++ b/workspace-server/internal/handlers/channels.go @@ -149,17 +149,18 @@ 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"}) + configJSON, mErr := json.Marshal(body.Config) + if mErr != nil { + log.Printf("Channels Create: marshal config for workspace %s: %v", workspaceID, mErr) + c.JSON(http.StatusInternalServerError, gin.H{"error": "marshal config failed"}) + return + } + allowedJSON, mErr := json.Marshal(body.AllowedUsers) + if mErr != nil { + log.Printf("Channels Create: marshal allowed_users for workspace %s: %v", workspaceID, mErr) + c.JSON(http.StatusInternalServerError, gin.H{"error": "marshal allowed_users failed"}) return } - - configJSON, _ := json.Marshal(body.Config) - allowedJSON, _ := json.Marshal(body.AllowedUsers) enabled := true if body.Enabled != nil { enabled = *body.Enabled @@ -209,16 +210,26 @@ func (h *ChannelHandler) Update(c *gin.Context) { // #319: re-encrypt sensitive fields on every config update — the // PATCH body carries plaintext (client already had them plaintext in // List response's unmasked path or typed fresh). - if err := channels.EncryptSensitiveFields(body.Config); err != nil { - log.Printf("Channels: encrypt update for workspace %s: %v", workspaceID, err) + if encErr := channels.EncryptSensitiveFields(body.Config); encErr != nil { + log.Printf("Channels: encrypt update for workspace %s: %v", workspaceID, encErr) c.JSON(http.StatusInternalServerError, gin.H{"error": "encrypt failed"}) return } - j, _ := json.Marshal(body.Config) + j, mErr := json.Marshal(body.Config) + if mErr != nil { + log.Printf("Channels Update: marshal config for channel %s: %v", channelID, mErr) + c.JSON(http.StatusInternalServerError, gin.H{"error": "marshal config failed"}) + return + } configArg = string(j) } if body.AllowedUsers != nil { - j, _ := json.Marshal(body.AllowedUsers) + j, mErr := json.Marshal(body.AllowedUsers) + if mErr != nil { + log.Printf("Channels Update: marshal allowed_users for channel %s: %v", channelID, mErr) + c.JSON(http.StatusInternalServerError, gin.H{"error": "marshal allowed_users failed"}) + return + } allowedArg = string(j) } -- 2.52.0