diff --git a/go/internal/store/agent_config.go b/go/internal/store/agent_config.go index fb3ccc6a..150fee11 100644 --- a/go/internal/store/agent_config.go +++ b/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) } } } diff --git a/go/internal/store/agent_config_test.go b/go/internal/store/agent_config_test.go index 2f0c33b5..123badfb 100644 --- a/go/internal/store/agent_config_test.go +++ b/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) {