Merge pull request #162 from Molecule-AI/fix/issue-138-field-whitelist

fix(auth): #138 — field-level authz on PATCH /workspaces/:id (canvas regression fix)

platform/internal/handlers/workspace.go

@@ -390,6 +390,20 @@ func (h *WorkspaceHandler) State(c *gin.Context) {
 	})
 }
 
+// sensitiveUpdateFields gates the #120/#138 field-level auth check inside
+// Update. Any key in this set requires a valid bearer token even when the
+// rest of the route is open — tier is a resource-escalation vector,
+// parent_id rewrites the A2A hierarchy, runtime swaps the container image
+// on next restart, workspace_dir redirects host bind-mounts. Cosmetic
+// fields (name, role, x, y, canvas) do not appear here and pass through
+// unauthenticated so canvas drag-reposition and inline rename keep working.
+var sensitiveUpdateFields = map[string]struct{}{
+	"tier":          {},
+	"parent_id":     {},
+	"runtime":       {},
+	"workspace_dir": {},
+}
+
 // Update handles PATCH /workspaces/:id
 func (h *WorkspaceHandler) Update(c *gin.Context) {
 	id := c.Param("id")
@@ -402,6 +416,35 @@ func (h *WorkspaceHandler) Update(c *gin.Context) {
 
 	ctx := c.Request.Context()
 
+	// #138 field-level authz: PATCH /workspaces/:id is on the open router so
+	// canvas drag-reposition (cookie-based, no bearer token) keeps working,
+	// BUT the sensitive fields below require a valid bearer via the usual
+	// admin-token check. Lazy-bootstrap: if no live admin tokens exist at all
+	// (fresh install) the check is a no-op and everyone passes through.
+	for field := range body {
+		if _, sensitive := sensitiveUpdateFields[field]; !sensitive {
+			continue
+		}
+		hasLive, hlErr := wsauth.HasAnyLiveTokenGlobal(ctx, db.DB)
+		if hlErr != nil {
+			log.Printf("wsauth: Update HasAnyLiveTokenGlobal failed: %v — allowing request", hlErr)
+			break
+		}
+		if !hasLive {
+			break // fresh install — fail-open
+		}
+		tok := wsauth.BearerTokenFromHeader(c.GetHeader("Authorization"))
+		if tok == "" {
+			c.JSON(http.StatusUnauthorized, gin.H{"error": "admin auth required for field: " + field})
+			return
+		}
+		if err := wsauth.ValidateAnyToken(ctx, db.DB, tok); err != nil {
+			c.JSON(http.StatusUnauthorized, gin.H{"error": "invalid admin auth token"})
+			return
+		}
+		break // one successful validation covers the whole body
+	}
+
 	// #120: guard — return 404 for nonexistent workspace IDs instead of
 	// silently applying zero-row UPDATEs and returning 200.
 	var exists bool

platform/internal/handlers/workspace_test.go

@@ -656,3 +656,88 @@ func TestWorkspaceState_ValidTokenReturnsStatus(t *testing.T) {
 		t.Errorf("status should be 'degraded', got %v", body["status"])
 	}
 }
+
+// ── #138 field-level auth tests ─────────────────────────────────────────────
+// Cosmetic PATCH (name/x/y/role) stays open so canvas drag-reposition works
+// without a bearer token. Sensitive fields (tier/parent_id/runtime/
+// workspace_dir) require a valid admin bearer once any live token exists.
+
+func TestWorkspaceUpdate_CosmeticField_NoBearer_FailOpen_NoTokens(t *testing.T) {
+	mock := setupTestDB(t)
+	setupTestRedis(t)
+	broadcaster := newTestBroadcaster()
+	handler := NewWorkspaceHandler(broadcaster, nil, "http://localhost:8080", t.TempDir())
+
+	// Body contains only cosmetic field → no wsauth probe ever fires.
+	mock.ExpectQuery("SELECT EXISTS.*workspaces WHERE id").
+		WithArgs("ws-cosmetic").
+		WillReturnRows(sqlmock.NewRows([]string{"exists"}).AddRow(true))
+	mock.ExpectExec("UPDATE workspaces SET name").
+		WithArgs("ws-cosmetic", "Cosmetic").
+		WillReturnResult(sqlmock.NewResult(0, 1))
+
+	w := httptest.NewRecorder()
+	c, _ := gin.CreateTestContext(w)
+	c.Params = gin.Params{{Key: "id", Value: "ws-cosmetic"}}
+	c.Request = httptest.NewRequest("PATCH", "/workspaces/ws-cosmetic",
+		bytes.NewBufferString(`{"name":"Cosmetic"}`))
+	c.Request.Header.Set("Content-Type", "application/json")
+	handler.Update(c)
+
+	if w.Code != http.StatusOK {
+		t.Errorf("cosmetic PATCH (no bearer) should pass; got %d: %s", w.Code, w.Body.String())
+	}
+}
+
+func TestWorkspaceUpdate_SensitiveField_NoBearer_TokensExist_Rejected(t *testing.T) {
+	mock := setupTestDB(t)
+	setupTestRedis(t)
+	broadcaster := newTestBroadcaster()
+	handler := NewWorkspaceHandler(broadcaster, nil, "http://localhost:8080", t.TempDir())
+
+	// HasAnyLiveTokenGlobal returns 1 — tokens exist on the platform.
+	mock.ExpectQuery("SELECT COUNT.*FROM workspace_auth_tokens").
+		WillReturnRows(sqlmock.NewRows([]string{"count"}).AddRow(1))
+
+	w := httptest.NewRecorder()
+	c, _ := gin.CreateTestContext(w)
+	c.Params = gin.Params{{Key: "id", Value: "ws-sensitive"}}
+	c.Request = httptest.NewRequest("PATCH", "/workspaces/ws-sensitive",
+		bytes.NewBufferString(`{"tier":4}`))
+	c.Request.Header.Set("Content-Type", "application/json")
+	// No Authorization header — must fail closed.
+	handler.Update(c)
+
+	if w.Code != http.StatusUnauthorized {
+		t.Errorf("sensitive PATCH without bearer: got %d, want 401 (%s)", w.Code, w.Body.String())
+	}
+}
+
+func TestWorkspaceUpdate_SensitiveField_NoTokensYet_FailOpen(t *testing.T) {
+	mock := setupTestDB(t)
+	setupTestRedis(t)
+	broadcaster := newTestBroadcaster()
+	handler := NewWorkspaceHandler(broadcaster, nil, "http://localhost:8080", t.TempDir())
+
+	// HasAnyLiveTokenGlobal returns 0 — fresh install, fail-open.
+	mock.ExpectQuery("SELECT COUNT.*FROM workspace_auth_tokens").
+		WillReturnRows(sqlmock.NewRows([]string{"count"}).AddRow(0))
+	mock.ExpectQuery("SELECT EXISTS.*workspaces WHERE id").
+		WithArgs("ws-bootstrap").
+		WillReturnRows(sqlmock.NewRows([]string{"exists"}).AddRow(true))
+	mock.ExpectExec("UPDATE workspaces SET tier").
+		WithArgs("ws-bootstrap", float64(4)).
+		WillReturnResult(sqlmock.NewResult(0, 1))
+
+	w := httptest.NewRecorder()
+	c, _ := gin.CreateTestContext(w)
+	c.Params = gin.Params{{Key: "id", Value: "ws-bootstrap"}}
+	c.Request = httptest.NewRequest("PATCH", "/workspaces/ws-bootstrap",
+		bytes.NewBufferString(`{"tier":4}`))
+	c.Request.Header.Set("Content-Type", "application/json")
+	handler.Update(c)
+
+	if w.Code != http.StatusOK {
+		t.Errorf("bootstrap fail-open: got %d, want 200 (%s)", w.Code, w.Body.String())
+	}
+}

platform/internal/router/router.go

@@ -99,22 +99,26 @@ func Setup(hub *ws.Hub, broadcaster *events.Broadcaster, prov *provisioner.Provi
 	// without a token (used by WorkspaceNode polling and health checks).
 	r.GET("/workspaces/:id", wh.Get)
 
-	// C1 + C20 + C18-adjacent + #120: workspace list and ALL mutating operations
-	// gated behind AdminAuth — any valid workspace bearer token grants access.
+	// PATCH /workspaces/:id — back on the open router per #138. Canvas
+	// drag-reposition uses session cookies not bearer tokens; gating the
+	// whole route behind AdminAuth broke drag-to-reposition and inline
+	// rename. Field-level authz lives inside WorkspaceHandler.Update:
+	//   - {x, y, canvas} only → passthrough (canvas position persist)
+	//   - name / role       → passthrough (inline rename)
+	//   - tier / parent_id / runtime / workspace_dir → require bearer token
+	// The #120 escalation vectors stay locked; only cosmetic fields are open.
+	r.PATCH("/workspaces/:id", wh.Update)
+
+	// C1 + C20: workspace list and life-cycle mutations gated behind AdminAuth.
 	// Fail-open when no tokens exist anywhere (fresh install / pre-Phase-30).
-	// This blocks:
+	// Blocks:
 	//   C1   — unauthenticated GET /workspaces (workspace topology exposure)
 	//   C20  — unauthenticated DELETE /workspaces/:id (mass-deletion attack)
 	//          unauthenticated POST /workspaces (workspace creation)
-	//   #120 — unauthenticated PATCH /workspaces/:id (tier escalation, parent_id
-	//          hierarchy manipulation, runtime swap, workspace_dir path hijack)
-	// NOTE: canvas position-persist (PATCH with {x,y}) uses the same AdminAuth
-	// token already required for GET /workspaces list on initial load.
 	{
 		wsAdmin := r.Group("", middleware.AdminAuth(db.DB))
 		wsAdmin.GET("/workspaces", wh.List)
 		wsAdmin.POST("/workspaces", wh.Create)
-		wsAdmin.PATCH("/workspaces/:id", wh.Update)
 		wsAdmin.DELETE("/workspaces/:id", wh.Delete)
 	}