fix(inference): strip client-supplied nvext from external requests - #1465
fix(inference): strip client-supplied nvext from external requests#1465fergusfinn wants to merge 1 commit into
Conversation
nvext is the operator/NVIDIA-extension namespace: nvext.agent_hints.priority carries the scheduling priority the platform injects on its own dispatch path (fusillade inject_deadline_priority). A client could set it directly through the public API to jump the engine's priority queue — the edge nginx strips the x-fusillade-* headers but cannot touch request bodies. The inference middleware already parses every external request body (the internal fusillade loopback carries x-fusillade-request-id and returns early above, and the edge strips that header from inbound traffic, so everything reaching the parse is external). Drop the top-level nvext object there, next to the existing id-field scrub, and re-derive the forwarded bytes only when the key was present so the common realtime path stays byte-identical.
Deploying control-layer with
|
| Latest commit: |
7678c66
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://c9806ade.control-layer.pages.dev |
| Branch Preview URL: | https://fix-strip-client-nvext.control-layer.pages.dev |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7678c6674f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if strip_client_nvext(&mut request_value) { | ||
| body_bytes = bytes::Bytes::from(request_value.to_string()); |
There was a problem hiding this comment.
Apply the nvext scrub to legacy completions
When an external client posts to /v1/completions, should_intercept returns false, so this scrub is never reached. The strict router explicitly exposes that endpoint (onwards/src/strict/mod.rs:185), and completions_handler forwards the original request bytes (onwards/src/strict/handlers.rs:451-505), allowing nvext.agent_hints.priority to reach the upstream engine and bypass the queue protection this change introduces. Include /completions in the intercepted paths or perform the scrub in a common outbound layer.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
This PR hardens the inference edge middleware (dwctl/src/inference/middleware.rs) against client spoofing of the operator-only nvext JSON namespace by stripping top-level nvext from external requests, while avoiding re-serialization on the common path.
Changes:
- Strip top-level
nvextfrom parsed request JSON and only re-serialize the forwarded bytes whennvextwas present. - Add
strip_client_nvexthelper to encapsulate the removal and report whether a change occurred. - Add unit tests covering removal, absent-is-noop, and non-object-is-noop cases.
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| if strip_client_nvext(&mut request_value) { | ||
| body_bytes = bytes::Bytes::from(request_value.to_string()); | ||
| } |
Why
nvextis the operator / NVIDIA-extension namespace.nvext.agent_hints.priorityis the engine scheduling priority the platform injects for itself on the dispatch path (fusillade'sinject_deadline_priority). Nothing stopped an external caller from setting it directly through the public API and jumping the engine's priority queue.The edge sso-stack nginx already strips the
x-fusillade-*headers to prevent exactly this kind of spoofing, but nginx can't parse request bodies, so a client-suppliednvextsails through to the engine (verified against prod: annvext.agent_hints.prioritytracer set via the public API landed as an engine metric label).What
The inference middleware already parses every request body into a
serde_json::Value(to readservice_tier/background) and already strips client-supplied id fields there. This adds a sibling strip of the top-levelnvextobject.The internal/external gate is the existing
x-fusillade-request-idearly-return: the fusillade daemon's loopback re-entry always carries that header (set in fusillade's HTTP client), and the edge strips it from inbound traffic, so everything reaching the parse is external by construction — no new gating needed. Internal dispatches keep their injectednvext.The forwarded bytes are re-derived from the parsed value only when
nvextwas actually present (≈never for legitimate traffic), so the common realtime path — which forwards the original bytes verbatim — stays byte-identical and pays no re-serialisation cost.Tests
Unit tests for
strip_client_nvext: removal + reported bool, absent-is-noop (body untouched), non-object-is-noop. Fullinference::middleware::testssuite green (22 passing).