Changeset 0.19.0 (#81)

server/handlers/channels.go

@@ -98,60 +98,7 @@ func writeTagsArg(tags []string) interface{} {
 	return pq.Array(tags)
 }
 
-// ── JSON scanner (settings column) ──────────
-// SQLite returns TEXT for JSON columns; json.RawMessage ([]byte) can't scan
-// from a string with modernc.org/sqlite. This wrapper handles both dialects.
-
-type jsonScanner struct{ dest *json.RawMessage }
-
-func scanJSON(dest *json.RawMessage) *jsonScanner { return &jsonScanner{dest: dest} }
-
-func (s *jsonScanner) Scan(src interface{}) error {
-	switch v := src.(type) {
-	case []byte:
-		*s.dest = json.RawMessage(v)
-	case string:
-		*s.dest = json.RawMessage(v)
-	case nil:
-		*s.dest = json.RawMessage("{}")
-	default:
-		*s.dest = json.RawMessage("{}")
-	}
-	return nil
-}
-
-// tagsScanner wraps a *[]string so rows.Scan can populate it on both dialects.
-type tagsScanner struct {
-	dest *[]string
-}
-
-func scanTags(dest *[]string) *tagsScanner {
-	return &tagsScanner{dest: dest}
-}
-
-func (s *tagsScanner) Scan(src interface{}) error {
-	if database.IsSQLite() {
-		var raw string
-		switch v := src.(type) {
-		case string:
-			raw = v
-		case []byte:
-			raw = string(v)
-		case nil:
-			*s.dest = []string{}
-			return nil
-		}
-		var arr []string
-		if err := json.Unmarshal([]byte(raw), &arr); err != nil {
-			*s.dest = []string{}
-			return nil
-		}
-		*s.dest = arr
-		return nil
-	}
-	// Postgres: delegate to pq
-	return pq.Array(s.dest).Scan(src)
-}
+// scanJSON, scanTags, SafeJSON → safe_json.go
 
 // ── Helpers ─────────────────────────────────
 
@@ -281,7 +228,7 @@ func (h *ChannelHandler) ListChannels(c *gin.Context) {
 		channels = append(channels, ch)
 	}
 
-	c.JSON(http.StatusOK, paginatedResponse{
+	SafeJSON(c, http.StatusOK, paginatedResponse{
 		Data:       channels,
 		Page:       page,
 		PerPage:    perPage,
@@ -399,7 +346,7 @@ func (h *ChannelHandler) CreateChannel(c *gin.Context) {
 		}
 	}
 
-	c.JSON(http.StatusCreated, ch)
+	SafeJSON(c, http.StatusCreated, ch)
 }
 
 // ── Get Channel ─────────────────────────────
@@ -441,7 +388,7 @@ func (h *ChannelHandler) GetChannel(c *gin.Context) {
 	}
 	ch.Tags = tags
 
-	c.JSON(http.StatusOK, ch)
+	SafeJSON(c, http.StatusOK, ch)
 }
 
 // ── Update Channel ──────────────────────────

server/handlers/safe_json.go

@@ -0,0 +1,116 @@
+package handlers
+
+import (
+	"encoding/json"
+	"log"
+	"net/http"
+
+	"github.com/gin-gonic/gin"
+	"github.com/lib/pq"
+
+	"git.gobha.me/xcaliber/chat-switchboard/database"
+)
+
+// ── SafeJSON ────────────────────────────────
+// SafeJSON pre-marshals the response body, guaranteeing that a
+// serialisation failure produces a clean 500 instead of a truncated
+// 200 with half-written JSON.  Use for any endpoint whose response
+// contains json.RawMessage or other pass-through data from the DB.
+
+func SafeJSON(c *gin.Context, code int, obj interface{}) {
+	body, err := json.Marshal(obj)
+	if err != nil {
+		log.Printf("⚠ SafeJSON: marshal failed: %v", err)
+		c.JSON(http.StatusInternalServerError, gin.H{
+			"error": "response serialization failed",
+		})
+		return
+	}
+	c.Data(code, "application/json; charset=utf-8", body)
+}
+
+// ── JSON column scanner ─────────────────────
+// Handles both Postgres ([]byte) and SQLite (string) return types.
+// Validates content before accepting it — corrupt data is replaced
+// with "{}" and a warning is logged so admins can find the bad row.
+
+type jsonScanner struct{ dest *json.RawMessage }
+
+func scanJSON(dest *json.RawMessage) *jsonScanner { return &jsonScanner{dest: dest} }
+
+func (s *jsonScanner) Scan(src interface{}) error {
+	switch v := src.(type) {
+	case []byte:
+		if len(v) == 0 || !json.Valid(v) {
+			log.Printf("⚠ scanJSON: invalid JSON in column (len=%d, preview=%.60q), defaulting to {}", len(v), v)
+			*s.dest = json.RawMessage("{}")
+			return nil
+		}
+		*s.dest = json.RawMessage(v)
+	case string:
+		b := []byte(v)
+		if len(b) == 0 || !json.Valid(b) {
+			log.Printf("⚠ scanJSON: invalid JSON in column (len=%d, preview=%.60q), defaulting to {}", len(b), b)
+			*s.dest = json.RawMessage("{}")
+			return nil
+		}
+		*s.dest = json.RawMessage(b)
+	case nil:
+		*s.dest = json.RawMessage("{}")
+	default:
+		*s.dest = json.RawMessage("{}")
+	}
+	return nil
+}
+
+// ── Tags column scanner ─────────────────────
+// SQLite stores tags as a JSON text array; Postgres uses text[].
+// Both paths validate and fall back to an empty slice on error.
+
+type tagsScanner struct {
+	dest *[]string
+}
+
+func scanTags(dest *[]string) *tagsScanner {
+	return &tagsScanner{dest: dest}
+}
+
+func (s *tagsScanner) Scan(src interface{}) error {
+	if src == nil {
+		*s.dest = []string{}
+		return nil
+	}
+
+	if database.IsSQLite() {
+		var raw string
+		switch v := src.(type) {
+		case string:
+			raw = v
+		case []byte:
+			raw = string(v)
+		default:
+			*s.dest = []string{}
+			return nil
+		}
+		if raw == "" {
+			*s.dest = []string{}
+			return nil
+		}
+		var arr []string
+		if err := json.Unmarshal([]byte(raw), &arr); err != nil {
+			log.Printf("⚠ scanTags: invalid JSON array in column (preview=%.60q), defaulting to []", raw)
+			*s.dest = []string{}
+			return nil
+		}
+		*s.dest = arr
+		return nil
+	}
+
+	// Postgres: delegate to pq, but catch errors
+	if err := pq.Array(s.dest).Scan(src); err != nil {
+		log.Printf("⚠ scanTags: pq.Array scan failed (%v), defaulting to []", err)
+		*s.dest = []string{}
+		return nil
+	}
+	return nil
+}

server/handlers/safe_json_test.go

@@ -0,0 +1,185 @@
+package handlers
+
+import (
+	"encoding/json"
+	"net/http"
+	"net/http/httptest"
+	"testing"
+
+	"github.com/gin-gonic/gin"
+)
+
+func TestScanJSON_ValidObject(t *testing.T) {
+	var dest json.RawMessage
+	s := scanJSON(&dest)
+	if err := s.Scan([]byte(`{"key":"value"}`)); err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	if string(dest) != `{"key":"value"}` {
+		t.Fatalf("expected {\"key\":\"value\"}, got %s", dest)
+	}
+}
+
+func TestScanJSON_ValidArray(t *testing.T) {
+	var dest json.RawMessage
+	s := scanJSON(&dest)
+	if err := s.Scan([]byte(`[1,2,3]`)); err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	if string(dest) != `[1,2,3]` {
+		t.Fatalf("expected [1,2,3], got %s", dest)
+	}
+}
+
+func TestScanJSON_EmptyString(t *testing.T) {
+	var dest json.RawMessage
+	s := scanJSON(&dest)
+	if err := s.Scan(""); err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	if string(dest) != "{}" {
+		t.Fatalf("expected {}, got %s", dest)
+	}
+}
+
+func TestScanJSON_Nil(t *testing.T) {
+	var dest json.RawMessage
+	s := scanJSON(&dest)
+	if err := s.Scan(nil); err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	if string(dest) != "{}" {
+		t.Fatalf("expected {}, got %s", dest)
+	}
+}
+
+func TestScanJSON_BinaryGarbage(t *testing.T) {
+	var dest json.RawMessage
+	s := scanJSON(&dest)
+	// The exact scenario from the bug: STX control character
+	if err := s.Scan([]byte{0x02, 0x03, 0x04}); err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	if string(dest) != "{}" {
+		t.Fatalf("expected {} for binary garbage, got %s", dest)
+	}
+}
+
+func TestScanJSON_TruncatedJSON(t *testing.T) {
+	var dest json.RawMessage
+	s := scanJSON(&dest)
+	if err := s.Scan([]byte(`{"key":"val`)); err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	if string(dest) != "{}" {
+		t.Fatalf("expected {} for truncated JSON, got %s", dest)
+	}
+}
+
+func TestScanJSON_StringDialect(t *testing.T) {
+	// SQLite returns strings, not []byte
+	var dest json.RawMessage
+	s := scanJSON(&dest)
+	if err := s.Scan(`{"sqlite":"mode"}`); err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	if string(dest) != `{"sqlite":"mode"}` {
+		t.Fatalf("expected {\"sqlite\":\"mode\"}, got %s", dest)
+	}
+}
+
+func TestScanJSON_StringDialect_Corrupt(t *testing.T) {
+	var dest json.RawMessage
+	s := scanJSON(&dest)
+	if err := s.Scan("\x02\x03garbage"); err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	if string(dest) != "{}" {
+		t.Fatalf("expected {} for corrupt string, got %s", dest)
+	}
+}
+
+func TestSafeJSON_ValidPayload(t *testing.T) {
+	gin.SetMode(gin.TestMode)
+	w := httptest.NewRecorder()
+	c, _ := gin.CreateTestContext(w)
+
+	SafeJSON(c, http.StatusOK, gin.H{"status": "ok"})
+
+	if w.Code != http.StatusOK {
+		t.Fatalf("expected 200, got %d", w.Code)
+	}
+	if ct := w.Header().Get("Content-Type"); ct != "application/json; charset=utf-8" {
+		t.Fatalf("expected JSON content-type, got %s", ct)
+	}
+
+	var result map[string]string
+	if err := json.Unmarshal(w.Body.Bytes(), &result); err != nil {
+		t.Fatalf("response is not valid JSON: %v", err)
+	}
+	if result["status"] != "ok" {
+		t.Fatalf("expected status=ok, got %s", result["status"])
+	}
+}
+
+func TestSafeJSON_CorruptRawMessage(t *testing.T) {
+	gin.SetMode(gin.TestMode)
+	w := httptest.NewRecorder()
+	c, _ := gin.CreateTestContext(w)
+
+	// Simulate the exact bug: a struct with a corrupt json.RawMessage
+	type response struct {
+		ID       string          `json:"id"`
+		Settings json.RawMessage `json:"settings"`
+	}
+	resp := response{
+		ID:       "test-123",
+		Settings: json.RawMessage{0x02, 0x03}, // binary garbage
+	}
+
+	SafeJSON(c, http.StatusOK, resp)
+
+	// Should get 500, not a truncated 200
+	if w.Code != http.StatusInternalServerError {
+		t.Fatalf("expected 500 for corrupt RawMessage, got %d", w.Code)
+	}
+
+	// Body should be valid JSON with error message
+	var result map[string]string
+	if err := json.Unmarshal(w.Body.Bytes(), &result); err != nil {
+		t.Fatalf("error response is not valid JSON: %v", err)
+	}
+	if result["error"] != "response serialization failed" {
+		t.Fatalf("unexpected error message: %s", result["error"])
+	}
+}
+
+func TestSafeJSON_PaginatedWithCorrupt(t *testing.T) {
+	gin.SetMode(gin.TestMode)
+	w := httptest.NewRecorder()
+	c, _ := gin.CreateTestContext(w)
+
+	// If scanJSON is hardened, this should never happen — but SafeJSON
+	// is the second line of defense if something slips through.
+	type item struct {
+		Settings json.RawMessage `json:"settings"`
+	}
+	type paginated struct {
+		Data  []item `json:"data"`
+		Total int    `json:"total"`
+	}
+
+	resp := paginated{
+		Data: []item{
+			{Settings: json.RawMessage(`{"ok":true}`)},
+			{Settings: json.RawMessage{0xFF, 0xFE}}, // corrupt
+		},
+		Total: 2,
+	}
+
+	SafeJSON(c, http.StatusOK, resp)
+
+	if w.Code != http.StatusInternalServerError {
+		t.Fatalf("expected 500 for paginated response with corrupt item, got %d", w.Code)
+	}
+}