ast: Disallowing partial object rules to have other partial object ru…

…le within their immediate extent (#5864) The compiler should disallow single-value rules to have other rules within their extent. Previously, the compiler would erroneously allow such overlaps, where the immediate "child" rule of a partial object rule in the rule tree was also a partial object rule. This condition is not expected during evaluation, which will result in a broken object merge. Fixes: #5855 Signed-off-by: Johan Fylling <[email protected]>
open-policy-agent · Apr 25, 2023 · 3311587 · 3311587
1 parent f4af919
commit 3311587
Show file tree

Hide file tree

Showing 2 changed files with 57 additions and 0 deletions.
diff --git a/ast/compile.go b/ast/compile.go
@@ -892,6 +892,9 @@ func (c *Compiler) checkRuleConflicts() {
 			//
 			//   data.p.q[r] { r := input.r } # data.p.q could be { "r": true }
 			//   data.p.q.r.s { true }
+			//
+			// data.p[r] := x { r = input.key; x = input.bar }
+			// data.p.q[r] := x { r = input.key; x = input.bar }
 
 			// But this is allowed:
 			//   data.p.q[r] = 1 { r := "r" }
@@ -900,7 +903,25 @@ func (c *Compiler) checkRuleConflicts() {
 			if r.Head.RuleKind() == SingleValue && len(node.Children) > 0 {
 				if len(ref) > 1 && !ref[len(ref)-1].IsGround() { // p.q[x] and p.q.s.t => check grandchildren
 					for _, c := range node.Children {
+						grandchildrenFound := false
+
+						if len(c.Values) > 0 {
+							childRules := extractRules(c.Values)
+							for _, childRule := range childRules {
+								childRef := childRule.Ref()
+								if childRule.Head.RuleKind() == SingleValue && !childRef[len(childRef)-1].IsGround() {
+									// The child is a partial object rule, so it's effectively "generating" grandchildren.
+									grandchildrenFound = true
+									break
+								}
+							}
+						}
+
 						if len(c.Children) > 0 {
+							grandchildrenFound = true
+						}
+
+						if grandchildrenFound {
 							singleValueConflicts = node.flattenChildren()
 							break
 						}

diff --git a/ast/compile_test.go b/ast/compile_test.go
@@ -1951,6 +1951,42 @@ func TestCompilerCheckRuleConflictsDotsInRuleHeads(t *testing.T) {
 				`),
 			err: "rego_type_error: single-value rule data.pkg.p.q[r] conflicts with [data.pkg.p.q.r.s]",
 		},
+		{
+			note: "single-value partial object with other partial object rule overlap, unknown keys (regression test for #5855)",
+			modules: modules(
+				`package pkg
+				p[r] := x { r = input.key; x = input.bar }
+				p.q[r] := x { r = input.key; x = input.bar }
+				`),
+			err: "rego_type_error: single-value rule data.pkg.p[r] conflicts with [data.pkg.p.q[r]]",
+		},
+		{
+			note: "single-value partial object with other partial object (implicit 'true' value) rule overlap, unknown keys",
+			modules: modules(
+				`package pkg
+				p[r] := x { r = input.key; x = input.bar }
+				p.q[r] { r = input.key }
+				`),
+			err: "rego_type_error: single-value rule data.pkg.p[r] conflicts with [data.pkg.p.q[r]]",
+		},
+		{
+			note: "single-value partial object with multi-value rule (ref head) overlap, unknown key",
+			modules: modules(
+				`package pkg
+				import future.keywords
+				p[r] := x { r = input.key; x = input.bar }
+				p.q contains r { r = input.key }
+				`),
+		},
+		{
+			note: "single-value partial object with multi-value rule overlap, unknown key",
+			modules: modules(
+				`package pkg
+				p[r] := x { r = input.key; x = input.bar }
+				p.q { true }
+				`),
+			err: "rego_type_error: conflicting rules data.pkg.p found",
+		},
 		{
 			note: "single-value rule with known and unknown key",
 			modules: modules(