From 600f88b17201c3227efcdeb1dfb482d4ac81d0cd Mon Sep 17 00:00:00 2001 From: "Molecule AI Dev Engineer A (Kimi)" Date: Sat, 23 May 2026 22:12:45 +0000 Subject: [PATCH] fix(workspace-server): check rows.Err() after iteration in MemoryHandler.List The List handler iterated over rows.Next() but never checked rows.Err() after the loop. If the database connection fails during iteration, the error is silently swallowed and partial results are returned with 200 OK. Add a rows.Err() guard that returns 500 when iteration encounters an error, plus a sqlmock test that injects a storage-engine fault mid-loop. Tracked: rows.Err() audit gap (follow-up to internal#348 / PR #1743). --- workspace-server/internal/handlers/memory.go | 5 ++++ .../internal/handlers/memory_test.go | 29 +++++++++++++++++++ 2 files changed, 34 insertions(+) diff --git a/workspace-server/internal/handlers/memory.go b/workspace-server/internal/handlers/memory.go index 8f945a26..04ed80b3 100644 --- a/workspace-server/internal/handlers/memory.go +++ b/workspace-server/internal/handlers/memory.go @@ -54,6 +54,11 @@ 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 iteration error: %v", err) + c.JSON(http.StatusInternalServerError, gin.H{"error": "query iteration failed"}) + return + } c.JSON(http.StatusOK, entries) } diff --git a/workspace-server/internal/handlers/memory_test.go b/workspace-server/internal/handlers/memory_test.go index d92d9c70..d5c0181b 100644 --- a/workspace-server/internal/handlers/memory_test.go +++ b/workspace-server/internal/handlers/memory_test.go @@ -4,6 +4,7 @@ import ( "bytes" "database/sql" "encoding/json" + "errors" "net/http" "net/http/httptest" "testing" @@ -74,6 +75,34 @@ func TestMemoryList_DBError(t *testing.T) { } } +// TestMemoryList_RowsErr_Returns500 verifies that a rows.Err() set during +// iteration causes the handler to return 500 rather than partial results. +func TestMemoryList_RowsErr_Returns500(t *testing.T) { + mock := setupTestDB(t) + setupTestRedis(t) + handler := NewMemoryHandler() + + cols := []string{"key", "value", "version", "expires_at", "updated_at"} + mock.ExpectQuery("SELECT key, value, version, expires_at, updated_at"). + WithArgs("ws-rowerr"). + WillReturnRows(sqlmock.NewRows(cols). + AddRow("ok-key", []byte(`"val"`), int64(1), nil, time.Now()). + RowError(0, errors.New("storage engine fault"))) + + w := httptest.NewRecorder() + c, _ := gin.CreateTestContext(w) + c.Params = gin.Params{{Key: "id", Value: "ws-rowerr"}} + c.Request = httptest.NewRequest("GET", "/workspaces/ws-rowerr/memory", nil) + handler.List(c) + + if w.Code != http.StatusInternalServerError { + t.Errorf("rows.Err() must yield 500, got %d: %s", w.Code, w.Body.String()) + } + if err := mock.ExpectationsWereMet(); err != nil { + t.Errorf("unmet sqlmock expectations: %v", err) + } +} + // ==================== GET /workspaces/:id/memory/:key (Get) ==================== func TestMemoryGet_Success(t *testing.T) {