ast: fix panic when indexing composite literal values in x in [...]

Building the rule index for `<ref> in <collection>` panicked with
"illegal value" whenever the collection contained an object or set
element (or an array nesting one), since updateMemberRefInValue
inserts each collection element into the trie as-is, without
restricting it to scalars/arrays like the equality-based indexing
does. Such elements now fall back to the trie's "any" node, like an
unbound Var: the rule stays a candidate for every input, and body
evaluation determines the actual result.

Other Value types considered (Ref, comprehensions, Call) can't
actually reach the trie from compiled Rego, since the compiler
rewrites them into separate statements before the index is built;
verified this individually against `opa eval`, so the panic remains
for them as a genuine invariant check.

Fixes #8918.

Signed-off-by: Stephan Renatus <stephan.renatus@gmail.com>
This commit is contained in:
Stephan Renatus
2026-07-21 11:41:39 +02:00
committed by Stephan Renatus
parent 33fc04b53a
commit 0def2cd01e
3 changed files with 144 additions and 0 deletions
+20
View File
@@ -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 <collection>` (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")
+84
View File
@@ -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
+40
View File
@@ -997,6 +997,46 @@ func TestRegoDisableIndexingWithMatch(t *testing.T) {
}
}
// TestRegoRuleIndexMemberWithCompositeValues guards against a regression where
// building the rule index for `<ref> in <collection>` 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"),