executors/webhook: cap response body at 64 KiB#11
Open
jaredzwick wants to merge 1 commit into
Open
Conversation
Stream the response body via reqwest's chunk() API and stop reading after
RESPONSE_BODY_LIMIT_BYTES (64 KiB). A misbehaving webhook target returning
a multi-GB body would otherwise allocate unbounded memory in the executor.
Behaviour on overflow:
- 2xx: body truncated, WebhookResult::body_truncated = true
- non-2xx: WebhookError::ResponseTooLarge { status, captured_bytes }
(replaces NonSuccessStatus so the error payload stays bounded)
execute_webhook now takes body_size_limit as an explicit parameter;
call sites pass RESPONSE_BODY_LIMIT_BYTES. Adds two new wiremock tests
covering both overflow paths.
Co-Authored-By: Paperclip <noreply@paperclip.ing>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
execute_webhookcalledresponse.text().awaitwith no size bound. Amisbehaving (or hostile) webhook target returning a multi-GB body would
allocate the whole body into memory inside the executor task, and then clone it
into
WebhookError::NonSuccessStatus { body }— a denial-of-service surface.Closes HIR-113.
What changed
RESPONSE_BODY_LIMIT_BYTES = 64 KiBpublic constant.response.text()with achunk()loop that stops at the cap(no new feature flags required; uses the existing reqwest API).
WebhookError::ResponseTooLarge { status, captured_bytes }variant fornon-2xx responses where the body exceeded the cap.
WebhookResultgainsbody_truncated: bool; set totrueon 2xx overflow.execute_webhooknow takes an explicitbody_size_limit: usizeparameter;all call sites pass
RESPONSE_BODY_LIMIT_BYTES.examples/cron_executor.rsupdated to pass the limit.behaviours.
Tests
Two new
wiremocktests:truncates_oversized_success_bodybody_truncated = true, body cappedreturns_response_too_large_on_oversized_non_2xx_bodyResponseTooLarge🤖 Generated with Claude Code