ci: rerun after mc#724 all-required fix lands

Covers the three pure validator functions introduced in #685/#688:

  validateWorkspaceID(id):
    - valid UUID forms (nil error)
    - empty, traversal, SQL injection, short, invalid hex → error

  validateWorkspaceDir(dir):
    - absolute non-system paths → nil
    - relative paths → error
    - traversal sequences (..) → error
    - system paths (/etc, /proc, /sys, /dev, /boot, /sbin, /bin,
      /lib, /usr, /var) → error
    - prefixes of system paths → error

  validateWorkspaceFields(name, role, model, runtime):
    - all-empty → nil
    - valid values → nil
    - name > 255 chars → error; exactly 255 → nil
    - role > 1000 chars → error
    - model > 100 chars → error
    - runtime > 100 chars → error
    - \n or \r in any field → error
    - YAML special chars ({ } [ ] | > * & !) in name/role → error
    - YAML chars allowed in model/runtime (only name/role are gated)

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

workspace-server/internal/handlers/socket_test.go

@@ -1,195 +0,0 @@
-package handlers
-
-import (
-	"context"
-	"database/sql"
-	"net/http"
-	"net/http/httptest"
-	"testing"
-
-	"github.com/DATA-DOG/go-sqlmock"
-	"github.com/Molecule-AI/molecule-monorepo/platform/internal/db"
-	"github.com/Molecule-AI/molecule-monorepo/platform/internal/ws"
-	"github.com/Molecule-AI/molecule-monorepo/platform/internal/wsauth"
-	"github.com/alicebob/miniredis/v2"
-	"github.com/gin-gonic/gin"
-	"github.com/redis/go-redis/v9"
-)
-
-// ─── Setup helpers ─────────────────────────────────────────────────────────────
-
-func init() {
-	gin.SetMode(gin.TestMode)
-}
-
-// socketTestDB wraps sqlmock setup with the redis setup needed for wsauth.
-func socketTestDB(t *testing.T) (sqlmock.Sqlmock, func()) {
-	t.Helper()
-	mockDB, mock, err := sqlmock.New()
-	if err != nil {
-		t.Fatalf("failed to create sqlmock: %v", err)
-	}
-
-	// Start a miniredis for the wsauth token subsystem.
-	mr, err := miniredis.Run()
-	if err != nil {
-		mockDB.Close()
-		t.Fatalf("failed to start miniredis: %v", err)
-	}
-	db.DB = mockDB
-	db.RDB = redis.NewClient(&redis.Options{Addr: mr.Addr()})
-
-	wsauth.ResetInboundSecretCacheForTesting()
-
-	cleanup := func() {
-		mockDB.Close()
-		mr.Close()
-		wsauth.ResetInboundSecretCacheForTesting()
-	}
-	return mock, cleanup
-}
-
-// ─── Test cases ────────────────────────────────────────────────────────────────
-// Phase 30.1/30.2 bearer-token auth gate on WebSocket upgrade.
-// SocketHandler.HandleConnect enforces:
-//   - Canvas clients (no X-Workspace-ID header) → bypass auth, upgrade proceeds
-//   - Workspace agents (X-Workspace-ID present) → HasAnyLiveToken probe → bearer validation
-
-func TestSocketHandler_HandleConnect_CanvasClient_NoAuthRequired(t *testing.T) {
-	mock, cleanup := socketTestDB(t)
-	defer cleanup()
-
-	// Create hub and drain the Register channel via Run.
-	hub := ws.NewHub(func(_, _ string) bool { return true })
-	go hub.Run()
-
-	h := NewSocketHandler(hub)
-	c, w := gin.CreateTestContext(httptest.NewRecorder())
-	c.Request = httptest.NewRequest("GET", "/ws", nil)
-	// No X-Workspace-ID → canvas client path.
-
-	h.HandleConnect(c)
-
-	// Canvas path has no DB expectations — HasAnyLiveToken not called.
-	if err := mock.ExpectationsWereMet(); err != nil {
-		t.Errorf("unmet sqlmock expectations: %v", err)
-	}
-	_ = w.Code // upgrade fails in test env (httptest doesn't do WS) — handler returns.
-}
-
-// TestSocketHandler_HandleConnect_AgentNoLiveToken_BypassesBearerCheck verifies
-// that agents with no live tokens (legacy pre-token workspaces) are grandfathered
-// through without being asked for a bearer token.
-func TestSocketHandler_HandleConnect_AgentNoLiveToken_BypassesBearerCheck(t *testing.T) {
-	mock, cleanup := socketTestDB(t)
-	defer cleanup()
-
-	// HasAnyLiveToken → no rows (no live tokens → n=0).
-	mock.ExpectQuery(`SELECT COUNT\(\*\) FROM workspace_auth_tokens WHERE workspace_id = \$1 AND revoked_at IS NULL`).
-		WithArgs("ws-agent").
-		WillReturnRows(sqlmock.NewRows([]string{"count"}).AddRow(0))
-
-	hub := ws.NewHub(func(_, _ string) bool { return true })
-	go hub.Run()
-
-	h := NewSocketHandler(hub)
-	c, _ := gin.CreateTestContext(httptest.NewRecorder())
-	c.Request = httptest.NewRequest("GET", "/ws", nil)
-	c.Request.Header.Set("X-Workspace-ID", "ws-agent")
-
-	h.HandleConnect(c)
-
-	if err := mock.ExpectationsWereMet(); err != nil {
-		t.Errorf("unmet sqlmock expectations: %v", err)
-	}
-}
-
-// TestSocketHandler_HandleConnect_DBErrorOnHasAnyLiveToken returns 500.
-func TestSocketHandler_HandleConnect_DBErrorOnHasAnyLiveToken(t *testing.T) {
-	mock, cleanup := socketTestDB(t)
-	defer cleanup()
-
-	mock.ExpectQuery(`SELECT COUNT\(\*\) FROM workspace_auth_tokens WHERE workspace_id = \$1 AND revoked_at IS NULL`).
-		WithArgs("ws-agent").
-		WillReturnError(sql.ErrConnDone)
-
-	hub := ws.NewHub(func(_, _ string) bool { return true })
-	go hub.Run()
-
-	h := NewSocketHandler(hub)
-	c, w := gin.CreateTestContext(httptest.NewRecorder())
-	c.Request = httptest.NewRequest("GET", "/ws", nil)
-	c.Request.Header.Set("X-Workspace-ID", "ws-agent")
-
-	h.HandleConnect(c)
-
-	if w.Code != http.StatusInternalServerError {
-		t.Errorf("expected 500 on DB error, got %d", w.Code)
-	}
-	if err := mock.ExpectationsWereMet(); err != nil {
-		t.Errorf("unmet sqlmock expectations: %v", err)
-	}
-}
-
-// TestSocketHandler_HandleConnect_MissingBearerToken returns 401.
-func TestSocketHandler_HandleConnect_MissingBearerToken(t *testing.T) {
-	mock, cleanup := socketTestDB(t)
-	defer cleanup()
-
-	// hasLive=true but no Authorization header.
-	mock.ExpectQuery(`SELECT COUNT\(\*\) FROM workspace_auth_tokens WHERE workspace_id = \$1 AND revoked_at IS NULL`).
-		WithArgs("ws-agent").
-		WillReturnRows(sqlmock.NewRows([]string{"count"}).AddRow(1))
-
-	hub := ws.NewHub(func(_, _ string) bool { return true })
-	go hub.Run()
-
-	h := NewSocketHandler(hub)
-	c, w := gin.CreateTestContext(httptest.NewRecorder())
-	c.Request = httptest.NewRequest("GET", "/ws", nil)
-	c.Request.Header.Set("X-Workspace-ID", "ws-agent")
-	// No Authorization header.
-
-	h.HandleConnect(c)
-
-	if w.Code != http.StatusUnauthorized {
-		t.Errorf("expected 401 on missing bearer token, got %d", w.Code)
-	}
-	if err := mock.ExpectationsWereMet(); err != nil {
-		t.Errorf("unmet sqlmock expectations: %v", err)
-	}
-}
-
-// TestSocketHandler_HandleConnect_InvalidBearerToken returns 401.
-func TestSocketHandler_HandleConnect_InvalidBearerToken(t *testing.T) {
-	mock, cleanup := socketTestDB(t)
-	defer cleanup()
-
-	// hasLive=true.
-	mock.ExpectQuery(`SELECT COUNT\(\*\) FROM workspace_auth_tokens WHERE workspace_id = \$1 AND revoked_at IS NULL`).
-		WithArgs("ws-agent").
-		WillReturnRows(sqlmock.NewRows([]string{"count"}).AddRow(1))
-
-	// ValidateToken → lookupTokenByHash: no matching hash.
-	mock.ExpectQuery(`SELECT t\.id, t\.workspace_id FROM workspace_auth_tokens t JOIN workspaces w`).
-		WithArgs(sqlmock.AnyArg()).
-		WillReturnError(context.DeadlineExceeded)
-
-	hub := ws.NewHub(func(_, _ string) bool { return true })
-	go hub.Run()
-
-	h := NewSocketHandler(hub)
-	c, w := gin.CreateTestContext(httptest.NewRecorder())
-	c.Request = httptest.NewRequest("GET", "/ws", nil)
-	c.Request.Header.Set("X-Workspace-ID", "ws-agent")
-	c.Request.Header.Set("Authorization", "Bearer invalid-token-xyz")
-
-	h.HandleConnect(c)
-
-	if w.Code != http.StatusUnauthorized {
-		t.Errorf("expected 401 on invalid bearer token, got %d", w.Code)
-	}
-	if err := mock.ExpectationsWereMet(); err != nil {
-		t.Errorf("unmet sqlmock expectations: %v", err)
-	}
-}

workspace-server/internal/handlers/workspace_crud_validators_test.go

@@ -0,0 +1,263 @@
+package handlers
+
+import (
+	"testing"
+)
+
+// ── validateWorkspaceID ─────────────────────────────────────────────────────────
+
+func TestValidateWorkspaceID_Valid(t *testing.T) {
+	cases := []string{
+		"550e8400-e29b-41d4-a716-446655440000",
+		"00000000-0000-0000-0000-000000000000",
+		"ffffffff-ffff-ffff-ffff-ffffffffffff",
+	}
+	for _, id := range cases {
+		t.Run(id, func(t *testing.T) {
+			if err := validateWorkspaceID(id); err != nil {
+				t.Errorf("validateWorkspaceID(%q) returned error: %v", id, err)
+			}
+		})
+	}
+}
+
+func TestValidateWorkspaceID_Invalid(t *testing.T) {
+	cases := []struct {
+		name string
+		id   string
+	}{
+		{"empty", ""},
+		{"not a UUID", "not-a-uuid"},
+		{"traversal attack", "../../etc/passwd"},
+		{"SQL injection", "'; DROP TABLE workspaces;--"},
+		{"UUID too short", "550e8400-e29b-41d4-a716"},
+		{"UUID with invalid hex chars", "550e8400-e29b-41d4-a716-44665544000g"},
+		{"UUID all zeros", "00000000000000000000000000000000"},
+	}
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			if err := validateWorkspaceID(tc.id); err == nil {
+				t.Errorf("validateWorkspaceID(%q): expected error, got nil", tc.id)
+			}
+		})
+	}
+}
+
+// ── validateWorkspaceDir ───────────────────────────────────────────────────────
+
+func TestValidateWorkspaceDir_Valid(t *testing.T) {
+	cases := []string{
+		"/opt/molecule/workspaces/dev",
+		"/home/user/.molecule/workspaces",
+		"/var/data/workspace-abc-123",
+		"/opt/services/molecule/tenant-workspaces",
+	}
+	for _, dir := range cases {
+		t.Run(dir, func(t *testing.T) {
+			if err := validateWorkspaceDir(dir); err != nil {
+				t.Errorf("validateWorkspaceDir(%q) returned error: %v", dir, err)
+			}
+		})
+	}
+}
+
+func TestValidateWorkspaceDir_RelativeRejected(t *testing.T) {
+	cases := []string{
+		"relative/path",
+		"./myworkspace",
+		"~/workspaces/dev",
+	}
+	for _, dir := range cases {
+		t.Run(dir, func(t *testing.T) {
+			if err := validateWorkspaceDir(dir); err == nil {
+				t.Errorf("validateWorkspaceDir(%q): expected error (relative path), got nil", dir)
+			}
+		})
+	}
+}
+
+func TestValidateWorkspaceDir_TraversalRejected(t *testing.T) {
+	cases := []string{
+		"/opt/molecule/../../../etc",
+		"/workspaces/dev/../../root",
+		"/opt/../opt/../etc",
+	}
+	for _, dir := range cases {
+		t.Run(dir, func(t *testing.T) {
+			if err := validateWorkspaceDir(dir); err == nil {
+				t.Errorf("validateWorkspaceDir(%q): expected error (traversal), got nil", dir)
+			}
+		})
+	}
+}
+
+func TestValidateWorkspaceDir_SystemPathsRejected(t *testing.T) {
+	cases := []string{
+		"/etc",
+		"/etc/molecule",
+		"/var",
+		"/var/log",
+		"/proc",
+		"/proc/self",
+		"/sys",
+		"/sys/kernel",
+		"/dev",
+		"/dev/null",
+		"/boot",
+		"/sbin",
+		"/bin",
+		"/lib",
+		"/usr",
+		"/usr/local",
+	}
+	for _, dir := range cases {
+		t.Run(dir, func(t *testing.T) {
+			if err := validateWorkspaceDir(dir); err == nil {
+				t.Errorf("validateWorkspaceDir(%q): expected error (system path), got nil", dir)
+			}
+		})
+	}
+}
+
+func TestValidateWorkspaceDir_PrefixMatchesBlocked(t *testing.T) {
+	// The blocklist checks prefix so /etc/foo must also be rejected.
+	cases := []string{
+		"/etc/molecule-config",
+		"/var/log/workspace",
+		"/usr/local/bin",
+		"/usr/bin/molecule",
+	}
+	for _, dir := range cases {
+		t.Run(dir, func(t *testing.T) {
+			if err := validateWorkspaceDir(dir); err == nil {
+				t.Errorf("validateWorkspaceDir(%q): expected error (prefix of blocked path), got nil", dir)
+			}
+		})
+	}
+}
+
+// ── validateWorkspaceFields ────────────────────────────────────────────────────
+
+func TestValidateWorkspaceFields_AllEmpty(t *testing.T) {
+	// All empty → valid (creation uses defaults; empty is allowed)
+	if err := validateWorkspaceFields("", "", "", ""); err != nil {
+		t.Errorf("validateWorkspaceFields with all empty: expected nil, got %v", err)
+	}
+}
+
+func TestValidateWorkspaceFields_Valid(t *testing.T) {
+	if err := validateWorkspaceFields("My Workspace", "Backend Engineer", "gpt-4o", "langgraph"); err != nil {
+		t.Errorf("validateWorkspaceFields with valid args: expected nil, got %v", err)
+	}
+}
+
+func TestValidateWorkspaceFields_NameTooLong(t *testing.T) {
+	longName := make([]byte, 256)
+	for i := range longName {
+		longName[i] = 'a'
+	}
+	if err := validateWorkspaceFields(string(longName), "", "", ""); err == nil {
+		t.Error("name > 255 chars: expected error, got nil")
+	}
+
+	// Exactly 255 chars is OK
+	validName := make([]byte, 255)
+	for i := range validName {
+		validName[i] = 'a'
+	}
+	if err := validateWorkspaceFields(string(validName), "", "", ""); err != nil {
+		t.Errorf("name exactly 255 chars: expected nil, got %v", err)
+	}
+}
+
+func TestValidateWorkspaceFields_RoleTooLong(t *testing.T) {
+	longRole := make([]byte, 1001)
+	for i := range longRole {
+		longRole[i] = 'x'
+	}
+	if err := validateWorkspaceFields("", string(longRole), "", ""); err == nil {
+		t.Error("role > 1000 chars: expected error, got nil")
+	}
+}
+
+func TestValidateWorkspaceFields_ModelTooLong(t *testing.T) {
+	longModel := make([]byte, 101)
+	for i := range longModel {
+		longModel[i] = 'x'
+	}
+	if err := validateWorkspaceFields("", "", string(longModel), ""); err == nil {
+		t.Error("model > 100 chars: expected error, got nil")
+	}
+}
+
+func TestValidateWorkspaceFields_RuntimeTooLong(t *testing.T) {
+	longRuntime := make([]byte, 101)
+	for i := range longRuntime {
+		longRuntime[i] = 'x'
+	}
+	if err := validateWorkspaceFields("", "", "", string(longRuntime)); err == nil {
+		t.Error("runtime > 100 chars: expected error, got nil")
+	}
+}
+
+func TestValidateWorkspaceFields_NewlineInName(t *testing.T) {
+	if err := validateWorkspaceFields("My\nWorkspace", "", "", ""); err == nil {
+		t.Error("name with \\n: expected error, got nil")
+	}
+}
+
+func TestValidateWorkspaceFields_CRLFInRole(t *testing.T) {
+	if err := validateWorkspaceFields("", "Backend\r\nEngineer", "", ""); err == nil {
+		t.Error("role with \\r\\n: expected error, got nil")
+	}
+}
+
+func TestValidateWorkspaceFields_NewlineInModel(t *testing.T) {
+	if err := validateWorkspaceFields("", "", "gpt-\n4o", ""); err == nil {
+		t.Error("model with \\n: expected error, got nil")
+	}
+}
+
+func TestValidateWorkspaceFields_NewlineInRuntime(t *testing.T) {
+	if err := validateWorkspaceFields("", "", "", "lang\rgraph"); err == nil {
+		t.Error("runtime with \\r: expected error, got nil")
+	}
+}
+
+func TestValidateWorkspaceFields_YAMLSpecialChars(t *testing.T) {
+	// yamlSpecialChars = "{}[]|>*&!"
+	// These must be rejected in name and role.
+	dangerous := []string{
+		"Workspace{evil}",
+		"Workspace[evil]",
+		"Workspace]evil[",
+		"Workspace|evil",
+		"Workspace>evil",
+		"Workspace*evil",
+		"Workspace&evil",
+		"Workspace!evil",
+		"Name{}",
+		"Role[]",
+	}
+	for _, v := range dangerous {
+		t.Run(v, func(t *testing.T) {
+			if err := validateWorkspaceFields(v, "", "", ""); err == nil {
+				t.Errorf("name %q: expected error (YAML special char), got nil", v)
+			}
+		})
+	}
+}
+
+func TestValidateWorkspaceFields_YAMLCharsAllowedInModelRuntime(t *testing.T) {
+	// YAML special chars are only blocked in name/role, not model/runtime.
+	if err := validateWorkspaceFields("", "", "model{}[]", "runtime*&!"); err != nil {
+		t.Errorf("model/runtime with YAML chars: expected nil, got %v", err)
+	}
+}
+
+func TestValidateWorkspaceFields_YAMLCharsAllowedInEmptyName(t *testing.T) {
+	// Empty name is fine; YAML char restriction is only on non-empty values.
+	if err := validateWorkspaceFields("", "Backend Engineer", "", ""); err != nil {
+		t.Errorf("empty name with valid role: expected nil, got %v", err)
+	}
+}