Skip to content

Commit c83ccd9

Browse files
committed
fix: parenthesize NegateExpr to avoid operator precedence ambiguity
not flow.name == "prod" could be parsed as (not flow.name) == "prod". Using not (flow.name == "prod") is unambiguous.
1 parent 41a4e99 commit c83ccd9

3 files changed

Lines changed: 6 additions & 5 deletions

File tree

internal/policy/expression.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,7 @@ func UnwrapExpr(expr string) string {
7979
}
8080

8181
// NegateExpr prefixes a raw (unwrapped) expression with the not operator.
82+
// Parentheses ensure correct evaluation regardless of operator precedence.
8283
func NegateExpr(raw string) string {
83-
return fmt.Sprintf("not %s", raw)
84+
return fmt.Sprintf("not (%s)", raw)
8485
}

internal/policy/expression_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -95,12 +95,12 @@ func TestUnwrapExpr_AlreadyRaw(t *testing.T) {
9595

9696
func TestNegateExpr(t *testing.T) {
9797
result := NegateExpr(`flow.name == "prod"`)
98-
assert.Equal(t, `not flow.name == "prod"`, result)
98+
assert.Equal(t, `not (flow.name == "prod")`, result)
9999
}
100100

101101
func TestCombineAndNegate(t *testing.T) {
102102
a := UnwrapExpr(FlowNameExpr("prod"))
103103
b := NegateExpr(UnwrapExpr(ArtifactNameMatchExpr("^datadog:.*")))
104104
result := CombineExprs("and", a, b)
105-
assert.Equal(t, `${{ flow.name == "prod" and not matches(artifact.name, "^datadog:.*") }}`, result)
105+
assert.Equal(t, `${{ flow.name == "prod" and not (matches(artifact.name, "^datadog:.*")) }}`, result)
106106
}

internal/policywizard/forms_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -447,7 +447,7 @@ func TestApply_ExprNegate_Yes_NegatesLastPending(t *testing.T) {
447447
m.applyFormValues(formValues{confirm: true})
448448

449449
require.Len(t, m.pendingExprs, 1)
450-
assert.Equal(t, `not flow.name == "prod"`, m.pendingExprs[0])
450+
assert.Equal(t, `not (flow.name == "prod")`, m.pendingExprs[0])
451451
}
452452

453453
func TestApply_ExprNegate_No_LeavesUnchanged(t *testing.T) {
@@ -587,7 +587,7 @@ func TestCombineFlow_NegatedExprWithOr(t *testing.T) {
587587

588588
require.Len(t, m.Policy.Artifacts.TrailCompliance.Exceptions, 1)
589589
assert.Equal(t,
590-
`${{ not flow.name == "staging" or exists(flow) }}`,
590+
`${{ not (flow.name == "staging") or exists(flow) }}`,
591591
m.Policy.Artifacts.TrailCompliance.Exceptions[0].If,
592592
)
593593
}

0 commit comments

Comments
 (0)