You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Require authentication for RL control-plane routes - #1662
The RL control-plane routes were mounted even when RL was disabled and accepted unauthenticated requests when enabled. This change mounts those routes only with --enable_rl, requires a Bearer token from LIGHTLLM_RL_API_KEY for every RL route, and rejects enabled startup when the key is missing.
Repository-configured Black and Flake8 checks passed.
python -m compileall passed for the changed Python files.
python -m pytest -q could not complete collection on Windows because Triton has no Windows wheel and uvloop does not support Windows; the same collection failures occurred on the unmodified base.
Summary by CodeRabbit
New Features
RL control-plane routes now require an API key sent as a Bearer token in the Authorization header.
Enabling RL without configuring LIGHTLLM_RL_API_KEY now prevents the server from starting.
Bug Fixes
Requests with missing or invalid credentials are rejected rather than accessing RL control-plane routes.
No actionable comments were generated in the recent review. 🎉
ℹ️ Recent review info
⚙️ Run configuration
Source mode: Auto
Repository: Unavailable
Branch: Unavailable
File: No configuration file from this repository applied
Commit: Unavailable
Layers: defaults
Review profile: CHILL
Plan: Advanced
Run ID: 2ed9cf72-bc4b-4084-a4f9-23f69ae3eab1
📥 Commits
Reviewing files that changed from the base of the PR and between b799a58 and ed56db5.
📒 Files selected for processing: 6
lightllm/server/api_cli.py
lightllm/server/api_http.py
lightllm/server/api_http_rl.py
lightllm/server/api_start.py
lightllm/server/rl_auth.py
unit_tests/server/test_rl_api_auth.py
Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 7 remain after this review.
📝 Walkthrough
RL control routes are mounted only when RL is enabled. Startup requires LIGHTLLM_RL_API_KEY, and the routes require a matching Bearer token. Tests cover mounting, authentication, request dispatch, and startup validation.
Changes: RL control route authentication
Layer / File(s)
Summary
API key and startup validation lightllm/server/rl_auth.py, lightllm/server/api_start.py, lightllm/server/api_cli.py
Adds API key lookup and startup validation. The CLI help describes the key and Bearer-token requirement.
Conditional mounting and request authentication lightllm/server/rl_auth.py, lightllm/server/api_http.py, lightllm/server/api_http_rl.py
Startup conditionally mounts the RL router. The router applies require_rl_auth to its routes, which reject missing configuration or invalid Bearer credentials.
Authentication and startup tests unit_tests/server/test_rl_api_auth.py
sequenceDiagram
participant Client
participant FastAPI
participant require_rl_auth
participant RLControlRoute
Client->>FastAPI: Send RL control request with Bearer token
FastAPI->>require_rl_auth: Resolve router dependency
require_rl_auth->>require_rl_auth: Compare token with configured API key
require_rl_auth-->>FastAPI: Authentication succeeds
FastAPI->>RLControlRoute: Dispatch request
Loading
Suggested reviewers:shihaobai, hiworldwzj
Merge Risk:⚪ Minimal · up to ed56d
RL control routes appear restricted to enabled deployments and protected by the separate Bearer credential. No merge-blocking issue was identified.
Pre-merge checks: 4 · 1
Failed checks: 1 warning
Check name
Status
Explanation
Resolution
Docstring Coverage
Docstring coverage is 21.05% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 19 functions across 6 files.
Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed checks: 4 passed
Check name
Status
Explanation
Description Check
Check skipped - CodeRabbit’s high-level summary is enabled.
Title check
The title clearly and concisely describes the main change: authentication for RL control-plane routes.
Linked Issues check
Issue #1609 requires conditional RL route exposure and authentication for RL control operations. The reviewed changes add mount_rl_router, which mounts the RL router only when enable_rl is true. T…
Out of Scope Changes check
The changes stay within issue #1609. The CLI help update documents the new RL credential requirement. Server startup wiring, the authentication helper, route protection, and focused tests directly imp…
Full details: Linked Issues check
Explanation
Issue #1609 requires conditional RL route exposure and authentication for RL control operations. The reviewed changes add mount_rl_router, which mounts the RL router only when enable_rl is true. The RL router applies require_rl_auth to every route. The dependency requires a Bearer token that matches LIGHTLLM_RL_API_KEY using constant-time comparison. validate_rl_auth rejects RL-enabled startup when the key is missing. The added tests cover disabled mounting, route authentication, valid dispatch, invalid tokens, missing configuration, and startup validation.
Full details: Out of Scope Changes check
Explanation
The changes stay within issue #1609. The CLI help update documents the new RL credential requirement. Server startup wiring, the authentication helper, route protection, and focused tests directly implement the issue requirements. No unrelated behavior or unrelated files are identified in the supplied change summary.
✨ Finishing Touches
🧪 Generate unit tests (beta)
Create a new PR
Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
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
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.
Fixes #1609
Issue: #1609
The RL control-plane routes were mounted even when RL was disabled and accepted unauthenticated requests when enabled. This change mounts those routes only with
--enable_rl, requires a Bearer token fromLIGHTLLM_RL_API_KEYfor every RL route, and rejects enabled startup when the key is missing.Validation:
python -m pytest -q unit_tests/server/test_rl_api_auth.py(7 passed)python -m compileallpassed for the changed Python files.python -m pytest -qcould not complete collection on Windows because Triton has no Windows wheel and uvloop does not support Windows; the same collection failures occurred on the unmodified base.Summary by CodeRabbit
Authorizationheader.LIGHTLLM_RL_API_KEYnow prevents the server from starting.