Support for typename import aliases

common/containers/container.go

@@ -19,6 +19,7 @@ package containers
 import (
 	"fmt"
 	"strings"
+	"unicode"
 
 	"github.com/google/cel-go/common/ast"
 )
@@ -212,6 +213,13 @@ type ContainerOption func(*Container) (*Container, error)
 func Abbrevs(qualifiedNames ...string) ContainerOption {
 	return func(c *Container) (*Container, error) {
 		for _, qn := range qualifiedNames {
+			qn = strings.TrimSpace(qn)
+			for _, r := range []rune(qn) {
+				if !isIdentifierChar(r) {
+					return nil, fmt.Errorf(
+						"invalid qualified name: %s, wanted name of the form 'qualified.name'", qn)
+				}
+			}
 			ind := strings.LastIndex(qn, ".")
 			if ind <= 0 || ind >= len(qn)-1 {
 				return nil, fmt.Errorf(
@@ -278,6 +286,10 @@ func aliasAs(kind, qualifiedName, alias string) ContainerOption {
 	}
 }
 
+func isIdentifierChar(r rune) bool {
+	return r <= unicode.MaxASCII && (r == '.' || r == '_' || unicode.IsLetter(r) || unicode.IsNumber(r))
+}
+
 // Name sets the fully-qualified name of the Container.
 func Name(name string) ContainerOption {
 	return func(c *Container) (*Container, error) {

common/containers/container_test.go

@@ -15,6 +15,7 @@
 package containers
 
 import (
+	"fmt"
 	"reflect"
 	"testing"
 
@@ -104,54 +105,79 @@ func TestContainers_Abbrevs(t *testing.T) {
 }
 
 func TestContainers_Aliasing_Errors(t *testing.T) {
-	_, err := NewContainer(Abbrevs("my.alias.R", "yer.other.R"))
-	wantErr := "abbreviation collides with existing reference: " +
-		"name=yer.other.R, abbreviation=R, existing=my.alias.R"
-	if err == nil || err.Error() != wantErr {
-		t.Errorf("got error %v, expected %s.", err, wantErr)
-	}
-
-	_, err = NewContainer(Name("a.b.c.M.N"), Abbrevs("my.alias.a", "yer.other.b"))
-	wantErr = "abbreviation collides with container name: name=my.alias.a, " +
-		"abbreviation=a, container=a.b.c.M.N"
-	if err == nil || err.Error() != wantErr {
-		t.Errorf("got error %v, expected %s.", err, wantErr)
-	}
-
-	_, err = NewContainer(Abbrevs(".bad"))
-	wantErr = "invalid qualified name: .bad, wanted name of the form 'qualified.name'"
-	if err == nil || err.Error() != wantErr {
-		t.Errorf("got error %v, expected %s.", err, wantErr)
-	}
-
-	_, err = NewContainer(Abbrevs("bad.alias."))
-	wantErr = "invalid qualified name: bad.alias., wanted name of the form 'qualified.name'"
-	if err == nil || err.Error() != wantErr {
-		t.Errorf("got error %v, expected %s.", err, wantErr)
-	}
-
-	_, err = NewContainer(Alias("a", "b"))
-	wantErr = "alias must refer to a valid qualified name: a"
-	if err == nil || err.Error() != wantErr {
-		t.Errorf("got error %v, expected %s.", err, wantErr)
-	}
-
-	_, err = NewContainer(Alias("my.alias", "b.c"))
-	wantErr = "alias must be non-empty and simple (not qualified): alias=b.c"
-	if err == nil || err.Error() != wantErr {
-		t.Errorf("got error %v, expected %s.", err, wantErr)
-	}
-
-	_, err = NewContainer(Alias(".my.qual.name", "a"))
-	wantErr = "qualified name must not begin with a leading '.': .my.qual.name"
-	if err == nil || err.Error() != wantErr {
-		t.Errorf("got error %v, expected %s.", err, wantErr)
-	}
-
-	_, err = NewContainer(Alias(".my.qual.name", "a"))
-	wantErr = "qualified name must not begin with a leading '.': .my.qual.name"
-	if err == nil || err.Error() != wantErr {
-		t.Errorf("got error %v, expected %s.", err, wantErr)
+	type aliasDef struct {
+		name  string
+		alias string
+	}
+	tests := []struct {
+		container string
+		abbrevs   []string
+		aliases   []aliasDef
+		err       string
+	}{
+		{
+			abbrevs: []string{"my.alias.R", "yer.other.R"},
+			err: "abbreviation collides with existing reference: " +
+				"name=yer.other.R, abbreviation=R, existing=my.alias.R",
+		},
+		{
+			container: "a.b.c.M.N",
+			abbrevs:   []string{"my.alias.a", "yer.other.b"},
+			err: "abbreviation collides with container name: name=my.alias.a, " +
+				"abbreviation=a, container=a.b.c.M.N",
+		},
+		{
+			abbrevs: []string{".bad"},
+			err:     "invalid qualified name: .bad, wanted name of the form 'qualified.name'",
+		},
+		{
+			abbrevs: []string{"bad.alias."},
+			err:     "invalid qualified name: bad.alias., wanted name of the form 'qualified.name'",
+		},
+		{
+			abbrevs: []string{"   bad_alias1"},
+			err:     "invalid qualified name: bad_alias1, wanted name of the form 'qualified.name'",
+		},
+		{
+			abbrevs: []string{"   bad.alias!  "},
+			err:     "invalid qualified name: bad.alias!, wanted name of the form 'qualified.name'",
+		},
+		{
+			aliases: []aliasDef{{name: "a", alias: "b"}},
+			err:     "alias must refer to a valid qualified name: a",
+		},
+		{
+			aliases: []aliasDef{{name: "my.alias", alias: "b.c"}},
+			err:     "alias must be non-empty and simple (not qualified): alias=b.c",
+		},
+		{
+			aliases: []aliasDef{{name: ".my.qual.name", alias: "a'"}},
+			err:     "qualified name must not begin with a leading '.': .my.qual.name",
+		},
+	}
+	for i, tst := range tests {
+		tc := tst
+		t.Run(fmt.Sprintf("[%d]", i), func(t *testing.T) {
+			opts := []ContainerOption{}
+			if tc.container != "" {
+				opts = append(opts, Name(tc.container))
+			}
+			if len(tc.abbrevs) != 0 {
+				opts = append(opts, Abbrevs(tc.abbrevs...))
+			}
+			if len(tc.aliases) != 0 {
+				for _, a := range tc.aliases {
+					opts = append(opts, Alias(a.name, a.alias))
+				}
+			}
+			_, err := NewContainer(opts...)
+			if err == nil {
+				t.Fatalf("NewContainer() succeeded, wanted err %s", tc.err)
+			}
+			if err.Error() != tc.err {
+				t.Errorf("NewContainer() got error %s, wanted error %s", err.Error(), tc.err)
+			}
+		})
 	}
 }
 

policy/BUILD.bazel

@@ -34,6 +34,7 @@ go_library(
         "//cel:go_default_library",
         "//common:go_default_library",
         "//common/ast:go_default_library",
+        "//common/containers:go_default_library",
         "//common/decls:go_default_library",
         "//common/operators:go_default_library",
         "//common/types:go_default_library",

policy/compiler.go

@@ -22,6 +22,7 @@ import (
 	"github.com/google/cel-go/cel"
 	"github.com/google/cel-go/common"
 	"github.com/google/cel-go/common/ast"
+	"github.com/google/cel-go/common/containers"
 	"github.com/google/cel-go/common/decls"
 	"github.com/google/cel-go/common/types"
 	"github.com/google/cel-go/common/types/ref"
@@ -211,17 +212,42 @@ func CompileRule(env *cel.Env, p *Policy, opts ...CompilerOption) (*CompiledRule
 		src:                  p.Source(),
 		maxNestedExpressions: defaultMaxNestedExpressions,
 	}
+	var err error
 	errs := common.NewErrors(c.src)
 	iss := cel.NewIssuesWithSourceInfo(errs, c.info)
 	for _, o := range opts {
-		if err := o(c); err != nil {
+		if err = o(c); err != nil {
 			iss.ReportErrorAtID(p.Name().ID, "error configuring compiler option: %s", err)
 			return nil, iss
 		}
 	}
-	rule, ruleIss := c.compileRule(p.Rule(), c.env, iss)
-	iss = iss.Append(ruleIss)
-	return rule, iss
+	c.env, err = c.env.Extend(cel.EagerlyValidateDeclarations(true))
+	if err != nil {
+		iss.ReportErrorAtID(p.Name().ID, "error configuring environment: %s", err)
+		return nil, iss
+	}
+
+	importCount := len(p.Imports())
+	if importCount > 0 {
+		importNames := make([]string, 0, importCount)
+		for _, imp := range p.Imports() {
+			typeName := imp.Name().Value
+			_, err := containers.NewContainer(containers.Abbrevs(typeName))
+			if err != nil {
+				iss.ReportErrorAtID(imp.Name().ID, "error configuring import: %s", err)
+			} else {
+				importNames = append(importNames, typeName)
+			}
+		}
+		env, err := c.env.Extend(cel.Abbrevs(importNames...))
+		if err != nil {
+			// validation happens earlier in the sequence, so this should be unreachable.
+			iss.ReportErrorAtID(p.Imports()[0].SourceID(), "error configuring imports: %s", err)
+		} else {
+			c.env = env
+		}
+	}
+	return c.compileRule(p.Rule(), c.env, iss)
 }
 
 type compiler struct {
@@ -234,7 +260,6 @@ type compiler struct {
 }
 
 func (c *compiler) compileRule(r *Rule, ruleEnv *cel.Env, iss *cel.Issues) (*CompiledRule, *cel.Issues) {
-	var err error
 	compiledVars := make([]*CompiledVariable, len(r.Variables()))
 	for i, v := range r.Variables() {
 		exprSrc := c.relSource(v.Expression())
@@ -254,9 +279,11 @@ func (c *compiler) compileRule(r *Rule, ruleEnv *cel.Env, iss *cel.Issues) (*Com
 		// Introduce the variable into the environment. By extending the environment, the variables
 		// are effectively scoped such that they must be declared before use.
 		varDecl := decls.NewVariable(fmt.Sprintf("%s.%s", variablePrefix, varName), varType)
-		ruleEnv, err = ruleEnv.Extend(cel.Variable(varDecl.Name(), varDecl.Type()))
+		varEnv, err := ruleEnv.Extend(cel.Variable(varDecl.Name(), varDecl.Type()))
 		if err != nil {
-			iss.ReportErrorAtID(v.Expression().ID, "invalid variable declaration")
+			iss.ReportErrorAtID(v.exprID, "invalid variable declaration: %s", err.Error())
+		} else {
+			ruleEnv = varEnv
 		}
 		compiledVar := &CompiledVariable{
 			exprID:  v.name.ID,

policy/compiler_test.go

@@ -86,6 +86,20 @@ func TestCompiledRuleHasOptionalOutput(t *testing.T) {
 	}
 }
 
+func TestMaxNestedExpressions_Error(t *testing.T) {
+	policyName := "required_labels"
+	wantError := `ERROR: testdata/required_labels/policy.yaml:15:8: error configuring compiler option: nested expression limit must be non-negative, non-zero value: -1
+ | name: "required_labels"
+ | .......^`
+	_, _, iss := compile(t, policyName, []ParserOption{}, []cel.EnvOption{}, []CompilerOption{MaxNestedExpressions(-1)})
+	if iss.Err() == nil {
+		t.Fatalf("compile(%s) did not error, wanted %s", policyName, wantError)
+	}
+	if iss.Err().Error() != wantError {
+		t.Errorf("compile(%s) got error %s, wanted %s", policyName, iss.Err().Error(), wantError)
+	}
+}
+
 func BenchmarkCompile(b *testing.B) {
 	for _, tst := range policyTests {
 		r := newRunner(b, tst.name, tst.expr, tst.parseOpts, tst.envOpts...)

policy/helper_test.go

@@ -63,33 +63,37 @@ var (
 		{
 			name: "nested_rule2",
 			expr: `
-	cel.bind(variables.permitted_regions, ["us", "uk", "es"], 
-	  resource.?user.orValue("").startsWith("bad") 
-	  ? cel.bind(variables.banned_regions, {"us": false, "ru": false, "ir": false}, 
+	cel.bind(variables.permitted_regions, ["us", "uk", "es"],
+	  resource.?user.orValue("").startsWith("bad")
+	  ? cel.bind(variables.banned_regions, {"us": false, "ru": false, "ir": false},
 	    (resource.origin in variables.banned_regions &&
-        !(resource.origin in variables.permitted_regions)) 
-		? {"banned": "restricted_region"} : {"banned": "bad_actor"}) 
-	  : (!(resource.origin in variables.permitted_regions) 
+        !(resource.origin in variables.permitted_regions))
+		? {"banned": "restricted_region"} : {"banned": "bad_actor"})
+	  : (!(resource.origin in variables.permitted_regions)
 	    ? {"banned": "unconfigured_region"} : {}))`,
 		},
 		{
 			name: "nested_rule3",
 			expr: `
-	cel.bind(variables.permitted_regions, ["us", "uk", "es"], 
-	  resource.?user.orValue("").startsWith("bad") 
+	cel.bind(variables.permitted_regions, ["us", "uk", "es"],
+	  resource.?user.orValue("").startsWith("bad")
 	  ? optional.of(
 	      cel.bind(variables.banned_regions, {"us": false, "ru": false, "ir": false},
 		  (resource.origin in variables.banned_regions &&
-          !(resource.origin in variables.permitted_regions)) 
-		  ? {"banned": "restricted_region"} : {"banned": "bad_actor"})) 
-	  : (!(resource.origin in variables.permitted_regions) 
+          !(resource.origin in variables.permitted_regions))
+		  ? {"banned": "restricted_region"} : {"banned": "bad_actor"}))
+	  : (!(resource.origin in variables.permitted_regions)
 	    ? optional.of({"banned": "unconfigured_region"}) : optional.none()))`,
 		},
 		{
 			name: "pb",
-			expr: `(spec.single_int32 > 10)
-			? optional.of("invalid spec, got single_int32=%d, wanted <= 10".format([spec.single_int32]))
-			: optional.none()`,
+			expr: `
+	(spec.single_int32 > google.expr.proto3.test.TestAllTypes{single_int64: 10}.single_int64)
+	? optional.of("invalid spec, got single_int32=%d, wanted <= 10".format([spec.single_int32]))
+	: ((spec.standalone_enum == google.expr.proto3.test.TestAllTypes.NestedEnum.BAR ||
+      google.expr.proto3.test.ImportedGlobalEnum.IMPORT_BAR in spec.imported_enums)
+	  ? optional.of("invalid spec, neither nested nor imported enums may refer to BAR or IMPORT_BAR")
+	  : optional.none())`,
 			envOpts: []cel.EnvOption{
 				cel.Types(&proto3pb.TestAllTypes{}),
 			},
@@ -168,25 +172,34 @@ var (
 	}{
 		{
 			name: "errors",
-			err: `ERROR: testdata/errors/policy.yaml:19:19: undeclared reference to 'spec' (in container '')
+			err: `ERROR: testdata/errors/policy.yaml:19:1: error configuring import: invalid qualified name: punc.Import!, wanted name of the form 'qualified.name'
+ |       punc.Import!
+ | ^
+ERROR: testdata/errors/policy.yaml:20:12: error configuring import: invalid qualified name: bad import, wanted name of the form 'qualified.name'
+ |   - name: "bad import"
+ | ...........^
+ERROR: testdata/errors/policy.yaml:24:19: undeclared reference to 'spec' (in container '')
  |       expression: spec.labels
  | ..................^
-ERROR: testdata/errors/policy.yaml:21:50: Syntax error: mismatched input 'resource' expecting ')'
+ERROR: testdata/errors/policy.yaml:25:7: invalid variable declaration: overlapping identifier for name 'variables.want'
+ |     - name: want
+ | ......^
+ERROR: testdata/errors/policy.yaml:28:50: Syntax error: mismatched input 'resource' expecting ')'
  |       expression: variables.want.filter(l, !(lin resource.labels))
  | .................................................^
-ERROR: testdata/errors/policy.yaml:21:66: Syntax error: extraneous input ')' expecting <EOF>
+ERROR: testdata/errors/policy.yaml:28:66: Syntax error: extraneous input ')' expecting <EOF>
  |       expression: variables.want.filter(l, !(lin resource.labels))
  | .................................................................^
-ERROR: testdata/errors/policy.yaml:23:27: Syntax error: mismatched input '2' expecting {'}', ','}
+ERROR: testdata/errors/policy.yaml:30:27: Syntax error: mismatched input '2' expecting {'}', ','}
  |       expression: "{1:305 2:569}"
  | ..........................^
-ERROR: testdata/errors/policy.yaml:31:75: Syntax error: extraneous input ']' expecting ')'
+ERROR: testdata/errors/policy.yaml:38:75: Syntax error: extraneous input ']' expecting ')'
  |         "missing one or more required labels: %s".format(variables.missing])
  | ..........................................................................^
-ERROR: testdata/errors/policy.yaml:34:67: undeclared reference to 'format' (in container '')
+ERROR: testdata/errors/policy.yaml:41:67: undeclared reference to 'format' (in container '')
  |         "invalid values provided on one or more labels: %s".format([variables.invalid])
  | ..................................................................^
-ERROR: testdata/errors/policy.yaml:38:16: incompatible output types: bool not assignable to string
+ERROR: testdata/errors/policy.yaml:45:16: incompatible output types: bool not assignable to string
  |       output: "'false'"
  | ...............^`,
 		},