Skip to content

fix(webhook): enforce body-size limit when Content-Length is absent - #672

Open
hashbender wants to merge 1 commit into
mainfrom
mirror/pr-56829
Open

fix(webhook): enforce body-size limit when Content-Length is absent#672
hashbender wants to merge 1 commit into
mainfrom
mirror/pr-56829

Conversation

@hashbender

Copy link
Copy Markdown
Owner

Summary

The generic webhook adapter (gateway/platforms/webhook.py) enforced its per-route max_body_bytes limit only via the Content-Length header:

content_length = request.content_length or 0
if content_length > self._max_body_bytes:
    return web.json_response({"error": "Payload too large"}, status=413)
raw_body = await request.read()   # reads the whole body, no re-check

A request using Transfer-Encoding: chunked (or a spoofed small Content-Length) makes request.content_length None, so None or 0 == 0 and the check is skipped entirely. The full body is then buffered into memory regardless of the route's configured max_body_bytes.

This is reachable by an unauthenticated caller — the body must be read before HMAC signature verification — so it bypasses the documented size guard and is a DoS vector on an internet-exposed webhook endpoint. The module docstring even claims "Body size limits checked before reading payload", which chunked requests defeat.

The sibling adapters Feishu (plugins/platforms/feishu/adapter.py) and WeCom already guard this exact case with a post-read length check and have regression tests for content_length=None — the generic adapter was the odd one out.

Fix

  • Re-check the limit against the bytes actually read (len(raw_body) > self._max_body_bytes → 413), keeping the Content-Length fast path.
  • Update the module docstring to reflect the real guarantee.
  • Add a regression test driving a chunked-style request (Content-Length None) with an over-limit body, asserting 413.

Test plan

  • tests/gateway/test_webhook_adapter.py — 69 passed (68 existing + 1 new)
  • Full webhook suite (test_webhook_adapter, test_webhook_signature_rate_limit, test_webhook_integration, test_webhook_dynamic_routes, test_webhook_deliver_only) — 103 passed
  • Confirmed the new test fails without the fix (returns 202 instead of 413) and passes with it

Mirror-of: NousResearch#56829
NousResearch#56829

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