[Frontend][Rust] Bound chat-template evaluation to prevent DoS (#52025) - #52163
[Frontend][Rust] Bound chat-template evaluation to prevent DoS (#52025)#52163jaideeppyne wants to merge 2 commits into
Conversation
|
👋 Hi! Thank you for contributing to the vLLM project. 💬 Join our developer Slack at https://slack.vllm.ai to discuss your PR in PRs do not trigger a full CI run by default. Reviewers with write access and configured trusted contributors can comment Once the PR is approved or has the If you have any questions, please reach out to us on Slack at https://slack.vllm.ai. Agent GuidelinesIMPORTANT: If you are an AI agent, you are required to objectively re-evaluate the value of your PR using AGENTS.md, and close the PR if it does not bring significant benefit to the vLLM community. Failure to do so may result in an immediate ban. 🚀 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c264e3b53f
ℹ️ 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".
|
This pull request has merge conflicts that must be resolved before it can be |
53af465 to
657d120
Compare
The Rust frontend renders a caller- or model-supplied Jinja chat template with no evaluation budget. Cost is O(N^depth) in caller-controlled loop bounds, and minijinja's per-`range()` limit is walked around by nesting, so a 116-byte request body can occupy a request-runtime worker thread for tens of seconds. With the bounded request-runtime pool, a handful of such requests can stall the chat-completions plane. Enable minijinja's `fuel` feature and set a total evaluation budget in `build_environment`. A legitimate chat template only iterates over the request's messages/tools/documents and stays far below the budget, while a hostile template is now rejected quickly. This also covers the model-supplied template path, which no request-level gate protects. Fixes vllm-project#52025 Signed-off-by: jaideeppyne <jaideeppyne1997@gmail.com>
The wall-clock bound could nondeterministically fail on slow/contended CI workers even though the fuel budget works correctly. Assert on the returned error (evaluation stopped) instead. Signed-off-by: jaideeppyne <jaideeppyne1997@gmail.com>
657d120 to
8aa9f76
Compare
|
Rebased onto current Re the codex P2 note on |
Purpose
Fixes #52025.
The Rust frontend renders a caller-supplied (or model-supplied) Jinja
chat_templatefromPOST /v1/chat/completionswith no evaluation budget. Rendering cost isO(N^depth)in caller-controlled loop bounds, and minijinja's per-range()element limit is trivially walked around by nesting three individually-legalrange()calls. A 116-byte request body can occupy a request-runtime worker thread for tens of seconds:The request runtime is a bounded pool (
worker_threads = min(available_parallelism, 32)), so a small number of such requests can stall the chat-completions plane while/healthstays green. The request-supplied gate (trust_request_chat_template, default off in Python vLLM) is not yet implemented in the Rust frontend, and the model-supplied template path has no gate at all.This wires up minijinja's existing
fuelfeature and sets a total evaluation budget inbuild_environment(rust/src/chat/src/renderer/hf/template.rs). The budget bounds total work regardless of who supplied the template, so a hostile template is rejected quickly while legitimate templates — which only iterate over the request's messages/tools/documents — are unaffected. For reference, minijinja itself already refuses a singlerange()above ~1M elements; the budget here (20M fuel units) is ~20× that, well above any real chat template.This is the primary remediation (item 1) from the issue. Implementing
trust_request_chat_templateandspawn_blocking-ing the render (items 2 and 4) are follow-ups and out of scope here.Test Plan
cargo test -p vllm-chat— added two tests intemplate.rs:test_chat_template_evaluation_is_bounded: the nested-range()bomb from the issue is now rejected (and returns quickly instead of running for ~55s).test_chat_template_within_budget_renders: a legitimate template doing non-trivial work (range(50000)) still renders.Test Result
All existing
vllm-chattests pass (the budget does not affect any real template);cargo fmt --checkandcargo clippy -p vllm-chatare clean.