Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
70 changes: 66 additions & 4 deletions platform/internal/registry/access.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,13 @@ import (
"github.com/Molecule-AI/molecule-monorepo/platform/internal/db"
)

// maxAncestorWalk caps the depth of the parent-chain walk in
// CanCommunicate. Org trees are realistically 3-5 deep
// (PM → Dev Lead → Backend Engineer is depth 3); 32 is a safety
// ceiling so a malformed cycle in the workspaces table can't loop
// forever.
const maxAncestorWalk = 32

type workspaceRef struct {
ID string
ParentID *string
Expand All @@ -26,8 +33,51 @@ func getWorkspaceRef(id string) (*workspaceRef, error) {
return &ws, nil
}

// CanCommunicate checks if two workspaces can talk to each other
// based on the hierarchy rules: siblings, parent-child, root-level siblings.
// isAncestorOf returns true if `ancestorID` is found anywhere on the
// parent-chain walk starting from `childID`. Walks at most maxAncestorWalk
// steps so a corrupt parent-cycle cannot loop forever. Returns false on any
// DB lookup error (logged) — fail-secure.
func isAncestorOf(ancestorID, childID string) bool {
current := childID
for i := 0; i < maxAncestorWalk; i++ {
ref, err := getWorkspaceRef(current)
if err != nil {
log.Printf("isAncestorOf: walk lookup %s: %v", current, err)
return false
}
if ref.ParentID == nil {
return false
}
if *ref.ParentID == ancestorID {
return true
}
current = *ref.ParentID
}
log.Printf("isAncestorOf: walk exceeded maxAncestorWalk=%d from %s — corrupt parent chain?",
maxAncestorWalk, childID)
return false
}

// CanCommunicate checks if two workspaces can talk to each other based on
// the org hierarchy. The rules:
//
// - self → self
// - siblings (same parent, including both root-level)
// - any ancestor → any descendant (e.g. PM → Backend Engineer)
// - any descendant → any ancestor (e.g. Security Auditor → PM)
//
// The third and fourth rules generalise the previous "direct parent ↔
// child" check. Originally this was strict 1-step parent/child only,
// which broke the audit-routing contract: Security Auditor (under Dev
// Lead, under PM) could not call delegate_task on PM to deliver an
// audit_summary, so it fell back to delegating to Dev Lead — bypassing
// PM's category_routing entirely.
//
// The relaxation preserves the hierarchy intent (no horizontal cross-team
// chatter — Frontend Engineer cannot directly message Backend Engineer
// unless they share a parent, which they do under Dev Lead) while
// unblocking the leadership-chain pattern that is fundamental to how
// audit summaries fan out across the org.
func CanCommunicate(callerID, targetID string) bool {
if callerID == targetID {
return true
Expand All @@ -54,15 +104,27 @@ func CanCommunicate(callerID, targetID string) bool {
return true
}

// Parent talking to child
// Direct parent → child (fast path; avoids the ancestor walk)
if target.ParentID != nil && caller.ID == *target.ParentID {
return true
}

// Child talking up to parent
// Direct child → parent (fast path)
if caller.ParentID != nil && target.ID == *caller.ParentID {
return true
}

// Distant ancestor → descendant: caller is somewhere up target's chain.
// Triggers extra DB lookups, only reached when the fast paths above didn't match.
if target.ParentID != nil && isAncestorOf(callerID, *target.ParentID) {
return true
}

// Distant descendant → ancestor: target is somewhere up caller's chain.
// (e.g. Security Auditor → PM, where Security Auditor's parent is Dev Lead.)
if caller.ParentID != nil && isAncestorOf(targetID, *caller.ParentID) {
return true
}

return false
}
88 changes: 80 additions & 8 deletions platform/internal/registry/access_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -97,9 +97,13 @@ func TestCanCommunicate_ChildToParent(t *testing.T) {

func TestCanCommunicate_Denied_DifferentParents(t *testing.T) {
mock := setupMockDB(t)
// ws-a (parent: p1) and ws-b (parent: p2) — not siblings
// ws-a (parent: p1) and ws-b (parent: p2) — not siblings, no shared ancestor.
expectLookup(mock, "ws-a", ptr("p1"))
expectLookup(mock, "ws-b", ptr("p2"))
// Walk #1: isAncestorOf(ws-a, p2) → p2 is parentless, false.
expectLookup(mock, "p2", nil)
// Walk #2: isAncestorOf(ws-b, p1) → p1 is parentless, false.
expectLookup(mock, "p1", nil)

if CanCommunicate("ws-a", "ws-b") {
t.Error("workspaces with different parents should NOT communicate")
Expand All @@ -108,9 +112,15 @@ func TestCanCommunicate_Denied_DifferentParents(t *testing.T) {

func TestCanCommunicate_Denied_CousinToRoot(t *testing.T) {
mock := setupMockDB(t)
// ws-child (parent: ws-mid) and ws-root (no parent, NOT ws-mid)
// ws-child (parent: ws-mid, which has its own root ws-other-root) and
// ws-root (a different parentless workspace).
// The ancestor walk from ws-child should reach ws-other-root but never
// ws-root, so communication is denied.
expectLookup(mock, "ws-child", ptr("ws-mid"))
expectLookup(mock, "ws-root", nil)
// Ancestor walk: starts at *caller.ParentID = ws-mid. Walks ws-mid → ws-other-root → nil.
expectLookup(mock, "ws-mid", ptr("ws-other-root"))
expectLookup(mock, "ws-other-root", nil)

if CanCommunicate("ws-child", "ws-root") {
t.Error("child should NOT communicate with unrelated root workspace")
Expand All @@ -136,13 +146,75 @@ func TestCanCommunicate_Denied_TargetNotFound(t *testing.T) {
}
}

func TestCanCommunicate_Denied_Grandchild(t *testing.T) {
func TestCanCommunicate_Allowed_GrandparentToGrandchild(t *testing.T) {
mock := setupMockDB(t)
// ws-grandparent and ws-grandchild (parent: ws-mid, NOT ws-grandparent)
expectLookup(mock, "ws-grandparent", nil)
expectLookup(mock, "ws-grandchild", ptr("ws-mid"))
// PM (no parent) → Backend Engineer (parent: Dev Lead, parent: PM).
// Originally rejected ("grandparent should NOT communicate with grandchild
// directly") — that broke audit_summary routing because Security Auditor
// could not delegate up to PM. The hierarchy is now ancestor↔descendant.
expectLookup(mock, "ws-pm", nil)
expectLookup(mock, "ws-be", ptr("ws-dl"))
// Ancestor walk: target.ParentID = ws-dl. isAncestorOf(ws-pm, ws-dl).
// Walks ws-dl → ws-pm → match. (Walk lookup #1: ws-dl.)
expectLookup(mock, "ws-dl", ptr("ws-pm"))

if !CanCommunicate("ws-pm", "ws-be") {
t.Error("PM should be able to communicate with Backend Engineer (descendant)")
}
}

func TestCanCommunicate_Allowed_GrandchildToGrandparent(t *testing.T) {
mock := setupMockDB(t)
// Security Auditor (parent: Dev Lead) → PM (parent of Dev Lead).
// This is the Security Auditor → PM audit_summary delivery path.
expectLookup(mock, "ws-sec", ptr("ws-dl"))
expectLookup(mock, "ws-pm", nil)
// Direct parent → child fast path: target.ParentID = nil, skip.
// Direct child → parent: caller.ParentID = ws-dl, target.ID = ws-pm,
// ws-dl != ws-pm, skip.
// Distant ancestor → descendant: target.ParentID = nil, skip.
// Distant descendant → ancestor: caller.ParentID = ws-dl. Walks
// isAncestorOf(ws-pm, ws-dl) → looks up ws-dl → returns ws-pm → match.
expectLookup(mock, "ws-dl", ptr("ws-pm"))

if !CanCommunicate("ws-sec", "ws-pm") {
t.Error("Security Auditor should be able to send audit_summary up to PM")
}
}

if CanCommunicate("ws-grandparent", "ws-grandchild") {
t.Error("grandparent should NOT communicate with grandchild directly")
func TestCanCommunicate_Allowed_DeepAncestor(t *testing.T) {
mock := setupMockDB(t)
// Four-level chain: ws-leaf (parent: ws-l3, parent: ws-l2, parent: ws-l1).
// ws-leaf → ws-l1 should be allowed.
expectLookup(mock, "ws-leaf", ptr("ws-l3"))
expectLookup(mock, "ws-l1", nil)
// Distant descendant → ancestor walk: starts at ws-l3.
// ws-l3 → ws-l2: not ws-l1, continue.
// ws-l2 → ws-l1: match!
expectLookup(mock, "ws-l3", ptr("ws-l2"))
expectLookup(mock, "ws-l2", ptr("ws-l1"))

if !CanCommunicate("ws-leaf", "ws-l1") {
t.Error("4-level descendant should reach root ancestor")
}
}

func TestCanCommunicate_Denied_UnrelatedAncestors(t *testing.T) {
mock := setupMockDB(t)
// Two separate org subtrees:
// tree A: ws-a-leaf → ws-a-mid → ws-a-root
// tree B: ws-b-leaf → ws-b-mid → ws-b-root
// ws-a-leaf → ws-b-root must be denied even though both have parents
// (no shared ancestor).
expectLookup(mock, "ws-a-leaf", ptr("ws-a-mid"))
expectLookup(mock, "ws-b-root", nil)
// Walk: isAncestorOf(ws-b-root, ws-a-mid).
// ws-a-mid → ws-a-root: not ws-b-root, continue.
// ws-a-root has no parent → false.
expectLookup(mock, "ws-a-mid", ptr("ws-a-root"))
expectLookup(mock, "ws-a-root", nil)

if CanCommunicate("ws-a-leaf", "ws-b-root") {
t.Error("workspaces in different subtrees should NOT communicate via the walk")
}
}
Loading