Scheduler: webhook deliveries and browser pushes have no overall deadline or size cap, so one receiver can stall a pass #365

New issue

Closed

opened 2026-10-01 10:46:10 +00:00 by tiagoagueda · 1 comment

tiagoagueda commented

2026-10-01 10:46:10 +00:00

Owner

Found on 2026-09-29 in a read-only review of postulo at 95348908d, reading the shared HTTP client and the two notifiers the scheduler calls inline.

Checked again on 2026-10-01 against main at 0a5cad587: still present. The code is now at plugins/http.py:139-145 (_build) and notifications/management/commands/send_due_reminders.py:189-198 (the pass); the other locations are unchanged.

#321 §3 records that the capture and logo fetches buffer the whole body and time out per read only. The shared client every connection uses has the same weakness, and two of its callers run inside the scheduler pass that serves every account. A receiver chosen by one account can hold that pass, and with it everybody's reminders, store copies and syncs, for as long as it keeps sending a byte now and then.

Where: plugins/http.py:137-143 (_build: httpx.Client(timeout=timeout, ...), with no streaming, byte limit or overall deadline); plugins/webhook/__init__.py:63 (TIMEOUT = 10.0, "How long one delivery may take") and :232-242 (post: http.post(...), buffered); plugins/browser/webpush.py:225-240 (push: client.post(subscription.endpoint, ...), buffered); notifications/webhooks.py:155-165 (send_pending, up to BATCH = 50 rows with no time budget); notifications/management/commands/send_due_reminders.py:187-196, where reminders (whose browser pushes run inside notify), copies, webhooks and syncs run one after another.
What happens: an httpx timeout limits each socket operation, not the request, so a receiver that answers slowly but steadily never trips the ten seconds; the response is read whole into memory, whatever its size. run_syncs has a budget, but it only decides whether to start the next sync; send_pending has none.
Who can trigger it: any account, with a webhook connection whose address is a public server it controls (a status change queues a delivery by default), or a browser connection whose stored endpoint is any https:// address (parse_subscription checks the scheme only).
Why it matters: the scheduler is one loop for the instance; while it is held, nobody's reminders, closing-date warnings, store copies or syncs go out, and a large reply costs the worker its memory.
Fix: put the limits where every caller gets them. In _build, read responses through a wrapper that streams and stops past a cap (a webhook or push reply needs a few kilobytes; a caller that needs more asks for it) and enforces a deadline on the whole request. Give webhooks.send_pending the same time budget the syncs have, and stop starting deliveries once it is spent.

Done when

tests/test_webhooks.py has a test with a transport that yields slowly past the deadline and one that yields more than the cap, each expecting the delivery to fail promptly with a recorded reason, and a test that send_pending stops starting deliveries once its budget is spent.

Related to #321 §3, which covers the same weakness in fetching._read_capped and logos.download only.

Found on 2026-09-29 in a read-only review of postulo at 95348908d, reading the shared HTTP client and the two notifiers the scheduler calls inline. Checked again on 2026-10-01 against `main` at `0a5cad587`: still present. The code is now at `plugins/http.py:139-145` (`_build`) and `notifications/management/commands/send_due_reminders.py:189-198` (the pass); the other locations are unchanged. #321 §3 records that the capture and logo fetches buffer the whole body and time out per read only. The shared client every connection uses has the same weakness, and two of its callers run inside the scheduler pass that serves every account. A receiver chosen by one account can hold that pass, and with it everybody's reminders, store copies and syncs, for as long as it keeps sending a byte now and then. - **Where:** `plugins/http.py:137-143` (`_build`: `httpx.Client(timeout=timeout, ...)`, with no streaming, byte limit or overall deadline); `plugins/webhook/__init__.py:63` (`TIMEOUT = 10.0`, "How long one delivery may take") and `:232-242` (`post`: `http.post(...)`, buffered); `plugins/browser/webpush.py:225-240` (`push`: `client.post(subscription.endpoint, ...)`, buffered); `notifications/webhooks.py:155-165` (`send_pending`, up to `BATCH = 50` rows with no time budget); `notifications/management/commands/send_due_reminders.py:187-196`, where reminders (whose browser pushes run inside `notify`), copies, webhooks and syncs run one after another. - **What happens:** an httpx timeout limits each socket operation, not the request, so a receiver that answers slowly but steadily never trips the ten seconds; the response is read whole into memory, whatever its size. `run_syncs` has a `budget`, but it only decides whether to start the next sync; `send_pending` has none. - **Who can trigger it:** any account, with a webhook connection whose address is a public server it controls (a status change queues a delivery by default), or a browser connection whose stored endpoint is any `https://` address (`parse_subscription` checks the scheme only). - **Why it matters:** the scheduler is one loop for the instance; while it is held, nobody's reminders, closing-date warnings, store copies or syncs go out, and a large reply costs the worker its memory. - **Fix:** put the limits where every caller gets them. In `_build`, read responses through a wrapper that streams and stops past a cap (a webhook or push reply needs a few kilobytes; a caller that needs more asks for it) and enforces a deadline on the whole request. Give `webhooks.send_pending` the same time budget the syncs have, and stop starting deliveries once it is spent. ## Done when - `tests/test_webhooks.py` has a test with a transport that yields slowly past the deadline and one that yields more than the cap, each expecting the delivery to fail promptly with a recorded reason, and a test that `send_pending` stops starting deliveries once its budget is spent. Related to #321 §3, which covers the same weakness in `fetching._read_capped` and `logos.download` only.

tiagoagueda added this to the 0.5.0 milestone

2026-10-01 10:46:10 +00:00

tiagoagueda added the

security

enhancement

labels

2026-10-01 10:46:10 +00:00

tiagoagueda commented

2026-10-01 11:03:10 +00:00

Author

Owner

Evidence from the review of #321 on 2026-09-30, for the browser-push half.

plugins/browser/webpush.py::push() calls client.post(subscription.endpoint). That reads the push service's whole answer, with br offered, and parse_subscription checks only the endpoint's scheme, so it can be any https:// address an account holder stores. Measured: a 316-byte brotli answer was held as 200 MB in the worker.

#321 adds http.read_body(response, limit, deadline): a streamed read with a size limit, bounded decompression and a whole-download deadline. It would fit both paths here.

Evidence from the review of #321 on 2026-09-30, for the browser-push half. `plugins/browser/webpush.py::push()` calls `client.post(subscription.endpoint)`. That reads the push service's whole answer, with `br` offered, and `parse_subscription` checks only the endpoint's scheme, so it can be any `https://` address an account holder stores. Measured: a 316-byte brotli answer was held as 200 MB in the worker. #321 adds `http.read_body(response, limit, deadline)`: a streamed read with a size limit, bounded decompression and a whole-download deadline. It would fit both paths here.

tiagoagueda referenced this issue

2026-10-10 10:56:24 +00:00

PST-052: Outbound responses are read with no size cap or total deadline, and name resolution has no timeout #781

tiagoagueda referenced this issue

2026-10-17 10:38:44 +00:00

SEC2-028: The webhook notifier accepts a plain http:// receiver, so notification text leaves in clear #970

tiagoagueda referenced this issue from a commit

2026-10-25 16:02:38 +00:00

Give the webhook pass a time budget, and test the receiver limits (#365)

tiagoagueda referenced this issue from a pull request that will close it,

2026-10-25 16:02:54 +00:00

Give the webhook pass a time budget, and test the receiver limits #1050