Conversation
praxis-bot
left a comment
There was a problem hiding this comment.
PR Review: feat(routing): emit the picked cluster as a configurable route header
Summary: Clean feature addition. The strip-then-set ordering is correct (removes apply before sets in the pipeline), the anti-spoofing covers all exit paths (skip, cluster-already-set, reject, pick), and HeaderValue::from_str blocks control-character injection. Test coverage is thorough: pick, no-config, spoofed-overwrite, no-pick strip, unencodable cluster, session reuse, and config validation. Two medium findings.
| Severity | Count |
|---|---|
| Medium | 2 |
The filter sets ctx.cluster for a downstream load_balancer filter to read. A gateway that routes by header instead (ext_proc behind Envoy) has no way to learn the pick. Add an optional route_header: when set, the filter emits the chosen cluster as that request header on every pick, including a session-affinity-reused pick, so a header-routing gateway honours the bound cluster. Unset, behavior is unchanged. The header name is validated at parse with the same rule as model_header. Signed-off-by: Sam Batschelet <sbatsche@redhat.com>
… unencodable From praxis-bot review: - When an earlier filter pre-sets ctx.cluster and route_header is configured, the preserve path now emits the route header too, so a header-routing gateway is not left without a destination. A forged client value is still stripped first. - emit_route_header logs an unencodable cluster name at warn rather than debug, so a misconfigured cluster is visible without debug tracing. emit_route_header now takes the cluster name, shared by the pick and preserve paths. Adds a regression test for the pre-set-cluster case. Signed-off-by: Sam Batschelet <sbatsche@redhat.com>
0903788 to
ea8f3d2
Compare
leseb
left a comment
There was a problem hiding this comment.
P2: route_header can overwrite authenticated provider-hop context.
Configuration accepts x-ai-routing-candidate, x-ai-routing-request-id, and x-ai-routing-revision. For a provider-hop candidate, the filter first queues the correct authenticated context, then queues the cluster under the same configured name. Praxis applies queued sets in order, so the cluster wins and the provider receives corrupted routing context. Reject all three AI-owned names. Configuration, provider-context writes, later route-header write.
P2: Praxis-reserved extension headers are accepted and subsequently stripped.
The local validator only reserves x-praxis-* and x-mcp-, while Praxis 0.5.4 also reserves x-ext-protocol- and x-ext-agent-*. Configuring one of those as the destination header passes filter construction but Praxis removes it before upstream dispatch, leaving the header-routing gateway without a destination. AI validator, Praxis contract.
P2: An unencodable cluster still reports successful routing without a destination header.
Praxis Bot noticed this path but requested only a warning-level log. The current fix logs warn! and continues. In the documented header-routing topology, downstream routing reads the header rather than ctx.cluster; the request therefore proceeds without its required destination and may hit a default route or fail later. Validate cluster encodability when route_header is configured, or fail the request. Current behavior.
|
since we are not going to be using v2 for grid I am going to close this |
Summary
Add an optional
route_headerto the intelligent_route filter. When set, the filter emits the chosen cluster as that request header on every pick; when unset, behavior is unchanged and no header is emitted.Motivation
The filter selects a cluster and sets it on the request context, which a downstream
load_balancerfilter reads. A gateway that routes by header instead (ext_proc behind Envoy) has no way to learn the pick, so the routing decision is not carried to it. This adds the decision-carrying header. It is independent of load scoring: it emits whichever candidate any policy picked.What changed
A
route_headerconfig field on the intelligent_route filter, validated at parse with the same rule asmodel_header. On both the fresh-selection and the session-affinity-reused paths, once a candidate is chosen the filter setsroute_headerto that candidate's cluster. A reused pick emits it too, so a header-routing gateway honours the bound cluster rather than falling back. A cluster name that is not a valid header value is logged and skipped, not fatal.The route header is not in the reserved header namespace, so the protocol layer's reserved-header guard does not reject a client that forges it. The filter strips a configured
route_headerfrom the request at entry, alongside the internal routing headers, so only a pick emits it. On the pick path the set wins over the strip; on a non-pick path (no model header, or a cluster already set upstream) the request fails closed with no client-supplied destination reaching the gateway.The filter doc is regenerated for the new field.
Header injection
The route header sits outside the reserved
x-praxis-/x-mcp-namespace, so the protocol layer's reserved-header rejection does not cover it, and the strip is the control. A dedicated security review confirmed injection is prevented on both the header-routing and the metadata-mirror topologies:route_headeraton_requestentry, before every exit: the no-model-header skip, the preset-cluster pass-through, and the reject paths. A non-pick request fails closed with no client value surviving.HeaderValue::from_str(control bytes, CR, LF), so a crafted cluster cannot smuggle a second header.Worked example (Envoy)
The filter change here is only the
route_headeremission. The consuming side is an ext_proc adapter inpicker_modethat mirrors the emitted header intoenvoy.lbdynamic metadata, and an Envoy config that routes on that metadata. Header alone does not steer Envoy (the route and host are chosen before a body-phase header mutation lands), so the value has to reach the load balancer as dynamic metadata.The ext_proc server running the filter:
Envoy accepts the metadata and forwards to the chosen host:
The route sends to the dynamic-forward-proxy cluster and rewrites the host from that header:
Flow: client sends
x-gateway-model-name,intelligent_routepicks a candidate and setsx-gateway-destination-endpointto its cluster FQDN,picker_modemirrors that intoenvoy.lbmetadata,header_mutationcopies it tox-chosen-host, anddynamic_forward_proxyroutes to that FQDN. The same metadata can instead drive anORIGINAL_DSTcluster subset. A request that reaches Envoy without a pick carries no picker-set metadata, and the filter strips any client-suppliedroute_header, so a forged value cannot route.