fix(governance): recurse into rule-parameter array elements

Address the final Copilot review comment: droppedKeyPath previously treated
array elements as opaque, so an unrecognized key inside an array element
(e.g. required_status_checks[].integration_ids, a typo for integration_id)
was silently dropped by go-github's JSON unmarshal without being caught --
the resulting rule would then accept a status check from any integration
instead of only the one requested.

This round-trip is entirely local (our own marshal/unmarshal of a go-github
struct, not a remote API response), so slice order and length are
deterministic and safe to compare by index. droppedKeyPath now delegates to
a new droppedValuePath helper that recurses into both nested objects and
array elements, reporting paths like
"required_status_checks[0].integration_ids" when a caller-supplied key
disappears in either a map or an array position.

Added a regression test for the array-nested typo, and a companion test
confirming valid array-nested parameters still round-trip without being
misflagged.

Note: committed unsigned -- the local 1Password SSH-signing agent is still
unavailable (session locked).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Author: SamMorrowDrums

pkg/github/rulesets.go

@@ -869,13 +869,15 @@ func buildRepositoryRulesetFromArgs(args map[string]any) (github.RepositoryRules
 }
 
 // droppedKeyPath recursively compares a caller-supplied object against its
-// round-tripped counterpart and returns the dotted path of the first key that
-// did not survive, or "" if every key survived. A caller-supplied key whose
-// value is a JSON zero value (false, 0, "", or an empty array/object) is
-// exempt, since it is indistinguishable from a field omitted by a
-// `json:",omitempty"` struct tag on the far side of the round-trip. Only
-// nested objects are recursed into; array elements are treated as opaque so
-// that reordering by the API cannot produce a false positive.
+// round-tripped counterpart and returns the path of the first key or array
+// element that did not survive (e.g. "required_status_checks[0].integration_id"),
+// or "" if everything survived. A caller-supplied value that is a JSON zero
+// value (false, 0, "", or an empty array/object) is exempt, since it is
+// indistinguishable from a field omitted by a `json:",omitempty"` struct tag
+// on the far side of the round-trip. This round-trip is entirely local (our
+// own JSON marshal/unmarshal of a go-github struct, not a remote API
+// response), so slice order and length are preserved deterministically and
+// array elements are safe to compare by index.
 func droppedKeyPath(requested, applied map[string]any) string {
 	for key, requestedValue := range requested {
 		appliedValue, ok := applied[key]
@@ -885,17 +887,49 @@ func droppedKeyPath(requested, applied map[string]any) string {
 			}
 			return key
 		}
-		requestedChild, requestedIsMap := requestedValue.(map[string]any)
-		appliedChild, appliedIsMap := appliedValue.(map[string]any)
-		if requestedIsMap && appliedIsMap {
-			if nested := droppedKeyPath(requestedChild, appliedChild); nested != "" {
-				return key + "." + nested
-			}
+		if nested := droppedValuePath(requestedValue, appliedValue); nested != "" {
+			return key + nested
 		}
 	}
 	return ""
 }
 
+// droppedValuePath recurses into map and array values on behalf of
+// droppedKeyPath. It returns a path suffix beginning with "." (object key) or
+// "[i]" (array index), or "" when requested and applied agree closely enough.
+func droppedValuePath(requested, applied any) string {
+	switch requestedTyped := requested.(type) {
+	case map[string]any:
+		appliedMap, ok := applied.(map[string]any)
+		if !ok {
+			return ""
+		}
+		if nested := droppedKeyPath(requestedTyped, appliedMap); nested != "" {
+			return "." + nested
+		}
+		return ""
+	case []any:
+		appliedArr, ok := applied.([]any)
+		if !ok {
+			return ""
+		}
+		for i, requestedElem := range requestedTyped {
+			if i >= len(appliedArr) {
+				if isZeroJSONValue(requestedElem) {
+					continue
+				}
+				return fmt.Sprintf("[%d]", i)
+			}
+			if nested := droppedValuePath(requestedElem, appliedArr[i]); nested != "" {
+				return fmt.Sprintf("[%d]%s", i, nested)
+			}
+		}
+		return ""
+	default:
+		return ""
+	}
+}
+
 // isZeroJSONValue reports whether v is the JSON zero value for its type
 // (false, 0, "", nil, or an empty array/object). Such values are
 // indistinguishable from an omitted field once round-tripped through a Go

pkg/github/rulesets_test.go

@@ -667,6 +667,94 @@ func Test_CreateRepositoryRuleset(t *testing.T) {
 		assert.NotEmpty(t, capturedBody)
 	})
 
+	t.Run("unrecognized key inside a rule parameter array element is rejected", func(t *testing.T) {
+		// "integration_ids" is a plausible typo for the real per-check field
+		// "integration_id" on required_status_checks[].  Unlike the top-level
+		// rule/condition round-trip, this array is produced by our own local
+		// JSON marshal/unmarshal of the go-github struct (not a remote API
+		// response), so element order is guaranteed stable and comparing by
+		// index is safe. Without this check, the typo would silently vanish and
+		// the resulting rule would accept a status check from any integration
+		// instead of only the one requested.
+		called := false
+		client := mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
+			"POST /repos/{owner}/{repo}/rulesets": func(w http.ResponseWriter, _ *http.Request) {
+				called = true
+				w.WriteHeader(http.StatusCreated)
+			},
+		}))
+		deps := BaseDeps{Client: client}
+		handler := toolDef.Handler(deps)
+		request := createMCPRequest(map[string]any{
+			"level":       "repository",
+			"owner":       "owner",
+			"repo":        "repo",
+			"name":        "x",
+			"enforcement": "active",
+			"rules": []any{
+				map[string]any{
+					"type": "required_status_checks",
+					"parameters": map[string]any{
+						"required_status_checks": []any{
+							map[string]any{
+								"context":         "ci",
+								"integration_ids": float64(42), // typo: should be integration_id
+							},
+						},
+						"strict_required_status_checks_policy": true,
+					},
+				},
+			},
+		})
+
+		result, err := handler(ContextWithDeps(context.Background(), deps), &request)
+		require.NoError(t, err)
+		require.True(t, result.IsError)
+		assert.Contains(t, getErrorResult(t, result).Text, "required_status_checks[0].integration_ids")
+		assert.False(t, called, "request must not be sent when a nested array element key is unrecognized")
+	})
+
+	t.Run("valid rule parameter array elements round-trip and are not misflagged", func(t *testing.T) {
+		var capturedBody []byte
+		client := mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
+			"POST /repos/{owner}/{repo}/rulesets": func(w http.ResponseWriter, r *http.Request) {
+				capturedBody, _ = io.ReadAll(r.Body)
+				w.WriteHeader(http.StatusCreated)
+				_, _ = w.Write(capturedBody)
+			},
+		}))
+		deps := BaseDeps{Client: client}
+		handler := toolDef.Handler(deps)
+		request := createMCPRequest(map[string]any{
+			"level":       "repository",
+			"owner":       "owner",
+			"repo":        "repo",
+			"name":        "x",
+			"enforcement": "active",
+			"rules": []any{
+				map[string]any{
+					"type": "required_status_checks",
+					"parameters": map[string]any{
+						"required_status_checks": []any{
+							map[string]any{
+								"context":        "ci",
+								"integration_id": float64(42),
+							},
+						},
+						"strict_required_status_checks_policy": true,
+					},
+				},
+			},
+		})
+
+		result, err := handler(ContextWithDeps(context.Background(), deps), &request)
+		require.NoError(t, err)
+		if result.IsError {
+			t.Fatalf("unexpected error: %s", getErrorResult(t, result).Text)
+		}
+		assert.NotEmpty(t, capturedBody)
+	})
+
 	t.Run("bypass_actors accepts exempt bypass mode and enterprise actor types", func(t *testing.T) {
 		var capturedBody github.RepositoryRuleset
 		client := mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{