Skip to content

Commit d59e05a

Browse files
Add cel_expression validation to PROTOVALIDATE lint rule (#4284)
[`cel_expression` was added to protovalidate][2] in [v1.1.0][1]. We ought to validate it in the same way that we validate the `cel` field. [1]: https://github.com/bufbuild/protovalidate/releases/tag/v1.1.0 [2]: bufbuild/protovalidate#432
1 parent 0adc664 commit d59e05a

File tree

5 files changed

+99
-6
lines changed

5 files changed

+99
-6
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
imports, and sort imports alphabetically.
1111
- Add LSP document link support.
1212
- Add LSP folding range support.
13+
- Update `PROTOVALIDATE` lint rule to support checking `cel_expression` fields for valid CEL.
1314

1415
## [v1.63.0] - 2026-01-06
1516

private/bufpkg/bufcheck/bufcheckserver/internal/buflintvalidate/cel.go

Lines changed: 67 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,10 @@ const (
3232
celFieldNumberInFieldRules = 23
3333
// https://buf.build/bufbuild/protovalidate/docs/main:buf.validate#buf.validate.MessageRules
3434
celFieldNumberInMessageRules = 3
35+
// https://buf.build/bufbuild/protovalidate/docs/main:buf.validate#buf.validate.MessageRules
36+
celExpressionFieldNumberInMessageRules = 5
37+
// https://buf.build/bufbuild/protovalidate/docs/main:buf.validate#buf.validate.FieldRules
38+
celExpressionFieldNumberInFieldRules = 29
3539
)
3640

3741
func checkCELForMessage(
@@ -40,6 +44,9 @@ func checkCELForMessage(
4044
messageDescriptor protoreflect.MessageDescriptor,
4145
message bufprotosource.Message,
4246
) error {
47+
if len(messageRules.GetCel()) == 0 && len(messageRules.GetCelExpression()) == 0 {
48+
return nil
49+
}
4350
celEnv, err := cel.NewEnv(
4451
cel.Lib(celpv.NewLibrary()),
4552
)
@@ -67,7 +74,32 @@ func checkCELForMessage(
6774
)
6875
add(message, messageRulesOptionLocation, nil, format, args...)
6976
},
77+
false, // isCELExpression
7078
)
79+
if len(messageRules.GetCelExpression()) > 0 {
80+
celExpressionRules := make([]*validate.Rule, len(messageRules.GetCelExpression()))
81+
for i, expr := range messageRules.GetCelExpression() {
82+
celExpressionRules[i] = &validate.Rule{
83+
Expression: &expr,
84+
}
85+
}
86+
checkCEL(
87+
celEnv,
88+
celExpressionRules,
89+
fmt.Sprintf("message %q", message.Name()),
90+
fmt.Sprintf("Message %q", message.Name()),
91+
"(buf.validate.message).cel_expression",
92+
func(index int, format string, args ...any) {
93+
messageRulesOptionLocation := message.OptionExtensionLocation(
94+
validate.E_Message,
95+
celExpressionFieldNumberInMessageRules,
96+
int32(index),
97+
)
98+
add(message, messageRulesOptionLocation, nil, format, args...)
99+
},
100+
true, // isCELExpression
101+
)
102+
}
71103
return nil
72104
}
73105

@@ -78,7 +110,7 @@ func checkCELForField(
78110
// forItems is true if the CEL rule is defined on a non-repeated field or on each item of a repeated field.
79111
forItems bool,
80112
) error {
81-
if len(fieldRules.GetCel()) == 0 {
113+
if len(fieldRules.GetCel()) == 0 && len(fieldRules.GetCelExpression()) == 0 {
82114
return nil
83115
}
84116
celEnv, err := cel.NewEnv(
@@ -109,7 +141,31 @@ func checkCELForField(
109141
args...,
110142
)
111143
},
144+
false, // isCELExpression
112145
)
146+
if len(fieldRules.GetCelExpression()) > 0 {
147+
celExpressionRules := make([]*validate.Rule, len(fieldRules.GetCelExpression()))
148+
for i, expr := range fieldRules.GetCelExpression() {
149+
celExpressionRules[i] = &validate.Rule{
150+
Expression: &expr,
151+
}
152+
}
153+
checkCEL(
154+
celEnv,
155+
celExpressionRules,
156+
fmt.Sprintf("field %q", adder.fieldName()),
157+
fmt.Sprintf("Field %q", adder.fieldName()),
158+
adder.getFieldRuleName(celExpressionFieldNumberInFieldRules),
159+
func(index int, format string, args ...any) {
160+
adder.addForPathf(
161+
[]int32{celExpressionFieldNumberInFieldRules, int32(index)},
162+
format,
163+
args...,
164+
)
165+
},
166+
true, // isCELExpression
167+
)
168+
}
113169
return nil
114170
}
115171

@@ -121,9 +177,14 @@ func checkCEL(
121177
parentNameCapitalized string,
122178
celName string,
123179
add func(int, string, ...any),
180+
isCELExpression bool,
124181
) bool {
125182
allCelExpressionsCompile := true
126183
idToConstraintIndices := make(map[string][]int, len(celRules))
184+
expressionField := celName + ".expression"
185+
if isCELExpression {
186+
expressionField = celName
187+
}
127188
for i, celConstraint := range celRules {
128189
if celID := celConstraint.GetId(); celID != "" {
129190
for _, char := range celID {
@@ -147,7 +208,7 @@ func checkCEL(
147208
idToConstraintIndices[celID] = append(idToConstraintIndices[celID], i)
148209
}
149210
if len(strings.TrimSpace(celConstraint.GetExpression())) == 0 {
150-
add(i, "%s has an empty %s.expression. Expressions should always be specified.", parentNameCapitalized, celName)
211+
add(i, "%s has an empty %s. Expressions should always be specified.", parentNameCapitalized, expressionField)
151212
continue
152213
}
153214
ast, compileIssues := celEnv.Compile(celConstraint.GetExpression())
@@ -168,8 +229,8 @@ func checkCEL(
168229
default:
169230
add(
170231
i,
171-
"%s.expression on %s evaluates to a %s, only string and boolean are allowed.",
172-
celName,
232+
"%s on %s evaluates to a %s, only string and boolean are allowed.",
233+
expressionField,
173234
parentName,
174235
cel.FormatCELType(ast.OutputType()),
175236
)
@@ -179,8 +240,8 @@ func checkCEL(
179240
for _, parsedIssue := range parseCelIssuesText(compileIssues.Err().Error()) {
180241
add(
181242
i,
182-
"%s.expression on %s fails to compile: %s",
183-
celName,
243+
"%s on %s fails to compile: %s",
244+
expressionField,
184245
parentName,
185246
parsedIssue,
186247
)

private/bufpkg/bufcheck/bufcheckserver/internal/buflintvalidate/predefined_rules.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,7 @@ func checkPredefinedRuleExtension(
114114
args...,
115115
)
116116
},
117+
false,
117118
)
118119
return nil
119120
}

private/bufpkg/bufcheck/lint_test.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -616,6 +616,10 @@ func TestRunProtovalidate(t *testing.T) {
616616
bufanalysistesting.NewFileAnnotation(t, "bytes.proto", 31, 5, 31, 45, "PROTOVALIDATE"),
617617
bufanalysistesting.NewFileAnnotation(t, "bytes.proto", 43, 5, 43, 65, "PROTOVALIDATE"),
618618
bufanalysistesting.NewFileAnnotation(t, "bytes.proto", 46, 45, 46, 106, "PROTOVALIDATE"),
619+
bufanalysistesting.NewFileAnnotation(t, "cel_expression.proto", 11, 37, 11, 85, "PROTOVALIDATE"),
620+
bufanalysistesting.NewFileAnnotation(t, "cel_expression.proto", 14, 38, 14, 84, "PROTOVALIDATE"),
621+
bufanalysistesting.NewFileAnnotation(t, "cel_expression.proto", 19, 5, 19, 53, "PROTOVALIDATE"),
622+
bufanalysistesting.NewFileAnnotation(t, "cel_expression.proto", 25, 3, 25, 65, "PROTOVALIDATE"),
619623
bufanalysistesting.NewFileAnnotation(t, "cel_field.proto", 10, 37, 14, 4, "PROTOVALIDATE"),
620624
bufanalysistesting.NewFileAnnotation(t, "cel_field.proto", 17, 5, 21, 6, "PROTOVALIDATE"),
621625
bufanalysistesting.NewFileAnnotation(t, "cel_field.proto", 29, 5, 33, 6, "PROTOVALIDATE"),
Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
syntax = "proto2";
2+
3+
import "buf/validate/validate.proto";
4+
5+
// Test cel_expression validation
6+
message TestCelExpression {
7+
// Valid expression
8+
optional string valid_field = 1 [(buf.validate.field).cel_expression = "this.size() > 5"];
9+
10+
// Invalid expression - type error
11+
optional string invalid_type = 2 [(buf.validate.field).cel_expression = "this > 5"];
12+
13+
// Invalid expression - syntax error
14+
optional int32 invalid_syntax = 3 [(buf.validate.field).cel_expression = "this +"];
15+
16+
// Multiple expressions, some valid, some invalid
17+
optional string mixed = 4 [
18+
(buf.validate.field).cel_expression = "this.size() > 0",
19+
(buf.validate.field).cel_expression = "this * 2"
20+
];
21+
22+
// Message-level cel_expression
23+
optional string foo = 5;
24+
option (buf.validate.message).cel_expression = "this.foo.size() > 0";
25+
option (buf.validate.message).cel_expression = "this.foo > 5";
26+
}

0 commit comments

Comments
 (0)