Skip to content

Commit 1bfe113

Browse files
TristonianJonesTristonianJones
committed
Replace 'emit' with 'output' to conform to spec update
1 parent f2aa09e commit 1bfe113

7 files changed

Lines changed: 74 additions & 84 deletions

File tree

‎policy/compiler_test.go‎

Lines changed: 22 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -274,16 +274,18 @@ func parseAndCompilePolicy(t testing.TB, name string, policySource string, envOp
274274
return env, ast, iss
275275
}
276276

277-
func newRunner(name, expr string, parseOpts []ParserOption, opts ...cel.EnvOption) *runner {
277+
func newRunner(name, expr string, parseOpts []ParserOption, envOpts ...cel.EnvOption) *runner {
278278
return &runner{
279279
name: name,
280280
parseOpts: parseOpts,
281+
envOpts: envOpts,
281282
expr: expr}
282283
}
283284

284285
type runner struct {
285286
name string
286287
parseOpts []ParserOption
288+
envOpts []cel.EnvOption
287289
env *cel.Env
288290
expr string
289291
prg cel.Program
@@ -306,6 +308,12 @@ func (r *runner) compileRule(t testing.TB) (*cel.Env, *CompiledRule, *cel.Issues
306308
if err != nil {
307309
t.Fatalf("cel.NewEnv() failed: %v", err)
308310
}
311+
if len(r.envOpts) > 0 {
312+
env, err = env.Extend(r.envOpts...)
313+
if err != nil {
314+
t.Fatalf("env.Extend() with env options failed: %v", err)
315+
}
316+
}
309317
// Configure declarations
310318
env, err = env.Extend(FromConfig(config))
311319
if err != nil {
@@ -621,9 +629,9 @@ func TestCompileYAMLPolicy_Aggregate(t *testing.T) {
621629
rule:
622630
aggregate:
623631
- condition: 'true'
624-
emit: '"PII"'
632+
output: '"PII"'
625633
- condition: 'true'
626-
emit: '"CONFIDENTIAL"'`,
634+
output: '"CONFIDENTIAL"'`,
627635
expectedUnparsed: `["PII"] + ["CONFIDENTIAL"]`,
628636
evals: []testEval{
629637
{
@@ -643,9 +651,9 @@ rule:
643651
expression: '"CONFIDENTIAL"'
644652
aggregate:
645653
- condition: 'true'
646-
emit: 'variables.val1'
654+
output: 'variables.val1'
647655
- condition: 'true'
648-
emit: 'variables.val2'`,
656+
output: 'variables.val2'`,
649657
expectedUnparsed: `cel.@block(["PII", "CONFIDENTIAL"], [@index0] + [@index1])`,
650658
evals: []testEval{
651659
{
@@ -663,11 +671,11 @@ rule:
663671
expression: "5"
664672
aggregate:
665673
- condition: "size(resource.payload) > variables.threshold"
666-
emit: '"CSE1"'
674+
output: '"CSE1"'
667675
- condition: "size(resource.payload) > variables.threshold"
668-
emit: '"CSE2"'
676+
output: '"CSE2"'
669677
- condition: 'true'
670-
emit: '"ALWAYS"'`,
678+
output: '"ALWAYS"'`,
671679
envOpts: []cel.EnvOption{
672680
cel.Variable("resource", cel.MapType(cel.StringType, cel.ListType(cel.IntType))),
673681
},
@@ -705,7 +713,7 @@ rule:
705713
- condition: "true"
706714
output: "payload.filter(x, x > variables.min_val).exists(y, y % 2 == 0)"
707715
- condition: "true"
708-
emit: "payload.all(x, x > 0)"`,
716+
output: "payload.all(x, x > 0)"`,
709717
envOpts: []cel.EnvOption{
710718
cel.Variable("cond", cel.BoolType),
711719
cel.Variable("payload", cel.ListType(cel.IntType)),
@@ -721,7 +729,7 @@ rule:
721729
rule:
722730
aggregate:
723731
- condition: 'true'
724-
emit: "'foo'"`,
732+
output: "'foo'"`,
725733
wantErr: "nested aggregate rules are not allowed",
726734
},
727735
{
@@ -736,7 +744,7 @@ rule:
736744
rule:
737745
aggregate:
738746
- condition: 'true'
739-
emit: "'foo'"`,
747+
output: "'foo'"`,
740748
wantErr: "nested aggregate rules are not allowed",
741749
},
742750
{
@@ -748,7 +756,7 @@ rule:
748756
rule:
749757
aggregate:
750758
- condition: 'true'
751-
emit: "'foo'"`,
759+
output: "'foo'"`,
752760
expectedUnparsed: `["foo"]`,
753761
},
754762
}
@@ -807,7 +815,7 @@ func TestCompiledRuleSemantic(t *testing.T) {
807815
rule:
808816
aggregate:
809817
- condition: 'true'
810-
emit: "'foo'"`
818+
output: "'foo'"`
811819
policy := parsePolicySource(t, "aggregate_semantic", policySource)
812820
env, err := cel.NewEnv()
813821
if err != nil {
@@ -827,7 +835,7 @@ func TestCompileYAMLPolicy_ConditionAlwaysFalse(t *testing.T) {
827835
rule:
828836
aggregate:
829837
- condition: 'false'
830-
emit: "'foo'"`
838+
output: "'foo'"`
831839
_, _, iss := parseAndCompilePolicy(t, "condition_always_false", policySource, nil, nil)
832840
if iss.Err() == nil {
833841
t.Fatalf("Compile() succeeded, wanted error")

‎policy/composer.go‎

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -180,7 +180,7 @@ func (opt *ruleComposerImpl) Optimize(ctx *cel.OptimizerContext, a *ast.AST) *as
180180
return ctx.NewAST(blockExpr)
181181
}
182182

183-
func (opt *ruleComposerImpl) optimizeRule(ctx *cel.OptimizerContext, r *CompiledRule, isAggregateParent bool) ast.Expr {
183+
func (opt *ruleComposerImpl) optimizeRule(ctx *cel.OptimizerContext, r *CompiledRule, asList bool) ast.Expr {
184184
// Visitor to rewrite variables-prefixed identifiers with index names.
185185
opt.enterScope()
186186
defer opt.exitScope()
@@ -190,7 +190,7 @@ func (opt *ruleComposerImpl) optimizeRule(ctx *cel.OptimizerContext, r *Compiled
190190
}
191191

192192
isAggregate := r.semantic == aggregate
193-
returnList := isAggregateParent || isAggregate
193+
returnList := isAggregate || asList
194194

195195
matches := r.Matches()
196196
matchCount := len(matches)
@@ -214,11 +214,10 @@ func (opt *ruleComposerImpl) optimizeRule(ctx *cel.OptimizerContext, r *Compiled
214214
// one.
215215
out := ctx.CopyASTAndMetadata(m.Output().Expr().NativeRep())
216216
if returnList {
217-
outList := ctx.NewList([]ast.Expr{out}, []int32{})
218-
currentStep = newNonOptionalCompositionStep(ctx, cond, outList)
219-
} else {
220-
currentStep = newNonOptionalCompositionStep(ctx, cond, out)
217+
out = ctx.NewList([]ast.Expr{out}, []int32{})
221218
}
219+
currentStep = newNonOptionalCompositionStep(ctx, cond, out)
220+
222221
} else if m.NestedRule() != nil {
223222
// If the match has a nested rule, then compute the rule and whether it has
224223
// an optional return value.
@@ -238,6 +237,10 @@ func (opt *ruleComposerImpl) optimizeRule(ctx *cel.OptimizerContext, r *Compiled
238237
} else {
239238
currentStep = newNonOptionalCompositionStep(ctx, cond, nestedRule)
240239
}
240+
} else {
241+
// Report an error for an unknown rule kind:
242+
ctx.ReportErrorAtID(cond.ID(), "unknown match kind: %v", m.SourceID())
243+
return nil
241244
}
242245

243246
if isAggregate {
@@ -247,10 +250,8 @@ func (opt *ruleComposerImpl) optimizeRule(ctx *cel.OptimizerContext, r *Compiled
247250
}
248251
}
249252

250-
if output == nil {
251-
if returnList {
252-
output = newNonOptionalCompositionStep(ctx, ctx.NewLiteral(types.True), ctx.NewList([]ast.Expr{}, []int32{}))
253-
}
253+
if output == nil && returnList {
254+
output = newNonOptionalCompositionStep(ctx, ctx.NewLiteral(types.True), ctx.NewList([]ast.Expr{}, []int32{}))
254255
}
255256

256257
matchExpr := output.expr()

‎policy/composer_test.go‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -171,7 +171,7 @@ func TestRuleComposerUnnest(t *testing.T) {
171171
for _, tst := range composerUnnestTests {
172172
tc := tst
173173
t.Run(tc.name, func(t *testing.T) {
174-
r := newRunner(tc.name, tc.expr, []ParserOption{})
174+
r := newRunner(tc.name, tc.expr, []ParserOption{}, tc.envOpts...)
175175
env, rule, iss := r.compileRule(t)
176176
if iss.Err() != nil {
177177
t.Fatalf("CompileRule() failed: %v", iss.Err())
@@ -214,9 +214,16 @@ func verifySourceInfoTransfer(t *testing.T, compiledRule *CompiledRule, composed
214214
ranges: &dstRanges})
215215
}
216216
ast.PostOrderVisit(composed.NativeRep().Expr(), &collectRanges{sourceInfo: composed.NativeRep().SourceInfo(), ranges: &dstRanges})
217+
for _, v := range compiledRule.variables {
218+
check(v.expr)
219+
}
217220
for _, match := range compiledRule.matches {
218221
check(match.cond)
219-
check(match.output.expr)
222+
if match.output != nil {
223+
check(match.output.expr)
224+
} else if match.nestedRule != nil {
225+
verifySourceInfoTransfer(t, match.nestedRule, composed)
226+
}
220227
}
221228
}
222229

‎policy/helper_test.go‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -158,6 +158,7 @@ var (
158158
expr string
159159
composed string
160160
composerOpts []ComposerOption
161+
envOpts []cel.EnvOption
161162
outputType *cel.Type
162163
}{
163164
{
@@ -231,6 +232,30 @@ var (
231232
(now.getHours() >= 20) ? @index5 : optional.of(@index3.format([@index0, @index2])))`,
232233
outputType: cel.OptionalType(cel.StringType),
233234
},
235+
{
236+
name: "agent_tool_execution_governance",
237+
composerOpts: []ComposerOption{ExpressionUnnestHeight(2)},
238+
envOpts: []cel.EnvOption{
239+
cel.Function("classifier.has_credit_card",
240+
cel.Overload("classifier_has_credit_card", []*cel.Type{cel.DynType}, cel.BoolType,
241+
cel.UnaryBinding(func(args ref.Val) ref.Val {
242+
if m, ok := args.(traits.Mapper); ok {
243+
return types.Bool(m.Contains(types.String("cc")) == types.True)
244+
}
245+
return types.False
246+
}))),
247+
cel.Function("classifier.has_email_or_phone",
248+
cel.Overload("classifier_has_email_or_phone", []*cel.Type{cel.DynType}, cel.BoolType,
249+
cel.UnaryBinding(func(args ref.Val) ref.Val {
250+
if m, ok := args.(traits.Mapper); ok {
251+
return types.Bool(m.Contains(types.String("email")) == types.True || m.Contains(types.String("phone")) == types.True)
252+
}
253+
return types.False
254+
}))),
255+
},
256+
composed: `cel.@block([tool.is_mutation && request.env == "prod", tool.is_mutation ? ["REQUIRE_PEER_CONFIRMATION"] : [], classifier.has_email_or_phone(tool.call.args) ? ["REDACT_PII"] : [], tool.call.args.batch_size > 10000, tool.call.args.batch_size > 1000, tool.call.args.batch_size > 100, request.is_emergency ? ["REQUIRE_VP_APPROVAL"] : (@index0 ? ["REQUIRE_TECH_LEAD_2FA"] : @index1)], @index6 + ((classifier.has_credit_card(tool.call.args) ? ["REDACT_PCI"] : @index2) + (@index3 ? ["THROTTLE_TIER_3"] : (@index4 ? ["THROTTLE_TIER_2"] : (@index5 ? ["THROTTLE_TIER_1"] : [])))))`,
257+
outputType: cel.ListType(cel.StringType),
258+
},
234259
}
235260

236261
policyErrorTests = []struct {

‎policy/parser.go‎

Lines changed: 3 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ import (
2525
"github.com/google/cel-go/common/ast"
2626
)
2727

28+
// SemanticType describes the evaluation semantic for a given policy block.
2829
type SemanticType int
2930

3031
const (
@@ -853,7 +854,7 @@ func (p *parserImpl) ParseVariable(ctx ParserContext, policy *Policy, node *yaml
853854
return p.parseVariableObject(ctx, policy, v, node)
854855
}
855856

856-
func (p *parserImpl) parseVariableInline(ctx ParserContext, policy *Policy, v *Variable, node *yaml.Node) *Variable {
857+
func (p *parserImpl) parseVariableInline(ctx ParserContext, _ *Policy, v *Variable, node *yaml.Node) *Variable {
857858
iterations := 0
858859
p.RangeMap(node, func(key, val *yaml.Node) bool {
859860
keyVal := ctx.NewString(key)
@@ -906,26 +907,14 @@ func (p *parserImpl) parseMatchInternal(ctx ParserContext, policy *Policy, r *Ru
906907
if p.assertYAMLType(id, node, yamlMap) == nil || !p.checkMapValid(ctx, id, node) {
907908
return m
908909
}
909-
ruleSem := firstMatch
910-
if r != nil {
911-
ruleSem = r.Semantic()
912-
} else {
913-
ruleSem = policy.Semantic()
914-
}
915-
isAggregate := ruleSem == aggregate
916910
m.SetCondition(ValueString{ID: ctx.NextID(), Value: "true"})
917911
p.RangeMap(node, func(key, val *yaml.Node) bool {
918912
keyID := ctx.CollectMetadata(key)
919913
fieldName := key.Value
920914
switch fieldName {
921915
case "condition":
922916
m.SetCondition(ctx.NewString(val))
923-
case "output", "emit":
924-
if fieldName == "output" && isAggregate {
925-
p.ReportErrorAtID(keyID, "Rule aggregate requires 'emit' tag instead of 'output'")
926-
} else if fieldName == "emit" && !isAggregate {
927-
p.ReportErrorAtID(keyID, "Rule match requires 'output' tag instead of 'emit'")
928-
}
917+
case "output":
929918
if m.HasRule() {
930919
p.ReportErrorAtID(keyID, "only the rule or the output may be set")
931920
}

‎policy/parser_test.go‎

Lines changed: 3 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -147,33 +147,13 @@ rule:
147147
},
148148
{
149149
txt: `
150-
rule:
151-
aggregate:
152-
- condition: "true"
153-
output: "'foo'"`,
154-
err: `ERROR: <input>:5:7: Rule aggregate requires 'emit' tag instead of 'output'
155-
| output: "'foo'"
156-
| ......^`,
157-
},
158-
{
159-
txt: `
160-
rule:
161-
match:
162-
- condition: "true"
163-
emit: "'foo'"`,
164-
err: `ERROR: <input>:5:7: Rule match requires 'output' tag instead of 'emit'
165-
| emit: "'foo'"
166-
| ......^`,
167-
},
168-
{
169-
txt: `
170150
rule:
171151
match:
172152
- condition: "true"
173153
output: "'foo'"
174154
aggregate:
175155
- condition: "true"
176-
emit: "'bar'"`,
156+
output: "'bar'"`,
177157
err: `ERROR: <input>:6:3: Only one of 'match' or 'aggregate' may be set in a rule
178158
| aggregate:
179159
| ..^`,
@@ -257,7 +237,7 @@ rule:
257237
match:
258238
- output: 'true'
259239
aggregate:
260-
- emit: 'true'`,
240+
- output: 'true'`,
261241
err: `ERROR: <input>:6:3: Only one of 'match' or 'aggregate' may be set in a rule
262242
| aggregate:
263243
| ..^`,
@@ -267,33 +247,13 @@ rule:
267247
name: test
268248
rule:
269249
aggregate:
270-
- emit: 'true'
250+
- output: 'true'
271251
match:
272252
- output: 'true'`,
273253
err: `ERROR: <input>:6:3: Only one of 'match' or 'aggregate' may be set in a rule
274254
| match:
275255
| ..^`,
276256
},
277-
{
278-
txt: `
279-
name: test
280-
rule:
281-
aggregate:
282-
- output: 'true'`,
283-
err: `ERROR: <input>:5:7: Rule aggregate requires 'emit' tag instead of 'output'
284-
| - output: 'true'
285-
| ......^`,
286-
},
287-
{
288-
txt: `
289-
name: test
290-
rule:
291-
match:
292-
- emit: 'true'`,
293-
err: `ERROR: <input>:5:7: Rule match requires 'output' tag instead of 'emit'
294-
| - emit: 'true'
295-
| ......^`,
296-
},
297257
}
298258

299259
for _, tst := range tests {

‎policy/testdata/aggregate_nested_mixed_semantics/policy.yaml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,4 +22,4 @@ rule:
2222
rule:
2323
aggregate:
2424
- condition: "true"
25-
emit: "'foo'"
25+
output: "'foo'"

0 commit comments

Comments
 (0)