push: re-subscribe on VAPID key rotation and prune dead 401/403 rows

After a VAPID keypair rotation, subscribers minted under the old key
silently stopped receiving notifications while the UI still reported
"on", and the dead rows were never cleaned up.

Frontend (enable): a browser holding an existing PushSubscription never
re-subscribed, so it kept using the old applicationServerKey. Now the
current server VAPID key is always fetched and compared against the
subscription's baked-in key; on a mismatch the stale subscription is
unsubscribed and re-created with the new key. The matching-key path is
unchanged.

Backend (send): a rotated key makes the push service reject delivery
with 401/403, which returned "error" and left the row in place forever.
Treat 401/403 as permanently dead alongside 404/410 so the caller prunes
the row. Genuinely transient failures (rate limits, 5xx) still return
"error" and keep the row.

Also correct the default VAPID contact to mailto:admin@thermograph.org
(the .app domain was a typo); the env override is unchanged.

Extends tests/notifications/test_push.py with the send() status mapping
and the contact default.

backend/notifications/push.py

@@ -37,7 +37,7 @@ log = logging.getLogger("thermograph.push")
 _DATA_DIR = paths.DATA_DIR
 _VAPID_PATH = os.environ.get("THERMOGRAPH_VAPID_FILE") or os.path.join(_DATA_DIR, "vapid.json")
 # The VAPID "sub" claim — a contact the push service can reach about our traffic.
-_CONTACT = os.environ.get("THERMOGRAPH_VAPID_CONTACT", "mailto:admin@thermograph.app")
+_CONTACT = os.environ.get("THERMOGRAPH_VAPID_CONTACT", "mailto:admin@thermograph.org")
 
 # pywebpush 2.0.0 forwards `timeout` straight to requests.post with no default of
 # its own — omit the kwarg here and the send blocks with NO timeout at all (the
@@ -174,10 +174,13 @@ def send(subscription_info: dict, payload: dict) -> str:
         return "ok"
     except WebPushException as e:
         status = getattr(getattr(e, "response", None), "status_code", None)
-        if status in (404, 410):
-            return "gone"          # endpoint retired — caller should delete it
-        # 401/403 = VAPID key mismatch, etc. Record it where it's visible (the errors
-        # JSONL / dashboard), not just the system journal.
+        if status in (401, 403, 404, 410):
+            # 404/410 = endpoint retired; 401/403 = VAPID mismatch (e.g. after a key
+            # rotation) — the row was minted under a key we can no longer sign for and
+            # will never authenticate again. All are permanently dead: caller deletes.
+            return "gone"
+        # Anything else (rate limits, 5xx, malformed payload) may be transient — keep
+        # the row and record it where it's visible (the errors JSONL / dashboard).
         log.warning("web push failed (status=%s): %s", status, e)
         audit.log_event("error", {"phase": "push", "status": status,
                                   "endpoint": (subscription_info.get("endpoint") or "")[:120]})

backend/tests/notifications/test_push.py

@@ -1,11 +1,16 @@
 """VAPID key resolution: the atomic first-writer-wins claim of the keypair file
 (push.py's `_claim_file`), and that `_load()` caches whatever that settles on
-rather than its own local generation when it loses the race.
+rather than its own local generation when it loses the race. Also `send()`'s
+mapping of push-service responses onto 'ok'/'gone'/'error'.
 
-No network — `send()`'s pywebpush call isn't exercised here (that's notify.py's
-`test_push_dispatched_on_new_notification` etc., which stub `push.send` itself)."""
+No network — where `send()` is exercised the pywebpush call is stubbed; the
+dispatch path (notify.py's `test_push_dispatched_on_new_notification` etc.) stubs
+`push.send` itself."""
 import json
 
+import pytest
+from pywebpush import WebPushException
+
 from notifications import push
 
 
@@ -94,3 +99,44 @@ def test_load_prefers_env_over_file(monkeypatch, tmp_path):
     result = push._load()
 
     assert result == {"private_key": "priv-env", "public_key": "pub-env"}
+
+
+# --- send() response mapping ------------------------------------------------
+_SUB = {"endpoint": "https://push.example.com/ep-1", "keys": {"p256dh": "BKEY", "auth": "YXV0aA"}}
+
+
+class _Resp:
+    def __init__(self, status_code):
+        self.status_code = status_code
+
+
+def _raise_status(status):
+    def _webpush(**kwargs):
+        raise WebPushException("boom", response=_Resp(status))
+    return _webpush
+
+
+@pytest.mark.parametrize("status", [401, 403, 404, 410])
+def test_send_prunes_permanently_dead_endpoints(monkeypatch, status):
+    # 404/410 = endpoint retired; 401/403 = VAPID key mismatch (e.g. after a key
+    # rotation). All are permanently dead, so send() reports 'gone' and the caller
+    # deletes the row — otherwise a rotated key leaves dead subscriptions forever.
+    monkeypatch.setattr(push, "webpush", _raise_status(status))
+    assert push.send(_SUB, {"hello": "world"}) == "gone"
+
+
+@pytest.mark.parametrize("status", [429, 500, 502])
+def test_send_keeps_row_on_transient_failure(monkeypatch, status):
+    # Rate limits / 5xx may recover; keep the row and report it as an error.
+    monkeypatch.setattr(push, "webpush", _raise_status(status))
+    assert push.send(_SUB, {"hello": "world"}) == "error"
+
+
+def test_send_ok(monkeypatch):
+    monkeypatch.setattr(push, "webpush", lambda **kwargs: None)
+    assert push.send(_SUB, {"hello": "world"}) == "ok"
+
+
+def test_default_contact_is_the_org_domain():
+    # The VAPID "sub" contact must be a domain we actually own; .app was a typo.
+    assert push._CONTACT == "mailto:admin@thermograph.org"

frontend/static/push-client.js

@@ -36,6 +36,21 @@ async function registration() {
   return navigator.serviceWorker.ready;
 }
 
+// Does an existing subscription's baked-in applicationServerKey still match the
+// server's current VAPID public key? After a key rotation it won't: the browser
+// keeps signing pushes the server can no longer authenticate, so we must replace
+// the subscription. `options.applicationServerKey` is an ArrayBuffer of the raw
+// key bytes (or null on browsers that don't expose it — treat that as a mismatch
+// and re-subscribe rather than leaving a possibly-stale subscription in place).
+function applicationServerKeyMatches(sub, serverKey) {
+  const current = sub.options && sub.options.applicationServerKey;
+  if (!current) return false;
+  const a = new Uint8Array(current);
+  if (a.length !== serverKey.length) return false;
+  for (let i = 0; i < a.length; i++) if (a[i] !== serverKey[i]) return false;
+  return true;
+}
+
 // Is this device currently subscribed? (a PushSubscription exists locally)
 export async function isEnabled() {
   if (!supported()) return false;
@@ -60,13 +75,24 @@ export async function enable() {
 
   const reg = await registration();
   let sub = await reg.pushManager.getSubscription();
+
+  // Always resolve the server's current VAPID key: it's needed to subscribe, and
+  // — when a subscription already exists — to detect a key rotation. A subscription
+  // minted under an old key silently stops receiving pushes, so on a mismatch we
+  // drop it and re-subscribe with the new key. Matching key = happy path, untouched.
+  const keyRes = await apiFetch(uv("push/vapid-key"));
+  if (!keyRes.ok) throw new Error("Couldn't fetch the server key.");
+  const { key } = await keyRes.json();
+  const serverKey = urlB64ToUint8Array(key);
+
+  if (sub && !applicationServerKeyMatches(sub, serverKey)) {
+    await sub.unsubscribe();
+    sub = null;
+  }
   if (!sub) {
-    const res = await apiFetch(uv("push/vapid-key"));
-    if (!res.ok) throw new Error("Couldn't fetch the server key.");
-    const { key } = await res.json();
     sub = await reg.pushManager.subscribe({
       userVisibleOnly: true,
-      applicationServerKey: urlB64ToUint8Array(key),
+      applicationServerKey: serverKey,
     });
   }
   const res = await apiFetch(uv("push/subscribe"), { method: "POST", json: sub.toJSON() });