Merge pull request #10 from inflearn/feature/hotfix-triage

fix: two carry-over hotfixes (referencegrant name filter, anthropic tracing negative idx)

Author: pokabookinflab

internal/controller/referencegrant.go

@@ -95,7 +95,7 @@ func (v *referenceGrantValidator) validateReference(
 	// Check if any ReferenceGrant allows this cross-namespace reference.
 	for i := range referenceGrants.Items {
 		grant := &referenceGrants.Items[i]
-		if v.isReferenceGrantValid(grant, routeNamespace, targetGroup, targetKind) {
+		if v.isReferenceGrantValid(grant, routeNamespace, targetGroup, targetKind, gwapiv1b1.ObjectName(targetName)) {
 			return nil
 		}
 	}
@@ -109,12 +109,13 @@ func (v *referenceGrantValidator) validateReference(
 }
 
 // isReferenceGrantValid checks if a ReferenceGrant allows an AIGatewayRoute to reference the
-// target resource identified by targetGroup/targetKind.
+// target resource identified by targetGroup/targetKind/targetName.
 func (v *referenceGrantValidator) isReferenceGrantValid(
 	grant *gwapiv1b1.ReferenceGrant,
 	fromNamespace string,
 	targetGroup gwapiv1b1.Group,
 	targetKind gwapiv1b1.Kind,
+	targetName gwapiv1b1.ObjectName,
 ) bool {
 	// Check if the grant allows references from the route's namespace.
 	fromAllowed := false
@@ -131,7 +132,7 @@ func (v *referenceGrantValidator) isReferenceGrantValid(
 
 	// Check if the grant allows references to the target resource.
 	for _, to := range grant.Spec.To {
-		if v.matchesTo(&to, targetGroup, targetKind) {
+		if v.matchesTo(&to, targetGroup, targetKind, targetName) {
 			return true
 		}
 	}
@@ -159,22 +160,31 @@ func (v *referenceGrantValidator) matchesFrom(from *gwapiv1b1.ReferenceGrantFrom
 	return true
 }
 
-// matchesTo checks if a ReferenceGrantTo matches the target resource identified by targetGroup/targetKind.
-func (v *referenceGrantValidator) matchesTo(to *gwapiv1b1.ReferenceGrantTo, targetGroup gwapiv1b1.Group, targetKind gwapiv1b1.Kind) bool {
-	// Check group
+// matchesTo checks if a ReferenceGrantTo matches the target resource identified by
+// targetGroup/targetKind/targetName.
+//
+// Per the Gateway API spec, ReferenceGrantTo.Name is optional. When set, the grant is scoped
+// to exactly that named resource; when unset, it acts as a wildcard covering every resource of
+// the group/kind. Previously we ignored the name field entirely, which quietly promoted any
+// name-scoped grant into a wildcard and let an AIGatewayRoute reference *any* backend of the
+// same kind in the target namespace — not what the grant author meant.
+//
+// https://gateway-api.sigs.k8s.io/api-types/referencegrant/
+func (v *referenceGrantValidator) matchesTo(
+	to *gwapiv1b1.ReferenceGrantTo,
+	targetGroup gwapiv1b1.Group,
+	targetKind gwapiv1b1.Kind,
+	targetName gwapiv1b1.ObjectName,
+) bool {
 	if to.Group != targetGroup {
 		return false
 	}
-
-	// Check kind
 	if to.Kind != targetKind {
 		return false
 	}
-
-	// If a specific name is specified, we would need to check it here,
-	// but ReferenceGrant typically doesn't specify individual resource names
-	// (that's handled by the Name field which is optional in the spec)
-	// For now, we only check group and kind as per Gateway API spec
-
+	// Name is optional in ReferenceGrantTo. Unset == wildcard; set == exact match required.
+	if to.Name != nil && *to.Name != targetName {
+		return false
+	}
 	return true
 }

internal/controller/referencegrant_test.go

@@ -442,6 +442,36 @@ func TestReferenceGrantValidator_MatchesFrom_WrongNamespace(t *testing.T) {
 	require.False(t, result, "should return false for wrong namespace")
 }
 
+// TestReferenceGrantValidator_MatchesTo_Name locks in the name-filter behaviour: when
+// ReferenceGrantTo.Name is set, the grant must apply only to the named resource; when unset,
+// it acts as a wildcard for the group/kind.
+func TestReferenceGrantValidator_MatchesTo_Name(t *testing.T) {
+	scheme := runtime.NewScheme()
+	_ = gwapiv1b1.Install(scheme)
+	_ = aigv1a1.AddToScheme(scheme)
+
+	fakeClient := fake.NewClientBuilder().WithScheme(scheme).Build()
+	validator := newReferenceGrantValidator(fakeClient)
+
+	t.Run("name unset → wildcard, matches any name", func(t *testing.T) {
+		to := &gwapiv1b1.ReferenceGrantTo{Group: aiServiceBackendGroup, Kind: aiServiceBackendKind}
+		require.True(t, validator.matchesTo(to, aiServiceBackendGroup, aiServiceBackendKind, "any-backend"))
+	})
+
+	t.Run("name set and matches → allowed", func(t *testing.T) {
+		named := gwapiv1b1.ObjectName("target-backend")
+		to := &gwapiv1b1.ReferenceGrantTo{Group: aiServiceBackendGroup, Kind: aiServiceBackendKind, Name: &named}
+		require.True(t, validator.matchesTo(to, aiServiceBackendGroup, aiServiceBackendKind, "target-backend"))
+	})
+
+	t.Run("name set but differs → denied", func(t *testing.T) {
+		named := gwapiv1b1.ObjectName("target-backend")
+		to := &gwapiv1b1.ReferenceGrantTo{Group: aiServiceBackendGroup, Kind: aiServiceBackendKind, Name: &named}
+		require.False(t, validator.matchesTo(to, aiServiceBackendGroup, aiServiceBackendKind, "other-backend"),
+			"name-scoped grant must not promote to a wildcard")
+	})
+}
+
 // TestReferenceGrantValidator_MatchesTo_WrongGroup tests matchesTo with wrong group
 func TestReferenceGrantValidator_MatchesTo_WrongGroup(t *testing.T) {
 	scheme := runtime.NewScheme()
@@ -456,7 +486,7 @@ func TestReferenceGrantValidator_MatchesTo_WrongGroup(t *testing.T) {
 		Kind:  aiServiceBackendKind,
 	}
 
-	result := validator.matchesTo(to, aiServiceBackendGroup, aiServiceBackendKind)
+	result := validator.matchesTo(to, aiServiceBackendGroup, aiServiceBackendKind, "target-backend")
 	require.False(t, result, "should return false for wrong group")
 }
 
@@ -474,7 +504,7 @@ func TestReferenceGrantValidator_MatchesTo_WrongKind(t *testing.T) {
 		Kind:  "WrongKind",
 	}
 
-	result := validator.matchesTo(to, aiServiceBackendGroup, aiServiceBackendKind)
+	result := validator.matchesTo(to, aiServiceBackendGroup, aiServiceBackendKind, "target-backend")
 	require.False(t, result, "should return false for wrong kind")
 }
 

internal/tracing/openinference/anthropic/messages.go

@@ -233,6 +233,19 @@ func buildResponseAttributes(resp *anthropic.MessagesResponse, config *openinfer
 	return attrs
 }
 
+// maxContentBlocks caps how large a single Anthropic streaming message's content array can
+// grow inside the tracing span. Well-formed responses stay well below this; the cap exists so
+// a hostile / buggy upstream cannot force a giant allocation by sending an out-of-range index.
+const maxContentBlocks = 1000
+
+// isValidContentBlockIndex applies the same non-negative / bounded check to every SSE index
+// path (ContentBlockStart / ContentBlockDelta / ContentBlockStop). Negative indices used to
+// slip through the delta/stop branches — the length check alone (`idx < len`) is satisfied by
+// negative values in Go, which then panic on the slice index below.
+func isValidContentBlockIndex(idx int) bool {
+	return idx >= 0 && idx < maxContentBlocks
+}
+
 // convertSSEToResponse converts a complete SSE stream to a single JSON-encoded
 // openai.ChatCompletionResponse. This will not serialize zero values including
 // fields whose values are zero or empty, or nested objects where all fields
@@ -270,9 +283,7 @@ func convertSSEToResponse(chunks []*anthropic.MessagesStreamChunk) *anthropic.Me
 
 		case event.ContentBlockStart != nil:
 			idx := event.ContentBlockStart.Index
-			// Guard against negative or unreasonably large indices from a hostile upstream.
-			const maxContentBlocks = 1000
-			if idx < 0 || idx >= maxContentBlocks {
+			if !isValidContentBlockIndex(idx) {
 				continue
 			}
 			// Grow slice if needed.
@@ -285,28 +296,35 @@ func convertSSEToResponse(chunks []*anthropic.MessagesStreamChunk) *anthropic.Me
 
 		case event.ContentBlockDelta != nil:
 			idx := event.ContentBlockDelta.Index
-			if idx < len(response.Content) {
-				block := &response.Content[idx]
-				delta := event.ContentBlockDelta.Delta
+			// The upper-bound check (idx < len) already blocks out-of-range access, but a
+			// negative index from a hostile / malformed upstream would satisfy `idx < len`
+			// yet still panic on the slice index below. Reject negatives explicitly.
+			if !isValidContentBlockIndex(idx) || idx >= len(response.Content) {
+				continue
+			}
+			block := &response.Content[idx]
+			delta := event.ContentBlockDelta.Delta
 
-				if block.Text != nil && delta.Text != "" {
-					block.Text.Text += delta.Text
-				}
-				if block.Tool != nil && delta.PartialJSON != "" {
-					toolInputs[idx] += delta.PartialJSON
+			if block.Text != nil && delta.Text != "" {
+				block.Text.Text += delta.Text
+			}
+			if block.Tool != nil && delta.PartialJSON != "" {
+				toolInputs[idx] += delta.PartialJSON
+			}
+			if block.Thinking != nil {
+				if delta.Thinking != "" {
+					block.Thinking.Thinking += delta.Thinking
 				}
-				if block.Thinking != nil {
-					if delta.Thinking != "" {
-						block.Thinking.Thinking += delta.Thinking
-					}
-					if delta.Signature != "" {
-						block.Thinking.Signature = delta.Signature
-					}
+				if delta.Signature != "" {
+					block.Thinking.Signature = delta.Signature
 				}
 			}
 
 		case event.ContentBlockStop != nil:
 			idx := event.ContentBlockStop.Index
+			if !isValidContentBlockIndex(idx) {
+				continue
+			}
 			if jsonStr, ok := toolInputs[idx]; ok {
 				if idx < len(response.Content) && response.Content[idx].Tool != nil {
 					var input map[string]any

internal/tracing/openinference/anthropic/messages_test.go

@@ -238,6 +238,42 @@ func TestConvertSSEToResponse(t *testing.T) {
 				Content: []anthropic.MessagesContentBlock{},
 			},
 		},
+		{
+			// Regression: negative index used to slip through the delta / stop branches
+			// (idx < len(response.Content) is true for negatives in Go) and panic on the
+			// slice access. Now guarded by isValidContentBlockIndex.
+			name: "negative delta and stop indices after a valid start do not panic",
+			chunks: []*anthropic.MessagesStreamChunk{
+				{
+					MessageStart: &anthropic.MessagesStreamChunkMessageStart{
+						ID: "msg_neg2", Model: "claude-3", Role: "assistant",
+						Usage: &anthropic.Usage{InputTokens: 1},
+					},
+				},
+				{
+					ContentBlockStart: &anthropic.MessagesStreamChunkContentBlockStart{
+						Index:        0,
+						ContentBlock: anthropic.MessagesContentBlock{Text: &anthropic.TextBlock{Type: "text"}},
+					},
+				},
+				{
+					ContentBlockDelta: &anthropic.MessagesStreamChunkContentBlockDelta{
+						Index: -5,
+						Delta: anthropic.ContentBlockDelta{Type: "text_delta", Text: "malicious"},
+					},
+				},
+				{
+					ContentBlockStop: &anthropic.MessagesStreamChunkContentBlockStop{Index: -1},
+				},
+			},
+			want: &anthropic.MessagesResponse{
+				ID: "msg_neg2", Model: "claude-3", Role: "assistant",
+				Usage: &anthropic.Usage{InputTokens: 1},
+				Content: []anthropic.MessagesContentBlock{
+					{Text: &anthropic.TextBlock{Type: "text", Text: ""}},
+				},
+			},
+		},
 		{
 			name: "content block index at cap is ignored",
 			chunks: []*anthropic.MessagesStreamChunk{