From 33e264d9a80180f422bd817f6039988e4434e325 Mon Sep 17 00:00:00 2001 From: "Molecule AI Dev Engineer A (Kimi)" Date: Mon, 1 Jun 2026 20:46:39 +0000 Subject: [PATCH] fix(registry): deny CanCommunicate for cross-tenant org roots (#1955) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Removes the root-level sibling bypass that treated any two workspaces with parent_id IS NULL as siblings. In a multi-tenant table each org root has parent_id IS NULL, so this check allowed cross-tenant communication between unrelated org roots. - Delete the caller.ParentID == nil && target.ParentID == nil fast-path - Update doc comment: root-level workspaces in different orgs are NOT siblings - Rename/adapt test: root siblings → cross-tenant roots denied Fixes #1955 Related: #1953, PR #1954, OFFSEC-015 Co-Authored-By: Claude Opus 4.7 --- workspace-server/internal/registry/access.go | 10 ++++------ workspace-server/internal/registry/access_test.go | 9 +++++---- 2 files changed, 9 insertions(+), 10 deletions(-) diff --git a/workspace-server/internal/registry/access.go b/workspace-server/internal/registry/access.go index 56a9c419e..a36c9d809 100644 --- a/workspace-server/internal/registry/access.go +++ b/workspace-server/internal/registry/access.go @@ -62,7 +62,7 @@ func isAncestorOf(ancestorID, childID string) bool { // the org hierarchy. The rules: // // - self → self -// - siblings (same parent, including both root-level) +// - siblings (same parent; root-level workspaces in different orgs are NOT siblings) // - any ancestor → any descendant (e.g. PM → Backend Engineer) // - any descendant → any ancestor (e.g. Security Auditor → PM) // @@ -94,15 +94,13 @@ func CanCommunicate(callerID, targetID string) bool { return false } - // Siblings — same parent (including root-level where both have no parent) + // Siblings — same parent. Root-level workspaces in different orgs + // are NOT siblings (they have no shared parent_id because each org + // root is its own tree; parent_id IS NULL does not mean "same org"). if caller.ParentID != nil && target.ParentID != nil && *caller.ParentID == *target.ParentID { return true } - // Root-level siblings — both have no parent - if caller.ParentID == nil && target.ParentID == nil { - return true - } // Direct parent → child (fast path; avoids the ancestor walk) if target.ParentID != nil && caller.ID == *target.ParentID { diff --git a/workspace-server/internal/registry/access_test.go b/workspace-server/internal/registry/access_test.go index b01a14691..963e1af5a 100644 --- a/workspace-server/internal/registry/access_test.go +++ b/workspace-server/internal/registry/access_test.go @@ -63,14 +63,15 @@ func TestCanCommunicate_Siblings(t *testing.T) { } } -func TestCanCommunicate_RootSiblings(t *testing.T) { +func TestCanCommunicate_Denied_CrossTenantRoots(t *testing.T) { mock := setupMockDB(t) - // Both at root level (no parent) + // Two different org roots (both parent_id IS NULL) must NOT + // communicate — parent_id IS NULL does not mean "same org". expectLookup(mock, "ws-a", nil) expectLookup(mock, "ws-b", nil) - if !CanCommunicate("ws-a", "ws-b") { - t.Error("root-level siblings should communicate") + if CanCommunicate("ws-a", "ws-b") { + t.Error("cross-tenant org roots should NOT communicate") } } -- 2.52.0