Skip to content

Commit

Permalink
ast: Disallow root document shadowing in leading term of rule refs
Browse files Browse the repository at this point in the history
when:
* strict-mode is enabled
* `rego.v1` is imported

Fixes: open-policy-agent#6291
Signed-off-by: Johan Fylling <[email protected]>
  • Loading branch information
johanfylling committed Nov 16, 2023
1 parent e71e519 commit 18c42ff
Show file tree
Hide file tree
Showing 2 changed files with 144 additions and 1 deletion.
7 changes: 6 additions & 1 deletion ast/compile.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))
}
Expand Down
138 changes: 138 additions & 0 deletions ast/compile_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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{
Expand Down Expand Up @@ -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{
Expand Down

0 comments on commit 18c42ff

Please sign in to comment.