From db346ee54d043f705be37fb040662f1a77ede2c5 Mon Sep 17 00:00:00 2001 From: jaylfc Date: Sat, 5 Sep 2026 06:27:59 +0000 Subject: [PATCH 1/2] fix(push): reuse the APNs provider token and prune tokens on 410 (tsk-42q2qf) Two independent defects in the Apple push sender. The sender minted a fresh ES256 provider token on every single push. Apple caps provider-token *generation*, not use: a notification burst earns 403 TooManyProviderTokenUpdates and pushes are then refused account-wide, not per-device. One token is now cached on the sender and reminted after 50 minutes -- inside the one-hour validity window and far under the generation cap. A clock that moves backwards counts as stale so a bad NTP step cannot pin a token past its real expiry, and Apple's own ExpiredProviderToken invalidates the cache at once rather than leaving every push refused until the timer fires. `send()` also collapsed every non-200 into a bare False, so 410 Unregistered -- Apple's permanent "this device token is dead" signal -- was indistinguishable from a retryable failure and the dead token was pushed to forever. 410 now raises ApnsUnregistered carrying the reason and apns-id; the device fan-out catches it and clears that token from the device row, scoped to the exact token that failed so a device that re-registered mid-fan-out keeps its new one. The device row itself is kept: the device is still paired and still visible to its owner. This mirrors the 404/410 prune the web-push path already performs. send_device_push therefore reports a "removed" count alongside sent/failed/ skipped, the same shape send_web_push returns. Every refusal now logs Apple's `reason` and the `apns-id`; neither was surfaced before, so a refused push could not be diagnosed at all. The ES256 signing itself is untouched. --- .../tsk-42q2qf-apns-token-reuse-and-410.md | 4 + .../design/whisplay-pocket-interface-spike.md | 2 +- tests/push/test_apns.py | 164 ++++++++++++++++++ tests/push/test_unifiedpush.py | 2 +- tests/test_notifications_push.py | 90 +++++++++- tinyagentos/device_store.py | 17 ++ tinyagentos/notifications_push.py | 55 +++++- tinyagentos/push/__init__.py | 6 + tinyagentos/push/apns.py | 87 +++++++++- 9 files changed, 410 insertions(+), 17 deletions(-) create mode 100644 changelog.d/tsk-42q2qf-apns-token-reuse-and-410.md diff --git a/changelog.d/tsk-42q2qf-apns-token-reuse-and-410.md b/changelog.d/tsk-42q2qf-apns-token-reuse-and-410.md new file mode 100644 index 000000000..aa92f2b97 --- /dev/null +++ b/changelog.d/tsk-42q2qf-apns-token-reuse-and-410.md @@ -0,0 +1,4 @@ +### Fixed +- Apple push no longer mints a fresh provider token for every notification: one token is now cached and reminted on a 50-minute timer, so a burst of notifications can no longer trip Apple's `TooManyProviderTokenUpdates` cap and get pushes refused for the whole account (tsk-42q2qf). +- A `410 Unregistered` from Apple now clears the dead push token from the device instead of being reported as a generic delivery failure, so an uninstalled or re-provisioned device is no longer pushed to forever. The device itself stays paired and visible; it simply has no push token until it registers a new one. +- Every refused push now logs Apple's own `reason` and the `apns-id`, so a refusal can be diagnosed instead of appearing as an unexplained non-delivery. An expired provider token also forces an immediate remint rather than waiting out the refresh timer. diff --git a/docs/design/whisplay-pocket-interface-spike.md b/docs/design/whisplay-pocket-interface-spike.md index 7e117a6f9..00861ed7a 100644 --- a/docs/design/whisplay-pocket-interface-spike.md +++ b/docs/design/whisplay-pocket-interface-spike.md @@ -155,7 +155,7 @@ permanently is a different product from one that wakes on push.** the same shape, just client-side on the Pi. - **Wake-on-push** (the "wakes on push" end of the spectrum) already exists for the device class — but only APNs / UnifiedPush for ios/watchos/android - (`notifications_push.py:388-429` `send_device_push`, branching on + (`notifications_push.py:417-466` `send_device_push`, branching on `device["platform"]` in `routes/devices.py`). A Pi has neither push endpoint, so **polling is the only delivery mode available to it today**; the device bearer has no push token registered and there is no `linux`/`embedded` platform diff --git a/tests/push/test_apns.py b/tests/push/test_apns.py index 4c7442d23..0db7f5126 100644 --- a/tests/push/test_apns.py +++ b/tests/push/test_apns.py @@ -107,3 +107,167 @@ async def test_http_sender_does_not_close_injected_client(): async def test_null_sender_aclose_is_noop(): # NullApnsSender exposes a no-op aclose so shutdown can call it uniformly. assert await NullApnsSender().aclose() is None + + +# --------------------------------------------------------------------------- +# tsk-42q2qf: provider-token reuse + 410 Unregistered handling +# --------------------------------------------------------------------------- + + +def _test_key_pem() -> str: + from cryptography.hazmat.primitives.asymmetric import ec + from cryptography.hazmat.primitives import serialization + + key = ec.generate_private_key(ec.SECP256R1()) + return key.private_bytes( + serialization.Encoding.PEM, + serialization.PrivateFormat.PKCS8, + serialization.NoEncryption(), + ).decode() + + +def _counting_mint(monkeypatch) -> list[int]: + """Replace build_apns_jwt with a counting passthrough; returns the counter.""" + from tinyagentos.push import apns as apns_mod + + real = apns_mod.build_apns_jwt + mints = [0] + + def counted(**kwargs): + mints[0] += 1 + return real(**kwargs) + + monkeypatch.setattr(apns_mod, "build_apns_jwt", counted) + return mints + + +def _sender_with(handler, pem: str): + client = httpx.AsyncClient(transport=httpx.MockTransport(handler)) + sender = HttpApnsSender( + key_pem=pem, key_id="KID", team_id="TID", bundle_id="com.taos.app", + host="api.push.apple.com", client=client, + ) + return sender, client + + +@pytest.mark.asyncio +async def test_provider_token_is_reused_across_pushes(monkeypatch): + # Apple caps provider-token GENERATION: minting one per push earns + # 403 TooManyProviderTokenUpdates and refuses pushes account-wide, so a + # burst must reuse one cached token rather than mint per request. + mints = _counting_mint(monkeypatch) + auths = set() + + def handler(req: httpx.Request) -> httpx.Response: + auths.add(req.headers.get("authorization")) + return httpx.Response(200) + + sender, client = _sender_with(handler, _test_key_pem()) + for _ in range(50): + assert await sender.send("devtoken", {"aps": {}}) is True + assert mints[0] == 1, f"expected 1 JWT mint across 50 pushes, got {mints[0]}" + assert len(auths) == 1 + await client.aclose() + + +@pytest.mark.asyncio +async def test_provider_token_refreshes_after_the_window(monkeypatch): + # The cached token must still be refreshed on a timer: Apple expires a + # provider token after an hour, so a long-lived process that never reminted + # would eventually push with a dead token. + from tinyagentos.push import apns as apns_mod + + mints = _counting_mint(monkeypatch) + clock = [1_700_000_000.0] + monkeypatch.setattr(apns_mod.time, "time", lambda: clock[0]) + + sender, client = _sender_with(lambda req: httpx.Response(200), _test_key_pem()) + await sender.send("devtoken", {"aps": {}}) + clock[0] += 10 * 60 + await sender.send("devtoken", {"aps": {}}) + assert mints[0] == 1, f"expected 1 mint 10 minutes in, got {mints[0]}" + + clock[0] += 55 * 60 + await sender.send("devtoken", {"aps": {}}) + assert mints[0] == 2, f"expected a refresh past the window, got {mints[0]} mints" + await client.aclose() + + +@pytest.mark.asyncio +async def test_410_raises_unregistered_carrying_reason_and_apns_id(): + # 410 Unregistered is Apple's permanent "this device token is dead" signal. + # Collapsing it into a plain False means the token is retried forever, so + # the sender must raise a distinguishable error the caller can prune on. + from tinyagentos.push.apns import ApnsUnregistered + + def handler(req: httpx.Request) -> httpx.Response: + return httpx.Response( + 410, + json={"reason": "Unregistered", "timestamp": 1700000000000}, + headers={"apns-id": "AAAA-BBBB"}, + ) + + sender, client = _sender_with(handler, _test_key_pem()) + with pytest.raises(ApnsUnregistered) as excinfo: + await sender.send("deadtoken", {"aps": {}}) + assert excinfo.value.reason == "Unregistered" + assert excinfo.value.apns_id == "AAAA-BBBB" + assert excinfo.value.push_token == "deadtoken" + await client.aclose() + + +@pytest.mark.asyncio +async def test_failure_reason_is_surfaced_in_logs(caplog): + # Neither apns-id nor Apple's own `reason` was ever logged, so a refusal was + # indistinguishable from any other non-200 and could not be diagnosed. + def handler(req: httpx.Request) -> httpx.Response: + return httpx.Response( + 410, json={"reason": "Unregistered"}, headers={"apns-id": "AAAA-BBBB"}, + ) + + sender, client = _sender_with(handler, _test_key_pem()) + with caplog.at_level("WARNING", logger="tinyagentos.push.apns"): + try: + await sender.send("deadtoken", {"aps": {}}) + except Exception: + pass + assert "Unregistered" in caplog.text + assert "AAAA-BBBB" in caplog.text + await client.aclose() + + +@pytest.mark.asyncio +async def test_non_410_refusal_logs_reason_and_returns_false(caplog): + # A retryable refusal (bad payload, bad topic) stays a plain False, but the + # reason must still reach the log. + def handler(req: httpx.Request) -> httpx.Response: + return httpx.Response(400, json={"reason": "BadDeviceToken"}) + + sender, client = _sender_with(handler, _test_key_pem()) + with caplog.at_level("WARNING", logger="tinyagentos.push.apns"): + assert await sender.send("devtoken", {"aps": {}}) is False + assert "BadDeviceToken" in caplog.text + await client.aclose() + + +@pytest.mark.asyncio +async def test_expired_provider_token_forces_a_remint(monkeypatch): + # Caching introduces a new failure mode: if the cached token expires early + # (clock skew), every push would be refused until the refresh timer fired. + # Apple's ExpiredProviderToken must therefore invalidate the cache at once. + mints = _counting_mint(monkeypatch) + statuses = [403, 200, 200] + + def handler(req: httpx.Request) -> httpx.Response: + if statuses.pop(0) == 403: + return httpx.Response(403, json={"reason": "ExpiredProviderToken"}) + return httpx.Response(200) + + sender, client = _sender_with(handler, _test_key_pem()) + assert await sender.send("devtoken", {"aps": {}}) is False + assert await sender.send("devtoken", {"aps": {}}) is True + assert await sender.send("devtoken", {"aps": {}}) is True + # Exactly one extra mint: the expiry invalidates the cache once, and the + # replacement token is then reused like any other. + assert mints[0] == 2, f"expected exactly one remint after ExpiredProviderToken, got {mints[0]}" + await client.aclose() diff --git a/tests/push/test_unifiedpush.py b/tests/push/test_unifiedpush.py index d4aef4c77..c79b4a3db 100644 --- a/tests/push/test_unifiedpush.py +++ b/tests/push/test_unifiedpush.py @@ -248,7 +248,7 @@ async def list_for_user(self, user_id): apns_sender=FakeApns(), up_sender=FakeUP(), ) - assert result == {"sent": 0, "failed": 0, "skipped": 0} + assert result == {"sent": 0, "failed": 0, "skipped": 0, "removed": 0} @pytest.mark.asyncio diff --git a/tests/test_notifications_push.py b/tests/test_notifications_push.py index 2e590c717..d734b37e9 100644 --- a/tests/test_notifications_push.py +++ b/tests/test_notifications_push.py @@ -686,7 +686,7 @@ async def list_for_user(self, user_id): apns_sender=FakeApns(), up_sender=FakeUP(), ) - assert result == {"sent": 0, "failed": 0, "skipped": 0} + assert result == {"sent": 0, "failed": 0, "skipped": 0, "removed": 0} async def test_no_push_token_device_is_skipped(self): class FakeApns: @@ -719,3 +719,91 @@ async def list_for_user(self, user_id): ) assert result["sent"] == 1 assert result["skipped"] == 1 + + # ----------------------------------------------------------------------- + # tsk-42q2qf: a 410 Unregistered must delete the dead device token + # ----------------------------------------------------------------------- + + async def test_apns_410_clears_the_dead_push_token(self, tmp_path): + # 410 Unregistered is permanent: the token must be dropped from the + # store, otherwise every later notification retries a dead device + # forever. Uses the real DeviceStore so the SQL is exercised. + from tinyagentos.device_store import DeviceStore + from tinyagentos.push.apns import ApnsUnregistered + + store = DeviceStore(tmp_path / "devices.db") + await store.init() + try: + dev = await store.register( + user_id="u1", platform="ios", push_token="deadtoken" + ) + + class FakeApns: + async def send(self, push_token, payload, *, topic=None): + raise ApnsUnregistered(push_token, apns_id="AAAA", reason="Unregistered") + + async def aclose(self): + pass + + class FakeUP: + async def send(self, *args, **kwargs): + return True + + async def aclose(self): + pass + + row = { + "id": 1, "title": "Hi", "message": "", "source": "system", + "user_id": "u1", "data": {}, + } + result = await send_device_push( + row, device_store=store, apns_sender=FakeApns(), up_sender=FakeUP(), + ) + assert result["removed"] == 1 + assert result["failed"] == 0 + after = await store.get(dev["device_id"]) + assert after is not None, "the device row itself must survive" + assert after["push_token"] == "", "expected the dead token removed from the store" + finally: + await store.close() + + async def test_apns_410_leaves_a_freshly_re_registered_token_alone(self, tmp_path): + # A device that re-registered between fan-out and the 410 response must + # keep its NEW token: the prune is scoped to the token that actually + # failed, so a live registration is not collateral damage. + from tinyagentos.device_store import DeviceStore + from tinyagentos.push.apns import ApnsUnregistered + + store = DeviceStore(tmp_path / "devices.db") + await store.init() + try: + dev = await store.register( + user_id="u1", platform="ios", push_token="oldtoken" + ) + + class FakeApns: + async def send(self, push_token, payload, *, topic=None): + await store.update_push_token(dev["device_id"], "freshtoken") + raise ApnsUnregistered(push_token, apns_id="AAAA", reason="Unregistered") + + async def aclose(self): + pass + + class FakeUP: + async def send(self, *args, **kwargs): + return True + + async def aclose(self): + pass + + row = { + "id": 1, "title": "Hi", "message": "", "source": "system", + "user_id": "u1", "data": {}, + } + await send_device_push( + row, device_store=store, apns_sender=FakeApns(), up_sender=FakeUP(), + ) + after = await store.get(dev["device_id"]) + assert after["push_token"] == "freshtoken" + finally: + await store.close() diff --git a/tinyagentos/device_store.py b/tinyagentos/device_store.py index c504d2abc..243e92029 100644 --- a/tinyagentos/device_store.py +++ b/tinyagentos/device_store.py @@ -110,6 +110,23 @@ async def update_push_token(self, device_id: str, push_token: str) -> dict | Non await self._db.commit() return await self.get(device_id) + async def clear_push_token(self, device_id: str, push_token: str) -> bool: + """Drop a push token the push service reported as permanently dead. + + Scoped to the exact token that failed, so a device that re-registered a + fresh token between the fan-out and the 410 response keeps the new one. + The device row itself is kept: the device is still paired and still + visible to its owner, it simply has no deliverable push token until it + registers another. Returns True when a row was cleared. + """ + assert self._db is not None + cur = await self._db.execute( + "UPDATE devices SET push_token = '' WHERE device_id = ? AND push_token = ?", + (device_id, push_token), + ) + await self._db.commit() + return cur.rowcount > 0 + async def touch(self, device_id: str) -> None: assert self._db is not None await self._db.execute( diff --git a/tinyagentos/notifications_push.py b/tinyagentos/notifications_push.py index f3e26adb1..da44177b3 100644 --- a/tinyagentos/notifications_push.py +++ b/tinyagentos/notifications_push.py @@ -19,7 +19,9 @@ * The whole send path is strictly best-effort: a missing VAPID key, no subscriptions, or any push error must never raise back into add(). * A 404/410 from the push service means the subscription is permanently gone, - so its row is pruned. + so its row is pruned. The device-push path does the same for APNs 410 + Unregistered: the dead push token is cleared from the device row (the row + itself stays, so the device remains paired and visible to its owner). * Secrets (auth, p256dh, private PEM) are never logged. """ from __future__ import annotations @@ -348,13 +350,30 @@ def _build_device_push_payload(row: dict) -> tuple[dict, list[dict] | None]: return {"title": title, "body": body, "data": payload_data}, actions +async def _clear_dead_push_token(device_store, device_id: str, push_token: str) -> None: + """Drop a push token the push service reported as permanently gone. + + Best-effort like the rest of the fan-out: a store failure is logged and the + send is still counted as removed, because the token is dead either way. + """ + if device_store is None or not device_id: + return + try: + await device_store.clear_push_token(device_id, push_token) + except Exception: # noqa: BLE001 - best-effort, never propagate + logger.warning("notif-push: failed to clear dead push token", exc_info=True) + + async def _send_one_device( device: dict, payload: dict, actions: list[dict] | None, apns_sender, up_sender, + device_store=None, ) -> str: + from tinyagentos.push.apns import ApnsUnregistered + platform = device.get("platform", "") push_token = device.get("push_token", "") if not push_token: @@ -379,6 +398,16 @@ async def _send_one_device( ok = await up_sender.send(push_token, up_payload) else: return "skipped" + except ApnsUnregistered as exc: + # 410 is permanent, not a retryable failure: keep counting it as such + # and the dead token is pushed to forever. Prune it instead, exactly as + # the web-push path prunes a 404/410 endpoint. + logger.info( + "notif-push: APNs token gone for device %s (reason=%s); clearing", + device.get("device_id"), exc.reason or "unknown", + ) + await _clear_dead_push_token(device_store, device.get("device_id", ""), push_token) + return "removed" except Exception: # noqa: BLE001 - best-effort, never propagate logger.warning("notif-push: device send failed for platform=%s token=%s", platform, push_token[:8], exc_info=True) return "failed" @@ -397,26 +426,32 @@ async def send_device_push( Looks up devices for ``row["user_id"]`` via ``device_store.list_for_user``. When ``user_id`` is None (broadcast), returns a no-op because device push is strictly per-user. For each device, dispatches to APNs or UnifiedPush - based on the ``platform`` column. Returns {"sent", "failed", "skipped"} - counts. Never raises. + based on the ``platform`` column. Returns {"sent", "failed", "skipped", + "removed"} counts, where "removed" is a device whose push token the service + reported as permanently gone and which was therefore pruned (the same shape + send_web_push reports). Never raises. """ + empty = {"sent": 0, "failed": 0, "skipped": 0, "removed": 0} user_id = row.get("user_id") if not user_id: - return {"sent": 0, "failed": 0, "skipped": 0} + return dict(empty) try: devices = await device_store.list_for_user(user_id) except Exception: # noqa: BLE001 - store read must never break add() logger.warning("notif-push: failed to list devices", exc_info=True) - return {"sent": 0, "failed": 0, "skipped": 0} + return dict(empty) if not devices: - return {"sent": 0, "failed": 0, "skipped": 0} + return dict(empty) payload, actions = _build_device_push_payload(row) results = await asyncio.gather( - *[_send_one_device(d, payload, actions, apns_sender, up_sender) for d in devices], + *[ + _send_one_device(d, payload, actions, apns_sender, up_sender, device_store) + for d in devices + ], return_exceptions=True, ) - sent = failed = skipped = 0 + sent = failed = skipped = removed = 0 for r in results: if isinstance(r, Exception): failed += 1 @@ -424,6 +459,8 @@ async def send_device_push( sent += 1 elif r == "skipped": skipped += 1 + elif r == "removed": + removed += 1 else: failed += 1 - return {"sent": sent, "failed": failed, "skipped": skipped} + return {"sent": sent, "failed": failed, "skipped": skipped, "removed": removed} diff --git a/tinyagentos/push/__init__.py b/tinyagentos/push/__init__.py index da5dbb398..cb1a1c25e 100644 --- a/tinyagentos/push/__init__.py +++ b/tinyagentos/push/__init__.py @@ -25,6 +25,12 @@ async def send_device_push( apns_sender: "ApnsSender", up_sender: "UnifiedPushSender", ) -> bool: + """Send one payload to one device. True when the push service accepted it. + + Propagates ApnsUnregistered: a 410 means the token is permanently dead, and + a single-device caller has to prune it rather than read it as a plain False. + The fan-out in notifications_push.send_device_push handles that pruning. + """ platform = device.get("platform", "") push_token = device.get("push_token", "") if not push_token: diff --git a/tinyagentos/push/apns.py b/tinyagentos/push/apns.py index e27db46fc..43867f738 100644 --- a/tinyagentos/push/apns.py +++ b/tinyagentos/push/apns.py @@ -13,13 +13,52 @@ logger = logging.getLogger(__name__) +# Apple caps provider-token GENERATION, not use: minting a fresh token per push +# earns 403 TooManyProviderTokenUpdates and refuses pushes account-wide. A token +# stays valid for an hour, so one cached token is reused and reminted after 50 +# minutes -- inside the validity window, and far under the generation cap. +_TOKEN_REFRESH_SECONDS = 50 * 60 + def _b64url(data: bytes) -> str: return base64.urlsafe_b64encode(data).rstrip(b"=").decode() +class ApnsUnregistered(Exception): + """APNs answered 410 Unregistered: this device token is permanently dead. + + Not a delivery failure to retry -- Apple is telling us the app was removed + or the token no longer belongs to this topic, so the caller must stop + pushing to it and drop it from the device store. Mirrors the 404/410 prune + the web-push path performs on WebPushException. + """ + + def __init__(self, push_token: str, *, apns_id: str | None = None, reason: str | None = None): + # The token is deliberately kept out of the message: it is a device + # identifier and this exception can reach a log formatter. + super().__init__(f"APNs 410 Unregistered (reason={reason or 'unknown'})") + self.push_token = push_token + self.apns_id = apns_id + self.reason = reason + + +def _apns_reason(resp: httpx.Response) -> str | None: + """Apple returns the failure cause as ``{"reason": "..."}`` in the body.""" + try: + body = resp.json() + except ValueError: + return None + return body.get("reason") if isinstance(body, dict) else None + + class ApnsSender(Protocol): async def send(self, push_token: str, payload: dict, *, topic: str | None = None) -> bool: + """True when APNs accepted the push, False for a retryable refusal. + + Raises ApnsUnregistered when APNs reports the token is permanently dead + (410), which the caller must handle by dropping the token rather than + counting it as another failed delivery. + """ ... async def aclose(self) -> None: @@ -82,12 +121,31 @@ def __init__( # Only close the client on aclose() if this sender created it; an # injected client is owned by the caller. self._owns_client = client is None + # Cached provider token, reused across pushes (see _provider_token). + self._jwt: str | None = None + self._jwt_minted_at = 0.0 + + def _provider_token(self, now: float) -> str: + """Return the cached provider token, reminting only when it is stale. + + Minting per push is what Apple refuses with TooManyProviderTokenUpdates, + so the token is cached and refreshed on a timer instead. A clock that + moves backwards also counts as stale, so a bad NTP step cannot pin a + token past its real expiry. No await runs between the staleness check + and the store, so concurrent senders on one event loop cannot interleave + into a double mint. + """ + age = now - self._jwt_minted_at + if self._jwt is None or not 0 <= age < _TOKEN_REFRESH_SECONDS: + self._jwt = build_apns_jwt( + key_pem=self._key_pem, key_id=self._key_id, + team_id=self._team_id, now=int(now), + ) + self._jwt_minted_at = now + return self._jwt async def send(self, push_token: str, payload: dict, *, topic: str | None = None) -> bool: - jwt = build_apns_jwt( - key_pem=self._key_pem, key_id=self._key_id, - team_id=self._team_id, now=int(time.time()), - ) + jwt = self._provider_token(time.time()) try: resp = await self._client.post( f"https://{self._host}/3/device/{push_token}", @@ -103,7 +161,26 @@ async def send(self, push_token: str, payload: dict, *, topic: str | None = None except httpx.HTTPError: logger.warning("APNs send failed for %s", push_token[:8], exc_info=True) return False - return resp.status_code == 200 + if resp.status_code == 200: + return True + # Every refusal carries Apple's own cause and a request id; without them + # a non-200 is undiagnosable, which is why they are logged before the + # status is turned into a return value. + apns_id = resp.headers.get("apns-id") + reason = _apns_reason(resp) + logger.warning( + "APNs push refused for %s: status=%s reason=%s apns-id=%s", + push_token[:8], resp.status_code, reason or "unknown", apns_id or "-", + ) + if resp.status_code == 410: + raise ApnsUnregistered(push_token, apns_id=apns_id, reason=reason) + if resp.status_code == 403 and reason == "ExpiredProviderToken": + # Caching a token introduces this failure mode: under clock skew the + # cached token can expire before the refresh timer fires, and every + # push would then be refused until it did. Drop it so the next send + # mints a replacement. + self._jwt = None + return False async def aclose(self) -> None: # Close the httpx client only if this sender created it; an injected From f6e88fc3f674ea2e7a5effc8914bd4e0d5815674 Mon Sep 17 00:00:00 2001 From: jaylfc Date: Sun, 6 Sep 2026 07:12:08 +0000 Subject: [PATCH 2/2] fold(#2801): invalidate the cache on InvalidProviderToken, floor iat against clock regression Two review findings on the token cache from the earlier merge: InvalidProviderToken (403) is just as permanent as ExpiredProviderToken -- a rotated signing key, or a cached token that is otherwise unparseable -- so it now also forces an immediate remint instead of refusing every push for the rest of the 50-minute cache window. A wall clock that steps backward (a bad NTP correction) previously let the new token's iat regress behind the last one this process actually used. Apple checks iat against its own correct clock, so a regressed iat combined with a full fresh cache window could let the cache keep reusing a token past Apple's real one-hour limit before the local refresh timer ever fired. iat is now floored at the previous iat so it freezes through a bad clock stretch rather than moving backward. Also drops a redundant dict(empty) copy in send_device_push's early-return paths (kilo-code-bot nit) and fixes a missing separator comment left by the origin/dev merge in tests/push/test_apns.py. Docs-Reviewed: fold pass, no installer/route change --- .../tsk-42q2qf-apns-token-reuse-and-410.md | 2 + tests/push/test_apns.py | 60 +++++++++++++++++++ tinyagentos/notifications_push.py | 6 +- tinyagentos/push/apns.py | 28 +++++++-- 4 files changed, 88 insertions(+), 8 deletions(-) diff --git a/changelog.d/tsk-42q2qf-apns-token-reuse-and-410.md b/changelog.d/tsk-42q2qf-apns-token-reuse-and-410.md index aa92f2b97..e0f4ed297 100644 --- a/changelog.d/tsk-42q2qf-apns-token-reuse-and-410.md +++ b/changelog.d/tsk-42q2qf-apns-token-reuse-and-410.md @@ -2,3 +2,5 @@ - Apple push no longer mints a fresh provider token for every notification: one token is now cached and reminted on a 50-minute timer, so a burst of notifications can no longer trip Apple's `TooManyProviderTokenUpdates` cap and get pushes refused for the whole account (tsk-42q2qf). - A `410 Unregistered` from Apple now clears the dead push token from the device instead of being reported as a generic delivery failure, so an uninstalled or re-provisioned device is no longer pushed to forever. The device itself stays paired and visible; it simply has no push token until it registers a new one. - Every refused push now logs Apple's own `reason` and the `apns-id`, so a refusal can be diagnosed instead of appearing as an unexplained non-delivery. An expired provider token also forces an immediate remint rather than waiting out the refresh timer. +- `InvalidProviderToken` (a rotated signing key, or an otherwise-unparseable cached token) now also forces an immediate remint, the same as `ExpiredProviderToken`, instead of refusing every push for the rest of the 50-minute cache window. +- The cached provider token's `iat` can no longer regress after a backward wall-clock step (a bad NTP correction): it is floored at the previous `iat`, so Apple's own clock cannot see the token as older than the real elapsed time and reject it before this cache's refresh timer would have fired. diff --git a/tests/push/test_apns.py b/tests/push/test_apns.py index 76e3a5247..c3c90b6b5 100644 --- a/tests/push/test_apns.py +++ b/tests/push/test_apns.py @@ -1,3 +1,4 @@ +import base64 import json import httpx import pytest @@ -141,6 +142,13 @@ def counted(**kwargs): return mints +def _decode_iat(jwt: str) -> int: + """Decode the `iat` claim out of a JWT built by build_apns_jwt.""" + payload_b64 = jwt.split(".")[1] + padded = payload_b64 + "=" * (-len(payload_b64) % 4) + return json.loads(base64.urlsafe_b64decode(padded))["iat"] + + def _sender_with(handler, pem: str): client = httpx.AsyncClient(transport=httpx.MockTransport(handler)) sender = HttpApnsSender( @@ -271,6 +279,58 @@ def handler(req: httpx.Request) -> httpx.Response: # replacement token is then reused like any other. assert mints[0] == 2, f"expected exactly one remint after ExpiredProviderToken, got {mints[0]}" await client.aclose() + + +@pytest.mark.asyncio +async def test_invalid_provider_token_forces_a_remint(monkeypatch): + # InvalidProviderToken is just as permanent as ExpiredProviderToken (a + # rotated signing key, or a cached token that is otherwise unparseable): + # every push would be refused for the rest of the 50-minute cache window + # unless this also invalidates the cache immediately. + mints = _counting_mint(monkeypatch) + statuses = [403, 200, 200] + + def handler(req: httpx.Request) -> httpx.Response: + if statuses.pop(0) == 403: + return httpx.Response(403, json={"reason": "InvalidProviderToken"}) + return httpx.Response(200) + + sender, client = _sender_with(handler, _test_key_pem()) + assert await sender.send("devtoken", {"aps": {}}) is False + assert await sender.send("devtoken", {"aps": {}}) is True + assert await sender.send("devtoken", {"aps": {}}) is True + assert mints[0] == 2, f"expected exactly one remint after InvalidProviderToken, got {mints[0]}" + await client.aclose() + + +@pytest.mark.asyncio +async def test_provider_token_iat_never_regresses_after_backward_clock_step(monkeypatch): + # A wall clock that steps backward (a bad NTP correction) must not pin the + # next token's iat to the regressed time. Apple checks iat against its OWN + # correct clock: a regressed iat combined with a full fresh 50-minute local + # cache window can let this cache keep reusing the token until Apple's real + # elapsed-since-iat time is already past the true one-hour limit, well + # before the local refresh timer would ever fire. + from tinyagentos.push import apns as apns_mod + + clock = [1_700_000_000.0] + monkeypatch.setattr(apns_mod.time, "time", lambda: clock[0]) + + sender, client = _sender_with(lambda req: httpx.Response(200), _test_key_pem()) + await sender.send("devtoken", {"aps": {}}) + iat1 = _decode_iat(sender._jwt) + assert iat1 == int(clock[0]) + + # The clock steps backward by 15 minutes and never corrects (a permanent + # skew), forcing an immediate remint (age goes negative). + clock[0] -= 15 * 60 + await sender.send("devtoken", {"aps": {}}) + iat2 = _decode_iat(sender._jwt) + assert iat2 >= iat1, f"iat regressed from {iat1} to {iat2} after a backward clock step" + await client.aclose() + + +# --------------------------------------------------------------------------- # tsk-cf7wzc: image + actions wiring for the native decision shell # --------------------------------------------------------------------------- diff --git a/tinyagentos/notifications_push.py b/tinyagentos/notifications_push.py index b04142422..bdffff65f 100644 --- a/tinyagentos/notifications_push.py +++ b/tinyagentos/notifications_push.py @@ -473,14 +473,14 @@ async def send_device_push( empty = {"sent": 0, "failed": 0, "skipped": 0, "removed": 0} user_id = row.get("user_id") if not user_id: - return dict(empty) + return empty try: devices = await device_store.list_for_user(user_id) except Exception: # noqa: BLE001 - store read must never break add() logger.warning("notif-push: failed to list devices", exc_info=True) - return dict(empty) + return empty if not devices: - return dict(empty) + return empty payload, actions = _build_device_push_payload(row) results = await asyncio.gather( diff --git a/tinyagentos/push/apns.py b/tinyagentos/push/apns.py index 6e9fad8b7..52c1c6646 100644 --- a/tinyagentos/push/apns.py +++ b/tinyagentos/push/apns.py @@ -171,6 +171,9 @@ def __init__( # Cached provider token, reused across pushes (see _provider_token). self._jwt: str | None = None self._jwt_minted_at = 0.0 + # The iat actually embedded in the cached token; floors every future + # iat so a regressed wall clock cannot pin a new token behind it. + self._jwt_iat = 0 def _provider_token(self, now: float) -> str: """Return the cached provider token, reminting only when it is stale. @@ -184,10 +187,21 @@ def _provider_token(self, now: float) -> str: """ age = now - self._jwt_minted_at if self._jwt is None or not 0 <= age < _TOKEN_REFRESH_SECONDS: + # iat must never regress: Apple checks it against its OWN correct + # clock, so a wall clock that steps backward (a bad NTP correction) + # must not pin the new token's iat earlier than the last one this + # process actually used. Flooring at the previous iat freezes the + # value through the bad stretch instead of moving it backward -- + # without this, a regressed iat combined with a full fresh + # cache window can let this cache keep reusing the token until + # Apple's real elapsed-since-iat time is already past the true + # one-hour limit, well before this cache's own refresh timer fires. + new_iat = max(int(now), self._jwt_iat) self._jwt = build_apns_jwt( key_pem=self._key_pem, key_id=self._key_id, - team_id=self._team_id, now=int(now), + team_id=self._team_id, now=new_iat, ) + self._jwt_iat = new_iat self._jwt_minted_at = now return self._jwt @@ -221,11 +235,15 @@ async def send(self, push_token: str, payload: dict, *, topic: str | None = None ) if resp.status_code == 410: raise ApnsUnregistered(push_token, apns_id=apns_id, reason=reason) - if resp.status_code == 403 and reason == "ExpiredProviderToken": + if resp.status_code == 403 and reason in ("ExpiredProviderToken", "InvalidProviderToken"): # Caching a token introduces this failure mode: under clock skew the - # cached token can expire before the refresh timer fires, and every - # push would then be refused until it did. Drop it so the next send - # mints a replacement. + # cached token can expire (ExpiredProviderToken) before the refresh + # timer fires, and every push would then be refused until it did. + # InvalidProviderToken is just as permanent -- a rotated signing + # key, or a cached token that is otherwise unparseable -- so it + # gets the same immediate cache drop rather than waiting out the + # rest of the refresh window. Drop it so the next send mints a + # replacement. self._jwt = None return False