From 27fe5ceac871d020e10dd4a92d4eb6fa7070400a Mon Sep 17 00:00:00 2001 From: Johan Fylling Date: Wed, 29 Jul 2026 19:06:54 +0200 Subject: [PATCH] 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 --- v1/ast/parser.go | 4 +- v1/ast/parser_logical_test.go | 105 +++++++++++++++++- v1/ast/parser_test.go | 61 +++++++++- .../v0_to_v1/test_not_future_import.rego | 3 +- 4 files changed, 164 insertions(+), 9 deletions(-) diff --git a/v1/ast/parser.go b/v1/ast/parser.go index 272cb8eb29..3ecf654a54 100644 --- a/v1/ast/parser.go +++ b/v1/ast/parser.go @@ -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 { diff --git a/v1/ast/parser_logical_test.go b/v1/ast/parser_logical_test.go index 9e7ea19670..7edff3879c 100644 --- a/v1/ast/parser_logical_test.go +++ b/v1/ast/parser_logical_test.go @@ -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` / diff --git a/v1/ast/parser_test.go b/v1/ast/parser_test.go index 48be6fb83b..4bf8995223 100644 --- a/v1/ast/parser_test.go +++ b/v1/ast/parser_test.go @@ -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)) + } + } }) } } diff --git a/v1/format/testfiles/v0_to_v1/test_not_future_import.rego b/v1/format/testfiles/v0_to_v1/test_not_future_import.rego index 936c38fdb5..489f163f23 100644 --- a/v1/format/testfiles/v0_to_v1/test_not_future_import.rego +++ b/v1/format/testfiles/v0_to_v1/test_not_future_import.rego @@ -2,7 +2,6 @@ package test import future.keywords.not -a if { +a { not input.x + input.y == 2 } -