Merge pull request #1883 from josephschorr/cel-lib

Improvements around usage of CEL

e2e/go.mod

@@ -16,8 +16,8 @@ require (
 )
 
 require (
-	github.com/antlr/antlr4/runtime/Go/antlr/v4 v4.0.0-20230512164433-5d1fd1a340c9 // indirect
-	github.com/authzed/cel-go v0.17.5 // indirect
+	github.com/antlr4-go/antlr/v4 v4.13.0 // indirect
+	github.com/authzed/cel-go v0.20.2 // indirect
 	github.com/aws/aws-sdk-go-v2 v1.26.1 // indirect
 	github.com/aws/aws-sdk-go-v2/config v1.27.11 // indirect
 	github.com/aws/aws-sdk-go-v2/credentials v1.17.11 // indirect

e2e/go.sum

@@ -25,12 +25,12 @@ github.com/IBM/pgxpoolprometheus v1.1.1 h1:xkWNUe87TIuBj/ypdSiDgNYktsuM7MoZCT8a+
 github.com/IBM/pgxpoolprometheus v1.1.1/go.mod h1:GFJDkHbidFfB2APbhBTSy2X4PKH3bLWsEMBhmzK1ipo=
 github.com/Masterminds/squirrel v1.5.4 h1:uUcX/aBc8O7Fg9kaISIUsHXdKuqehiXAMQTYX8afzqM=
 github.com/Masterminds/squirrel v1.5.4/go.mod h1:NNaOrjSoIDfDA40n7sr2tPNZRfjzjA400rg+riTZj10=
-github.com/antlr/antlr4/runtime/Go/antlr/v4 v4.0.0-20230512164433-5d1fd1a340c9 h1:goHVqTbFX3AIo0tzGr14pgfAW2ZfPChKO21Z9MGf/gk=
-github.com/antlr/antlr4/runtime/Go/antlr/v4 v4.0.0-20230512164433-5d1fd1a340c9/go.mod h1:pSwJ0fSY5KhvocuWSx4fz3BA8OrA1bQn+K1Eli3BRwM=
+github.com/antlr4-go/antlr/v4 v4.13.0 h1:lxCg3LAv+EUK6t1i0y1V6/SLeUi0eKEKdhQAlS8TVTI=
+github.com/antlr4-go/antlr/v4 v4.13.0/go.mod h1:pfChB/xh/Unjila75QW7+VU4TSnWnnk9UTnmpPaOR2g=
 github.com/authzed/authzed-go v0.11.2-0.20240418174337-42f221719227 h1:VczJwysQbGiSnJeyROxmF6/u8K7GZviVbIc4XGm9u1o=
 github.com/authzed/authzed-go v0.11.2-0.20240418174337-42f221719227/go.mod h1:EFCDZMQbrhJSpSRUlAooJdACESdA4VnlIkCz1s0Pw+g=
-github.com/authzed/cel-go v0.17.5 h1:lfpkNrR99B5QRHg5qdG9oLu/kguVlZC68VJuMk8tH9Y=
-github.com/authzed/cel-go v0.17.5/go.mod h1:XL/zEq5hKGVF8aOdMbG7w+BQPihLjY2W8N+UIygDA2I=
+github.com/authzed/cel-go v0.20.2 h1:GlmLecGry7Z8HU0k+hmaHHUV05ZHrsFxduXHtIePvck=
+github.com/authzed/cel-go v0.20.2/go.mod h1:pJHVFWbqUHV1J+klQoZubdKswlbxcsbojda3mye9kiU=
 github.com/authzed/grpcutil v0.0.0-20240123092924-129dc0a6a6e1 h1:zBfQzia6Hz45pJBeURTrv1b6HezmejB6UmiGuBilHZM=
 github.com/authzed/grpcutil v0.0.0-20240123092924-129dc0a6a6e1/go.mod h1:s3qC7V7XIbiNWERv7Lfljy/Lx25/V1Qlexb0WJuA8uQ=
 github.com/aws/aws-sdk-go-v2 v1.26.1 h1:5554eUqIYVWpU0YmeeYZ0wU64H2VLBs8TlhRB2L+EkA=

go.mod

@@ -9,7 +9,11 @@ require (
 	github.com/IBM/pgxpoolprometheus v1.1.1
 	github.com/Masterminds/squirrel v1.5.4
 	github.com/authzed/authzed-go v0.11.2-0.20240418174337-42f221719227
-	github.com/authzed/cel-go v0.17.5
+
+	// NOTE: We are using a *copy* of `cel-go` here to ensure there isn't a conflict
+	// with the version used in Kubernetes. This is a temporary measure until we can
+	// upgrade Kubernetes to a version that uses a newer version of `cel-go`.
+	github.com/authzed/cel-go v0.20.2
 	github.com/authzed/consistent v0.1.0
 	github.com/authzed/grpcutil v0.0.0-20240123092924-129dc0a6a6e1
 	github.com/aws/aws-sdk-go v1.51.32
@@ -107,6 +111,7 @@ require (
 	cloud.google.com/go/auth v0.3.0 // indirect
 	cloud.google.com/go/auth/oauth2adapt v0.2.2 // indirect
 	github.com/GoogleCloudPlatform/grpc-gcp-go/grpcgcp v1.5.0 // indirect
+	github.com/antlr4-go/antlr/v4 v4.13.0 // indirect
 	github.com/aws/aws-sdk-go-v2/credentials v1.17.11 // indirect
 	github.com/aws/aws-sdk-go-v2/feature/ec2/imds v1.16.1 // indirect
 	github.com/aws/aws-sdk-go-v2/internal/configsources v1.3.5 // indirect
@@ -152,7 +157,6 @@ require (
 	github.com/alexkohler/nakedret/v2 v2.0.4 // indirect
 	github.com/alexkohler/prealloc v1.0.0 // indirect
 	github.com/alingse/asasalint v0.0.11 // indirect
-	github.com/antlr/antlr4/runtime/Go/antlr/v4 v4.0.0-20230512164433-5d1fd1a340c9 // indirect
 	github.com/ashanbrown/forbidigo v1.6.0 // indirect
 	github.com/ashanbrown/makezero v1.1.1 // indirect
 	github.com/beorn7/perks v1.0.1 // indirect

go.sum

@@ -683,8 +683,8 @@ github.com/alingse/asasalint v0.0.11 h1:SFwnQXJ49Kx/1GghOFz1XGqHYKp21Kq1nHad/0WQ
 github.com/alingse/asasalint v0.0.11/go.mod h1:nCaoMhw7a9kSJObvQyVzNTPBDbNpdocqrSP7t/cW5+I=
 github.com/andybalholm/brotli v1.0.4/go.mod h1:fO7iG3H7G2nSZ7m0zPUDn85XEX2GTukHGRSepvi9Eig=
 github.com/antihax/optional v1.0.0/go.mod h1:uupD/76wgC+ih3iEmQUL+0Ugr19nfwCT1kdvxnR2qWY=
-github.com/antlr/antlr4/runtime/Go/antlr/v4 v4.0.0-20230512164433-5d1fd1a340c9 h1:goHVqTbFX3AIo0tzGr14pgfAW2ZfPChKO21Z9MGf/gk=
-github.com/antlr/antlr4/runtime/Go/antlr/v4 v4.0.0-20230512164433-5d1fd1a340c9/go.mod h1:pSwJ0fSY5KhvocuWSx4fz3BA8OrA1bQn+K1Eli3BRwM=
+github.com/antlr4-go/antlr/v4 v4.13.0 h1:lxCg3LAv+EUK6t1i0y1V6/SLeUi0eKEKdhQAlS8TVTI=
+github.com/antlr4-go/antlr/v4 v4.13.0/go.mod h1:pfChB/xh/Unjila75QW7+VU4TSnWnnk9UTnmpPaOR2g=
 github.com/apache/arrow/go/v10 v10.0.1/go.mod h1:YvhnlEePVnBS4+0z3fhPfUy7W1Ikj0Ih0vcRo/gZ1M0=
 github.com/apache/arrow/go/v11 v11.0.0/go.mod h1:Eg5OsL5H+e299f7u5ssuXsuHQVEGC4xei5aX110hRiI=
 github.com/apache/thrift v0.16.0/go.mod h1:PHK3hniurgQaNMZYaCLEqXKsYK8upmhPbmdP2FXSqgU=
@@ -694,8 +694,8 @@ github.com/ashanbrown/makezero v1.1.1 h1:iCQ87C0V0vSyO+M9E/FZYbu65auqH0lnsOkf5Fc
 github.com/ashanbrown/makezero v1.1.1/go.mod h1:i1bJLCRSCHOcOa9Y6MyF2FTfMZMFdHvxKHxgO5Z1axI=
 github.com/authzed/authzed-go v0.11.2-0.20240418174337-42f221719227 h1:VczJwysQbGiSnJeyROxmF6/u8K7GZviVbIc4XGm9u1o=
 github.com/authzed/authzed-go v0.11.2-0.20240418174337-42f221719227/go.mod h1:EFCDZMQbrhJSpSRUlAooJdACESdA4VnlIkCz1s0Pw+g=
-github.com/authzed/cel-go v0.17.5 h1:lfpkNrR99B5QRHg5qdG9oLu/kguVlZC68VJuMk8tH9Y=
-github.com/authzed/cel-go v0.17.5/go.mod h1:XL/zEq5hKGVF8aOdMbG7w+BQPihLjY2W8N+UIygDA2I=
+github.com/authzed/cel-go v0.20.2 h1:GlmLecGry7Z8HU0k+hmaHHUV05ZHrsFxduXHtIePvck=
+github.com/authzed/cel-go v0.20.2/go.mod h1:pJHVFWbqUHV1J+klQoZubdKswlbxcsbojda3mye9kiU=
 github.com/authzed/consistent v0.1.0 h1:tlh1wvKoRbjRhMm2P+X5WQQyR54SRoS4MyjLOg17Mp8=
 github.com/authzed/consistent v0.1.0/go.mod h1:plwHlrN/EJUCwQ+Bca0MhM1KnisPs7HEkZI5giCXrcc=
 github.com/authzed/grpcutil v0.0.0-20240123092924-129dc0a6a6e1 h1:zBfQzia6Hz45pJBeURTrv1b6HezmejB6UmiGuBilHZM=

internal/namespace/caveats.go

@@ -41,7 +41,11 @@ func ValidateCaveatDefinition(caveat *core.CaveatDefinition) error {
 		)
 	}
 
-	referencedNames := deserialized.ReferencedParameters(maps.Keys(caveat.ParameterTypes))
+	referencedNames, err := deserialized.ReferencedParameters(maps.Keys(caveat.ParameterTypes))
+	if err != nil {
+		return err
+	}
+
 	for paramName, paramType := range caveat.ParameterTypes {
 		_, err := caveattypes.DecodeParameterType(paramType)
 		if err != nil {

pkg/caveats/compile.go

@@ -59,19 +59,24 @@ func (cc CompiledCaveat) Serialize() ([]byte, error) {
 }
 
 // ReferencedParameters returns the names of the parameters referenced in the expression.
-func (cc CompiledCaveat) ReferencedParameters(parameters []string) *mapz.Set[string] {
+func (cc CompiledCaveat) ReferencedParameters(parameters []string) (*mapz.Set[string], error) {
 	referencedParams := mapz.NewSet[string]()
 	definedParameters := mapz.NewSet[string]()
 	definedParameters.Extend(parameters)
 
-	referencedParameters(definedParameters, cc.ast.Expr(), referencedParams)
-	return referencedParams
+	checked, err := cel.AstToCheckedExpr(cc.ast)
+	if err != nil {
+		return nil, err
+	}
+
+	referencedParameters(definedParameters, checked.Expr, referencedParams)
+	return referencedParams, nil
 }
 
 // CompileCaveatWithName compiles a caveat string into a compiled caveat with a given name,
 // or returns the compilation errors.
 func CompileCaveatWithName(env *Environment, exprString, name string) (*CompiledCaveat, error) {
-	c, err := CompileCaveatWithSource(env, name, common.NewStringSource(exprString, name))
+	c, err := CompileCaveatWithSource(env, name, common.NewStringSource(exprString, name), nil)
 	if err != nil {
 		return nil, err
 	}
@@ -80,7 +85,7 @@ func CompileCaveatWithName(env *Environment, exprString, name string) (*Compiled
 }
 
 // CompileCaveatWithSource compiles a caveat source into a compiled caveat, or returns the compilation errors.
-func CompileCaveatWithSource(env *Environment, name string, source common.Source) (*CompiledCaveat, error) {
+func CompileCaveatWithSource(env *Environment, name string, source common.Source, startPosition SourcePosition) (*CompiledCaveat, error) {
 	celEnv, err := env.asCelEnvironment()
 	if err != nil {
 		return nil, err
@@ -92,7 +97,42 @@ func CompileCaveatWithSource(env *Environment, name string, source common.Source
 
 	ast, issues := celEnv.CompileSource(source)
 	if issues != nil && issues.Err() != nil {
-		return nil, CompilationErrors{issues.Err(), issues}
+		if startPosition == nil {
+			return nil, CompilationErrors{issues.Err(), issues}
+		}
+
+		// Construct errors with the source location adjusted based on the starting source position
+		// in the parent schema (if any). This ensures that the errors coming out of CEL show the correct
+		// *overall* location information..
+		line, col, err := startPosition.LineAndColumn()
+		if err != nil {
+			return nil, err
+		}
+
+		adjustedErrors := common.NewErrors(source)
+		for _, existingErr := range issues.Errors() {
+			location := existingErr.Location
+
+			// NOTE: Our locations are zero-indexed while CEL is 1-indexed, so we need to adjust the line/column values accordingly.
+			if location.Line() == 1 {
+				location = common.NewLocation(line+location.Line(), col+location.Column())
+			} else {
+				location = common.NewLocation(line+location.Line(), location.Column())
+			}
+
+			adjustedError := &common.Error{
+				Message:  existingErr.Message,
+				ExprID:   existingErr.ExprID,
+				Location: location,
+			}
+
+			adjustedErrors = adjustedErrors.Append([]*common.Error{
+				adjustedError,
+			})
+		}
+
+		adjustedIssues := cel.NewIssues(adjustedErrors)
+		return nil, CompilationErrors{adjustedIssues.Err(), adjustedIssues}
 	}
 
 	if ast.OutputType() != cel.BoolType {
@@ -107,7 +147,7 @@ func CompileCaveatWithSource(env *Environment, name string, source common.Source
 // compileCaveat compiles a caveat string into a compiled caveat, or returns the compilation errors.
 func compileCaveat(env *Environment, exprString string) (*CompiledCaveat, error) {
 	s := common.NewStringSource(exprString, "caveat")
-	return CompileCaveatWithSource(env, "caveat", s)
+	return CompileCaveatWithSource(env, "caveat", s, nil)
 }
 
 // DeserializeCaveat deserializes a byte-serialized caveat back into a CompiledCaveat.

pkg/caveats/env.go

@@ -78,7 +78,7 @@ func (e *Environment) asCelEnvironment() (*cel.Env, error) {
 
 	// EnableMacroCallTracking: enables tracking of call macros so when we call AstToString we get
 	// back out the expected expressions.
-	// See: https://github.com/google/cel-go/issues/474
+	// See: https://github.com/authzed/cel-go/issues/474
 	opts = append(opts, cel.EnableMacroCallTracking())
 
 	// ParserExpressionSizeLimit: disable the size limit for codepoints in expressions.

pkg/caveats/eval.go

@@ -8,7 +8,6 @@ import (
 	"github.com/authzed/cel-go/cel"
 	"github.com/authzed/cel-go/common/types"
 	"github.com/authzed/cel-go/common/types/ref"
-	"github.com/authzed/cel-go/interpreter"
 )
 
 // EvaluationConfig is configuration given to an EvaluateCaveatWithConfig call.
@@ -47,8 +46,12 @@ func (cr CaveatResult) PartialValue() (*CompiledCaveat, error) {
 		return nil, fmt.Errorf("result is fully evaluated")
 	}
 
-	expr := interpreter.PruneAst(cr.parentCaveat.ast.Expr(), cr.parentCaveat.ast.SourceInfo().GetMacroCalls(), cr.details.State())
-	return &CompiledCaveat{cr.parentCaveat.celEnv, cel.ParsedExprToAst(expr), cr.parentCaveat.name}, nil
+	ast, err := cr.parentCaveat.celEnv.ResidualAst(cr.parentCaveat.ast, cr.details)
+	if err != nil {
+		return nil, err
+	}
+
+	return &CompiledCaveat{cr.parentCaveat.celEnv, ast, cr.parentCaveat.name}, nil
 }
 
 // ContextValues returns the context values used when computing this result.

pkg/caveats/source.go

@@ -1,8 +1,6 @@
 package caveats
 
 import (
-	"strings"
-
 	"github.com/authzed/cel-go/common"
 )
 
@@ -16,31 +14,6 @@ type SourcePosition interface {
 }
 
 // NewSource creates a new source for compilation into a caveat.
-func NewSource(expressionString string, startPosition SourcePosition, name string) (common.Source, error) {
-	startingRunePosition, err := startPosition.RunePosition()
-	if err != nil {
-		return nil, err
-	}
-
-	startingLine, startingCol, err := startPosition.LineAndColumn()
-	if err != nil {
-		return nil, err
-	}
-
-	// Synthesize a contents string with the same positioning as the caveat expression,
-	// but with all tokens before the expression string replaced with whitespace. This is
-	// done to ensure that the positions produced by the CEL parser match those in the parent
-	// schema/string. Otherwise, CEL would report the positions relative to only the expression
-	// itself, which is not as nice for debugging.
-	// NOTE: This is likely not the most efficient means of doing this, so reevaluate if/when the
-	// CEL location code allows for better customization of offsets or starts returning more
-	// well-typed errors that we can more easily rewrite.
-	adjustedContents := strings.Repeat(" ", startingRunePosition-startingLine-startingCol)
-	for i := 0; i < startingLine; i++ {
-		adjustedContents += "\n"
-	}
-	adjustedContents += strings.Repeat(" ", startingCol)
-	adjustedContents += expressionString
-
-	return common.NewStringSource(adjustedContents, name), nil
+func NewSource(expressionString string, name string) (common.Source, error) {
+	return common.NewStringSource(expressionString, name), nil
 }

pkg/caveats/structure_test.go

@@ -95,9 +95,12 @@ func TestReferencedParameters(t *testing.T) {
 
 			sort.Strings(tc.referencedParamNames)
 
-			found := compiled.ReferencedParameters(maps.Keys(tc.env.variables)).AsSlice()
-			sort.Strings(found)
-			require.Equal(t, tc.referencedParamNames, found)
+			found, err := compiled.ReferencedParameters(maps.Keys(tc.env.variables))
+			require.NoError(t, err)
+
+			foundSlice := found.AsSlice()
+			sort.Strings(foundSlice)
+			require.Equal(t, tc.referencedParamNames, foundSlice)
 		})
 	}
 }

pkg/namespace/builder.go

@@ -122,6 +122,10 @@ func CaveatDefinition(env *caveats.Environment, name string, expr string) (*core
 
 // CompiledCaveatDefinition returns a new caveat definition.
 func CompiledCaveatDefinition(env *caveats.Environment, name string, compiled *caveats.CompiledCaveat) (*core.CaveatDefinition, error) {
+	if compiled == nil {
+		return nil, spiceerrors.MustBugf("compiled caveat is nil")
+	}
+
 	serialized, err := compiled.Serialize()
 	if err != nil {
 		return nil, err

pkg/schemadsl/compiler/translator.go

@@ -150,12 +150,12 @@ func translateCaveatDefinition(tctx translationContext, defNode *dslNode) (*core
 		return nil, defNode.ErrorWithSourcef(expressionString, "invalid expression: %w", err)
 	}
 
-	source, err := caveats.NewSource(expressionString, rnge.Start(), caveatPath)
+	source, err := caveats.NewSource(expressionString, caveatPath)
 	if err != nil {
 		return nil, defNode.ErrorWithSourcef(expressionString, "invalid expression: %w", err)
 	}
 
-	compiled, err := caveats.CompileCaveatWithSource(env, caveatPath, source)
+	compiled, err := caveats.CompileCaveatWithSource(env, caveatPath, source, rnge.Start())
 	if err != nil {
 		return nil, expressionStringNode.ErrorWithSourcef(expressionString, "invalid expression for caveat `%s`: %w", definitionName, err)
 	}

tools/analyzers/go.work.sum

@@ -1192,10 +1192,10 @@ github.com/golangci/maligned v0.0.0-20180506175553-b1d89398deca h1:kNY3/svz5T29M
 github.com/golangci/maligned v0.0.0-20180506175553-b1d89398deca/go.mod h1:tvlJhZqDe4LMs4ZHD0oMUlt9G2LWuDGoisJTBzLMV9o=
 github.com/google/btree v1.0.1 h1:gK4Kx5IaGY9CD5sPJ36FHiBJ6ZXl0kilRiiCj+jdYp4=
 github.com/google/btree v1.0.1/go.mod h1:xXMiIv4Fb/0kKde4SpL7qlzvu5cMJDRkFDxJfI9uaxA=
-github.com/google/cel-go v0.17.1 h1:s2151PDGy/eqpCI80/8dl4VL3xTkqI/YubXLXCFw0mw=
-github.com/google/cel-go v0.17.1/go.mod h1:HXZKzB0LXqer5lHHgfWAnlYwJaQBDKMjxjulNQzhwhY=
-github.com/google/cel-go v0.17.7 h1:6ebJFzu1xO2n7TLtN+UBqShGBhlD85bhvglh5DpcfqQ=
-github.com/google/cel-go v0.17.7/go.mod h1:HXZKzB0LXqer5lHHgfWAnlYwJaQBDKMjxjulNQzhwhY=
+github.com/authzed/cel-go v0.17.1 h1:s2151PDGy/eqpCI80/8dl4VL3xTkqI/YubXLXCFw0mw=
+github.com/authzed/cel-go v0.17.1/go.mod h1:HXZKzB0LXqer5lHHgfWAnlYwJaQBDKMjxjulNQzhwhY=
+github.com/authzed/cel-go v0.17.7 h1:6ebJFzu1xO2n7TLtN+UBqShGBhlD85bhvglh5DpcfqQ=
+github.com/authzed/cel-go v0.17.7/go.mod h1:HXZKzB0LXqer5lHHgfWAnlYwJaQBDKMjxjulNQzhwhY=
 github.com/google/go-github/v41 v41.0.0 h1:HseJrM2JFf2vfiZJ8anY2hqBjdfY1Vlj/K27ueww4gg=
 github.com/google/go-github/v41 v41.0.0/go.mod h1:XgmCA5H323A9rtgExdTcnDkcqp6S30AVACCBDOonIxg=
 github.com/google/go-pkcs11 v0.2.0 h1:5meDPB26aJ98f+K9G21f0AqZwo/S5BJMJh8nuhMbdsI=