Skip to content

feat: validate routing CEL expressions at write time in create/update handlers - #5288

Merged
Pratham-Mishra04 merged 1 commit into
devfrom
07-16-feat_routing_rules_cel_expression_validation_backend
Jul 17, 2026
Merged

feat: validate routing CEL expressions at write time in create/update handlers#5288
Pratham-Mishra04 merged 1 commit into
devfrom
07-16-feat_routing_rules_cel_expression_validation_backend

Conversation

@impoiler

@impoiler impoiler commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Summary

Routing rule CEL expressions were previously only compiled and validated at first evaluation, meaning a malformed expression could be persisted successfully and fail silently at runtime. This PR adds write-time validation so that invalid CEL expressions are rejected with a 400 error when a routing rule is created or updated.

Changes

  • Added ValidateRoutingCELExpression in plugins/governance/routing.go that builds the CEL environment once (via sync.Once) and compiles the normalized expression, returning a descriptive error on any syntax or type problem. Empty/whitespace-only expressions are treated as match-all and pass validation.
  • Wired ValidateRoutingCELExpression into the createRoutingRule and updateRoutingRule HTTP handlers so invalid expressions are rejected at write time. The update handler only validates when CelExpression is explicitly supplied, so unrelated updates (e.g. toggling enabled) are unaffected.
  • Added TestValidateRoutingCELExpression covering valid expressions (empty, whitespace, equality, contains, header lookup, conjunction) and invalid ones (unknown identifiers, syntax errors, type mismatches, dangling operators).

Type of change

  • Bug fix
  • Feature
  • Refactor
  • Documentation
  • Chore/CI

Affected areas

  • Core (Go)
  • Transports (HTTP)
  • Providers/Integrations
  • Plugins
  • UI (React)
  • Docs

How to test

go test ./plugins/governance/... ./transports/bifrost-http/...

To validate manually, attempt to create or update a routing rule with an invalid CEL expression (e.g. model ==) and confirm a 400 response is returned with a descriptive error message. A valid expression (e.g. model == "gpt-4o") should be accepted as before.

Screenshots/Recordings

N/A

Breaking changes

  • Yes
  • No

Related issues

Security considerations

CEL expressions are now compiled before persistence, reducing the risk of storing expressions that could cause unexpected runtime behaviour or panics during evaluation.

Checklist

  • I read docs/contributing/README.md and followed the guidelines
  • I added/updated tests where appropriate
  • I updated documentation where needed
  • I verified builds succeed (Go and UI)
  • I verified the CI pipeline passes locally if applicable

@coderabbitai

coderabbitai Bot commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Warning

Review limit reached

You’ve reached a temporary PR review limit under our Fair Usage Limits Policy.

Your recent review volume is higher than typical usage, so adaptive limits are currently applied.

Next review available in: 54 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 0b6971bc-77ba-439b-9b3d-e55d16e395d7

📥 Commits

Reviewing files that changed from the base of the PR and between 0698f7b and 8b87614.

📒 Files selected for processing (3)
  • plugins/governance/routing.go
  • plugins/governance/routing_test.go
  • transports/bifrost-http/handlers/governance.go
📝 Walkthrough

Walkthrough

Routing rule handlers now validate CEL expressions before creating or updating rules. A cached governance helper normalizes map keys, performs routing schema validation, compiles expressions, and reports invalid input. Tests cover valid, empty, and invalid expressions.

Changes

Routing CEL validation

Layer / File(s) Summary
Routing CEL validation helper
plugins/governance/routing.go, plugins/governance/routing_test.go
Adds cached CEL environment initialization, map-key normalization, schema validation, compilation, and table-driven coverage for valid and invalid expressions.
Routing rule handler integration
transports/bifrost-http/handlers/governance.go
Validates expressions during rule creation and conditional validation during updates, returning HTTP 400 for invalid supplied expressions.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant HTTPClient
  participant createRoutingRule
  participant ValidateRoutingCELExpression
  participant routing.ValidateCELExpression
  HTTPClient->>createRoutingRule: Submit cel_expression
  createRoutingRule->>ValidateRoutingCELExpression: Validate expression
  ValidateRoutingCELExpression->>routing.ValidateCELExpression: Validate normalized expression
  routing.ValidateCELExpression-->>ValidateRoutingCELExpression: Return validation result
  ValidateRoutingCELExpression-->>createRoutingRule: Return error or success
  createRoutingRule-->>HTTPClient: Return HTTP 400 or create rule
Loading

Suggested reviewers: akshaydeo, pratham-mishra04

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly matches the main change: validating routing CEL expressions at write time in create/update handlers.
Description check ✅ Passed The description follows the template well, covering summary, changes, testing, impacted areas, security, and breaking changes.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch 07-16-feat_routing_rules_cel_expression_validation_backend

Comment @coderabbitai help to get the list of available commands.

impoiler commented Jul 16, 2026

Copy link
Copy Markdown
Contributor Author

@impoiler impoiler self-assigned this Jul 16, 2026
@impoiler impoiler changed the title feat: routing rules CEL expression validation backend feat: validate routing CEL expressions at write time in create/update handlers Jul 16, 2026
@impoiler
impoiler changed the base branch from 07-16-fix_merge_responses_and_chat_completion_mode_while_calculating_the_cost to graphite-base/5288 July 16, 2026 08:38
@impoiler
impoiler force-pushed the 07-16-feat_routing_rules_cel_expression_validation_backend branch from 11b071c to 0698f7b Compare July 16, 2026 08:38
@impoiler
impoiler force-pushed the graphite-base/5288 branch from a4770ad to c0909f9 Compare July 16, 2026 08:38
@impoiler
impoiler changed the base branch from graphite-base/5288 to dev July 16, 2026 08:38
@impoiler
impoiler marked this pull request as ready for review July 16, 2026 08:40
@greptile-apps

greptile-apps Bot commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Confidence Score: 5/5

This looks safe to merge.

  • No blocking issues found in the changed code.

Important Files Changed

Filename Overview
plugins/governance/routing.go Adds reusable write-time CEL validation using the runtime routing environment and normalization logic.
plugins/governance/routing_test.go Adds table-driven tests for accepted and rejected CEL expressions.
transports/bifrost-http/handlers/governance.go Validates supplied CEL expressions before routing rules are created or updated.

Reviews (2): Last reviewed commit: "feat: routing rules CEL expression valid..." | Re-trigger Greptile

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
transports/bifrost-http/handlers/governance.go (1)

4007-4011: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Add handler-level regression tests for the 400 contract.

The helper tests cover compilation, but not that create/update handlers return 400 and skip persistence when cel_expression is invalid. Add focused tests for both paths, including an update where the field is omitted.

Also applies to: 4124-4129

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@transports/bifrost-http/handlers/governance.go` around lines 4007 - 4011, Add
focused regression tests for the create and update governance handlers around
their ValidateRoutingCELExpression calls, asserting malformed cel_expression
returns HTTP 400 and does not persist changes. Include an update case with
cel_expression omitted to confirm it remains valid and follows the existing
update behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@transports/bifrost-http/handlers/governance.go`:
- Around line 4007-4011: Add focused regression tests for the create and update
governance handlers around their ValidateRoutingCELExpression calls, asserting
malformed cel_expression returns HTTP 400 and does not persist changes. Include
an update case with cel_expression omitted to confirm it remains valid and
follows the existing update behavior.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: a4a8cb09-e54b-4dad-820e-a39952573b6e

📥 Commits

Reviewing files that changed from the base of the PR and between c0909f9 and 0698f7b.

📒 Files selected for processing (3)
  • plugins/governance/routing.go
  • plugins/governance/routing_test.go
  • transports/bifrost-http/handlers/governance.go

Pratham-Mishra04 commented Jul 17, 2026

Copy link
Copy Markdown
Collaborator

Merge activity

  • Jul 17, 11:09 AM UTC: A user started a stack merge that includes this pull request via Graphite.
  • Jul 17, 11:10 AM UTC: Graphite rebased this pull request as part of a merge.
  • Jul 17, 11:11 AM UTC: Graphite rebased this pull request as part of a merge.
  • Jul 17, 11:11 AM UTC: @Pratham-Mishra04 merged this pull request with Graphite.

@Pratham-Mishra04
Pratham-Mishra04 force-pushed the 07-16-feat_routing_rules_cel_expression_validation_backend branch from 0698f7b to e36d64e Compare July 17, 2026 11:09
@Pratham-Mishra04
Pratham-Mishra04 force-pushed the 07-16-feat_routing_rules_cel_expression_validation_backend branch from e36d64e to 8b87614 Compare July 17, 2026 11:10
@Pratham-Mishra04
Pratham-Mishra04 merged commit 761f1b6 into dev Jul 17, 2026
14 of 15 checks passed
@Pratham-Mishra04
Pratham-Mishra04 deleted the 07-16-feat_routing_rules_cel_expression_validation_backend branch July 17, 2026 11:11
akshaydeo pushed a commit that referenced this pull request Jul 17, 2026
… handlers (#5288)

## Summary

Routing rule CEL expressions were previously only compiled and validated at first evaluation, meaning a malformed expression could be persisted successfully and fail silently at runtime. This PR adds write-time validation so that invalid CEL expressions are rejected with a `400` error when a routing rule is created or updated.

## Changes

- Added `ValidateRoutingCELExpression` in `plugins/governance/routing.go` that builds the CEL environment once (via `sync.Once`) and compiles the normalized expression, returning a descriptive error on any syntax or type problem. Empty/whitespace-only expressions are treated as match-all and pass validation.
- Wired `ValidateRoutingCELExpression` into the `createRoutingRule` and `updateRoutingRule` HTTP handlers so invalid expressions are rejected at write time. The update handler only validates when `CelExpression` is explicitly supplied, so unrelated updates (e.g. toggling `enabled`) are unaffected.
- Added `TestValidateRoutingCELExpression` covering valid expressions (empty, whitespace, equality, `contains`, header lookup, conjunction) and invalid ones (unknown identifiers, syntax errors, type mismatches, dangling operators).

## Type of change

- [ ] Bug fix
- [x] Feature
- [ ] Refactor
- [ ] Documentation
- [ ] Chore/CI

## Affected areas

- [ ] Core (Go)
- [x] Transports (HTTP)
- [ ] Providers/Integrations
- [x] Plugins
- [ ] UI (React)
- [ ] Docs

## How to test

```sh
go test ./plugins/governance/... ./transports/bifrost-http/...
```

To validate manually, attempt to create or update a routing rule with an invalid CEL expression (e.g. `model ==`) and confirm a `400` response is returned with a descriptive error message. A valid expression (e.g. `model == "gpt-4o"`) should be accepted as before.

## Screenshots/Recordings

N/A

## Breaking changes

- [ ] Yes
- [x] No

## Related issues

## Security considerations

CEL expressions are now compiled before persistence, reducing the risk of storing expressions that could cause unexpected runtime behaviour or panics during evaluation.

## Checklist

- [ ] I read `docs/contributing/README.md` and followed the guidelines
- [x] I added/updated tests where appropriate
- [ ] I updated documentation where needed
- [x] I verified builds succeed (Go and UI)
- [ ] I verified the CI pipeline passes locally if applicable
akshaydeo pushed a commit that referenced this pull request Jul 18, 2026
… handlers (#5288)

## Summary

Routing rule CEL expressions were previously only compiled and validated at first evaluation, meaning a malformed expression could be persisted successfully and fail silently at runtime. This PR adds write-time validation so that invalid CEL expressions are rejected with a `400` error when a routing rule is created or updated.

## Changes

- Added `ValidateRoutingCELExpression` in `plugins/governance/routing.go` that builds the CEL environment once (via `sync.Once`) and compiles the normalized expression, returning a descriptive error on any syntax or type problem. Empty/whitespace-only expressions are treated as match-all and pass validation.
- Wired `ValidateRoutingCELExpression` into the `createRoutingRule` and `updateRoutingRule` HTTP handlers so invalid expressions are rejected at write time. The update handler only validates when `CelExpression` is explicitly supplied, so unrelated updates (e.g. toggling `enabled`) are unaffected.
- Added `TestValidateRoutingCELExpression` covering valid expressions (empty, whitespace, equality, `contains`, header lookup, conjunction) and invalid ones (unknown identifiers, syntax errors, type mismatches, dangling operators).

## Type of change

- [ ] Bug fix
- [x] Feature
- [ ] Refactor
- [ ] Documentation
- [ ] Chore/CI

## Affected areas

- [ ] Core (Go)
- [x] Transports (HTTP)
- [ ] Providers/Integrations
- [x] Plugins
- [ ] UI (React)
- [ ] Docs

## How to test

```sh
go test ./plugins/governance/... ./transports/bifrost-http/...
```

To validate manually, attempt to create or update a routing rule with an invalid CEL expression (e.g. `model ==`) and confirm a `400` response is returned with a descriptive error message. A valid expression (e.g. `model == "gpt-4o"`) should be accepted as before.

## Screenshots/Recordings

N/A

## Breaking changes

- [ ] Yes
- [x] No

## Related issues

## Security considerations

CEL expressions are now compiled before persistence, reducing the risk of storing expressions that could cause unexpected runtime behaviour or panics during evaluation.

## Checklist

- [ ] I read `docs/contributing/README.md` and followed the guidelines
- [x] I added/updated tests where appropriate
- [ ] I updated documentation where needed
- [x] I verified builds succeed (Go and UI)
- [ ] I verified the CI pipeline passes locally if applicable
@coderabbitai coderabbitai Bot mentioned this pull request Jul 30, 2026
18 tasks
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