diff --git a/v1/ast/index.go b/v1/ast/index.go index 46c4a2fd34..93a0f8d330 100644 --- a/v1/ast/index.go +++ b/v1/ast/index.go @@ -813,6 +813,18 @@ func (node *trieNode) insertValue(value Value) *trieNode { case *Array: node.array = util.Or(node.array, newTrieNodeImpl) return node.array.insertArray(value) + + // `x in ` (see updateMemberRefInValue) inserts each element of + // the literal collection as-is, without restricting it to scalars/arrays + // like the equality-based indexing does (see indexValue). A ground + // Object or Set element can't be indexed precisely, so - like Var - it + // falls back to the "any" node: the rule stays a candidate for every + // input value. (The other composite Value types - Ref, comprehensions, + // Call - can't actually reach here: the compiler rewrites them into + // separate statements, bound to a Var, before the index is built.) + case Object, Set: + node.any = util.Or(node.any, newTrieNodeImpl) + return node.any } panic("illegal value") @@ -834,6 +846,14 @@ func (node *trieNode) insertArray(arr *Array) *trieNode { node.scalars.Put(head, child) } return child.insertArray(arr.Slice(1, -1)) + + // Same reasoning as in insertValue above: an array element can itself be + // a nested array, object, or set, none of which can be indexed precisely + // at this position, so fall back to "any" and keep indexing the + // remaining elements. + case *Array, Object, Set: + node.any = util.Or(node.any, newTrieNodeImpl) + return node.any.insertArray(arr.Slice(1, -1)) } panic("illegal value") diff --git a/v1/ast/index_test.go b/v1/ast/index_test.go index 35ce2cf805..0d585a854d 100644 --- a/v1/ast/index_test.go +++ b/v1/ast/index_test.go @@ -929,6 +929,90 @@ func TestBaseDocEqIndexing(t *testing.T) { input: `{"role": "guest"}`, expectedRS: []string{}, }, + { + // Regression test: elements of a collection literal in `x in [...]` + // used to be inserted into the trie as-is (see updateMemberRefInValue), + // without restricting them to scalars/arrays like the equality-based + // indexing does (see indexValue). A non-scalar element (object or set) + // made the trie panic with "illegal value", since it only knew how to + // store scalars and arrays of scalars. Since the ref can't be indexed + // precisely here, it falls back to the trie's "any" node (like Var): + // the rule remains a candidate for every input, and it's up to body + // evaluation (not the index) to determine the actual result. + note: "internal.member_2: composite (object) element in rhs array (no match, but still a candidate)", + module: module(`package test + p if { + __local0__ = input.role + internal.member_2(__local0__, [{"nested": 1}]) + }`), + ruleset: "p", + input: `{"role": "other"}`, + expectedRS: []string{ + `p if { __local0__ = input.role; internal.member_2(__local0__, [{"nested": 1}]) }`}, + }, + { + // Same fallback as above, for the other composite type in the case (Set). + note: "internal.member_2: composite (set) element in rhs array (candidate regardless of match)", + module: module(`package test + p if { + __local0__ = input.role + internal.member_2(__local0__, [{1, 2}]) + }`), + ruleset: "p", + input: `{"role": "other"}`, + expectedRS: []string{ + `p if { __local0__ = input.role; internal.member_2(__local0__, [{1, 2}]) }`}, + }, + { + // insertArray's handling of array elements had the same gap for + // nested arrays specifically (no "any" fallback for a non-scalar + // head element), independent of the object/set case above. Unlike + // that case, this fallback lives under the ref's "array" child (not + // its top-level "any" node), so it only makes the rule a candidate + // for inputs that are themselves arrays of matching length - it + // doesn't degrade indexing for unrelated input types. + note: "internal.member_2: nested array-in-array element in rhs (array input still a candidate)", + module: module(`package test + p if { + __local0__ = input.role + internal.member_2(__local0__, [[[1, 2]], [[3, 4]]]) + }`), + ruleset: "p", + input: `{"role": [[9, 9]]}`, + expectedRS: []string{ + `p if { __local0__ = input.role; internal.member_2(__local0__, [[[1, 2]], [[3, 4]]]) }`}, + }, + { + // A non-array input can never equal either collection element (both + // are arrays), so the index stays precise here: no panic, and no + // over-broad "any" fallback leaking into unrelated input types. + note: "internal.member_2: nested array-in-array element in rhs (scalar input not a candidate)", + module: module(`package test + p if { + __local0__ = input.role + internal.member_2(__local0__, [[[1, 2]], [[3, 4]]]) + }`), + ruleset: "p", + input: `{"role": "other"}`, + expectedRS: []string{}, + }, + { + // insertArray's fallback for a non-scalar, non-array head element + // (object/set nested inside an array, as opposed to the object/set + // case above which is a direct element of the outer collection) + // exercises the same "any" branch as the *Array case just above, but + // for Object/Set. + note: "internal.member_2: composite (object) element nested inside array element in rhs (candidate regardless of match)", + module: module(`package test + p if { + __local0__ = input.role + internal.member_2(__local0__, [[1, {"nested": 1}]]) + }`), + ruleset: "p", + input: `{"role": [1, "x"]}`, + expectedRS: []string{ + `p if { __local0__ = input.role; internal.member_2(__local0__, [[1, {"nested": 1}]]) }`}, + }, { note: "internal.member_2: var with array value in rhs (match)", module: module(`package test diff --git a/v1/rego/rego_test.go b/v1/rego/rego_test.go index 508c22abf2..0b0cbaa5dc 100644 --- a/v1/rego/rego_test.go +++ b/v1/rego/rego_test.go @@ -997,6 +997,46 @@ func TestRegoDisableIndexingWithMatch(t *testing.T) { } } +// TestRegoRuleIndexMemberWithCompositeValues guards against a regression where +// building the rule index for ` in ` panicked with "illegal +// value" whenever the collection contained non-scalar elements (objects, +// sets, or arrays thereof), since the index trie only knew how to store +// scalars and arrays of scalars. +func TestRegoRuleIndexMemberWithCompositeValues(t *testing.T) { + mod := ` + package test + + allow if input.resource in [{"nested": 1}] + ` + + cases := []struct { + note string + input map[string]any + allowed bool + }{ + {"matches composite element", map[string]any{"resource": map[string]any{"nested": 1.0}}, true}, + {"does not match composite element", map[string]any{"resource": map[string]any{"nested": 2.0}}, false}, + } + + for _, tc := range cases { + t.Run(tc.note, func(t *testing.T) { + rs, err := New( + Query("data.test.allow"), + Module("test.rego", mod), + Input(tc.input), + ).Eval(t.Context()) + if err != nil { + t.Fatalf("unexpected error (this is the panic path if the fix regresses): %s", err) + } + + allowed := len(rs) == 1 && len(rs[0].Expressions) == 1 && rs[0].Expressions[0].Value == true + if allowed != tc.allowed { + t.Fatalf("expected allowed=%v, got %v (result: %v)", tc.allowed, allowed, rs) + } + }) + } +} + func TestRegoCatchPathConflicts(t *testing.T) { r := New( Query("data"),