ast: Disallow root document shadowing in leading term of rule refs

when: * strict-mode is enabled * `rego.v1` is imported Fixes: #6291 Signed-off-by: Johan Fylling <[email protected]>
open-policy-agent · Nov 16, 2023 · ba021ab · ba021ab
1 parent 946d12b
commit ba021ab
Show file tree

Hide file tree

Showing 2 changed files with 144 additions and 1 deletion.
diff --git a/ast/compile.go b/ast/compile.go
@@ -1715,7 +1715,12 @@ func checkKeywordOverrides(node interface{}, strict bool) Errors {
 	errs := Errors{}
 
 	WalkRules(node, func(rule *Rule) bool {
-		name := rule.Head.Name.String()
+		var name string
+		if len(rule.Head.Reference) > 0 {
+			name = rule.Head.Reference[0].Value.(Var).String()
+		} else {
+			name = rule.Head.Name.String()
+		}
 		if RootDocumentRefs.Contains(RefTerm(VarTerm(name))) {
 			errs = append(errs, NewError(CompileErr, rule.Location, "rules must not shadow %v (use a different rule name)", name))
 		}

diff --git a/ast/compile_test.go b/ast/compile_test.go
@@ -2997,6 +2997,60 @@ func TestCompilerCheckKeywordOverrides(t *testing.T) {
 				},
 			},
 		},
+		{
+			note: "rule names (set construction)",
+			module: `package test
+				input.a { true }
+				p { true }
+				data.b { true }
+			`,
+			expectedErrors: Errors{
+				&Error{
+					Location: NewLocation([]byte("input.a { true }"), "", 2, 5),
+					Message:  "rules must not shadow input (use a different rule name)",
+				},
+				&Error{
+					Location: NewLocation([]byte("data.b { true }"), "", 4, 5),
+					Message:  "rules must not shadow data (use a different rule name)",
+				},
+			},
+		},
+		{
+			note: "rule names (object construction)",
+			module: `package test
+				input.a := 1 { true }
+				p { true }
+				data.b := 2 { true }
+			`,
+			expectedErrors: Errors{
+				&Error{
+					Location: NewLocation([]byte("input.a := 1 { true }"), "", 2, 5),
+					Message:  "rules must not shadow input (use a different rule name)",
+				},
+				&Error{
+					Location: NewLocation([]byte("data.b := 2 { true }"), "", 4, 5),
+					Message:  "rules must not shadow data (use a different rule name)",
+				},
+			},
+		},
+		{
+			note: "leading term in rule refs",
+			module: `package test
+				input.a.b { true }
+				p { true }
+				data.b.c := "foo" { true }
+			`,
+			expectedErrors: Errors{
+				&Error{
+					Location: NewLocation([]byte("input.a.b { true }"), "", 2, 5),
+					Message:  "rules must not shadow input (use a different rule name)",
+				},
+				&Error{
+					Location: NewLocation([]byte(`data.b.c := "foo" { true }`), "", 4, 5),
+					Message:  "rules must not shadow data (use a different rule name)",
+				},
+			},
+		},
 		{
 			note: "global assignments",
 			module: `package test
@@ -3527,6 +3581,48 @@ func TestCompileRegoV1Import(t *testing.T) {
 				},
 			},
 		},
+		{
+			note: "rule (object) shadows input",
+			modules: map[string]string{
+				"policy.rego": `package test
+					import rego.v1
+					input.a := "b" if { true }`,
+			},
+			expectedErrors: Errors{
+				&Error{
+					Message:  "rules must not shadow input (use a different rule name)",
+					Location: &Location{Text: []byte(`input.a := "b" if { true }`), File: "policy.rego", Row: 3, Col: 6},
+				},
+			},
+		},
+		{
+			note: "rule (set) shadows input",
+			modules: map[string]string{
+				"policy.rego": `package test
+					import rego.v1
+					input contains "a" if { true }`,
+			},
+			expectedErrors: Errors{
+				&Error{
+					Message:  "rules must not shadow input (use a different rule name)",
+					Location: &Location{Text: []byte(`input contains "a" if { true }`), File: "policy.rego", Row: 3, Col: 6},
+				},
+			},
+		},
+		{
+			note: "rule ref shadows input",
+			modules: map[string]string{
+				"policy.rego": `package test
+					import rego.v1
+					input.a.b.c := 1`,
+			},
+			expectedErrors: Errors{
+				&Error{
+					Message:  "rules must not shadow input (use a different rule name)",
+					Location: &Location{Text: []byte("input.a.b.c := 1"), File: "policy.rego", Row: 3, Col: 6},
+				},
+			},
+		},
 		{
 			note: "rule shadows input (multiple modules)",
 			modules: map[string]string{
@@ -3578,6 +3674,48 @@ func TestCompileRegoV1Import(t *testing.T) {
 				},
 			},
 		},
+		{
+			note: "rule (object) shadows input",
+			modules: map[string]string{
+				"policy.rego": `package test
+					import rego.v1
+					data.a := "b" if { true }`,
+			},
+			expectedErrors: Errors{
+				&Error{
+					Message:  "rules must not shadow data (use a different rule name)",
+					Location: &Location{Text: []byte(`data.a := "b" if { true }`), File: "policy.rego", Row: 3, Col: 6},
+				},
+			},
+		},
+		{
+			note: "rule (set) shadows input",
+			modules: map[string]string{
+				"policy.rego": `package test
+					import rego.v1
+					data contains "a" if { true }`,
+			},
+			expectedErrors: Errors{
+				&Error{
+					Message:  "rules must not shadow data (use a different rule name)",
+					Location: &Location{Text: []byte(`data contains "a" if { true }`), File: "policy.rego", Row: 3, Col: 6},
+				},
+			},
+		},
+		{
+			note: "rule ref shadows input",
+			modules: map[string]string{
+				"policy.rego": `package test
+					import rego.v1
+					data.a.b.c := 1`,
+			},
+			expectedErrors: Errors{
+				&Error{
+					Message:  "rules must not shadow data (use a different rule name)",
+					Location: &Location{Text: []byte("data.a.b.c := 1"), File: "policy.rego", Row: 3, Col: 6},
+				},
+			},
+		},
 		{
 			note: "rule shadows data (multiple modules)",
 			modules: map[string]string{