[BUG] CelValue runtime path mishandles fixed32/fixed64 and FieldMask field access

[andrewparmet]

Describe the bug

My motivation is to build support for protokt (alternate protobuf codegen + runtime) in protovalidate-java, which uses CEL.

I think I've encountered two independent bugs in the planner runtime; both surface when CEL navigates a com.google.protobuf.Message whose conversion goes through ProtoCelValueConverter:

1. proto fixed32/fixed64 fields resolve as signed int, breaking uint overloads: the CEL spec maps proto fixed32/fixed64 to CEL uint, and the CelValue path via ProtoCelValueConverter.fromProtoMessageFieldToCelValue does not: it has cases for UINT32/UINT64 but not FIXED32/FIXED64, so those fall through to normalizePrimitive and surface as Integer/Long.

2. FieldMask-typed fields can't be navigated as messages: BaseProtoCelValueConverter.fromWellKnownProto converts FieldMask to a comma-separated string of its paths, which is different from cel-go and cel-java's ProtoLiteAdapter.adaptValueToWellKnownProto.

To Reproduce
Check which components this affects:

• parser
• checker
• runtime

Reproducers (in Kotlin to get dependencies inline):

1. @file:Repository("https://repo1.maven.org/maven2")
   @file:DependsOn("dev.cel:cel:0.13.0")
   @file:DependsOn("com.google.protobuf:protobuf-java:4.30.1")
   
   import com.google.protobuf.DescriptorProtos.DescriptorProto
   import com.google.protobuf.DescriptorProtos.FieldDescriptorProto
   import com.google.protobuf.DescriptorProtos.FileDescriptorProto
   import com.google.protobuf.Descriptors.FileDescriptor
   import com.google.protobuf.DynamicMessage
   import dev.cel.bundle.CelFactory
   import dev.cel.common.types.StructTypeReference
   
   // A minimal one-field message: `Msg { fixed32 f = 1; }`, built from a runtime
   // descriptor so the reproducer needs no .proto compilation.
   val fileProto =
       FileDescriptorProto.newBuilder()
           .setName("repro.proto")
           .setSyntax("proto3")
           .addMessageType(
               DescriptorProto.newBuilder()
                   .setName("Msg")
                   .addField(
                       FieldDescriptorProto.newBuilder()
                           .setName("f")
                           .setNumber(1)
                           .setType(FieldDescriptorProto.Type.TYPE_FIXED32)
                           .setLabel(FieldDescriptorProto.Label.LABEL_OPTIONAL)
                   )
           )
           .build()
   
   val fileDescriptor = FileDescriptor.buildFrom(fileProto, arrayOf())
   val msgDescriptor = fileDescriptor.findMessageTypeByName("Msg")
   
   val cel =
       CelFactory.plannerCelBuilder()
           .addMessageTypes(msgDescriptor)
           .addVar("msg", StructTypeReference.create("Msg"))
           .build()
   
   // The checker types a proto fixed32 field as CEL `uint`; `msg.f > 5u` type-checks.
   val ast = cel.compile("msg.f > 5u").ast
   val program = cel.createProgram(ast)
   
   val msg =
       DynamicMessage.newBuilder(msgDescriptor)
           .setField(msgDescriptor.findFieldByNumber(1), 6)
           .build()
   
   val result = program.eval(mapOf("msg" to msg))
   println("msg.f > 5u  ->  $result")
   check(result == true) { "expected true" }
   println("OK")

   Throws dev.cel.runtime.CelEvaluationException: evaluation error at <input>:6: No matching overload for function '_>_'. Overload candidates: greater_uint64.. Potential fix: Handle FIXED32/FIXED64 as unsigned in ProtoCelValueConverter #1073

2. @file:Repository("https://repo1.maven.org/maven2")
   @file:DependsOn("dev.cel:cel:0.13.0")
   @file:DependsOn("com.google.protobuf:protobuf-java:4.30.1")
   
   import com.google.protobuf.DescriptorProtos.DescriptorProto
   import com.google.protobuf.DescriptorProtos.FieldDescriptorProto
   import com.google.protobuf.DescriptorProtos.FileDescriptorProto
   import com.google.protobuf.Descriptors.FileDescriptor
   import com.google.protobuf.DynamicMessage
   import com.google.protobuf.FieldMask
   import dev.cel.bundle.CelFactory
   import dev.cel.common.types.StructTypeReference
   
   // A minimal one-field message: `Msg { google.protobuf.FieldMask m = 1; }`.
   val fileProto =
       FileDescriptorProto.newBuilder()
           .setName("repro.proto")
           .setSyntax("proto3")
           .addDependency("google/protobuf/field_mask.proto")
           .addMessageType(
               DescriptorProto.newBuilder()
                   .setName("Msg")
                   .addField(
                       FieldDescriptorProto.newBuilder()
                           .setName("m")
                           .setNumber(1)
                           .setType(FieldDescriptorProto.Type.TYPE_MESSAGE)
                           .setTypeName(".google.protobuf.FieldMask")
                           .setLabel(FieldDescriptorProto.Label.LABEL_OPTIONAL)
                   )
           )
           .build()
   
   val fileDescriptor =
       FileDescriptor.buildFrom(fileProto, arrayOf(FieldMask.getDescriptor().file))
   val msgDescriptor = fileDescriptor.findMessageTypeByName("Msg")
   
   val cel =
       CelFactory.plannerCelBuilder()
           .addMessageTypes(msgDescriptor)
           .addVar("msg", StructTypeReference.create("Msg"))
           .build()
   
   // `msg.m.paths` selects the repeated `paths` field of the FieldMask.
   val ast = cel.compile("msg.m.paths").ast
   val program = cel.createProgram(ast)
   
   val msg =
       DynamicMessage.newBuilder(msgDescriptor)
           .setField(
               msgDescriptor.findFieldByNumber(1),
               FieldMask.newBuilder().addPaths("a").addPaths("b").build()
           )
           .build()
   
   val result = program.eval(mapOf("msg" to msg))
   println("msg.m.paths  ->  $result")
   check(result == listOf("a", "b")) { "expected [a, b]" }
   println("OK")

   Throws dev.cel.runtime.CelEvaluationException: evaluation error at <input>:5: Error resolving field 'paths'. Field selections must be performed on messages or maps.. Potential fix: Preserve FieldMask as a message in the CelValue runtime path #1074

Additional context
For the full context see bufbuild/protovalidate-java#202 and open-toast/protokt#402

[l46kok]

Thanks for the detailed report. Both are definitely bugs.

I also appreciate the detailed findings in bufbuild/protovalidate-java#202. One thing I want to note is that we intend on dropping CelValue support for the legacy runtime going forward. So using the planner for CelValue handlings is definitely recommended.