fix: don't mutate shared yaml.Node when rendering schemas concurrently

datamodel/high/base/schema_proxy_test.go

@@ -11,6 +11,7 @@ import (
 	"path/filepath"
 	"strings"
 	"sync"
+	"sync/atomic"
 	"testing"
 
 	"github.com/pb33f/libopenapi/datamodel"
@@ -2215,3 +2216,107 @@ func TestCreateSchemaProxyRefWithSchema_IsRefWithSiblings(t *testing.T) {
 	refWithNilSchema := CreateSchemaProxyRefWithSchema("#/components/schemas/Pet", nil)
 	assert.False(t, refWithNilSchema.isRefWithSiblings())
 }
+
+// findScalarNode returns the first scalar node in the tree whose value matches.
+func findScalarNode(n *yaml.Node, value string) *yaml.Node {
+	if n == nil {
+		return nil
+	}
+	if n.Kind == yaml.ScalarNode && n.Value == value {
+		return n
+	}
+	for _, c := range n.Content {
+		if found := findScalarNode(c, value); found != nil {
+			return found
+		}
+	}
+	return nil
+}
+
+// TestSchemaProxy_ConcurrentRender_DoesNotMutateSharedNodes is a regression test
+// for a data race in inline rendering. Rendering a schema runs the underlying
+// *yaml.Node through (*yaml.Node).Encode, whose desolve stage rewrites Tag/Style
+// in place. Because the representer aliases input nodes, rendering mutated the
+// model's shared scalar nodes: while one goroutine rendered a schema, another
+// reading the same node (e.g. a const type check) could observe the tag briefly
+// cleared to "" and report a bogus "const value type does not match" error.
+//
+// The renders below must never mutate the captured `const` scalar's tag.
+// Run under `-race` to also catch the underlying data race directly.
+func TestSchemaProxy_ConcurrentRender_DoesNotMutateSharedNodes(t *testing.T) {
+	const ymlComponents = `components:
+    schemas:
+     Status:
+       type: object
+       properties:
+         state:
+           type: string
+           const: ok`
+
+	idx := func() *index.SpecIndex {
+		var idxNode yaml.Node
+		require.NoError(t, yaml.Unmarshal([]byte(ymlComponents), &idxNode))
+		return index.NewSpecIndexWithConfig(&idxNode, index.CreateOpenAPIIndexConfig())
+	}()
+
+	const ymlSchema = `type: object
+properties:
+  state:
+    type: string
+    const: ok`
+	var node yaml.Node
+	require.NoError(t, yaml.Unmarshal([]byte(ymlSchema), &node))
+
+	// The `const: ok` scalar is a string; rendering must leave its tag intact.
+	constNode := findScalarNode(node.Content[0], "ok")
+	require.NotNil(t, constNode)
+	require.Equal(t, "!!str", constNode.Tag)
+
+	lowProxy := new(lowbase.SchemaProxy)
+	require.NoError(t, lowProxy.Build(context.Background(), nil, node.Content[0], idx))
+	highProxy := NewSchemaProxy(&low.NodeReference[*lowbase.SchemaProxy]{
+		Value:     lowProxy,
+		ValueNode: node.Content[0],
+	})
+
+	var corrupted atomic.Int64
+	stop := make(chan struct{})
+
+	var readers sync.WaitGroup
+	for i := 0; i < 4; i++ {
+		readers.Add(1)
+		go func() {
+			defer readers.Done()
+			for {
+				select {
+				case <-stop:
+					return
+				default:
+					if constNode.Tag != "!!str" {
+						corrupted.Add(1)
+					}
+				}
+			}
+		}()
+	}
+
+	var writers sync.WaitGroup
+	for i := 0; i < 50; i++ {
+		writers.Add(1)
+		go func() {
+			defer writers.Done()
+			for j := 0; j < 20; j++ {
+				_, err := highProxy.MarshalYAMLInlineWithContext(NewInlineRenderContext())
+				assert.NoError(t, err)
+			}
+		}()
+	}
+
+	writers.Wait()
+	close(stop)
+	readers.Wait()
+
+	assert.Equal(t, int64(0), corrupted.Load(),
+		"rendering mutated the shared const scalar's tag (was cleared by desolve)")
+	assert.Equal(t, "!!str", constNode.Tag, "shared const scalar tag must survive rendering")
+}

datamodel/high/node_builder.go

@@ -348,6 +348,36 @@ func (n *NodeBuilder) Render() *yaml.Node {
 	return m
 }
 
+// encodeSafeValue returns a value safe to pass to (*yaml.Node).Encode. When the
+// value is a *yaml.Node it returns a deep copy: Encode desolves the represented
+// graph in place (Desolve rewrites Tag/Style), and the representer aliases input
+// nodes, so encoding a model-owned node would mutate it. With concurrent renders
+// (e.g. linters running rules in parallel) that mutation races with readers of
+// the same node. Encoding a copy keeps shared nodes immutable.
+func encodeSafeValue(value any) any {
+	if vn, ok := value.(*yaml.Node); ok {
+		return deepCopyYAMLNode(vn)
+	}
+	return value
+}
+
+func deepCopyYAMLNode(n *yaml.Node) *yaml.Node {
+	if n == nil {
+		return nil
+	}
+	c := *n
+	if n.Alias != nil {
+		c.Alias = deepCopyYAMLNode(n.Alias)
+	}
+	if n.Content != nil {
+		c.Content = make([]*yaml.Node, len(n.Content))
+		for i, child := range n.Content {
+			c.Content[i] = deepCopyYAMLNode(child)
+		}
+	}
+	return &c
+}
+
 // AddYAMLNode will add a new *yaml.Node to the parent node, using the tag, key and value provided.
 // If the value is nil, then the node will not be added. This method is recursive, so it will dig down
 // into any non-scalar types.
@@ -492,7 +522,7 @@ func (n *NodeBuilder) AddYAMLNode(parent *yaml.Node, entry *nodes.NodeEntry) *ya
 			break
 		}
 
-		err := rawNode.Encode(value)
+		err := rawNode.Encode(encodeSafeValue(value))
 		if err != nil {
 			return parent
 		} else {
@@ -645,7 +675,7 @@ func (n *NodeBuilder) AddYAMLNode(parent *yaml.Node, entry *nodes.NodeEntry) *ya
 						}
 					}
 
-					err := rawNode.Encode(value)
+					err := rawNode.Encode(encodeSafeValue(value))
 					if err != nil {
 						return parent
 					} else {

datamodel/high/node_builder_test.go

@@ -1700,3 +1700,43 @@ func TestOriginalFloatLexeme(t *testing.T) {
 		})
 	}
 }
+
+func TestEncodeSafeValue_DeepCopiesYAMLNodes(t *testing.T) {
+	// non-*yaml.Node values pass through unchanged.
+	assert.Equal(t, 42, encodeSafeValue(42))
+	assert.Equal(t, "hello", encodeSafeValue("hello"))
+
+	// nil nodes return nil rather than panicking.
+	assert.Nil(t, deepCopyYAMLNode(nil))
+	assert.Nil(t, encodeSafeValue((*yaml.Node)(nil)))
+
+	// a *yaml.Node is deep-copied: every node is a distinct pointer, including
+	// content children and aliases, so mutating the copy can't affect the model.
+	anchor := &yaml.Node{Kind: yaml.ScalarNode, Tag: "!!str", Value: "anchored"}
+	orig := &yaml.Node{
+		Kind: yaml.MappingNode,
+		Content: []*yaml.Node{
+			{Kind: yaml.ScalarNode, Tag: "!!str", Value: "key"},
+			{Kind: yaml.ScalarNode, Tag: "!!str", Value: "val", Style: yaml.DoubleQuotedStyle},
+			{Kind: yaml.AliasNode, Value: "ref", Alias: anchor},
+		},
+	}
+
+	copied, ok := encodeSafeValue(orig).(*yaml.Node)
+	assert.True(t, ok)
+	assert.NotSame(t, orig, copied)
+	assert.Equal(t, len(orig.Content), len(copied.Content))
+	for i := range orig.Content {
+		assert.NotSame(t, orig.Content[i], copied.Content[i])
+		assert.Equal(t, orig.Content[i].Value, copied.Content[i].Value)
+	}
+
+	// the alias child is deep-copied too.
+	assert.NotNil(t, copied.Content[2].Alias)
+	assert.NotSame(t, anchor, copied.Content[2].Alias)
+	assert.Equal(t, "anchored", copied.Content[2].Alias.Value)
+
+	// mutating the copy must not touch the original (this is the whole point).
+	copied.Content[1].Tag = ""
+	assert.Equal(t, "!!str", orig.Content[1].Tag)
+}