Browser notifier follow-up: push only to public addresses, show no reply body, and retire withdrawn subscriptions #216

New issue

Closed

opened 2026-09-15 21:23:20 +00:00 by tiagoagueda · 0 comments

tiagoagueda commented

2026-09-15 21:23:20 +00:00

Owner

Follow-up to #209, from the 2026-09-15 code audit. Three things in the Browser notifier are wrong as shipped.

1. Push goes through the connection client and echoes the reply

The push endpoint comes from a secret textarea the person can edit (plugins/browser/__init__.py:85-97); only https:// is checked (webpush.py:196).
It is posted through api.client (webpush.py:212-216), so POSTULO_CONNECTIONS_ALLOW_PRIVATE governs it.
A failure puts 200 characters of the reply body into the error (webpush.py:241-245). test() returns that text, and ConnectionTestView shows it and stores it with record_test.
On an instance with private destinations allowed, any account can make the server POST to internal HTTPS services and read the start of each reply.
tests/test_browser_notifier.py switches private destinations on just so the fake push service resolves, which hides the problem.

Fix: a push service is always public, so use public_only_client(follow_redirects=False). Report only the status code in errors. Stub DNS in the tests instead of turning the switch on, and add a test that a private endpoint is refused with the switch on.

2. A withdrawn subscription is retried forever

webpush.push raises PushFailed(gone=True) on 404/410.
send() leaves an inbox copy and re-raises; notifications/service.py only records the error. .gone is read only by test().
Every later event POSTs to the push service again and fails again.

Fix: on gone, drop the subscription secret and switch the connection to Only while Postulo is open (or disable it). Leave an inbox notice saying why and how to allow notifications again.

3. Reply text in `last_error`

Whatever the fix for 1, record_test and connection.last_error should never hold a remote body. Check the other notifiers for the same pattern.

Follow-up to #209, from the 2026-09-15 code audit. Three things in the Browser notifier are wrong as shipped. ## 1. Push goes through the connection client and echoes the reply - The push endpoint comes from a secret textarea the person can edit (`plugins/browser/__init__.py:85-97`); only `https://` is checked (`webpush.py:196`). - It is posted through `api.client` (`webpush.py:212-216`), so `POSTULO_CONNECTIONS_ALLOW_PRIVATE` governs it. - A failure puts 200 characters of the reply body into the error (`webpush.py:241-245`). `test()` returns that text, and `ConnectionTestView` shows it and stores it with `record_test`. - On an instance with private destinations allowed, any account can make the server POST to internal HTTPS services and read the start of each reply. - `tests/test_browser_notifier.py` switches private destinations on just so the fake push service resolves, which hides the problem. **Fix:** a push service is always public, so use `public_only_client(follow_redirects=False)`. Report only the status code in errors. Stub DNS in the tests instead of turning the switch on, and add a test that a private endpoint is refused with the switch **on**. ## 2. A withdrawn subscription is retried forever - `webpush.push` raises `PushFailed(gone=True)` on 404/410. - `send()` leaves an inbox copy and re-raises; `notifications/service.py` only records the error. `.gone` is read only by `test()`. - Every later event POSTs to the push service again and fails again. **Fix:** on `gone`, drop the subscription secret and switch the connection to *Only while Postulo is open* (or disable it). Leave an inbox notice saying why and how to allow notifications again. ## 3. Reply text in `last_error` Whatever the fix for 1, `record_test` and `connection.last_error` should never hold a remote body. Check the other notifiers for the same pattern.

tiagoagueda added the

bug

security

labels

2026-09-15 21:23:20 +00:00

tiagoagueda added this to the 0.3.0 milestone

2026-09-15 21:33:18 +00:00

tiagoagueda referenced this issue from a commit

2026-09-16 06:52:03 +00:00

Push to the open web only, say nothing the push service replied, and stop once a browser withdraws

tiagoagueda closed this issue

2026-09-16 06:52:03 +00:00