fix(llm-gateway): reject priority header#341
Merged
Merged
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe LLM API gateway now rejects client-supplied ChangesLLM priority header rejection
Estimated code review effort: 2 (Simple) | ~10 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant Echo
participant NVCFAuth
participant PriorityGuard
participant LLMHandler
Client->>Echo: Send LLM request
Echo->>NVCFAuth: Authenticate request
NVCFAuth->>PriorityGuard: Continue authorized request
PriorityGuard-->>Client: Return 400 for X-Priority
PriorityGuard->>LLMHandler: Forward request without X-Priority
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.12.2)level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain main module or its selected dependencies" Comment |
along-2017
force-pushed
the
fix/llm-gateway-reject-priority-header
branch
2 times, most recently
from
July 22, 2026 05:50
223a470 to
c860c2b
Compare
X-Priority is a gateway-owned trusted header: the gateway sets it from the priority resolved by NVCF API at auth time, so a client must not be able to self-assign it and influence Stargate routing or queue admission. Reject any LLM API request that arrives with an X-Priority header, returning 400 Bad Request before any outbound request is built. The check fires on header presence rather than value, so a present-but-empty X-Priority is rejected too. The guard is route middleware on the LLM route group, so it covers exactly the proxied endpoints (/v1/chat/completions, /v1/responses, /v1/embeddings, and any route later added to the group) and leaves non-proxied routes such as the health endpoints untouched. Because group middleware runs after the global middlewares, authentication still runs first: an unauthenticated request gets 401 before the header check. Signed-off-by: along <along@nvidia.com>
along-2017
force-pushed
the
fix/llm-gateway-reject-priority-header
branch
from
July 22, 2026 16:29
c860c2b to
2d0e3e8
Compare
along-2017
marked this pull request as ready for review
July 22, 2026 17:37
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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.
Inline comments:
In `@src/invocation-plane-services/llm-api-gateway/api/priority_guard_test.go`:
- Around line 46-53: Extend the priority test matrix around the existing guarded
routes and priority values with a case for an LLM route that omits X-Priority.
Assert that this request is not rejected by the priority guard and proceeds to
authorization, while preserving the existing explicit and empty-header cases and
the /healthz behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 267a28d9-6e0b-40bd-8135-23480fdeb5a8
📒 Files selected for processing (5)
src/invocation-plane-services/llm-api-gateway/README.mdsrc/invocation-plane-services/llm-api-gateway/api/BUILD.bazelsrc/invocation-plane-services/llm-api-gateway/api/priority_guard.gosrc/invocation-plane-services/llm-api-gateway/api/priority_guard_test.gosrc/invocation-plane-services/llm-api-gateway/api/routes.go
…iority Add a positive control asserting rejectClientSuppliedPriority forwards a request that carries no X-Priority to the next handler. The existing matrix only covered rejection, so a guard that rejected unconditionally would have satisfied every case while breaking all legitimate LLM traffic; this test fails on exactly that mutation. Exercises the guard branch directly because a no-header request reaching the real LLM handler needs the full provider stack, which is out of scope for a guard test. Signed-off-by: along <along@nvidia.com>
FamousDirector
approved these changes
Jul 22, 2026
Contributor
|
🎉 This PR is included in version nvcf-llm-api-gateway-v0.10.1 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
TL;DR
X-Priorityis a gateway-owned trusted header. A client must not be able to self-assign it. This rejects any LLM API request that arrives with a client-suppliedX-Prioritywith 400 Bad Request.Additional Details
/v1/chat/completions,/v1/responses,/v1/embeddings, and any route later added to the group) and leaves routes registered directly on the server, such as the health endpoints, untouched.X-Priorityis rejected too.For the Reviewer
api/priority_guard.gois the guard;api/routes.gowires it onto the LLM group viae.Group("", rejectClientSuppliedPriority).api/priority_guard_test.godrives the realRegisterRouteswiring, so it fails if the guard is ever dropped from the group.For QA
go test ./api/...andbazel test //api:api_testpass. Manual: a request to any of the three LLM endpoints carryingX-Priority(any value, including empty) returns 400;/healthzwithX-Prioritystill returns 200.Issues
Closes #187.
Checklist
Summary by CodeRabbit
X-Priorityheader with an HTTP 400 response indicating the header is reserved./healthzand/readyzremain unaffected even whenX-Priorityis present.X-Priorityis reserved for gateway use.