Skip to content

Require authentication for RL control-plane routes - #1662

Open
mikemikimike wants to merge 1 commit into
ModelTC:mainfrom
mikemikimike:fix/rl-control-api-auth-1609
Open

mikemikimike wants to merge 1 commit into
ModelTC:mainfrom
mikemikimike:fix/rl-control-api-auth-1609

Conversation

@mikemikimike

@mikemikimike mikemikimike commented Oct 23, 2026 •

Copy link
Copy Markdown

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 from LIGHTLLM_RL_API_KEY for 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)
  • 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.

@coderabbitai

coderabbitai Bot commented Oct 23, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

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
Tests cover router mounting, authentication responses, valid-token dispatch, and startup validation.

Priority: ⬆️ High

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Bug fix · Severity of issue fixed: High

Sequence Diagram(s)

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: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning 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 Passed checks: 4 passed
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly and concisely describes the main change: authentication for RL control-plane routes.
Linked Issues check Passed 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 Passed 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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant