Skip to content

DGS-24222 Add support for inline validation rules - #522

Open
Robert Yokota (rayokota) wants to merge 19 commits into
masterfrom
add-cel-inline
Open

DGS-24222 Add support for inline validation rules#522
Robert Yokota (rayokota) wants to merge 19 commits into
masterfrom
add-cel-inline

Conversation

@rayokota

Copy link
Copy Markdown
Member

What

Ports inline validation rules from the JVM client
(schema-registry#4291) to the
TypeScript Schema Registry client. Schemas can now carry CHECK-style constraints that are
evaluated at serialize time.

  • For Avro/JSON Schema, rules live in the confluent:rules property on records/objects and
    their fields; for Protobuf, in the existing Meta extension on messages and fields.
  • New validateAvroMessage / validateJsonMessage / validateProtobufMessage, each
    mirroring the transform walker already in that serde module. Every rule encountered is
    evaluated and all violations are collected (no fail-fast by default), each with a
    dotted path (addr.zip, tags[3], scores["foo"]).
  • New CelValidator (rules/cel/cel-validator.ts) evaluates a rule with this and now
    bound; a rule must return a boolean (false = failed) or a string (non-empty = failed,
    with that string as the failure message).
  • Failures are aggregated into a single SerializationError listing every violation, with
    text identical to the JVM client's.
  • New SerializerConfig fields:
    • validationRulesExecutionDISABLED (default), BEFORE_DOMAIN_RULES, AFTER_DOMAIN_RULES
    • validationRulesFailFast — stop at the first violation (default false)
    • validationRuleExecutor — override the executor (defaults to CelValidator)

Checklist

  • [Y] Contains customer facing changes? Including API/behavior changes
  • [Y] Did you add sufficient unit test and/or integration test coverage for this PR?
    • If not, please explain why it is not required

References

JIRA:

Test & Review

Open questions / Follow-ups

@rayokota
Robert Yokota (rayokota) requested a review from a team as a code owner August 12, 2026 00:35
Copilot AI lite review requested due to automatic review settings August 12, 2026 00:35
@rayokota
Robert Yokota (rayokota) requested a review from a team as a code owner August 12, 2026 00:35

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR ports “inline validation rules” (CHECK-style constraints embedded in schemas) from the JVM Schema Registry client into the TypeScript client, enabling schema-declared constraints to be evaluated during serialization (with aggregated violations and configurable execution timing).

Changes:

  • Adds inline validation rule data model + executor plumbing to the core serde layer, including new SerializerConfig flags and aggregated SerializationError reporting.
  • Implements per-format walkers (validateAvroMessage, validateJsonMessage, validateProtobufMessage) that traverse messages, evaluate inline rules, and collect violations (optionally fail-fast).
  • Extends CEL support to evaluate protobuf field access correctly by threading a protobuf Registry through rule execution contexts.

Reviewed changes

Copilot reviewed 15 out of 17 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
schemaregistry/serde/serde.ts Adds validation config flags, executor wiring, violation aggregation, and shared validation rule utilities/types.
schemaregistry/serde/avro.ts Extracts inline rules from Avro schema JSON and validates messages by walking avsc types.
schemaregistry/serde/json.ts Adds JSON Schema validation walker for confluent:rules with JVM-compatible paths ($...).
schemaregistry/serde/protobuf.ts Adds protobuf inline-rule walker using confluent.Meta extensions and integrates validation into serializer phases.
schemaregistry/rules/cel/cel-executor.ts Updates CEL env setup for newer @bufbuild/cel and adds registry-aware env selection for protobuf field access.
schemaregistry/index.ts Resolves Rule type name clash via explicit re-exports (Rule vs MetaRule).
schemaregistry/confluent/meta_pb.ts Regenerates protobuf TS types to include inline validation rules in Meta and new Rule message.
proto/confluent/meta.proto Adds repeated Rule rules to Meta and defines the Rule message in the proto.
schemaregistry/package.json Bumps @bufbuild/cel dependency to ^0.6.0.
package-lock.json Lockfile updates for the CEL bump and transitive deps.
schemaregistry/test/serde/validation-serdes.spec.ts Serializer-level integration tests for validation execution modes, fail-fast, and custom executor.
schemaregistry/test/serde/validate-message.spec.ts Walker-level tests verifying recursion/pathing and rule dispatch for Avro/JSON/Protobuf.
schemaregistry/test/rules/cel/cel-validator.spec.ts Unit tests for CEL validator semantics (bool/string results, now, error surfacing, caching).
schemaregistry/test/serde/test/validation_widget_pb.ts Generated protobuf TS descriptors used by tests.
proto/test/schemaregistry/serde/validation_widget.proto Test protobuf schema defining inline rule annotations in confluent.Meta.
schemaregistry/test/serde/protobuf.spec.ts Adds test ensuring CEL domain rules can read protobuf message fields when registry is available.
Suppressed comments (1)

schemaregistry/serde/avro.ts:601

  • Similarly, the Avro record case treats arrays as valid "object" records. Guarding against Array values avoids walking/validating with incorrect field lookups when the message shape is wrong.
      const recordSchema = schema as RecordType
      if (typeof msg !== 'object') {
        return
      }

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread schemaregistry/serde/serde.ts
Comment thread schemaregistry/serde/serde.ts
Comment thread schemaregistry/serde/avro.ts

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 15 out of 17 changed files in this pull request and generated 1 comment.

Suppressed comments (4)

schemaregistry/serde/serde.ts:571

  • Caching the default executor by assigning it to this.config() mutates the caller-owned configuration object. A valid frozen config (Object.freeze(...)) will throw here on the first validation-enabled serialization, and reusing one config object also unintentionally shares executor state across serializers. Cache the lazily created executor in a private Serializer field instead.
      this.config().validationRuleExecutor = executor

schemaregistry/package.json:36

  • @bufbuild/cel@0.6.0 declares a peer dependency on @bufbuild/protobuf ^2.6.2, while this package still advertises ^2.0.0. Consumers are therefore allowed to resolve protobuf 2.0–2.6.1 even though the new CEL dependency does not support it, leading to peer-resolution failures or an unsupported runtime combination. Raise the protobuf minimum to ^2.6.2 and regenerate the lockfile.
    "@bufbuild/cel": "^0.6.0",

schemaregistry/serde/protobuf.ts:213

  • This fallback can silently validate against the caller's descriptor instead of the registered schema. In particular, toMessageDescFromName only scans top-level fd.messages, so serializing a nested message with useLatestVersion throws here and then skips any rules that exist only in the registered descriptor. Resolve nested message descriptors and propagate schema parsing/lookup failures rather than bypassing configured validation.
      const fileDesc = await this.toFileDesc(this.client, info)
      desc = this.toMessageDescFromName(fileDesc, messageDesc.typeName)
    } catch (e) {
      // Leave desc as the caller's descriptor.
    }

schemaregistry/serde/avro.ts:487

  • A nested Avro record may declare a fully qualified name such as com.other.Inner; Avro treats that as the fullname and ignores the enclosing namespace. This prefixing stores its rules under parent.com.other.Inner while recordSchema.name remains com.other.Inner, causing all rules on that record to be skipped.
        if (recordNs !== '' && !recordName.startsWith(recordNs)) {
          recordName = recordNs + '.' + recordName

Comment thread schemaregistry/serde/json.ts
@sonarqube-confluent

Copy link
Copy Markdown

Quality Gate failed Quality Gate failed

Failed conditions
79.9% Coverage on New Code (required ≥ 80%)

See analysis details on SonarQube

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 15 out of 17 changed files in this pull request and generated 1 comment.

Suppressed comments (4)

schemaregistry/serde/protobuf.ts:843

  • Message-valued field rules receive a DescField, but CelValidator.envFor() only creates a protobuf registry for a DescMessage. A rule such as this.x > 0 on a singular/repeated/map message field therefore uses the registry-less CEL environment and fails with “not found in registry.” Pass the child message descriptor for message-valued fields (and the containing descriptor for other fields).
      await evaluateValidationRule(executor, rule, fd, value, childPath, out)

schemaregistry/serde/avro.ts:485

  • This prefix check drops validation rules when an unqualified record name begins with its namespace (for example namespace test, name testPerson). avsc names that record test.testPerson, while this map stores testPerson, so neither record nor field rules are found during the walk. Detect a fully qualified name by the presence of a dot instead.
        if (recordNs !== '' && !recordName.startsWith(recordNs)) {

schemaregistry/serde/json.ts:137

  • The referenced JVM implementation performs structural JSON validation before AFTER_DOMAIN_RULES inline validation. Here CEL runs first, so a transformed value that violates its JSON Schema can produce a CEL violation/evaluation error instead of the source client's Invalid message error. Move this block below the existing structural-validation block to preserve the ported behavior.
    if (this.validationEnabled(ValidationRulesExecution.AFTER_DOMAIN_RULES)) {
      await this.validateInlineRules(info, msg)
    }

schemaregistry/index.ts:26

  • The public barrel still omits CelValidator, even though this PR exposes it as the default executor and the other CEL executors are exported at schemaregistry/index.ts:3-4. Consumers otherwise have to depend on an internal deep path to construct the documented executor. Re-export it from the package root.
export type { Rule } from './schemaregistry-client'
export type { Rule as MetaRule } from './confluent/meta_pb'

Comment thread schemaregistry/serde/protobuf.ts Outdated

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 15 out of 17 changed files in this pull request and generated 1 comment.

Suppressed comments (1)

schemaregistry/serde/protobuf.ts:863

  • Field-level rules on message-valued fields receive the DescField as their schema hint. CelValidator.envFor() only creates a protobuf-aware environment for a DescMessage, so expressions such as this.x > 0 on a nested-message field fail with a registry lookup error; repeated/map message fields have the same problem. Pass fd.message for message-bearing fields, as is already done when recursing below.
    for (const rule of getFieldValidationRules(fd)) {
      await evaluateValidationRule(executor, rule, fd, value, childPath, out)

Comment thread schemaregistry/serde/avro.ts Outdated

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 15 out of 17 changed files in this pull request and generated 2 comments.

Suppressed comments (2)

schemaregistry/rules/cel/cel-executor.ts:76

  • This program is now bound to ctx.registry, but the cache key above contains only the expression, schema type, and root schema text. CelExecutor is globally shared, so two serdes using the same root protobuf schema with different referenced-schema versions/registries reuse the first registry's plan; field lookup can then use stale dependency descriptors or fail. Partition the program cache by registry identity (for example, a WeakMap<Registry, LRUCache<...>>), rather than assuming the root schema string uniquely determines the registry.
      program = plan(this.envFor(ctx.registry), parsedExpr)
      this.cache.set(ruleJson, program)

schemaregistry/index.ts:22

  • CelValidator is a new user-facing executor (and is used explicitly through validationRuleExecutor in this PR), but it is not exported from the package barrel. Consumers importing from @confluentinc/schemaregistry cannot construct it, unlike the other CEL executors exported above. Export it here as part of the public API.
// `Rule` is exported both by schemaregistry-client (the data contract rule) and by

Comment thread schemaregistry/serde/protobuf.ts Outdated
Comment thread schemaregistry/serde/protobuf.ts

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 15 out of 17 changed files in this pull request and generated 1 comment.

Suppressed comments (2)

schemaregistry/index.ts:26

  • CelValidator is advertised as the default/override executor, but it is not exported from the package entry point. Since the package only publishes dist/index.js and dist/index.d.ts (schemaregistry/package.json:5-10), consumers cannot import this new class (the tests only work via source-relative imports). Export it here so the public API matches the new configuration surface.
export type { Rule as MetaRule } from './confluent/meta_pb'

schemaregistry/serde/avro.ts:609

  • A field rule receives the wrapped union object (for example { DemoSchema: value }) before recursion unwraps it, so expressions written for the JVM behavior such as this.x > 0 fail on valid wrapped-union values. The existing transform path resolves the union first, and the referenced JVM implementation binds the concrete field value. Resolve field.type with resolveUnion and pass its submsg to the field-rule executor while retaining the declared field schema as the schema argument.
        if (value != null) {
          for (const rule of rules.fieldRules.get(`${recordName}.${field.name}`) ?? []) {
            await evaluateValidationRule(executor, rule, field.type, value, childPath, out)

Comment thread schemaregistry/rules/cel/cel-executor.ts
@rayokota
Robert Yokota (rayokota) requested a balanced review from Copilot August 13, 2026 04:10

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 18 out of 20 changed files in this pull request and generated 1 comment.

Suppressed comments (2)

schemaregistry/serde/protobuf.ts:1003

  • Oneof membership is part of the JavaScript message shape, but this comparison ignores it. If the registered schema moves a field into/out of a oneof (or renames the oneof), needsSchemaView() returns false and message-level rules receive the runtime object; the schema descriptor then looks under a different oneof container, so this.<field> observes the default/absent value. Treat a oneof-container mismatch as requiring a schema view.
    if (schemaFd.name !== runtimeFd.name || schemaFd.fieldKind !== runtimeFd.fieldKind) {
      return false
    }

schemaregistry/index.ts:26

  • The package publishes only dist/index, but the new CelValidator is not re-exported here. Package consumers therefore cannot import the executor introduced by this API (unlike CelExecutor and CelFieldExecutor), including when explicitly supplying the default through validationRuleExecutor. Export the validator from the public entry point.
export type { Rule as MetaRule } from './confluent/meta_pb'

Comment thread schemaregistry/serde/protobuf.ts

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 19 out of 20 changed files in this pull request and generated 1 comment.

Suppressed comments (1)

schemaregistry/serde/protobuf.ts:1095

  • When useLatestVersion selects a compatible schema that adds a field, that field has no entry in runtimeDescriptor.fields, so this loop never evaluates its field-level rules. This happens even though schemaMsg was explicitly created with the registered schema's default/empty value, allowing a newly added repeated or implicit-presence scalar field to pass unchecked. Drive schema-only fields from descriptor.fields/schemaMsg as well, while retaining runtime presence checks for fields known to the producer.
  for (const runtimeFd of runtimeDescriptor.fields) {
    const fd = schemaFieldFor(descriptor, runtimeDescriptor, runtimeFd)

Comment thread schemaregistry/serde/protobuf.ts

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 19 out of 20 changed files in this pull request and generated no new comments.

Suppressed comments (4)

schemaregistry/rules/cel/cel-validator.ts:129

  • For protobuf maps with numeric or boolean keys, Object.entries() leaves every key as a string. The CEL runtime converts that plain object to a string-keyed map, so expressions such as this[1] cannot find an entry stored under the JavaScript key "1". Adapt keys according to fd.mapKey into a CEL-compatible Map, as well as adapting scalar values.
    case 'map':
      if (fd.mapKind !== 'scalar' || msg == null || typeof msg !== 'object') {
        return msg
      }
      return Object.fromEntries(Object.entries(msg).map(
        ([key, value]) => [key, celFromScalar(fd.scalar, value as ScalarValue)]))

schemaregistry/rules/cel/cel-validator.ts:113

  • Non-Protobuf values bypass type adaptation here. @bufbuild/cel interprets a JavaScript number as CEL double (as this file notes above), so Avro int/long and JSON Schema integer rules get floating-point semantics. For example, this / 2 == 2 incorrectly fails for integer 5, while CEL/JVM integer division yields 2. Use the Avro/JSON schema hint to adapt integer scalars—and integer elements/values in collections—to CEL integers without converting declared float/double fields.

This issue also appears on line 124 of the same file.

function celValue(schema: any, msg: any): any {
  if (schema == null || typeof schema !== 'object' || !('fieldKind' in schema)) {
    // A message-level rule: the schema is a DescMessage, and the fields inside resolve
    // through the registry, which already knows their types.
    return msg

schemaregistry/rules/cel/cel-field-executor.ts:24

  • Repeated scalar fields are transformed element-by-element in protobuf.ts, but their descriptor has fieldKind === 'list', so this helper returns each element unchanged. Consequently repeated int32 values still reach CEL as doubles and repeated uint32/uint64 values use the wrong numeric type, causing arithmetic overload failures. Convert list-scalar elements using the list descriptor's scalar type too.
function celScalarValue(fieldCtx: FieldContext, fieldValue: any): any {
  const fd = fieldCtx.fieldDescriptor as DescField | undefined
  if (fd == null || fd.fieldKind !== 'scalar') {
    return fieldValue
  }
  return celFromScalar(fd.scalar, fieldValue as ScalarValue)

schemaregistry/index.ts:26

  • The new CelValidator is not exported from the package entry point, although the other CEL executors are and the public validation walkers require an executor. Consumers using @confluentinc/schemaregistry therefore cannot access the default validator through the supported top-level API. Re-export it here.
export type { Rule as MetaRule } from './confluent/meta_pb'

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 19 out of 20 changed files in this pull request and generated no new comments.

Suppressed comments (4)

schemaregistry/serde/json.ts:672

  • An object-valued property's rules are evaluated here and then evaluated again when validate() recurses into that property and validateObject() treats the same confluent:rules as object-level rules. A failing rule on a nested object therefore produces two identical violations (and a passing rule executes twice). Evaluate each schema node's rules once, including primitive property nodes, rather than evaluating property rules in the parent and object rules again in the child.
      for (const rule of getInlineValidationRules(propSchema)) {
        await evaluateValidationRule(executor, rule, propSchema, value, fullName, out)

schemaregistry/serde/protobuf.ts:992

  • This equality check misses changes to json_name/localName. Such a wire-compatible change can leave name and the field kind unchanged while moving the value to a different JavaScript property. needsSchemaView() then incorrectly returns false, so a message-level CEL rule built from the registered descriptor reads the runtime object using the wrong local property. Include localName in the comparison.
    if (schemaFd.name !== runtimeFd.name || schemaFd.fieldKind !== runtimeFd.fieldKind) {
      return false

schemaregistry/index.ts:26

  • CelValidator is not exported from the package entry point, although it is a new public executor and the existing CEL executors are root exports. Since the published package exposes dist/index as its only main/types entry point, consumers cannot import the validator through the supported package API. Export it here.
// `Rule` is exported both by schemaregistry-client (the data contract rule) and by
// confluent/meta_pb (the protobuf message carrying inline validation rules). Re-export
// them explicitly so the data contract rule keeps the unqualified name.
export type { Rule } from './schemaregistry-client'
export type { Rule as MetaRule } from './confluent/meta_pb'

schemaregistry/rules/cel/cel-field-executor.ts:23

  • Repeated scalar fields are transformed element-by-element, but their fieldDescriptor remains a list descriptor. This branch therefore returns each element unchanged instead of converting it with the list's declared scalar type, so CEL_FIELD expressions using integer/unsigned overloads still fail for repeated protobuf scalars. Handle fieldKind === 'list' && listKind === 'scalar' the same way cel-validator.ts converts list elements.
  if (fd == null || fd.fieldKind !== 'scalar') {
    return fieldValue
  }
  return celFromScalar(fd.scalar, fieldValue as ScalarValue)

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 19 out of 20 changed files in this pull request and generated no new comments.

Suppressed comments (5)

schemaregistry/serde/avro.ts:610

  • Field-level rules receive the raw Avro field value before union dispatch. For a wrapped union, that value is { branchName: value }, while resolveUnion() explicitly unwraps it for the transform walk. A rule such as this >= 0 will therefore evaluate against an object and fail instead of validating the selected value. Resolve the union and bind this to its submsg (while preserving null skipping) before invoking the executor.
        if (value != null) {
          for (const rule of rules.fieldRules.get(`${recordName}.${field.name}`) ?? []) {
            await evaluateValidationRule(executor, rule, field.type, value, childPath, out)

schemaregistry/serde/protobuf.ts:764

  • Scalar-valued protobuf maps now bypass the field transform entirely. Before this rewrite, a map value reached the leaf path and the field rule could validate/transform the collection; now CEL_FIELD conditions on maps (for example, checking map size) silently do nothing. Preserve leaf processing for scalar maps while only recursively walking message-valued maps.
  if (fd.fieldKind === 'map') {
    if (typeof value !== 'object' || fd.mapKind !== 'message') {
      // A map of scalars has no tags below it to act on, which is also why the validation
      // walk does not descend into one.
      return value

schemaregistry/rules/cel/cel-field-executor.ts:23

  • Repeated scalar fields are transformed element-by-element, but their descriptor has fieldKind === 'list', so this guard returns each element unchanged. Consequently repeated int32 values still enter CEL as doubles and repeated uint64 values as signed ints, defeating the type-preserving conversion added here. Treat scalar lists like scalar fields because this helper receives one list element at a time.
  const fd = fieldCtx.fieldDescriptor as DescField | undefined
  if (fd == null || fd.fieldKind !== 'scalar') {
    return fieldValue
  }
  return celFromScalar(fd.scalar, fieldValue as ScalarValue)

schemaregistry/package.json:37

  • @bufbuild/cel 0.6 requires the peer @bufbuild/protobuf ^2.6.2, but this package still declares ^2.0.0. Consumers pinned to protobuf 2.0–2.6.1 can therefore hit an unsatisfied peer dependency even though this package claims that range is supported. Raise the protobuf minimum to the CEL peer requirement.
    "@bufbuild/cel": "^0.6.0",

schemaregistry/index.ts:22

  • The new public CelValidator is not re-exported from the package entry point, unlike the existing CEL executors. Consumers cannot import the default validation executor from the documented package API or use it explicitly in validationRuleExecutor. Add it to the root exports.
// `Rule` is exported both by schemaregistry-client (the data contract rule) and by

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 19 out of 20 changed files in this pull request and generated no new comments.

Suppressed comments (3)

schemaregistry/rules/cel/cel-validator.ts:113

  • Avro and JSON numeric values bypass schema-aware conversion here, so every JavaScript number is bound as a CEL double. As a result, valid CHECK expressions that rely on the declared integer type behave differently from the JVM client: for example, this / 2 == 2 on an Avro int or JSON Schema integer fails for 5 (or integer-only operators have no overload). The conversion must use the Avro/JSON schema recursively, including numeric fields reached by record/object-level rules, rather than returning the raw value.
function celValue(schema: any, msg: any): any {
  if (schema == null || typeof schema !== 'object' || !('fieldKind' in schema)) {
    // A message-level rule: the schema is a DescMessage, and the fields inside resolve
    // through the registry, which already knows their types.
    return msg

schemaregistry/rules/cel/cel-validator.ts:134

  • Protobuf-es exposes map keys as object-property strings, and this branch only converts map values before returning another plain object. Numeric and boolean map keys therefore remain CEL strings, so a rule such as this[1] on map<int32, ...> cannot find its entry (and may surface an evaluation violation). Build a CEL map whose keys are converted with fd.mapKey as well as converting its values.
    case 'map':
      if (msg == null || typeof msg !== 'object') {
        return msg
      }
      if (fd.mapKind === 'scalar') {

schemaregistry/rules/cel/cel-field-executor.ts:24

  • The protobuf walker invokes this helper once per repeated scalar element, but the attached descriptor still has fieldKind === 'list'. This guard therefore leaves repeated int32/uint* elements as raw JavaScript numbers, recreating the double/signed typing problem this conversion is intended to solve. Handle scalar-list descriptors using their element scalar type too.
function celScalarValue(fieldCtx: FieldContext, fieldValue: any): any {
  const fd = fieldCtx.fieldDescriptor as DescField | undefined
  if (fd == null || fd.fieldKind !== 'scalar') {
    return fieldValue
  }

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 20 out of 21 changed files in this pull request and generated no new comments.

Suppressed comments (2)

schemaregistry/rules/cel/cel-field-executor.ts:23

  • Repeated scalar fields are transformed element-by-element while fieldCtx.fieldDescriptor remains the repeated field descriptor. Because this guard accepts only fieldKind === 'scalar', repeated int32 values still reach CEL as doubles and repeated uint64 values as signed ints, so expressions such as value / 2 or value % 10u fail despite the declared element type. Convert list scalar elements using the descriptor's scalar as well.
  const fd = fieldCtx.fieldDescriptor as DescField | undefined
  if (fd == null || fd.fieldKind !== 'scalar') {
    return fieldValue
  }
  return celFromScalar(fd.scalar, fieldValue as ScalarValue)

schemaregistry/serde/protobuf.ts:770

  • This now bypasses field transforms entirely for scalar/enum protobuf maps. Protobuf-es represents maps as plain objects, so the previous walker reached the leaf branch and invoked the active CEL_FIELD/custom transform on the map value; after this change such rules silently stop running. Keep message-map recursion, but pass non-message maps to transformLeaf so the field's rule and tags are still honored.
  if (fd.fieldKind === 'map') {
    if (typeof value !== 'object' || fd.mapKind !== 'message') {
      // A map of scalars has no tags below it to act on, which is also why the validation
      // walk does not descend into one.
      return value

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants