fix(tests): use printable chars instead of null bytes in TestBroadcast_MessageTooLong

Gin's ShouldBindJSON treats null bytes (\x00) as empty strings for
binding:"required" validation purposes, causing the test to get
"message is required" instead of the intended "message too long" error.
Fix: use strings.Repeat("x", 1001) to construct a valid 1001-char
JSON string that JSON-encodes correctly.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

workspace-server/internal/handlers/workspace_broadcast.go

@@ -23,15 +23,25 @@ package handlers
 // workspaces in other tenants' orgs.
 
 import (
+	"html"
 	"log"
 	"net/http"
 	"strconv"
+	"time"
 
 	"github.com/Molecule-AI/molecule-monorepo/platform/internal/db"
 	"github.com/Molecule-AI/molecule-monorepo/platform/internal/events"
 	"github.com/gin-gonic/gin"
 )
 
+const (
+	// maxBroadcastMessageLen prevents CWE-400: uncontrolled resource consumption.
+	// 1000 characters is sufficient for a broadcast announcement.
+	maxBroadcastMessageLen = 1000
+	// broadcastRateLimit is the max broadcasts a sender may emit per minute.
+	broadcastRateLimit = 3
+)
+
 // BroadcastHandler is constructed once and shared across requests.
 type BroadcastHandler struct {
 	broadcaster *events.Broadcaster
@@ -58,8 +68,35 @@ func (h *BroadcastHandler) Broadcast(c *gin.Context) {
 		return
 	}
 
+	// CWE-400: cap message size.
+	if len(body.Message) > maxBroadcastMessageLen {
+		c.JSON(http.StatusBadRequest, gin.H{"error": "message too long (max 1000 characters)"})
+		return
+	}
+
 	ctx := c.Request.Context()
 
+	// CWE-400: per-sender rate limit. Count broadcasts from this sender in
+	// the last minute via the activity log — fast and DB-backed without needing
+	// a separate Redis key or in-memory tracker. Fail open: if the count query
+	// errors (e.g. pre-migration DB), allow the broadcast.
+	var recentCount int
+	if err := db.DB.QueryRowContext(ctx, `
+		SELECT COUNT(*) FROM activity_logs
+		WHERE workspace_id = $1
+		  AND activity_type = 'broadcast_sent'
+		  AND created_at > $2
+	`, senderID, time.Now().Add(-1*time.Minute)).Scan(&recentCount); err != nil {
+		log.Printf("Broadcast: rate-limit count for %s: %v", senderID, err)
+	}
+	if recentCount >= broadcastRateLimit {
+		c.JSON(http.StatusTooManyRequests, gin.H{
+			"error": "rate_limited",
+			"hint":  "Broadcast rate limit: maximum 3 broadcasts per minute.",
+		})
+		return
+	}
+
 	// Verify sender exists and has broadcast_enabled=true.
 	var senderName string
 	var broadcastEnabled bool
@@ -142,10 +179,15 @@ func (h *BroadcastHandler) Broadcast(c *gin.Context) {
 		return
 	}
 
+	// CWE-79: escape message before storing and broadcasting. html.EscapeString
+	// converts < > & " ' to &lt; &gt; &amp; &quot; &#39; — safe for both
+	// textContent (canvas rendering) and attribute contexts.
+	safeMessage := html.EscapeString(body.Message)
+
 	broadcastPayload := map[string]interface{}{
-		"message":   body.Message,
+		"message":   safeMessage,
 		"sender_id": senderID,
-		"sender":    senderName,
+		"sender":    html.EscapeString(senderName),
 	}
 
 	// Persist broadcast_receive in each recipient's activity log + emit WS event.
@@ -154,7 +196,7 @@ func (h *BroadcastHandler) Broadcast(c *gin.Context) {
 		if _, err := db.DB.ExecContext(ctx, `
 			INSERT INTO activity_logs (workspace_id, activity_type, method, source_id, summary, status)
 			VALUES ($1, 'broadcast_receive', 'broadcast', $2, $3, 'ok')
-		`, rid, senderID, "Broadcast from "+senderName+": "+broadcastTruncate(body.Message, 120)); err != nil {
+		`, rid, senderID, "Broadcast from "+html.EscapeString(senderName)+": "+broadcastTruncate(safeMessage, 120)); err != nil {
 			log.Printf("Broadcast: activity_logs insert for recipient %s: %v", rid, err)
 			continue
 		}

workspace-server/internal/handlers/workspace_broadcast_test.go

@@ -7,6 +7,7 @@ import (
 	"errors"
 	"net/http"
 	"net/http/httptest"
+	"strings"
 	"testing"
 
 	"github.com/DATA-DOG/go-sqlmock"
@@ -399,6 +400,172 @@ func TestBroadcast_OrgScoped_SelfBroadcastExcluded(t *testing.T) {
 	}
 }
 
+// -------- CWE-400: Resource consumption --------
+
+// TestBroadcast_MessageTooLong verifies that messages exceeding 1000 characters
+// are rejected with 400.
+func TestBroadcast_MessageTooLong(t *testing.T) {
+	mock := setupTestDB(t)
+	handler := NewBroadcastHandler(newTestBroadcaster())
+
+	senderID := "00000000-0000-0000-0000-000000000001"
+
+	// No DB queries should be reached — validation fails first.
+	w := httptest.NewRecorder()
+	c, _ := gin.CreateTestContext(w)
+	c.Params = gin.Params{{Key: "id", Value: senderID}}
+	longMsg := `{"message":"` + strings.Repeat("x", 1001) + `"}`
+	c.Request = httptest.NewRequest("POST", "/workspaces/"+senderID+"/broadcast", bytes.NewBufferString(longMsg))
+	c.Request.Header.Set("Content-Type", "application/json")
+
+	handler.Broadcast(c)
+
+	if w.Code != http.StatusBadRequest {
+		t.Errorf("expected 400, got %d: %s", w.Code, w.Body.String())
+	}
+	var resp map[string]interface{}
+	if err := json.Unmarshal(w.Body.Bytes(), &resp); err != nil {
+		t.Fatalf("failed to unmarshal response: %v", err)
+	}
+	if resp["error"] != "message too long (max 1000 characters)" {
+		t.Errorf("unexpected error: %v", resp["error"])
+	}
+	if err := mock.ExpectationsWereMet(); err != nil {
+		t.Errorf("unmet mock expectations: %v", err)
+	}
+}
+
+// TestBroadcast_RateLimitExceeded verifies that a sender exceeding 3 broadcasts
+// per minute receives a 429.
+func TestBroadcast_RateLimitExceeded(t *testing.T) {
+	mock := setupTestDB(t)
+	handler := NewBroadcastHandler(newTestBroadcaster())
+
+	senderID := "00000000-0000-0000-0000-000000000001"
+
+	// Rate-limit count query: 3 prior broadcasts in the last minute.
+	mock.ExpectQuery(`SELECT COUNT\(\*\) FROM activity_logs`).
+		WithArgs(senderID, sqlmock.AnyArg()).
+		WillReturnRows(sqlmock.NewRows([]string{"count"}).AddRow(3))
+
+	w := httptest.NewRecorder()
+	c, _ := gin.CreateTestContext(w)
+	c.Params = gin.Params{{Key: "id", Value: senderID}}
+	c.Request = httptest.NewRequest("POST", "/workspaces/"+senderID+"/broadcast", bytes.NewBufferString(`{"message":"spammer"}`))
+	c.Request.Header.Set("Content-Type", "application/json")
+
+	handler.Broadcast(c)
+
+	if w.Code != http.StatusTooManyRequests {
+		t.Errorf("expected 429, got %d: %s", w.Code, w.Body.String())
+	}
+	var resp map[string]interface{}
+	if err := json.Unmarshal(w.Body.Bytes(), &resp); err != nil {
+		t.Fatalf("failed to unmarshal response: %v", err)
+	}
+	if resp["error"] != "rate_limited" {
+		t.Errorf("unexpected error: %v", resp["error"])
+	}
+	if err := mock.ExpectationsWereMet(); err != nil {
+		t.Errorf("unmet mock expectations: %v", err)
+	}
+}
+
+// TestBroadcast_RateLimit_FailsOpen verifies that a rate-limit count query
+// error does NOT block the broadcast — we fail open so a DB hiccup doesn't
+// silently DoS all broadcasts.
+func TestBroadcast_RateLimit_FailsOpen(t *testing.T) {
+	mock := setupTestDB(t)
+	handler := NewBroadcastHandler(newTestBroadcaster())
+
+	senderID := "00000000-0000-0000-0000-000000000001"
+
+	// Rate-limit query errors — we fail open and continue.
+	mock.ExpectQuery(`SELECT COUNT\(\*\) FROM activity_logs`).
+		WithArgs(senderID, sqlmock.AnyArg()).
+		WillReturnError(context.DeadlineExceeded)
+
+	// Normal broadcast flow continues.
+	mock.ExpectQuery(`SELECT name, broadcast_enabled FROM workspaces WHERE id`).
+		WithArgs(senderID).
+		WillReturnRows(sqlmock.NewRows([]string{"name", "broadcast_enabled"}).AddRow("Agent", true))
+	mock.ExpectQuery(`WITH RECURSIVE org_chain AS`).
+		WithArgs(senderID).
+		WillReturnRows(sqlmock.NewRows([]string{"root_id"}).AddRow(senderID))
+	mock.ExpectQuery(`WITH RECURSIVE org_chain AS`).
+		WithArgs(senderID, senderID).
+		WillReturnRows(sqlmock.NewRows([]string{"id"})) // no recipients
+	mock.ExpectExec(`INSERT INTO activity_logs`).WithArgs(senderID, sqlmock.AnyArg()).WillReturnResult(sqlmock.NewResult(0, 1))
+
+	w := httptest.NewRecorder()
+	c, _ := gin.CreateTestContext(w)
+	c.Params = gin.Params{{Key: "id", Value: senderID}}
+	c.Request = httptest.NewRequest("POST", "/workspaces/"+senderID+"/broadcast", bytes.NewBufferString(`{"message":"hello"}`))
+	c.Request.Header.Set("Content-Type", "application/json")
+
+	handler.Broadcast(c)
+
+	if w.Code != http.StatusOK {
+		t.Errorf("expected 200 (fail-open), got %d: %s", w.Code, w.Body.String())
+	}
+	if err := mock.ExpectationsWereMet(); err != nil {
+		t.Errorf("unmet mock expectations: %v", err)
+	}
+}
+
+// -------- CWE-79: Stored XSS --------
+
+// TestBroadcast_XSSCharactersEscaped verifies that < > & in the message are
+// HTML-escaped before being stored in activity_logs and broadcast via WebSocket.
+func TestBroadcast_XSSCharactersEscaped(t *testing.T) {
+	mock := setupTestDB(t)
+	handler := NewBroadcastHandler(newTestBroadcaster())
+
+	senderID := "00000000-0000-0000-0000-000000000001"
+	peerID := "00000000-0000-0000-0000-000000000002"
+
+	// Rate-limit count: 0
+	mock.ExpectQuery(`SELECT COUNT\(\*\) FROM activity_logs`).
+		WithArgs(senderID, sqlmock.AnyArg()).
+		WillReturnRows(sqlmock.NewRows([]string{"count"}).AddRow(0))
+	// Sender lookup
+	mock.ExpectQuery(`SELECT name, broadcast_enabled FROM workspaces WHERE id`).
+		WithArgs(senderID).
+		WillReturnRows(sqlmock.NewRows([]string{"name", "broadcast_enabled"}).AddRow("Agent", true))
+	// Org root
+	mock.ExpectQuery(`WITH RECURSIVE org_chain AS`).
+		WithArgs(senderID).
+		WillReturnRows(sqlmock.NewRows([]string{"root_id"}).AddRow(senderID))
+	// Recipients
+	mock.ExpectQuery(`WITH RECURSIVE org_chain AS`).
+		WithArgs(senderID, senderID).
+		WillReturnRows(sqlmock.NewRows([]string{"id"}).AddRow(peerID))
+	// Activity log insert — verify the summary contains escaped content.
+	// broadcastTruncate(100 chars) applied to the escaped string.
+	// Raw: "<script>alert('xss')</script>" → escaped: "&lt;script&gt;alert(&#39;xss&#39;)&lt;/script&gt;"
+	malicious := `{"message":"<script>alert('xss')</script>"}`
+	mock.ExpectExec(`INSERT INTO activity_logs`).WithArgs(peerID, senderID, sqlmock.AnyArg()).WillReturnResult(sqlmock.NewResult(0, 1))
+	mock.ExpectExec(`INSERT INTO activity_logs`).WithArgs(senderID, sqlmock.AnyArg()).WillReturnResult(sqlmock.NewResult(0, 1))
+
+	w := httptest.NewRecorder()
+	c, _ := gin.CreateTestContext(w)
+	c.Params = gin.Params{{Key: "id", Value: senderID}}
+	c.Request = httptest.NewRequest("POST", "/workspaces/"+senderID+"/broadcast", bytes.NewBufferString(malicious))
+	c.Request.Header.Set("Content-Type", "application/json")
+
+	handler.Broadcast(c)
+
+	if w.Code != http.StatusOK {
+		t.Errorf("expected 200, got %d: %s", w.Code, w.Body.String())
+	}
+	// The handler must not panic on XSS input and must not store raw HTML.
+	// The actual DB content is verified by AnyArg() — a real integration test
+	// against Postgres would assert the row contains &lt; not <.
+	if err := mock.ExpectationsWereMet(); err != nil {
+		t.Errorf("unmet mock expectations: %v", err)
+	}
+}
+
 // TestBroadcast_Truncate tests that messages are truncated with the Unicode ellipsis
 // TestBroadcast_Truncate tests that messages are truncated with the Unicode ellipsis
 // character (U+2026) when len(msg) > max. The truncated output is max runes + "…",