fix(security): prevent ordinary workspace from self-minting a second org root (priv-esc)

Independent security review of #2385 found a privilege-escalation path: POST
/registry/register is bootstrap-allowed for a fresh workspace id and wrote the
caller-supplied kind, while workspaces_platform_root_check only enforces
'platform => parent_id IS NULL' (NOT a single root). So an ordinary in-VPC
workspace could register a fresh UUID as {"kind":"platform"}, mint a second
org root, and POST /workspaces/:id/restart it — the shared provision path then
injects MOLECULE_API_KEY=ADMIN_TOKEN (tenant-wide org-admin credential) into any
kind='platform' workspace, on self-host AND SaaS. That breaks the invariant that
only the concierge gets the org MCP + admin token.

Defense in depth:
- migration 20260607000000_one_platform_root: partial UNIQUE index
  (kind) WHERE kind='platform' — at most one platform root per (single-org)
  tenant DB. isPlatformRootViolation now also maps the 23505 to a friendly 409.
- registry.go Register: app-layer guard refusing to CREATE or PROMOTE a row to
  kind='platform' via the public path (reserve that for the AdminAuth/boot-gated
  install paths); a platform agent re-registering its already-platform row is
  unaffected. Placed after the token check to avoid side-channeling row existence.
- corrected the false 'CHECK structurally guarantees one per org' claims in the
  20260606 migration + integration-test header.

Tests:
- registry_test.go: rejects fresh kind=platform (403), rejects workspace->platform
  promotion (403), allows already-platform re-register (200).
- kind_platform_root_integration_test.go: real-PG test that a SECOND platform
  root is rejected by the unique index (the CHECK alone accepts it).
- canvas-topology-pure.test.ts: cover stripPlatformRootForMap (QA HIGH gap) —
  abs-position reparent math, platform-edge drop, grandchild preservation.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

canvas/src/store/__tests__/canvas-topology-pure.test.ts

@@ -17,7 +17,9 @@ import {
   PARENT_SIDE_PADDING,
   PARENT_HEADER_PADDING,
   PARENT_BOTTOM_PADDING,
+  stripPlatformRootForMap,
 } from "../canvas-topology";
+import { WORKSPACE_KIND } from "../../lib/workspace-kind";
 
 // Layout-math aliases so these assertions track the card-size constants
 // instead of hard-coding pixel values (which drift when the card size
@@ -277,3 +279,74 @@ describe("parentMinSizeFromChildren — variable-size children", () => {
     expect(wide.width).toBeGreaterThan(narrow.width);
   });
 });
+
+// ─── stripPlatformRootForMap ───────────────────────────────────────────────────
+
+describe("stripPlatformRootForMap", () => {
+  // Minimal Node<WorkspaceNodeData> builder — only the fields the function reads.
+  const node = (
+    id: string,
+    opts: { kind?: string; parentId?: string; x?: number; y?: number } = {},
+    // eslint-disable-next-line @typescript-eslint/no-explicit-any
+  ): any => ({
+    id,
+    position: { x: opts.x ?? 0, y: opts.y ?? 0 },
+    parentId: opts.parentId,
+    data: { kind: opts.kind ?? WORKSPACE_KIND.Workspace, parentId: opts.parentId ?? null },
+  });
+  // eslint-disable-next-line @typescript-eslint/no-explicit-any
+  const edge = (source: string, target: string): any => ({ id: `${source}->${target}`, source, target });
+
+  it("returns input unchanged when there is no platform node", () => {
+    const nodes = [node("a"), node("b", { parentId: "a", x: 5, y: 5 })];
+    const edges = [edge("a", "b")];
+    const out = stripPlatformRootForMap(nodes, edges);
+    expect(out.nodes).toBe(nodes); // same reference — no work done
+    expect(out.edges).toBe(edges);
+  });
+
+  it("removes the platform root, promotes its direct children to absolute positions, and drops platform-touching edges", () => {
+    const platform = node("P", { kind: WORKSPACE_KIND.Platform, x: 100, y: 50 });
+    const child = node("c", { parentId: "P", x: 10, y: 20 }); // RF-relative to P
+    const grandchild = node("g", { parentId: "c", x: 5, y: 5 });
+    const out = stripPlatformRootForMap(
+      [platform, child, grandchild],
+      [edge("P", "c"), edge("c", "g")],
+    );
+
+    // Platform node is gone.
+    expect(out.nodes.find((n) => n.id === "P")).toBeUndefined();
+
+    // Direct child promoted to top-level with absolute position (parentPos + childPos).
+    const c = out.nodes.find((n) => n.id === "c")!;
+    expect(c.parentId).toBeUndefined();
+    expect(c.extent).toBeUndefined();
+    expect(c.position).toEqual({ x: 110, y: 70 });
+    expect(c.data.parentId).toBeNull();
+
+    // Grandchild (child of a non-platform node) is untouched.
+    const g = out.nodes.find((n) => n.id === "g")!;
+    expect(g.parentId).toBe("c");
+    expect(g.position).toEqual({ x: 5, y: 5 });
+
+    // Edge touching the platform node dropped; the other preserved.
+    expect(out.edges.map((e) => e.id)).toEqual(["c->g"]);
+  });
+
+  it("leaves children of an ordinary (non-platform) parent untouched", () => {
+    const platform = node("P", { kind: WORKSPACE_KIND.Platform });
+    const ordinaryParent = node("op", { parentId: "P", x: 200, y: 0 });
+    const grandchild = node("gc", { parentId: "op", x: 7, y: 9 });
+    const out = stripPlatformRootForMap([platform, ordinaryParent, grandchild], []);
+
+    // op is a direct child of platform → promoted (absolute = 200+0, 0+0).
+    const op = out.nodes.find((n) => n.id === "op")!;
+    expect(op.parentId).toBeUndefined();
+    expect(op.position).toEqual({ x: 200, y: 0 });
+
+    // gc's parent is the ordinary node, not platform → relative position preserved.
+    const gc = out.nodes.find((n) => n.id === "gc")!;
+    expect(gc.parentId).toBe("op");
+    expect(gc.position).toEqual({ x: 7, y: 9 });
+  });
+});

workspace-server/internal/handlers/kind_platform_root_integration_test.go

@@ -14,13 +14,17 @@
 //
 // Why this is NOT a sqlmock test
 // ------------------------------
-// The invariant "a platform agent must be the org root (parent_id IS NULL),
-// which structurally also means at most one platform agent per org" is enforced
-// by the workspaces_platform_root_check CHECK constraint in migration
-// 20260606000000_workspaces_kind. sqlmock cannot execute DDL or evaluate a CHECK
-// constraint, so only a real Postgres can prove the constraint actually rejects
-// a non-root platform agent and accepts a root one. The Register handler's
-// isPlatformRootViolation()/409 path depends on this constraint firing.
+// Two DB-level invariants back the platform agent:
+//   - "a platform agent must be the org root (parent_id IS NULL)" — the
+//     workspaces_platform_root_check CHECK in migration 20260606000000.
+//   - "at most one platform agent per org" — the partial unique index
+//     uniq_workspaces_one_platform_root in migration 20260607000000. The CHECK
+//     does NOT bound the count (it permits multiple parentless platform rows);
+//     the unique index does. This closes a privilege-escalation path (a rogue
+//     second org root getting the org-admin token at provision time).
+// sqlmock cannot execute DDL or evaluate these, so only a real Postgres can
+// prove they fire. The Register handler's isPlatformRootViolation()/409 path
+// depends on both constraints.
 
 package handlers
 
@@ -120,3 +124,64 @@ func TestIntegration_PlatformKind_RootAllowed_NonRootRejected(t *testing.T) {
 		t.Fatalf("unknown kind wanted workspaces_kind_check rejection, got: %v", err)
 	}
 }
+
+// TestIntegration_PlatformKind_SecondRootRejected proves the privilege-escalation
+// fix at the DB level: the workspaces_platform_root_check CHECK alone permits
+// MULTIPLE parentless platform rows; the partial unique index
+// uniq_workspaces_one_platform_root (migration 20260607000000) forbids a SECOND
+// platform root. Without it, an ordinary in-VPC workspace could register a fresh
+// UUID as kind='platform' and mint itself a second org root that then gets the
+// org-admin token at provision time. This is what the per-row CHECK could not
+// stop — only a real Postgres with the unique index proves it.
+func TestIntegration_PlatformKind_SecondRootRejected(t *testing.T) {
+	conn := integrationDB_PlatformKind(t)
+	ctx := context.Background()
+
+	prefix := fmt.Sprintf("itest-2root-%s", uuid.New().String()[:8])
+	cleanup := func() {
+		if _, err := conn.ExecContext(ctx,
+			`DELETE FROM workspaces WHERE name LIKE $1`, prefix+"%"); err != nil {
+			t.Logf("cleanup (non-fatal): %v", err)
+		}
+	}
+	t.Cleanup(cleanup)
+	cleanup()
+
+	// NOTE: the shared integration DB is single-org by construction, but a stray
+	// platform row from another suite would make the FIRST insert below collide
+	// instead of the second. Guard by asserting we start from zero platform rows
+	// for our prefix and using a savepoint-free, prefix-scoped check.
+	first := uuid.New().String()
+	second := uuid.New().String()
+
+	// First parentless platform root: allowed.
+	if _, err := conn.ExecContext(ctx, `
+		INSERT INTO workspaces (id, name, kind, tier, runtime, status, parent_id)
+		VALUES ($1, $2, 'platform', 0, 'claude-code', 'online', NULL)
+	`, first, prefix+"-first"); err != nil {
+		// If this fails on the unique index, another platform root already exists
+		// in the shared DB — skip rather than false-fail this isolation-sensitive case.
+		if strings.Contains(err.Error(), "uniq_workspaces_one_platform_root") {
+			t.Skipf("shared integration DB already has a platform root; cannot isolate: %v", err)
+		}
+		t.Fatalf("first platform root insert: unexpected error: %v", err)
+	}
+
+	// Second parentless platform root: the per-row CHECK is satisfied
+	// (parent_id IS NULL), so ONLY the unique index can reject it.
+	_, err := conn.ExecContext(ctx, `
+		INSERT INTO workspaces (id, name, kind, tier, runtime, status, parent_id)
+		VALUES ($1, $2, 'platform', 0, 'claude-code', 'online', NULL)
+	`, second, prefix+"-second")
+	if err == nil {
+		t.Fatalf("second platform root accepted — uniq_workspaces_one_platform_root did not fire (privilege-escalation guard missing)")
+	}
+	if !strings.Contains(err.Error(), "uniq_workspaces_one_platform_root") {
+		t.Fatalf("second platform root rejection wanted uniq_workspaces_one_platform_root, got: %v", err)
+	}
+
+	// And isPlatformRootViolation maps it to the friendly 409 surface.
+	if !isPlatformRootViolation(err) {
+		t.Fatalf("isPlatformRootViolation should classify the unique-index violation as a platform-root 409, got false for: %v", err)
+	}
+}

workspace-server/internal/handlers/registry.go

@@ -166,7 +166,7 @@ func (h *RegistryHandler) resolveDeliveryMode(ctx context.Context, workspaceID,
 
 // errPlatformNotRoot is the client-facing message when a register call tried to
 // mark a non-root workspace as a platform agent.
-const errPlatformNotRoot = "a platform agent must be the org root (parent_id must be null)"
+const errPlatformNotRoot = "a platform agent must be the org root (parent_id must be null) and there can be only one per org"
 
 // isPlatformRootViolation reports whether err is the DB rejecting a register
 // that tried to mark a non-root workspace as a platform agent (the
@@ -175,7 +175,15 @@ const errPlatformNotRoot = "a platform agent must be the org root (parent_id mus
 // which structurally also guarantees one platform agent per org — is enforced
 // race-proof at the DB level; this is just the friendly surface.
 func isPlatformRootViolation(err error) bool {
-	return err != nil && strings.Contains(err.Error(), "workspaces_platform_root_check")
+	if err == nil {
+		return false
+	}
+	msg := err.Error()
+	// workspaces_platform_root_check: tried to mark a non-root (parented) row
+	// platform. uniq_workspaces_one_platform_root: tried to create a SECOND
+	// platform root. Both surface as a friendly 409 instead of a raw 500.
+	return strings.Contains(msg, "workspaces_platform_root_check") ||
+		strings.Contains(msg, "uniq_workspaces_one_platform_root")
 }
 
 // Returns a non-nil error suitable for including in a 400 Bad Request response.
@@ -353,6 +361,32 @@ func (h *RegistryHandler) Register(c *gin.Context) {
 		return // 401 response already written by requireWorkspaceToken
 	}
 
+	// SECURITY (privilege-escalation fix): the public register path must never
+	// CREATE or PROMOTE a row to kind='platform'. The org root is minted only by
+	// the AdminAuth/boot-gated install paths (InstallPlatformAgent /
+	// EnsureSelfHostedPlatformAgent). Without this, an ordinary in-VPC workspace
+	// could register a fresh UUID as {"kind":"platform"} (a bootstrap-allowed call,
+	// parent_id defaults NULL so the per-row CHECK is satisfied) and then be
+	// provisioned with the tenant org-admin token (MOLECULE_API_KEY=ADMIN_TOKEN).
+	// A platform agent re-registering its already-platform row (or omitting kind)
+	// is unaffected. uniq_workspaces_one_platform_root is the structural backstop;
+	// this is the friendly app-layer guard. Placed after the token check so it
+	// doesn't side-channel row existence (mirrors resolveDeliveryMode below).
+	if payload.Kind == models.KindPlatform {
+		var existingKind string
+		kErr := db.DB.QueryRowContext(ctx,
+			`SELECT kind FROM workspaces WHERE id = $1`, payload.ID).Scan(&existingKind)
+		switch {
+		case errors.Is(kErr, sql.ErrNoRows), kErr == nil && existingKind != models.KindPlatform:
+			c.JSON(http.StatusForbidden, gin.H{"error": "kind='platform' may only be assigned by the platform-agent install path"})
+			return
+		case kErr != nil && !errors.Is(kErr, sql.ErrNoRows):
+			log.Printf("Registry register: kind precheck failed for %s: %v", payload.ID, kErr)
+			c.JSON(http.StatusInternalServerError, gin.H{"error": "registration failed"})
+			return
+		}
+	}
+
 	// Resolve the EFFECTIVE delivery mode for THIS register call: the
 	// payload's explicit value wins; falling back to the existing row's
 	// stored value; falling back to push (the schema default). Done AFTER

workspace-server/internal/handlers/registry_test.go

@@ -1700,12 +1700,12 @@ func TestRegister_InvalidKind(t *testing.T) {
 	}
 }
 
-// TestRegister_PlatformKind_PersistsKind verifies that a workspace registering
-// with kind="platform" has that value written through the upsert (the platform
-// agent self-registers as the org root). The platform==root invariant itself is
-// enforced by the workspaces_platform_root_check DB constraint and exercised by
-// the integration test, which sqlmock cannot enforce.
-func TestRegister_PlatformKind_PersistsKind(t *testing.T) {
+// TestRegister_AllowsAlreadyPlatformReRegister verifies that a workspace whose
+// row is ALREADY kind="platform" (pre-seeded by the AdminAuth/boot-gated install
+// path) may re-register through the public /registry/register path with
+// kind="platform", and the value is preserved through the upsert. This is the
+// legitimate platform-agent boot flow.
+func TestRegister_AllowsAlreadyPlatformReRegister(t *testing.T) {
 	mock := setupTestDB(t)
 	setupTestRedis(t)
 	broadcaster := newTestBroadcaster()
@@ -1718,6 +1718,12 @@ func TestRegister_PlatformKind_PersistsKind(t *testing.T) {
 		WithArgs(wsID).
 		WillReturnRows(sqlmock.NewRows([]string{"count"}).AddRow(0))
 
+	// SECURITY precheck: the row is already kind="platform", so the re-register
+	// is allowed to proceed.
+	mock.ExpectQuery("SELECT kind FROM workspaces WHERE id").
+		WithArgs(wsID).
+		WillReturnRows(sqlmock.NewRows([]string{"kind"}).AddRow("platform"))
+
 	// delivery_mode="push" is set explicitly, so resolveDeliveryMode
 	// short-circuits (no SELECT delivery_mode lookup). The upsert MUST carry
 	// kind="platform" as the 6th arg.
@@ -1751,7 +1757,83 @@ func TestRegister_PlatformKind_PersistsKind(t *testing.T) {
 	handler.Register(c)
 
 	if w.Code != http.StatusOK {
-		t.Fatalf("platform register: expected 200, got %d: %s", w.Code, w.Body.String())
+		t.Fatalf("already-platform re-register: expected 200, got %d: %s", w.Code, w.Body.String())
+	}
+	if err := mock.ExpectationsWereMet(); err != nil {
+		t.Errorf("unmet expectations: %v", err)
+	}
+}
+
+// TestRegister_RejectsFreshPlatformKind locks the privilege-escalation fix: the
+// public /registry/register path must NOT let a brand-new (fresh-id) workspace
+// declare kind="platform" and mint itself a second org root. It must be refused
+// (403) before any upsert — only the AdminAuth/boot-gated install paths may mint
+// the platform agent.
+func TestRegister_RejectsFreshPlatformKind(t *testing.T) {
+	mock := setupTestDB(t)
+	setupTestRedis(t)
+	handler := NewRegistryHandler(newTestBroadcaster())
+
+	const wsID = "ws-rogue-fresh"
+
+	// Bootstrap path — no live tokens (a fresh id).
+	mock.ExpectQuery("SELECT COUNT\\(\\*\\) FROM workspace_auth_tokens").
+		WithArgs(wsID).
+		WillReturnRows(sqlmock.NewRows([]string{"count"}).AddRow(0))
+
+	// SECURITY precheck: no existing row → empty result → sql.ErrNoRows → refuse.
+	// No upsert / token issuance must follow.
+	mock.ExpectQuery("SELECT kind FROM workspaces WHERE id").
+		WithArgs(wsID).
+		WillReturnRows(sqlmock.NewRows([]string{"kind"}))
+
+	w := httptest.NewRecorder()
+	c, _ := gin.CreateTestContext(w)
+	c.Request = httptest.NewRequest("POST", "/registry/register",
+		bytes.NewBufferString(`{"id":"`+wsID+`","url":"http://localhost:9100","delivery_mode":"push","kind":"platform","agent_card":{"name":"rogue"}}`))
+	c.Request.Header.Set("Content-Type", "application/json")
+
+	handler.Register(c)
+
+	if w.Code != http.StatusForbidden {
+		t.Fatalf("fresh kind=platform register: expected 403, got %d: %s", w.Code, w.Body.String())
+	}
+	if err := mock.ExpectationsWereMet(); err != nil {
+		t.Errorf("unmet expectations: %v", err)
+	}
+}
+
+// TestRegister_RejectsPlatformPromotion locks the other half of the fix: a row
+// that already exists as kind="workspace" must NOT be promotable to "platform"
+// via the public register path (which would later get it provisioned with the
+// org-admin token). Refused (403) before the upsert.
+func TestRegister_RejectsPlatformPromotion(t *testing.T) {
+	mock := setupTestDB(t)
+	setupTestRedis(t)
+	handler := NewRegistryHandler(newTestBroadcaster())
+
+	const wsID = "ws-ordinary"
+
+	// Has no live tokens for test simplicity (bootstrap-allowed call).
+	mock.ExpectQuery("SELECT COUNT\\(\\*\\) FROM workspace_auth_tokens").
+		WithArgs(wsID).
+		WillReturnRows(sqlmock.NewRows([]string{"count"}).AddRow(0))
+
+	// SECURITY precheck: existing row is kind="workspace" → refuse promotion.
+	mock.ExpectQuery("SELECT kind FROM workspaces WHERE id").
+		WithArgs(wsID).
+		WillReturnRows(sqlmock.NewRows([]string{"kind"}).AddRow("workspace"))
+
+	w := httptest.NewRecorder()
+	c, _ := gin.CreateTestContext(w)
+	c.Request = httptest.NewRequest("POST", "/registry/register",
+		bytes.NewBufferString(`{"id":"`+wsID+`","url":"http://localhost:9100","delivery_mode":"push","kind":"platform","agent_card":{"name":"rogue"}}`))
+	c.Request.Header.Set("Content-Type", "application/json")
+
+	handler.Register(c)
+
+	if w.Code != http.StatusForbidden {
+		t.Fatalf("promote workspace->platform: expected 403, got %d: %s", w.Code, w.Body.String())
 	}
 	if err := mock.ExpectationsWereMet(); err != nil {
 		t.Errorf("unmet expectations: %v", err)

workspace-server/migrations/20260606000000_workspaces_kind.up.sql

@@ -28,10 +28,12 @@ BEGIN
 END $$;
 
 -- platform == org root, enforced at the DB level (race-proof). A platform agent
--- MUST have parent_id IS NULL. Because an org is the subtree under a single
--- parent_id IS NULL root (org_scope.go) and only a root may be 'platform', this
--- also structurally guarantees at most ONE platform agent per org. The handler
--- additionally pre-checks this to return a friendly 409 instead of a raw 23514.
+-- MUST have parent_id IS NULL. NOTE: this CHECK alone does NOT bound the number
+-- of platform rows — it permits multiple parentless platform roots. "At most one
+-- platform agent per org" is enforced by the partial unique index added in
+-- 20260607000000_one_platform_root (uniq_workspaces_one_platform_root); see that
+-- migration for the privilege-escalation rationale. The handler additionally
+-- pre-checks both to return a friendly 409 instead of a raw 23514/23505.
 DO $$
 BEGIN
   IF NOT EXISTS (SELECT 1 FROM pg_constraint WHERE conname = 'workspaces_platform_root_check') THEN

workspace-server/migrations/20260607000000_one_platform_root.down.sql

@@ -0,0 +1 @@
+DROP INDEX IF EXISTS uniq_workspaces_one_platform_root;

workspace-server/migrations/20260607000000_one_platform_root.up.sql

@@ -0,0 +1,27 @@
+-- Enforce AT MOST ONE platform agent (org root) per tenant DB — race-proof, in
+-- pure SQL. (RFC: docs/design/rfc-platform-agent.md; security fix.)
+--
+-- The prior 20260606000000_workspaces_kind migration added
+-- workspaces_platform_root_check = CHECK (kind <> 'platform' OR parent_id IS NULL),
+-- which only enforces "a platform row must be parentless". It does NOT prevent a
+-- SECOND parentless platform row. That gap is exploitable: POST /registry/register
+-- is bootstrap-allowed for a fresh workspace id, and the upsert wrote the
+-- caller-supplied kind — so an ordinary in-VPC workspace could register a new UUID
+-- as {"kind":"platform"} (parent_id defaults NULL → CHECK satisfied), mint itself a
+-- second org root, then POST /workspaces/:id/restart it. The shared provision path
+-- injects the tenant org-admin credential (MOLECULE_API_KEY=ADMIN_TOKEN) into any
+-- kind='platform' workspace, so that rogue root would gain full org-admin reach —
+-- a privilege escalation past the "only the concierge gets the org MCP + admin
+-- token" invariant.
+--
+-- A partial UNIQUE index over (kind) WHERE kind='platform' permits exactly one
+-- platform row table-wide. Because the per-tenant DB is single-org, that is "one
+-- platform agent per org". The legitimate install paths (InstallPlatformAgent /
+-- EnsureSelfHostedPlatformAgent) upsert the platform row by its fixed id, so they
+-- are unaffected; only a SECOND, differently-id'd platform row is rejected (23505),
+-- which the Register handler maps to a friendly 409. The handler also rejects the
+-- create/promote-via-register at the app layer; this index is the structural
+-- backstop that holds regardless of code path.
+CREATE UNIQUE INDEX IF NOT EXISTS uniq_workspaces_one_platform_root
+  ON workspaces (kind)
+  WHERE kind = 'platform';