Validate deferred component pointer is ancestor (#9388)

## What changed? Validate deferred component pointer is an ancestor component. ## Why? Components should not be able to declare parent-child relationship with components not managed in the same lifecycle (ie. different tree, sibling component). ## How did you test it? - [X] built - [X] run locally and tested manually - [X] covered by existing tests - [X] added new unit test(s) - [ ] added new functional test(s)
2026-08-30 18:41:49 -07:00 · 2026-04-22 11:36:02 -04:00
parent a42916b300
commit 06f38afcc4
5 changed files with 177 additions and 85 deletions
--- a/chasm/field.go
+++ b/chasm/field.go
@@ -51,8 +51,10 @@ func NewComponentField[C Component](
 }

 // ComponentPointerTo returns a CHASM field populated with a pointer to the given
-// component. Pointers are resolved at the time the transaction is closed, and the
-// transaction will fail if any pointers cannot be resolved.
+// component. The target component must be a proper ancestor of the referring
+// component within the same component tree. Pointers to non-ancestor components
+// (e.g., siblings, descendants, or components from a different tree) will cause
+// the transaction to fail when it is closed.
 func ComponentPointerTo[C Component](
 	ctx MutableContext,
 	c C,
--- a/chasm/field_test.go
+++ b/chasm/field_test.go
@@ -191,13 +191,6 @@ func (s *fieldSuite) TestDeferredPointerResolution() {
 		HandleGetWorkflowKey:      func() definition.WorkflowKey { return workflowKey },
 	}

-	// Create component structure that will simulate StartExecution scenario.
-	sc2 := &TestSubComponent2{
-		SubComponent2Data: &protoMessageType{
-			CreateRequestId: "sub-component2-data",
-		},
-	}
-
 	sc1 := &TestSubComponent1{
 		SubComponent1Data: &protoMessageType{
 			CreateRequestId: "sub-component1-data",
@@ -215,52 +208,33 @@ func (s *fieldSuite) TestDeferredPointerResolution() {
 	s.NoError(err)

 	// Get components from tree to mark nodes as needing sync.
-
 	rootComponentInterface, err := rootNode.Component(ctx, ComponentRef{})
 	s.NoError(err)
 	rootComponent = rootComponentInterface.(*TestComponent)
 	sc1 = rootComponent.SubComponent1.Get(ctx)

-	// Create deferred pointers.
-	sc1.SubComponent2Pointer = ComponentPointerTo(ctx, sc2)
-	rootComponent.SubComponent2 = NewComponentField(nil, sc2)
+	// sc1 (child) points to rootComponent (parent) via component pointer.
+	sc1.RootPointer = ComponentPointerTo(ctx, rootComponent)

-	data := &protoMessageType{CreateRequestId: "sub-data-1"}
-	sc1.DataPointer = DataPointerTo(ctx, data)
-	rootComponent.SubData1 = NewDataField(ctx, data)
-
-	// Verify it's a deferred pointer storing the component directly.
-	s.Equal(fieldTypeDeferredPointer, sc1.SubComponent2Pointer.Internal.fieldType())
-	s.Equal(fieldTypeDeferredPointer, sc1.DataPointer.Internal.fieldType())
-	s.Equal(sc2, sc1.SubComponent2Pointer.Internal.v)
-	s.Equal(data, sc1.DataPointer.Internal.v)
+	// Verify deferred state.
+	s.Equal(fieldTypeDeferredPointer, sc1.RootPointer.Internal.fieldType())
+	s.Equal(rootComponent, sc1.RootPointer.Internal.v)

 	// CloseTransaction should resolve the deferred pointer.
 	mutations, err := rootNode.CloseTransaction()
 	s.NoError(err)
 	s.NotEmpty(mutations.UpdatedNodes)

-	// Verify the pointers were resolved to a regular pointer with path.
-	s.Equal(fieldTypePointer, sc1.SubComponent2Pointer.Internal.fieldType())
-	s.Equal(fieldTypePointer, sc1.DataPointer.Internal.fieldType())
+	// Verify the pointer was resolved to a regular pointer with path.
+	s.Equal(fieldTypePointer, sc1.RootPointer.Internal.fieldType())

-	cResolvedPath, ok := sc1.SubComponent2Pointer.Internal.v.([]string)
+	cResolvedPath, ok := sc1.RootPointer.Internal.v.([]string)
 	s.True(ok)
-	s.Equal([]string{"SubComponent2"}, cResolvedPath)
-	dResolvedPath, ok := sc1.DataPointer.Internal.v.([]string)
-	s.True(ok)
-	s.Equal([]string{"SubData1"}, dResolvedPath)
+	s.Equal([]string{}, cResolvedPath)

-	// Verify we can dereference the pointers.
-	resolvedComponent := sc1.SubComponent2Pointer.Get(ctx)
-	s.Equal(sc2, resolvedComponent)
-
-	// TODO - this doesn't resolve, but I've manually verified the tree structure looks correct
-	// TODO
-	// TODO
-	// resolvedData, err := sc1.DataPointer.Get(ctx)
-	// s.NoError(err)
-	// s.Equal(sc2.SubComponent2Data, resolvedData)
+	// Verify we can dereference the component pointer.
+	resolvedComponent := sc1.RootPointer.Get(ctx)
+	s.Equal(rootComponent, resolvedComponent)
 }

 func (s *fieldSuite) TestMixedPointerScenario() {
@@ -275,13 +249,13 @@ func (s *fieldSuite) TestMixedPointerScenario() {
 		HandleGetWorkflowKey:      func() definition.WorkflowKey { return workflowKey },
 	}

-	existingComponent := &TestSubComponent11{
-		SubComponent11Data: &protoMessageType{CreateRequestId: "existing-component"},
+	sc11 := &TestSubComponent11{
+		SubComponent11Data: &protoMessageType{CreateRequestId: "sub-component11-data"},
 	}

 	sc1 := &TestSubComponent1{
 		SubComponent1Data: &protoMessageType{CreateRequestId: "sub-component1-data"},
-		SubComponent11:    NewComponentField(nil, existingComponent),
+		SubComponent11:    NewComponentField(nil, sc11),
 	}

 	rootComponent := &TestComponent{
@@ -296,49 +270,42 @@ func (s *fieldSuite) TestMixedPointerScenario() {
 	rootComponentInterface, err := rootNode.Component(ctx, ComponentRef{})
 	s.NoError(err)
 	rootComponent = rootComponentInterface.(*TestComponent)
+	sc1 = rootComponent.SubComponent1.Get(ctx)
+	sc11 = sc1.SubComponent11.Get(ctx)

-	rootComponent.SubComponent11Pointer = ComponentPointerTo(ctx, existingComponent)
+	// Transaction 1: sc11 points to root (grandparent).
+	sc11.GrandparentPointer = ComponentPointerTo(ctx, rootComponent)

-	// Close the transaction to resolve SubComponent11Pointer's field to existingComponent.
 	_, err = rootNode.CloseTransaction()
 	s.NoError(err)
-	s.Equal(fieldTypePointer, rootComponent.SubComponent11Pointer.Internal.fieldType())
-
-	// For a new transaction, get the components from the tree again,
-	// otherwise those nodes will not be marked as dirty.
+	s.Equal(fieldTypePointer, sc11.GrandparentPointer.Internal.fieldType())

+	// Transaction 2: sc1 points to root (parent).
 	ctx2 := NewMutableContext(context.Background(), rootNode)
 	rootComponentInterface, err = rootNode.Component(ctx2, ComponentRef{})
 	s.NoError(err)

 	rootComponent = rootComponentInterface.(*TestComponent)
 	sc1 = rootComponent.SubComponent1.Get(ctx2)
+	sc11 = sc1.SubComponent11.Get(ctx2)

-	// Now, add a new component and deferred pointer for it.
-	newComponent := &TestSubComponent2{
-		SubComponent2Data: &protoMessageType{CreateRequestId: "new-component"},
-	}
+	sc1.RootPointer = ComponentPointerTo(ctx2, rootComponent)

-	sc1.SubComponent2Pointer = ComponentPointerTo(ctx2, newComponent)
-
-	// Now add the component to the tree so it can be resolved during CloseTransaction.
-	rootComponent.SubComponent2 = NewComponentField(ctx, newComponent)
-
-	s.Equal(fieldTypePointer, rootComponent.SubComponent11Pointer.Internal.fieldType())
-	s.Equal(fieldTypeDeferredPointer, sc1.SubComponent2Pointer.Internal.fieldType())
+	s.Equal(fieldTypePointer, sc11.GrandparentPointer.Internal.fieldType())
+	s.Equal(fieldTypeDeferredPointer, sc1.RootPointer.Internal.fieldType())

 	_, err = rootNode.CloseTransaction()
 	s.NoError(err)

 	// Ensure both pointers have been resolved.
-	s.Equal(fieldTypePointer, rootComponent.SubComponent11Pointer.Internal.fieldType())
-	s.Equal(fieldTypePointer, sc1.SubComponent2Pointer.Internal.fieldType())
+	s.Equal(fieldTypePointer, sc11.GrandparentPointer.Internal.fieldType())
+	s.Equal(fieldTypePointer, sc1.RootPointer.Internal.fieldType())

-	resolved1 := rootComponent.SubComponent11Pointer.Get(ctx2)
-	s.Equal(existingComponent, resolved1)
+	resolved1 := sc11.GrandparentPointer.Get(ctx2)
+	s.Equal(rootComponent, resolved1)

-	resolved2 := sc1.SubComponent2Pointer.Get(ctx2)
-	s.Equal(newComponent, resolved2)
+	resolved2 := sc1.RootPointer.Get(ctx2)
+	s.Equal(rootComponent, resolved2)
 }

 func (s *fieldSuite) TestUnresolvableDeferredPointerError() {
@@ -383,3 +350,89 @@ func (s *fieldSuite) TestUnresolvableDeferredPointerError() {
 	s.Error(err)
 	s.Contains(err.Error(), "failed to resolve deferred pointer during transaction close")
 }
+
+func (s *fieldSuite) TestNonAncestorComponentPointerRejected() {
+	workflowKey := definition.NewWorkflowKey(
+		primitives.NewUUID().String(),
+		primitives.NewUUID().String(),
+		primitives.NewUUID().String(),
+	)
+	s.nodeBackend = &MockNodeBackend{
+		HandleNextTransitionCount: func() int64 { return 1 },
+		HandleGetCurrentVersion:   func() int64 { return 1 },
+		HandleGetWorkflowKey:      func() definition.WorkflowKey { return workflowKey },
+	}
+
+	s.logger.(*testlogger.TestLogger).
+		Expect(testlogger.Error, "failed to resolve deferred pointer during transaction close")
+
+	sc11 := &TestSubComponent11{
+		SubComponent11Data: &protoMessageType{CreateRequestId: "sub-component11-data"},
+	}
+
+	sc1 := &TestSubComponent1{
+		SubComponent1Data: &protoMessageType{CreateRequestId: "sub-component1-data"},
+		SubComponent11:    NewComponentField(nil, sc11),
+	}
+
+	rootComponent := &TestComponent{
+		ComponentData: &protoMessageType{CreateRequestId: "component-data"},
+		SubComponent1: NewComponentField(nil, sc1),
+	}
+
+	rootNode, ctx, err := s.setupComponentWithTree(rootComponent)
+	s.NoError(err)
+
+	rootComponentInterface, err := rootNode.Component(ctx, ComponentRef{})
+	s.NoError(err)
+	rootComponent = rootComponentInterface.(*TestComponent)
+	sc1 = rootComponent.SubComponent1.Get(ctx)
+	sc11 = sc1.SubComponent11.Get(ctx)
+
+	// Root pointing to descendant sc11 should be rejected.
+	rootComponent.SubComponent11Pointer = ComponentPointerTo(ctx, sc11)
+
+	_, err = rootNode.CloseTransaction()
+	s.Error(err)
+	s.Contains(err.Error(), "is not an ancestor of component")
+}
+
+func (s *fieldSuite) TestChildComponentPointerRejected() {
+	workflowKey := definition.NewWorkflowKey(
+		primitives.NewUUID().String(),
+		primitives.NewUUID().String(),
+		primitives.NewUUID().String(),
+	)
+	s.nodeBackend = &MockNodeBackend{
+		HandleNextTransitionCount: func() int64 { return 1 },
+		HandleGetCurrentVersion:   func() int64 { return 1 },
+		HandleGetWorkflowKey:      func() definition.WorkflowKey { return workflowKey },
+	}
+
+	s.logger.(*testlogger.TestLogger).
+		Expect(testlogger.Error, "failed to resolve deferred pointer during transaction close")
+
+	sc1 := &TestSubComponent1{
+		SubComponent1Data: &protoMessageType{CreateRequestId: "sub-component1-data"},
+	}
+
+	rootComponent := &TestComponent{
+		ComponentData: &protoMessageType{CreateRequestId: "component-data"},
+		SubComponent1: NewComponentField(nil, sc1),
+	}
+
+	rootNode, ctx, err := s.setupComponentWithTree(rootComponent)
+	s.NoError(err)
+
+	rootComponentInterface, err := rootNode.Component(ctx, ComponentRef{})
+	s.NoError(err)
+	rootComponent = rootComponentInterface.(*TestComponent)
+	sc1 = rootComponent.SubComponent1.Get(ctx)
+
+	// Root pointing to child sc1 via interface pointer should be rejected.
+	rootComponent.SubComponentInterfacePointer = ComponentPointerTo[Component](ctx, sc1)
+
+	_, err = rootNode.CloseTransaction()
+	s.Error(err)
+	s.Contains(err.Error(), "is not an ancestor of component")
+}
--- a/chasm/test_component_test.go
+++ b/chasm/test_component_test.go
@@ -40,12 +40,11 @@ type (
 	TestSubComponent1 struct {
 		UnimplementedComponent

-		SubComponent1Data    *protoMessageType
-		SubComponent11       Field[*TestSubComponent11]
-		SubComponent11_2     Field[*TestSubComponent11]
-		SubData11            Field[*protoMessageType] // Random proto message.
-		SubComponent2Pointer Field[*TestSubComponent2]
-		DataPointer          Field[*protoMessageType]
+		SubComponent1Data *protoMessageType
+		SubComponent11    Field[*TestSubComponent11]
+		SubComponent11_2  Field[*TestSubComponent11]
+		SubData11         Field[*protoMessageType] // Random proto message.
+		RootPointer       Field[*TestComponent]

 		ParentPtr ParentPtr[*TestComponent]
 	}
@@ -54,6 +53,8 @@ type (
 		UnimplementedComponent

 		SubComponent11Data *protoMessageType
+		GrandparentPointer Field[*TestComponent]
+		ParentComponentPtr Field[*TestSubComponent1]

 		ParentPtr ParentPtr[*TestSubComponent1]
 	}
--- a/chasm/tree.go
+++ b/chasm/tree.go
@@ -2072,6 +2072,15 @@ func (n *Node) resolveDeferredPointers() error {
 				switch value := internal.value().(type) {
 				case Component:
 					resolvedPath, err = n.componentNodePath(value)
+					if err == nil {
+						targetNode := n.valueToNode[value]
+						if !targetNode.isAncestorOf(node) {
+							err = fmt.Errorf(
+								"pointer target is not an ancestor of component at path %v",
+								node.path(),
+							)
+						}
+					}
 				case proto.Message:
 					resolvedPath, err = n.dataNodePath(value)
 				default:
@@ -2472,6 +2481,19 @@ func (n *Node) findNode(
 	return childNode.findNode(path[1:])
 }

+// isAncestorOf returns true if n is a proper ancestor of descendant.
+// It walks from descendant up through parent links to check if n is encountered.
+func (n *Node) isAncestorOf(descendant *Node) bool {
+	current := descendant.parent
+	for current != nil {
+		if current == n {
+			return true
+		}
+		current = current.parent
+	}
+	return false
+}
+
 func (n *Node) delete(isSystemDelete bool) error {
 	for _, childNode := range n.children {
 		if err := childNode.delete(isSystemDelete); err != nil {
--- a/chasm/tree_test.go
+++ b/chasm/tree_test.go
@@ -455,7 +455,7 @@ func (s *nodeSuite) TestPointerAttributes() {
 		SubComponent11: NewComponentField(nil, sc11),
 	}

-	s.Run("Sync and serialize component with pointer", func() {
+	s.Run("Sync and serialize component with ancestor pointer", func() {
 		var nilSerializedNodes map[string]*persistencespb.ChasmNode
 		rootNode, err := s.newTestTree(nilSerializedNodes)
 		s.NoError(err)
@@ -466,26 +466,33 @@ func (s *nodeSuite) TestPointerAttributes() {
 			MSPointer:                    NewMSPointer(s.nodeBackend),
 			SubComponent1:                NewComponentField(nil, sc1),
 			SubComponentInterfacePointer: NewComponentField[Component](nil, sc1),
-			SubComponent11Pointer:        ComponentPointerTo(ctx, sc11),
 		}
+
+		// sc11 points to root (grandparent) -- an ancestor pointer.
+		sc11.GrandparentPointer = ComponentPointerTo(ctx, rootComponent)
+
 		s.NoError(rootNode.SetRootComponent(rootComponent))

-		s.Equal(fieldTypeDeferredPointer, rootComponent.SubComponent11Pointer.Internal.ft)
+		s.Equal(fieldTypeDeferredPointer, sc11.GrandparentPointer.Internal.ft)

 		mutations, err := rootNode.CloseTransaction()
 		s.NoError(err)
-		s.Len(mutations.UpdatedNodes, 5, "root, SubComponent1, SubComponent11, SubComponent11Pointer, and SubComponentInterfacePointer must be updated")
+		s.Len(mutations.UpdatedNodes, 5, "root, SubComponent1, SubComponent11, GrandparentPointer, and SubComponentInterfacePointer must be updated")
 		s.Empty(mutations.DeletedNodes)

-		s.Equal([]string{"SubComponent1", "SubComponent11"}, rootNode.children["SubComponent11Pointer"].serializedNode.GetMetadata().GetPointerAttributes().GetNodePath())
+		sc11Node := rootNode.children["SubComponent1"].children["SubComponent11"]
+		s.Equal(
+			[]string{},
+			sc11Node.children["GrandparentPointer"].serializedNode.GetMetadata().GetPointerAttributes().GetNodePath(),
+		)

-		// Save it use in other subtests.
+		// Save for use in other subtests.
 		persistedNodes = common.CloneProtoMap(mutations.UpdatedNodes)
 	})

 	s.NotNil(persistedNodes)

-	s.Run("Deserialize pointer component", func() {
+	s.Run("Deserialize ancestor pointer component", func() {
 		rootNode, err := s.newTestTree(persistedNodes)
 		s.NoError(err)

@@ -497,9 +504,14 @@ func (s *nodeSuite) TestPointerAttributes() {
 		s.NotNil(testComponent.MSPointer)

 		chasmContext := NewMutableContext(context.Background(), rootNode)
-		sc11Des := testComponent.SubComponent11Pointer.Get(chasmContext)
+		sc1Des := testComponent.SubComponent1.Get(chasmContext)
+		s.NotNil(sc1Des)
+		sc11Des := sc1Des.SubComponent11.Get(chasmContext)
 		s.NotNil(sc11Des)
-		s.Equal(sc11.SubComponent11Data.GetRunId(), sc11Des.SubComponent11Data.GetRunId())
+
+		rootViaPointer := sc11Des.GrandparentPointer.Get(chasmContext)
+		s.NotNil(rootViaPointer)
+		s.Equal(testComponent, rootViaPointer)

 		ifacePtr := testComponent.SubComponentInterfacePointer.Get(chasmContext)
 		s.NotNil(ifacePtr)
@@ -509,7 +521,7 @@ func (s *nodeSuite) TestPointerAttributes() {
 		s.ProtoEqual(sc1ptr.SubComponent1Data, sc1.SubComponent1Data)
 	})

-	s.Run("Clear pointer by setting it to the empty field", func() {
+	s.Run("Clear ancestor pointer by setting it to the empty field", func() {
 		rootNode, err := s.newTestTree(persistedNodes)
 		s.NoError(err)

@@ -517,13 +529,15 @@ func (s *nodeSuite) TestPointerAttributes() {
 		component, err := rootNode.Component(mutableContext, ComponentRef{})
 		s.NoError(err)
 		testComponent := component.(*TestComponent)
+		sc1Des := testComponent.SubComponent1.Get(mutableContext)
+		sc11Des := sc1Des.SubComponent11.Get(mutableContext)

-		testComponent.SubComponent11Pointer = NewEmptyField[*TestSubComponent11]()
+		sc11Des.GrandparentPointer = NewEmptyField[*TestComponent]()

 		mutation, err := rootNode.CloseTransaction()
 		s.NoError(err)
-		s.Len(mutation.UpdatedNodes, 1, "root should be updated")
-		s.Len(mutation.DeletedNodes, 1, "SubComponent11Pointer must be deleted")
+		s.NotEmpty(mutation.UpdatedNodes)
+		s.Len(mutation.DeletedNodes, 1, "GrandparentPointer must be deleted")
 	})
 }