[security] fix(proxy): require auth for node management#4579
Open
Hinotoi-agent wants to merge 1 commit into
Open
[security] fix(proxy): require auth for node management#4579Hinotoi-agent wants to merge 1 commit into
Hinotoi-agent wants to merge 1 commit into
Conversation
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
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.
Summary
This PR hardens the proxy node-management boundary so
/nodes/*routes are no longer exempt from bearer-token authentication when proxy API keys are configured./nodesauthentication bypass fromAuthenticationMiddleware.api_keysvalues before middleware setup so a single configured key is treated as one token, not as an iterable of characters.Security issues covered
Before this PR
AuthenticationMiddlewareskipped every path with the/nodesprefix./nodes/add,/nodes/remove,/nodes/terminate, and/nodes/statuswere reachable without a bearer token when proxy API keys were enabled.status.modelslist, causing the proxy to route matching model requests to the supplied URL.After this PR
/health,/docs,/redoc) bypass bearer-token auth./nodes/*routes now require the configured bearer token whenever proxy API keys are enabled.Why this matters
The proxy node registry is a model-serving control plane. It decides which backend receives
/v1/chat/completionsand/v1/completionstraffic. If unauthenticated callers can mutate that registry, they can steer prompts, generation parameters, and proxied request headers to an arbitrary backend, tamper with completions, or disrupt service by removing/terminating legitimate nodes.How this differs from related PRs
PR #4338 introduced the shared authentication middleware and documented the
/nodesskip prefix as an operational exception for proxy-to-node coordination. This PR is a follow-up hardening change for the management-route trust boundary: it keeps health/docs public, but does not treat mutating node-management endpoints as public coordination traffic.PR #4354 is an open proxy refactor. This PR is intentionally narrow and applies to current
main: it fixes the authentication boundary in the existing middleware and adds a focused regression test for the route behavior.Attack flow
Affected code
lmdeploy/serve/utils/server_utils.py,lmdeploy/serve/proxy/proxy.py,lmdeploy/serve/openai/api_server.pyRoot cause
Issue: proxy node-management routes bypass API-key auth
/nodesprefix.CVSS assessment
CVSS:3.1/AV:N/AC:L/PR:N/UI:N/S:C/C:H/I:H/A:HRationale:
Safe reproduction steps
POST /nodes/addwith a JSON body containing a backend URL andstatus.modelsfor an existing model./nodes/statuswithout a bearer token./nodes/addand/nodes/statusreturn401; the same requests succeed withAuthorization: Bearer <configured-key>.The included regression test reproduces this boundary locally with FastAPI
TestClientand the realAuthenticationMiddlewareimplementation.Expected vulnerable behavior
/nodes/addreturns success and mutates the node registry./nodes/statusdiscloses registered backend nodes.Changes in this PR
/nodesfrom the authentication middleware skip-prefix list./health,/docs, and/redocpublic as passive endpoints.api_keysvalues in proxy setup before creatingAuthenticationMiddleware./nodes/*routes and public/healthbehavior.Files changed
lmdeploy/serve/utils/server_utils.py/nodesfrom unauthenticated skip prefixeslmdeploy/serve/proxy/proxy.pyapi_keysbefore middleware creationlmdeploy/serve/openai/api_server.pytests/test_lmdeploy/serve/test_authentication_middleware.pyMaintainer impact
--api-keysare unchanged.Fix rationale
Node registration, removal, and termination are privileged control-plane operations. They should not share the same unauthenticated treatment as health checks or documentation routes. The narrowest durable boundary is to keep passive endpoints public and require bearer-token authentication for all node-management routes when API keys are configured.
Type of change
Test plan
AuthenticationMiddlewareroute behavior.pre-commitwas not run locally becausepre-commitis not installed in this environment.Executed with:
Result:
Token usage
Disclosure notes
This PR is bounded to the proxy node-management authentication boundary. It does not claim to change model execution behavior, prompt handling, or the broader proxy refactor in #4354. No unrelated files were intentionally changed.