mirror of
https://github.com/open-policy-agent/opa.git
synced 2026-08-12 19:32:48 -06:00
ast: Fix leaky future.keywords.not import in Rego v0 (#8953)
Fixing an issue where the `future.keywords.not` import would erroneously
import other future keywords.
E.g. consider the following v0 module:
```rego
package example
import future.keywords.not
p if {
not input.x
}
```
The `future.keywords.not` import also imports the `if` keyword. This fix
makes the above module invalid.
Signed-off-by: Johan Fylling <johan.dev@fylling.se>
This commit is contained in:
+2
-2
@@ -3740,9 +3740,9 @@ func (p *Parser) futureImport(imp *Import, allowedFutureKeywords map[string]toke
|
||||
|
||||
if keyword == "not" {
|
||||
p.notBodies = true
|
||||
} else {
|
||||
kwds = []string{keyword} // overwrite
|
||||
}
|
||||
|
||||
kwds = []string{keyword} // overwrite
|
||||
}
|
||||
|
||||
for _, kw := range kwds {
|
||||
|
||||
@@ -448,7 +448,9 @@ func TestParseLogical_RefsContainingAndOr(t *testing.T) {
|
||||
func TestParseLogical_NoLeakageOnImport(t *testing.T) {
|
||||
t.Run("import error does not leak keyword names", func(t *testing.T) {
|
||||
opts := ParserOptions{Capabilities: CapabilitiesForThisVersion()}
|
||||
input := "package x\nimport future.keywords.and\n"
|
||||
input := `package x
|
||||
import future.keywords.and
|
||||
`
|
||||
_, _, err := ParseStatementsWithOpts("", input, opts)
|
||||
if err == nil {
|
||||
t.Fatal("expected parse error for import of and without experimental caps")
|
||||
@@ -468,11 +470,110 @@ func TestParseLogical_NoLeakageOnImport(t *testing.T) {
|
||||
Capabilities: CapabilitiesForThisVersion(CapabilitiesExperimentalKeywords(true)),
|
||||
FutureKeywords: []string{"and"},
|
||||
}
|
||||
input := "package x\nallow if { x and y }\n"
|
||||
input := `package x
|
||||
allow if { x and y }
|
||||
`
|
||||
if _, _, err := ParseStatementsWithOpts("", input, opts); err != nil {
|
||||
t.Fatalf("unexpected error: %v", err)
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("unrelated keyword import does not activate logical keywords", func(t *testing.T) {
|
||||
tests := []struct {
|
||||
note string
|
||||
module string
|
||||
}{
|
||||
{
|
||||
note: "and, only future.keywords.not imported",
|
||||
module: `package x
|
||||
import future.keywords.not
|
||||
|
||||
p if input.a and input.b
|
||||
`,
|
||||
},
|
||||
{
|
||||
note: "or, only future.keywords.not imported",
|
||||
module: `package x
|
||||
import future.keywords.not
|
||||
|
||||
p if input.a or input.b
|
||||
`,
|
||||
},
|
||||
{
|
||||
note: "and, only future.keywords.in imported",
|
||||
module: `package x
|
||||
import future.keywords.in
|
||||
|
||||
p if input.a and input.b
|
||||
`,
|
||||
},
|
||||
{
|
||||
note: "or, only future.keywords.in imported",
|
||||
module: `package x
|
||||
import future.keywords.in
|
||||
|
||||
p if input.a or input.b
|
||||
`,
|
||||
},
|
||||
}
|
||||
opts := ParserOptions{
|
||||
RegoVersion: RegoV1,
|
||||
Capabilities: CapabilitiesForThisVersion(CapabilitiesExperimentalKeywords(true)),
|
||||
}
|
||||
for _, tc := range tests {
|
||||
t.Run(tc.note, func(t *testing.T) {
|
||||
// `and`/`or` are plain identifiers here, so three bare terms in a
|
||||
// row is not a valid expression. Asserting on the error text would
|
||||
// be brittle; what matters is that no logical node is produced.
|
||||
mod, err := ParseModuleWithOpts("test.rego", tc.module, opts)
|
||||
if err == nil {
|
||||
t.Fatalf("expected parse error, got body: %v (%T)",
|
||||
mod.Rules[0].Body, mod.Rules[0].Body[0].Terms)
|
||||
}
|
||||
})
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("unrelated keyword import does not reserve logical keywords as var names", func(t *testing.T) {
|
||||
tests := []struct {
|
||||
note string
|
||||
module string
|
||||
}{
|
||||
{
|
||||
note: "and",
|
||||
module: `package x
|
||||
import future.keywords.not
|
||||
|
||||
p if {
|
||||
and := 1
|
||||
and == 1
|
||||
}
|
||||
`,
|
||||
},
|
||||
{
|
||||
note: "or",
|
||||
module: `package x
|
||||
import future.keywords.not
|
||||
|
||||
p if {
|
||||
or := 1
|
||||
or == 1
|
||||
}
|
||||
`,
|
||||
},
|
||||
}
|
||||
opts := ParserOptions{
|
||||
RegoVersion: RegoV1,
|
||||
Capabilities: CapabilitiesForThisVersion(CapabilitiesExperimentalKeywords(true)),
|
||||
}
|
||||
for _, tc := range tests {
|
||||
t.Run(tc.note, func(t *testing.T) {
|
||||
if _, err := ParseModuleWithOpts("test.rego", tc.module, opts); err != nil {
|
||||
t.Errorf("expected %q to remain usable as a variable name, got: %v", tc.note, err)
|
||||
}
|
||||
})
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
// TestParseLogical_PartialActivation exercises the case where one of `and` /
|
||||
|
||||
+58
-3
@@ -2937,10 +2937,13 @@ func TestFutureImports(t *testing.T) {
|
||||
|
||||
func TestFutureAndRegoV1ImportsExtraction(t *testing.T) {
|
||||
// These tests assert that "import future..." and "import rego.v1" statements in policies cause
|
||||
// the proper keywords to be added to the parser's list of known keywords.
|
||||
// the proper keywords to be added to the parser's list of known keywords, and that they don't add any others.
|
||||
tests := []struct {
|
||||
note, imp string
|
||||
exp map[string]tokens.Token
|
||||
note, imp string
|
||||
regoVersion RegoVersion
|
||||
capabilities *Capabilities
|
||||
exp map[string]tokens.Token
|
||||
absent []string
|
||||
}{
|
||||
{
|
||||
note: "simple import",
|
||||
@@ -2979,10 +2982,57 @@ func TestFutureAndRegoV1ImportsExtraction(t *testing.T) {
|
||||
"if": tokens.If,
|
||||
},
|
||||
},
|
||||
{
|
||||
// A single-keyword import must not activate any other keyword.
|
||||
note: "not imported in v0 does not enable the v0 future keywords",
|
||||
regoVersion: RegoV0,
|
||||
imp: "import future.keywords.not",
|
||||
absent: []string{"in", "every", "contains", "if"},
|
||||
},
|
||||
{
|
||||
note: "not imported in v0 does not enable experimental future keywords",
|
||||
regoVersion: RegoV0,
|
||||
capabilities: CapabilitiesForThisVersion(CapabilitiesExperimentalKeywords(true)),
|
||||
imp: "import future.keywords.not",
|
||||
absent: []string{"and", "or"},
|
||||
},
|
||||
{
|
||||
note: "in imported in v0 does not enable the other v0 future keywords",
|
||||
regoVersion: RegoV0,
|
||||
imp: "import future.keywords.in",
|
||||
exp: map[string]tokens.Token{"in": tokens.In},
|
||||
absent: []string{"every", "contains", "if"},
|
||||
},
|
||||
{
|
||||
note: "not imported does not enable experimental keywords",
|
||||
capabilities: CapabilitiesForThisVersion(CapabilitiesExperimentalKeywords(true)),
|
||||
imp: "import future.keywords.not",
|
||||
absent: []string{"and", "or"},
|
||||
},
|
||||
{
|
||||
note: "in imported does not enable experimental keywords",
|
||||
capabilities: CapabilitiesForThisVersion(CapabilitiesExperimentalKeywords(true)),
|
||||
imp: "import future.keywords.in",
|
||||
exp: map[string]tokens.Token{"in": tokens.In},
|
||||
absent: []string{"and", "or"},
|
||||
},
|
||||
{
|
||||
note: "and imported does not enable or",
|
||||
capabilities: CapabilitiesForThisVersion(CapabilitiesExperimentalKeywords(true)),
|
||||
imp: "import future.keywords.and",
|
||||
exp: map[string]tokens.Token{"and": tokens.LogicalAnd},
|
||||
absent: []string{"or"},
|
||||
},
|
||||
}
|
||||
for _, tc := range tests {
|
||||
t.Run(tc.note, func(t *testing.T) {
|
||||
parser := NewParser().WithFilename("").WithReader(bytes.NewBufferString(tc.imp))
|
||||
if tc.regoVersion != RegoUndefined {
|
||||
parser = parser.WithRegoVersion(tc.regoVersion)
|
||||
}
|
||||
if tc.capabilities != nil {
|
||||
parser = parser.WithCapabilities(tc.capabilities)
|
||||
}
|
||||
_, _, errs := parser.Parse()
|
||||
if exp, act := 0, len(errs); exp != act {
|
||||
t.Fatalf("expected %d errors, got %d: %v", exp, act, errs)
|
||||
@@ -2993,6 +3043,11 @@ func TestFutureAndRegoV1ImportsExtraction(t *testing.T) {
|
||||
t.Errorf("expected keyword %q to yield token %v, got %v", kw, exp, act)
|
||||
}
|
||||
}
|
||||
for _, kw := range tc.absent {
|
||||
if parser.s.s.IsKeyword(kw) {
|
||||
t.Errorf("expected %q not to be a keyword, but it yields token %v", kw, parser.s.s.Keyword(kw))
|
||||
}
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
@@ -2,7 +2,6 @@ package test
|
||||
|
||||
import future.keywords.not
|
||||
|
||||
a if {
|
||||
a {
|
||||
not input.x + input.y == 2
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user