chore(proxy): surface form-parse failures instead of caching empty body

Starlette's ``request.form()`` raises ``MultiPartException`` /
``ValueError`` / ``AssertionError`` on malformed multipart input
(missing boundary, malformed chunk encoding, etc.). The outer
``except Exception: return {}`` swallowed every form-parse failure
and cached an empty parsed body — auth-time pre-reads saw ``{}`` and
skipped every banned-param check while a later raw-body re-read in
the handler still saw the original payload. Same TOCTOU shape as the
substring-match bypass: the auth gate and the handler don't agree on
what the body is.

Wrap ``request.form()`` in a narrow ``try`` that converts any parse
failure to a 400 ``ProxyException``. The outer broad ``except`` is
retained for unrelated unexpected errors but no longer covers
form-parse-side bypass shapes.

Adds a regression test parametrised over the exception classes
Starlette can raise from ``request.form()``.
This commit is contained in:
user 2026-05-14 19:46:20 +00:00
parent 56eb5ef550
commit db729d4768
No known key found for this signature in database
2 changed files with 45 additions and 1 deletions

View file

@ -66,7 +66,23 @@ async def _read_request_body(request: Optional[Request]) -> Dict:
content_type = _request_headers.get("content-type", "")
if _is_form_content_type(content_type):
parsed_body = dict(await request.form())
try:
form_data = await request.form()
except Exception as e:
# ``request.form()`` raises on malformed multipart (missing
# boundary, malformed chunk encoding, …). Surface as 400 so
# the auth-time pre-read does not silently cache ``{}`` while
# a later raw-body re-read sees the original payload —
# banned-param checks must see the same body the handler
# acts on.
verbose_proxy_logger.error(f"Invalid form payload: {e}")
raise ProxyException(
message=f"Invalid form payload: {e}",
type="invalid_request_error",
param="request_body",
code=status.HTTP_400_BAD_REQUEST,
)
parsed_body = dict(form_data)
if "metadata" in parsed_body and isinstance(parsed_body["metadata"], str):
parsed_body["metadata"] = json.loads(parsed_body["metadata"])
else:

View file

@ -930,6 +930,34 @@ class TestReadRequestBodyNonCanonicalContentType:
mock_request.form.assert_awaited_once()
class TestReadRequestBodyFormParseFailure:
"""
A failed ``request.form()`` parse (e.g. multipart with missing boundary)
must surface as a 400, not silently return ``{}`` — otherwise the
auth-time pre-read sees an empty body while a later raw-body re-read
sees the original payload, defeating every banned-param check.
"""
@pytest.mark.asyncio
@pytest.mark.parametrize(
"raised_exception",
[
ValueError("Missing boundary in multipart."),
AssertionError("malformed chunk"),
RuntimeError("form parser exploded"),
],
)
async def test_form_parse_failure_raises_400(self, raised_exception):
mock_request = MagicMock()
mock_request.form = AsyncMock(side_effect=raised_exception)
mock_request.headers = {"content-type": "multipart/form-data"}
mock_request.scope = {}
with pytest.raises(ProxyException) as exc_info:
await _read_request_body(mock_request)
assert str(exc_info.value.code) == "400"
class TestIsJsonContentType:
@pytest.mark.parametrize(
"content_type",