Compare commits
1 Commits
| Author | SHA1 | Date | |
|---|---|---|---|
| 3391f9b29e |
@@ -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 List: json.Unmarshal(config) for channel %s: %v", id, err)
|
||||
config = map[string]interface{}{}
|
||||
}
|
||||
// #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 List: json.Unmarshal(allowed) for channel %s: %v", id, err)
|
||||
allowed = []string{}
|
||||
}
|
||||
|
||||
entry := map[string]interface{}{
|
||||
"id": id,
|
||||
@@ -104,6 +110,9 @@ func (h *ChannelHandler) List(c *gin.Context) {
|
||||
}
|
||||
result = append(result, entry)
|
||||
}
|
||||
if err := rows.Err(); err != nil {
|
||||
log.Printf("Channels List rows.Err: %v", err)
|
||||
}
|
||||
|
||||
c.JSON(http.StatusOK, result)
|
||||
}
|
||||
@@ -149,17 +158,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, err := json.Marshal(body.Config)
|
||||
if err != nil {
|
||||
log.Printf("Channels: marshal config for workspace %s: %v", workspaceID, err)
|
||||
c.JSON(http.StatusInternalServerError, gin.H{"error": "marshal config failed"})
|
||||
return
|
||||
}
|
||||
allowedJSON, err := json.Marshal(body.AllowedUsers)
|
||||
if err != nil {
|
||||
log.Printf("Channels: marshal allowed_users for workspace %s: %v", workspaceID, err)
|
||||
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 +219,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)
|
||||
}
|
||||
|
||||
@@ -496,8 +516,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 Webhook: json.Unmarshal(config) for row %s: %v", row.ID, err)
|
||||
continue
|
||||
}
|
||||
if err := json.Unmarshal(allowedJSON, &row.AllowedUsers); err != nil {
|
||||
log.Printf("Channels Webhook: json.Unmarshal(allowed) for row %s: %v", row.ID, err)
|
||||
row.AllowedUsers = []string{}
|
||||
}
|
||||
if err := channels.DecryptSensitiveFields(row.Config); err != nil {
|
||||
log.Printf("Channels: decrypt webhook row %s: %v", row.ID, err)
|
||||
continue
|
||||
@@ -514,6 +540,9 @@ func (h *ChannelHandler) Webhook(c *gin.Context) {
|
||||
candidates = append(candidates, row)
|
||||
}
|
||||
}
|
||||
if err := rows.Err(); err != nil {
|
||||
log.Printf("Channels Webhook rows.Err: %v", err)
|
||||
}
|
||||
|
||||
if targetSlug != "" {
|
||||
// [slug] routing — match against config username (lowercased)
|
||||
|
||||
@@ -72,6 +72,74 @@ func TestChannelHandler_ListAdapters(t *testing.T) {
|
||||
|
||||
// ==================== List ====================
|
||||
|
||||
// TestChannelHandler_List_InvalidJSONBFallsBackToEmpty exercises the graceful-degradation
|
||||
// path when the channel_config or allowed_users column contains bytes that are not valid
|
||||
// JSON. Previously json.Unmarshal errors were silently discarded, leaving config as nil (map
|
||||
// interface{}) and allowed as nil (slice interface{}). The handler now logs the error and
|
||||
// falls back to an empty map/array so the rest of the channel list is still returned.
|
||||
func TestChannelHandler_List_InvalidJSONBFallsBackToEmpty(t *testing.T) {
|
||||
mock := setupTestDB(t)
|
||||
handler := NewChannelHandler(newTestChannelManager())
|
||||
|
||||
// First row: valid config, invalid allowed_users
|
||||
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-bad-allowed", "ws-1", "telegram",
|
||||
[]byte(`{"bot_token":"123:ABC","chat_id":"-100"}`),
|
||||
true, []byte(`{not json}`), nil, 1, nil, nil,
|
||||
).AddRow(
|
||||
// Second row: invalid config, valid allowed_users
|
||||
"ch-bad-config", "ws-1", "telegram",
|
||||
[]byte(`also not json`),
|
||||
true, []byte(`["user-2"]`), nil, 2, nil, nil,
|
||||
).AddRow(
|
||||
// Third row: both valid — ensures partial corruption doesn't block the list
|
||||
"ch-good", "ws-1", "telegram",
|
||||
[]byte(`{"bot_token":"456:DEF","chat_id":"-200"}`),
|
||||
true, []byte(`["user-3"]`), nil, 3, nil, nil,
|
||||
)
|
||||
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)
|
||||
|
||||
if w.Code != 200 {
|
||||
t.Errorf("expected 200, got %d: %s", w.Code, w.Body.String())
|
||||
}
|
||||
var result []map[string]interface{}
|
||||
json.Unmarshal(w.Body.Bytes(), &result)
|
||||
if len(result) != 3 {
|
||||
t.Errorf("expected 3 channels (graceful degradation), got %d", len(result))
|
||||
}
|
||||
// Valid row must appear with correct id
|
||||
foundGood := false
|
||||
for _, ch := range result {
|
||||
if ch["id"] == "ch-good" {
|
||||
foundGood = true
|
||||
// config should be a non-empty map
|
||||
cfg, ok := ch["config"].(map[string]interface{})
|
||||
if !ok || cfg == nil {
|
||||
t.Error("ch-good config should be a non-nil map")
|
||||
}
|
||||
allowed, ok := ch["allowed_users"].([]interface{})
|
||||
if !ok || allowed == nil {
|
||||
t.Error("ch-good allowed_users should be a non-nil array")
|
||||
}
|
||||
}
|
||||
}
|
||||
if !foundGood {
|
||||
t.Error("valid channel 'ch-good' should be in the list despite other rows having bad JSON")
|
||||
}
|
||||
}
|
||||
|
||||
func TestChannelHandler_List(t *testing.T) {
|
||||
mock := setupTestDB(t)
|
||||
handler := NewChannelHandler(newTestChannelManager())
|
||||
|
||||
@@ -54,6 +54,9 @@ func (h *MemoryHandler) List(c *gin.Context) {
|
||||
entry.Value = json.RawMessage(value)
|
||||
entries = append(entries, entry)
|
||||
}
|
||||
if err := rows.Err(); err != nil {
|
||||
log.Printf("Memory List rows.Err: %v", err)
|
||||
}
|
||||
|
||||
c.JSON(http.StatusOK, entries)
|
||||
}
|
||||
|
||||
@@ -57,6 +57,46 @@ func TestMemoryList_Success(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// TestMemoryList_RowsErrLogged covers the rows.Err() check added after the
|
||||
// rows.Next() loop in List. When the query returns rows but a subsequent
|
||||
// database error occurs (e.g. connection drops mid-stream), rows.Err() catches
|
||||
// it and the handler returns what it collected so far with a log entry.
|
||||
func TestMemoryList_RowsErrLogged(t *testing.T) {
|
||||
mock := setupTestDB(t)
|
||||
setupTestRedis(t)
|
||||
handler := NewMemoryHandler()
|
||||
|
||||
now := time.Now()
|
||||
// Return one good row, then simulate a DB-level error after the last row.
|
||||
rows := sqlmock.NewRows([]string{"key", "value", "version", "expires_at", "updated_at"}).
|
||||
AddRow("good-key", []byte(`"ok"`), int64(1), nil, now).
|
||||
RowError(0, sql.ErrConnDone)
|
||||
|
||||
mock.ExpectQuery("SELECT key, value, version").
|
||||
WithArgs("ws-rows-err").
|
||||
WillReturnRows(rows)
|
||||
|
||||
w := httptest.NewRecorder()
|
||||
c, _ := gin.CreateTestContext(w)
|
||||
c.Params = gin.Params{{Key: "id", Value: "ws-rows-err"}}
|
||||
c.Request = httptest.NewRequest("GET", "/workspaces/ws-rows-err/memory", nil)
|
||||
|
||||
handler.List(c)
|
||||
|
||||
// rows.Err() is logged but non-fatal — partial results are still returned.
|
||||
if w.Code != http.StatusOK {
|
||||
t.Errorf("expected 200 (partial results), got %d: %s", w.Code, w.Body.String())
|
||||
}
|
||||
var resp []MemoryEntry
|
||||
if err := json.Unmarshal(w.Body.Bytes(), &resp); err != nil {
|
||||
t.Fatalf("response should be valid JSON even after rows.Err: %v", err)
|
||||
}
|
||||
// The one successfully scanned row must still be returned.
|
||||
if len(resp) != 1 || resp[0].Key != "good-key" {
|
||||
t.Errorf("expected 1 entry with key 'good-key', got %d entries", len(resp))
|
||||
}
|
||||
}
|
||||
|
||||
func TestMemoryList_DBError(t *testing.T) {
|
||||
mock := setupTestDB(t)
|
||||
setupTestRedis(t)
|
||||
|
||||
Reference in New Issue
Block a user