fix(videos): encode the variant query param

``variant`` is user-controlled (passed through from
``litellm.video_content(variant=...)``) and was interpolated raw into
the URL query string.  A value like ``thumbnail&extra=1`` would inject
additional query parameters into the upstream request — the same
class of issue this PR's path-segment encoding addresses.  Wrap the
value in ``quote(value, safe="")`` so ``&`` / ``=`` / ``#`` cannot
terminate the ``variant`` value or open a new parameter.

Adds a regression test asserting that a malicious ``thumbnail&extra=1``
ends up percent-encoded in the URL, and that the legitimate
``thumbnail`` value still round-trips cleanly.
This commit is contained in:
user 2026-05-01 00:32:02 +00:00
parent dc3e0739fb
commit fc580ae1ec
2 changed files with 34 additions and 1 deletions

View File

@ -1,6 +1,7 @@
import mimetypes
from io import BufferedReader, BytesIO
from typing import TYPE_CHECKING, Any, Dict, List, Optional, Tuple, Union, cast
from urllib.parse import quote
import httpx
from httpx._types import RequestFiles
@ -228,7 +229,11 @@ class OpenAIVideoConfig(BaseVideoConfig):
# Construct the URL for video content download
url = f"{api_base.rstrip('/')}/{encoded_video_id}/content"
if variant is not None:
url = f"{url}?variant={variant}"
# Encode the user-controlled ``variant`` so a value like
# ``thumbnail&extra=1`` cannot inject additional query params
# into the upstream request — same hardening rationale as the
# path-segment encoding above.
url = f"{url}?variant={quote(variant, safe='')}"
# No additional data needed for GET content request
data: Dict[str, Any] = {}

View File

@ -20,6 +20,34 @@ def test_video_content_request_encodes_video_id_path_segment():
assert params == {}
def test_video_content_request_encodes_variant_query_param():
"""``variant`` is user-controlled and was previously interpolated raw
into the query string. A value like ``thumbnail&extra=1`` would
inject additional query parameters into the upstream request."""
config = OpenAIVideoConfig()
url, _ = config.transform_video_content_request(
video_id="vid_123",
api_base="https://api.openai.com/v1/videos",
litellm_params=GenericLiteLLMParams(),
headers={},
variant="thumbnail&extra=1",
)
# ``&`` and ``=`` must be percent-encoded so they cannot terminate
# the ``variant`` value or open a new query parameter.
assert "?variant=thumbnail%26extra%3D1" in url
# Sanity: the legitimate "thumbnail" value still round-trips cleanly.
url2, _ = config.transform_video_content_request(
video_id="vid_123",
api_base="https://api.openai.com/v1/videos",
litellm_params=GenericLiteLLMParams(),
headers={},
variant="thumbnail",
)
assert url2.endswith("?variant=thumbnail")
def test_wrapped_character_id_is_decoded_then_encoded_as_path_segment():
config = OpenAIVideoConfig()
character_id = encode_character_id_with_provider(