feat(openai): add a request-head operation classifier - #890
Conversation
3595136 to
cdd1fbb
Compare
cdd1fbb to
d8dcf7c
Compare
Operation identity was derived per filter from path string matching, so every consumer re-implemented the same method and path checks and no single component owned the answer. Add `openai_operation`, which classifies supported operations from the request head alone and publishes the result four ways: a typed `OpenAiOperationMatch` in request extensions, `openai_operation.family` and `openai_operation.operation_id` metadata, matching filter results, and configurable proxy-owned routing headers. The filter takes the default `BodyAccess::None` and `BodyMode::Stream`, so it declares no body access and no buffering. Transport is read from the opening handshake headers, which is what separates the Responses WebSocket operation from `POST /v1/responses` at the same path without inspecting a payload. Routing headers are overwritten on a match and removed when nothing matches, so a client-supplied value cannot survive. Unmatched requests are otherwise left unchanged; whether they are rejected, forwarded to a fallback, or handled some other way stays a routing policy decision. Note on consuming the classification: `x-praxis-ai-*` is a reserved prefix, so the protocol layer rejects client requests carrying one and strips them before forwarding. Header-phase mutations are also still pending while the `router` filter matches the downstream request, so routing on these headers is not available in the same phase. Downstream filters read the typed match from request extensions, and pipelines branch on the published filter results. Registry-driven routing is tracked separately. Conversations exposes its route module to the crate so the classifier can consult both registries through one entry point. Closes praxis-proxy#744 Signed-off-by: Charlie Doern <cdoern@redhat.com>
d8dcf7c to
f4331dc
Compare
praxis-bot
left a comment
There was a problem hiding this comment.
praxis-bot review: feat(openai): add a request-head operation classifier
Clean, well-structured filter with strong security properties and thorough test coverage. The classification logic is correct, the anti-spoofing headers are properly handled (overwrite on match, remove on miss), and the WebSocket transport detection follows the RFC precisely. 15 unit tests and 6 functional integration tests cover the published contract end to end.
One medium finding on the generated filter documentation.
| Severity | Findings |
|---|---|
| Critical | 0 |
| Large | 0 |
| Medium | 1 |
Addresses review feedback on praxis-proxy#890. `generate-filter-docs` takes the first `pub` type it encounters in the module, and `OpenAiOperationMatch` was declared before `OpenaiOperationFilter`. The generated page therefore described the match type rather than the filter, and its configuration notes carried implementation details about `'static` fields and borrowed path parameters that are irrelevant to configuring the filter. The reference table inherited the same description. Declare the filter and its inherent impl before the match type it publishes, so the generator reads the intended doc comment. The generated page and reference entry are regenerated. Signed-off-by: Charlie Doern <cdoern@redhat.com>
leseb
left a comment
There was a problem hiding this comment.
P2 — Unsupported upgrade headers suppress HTTP classification. mod.rs:118 selects WebSocket transport without requiring GET. Adding upgrade headers to a normal operation such as POST /v1/responses leaves it unclassified. This is currently latent because WebSocket and classifier-based policy routing are not supported yet, but should be fixed before consumers land.
P2 — Unsafe header targets are accepted. config.rs:87 permits targets such as authorization, host, and content-length, potentially overwriting or removing authentication and transport state. Use the repository’s shared promotion-header validation.
P2 — Duplicate output headers are accepted. The same validation permits family and operation to use the same header name. The operation value then overwrites the family value. Reject duplicates case-insensitively.
P2 — Functional tests do not observe classification. operation_classifier.rs:47 tests forwarding and core reserved-header handling, but never consumes the classifier result. The suite would pass with a no-op classifier. Route distinct operations through on_result to distinct backends and assert the selected backend.
| pub(crate) struct OperationHeaders { | ||
| /// Header name for the API family. `null` disables the header. | ||
| #[serde(default = "default_family_header")] | ||
| pub family: Option<String>, |
There was a problem hiding this comment.
let's use application_protocol instead so this matches praxis-proxy/praxis#1106 - sorry the change, just realized now this would be better
Closes #744
Stacked on #788
The first commit,
e209fa8f, is the #743 Responses registry under review in #788.It is included because the classifier consults that registry. Merge #788 first;
this branch then contains only its own commit. Review just
35951360here.Summary
Operation identity was derived per filter from path string matching, so every
consumer re-implemented the same method and path checks and no single component
owned the answer.
openai_operationclassifies supported operations from the request head aloneand publishes the result four ways:
OpenAiOperationMatchin request extensionsopenai_operation.family,openai_operation.operation_idmetadatafamily,operation_idon_resultbranch conditionsx-praxis-ai-family,x-praxis-ai-operationheadersThe filter takes the default
BodyAccess::NoneandBodyMode::Stream, so itdeclares no body access and no buffering — the classification is available
before any body-handling decision is made.
Transport comes from the opening handshake headers (RFC 6455 §4.1, with
Connectiontreated as a token list per RFC 9110 §7.6.1). That is what separatesthe Responses WebSocket operation from
POST /v1/responsesat the same pathwithout inspecting a payload.
Note on consuming the classification
Two findings from wiring this up, both worth knowing before #745 starts.
x-praxis-ai-*is a reserved prefix. The protocol layer rejects a clientrequest carrying one with a 400, and strips them before forwarding upstream. So
the "client-supplied headers cannot spoof the classifier" criterion is already
enforced at the ingress boundary, ahead of any filter. This filter's
overwrite-on-match and remove-on-miss remain as defense in depth.
Header-phase mutations are not visible to
router. The router matches thedownstream request headers, while a header-phase filter's changes are pending
until the request is forwarded.
openai_responses_formatonly routes onx-praxis-ai-formatbecause it runs in the pre-read body phase, which landsbefore the header-phase pipeline. A head-only classifier has no equivalent.
Consequently the published headers have no in-pipeline consumer today. The
working mechanisms are request extensions for downstream filters and filter
results for branch chains. The header code is kept because it is correct, cheap,
and becomes useful with a non-reserved header name or a future consumer — but
#745's plan to route on these headers needs revisiting before it is picked up.
Testing
cargo test --workspace— all suites pass, including 2594 inpraxis-ai-apisand 479 integration tests
make lint— passes end to endmake doc— clean15 unit tests cover the published outputs: both families classified, WebSocket
separated from create by transport, static endpoints not consumed as
identifiers, unsupported methods publishing nothing, configurable and disabled
header names, invalid header names rejected at startup, and the
Connection/Upgradetoken-list edge cases.6 functional tests cover the example config's observable boundary behaviour:
classified and unclassified traffic forwarded unharmed, the classifier's own
headers never crossing to upstream, and client-supplied reserved headers
rejected at ingress on both classified and unclassified paths.
Not in scope
Switching the existing classifier and Responses filters to consume this match is
#742 and #741. Registry-driven routing is #745.