fix(store): descend through non-string-keyed nodes in models.yml credential denylist (RIG-3116)

go/internal/store/agent_config.go

@@ -1000,6 +1000,42 @@ func yamlMapIndex(node any, key string) (any, bool) {
 	}
 }
 
+// yamlMapEntry is one entry of a YAML-decoded mapping node; key is rendered to a
+// string for diagnostics (see yamlMapEntries).
+type yamlMapEntry struct {
+	key string
+	val any
+}
+
+// yamlMapEntries returns every entry of a YAML-decoded mapping node as a slice,
+// handling both map[string]any and the map[any]any yaml.v3 produces when ANY key
+// is non-string. Non-string keys are rendered with fmt.Sprint (they name a
+// provider/header and are used only for diagnostics). A non-mapping node yields
+// nil. It is the yamlMapIndex analog for a security walk that must ENUMERATE
+// members: iterating through both shapes means a non-string sibling key can no
+// longer flip the node's Go type and fail-open a map[string]any-only assertion
+// that would skip a credential-bearing sibling. Returning a slice (not a
+// normalized map) is collision-safe: a non-string key that renders to the same
+// string as a sibling cannot overwrite and hide it.
+func yamlMapEntries(node any) []yamlMapEntry {
+	switch m := node.(type) {
+	case map[string]any:
+		out := make([]yamlMapEntry, 0, len(m))
+		for k, v := range m {
+			out = append(out, yamlMapEntry{key: k, val: v})
+		}
+		return out
+	case map[any]any:
+		out := make([]yamlMapEntry, 0, len(m))
+		for k, v := range m {
+			out = append(out, yamlMapEntry{key: fmt.Sprint(k), val: v})
+		}
+		return out
+	default:
+		return nil
+	}
+}
+
 // rejectCredentialModels rejects a models.yml member that sets either of the two
 // credential-bearing provider surfaces (CP-4):
 //
@@ -1016,29 +1052,25 @@ func yamlMapIndex(node any, key string) (any, bool) {
 // `!command`) passes; anything else (contains spaces, punctuation like a bearer
 // token, a URL, etc.) is treated as a literal secret and rejected.
 func rejectCredentialModels(mapping map[string]any, joined string) error {
-	providers, ok := mapping["providers"].(map[string]any)
-	if !ok {
+	providers, present := mapping["providers"]
+	if !present {
 		return nil
 	}
-	for name, raw := range providers {
-		prov, ok := raw.(map[string]any)
-		if !ok {
-			continue
-		}
-		if v, present := prov["apiKey"]; present && v != nil {
-			return fmt.Errorf("%w: models member %q sets providers.%s.apiKey (provider credentials never ride the config bundle)", ErrInvalidArgument, joined, name)
+	for _, p := range yamlMapEntries(providers) {
+		if v, present := yamlMapIndex(p.val, "apiKey"); present && v != nil {
+			return fmt.Errorf("%w: models member %q sets providers.%s.apiKey (provider credentials never ride the config bundle)", ErrInvalidArgument, joined, p.key)
 		}
-		headers, ok := prov["headers"].(map[string]any)
+		headers, ok := yamlMapIndex(p.val, "headers")
 		if !ok {
 			continue
 		}
-		for h, hv := range headers {
-			s, ok := hv.(string)
+		for _, hdr := range yamlMapEntries(headers) {
+			s, ok := hdr.val.(string)
 			if !ok {
 				continue
 			}
 			if !isEnvIndirection(s) {
-				return fmt.Errorf("%w: models member %q sets providers.%s.headers.%s to a literal secret (pin it to an env reference instead)", ErrInvalidArgument, joined, name, h)
+				return fmt.Errorf("%w: models member %q sets providers.%s.headers.%s to a literal secret (pin it to an env reference instead)", ErrInvalidArgument, joined, p.key, hdr.key)
 			}
 		}
 	}

go/internal/store/agent_config_test.go

@@ -421,6 +421,10 @@ func TestValidateConfigBundleAcceptsNewMembers(t *testing.T) {
 		{"top-level models.yml mapping", []tarEntry{{name: "models.yml", content: "providers:\n  x:\n    baseUrl: https://y\n"}}},
 		{"models.yml headers env reference", []tarEntry{{name: "models.yml", content: "providers:\n  x:\n    headers:\n      X-Org: MY_ORG_ENV\n"}}},
 		{"models.yml headers !command indirection", []tarEntry{{name: "models.yml", content: "providers:\n  x:\n    headers:\n      Authorization: \"!op read secret\"\n"}}},
+		// Symmetric to the shielded-credential models.yml reject cases: a nested
+		// non-string key with NO credential leaf must still be ACCEPTED. Guards
+		// against yamlMapEntries over-rejecting benign models config.
+		{"models.yml nested non-string key no credential", []tarEntry{{name: "models.yml", content: "providers:\n  x:\n    0: junk\n    baseUrl: https://y\n"}}},
 	}
 	for _, tc := range cases {
 		t.Run(tc.name, func(t *testing.T) {
@@ -529,6 +533,42 @@ func TestValidateConfigBundleRejectsCredentialKeys(t *testing.T) {
 			member:  tarEntry{name: "models.yml", content: "providers:\n  x:\n    headers:\n      Authorization: \"Bearer sk-live-123\"\n"},
 			wantSub: "providers.x.headers.Authorization",
 		},
+		// A non-string sibling key flips the enclosing yaml.v3 node to
+		// map[any]any; a map[string]any-only descent fails OPEN and rides the
+		// credential past the door. rejectCredentialModels must descend through
+		// both map shapes (yamlMapEntries/yamlMapIndex) so no sibling can shield
+		// a credential leaf. Three shield sites, one per assertion the walk makes.
+		{
+			// (a) provider-level shield: a non-string sibling under the provider
+			// flips the provider node, hiding its apiKey.
+			name:    "models apiKey shielded by non-string sibling at provider",
+			member:  tarEntry{name: "models.yml", content: "providers:\n  openai:\n    0: junk\n    apiKey: sk-secret\n"},
+			wantSub: "providers.openai.apiKey",
+		},
+		{
+			// (b) providers-level shield: a non-string key directly under
+			// providers flips the providers node, hiding EVERY provider.
+			name:    "models apiKey shielded by non-string sibling at providers",
+			member:  tarEntry{name: "models.yml", content: "providers:\n  0: junk\n  openai:\n    apiKey: sk-secret\n"},
+			wantSub: "providers.openai.apiKey",
+		},
+		{
+			// (c) headers-level shield: a non-string sibling in headers flips the
+			// headers node, hiding a literal-secret header.
+			name:    "models header literal secret shielded by non-string sibling",
+			member:  tarEntry{name: "models.yml", content: "providers:\n  x:\n    headers:\n      0: junk\n      Authorization: \"Bearer sk-live-123\"\n"},
+			wantSub: "providers.x.headers.Authorization",
+		},
+		{
+			// (d) combined shield: a non-string sibling at BOTH the providers
+			// node AND the provider node simultaneously. The two helpers descend
+			// linearly with no shared state, so this cannot fail if (a)+(b) pass;
+			// pinned explicitly to defend the composition against a future refactor
+			// that couples the descents.
+			name:    "models apiKey shielded by non-string siblings at providers and provider",
+			member:  tarEntry{name: "models.yml", content: "providers:\n  0: junk\n  openai:\n    1: junk\n    apiKey: sk-secret\n"},
+			wantSub: "providers.openai.apiKey",
+		},
 	}
 	for _, tc := range cases {
 		t.Run(tc.name, func(t *testing.T) {