Skip to content

executors/webhook: cap response body at 64 KiB#11

Open
jaredzwick wants to merge 1 commit into
mainfrom
hir-113-cap-webhook-response
Open

executors/webhook: cap response body at 64 KiB#11
jaredzwick wants to merge 1 commit into
mainfrom
hir-113-cap-webhook-response

Conversation

@jaredzwick

Copy link
Copy Markdown
Collaborator

Why

execute_webhook called response.text().await with no size bound. A
misbehaving (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

  • Added RESPONSE_BODY_LIMIT_BYTES = 64 KiB public constant.
  • Replaced response.text() with a chunk() loop that stops at the cap
    (no new feature flags required; uses the existing reqwest API).
  • New WebhookError::ResponseTooLarge { status, captured_bytes } variant for
    non-2xx responses where the body exceeded the cap.
  • WebhookResult gains body_truncated: bool; set to true on 2xx overflow.
  • execute_webhook now takes an explicit body_size_limit: usize parameter;
    all call sites pass RESPONSE_BODY_LIMIT_BYTES.
  • examples/cron_executor.rs updated to pass the limit.
  • README Action Executors → Webhook documents the cap and both overflow
    behaviours.

Tests

Two new wiremock tests:

Test Covers
truncates_oversized_success_body 2xx body > cap → body_truncated = true, body capped
returns_response_too_large_on_oversized_non_2xx_body non-2xx body > cap → ResponseTooLarge
test result: ok. 20 passed; 0 failed; 0 ignored

🤖 Generated with Claude Code

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>
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