From 0def2cd01ee290ea6af84283adcedd515d30ac90 Mon Sep 17 00:00:00 2001 From: Stephan Renatus Date: Tue, 21 Jul 2026 11:41:39 +0200 Subject: [PATCH] ast: fix panic when indexing composite literal values in `x in [...]` Building the rule index for ` in ` 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 --- v1/ast/index.go | 20 +++++++++++ v1/ast/index_test.go | 84 ++++++++++++++++++++++++++++++++++++++++++++ v1/rego/rego_test.go | 40 +++++++++++++++++++++ 3 files changed, 144 insertions(+) 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"),