fix(compilers/openapi): refuse a $ref resolving through itself

compilers/openapi/cycles_test.go

@@ -35,6 +35,18 @@ var cycleReproducers = []struct{ name, file string }{
 	{"webhook-mutual", "cycle_webhook_mutual"},
 	{"response-via-path", "cycle_response_via_path"},
 	{"path-item-via-component", "cycle_path_item_via_component"},
+	// A reference whose pointer passes through a reference already being
+	// resolved. Distinct from the cycles above: the hop never completes, so
+	// speakeasy's own guard cannot see it and it deadlocks rather than faulting.
+	{"path-item-prefix-self", "cycle_path_item_prefix_self"},
+	{"path-item-prefix-sibling", "cycle_path_item_prefix_sibling"},
+	{"path-item-prefix-chain", "cycle_path_item_prefix_chain"},
+	{"component-path-item-prefix", "cycle_component_path_item_prefix"},
+	{"webhook-prefix-self", "cycle_webhook_prefix_self"},
+	// A self-reference only the resolver's pointer normalization reveals, which
+	// overflows the stack rather than deadlocking: the resolution cache ends up
+	// pointing at its own reference and GetObject's delegation recurses.
+	{"pointer-whitespace-self", "cycle_pointer_whitespace_self"},
 }
 
 func TestCompile_CyclicSpecDoesNotCrash(t *testing.T) {
@@ -249,3 +261,30 @@ func FuzzCycleDetector(f *testing.F) {
 			[]compilers.Source{{Path: "fuzz.yaml", Data: data}}, compilers.Options{})
 	})
 }
+
+// TestCompile_ReentrantPrefixRefusedInBothDeclarationOrders pins what the
+// safe-memo narrowing exists for. Whether a re-entrant hop is seen depends on
+// which reference the walk reaches as a chain root first, so a single order
+// proves nothing: with a memo keyed on the node alone, declaring the dangling
+// reference first records it as terminating and the later chain short-circuits
+// straight past the hop. Both orders deadlock in the resolver, so both must be
+// refused.
+func TestCompile_ReentrantPrefixRefusedInBothDeclarationOrders(t *testing.T) {
+	t.Parallel()
+	const head = "openapi: 3.1.0\ninfo: {title: t, version: '1'}\npaths:\n"
+	orders := []struct{ name, body string }{
+		{"dangling-declared-first", "  /b: {$ref: '#/paths/~1a/t'}\n  /a: {$ref: '#/paths/~1b'}\n"},
+		{"dangling-declared-last", "  /a: {$ref: '#/paths/~1b'}\n  /b: {$ref: '#/paths/~1a/t'}\n"},
+	}
+	for _, tc := range orders {
+		t.Run(tc.name, func(t *testing.T) {
+			t.Parallel()
+			doc, diags, err := New().Compile(t.Context(),
+				[]compilers.Source{{Path: "order.yaml", Data: []byte(head + tc.body)}},
+				compilers.Options{})
+			require.NoError(t, err, "a re-entrant reference is a spec problem, not a Go error")
+			assert.Nil(t, doc, "the compiler refuses a re-entrant reference")
+			assertHasErrorCode(t, diags, diag.CyclicRef)
+		})
+	}
+}

compilers/openapi/internal/diag/diag.go

@@ -26,9 +26,11 @@ const (
 	UnsupportedVersion = "openapi/unsupported-version"
 	// UnresolvedRef reports a $ref that could not be resolved.
 	UnresolvedRef = "openapi/unresolved-ref"
-	// CyclicRef reports a degenerate reference cycle — a recursive YAML anchor or
-	// a chain of $ref-only schemas that never reaches a concrete type — caught
-	// before it can crash the parser with a stack overflow.
+	// CyclicRef reports a degenerate reference cycle — a recursive YAML anchor, a
+	// chain of $ref-only schemas that never reaches a concrete type, or a
+	// reference whose pointer resolves through a reference already being resolved
+	// — caught before it can crash the parser with a stack overflow or deadlock
+	// the resolver on a lock its own goroutine holds.
 	CyclicRef = "openapi/cyclic-ref"
 	// CycleScanFailed reports that the pre-parse cycle scan did not run to
 	// completion — either it aborted (a detector bug) or the document exceeded one

compilers/openapi/internal/nodeview/nodeview.go

@@ -9,6 +9,7 @@
 package nodeview
 
 import (
+	"net/url"
 	"strconv"
 	"strings"
 
@@ -27,6 +28,12 @@ import (
 // borrowed upward would invert the dependency.
 const maxAliasChain = 10000
 
+// maxPointerSegments bounds how many tokens one JSON pointer walk follows. A
+// pointer names a position in the document, so a real one is a handful of
+// segments deep; the bound is what keeps the walk terminating on a pointer built
+// to be long rather than to name anything, per the bounded-recursion rule.
+const maxPointerSegments = 1024
+
 // MergeDepthLimit bounds how deep a chain of `<<` merge keys the mapping view
 // expands. It is far tighter than maxAliasChain: each merge level
 // re-materializes every pair beneath it, so expanding a chain of depth d costs
@@ -301,35 +308,84 @@ func (v *View) PureRefTarget(n *yaml.Node) (string, bool) {
 }
 
 // PureRefTargetOf is PureRefTarget over an already-expanded pair list, so a
-// caller that needs both the pairs and the target expands the mapping once.
+// caller that needs both the pairs and the target expands the mapping once. The
+// target is normalized by InternalPointer, so it is a bare pointer ('/a/b'),
+// not the '#/a/b' the source spells.
 func PureRefTargetOf(pairs []Pair) (string, bool) {
 	for _, p := range pairs {
 		if p.Key != "$ref" {
 			continue
 		}
-		if p.Val == nil || p.Val.Kind != yaml.ScalarNode || !strings.HasPrefix(p.Val.Value, "#/") {
+		if p.Val == nil || p.Val.Kind != yaml.ScalarNode {
 			return "", false
 		}
-		return p.Val.Value, true
+		return InternalPointer(p.Val.Value)
 	}
 	return "", false
 }
 
-// ResolvePointer resolves an internal JSON pointer ('#/a/b') against the root
-// node, returning the targeted node or nil when the path does not exist. Alias
-// nodes along the path are dereferenced so navigation follows structure.
-func (v *View) ResolvePointer(root *yaml.Node, ref string) *yaml.Node {
+// InternalPointer reports the JSON pointer a $ref value names inside this
+// document, and whether it names this document at all.
+//
+// It mirrors the resolver exactly: speakeasy splits a $ref on '#', treats what
+// precedes it as a URI and what follows as the pointer, trims whitespace from
+// both, and percent-decodes the pointer (references/reference.go GetURI and
+// GetJSONPointer, v1.24.0). A ref whose URI half is empty names this document.
+//
+// Reading the raw value instead is not a near-enough approximation, it is a hole
+// in the cycle scan: a pointer this package calls dangling but the resolver
+// resolves is a reference the scan cannot see, and '#/paths/~1a ' — one trailing
+// space — is enough to be one. A dependency bump should re-check those two
+// methods, as MergeDepthLimit's comment does for the behavior it tracks.
+func InternalPointer(ref string) (string, bool) {
+	parts := strings.Split(ref, "#")
+	if len(parts) < 2 || strings.TrimSpace(parts[0]) != "" {
+		return "", false // no fragment, or a fragment in another document
+	}
+	pointer := strings.TrimSpace(parts[1])
+	if decoded, err := url.QueryUnescape(pointer); err == nil {
+		pointer = decoded
+	}
+	return pointer, true
+}
+
+// PointerPath walks a normalized internal JSON pointer ('/a/b', as
+// InternalPointer returns it) against the root node, keeping every node it
+// passes through: element 0 is the root and each later element is the node
+// reached by one more token. complete reports whether every token resolved;
+// when it is false the walk stopped at the last element returned, and there is
+// no destination. Alias nodes along the path are dereferenced so navigation
+// follows structure.
+//
+// It yields the whole path rather than just the target because a pointer's
+// danger is not always at its destination. speakeasy resolves a reference while
+// holding that reference's own lock and read-locks every reference the pointer
+// walk passes through, so a pointer that traverses a reference already being
+// resolved deadlocks before it ever arrives (v1.24.0, openapi/reference.go
+// resolve/GetObject). A target alone cannot express that.
+func (v *View) PointerPath(root *yaml.Node, pointer string) (path []*yaml.Node, complete bool) {
 	cur := Deref(root)
-	for raw := range strings.SplitSeq(strings.TrimPrefix(ref, "#"), "/") {
+	if cur == nil {
+		return nil, false
+	}
+	path = append(path, cur)
+
+	segments := 0
+	for raw := range strings.SplitSeq(pointer, "/") {
 		if raw == "" {
 			continue
 		}
-		cur = v.ChildByToken(Deref(cur), ids.UnescapeSegment(raw))
+		segments++
+		if segments > maxPointerSegments {
+			return path, false
+		}
+		cur = Deref(v.ChildByToken(cur, ids.UnescapeSegment(raw)))
 		if cur == nil {
-			return nil
+			return path, false
 		}
+		path = append(path, cur)
 	}
-	return Deref(cur)
+	return path, true
 }
 
 // ChildByToken returns the child of a mapping (by key) or sequence (by index)

compilers/openapi/internal/nodeview/nodeview_internal_test.go

@@ -1,6 +1,7 @@
 package nodeview
 
 import (
+	"strings"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -59,7 +60,7 @@ func TestChildByToken_NilNode(t *testing.T) {
 	assert.Nil(t, New().ChildByToken(nil, "anything"))
 }
 
-func TestResolvePointer_Cases(t *testing.T) {
+func TestPointerPath_Cases(t *testing.T) {
 	t.Parallel()
 	leaf := yscalar("leaf")
 	target := ymap(yscalar("b"), leaf)
@@ -70,27 +71,54 @@ func TestResolvePointer_Cases(t *testing.T) {
 		yscalar("c~d"), yscalar("tilde"),
 	)
 	tests := []struct {
-		name string
-		ref  string
-		want *yaml.Node
+		name    string
+		pointer string
+		want    *yaml.Node
 	}{
-		{"sequence index in range", "#/arr/1", root.Content[1].Content[1]},
-		{"sequence index out of range", "#/arr/9", nil},
-		{"sequence index non-numeric", "#/arr/x", nil},
-		{"alias dereferenced along path", "#/via/b", leaf},
-		{"escaped slash token", "#/a~1b", root.Content[5]},
-		{"escaped tilde token", "#/c~0d", root.Content[7]},
-		{"missing key", "#/nope", nil},
+		{"sequence index in range", "/arr/1", root.Content[1].Content[1]},
+		{"sequence index out of range", "/arr/9", nil},
+		{"sequence index non-numeric", "/arr/x", nil},
+		{"alias dereferenced along path", "/via/b", leaf},
+		{"escaped slash token", "/a~1b", root.Content[5]},
+		{"escaped tilde token", "/c~0d", root.Content[7]},
+		{"missing key", "/nope", nil},
 	}
 	for _, tc := range tests {
 		t.Run(tc.name, func(t *testing.T) {
 			t.Parallel()
-			got := New().ResolvePointer(root, tc.ref)
+			path, complete := New().PointerPath(root, tc.pointer)
 			if tc.want == nil {
-				assert.Nil(t, got)
+				assert.False(t, complete, "the pointer names nothing")
 				return
 			}
-			assert.Same(t, tc.want, got)
+			require.True(t, complete)
+			assert.Same(t, tc.want, path[len(path)-1], "the last element is the destination")
+		})
+	}
+}
+
+func TestInternalPointer_MatchesTheResolversNormalization(t *testing.T) {
+	t.Parallel()
+	tests := []struct {
+		name, ref, want string
+		internal        bool
+	}{
+		{name: "plain", ref: "#/components/schemas/A", want: "/components/schemas/A", internal: true},
+		{name: "trailing space", ref: "#/paths/~1a ", want: "/paths/~1a", internal: true},
+		{name: "leading space", ref: " #/paths/~1a", want: "/paths/~1a", internal: true},
+		{name: "percent-decoded", ref: "#/paths/%7E1a", want: "/paths/~1a", internal: true},
+		{name: "second hash ends the pointer", ref: "#/a#b", want: "/a", internal: true},
+		{name: "bare hash names the root", ref: "#", want: "", internal: true},
+		{name: "undecodable escape kept raw", ref: "#/a%zz", want: "/a%zz", internal: true},
+		{name: "no fragment", ref: "other.yaml", internal: false},
+		{name: "another document", ref: "other.yaml#/components/schemas/A", internal: false},
+	}
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			t.Parallel()
+			got, internal := InternalPointer(tc.ref)
+			assert.Equal(t, tc.internal, internal)
+			assert.Equal(t, tc.want, got)
 		})
 	}
 }
@@ -200,7 +228,7 @@ func TestPureRefTarget_Cases(t *testing.T) {
 		want string
 	}{
 		{"sibling key before the ref", ymap(yscalar("type"), yscalar("object"),
-			yscalar("$ref"), yscalar("#/components/schemas/A")), "#/components/schemas/A"},
+			yscalar("$ref"), yscalar("#/components/schemas/A")), "/components/schemas/A"},
 		{"external ref is not internal", ymap(yscalar("$ref"), yscalar("other.yaml#/A")), ""},
 		{"non-scalar ref value", ymap(yscalar("$ref"), ymap(yscalar("a"), yscalar("b"))), ""},
 		{"nil ref value via broken alias", ymap(yscalar("$ref"), yalias(nil)), ""},
@@ -426,3 +454,54 @@ func yamlDoc(t *testing.T, src string) *yaml.Node {
 	require.NoError(t, yaml.Unmarshal([]byte(src), &doc))
 	return DocumentRoot(&doc)
 }
+
+func TestPointerPath_KeepsTheNodesTheWalkPassesThrough(t *testing.T) {
+	t.Parallel()
+	leaf := yscalar("leaf")
+	inner := ymap(yscalar("b"), leaf)
+	root := ymap(yscalar("a"), inner)
+
+	path, complete := New().PointerPath(root, "/a/b")
+	require.True(t, complete, "every token resolves")
+	assert.Equal(t, []*yaml.Node{root, inner, leaf}, path,
+		"element 0 is the root and each later element is one more token")
+}
+
+func TestPointerPath_IncompleteStopsAtTheLastNodeReached(t *testing.T) {
+	t.Parallel()
+	inner := ymap(yscalar("b"), yscalar("leaf"))
+	root := ymap(yscalar("a"), inner)
+
+	path, complete := New().PointerPath(root, "/a/missing/deeper")
+	assert.False(t, complete, "a token that names nothing stops the walk")
+	assert.Equal(t, []*yaml.Node{root, inner}, path,
+		"an unresolvable pointer has no destination, so every node it reached is one it passed through")
+}
+
+func TestPointerPath_RootTokenlessAndNil(t *testing.T) {
+	t.Parallel()
+	root := ymap(yscalar("a"), yscalar("v"))
+
+	path, complete := New().PointerPath(root, "")
+	assert.True(t, complete, "a pointer with no tokens names the root")
+	assert.Equal(t, []*yaml.Node{root}, path)
+
+	path, complete = New().PointerPath(nil, "/a")
+	assert.False(t, complete)
+	assert.Nil(t, path, "a nil root reaches nothing")
+}
+
+func TestPointerPath_SegmentCapStopsTheWalk(t *testing.T) {
+	t.Parallel()
+	// A mapping whose only key is "a" and whose value is itself cannot be built
+	// from parsed YAML, but an alias can stand in: the walk follows "a" as long
+	// as tokens last, so only the cap can end it.
+	root := ymap(yscalar("a"), nil)
+	root.Content[1] = root
+
+	ref := strings.Repeat("/a", maxPointerSegments+1)
+	path, complete := New().PointerPath(root, ref)
+	assert.False(t, complete, "a pointer past the segment cap does not resolve")
+	assert.Len(t, path, maxPointerSegments+1,
+		"the walk stops at the cap: the root plus one node per followed token")
+}