Skip to content

Sanitize playground output, unblock event loop, scope routing result per request - #30

Open
Sajjad-Fazeli wants to merge 1 commit into
NVIDIA-AI-Blueprints:v3from
Sajjad-Fazeli:fix/playground-xss-and-runtime
Open

Sanitize playground output, unblock event loop, scope routing result per request#30
Sajjad-Fazeli wants to merge 1 commit into
NVIDIA-AI-Blueprints:v3from
Sajjad-Fazeli:fix/playground-xss-and-runtime

Conversation

@Sajjad-Fazeli

Copy link
Copy Markdown

Summary

Three runtime/security fixes in the serving adapters (full-mode app, router-only sidecar, and LiteLLM proxy middleware).

1. Playground XSS

The playground rendered streamed model output with marked.parse() directly into innerHTML with no sanitizer, and interpolated judge verdicts, model display names, confidence labels, and status strings into innerHTML unescaped. A model response — or a prompt-injected one — containing HTML such as <img src=x onerror=...> would execute script in the browser.

  • Markdown is now sanitized with DOMPurify before insertion
  • All other untrusted interpolations are HTML-escaped via a small esc() helper
  • Both paths fall back to textContent if a CDN library fails to load, so raw model output is never injected

2. Event-loop blocking

The SSE /api/chat endpoint and the router-only /v1/route endpoint called router.route() — a synchronous encoder forward pass — directly on the event loop, stalling every concurrent request for its full duration. Both now dispatch through asyncio.to_thread, matching what the custom routing strategy already does in async_get_available_deployment.

3. Cross-request routing-result race

The strategy stored each routing decision on a single shared _last_result attribute. completions.py and the proxy response middleware read it back after awaiting the upstream completion to attach routing metadata / rewrite the response model field — so under concurrency, request A could read request B's decision and report the wrong selected model.

  • The result is now held in a request-scoped contextvar slot installed via strategy.begin_request() (same pattern the per-request tolerance override already uses)
  • last_result falls back to the shared attribute when no slot is installed, preserving existing single-request behavior and the library API

Changes

  • static/playground.js: esc() + renderMarkdown() helpers; escape/sanitize every model-, judge-, and config-derived value written to innerHTML
  • static/index.html: load DOMPurify alongside marked
  • adapters/litellm/chat.py, adapters/http/route.py: route via asyncio.to_thread
  • adapters/litellm/strategy.py: request-scoped result slot (begin_request(), _record_result(), contextvar-aware last_result)
  • adapters/litellm/completions.py, adapters/litellm/proxy.py: call begin_request() before dispatch
  • Tests: concurrent requests each read back their own routing result; last_result still works without begin_request()

Tests

  • python -m pytest tests/adapters/test_litellm.py -q — 15 passed
  • python -m pytest -q — 227 passed, 30 skipped; the 6 failures are pre-existing on v3 (4 are the httpx ASGITransport incompatibility addressed by Harden playground streaming for reasoning chunks #26, plus environment-specific test_gpu/test_trunk)
  • python -m ruff check src/model_router_toolkit/adapters/ tests/adapters/test_litellm.py — clean
  • node --check src/model_router_toolkit/adapters/litellm/static/playground.js — clean

…uting result per request

Three runtime/security fixes in the serving adapters:

XSS: the playground rendered model output via marked.parse() straight
into innerHTML with no sanitizer, and interpolated judge verdicts,
model display names, and status strings into innerHTML unescaped. A
model response (or a prompt-injected one) containing markup could
execute script in the browser. Markdown is now sanitized with DOMPurify
and all other untrusted interpolations are HTML-escaped; both paths fall
back to textContent if a CDN library is unavailable.

Event-loop blocking: the SSE chat endpoint and the router-only /v1/route
endpoint called router.route() — a synchronous encoder forward pass —
directly on the event loop, stalling all concurrent requests for its
duration. Both now dispatch through asyncio.to_thread, matching the
custom routing strategy.

Cross-request races: the strategy stored the routing decision on a
single shared attribute that completions.py and the proxy middleware
read after awaiting the upstream call, so concurrent requests could read
each other's selected model and stamp the wrong model onto a response.
The result is now held in a request-scoped contextvar slot installed via
begin_request(); last_result falls back to the shared attribute when no
slot is set, preserving existing behavior.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant