From d7458cf70f3009ccf28129c5d4c1e39e8141a279 Mon Sep 17 00:00:00 2001
From: devswha <devswha@gmail.com>
Date: Wed, 28 Oct 2026 11:16:44 +0900
Subject: [PATCH 1/3] fix(push): keep a retired endpoint refused past 30 days
 and under another spelling, and forget it when its files cannot be written

A retired endpoint's mark expired after 30 days, after which any device that knew the
endpoint could register it with keys of its own, and another spelling of the endpoint (a
fragment, an uppercase host, :443, dot segments) was never matched against the mark at all.
The mark now has no time limit (only RETIRED_LIMIT bounds the list, and the owner's
revocation removes it), and endpoints are stored, compared and retired in one form: the
WHATWG serialization without the fragment. A query string and percent-encoding are kept, as
a push service (WNS) may tell them apart. Marks and delivery history written before this
under the spelling handed over are still found by that spelling, and the state merge
compares the normalized forms too (#2012).

A push answered 404/410 wrote push-retired.json before removing the subscription, so a
failed write threw and kept the dead subscription registered. That write is telemetry and
no longer throws there, nor in a revocation; and when push-subscriptions.json itself cannot
be written, the retired subscription is dropped in memory until a write succeeds (#2013).

Closes #2012
Closes #2013
---
 changelog.d/push-retired-endpoint-checks.md |   8 ++
 server/push.test.ts                         | 140 ++++++++++++++++----
 server/push.ts                              |  93 +++++++++----
 server/state-merge.test.ts                  |  13 ++
 server/state-merge.ts                       |   9 +-
 5 files changed, 206 insertions(+), 57 deletions(-)
 create mode 100644 changelog.d/push-retired-endpoint-checks.md

diff --git a/changelog.d/push-retired-endpoint-checks.md b/changelog.d/push-retired-endpoint-checks.md
new file mode 100644
index 0000000000..b9cf2f994e
--- /dev/null
+++ b/changelog.d/push-retired-endpoint-checks.md
@@ -0,0 +1,8 @@
+### Fixed
+- A push endpoint its push service retired stays refused, however long ago that was, until its
+  owner's device is revoked. Before, after 30 days another device that knew the endpoint could
+  register it as its own, and so could another spelling of it (a `#fragment`, an uppercase host or
+  `:443`) at any time.
+- A subscription its push service retired is removed even when `push-retired.json` cannot be
+  written (a full disk, a read-only state directory). Before, the failed write left it registered
+  and the next alert went to the dead endpoint again.
diff --git a/server/push.test.ts b/server/push.test.ts
index f0c5c93a33..14abfa97db 100644
--- a/server/push.test.ts
+++ b/server/push.test.ts
@@ -345,23 +345,96 @@ describe("push delivery", () => {
     expect((await post("/api/push/subscribe", { subscription: other }))?.status).toBe(204);
   });
 
-  it("forgets a retired endpoint after its time to live and ignores a malformed file", async () => {
+  // regression (fails on main): 30 days after the retirement the mark expired, and a watch device registered the owner's endpoint (#2012)
+  it("never hands a retired endpoint to another device, however long ago it was retired, and ignores a malformed file", async () => {
     let clock = 1_000_000;
     const push = createPushService({ loopbackHttp: true, stateDir, now: () => clock });
-    push.subscribe(fake.subscription);
+    push.subscribe(fake.subscription, undefined, "owner");
     fake.answerWith(410);
-    await push.sendTest(fake.subscription.endpoint);
-    expect(push.retired(fake.subscription.endpoint)).toBe(true);
-    // control: inside the time to live it is still remembered (read first: a load past it rewrites the file without it)
-    expect(createPushService({ loopbackHttp: true, stateDir, now: () => 1_000_000 + 1000 }).retired(fake.subscription.endpoint)).toBe(true);
+    expect(await push.sendTest(fake.subscription.endpoint)).toEqual({ ok: false, status: 410, gone: true });
     clock += 31 * 24 * 60 * 60 * 1000;
-    expect(createPushService({ loopbackHttp: true, stateDir, now: () => clock }).retired(fake.subscription.endpoint)).toBe(false);
-    expect(readFileSync(join(stateDir, "push-retired.json"), "utf8")).not.toContain("hash");
+    // the watch device holds the endpoint and keys of its own
+    const theirs = { endpoint: fake.subscription.endpoint, keys: { p256dh: Buffer.alloc(65, 4).toString("base64url"), auth: Buffer.alloc(16, 7).toString("base64url") } };
+    const subscribeAs = (service: PushService) => handlePushRequest(new Request("http://localhost/api/push/subscribe", {
+      method: "POST", headers: { "content-type": "application/json" }, body: JSON.stringify({ subscription: theirs }),
+    }), "/api/push/subscribe", service, "watch");
+    for (const service of [push, createPushService({ loopbackHttp: true, stateDir, now: () => clock })]) {
+      const response = (await subscribeAs(service))!;
+      expect(response.status).toBe(410);
+      expect(((await response.json()) as { error: { code: string } }).error.code).toBe("subscription_gone");
+    }
+    expect(JSON.parse(readFileSync(join(stateDir, "push-subscriptions.json"), "utf8"))).toEqual([]);
+    expect(readFileSync(join(stateDir, "push-retired.json"), "utf8")).toContain("owner");
     // a malformed file is ignored, never fatal
     writeFileSync(join(stateDir, "push-retired.json"), "not json");
     expect(createPushService({ loopbackHttp: true, stateDir }).retired(fake.subscription.endpoint)).toBe(false);
   });
 
+  // regression: a mark or a history written before endpoints were normalized is the hash of the spelling handed over
+  it("keeps refusing, and forgets on goodbye, an endpoint retired before endpoints were normalized", async () => {
+    const legacy = `${fake.subscription.endpoint}#legacy`;
+    const raw = createHash("sha256").update(legacy).digest("base64url");
+    writeFileSync(join(stateDir, "push-retired.json"), JSON.stringify([{ hash: raw, at: 1, owner: "owner" }]));
+    writeFileSync(join(stateDir, "push-delivery.json"), JSON.stringify([{ hash: raw, owner: "owner", keys: "k", success_at: null, failure_at: Date.now(), failure: "gone", at: Date.now() }]));
+    const push = createPushService({ loopbackHttp: true, stateDir });
+    const request = (method: string, path: string, body: unknown) => handlePushRequest(new Request(`http://localhost${path}`, {
+      method, headers: { "content-type": "application/json" }, body: JSON.stringify(body),
+    }), path, push, "watch");
+    expect((await request("POST", "/api/push/subscribe", { subscription: { ...fake.subscription, endpoint: legacy } }))?.status).toBe(410);
+    expect((await request("POST", "/api/push/test", { endpoint: legacy }))?.status).toBe(410);
+    expect(existsSync(join(stateDir, "push-subscriptions.json"))).toBe(false);
+    expect((await request("DELETE", "/api/push/subscribe", { endpoint: legacy }))?.status).toBe(204);
+    expect(JSON.parse(readFileSync(join(stateDir, "push-delivery.json"), "utf8"))).toEqual([]);
+  });
+
+  // regression: a subscription file that cannot be written kept the retired subscription, and the next push went to it again
+  it("stops pushing to a retired subscription when the subscription file cannot be written", async () => {
+    const push = createPushService({ loopbackHttp: true, stateDir });
+    push.subscribe(fake.subscription, undefined, null);
+    push.publicKey(); // the key pair is on disk before the disk fills
+    fake.answerWith(410);
+    const rename = spyOn(privateStateFs, "renameSync").mockImplementation(() => { throw Object.assign(new Error("ENOSPC: no space left on device"), { code: "ENOSPC" }); });
+    const quiet = spyOn(console, "error").mockImplementation(() => undefined);
+    try {
+      expect(await push.sendTest(fake.subscription.endpoint)).toEqual({ ok: false, status: 410, gone: true });
+      expect(await push.sendTest(fake.subscription.endpoint)).toBeNull();
+      expect(fake.received).toHaveLength(0);
+      expect(quiet.mock.calls.some(([line]) => String(line).includes("push-subscriptions.json"))).toBe(true);
+    } finally { rename.mockRestore(); quiet.mockRestore(); }
+    // the device's goodbye, though nothing of it is left in memory, writes the file without it
+    push.unsubscribe(fake.subscription.endpoint);
+    expect(JSON.parse(readFileSync(join(stateDir, "push-subscriptions.json"), "utf8"))).toEqual([]);
+    expect(await createPushService({ loopbackHttp: true, stateDir }).sendTest(fake.subscription.endpoint)).toBeNull();
+    // the next write that succeeds leaves it off disk too
+    push.subscribe({ ...fake.subscription, endpoint: `${fake.subscription.endpoint}-other` }, undefined, null);
+    expect((JSON.parse(readFileSync(join(stateDir, "push-subscriptions.json"), "utf8")) as { endpoint: string }[]).map((entry) => entry.endpoint)).toEqual([`${fake.subscription.endpoint}-other`]);
+  });
+
+  it("stores an endpoint in one form, the URL without its fragment", () => {
+    const { keys } = fake.subscription;
+    expect(parseSubscription({ endpoint: "https://PUSH.Example:443/wpush/./v2/x#attacker", keys })?.endpoint).toBe("https://push.example/wpush/v2/x");
+    expect(parseSubscription({ endpoint: "https://push.example/wpush/v2/x", keys })?.endpoint).toBe("https://push.example/wpush/v2/x");
+  });
+
+  // regression (fails on main): a fragment, or any other spelling of the same URL, got past the retired check (#2012)
+  it("refuses another spelling of a retired endpoint as that endpoint", async () => {
+    const push = subscribed();
+    fake.answerWith(410);
+    expect(await push.sendTest(fake.subscription.endpoint)).toEqual({ ok: false, status: 410, gone: true });
+    const post = (path: string, body: unknown) => handlePushRequest(new Request(`http://localhost${path}`, {
+      method: "POST", headers: { "content-type": "application/json" }, body: JSON.stringify(body),
+    }), path, push, "watch");
+    const url = new URL(fake.subscription.endpoint);
+    const variants = [`${fake.subscription.endpoint}#attacker`, `HTTP://${url.host}/push/x/..${url.pathname.slice("/push".length)}`];
+    for (const endpoint of variants) {
+      const again = (await post("/api/push/subscribe", { subscription: { ...fake.subscription, endpoint } }))!;
+      expect(again.status).toBe(410);
+      const tested = (await post("/api/push/test", { endpoint }))!;
+      expect(tested.status).toBe(410);
+    }
+    expect(JSON.parse(readFileSync(join(stateDir, "push-subscriptions.json"), "utf8"))).toEqual([]);
+  });
+
   it("keeps a device through a failure that is not a goodbye", async () => {
     const push = subscribed();
     fake.answerWith(500);
@@ -746,7 +819,7 @@ describe("push delivery health (#1039)", () => {
     const response = (await statusRequest(restarted, fake.subscription, null))!;
     expect(response.status).toBe(200);
     expect(await response.json()).toEqual(expected);
-    // forgotten after its time to live, like the retired endpoint
+    // forgotten after its time to live (the retired mark is not: #2012)
     expect(createPushService({ loopbackHttp: true, stateDir, now: () => clock + 31 * 24 * 60 * 60 * 1000 }).status(fake.subscription, null)).toBeNull();
     // an explicit unsubscribe removes it
     restarted.unsubscribe(fake.subscription.endpoint);
@@ -754,7 +827,7 @@ describe("push delivery health (#1039)", () => {
     expect(JSON.parse(readFileSync(path, "utf8"))).toEqual([]);
   });
 
-  it("expires on the running service; a goodbye takes the history, and only a revocation also the retired mark", async () => {
+  it("expires the history on the running service, never the retired mark; a goodbye takes the history, and only a revocation also the mark", async () => {
     let clock = 1_700_000_000_000;
     const push = createPushService({ loopbackHttp: true, stateDir, now: () => clock });
     const endpoint = fake.subscription.endpoint;
@@ -765,13 +838,13 @@ describe("push delivery health (#1039)", () => {
       expect(push.status(fake.subscription, owner)?.last_failure).toBe("gone");
       expect(push.retired(endpoint)).toBe(true);
     };
-    // 31 days on, without a restart: the history and the retired mark are gone, on disk too
+    // 31 days on, without a restart: the history is gone, on disk too, and the retired mark stays (#2012)
     await retire(null);
     clock += 31 * 24 * 60 * 60 * 1000;
     expect(push.status(fake.subscription, null)).toBeNull();
-    expect(push.retired(endpoint)).toBe(false);
+    expect(push.retired(endpoint)).toBe(true);
     expect(JSON.parse(readFileSync(join(stateDir, "push-delivery.json"), "utf8"))).toEqual([]);
-    expect(JSON.parse(readFileSync(join(stateDir, "push-retired.json"), "utf8"))).toEqual([]);
+    expect(JSON.parse(readFileSync(join(stateDir, "push-retired.json"), "utf8"))).toHaveLength(1);
     // an explicit unsubscribe forgets the history, and keeps the mark: it proves neither owner nor keys
     await retire(null);
     push.unsubscribe(endpoint);
@@ -839,6 +912,23 @@ describe("push delivery health (#1039)", () => {
     expect(createPushService({ loopbackHttp: true, stateDir, now: () => clock }).retired(endpoint)).toBe(true);
   });
 
+  it("revocation takes the device's history and marks when the retired list cannot be written", async () => {
+    const push = createPushService({ loopbackHttp: true, stateDir });
+    push.subscribe(fake.subscription, undefined, "device-a");
+    fake.answerWith(410);
+    expect(await push.sendTest(fake.subscription.endpoint)).toEqual({ ok: false, status: 410, gone: true });
+    // the list cannot be replaced any more: a directory stands at its path
+    rmSync(join(stateDir, "push-retired.json"));
+    mkdirSync(join(stateDir, "push-retired.json"));
+    const quiet = spyOn(console, "error").mockImplementation(() => undefined);
+    try {
+      push.revokeDevice("device-a");
+      expect(push.retired(fake.subscription.endpoint)).toBe(false);
+      expect(push.status(fake.subscription, "device-a")).toBeNull();
+      expect(JSON.parse(readFileSync(join(stateDir, "push-delivery.json"), "utf8"))).toEqual([]);
+    } finally { quiet.mockRestore(); }
+  });
+
   it("foreign and unknown health are indistinguishable", async () => {
     const push = createPushService({ loopbackHttp: true, stateDir });
     push.subscribe(fake.subscription, undefined, "device-a");
@@ -977,24 +1067,24 @@ describe("push delivery health (#1039)", () => {
     expect(hashes("push-retired.json")).toEqual(["h-new"]);
   });
 
-  it("answers a subscribe when the retired list's expiry cannot be written", async () => {
-    let clock = 1_700_000_000_000;
-    const push = createPushService({ loopbackHttp: true, stateDir, now: () => clock });
-    push.subscribe(fake.subscription, undefined, null);
-    fake.answerWith(410);
-    expect(await push.sendTest(fake.subscription.endpoint)).toEqual({ ok: false, status: 410, gone: true });
-    // the list cannot be replaced any more: a directory stands at its path
-    rmSync(join(stateDir, "push-retired.json"));
+  // regression (fails on main): the failed write threw before the subscription was removed, and the next push went to it again (#2013)
+  it("forgets a retired subscription when the retired list cannot be written, and keeps the mark in memory", async () => {
+    // the list cannot be written: a directory stands at its path
     mkdirSync(join(stateDir, "push-retired.json"));
-    clock += 31 * 24 * 60 * 60 * 1000;
     const quiet = spyOn(console, "error").mockImplementation(() => undefined);
     try {
-      // the expired mark is dropped in memory, and the subscribe that reads the list is answered
+      const push = createPushService({ loopbackHttp: true, stateDir });
+      push.subscribe(fake.subscription, undefined, null);
+      fake.answerWith(410);
+      expect(await push.sendTest(fake.subscription.endpoint)).toEqual({ ok: false, status: 410, gone: true });
+      expect(JSON.parse(readFileSync(join(stateDir, "push-subscriptions.json"), "utf8"))).toEqual([]);
+      expect(await push.sendTest(fake.subscription.endpoint)).toBeNull();
+      expect(push.retired(fake.subscription.endpoint)).toBe(true);
+      // the browser handing it over again is still told to subscribe anew
       const response = await handlePushRequest(new Request("http://localhost/api/push/subscribe", {
         method: "POST", headers: { "content-type": "application/json" }, body: JSON.stringify({ subscription: fake.subscription }),
       }), "/api/push/subscribe", push, null);
-      expect(response?.status).toBe(204);
-      expect(push.retired(fake.subscription.endpoint)).toBe(false);
+      expect(response?.status).toBe(410);
       expect(quiet.mock.calls.some(([line]) => String(line).includes("push-retired.json"))).toBe(true);
     } finally { quiet.mockRestore(); }
   });
diff --git a/server/push.ts b/server/push.ts
index d61f2221da..24b94521dd 100644
--- a/server/push.ts
+++ b/server/push.ts
@@ -9,9 +9,11 @@
  * - push-subscriptions.json: survives restarts, so a reboot does not unsubscribe phones.
  *   A 404/410 from a push service means the device dropped it; the record is deleted.
  * - push-retired.json: the endpoint is also remembered, as a hash with the owner it was retired for
- *   (`RETIRED_LIMIT`, `RETIRED_TTL_MS`), so a restart does not forget it and a revocation finds it: a browser can keep handing over a subscription its push
+ *   (`RETIRED_LIMIT`), so a restart does not forget it and a revocation finds it: a browser can keep handing over a subscription its push
  *   service retired, and registering it again would only fail the next push, so it is refused as
- *   `subscription_gone` and the device subscribes anew (#1038).
+ *   `subscription_gone` and the device subscribes anew (#1038). A mark has no time limit: one that
+ *   expired would let any device that knows the endpoint register it as its own (#2012). Only its
+ *   owner's revocation removes it, or a thousand later retirements.
  * - push-keys.json (PUSH_EXTRA_KEYS): key pairs other than vapid.json's, each with the endpoints
  *   made for it. Only state-merge.ts writes it, for subscriptions taken over from the other state
  *   dir of an install that had two: each is signed with the key it was made for until its device
@@ -20,7 +22,7 @@
  *   `last_failure_at`, `last_failure`), by endpoint hash, with the owner and a hash of the keys it
  *   was recorded for. A device reads its own record through POST /api/push/status once it proves
  *   both, so a subscription its push service retired can still say why alerts stopped (#1039).
- *   Bounded like the retired list, and expired on every read and write; an explicit unsubscribe or
+ *   Bounded like the retired list, and expired (`DELIVERY_TTL_MS`) on every read and write; an explicit unsubscribe or
  *   the device's revocation removes it together with the endpoint's retired mark.
  *
  * Requests are built by web-push's generateRequestDetails (RFC 8291 aes128gcm + RFC
@@ -73,15 +75,13 @@ const PUSH_TIMEOUT_MS = 10_000;
 const DEFAULT_SUBJECT = "https://github.com/devswha/ovis";
 /** How many retired endpoints are remembered; the oldest is forgotten first. */
 const RETIRED_LIMIT = 1000;
-/** How long a retired endpoint is remembered: a browser that kept it this long has long since moved on. */
-const RETIRED_TTL_MS = 30 * 24 * 60 * 60 * 1000;
 /** The key pairs, besides vapid.json's, that subscriptions taken over from another state dir were made for. */
 export const PUSH_EXTRA_KEYS = "push-keys.json";
 
 type VapidKeys = { publicKey: string; privateKey: string };
-/** The delivery records are bounded the same way: as many, for as long since their last write. */
+/** The delivery records are bounded the same way, and kept for as long since their last write. */
 const DELIVERY_LIMIT = RETIRED_LIMIT;
-const DELIVERY_TTL_MS = RETIRED_TTL_MS;
+const DELIVERY_TTL_MS = 30 * 24 * 60 * 60 * 1000;
 
 /** How long an alert waits for the pane to change first, and the turn a finish is worth telling. */
 export interface AlertTiming {
@@ -152,7 +152,7 @@ export interface PushService {
   subscribe(subscription: PushSubscriptionRecord, alerts?: AlertPrefs, deviceId?: string | null, browserKeys?: readonly string[]): void;
   revokeDevice(id: string): void;
   unsubscribe(endpoint: string): void;
-  /** whether a push service answered 404/410 for this endpoint since the server started */
+  /** whether a push service answered 404/410 for this endpoint (`endpoint` as normalizeEndpoint gives it) */
   retired(endpoint: string): boolean;
   /** One confirmation push to one device, so enabling alerts proves the whole path works. */
   sendTest(endpoint: string): Promise<PushDelivery | null>;
@@ -231,6 +231,23 @@ function decodedLength(value: unknown): number {
   return Buffer.from(value, "base64url").length;
 }
 
+/**
+ * The one form an endpoint is stored, compared and retired in: the URL as WHATWG serializes it (host
+ * in lowercase, no default port, no dot segments), without its fragment, which never reaches the push
+ * service. Those spellings of a retired endpoint are then that same endpoint (#2012); a query string,
+ * percent-encoding and a trailing slash are kept as they are, since a push service may tell them apart. What does not
+ * parse is returned as it is.
+ */
+export function normalizeEndpoint(endpoint: string): string {
+  try {
+    const url = new URL(endpoint);
+    url.hash = "";
+    return url.href;
+  } catch {
+    return endpoint;
+  }
+}
+
 /** The PushSubscription JSON a browser produces, checked before anything is stored. */
 export function parseSubscription(value: unknown, options: { loopbackHttp?: boolean } = {}): PushSubscriptionRecord | null {
   if (typeof value !== "object" || value === null) return null;
@@ -252,7 +269,7 @@ export function parseSubscription(value: unknown, options: { loopbackHttp?: bool
   }
   // p256dh is an uncompressed P-256 point (65 bytes), auth a 16-byte secret
   if (decodedLength(keys?.p256dh) !== 65 || decodedLength(keys?.auth) !== 16) return null;
-  return { endpoint, keys: { p256dh: keys!.p256dh as string, auth: keys!.auth as string } };
+  return { endpoint: normalizeEndpoint(endpoint), keys: { p256dh: keys!.p256dh as string, auth: keys!.auth as string } };
 }
 
 export function statusMessage(paneId: string, title: string, status: AgentStatus): PushPayload {
@@ -307,7 +324,8 @@ export function createPushService(options: PushServiceOptions): PushService {
         for (const entry of Array.isArray(stored?.keys) ? stored.keys as unknown[] : []) {
           const { publicKey, privateKey, endpoints } = (entry ?? {}) as { publicKey?: unknown; privateKey?: unknown; endpoints?: unknown };
           if (typeof publicKey !== "string" || typeof privateKey !== "string" || !Array.isArray(endpoints)) continue;
-          for (const at of endpoints) if (typeof at === "string") extraKeys.set(at, { publicKey, privateKey });
+          // the subscriptions are loaded normalized (parseSubscription): so are the endpoints they are looked up by
+          for (const at of endpoints) if (typeof at === "string") extraKeys.set(normalizeEndpoint(at), { publicKey, privateKey });
         }
       } catch (error) {
         // those subscriptions are then signed with vapid.json's key, which their push service refuses
@@ -372,15 +390,15 @@ export function createPushService(options: PushServiceOptions): PushService {
   }
 
   /**
-   * Writes the list back without what expired or was dropped. Never throws: a read of the list (a
-   * subscribe's retired check) must not fail on it. What was dropped stays dropped in memory, and the
-   * next write that succeeds leaves it off disk too.
+   * Writes the list back. Never throws: neither a read of the list (a subscribe's retired check) nor a
+   * retirement may fail on it, as the list is telemetry (#2013). The list stays as it is in memory, and
+   * the next write that succeeds puts it on disk.
    */
   function rewriteRetired(): void {
     try {
       persistRetired();
     } catch (error) {
-      console.error(`Cannot write ${retiredPath} (${(error as NodeJS.ErrnoException).code ?? "unknown error"}); its expired entries stay on disk until the next write`);
+      console.error(`Cannot write ${retiredPath} (${(error as NodeJS.ErrnoException).code ?? "unknown error"}); retired endpoints are kept in memory until a write succeeds`);
     }
   }
 
@@ -389,10 +407,7 @@ export function createPushService(options: PushServiceOptions): PushService {
   }
 
   function retiredStore(): Map<string, RetiredMark> {
-    if (retiredEndpoints) {
-      if (expire(retiredEndpoints, (mark) => mark.at, RETIRED_TTL_MS)) rewriteRetired();
-      return retiredEndpoints;
-    }
+    if (retiredEndpoints) return retiredEndpoints;
     retiredEndpoints = new Map();
     let stored: unknown;
     try {
@@ -401,13 +416,12 @@ export function createPushService(options: PushServiceOptions): PushService {
       return retiredEndpoints; // an unreadable file only means a retired endpoint registers again once
     }
     if (!Array.isArray(stored)) return retiredEndpoints;
-    const cutoff = (options.now ?? Date.now)() - RETIRED_TTL_MS;
     for (const entry of stored.slice(-RETIRED_LIMIT)) {
       const { hash, at, owner } = (entry ?? {}) as { hash?: unknown; at?: unknown; owner?: unknown };
-      if (typeof hash !== "string" || typeof at !== "number" || at <= cutoff) continue;
+      if (typeof hash !== "string" || typeof at !== "number") continue;
       retiredEndpoints.set(hash, { at, ...(owner === null || typeof owner === "string" ? { owner } : {}) });
     }
-    // what was dropped (expired, over the bound, malformed) leaves the file too: a revocation later finds
+    // what was dropped (over the bound, malformed) leaves the file too: a revocation later finds
     // nothing of it to remove, and the device's marks must not stay on disk
     if (retiredEndpoints.size !== stored.length) {
       rewriteRetired();
@@ -419,7 +433,10 @@ export function createPushService(options: PushServiceOptions): PushService {
   function persist(next: Map<string, PushSubscriptionRecord>): void {
     writePrivateState(subscriptionsPath, [...next.values()]);
     subscriptions = next;
+    stale = false;
   }
+  /** the file still lists a subscription dropped in memory only (a retirement whose write failed): the next goodbye writes it */
+  let stale = false;
 
   function without(remove: (endpoint: string, subscription: PushSubscriptionRecord) => boolean): Map<string, PushSubscriptionRecord> | null {
     const next = new Map([...store()].filter(([endpoint, subscription]) => !remove(endpoint, subscription)));
@@ -558,9 +575,19 @@ export function createPushService(options: PushServiceOptions): PushService {
       retired.delete(identity.hash);
       retired.set(identity.hash, { at: (options.now ?? Date.now)(), ...(identity.owner === undefined ? {} : { owner: identity.owner }) });
       if (retired.size > RETIRED_LIMIT) retired.delete(retired.keys().next().value!);
-      persistRetired();
+      // the list is telemetry: a write that fails keeps the mark in memory, and never keeps the dead subscription (#2013)
+      rewriteRetired();
       const next = without((endpoint) => endpoint === subscription.endpoint);
-      if (next) persist(next);
+      if (next) {
+        try {
+          persist(next);
+        } catch (error) {
+          // the push service said it is gone: nothing is sent to it again, and the next write that succeeds leaves it off disk
+          subscriptions = next;
+          stale = true;
+          console.error(`Cannot write ${subscriptionsPath} (${(error as NodeJS.ErrnoException).code ?? "unknown error"}); the retired subscription is dropped in memory until a write succeeds`);
+        }
+      }
     } else {
       console.error(`web push to ${new URL(subscription.endpoint).host} failed: ${status ?? "no response"}`);
     }
@@ -686,17 +713,19 @@ export function createPushService(options: PushServiceOptions): PushService {
         retired.delete(hash);
         unmarked = true;
       }
-      if (unmarked) persistRetired();
+      // a write that fails must not keep the device's history: the next write that succeeds drops its marks from disk
+      if (unmarked) rewriteRetired();
       forgetDeliveries((record) => record.owner !== id);
     },
 
     unsubscribe(endpoint) {
       const next = without((key) => key === endpoint);
       if (next) persist(next);
+      else if (stale) persist(new Map(store()));
       const hash = endpointHash(endpoint);
       // an explicit goodbye takes the endpoint's history with it, but never its retired mark: this route proves
       // neither owner nor keys, and a mark it cleared would let anyone register a retired endpoint again with
-      // keys of their own (#1038). The mark goes with its time to live, or with its owner's revocation
+      // keys of their own (#1038). The mark goes only with its owner's revocation
       forgetDeliveries((record) => record.hash !== hash);
     },
 
@@ -881,7 +910,9 @@ export async function handlePushRequest(request: Request, pathname: string, push
   if (route === "POST /api/push/subscribe") {
     const subscription = push.parse(body.subscription);
     if (!subscription) return badRequest("invalid_subscription", "subscription needs an https endpoint and p256dh/auth keys");
-    if (push.retired(subscription.endpoint)) return subscriptionGone();
+    // a mark written before endpoints were normalized is the hash of the spelling as it was handed over
+    const handed = (body.subscription as { endpoint: string }).endpoint;
+    if (push.retired(subscription.endpoint) || push.retired(handed)) return subscriptionGone();
     push.subscribe(subscription, body.alerts === undefined ? undefined : parseAlerts(body.alerts), deviceId, browserKeys);
     return new Response(null, { status: 204 });
   }
@@ -895,12 +926,16 @@ export async function handlePushRequest(request: Request, pathname: string, push
     return status ? jsonResponse(status, 200, { "cache-control": "no-store" }) : subscriptionNotFound();
   }
   if (typeof body.endpoint !== "string") return badRequest("missing_endpoint", "endpoint is required");
+  // the subscriptions are stored normalized: so is the endpoint they are looked up by
+  const endpoint = normalizeEndpoint(body.endpoint);
   if (route === "DELETE /api/push/subscribe") {
-    push.unsubscribe(body.endpoint);
+    push.unsubscribe(endpoint);
+    // and the history recorded, before endpoints were normalized, under the spelling handed over
+    if (body.endpoint !== endpoint) push.unsubscribe(body.endpoint);
     return new Response(null, { status: 204 });
   }
-  const delivery = await push.sendTest(body.endpoint);
-  if (!delivery) return push.retired(body.endpoint) ? subscriptionGone() : subscriptionNotFound();
+  const delivery = await push.sendTest(endpoint);
+  if (!delivery) return push.retired(endpoint) || push.retired(body.endpoint) ? subscriptionGone() : subscriptionNotFound();
   if (delivery.ok) return new Response(null, { status: 204 });
   if (delivery.gone) return subscriptionGone();
   return jsonResponse({ error: { code: "push_failed", message: `push service answered ${delivery.status ?? "nothing"}` } }, 502);
diff --git a/server/state-merge.test.ts b/server/state-merge.test.ts
index c09fe68d2a..ed5655c61a 100644
--- a/server/state-merge.test.ts
+++ b/server/state-merge.test.ts
@@ -516,6 +516,19 @@ describe("push subscriptions of the install's other state dir", () => {
     return fake;
   };
 
+  // regression: the merge compared raw spellings, so another spelling of an endpoint retired here came back registered (#2012)
+  it("leave out another spelling of an endpoint retired here", async () => {
+    const { active, sibling } = install();
+    const key = webpush.generateVAPIDKeys();
+    const fake = await subscribe(sibling, key);
+    writeJson(join(sibling, "push-subscriptions.json"), [{ ...fake.subscription, endpoint: `${fake.subscription.endpoint}#other`, device_id: null }]);
+    const retiredHash = createHash("sha256").update(fake.subscription.endpoint).digest("base64url");
+    writeJson(join(active, "push-retired.json"), [{ hash: retiredHash, at: 1, owner: "device-a" }]);
+    mergeSiblingState(active, sibling, at(0));
+    const held = existsSync(join(active, "push-subscriptions.json")) ? JSON.parse(readFileSync(join(active, "push-subscriptions.json"), "utf8")) as unknown[] : [];
+    expect(held).toEqual([]);
+  });
+
   it("bring their key along where this dir has none", async () => {
     const { active, sibling } = install();
     const key = webpush.generateVAPIDKeys();
diff --git a/server/state-merge.ts b/server/state-merge.ts
index 672e5e5fef..89e2c5bb4f 100644
--- a/server/state-merge.ts
+++ b/server/state-merge.ts
@@ -33,7 +33,7 @@ import { join, resolve } from "node:path";
 import { MAX_PREFERENCES_BYTES, sharedPreferencesBody, type PreferencesChange } from "../shared/preferences.ts";
 import { readDeviceRegistry, type StoredDevice } from "./devices.ts";
 import { preparePrivateState, publishPrivateState, writePrivateState } from "./private-state.ts";
-import { PUSH_EXTRA_KEYS } from "./push.ts";
+import { normalizeEndpoint, PUSH_EXTRA_KEYS } from "./push.ts";
 import { readStateDocument, writeStateDocument } from "./state-document.ts";
 import { stateDirNames } from "./update-state.ts";
 
@@ -467,8 +467,11 @@ function mergePush(active: string, sibling: string, base: MergeRecord | null, st
       const value = readPushFile(join(active, "push-retired.json"));
       retired = (Array.isArray(value) ? value : []).map((entry) => (entry as { hash?: unknown } | null)?.hash).filter((hash): hash is string => typeof hash === "string");
     } catch { /* telemetry: an endpoint it would have kept out only fails its next push */ }
-    const settled = (subscription: Subscription) => mine.some((held) => held.endpoint === subscription.endpoint)
-      || before.includes(endpointHash(subscription.endpoint)) || retired.includes(endpointHash(subscription.endpoint));
+    // push.ts stores and retires endpoints normalized: another spelling of one held or retired here is that one
+    const canonical = (subscription: Subscription) => normalizeEndpoint(subscription.endpoint);
+    const settled = (subscription: Subscription) => mine.some((held) => canonical(held) === canonical(subscription))
+      || before.includes(endpointHash(subscription.endpoint))
+      || retired.includes(endpointHash(subscription.endpoint)) || retired.includes(endpointHash(canonical(subscription)));
     const taken = theirs.filter((subscription) => !settled(subscription) && keyOf(subscription.endpoint) !== null);
     const record = recorded(theirs.filter((subscription) => settled(subscription) || taken.includes(subscription)));
     if (taken.length === 0) return { record, taken: 0 };

From 70d12fe52dd48c33091e603aec8c5200f3bda8ca Mon Sep 17 00:00:00 2001
From: devswha <devswha@gmail.com>
Date: Wed, 28 Oct 2026 11:49:03 +0900
Subject: [PATCH 2/3] fix(push): normalize only the retired check, and keep
 stored endpoints as handed over

Storing endpoints normalized changed the identity of state kept under the spelling a
browser handed over: a key pair push-keys.json kept for an old spelling signed for the
live one (403), and the other state dir's next write respelled a subscription taken over
and deleted here, so the next merge took it again (merge-pass review of #2140).

Subscriptions, their key pairs, their history and the merge records stay under the
spelling handed over, as before. Only a retired mark is keyed by the normalized endpoint,
and the retired check also takes a mark from before by the spelling handed over. A
subscription the file still lists while its endpoint is retired (its removal could not be
written) is not loaded again after a restart (#2013).
---
 server/push.test.ts        | 22 +++++++++++-----
 server/push.ts             | 53 +++++++++++++++++++++-----------------
 server/state-merge.test.ts | 15 +++++++++++
 server/state-merge.ts      |  7 +++--
 4 files changed, 63 insertions(+), 34 deletions(-)

diff --git a/server/push.test.ts b/server/push.test.ts
index 14abfa97db..4edee4ce96 100644
--- a/server/push.test.ts
+++ b/server/push.test.ts
@@ -3,6 +3,7 @@ import { createHash } from "node:crypto";
 import { existsSync, mkdirSync, mkdtempSync, readdirSync, readFileSync, rmSync, statSync, writeFileSync } from "node:fs";
 import { tmpdir } from "node:os";
 import { join } from "node:path";
+import webpush from "web-push";
 
 import { paneAlertsMuted, type AlertPrefs } from "../shared/notify-policy.ts";
 import { paneStorageId } from "../shared/machines.ts";
@@ -370,8 +371,8 @@ describe("push delivery", () => {
     expect(createPushService({ loopbackHttp: true, stateDir }).retired(fake.subscription.endpoint)).toBe(false);
   });
 
-  // regression: a mark or a history written before endpoints were normalized is the hash of the spelling handed over
-  it("keeps refusing, and forgets on goodbye, an endpoint retired before endpoints were normalized", async () => {
+  // a mark written before #2012 is the hash of the spelling handed over, and so is the history
+  it("keeps refusing, and forgets on goodbye, an endpoint retired under the spelling it was handed over", async () => {
     const legacy = `${fake.subscription.endpoint}#legacy`;
     const raw = createHash("sha256").update(legacy).digest("base64url");
     writeFileSync(join(stateDir, "push-retired.json"), JSON.stringify([{ hash: raw, at: 1, owner: "owner" }]));
@@ -401,6 +402,9 @@ describe("push delivery", () => {
       expect(fake.received).toHaveLength(0);
       expect(quiet.mock.calls.some(([line]) => String(line).includes("push-subscriptions.json"))).toBe(true);
     } finally { rename.mockRestore(); quiet.mockRestore(); }
+    // a restart, with the file still listing it, does not bring it back: it is retired
+    expect(await createPushService({ loopbackHttp: true, stateDir }).sendTest(fake.subscription.endpoint)).toBeNull();
+    expect(fake.received).toHaveLength(0);
     // the device's goodbye, though nothing of it is left in memory, writes the file without it
     push.unsubscribe(fake.subscription.endpoint);
     expect(JSON.parse(readFileSync(join(stateDir, "push-subscriptions.json"), "utf8"))).toEqual([]);
@@ -410,10 +414,16 @@ describe("push delivery", () => {
     expect((JSON.parse(readFileSync(join(stateDir, "push-subscriptions.json"), "utf8")) as { endpoint: string }[]).map((entry) => entry.endpoint)).toEqual([`${fake.subscription.endpoint}-other`]);
   });
 
-  it("stores an endpoint in one form, the URL without its fragment", () => {
-    const { keys } = fake.subscription;
-    expect(parseSubscription({ endpoint: "https://PUSH.Example:443/wpush/./v2/x#attacker", keys })?.endpoint).toBe("https://push.example/wpush/v2/x");
-    expect(parseSubscription({ endpoint: "https://push.example/wpush/v2/x", keys })?.endpoint).toBe("https://push.example/wpush/v2/x");
+  // regression: storing endpoints normalized let a key pair push-keys.json kept for another spelling sign for this one
+  it("keeps a subscription and the key pair it was made for under the spelling it was handed over", async () => {
+    const { endpoint, keys } = fake.subscription;
+    expect(parseSubscription({ endpoint: `${endpoint}#x`, keys }, { loopbackHttp: true })?.endpoint).toBe(`${endpoint}#x`);
+    // a foreign key pair left for an old spelling of this endpoint, no longer subscribed
+    const stale = webpush.generateVAPIDKeys();
+    writeFileSync(join(stateDir, "push-keys.json"), JSON.stringify({ keys: [{ ...stale, endpoints: [`${endpoint}#old-import`] }] }));
+    const push = subscribed();
+    expect(await push.sendTest(endpoint)).toEqual({ ok: true });
+    expect(fake.received[0]).toMatchObject({ vapidKey: push.publicKey(), vapidValid: true });
   });
 
   // regression (fails on main): a fragment, or any other spelling of the same URL, got past the retired check (#2012)
diff --git a/server/push.ts b/server/push.ts
index 24b94521dd..1b30b55c0f 100644
--- a/server/push.ts
+++ b/server/push.ts
@@ -152,7 +152,7 @@ export interface PushService {
   subscribe(subscription: PushSubscriptionRecord, alerts?: AlertPrefs, deviceId?: string | null, browserKeys?: readonly string[]): void;
   revokeDevice(id: string): void;
   unsubscribe(endpoint: string): void;
-  /** whether a push service answered 404/410 for this endpoint (`endpoint` as normalizeEndpoint gives it) */
+  /** whether a push service answered 404/410 for this endpoint, in any spelling normalizeEndpoint takes as one */
   retired(endpoint: string): boolean;
   /** One confirmation push to one device, so enabling alerts proves the whole path works. */
   sendTest(endpoint: string): Promise<PushDelivery | null>;
@@ -232,11 +232,13 @@ function decodedLength(value: unknown): number {
 }
 
 /**
- * The one form an endpoint is stored, compared and retired in: the URL as WHATWG serializes it (host
- * in lowercase, no default port, no dot segments), without its fragment, which never reaches the push
- * service. Those spellings of a retired endpoint are then that same endpoint (#2012); a query string,
- * percent-encoding and a trailing slash are kept as they are, since a push service may tell them apart. What does not
- * parse is returned as it is.
+ * The form an endpoint is retired in: the URL as WHATWG serializes it (host in lowercase, no default
+ * port, no dot segments), without its fragment, which never reaches the push service. Those spellings
+ * of a retired endpoint are then that same endpoint (#2012); a query string, percent-encoding and a
+ * trailing slash are kept as they are, since a push service may tell them apart (WNS keeps its token in
+ * the query). What does not parse is returned as it is. Only the retired check uses it: a subscription,
+ * its key pair (push-keys.json), its history and state-merge.ts's records stay under the spelling the
+ * browser handed over, so nothing stored before has to be found under another one.
  */
 export function normalizeEndpoint(endpoint: string): string {
   try {
@@ -269,7 +271,7 @@ export function parseSubscription(value: unknown, options: { loopbackHttp?: bool
   }
   // p256dh is an uncompressed P-256 point (65 bytes), auth a 16-byte secret
   if (decodedLength(keys?.p256dh) !== 65 || decodedLength(keys?.auth) !== 16) return null;
-  return { endpoint: normalizeEndpoint(endpoint), keys: { p256dh: keys!.p256dh as string, auth: keys!.auth as string } };
+  return { endpoint, keys: { p256dh: keys!.p256dh as string, auth: keys!.auth as string } };
 }
 
 export function statusMessage(paneId: string, title: string, status: AgentStatus): PushPayload {
@@ -298,6 +300,8 @@ export function createPushService(options: PushServiceOptions): PushService {
 
   let vapid: { publicKey: string; privateKey: string } | null = null;
   let subscriptions: Map<string, PushSubscriptionRecord> | null = null;
+  /** the file still lists a subscription dropped in memory only (a retirement whose write failed): the next goodbye writes it */
+  let stale = false;
   const lastStatus = new Map<string, AgentStatus>();
   const titles = new Map<string, string>();
   // the workspace a pane was last seen in: an alert names it, so the tap of an ended pane can open
@@ -311,6 +315,10 @@ export function createPushService(options: PushServiceOptions): PushService {
    */
   let retiredEndpoints: Map<string, RetiredMark> | null = null;
   const endpointHash = (endpoint: string): string => createHash("sha256").update(endpoint).digest("base64url");
+  /** what a retired mark is keyed by: one key for every spelling of the endpoint (#2012) */
+  const retiredKey = (endpoint: string): string => endpointHash(normalizeEndpoint(endpoint));
+  /** a mark from before #2012 is keyed by the spelling as it was handed over */
+  const isRetired = (endpoint: string): boolean => retiredStore().has(retiredKey(endpoint)) || retiredStore().has(endpointHash(endpoint));
   /** endpoint -> the key pair it was made for, when that is not vapid.json's; loaded on first need */
   let extraKeys: Map<string, VapidKeys> | null = null;
 
@@ -324,8 +332,7 @@ export function createPushService(options: PushServiceOptions): PushService {
         for (const entry of Array.isArray(stored?.keys) ? stored.keys as unknown[] : []) {
           const { publicKey, privateKey, endpoints } = (entry ?? {}) as { publicKey?: unknown; privateKey?: unknown; endpoints?: unknown };
           if (typeof publicKey !== "string" || typeof privateKey !== "string" || !Array.isArray(endpoints)) continue;
-          // the subscriptions are loaded normalized (parseSubscription): so are the endpoints they are looked up by
-          for (const at of endpoints) if (typeof at === "string") extraKeys.set(normalizeEndpoint(at), { publicKey, privateKey });
+          for (const at of endpoints) if (typeof at === "string") extraKeys.set(at, { publicKey, privateKey });
         }
       } catch (error) {
         // those subscriptions are then signed with vapid.json's key, which their push service refuses
@@ -363,6 +370,11 @@ export function createPushService(options: PushServiceOptions): PushService {
     if (Array.isArray(stored)) {
       for (const entry of stored) {
         const parsed = parseSubscription(entry, options);
+        // retired, but its removal could not be written (#2013): it stays gone, and the next goodbye writes the file without it
+        if (parsed && isRetired(parsed.endpoint)) {
+          stale = true;
+          continue;
+        }
         const alerts = (entry as { alerts?: unknown } | null)?.alerts;
         if (parsed) {
           const owner = (entry as PushSubscriptionRecord).device_id;
@@ -435,8 +447,6 @@ export function createPushService(options: PushServiceOptions): PushService {
     subscriptions = next;
     stale = false;
   }
-  /** the file still lists a subscription dropped in memory only (a retirement whose write failed): the next goodbye writes it */
-  let stale = false;
 
   function without(remove: (endpoint: string, subscription: PushSubscriptionRecord) => boolean): Map<string, PushSubscriptionRecord> | null {
     const next = new Map([...store()].filter(([endpoint, subscription]) => !remove(endpoint, subscription)));
@@ -572,8 +582,9 @@ export function createPushService(options: PushServiceOptions): PushService {
     recordDelivery(identity, classifyFailure(status));
     if (gone) {
       const retired = retiredStore();
-      retired.delete(identity.hash);
-      retired.set(identity.hash, { at: (options.now ?? Date.now)(), ...(identity.owner === undefined ? {} : { owner: identity.owner }) });
+      const mark = retiredKey(subscription.endpoint);
+      retired.delete(mark);
+      retired.set(mark, { at: (options.now ?? Date.now)(), ...(identity.owner === undefined ? {} : { owner: identity.owner }) });
       if (retired.size > RETIRED_LIMIT) retired.delete(retired.keys().next().value!);
       // the list is telemetry: a write that fails keeps the mark in memory, and never keeps the dead subscription (#2013)
       rewriteRetired();
@@ -745,7 +756,7 @@ export function createPushService(options: PushServiceOptions): PushService {
       return { registered: false, last_success_at: iso(mine.success_at), last_failure_at: iso(mine.failure_at), last_failure: mine.failure };
     },
 
-    retired: (endpoint) => retiredStore().has(endpointHash(endpoint)),
+    retired: isRetired,
 
     async sendTest(endpoint) {
       const subscription = store().get(endpoint);
@@ -910,9 +921,7 @@ export async function handlePushRequest(request: Request, pathname: string, push
   if (route === "POST /api/push/subscribe") {
     const subscription = push.parse(body.subscription);
     if (!subscription) return badRequest("invalid_subscription", "subscription needs an https endpoint and p256dh/auth keys");
-    // a mark written before endpoints were normalized is the hash of the spelling as it was handed over
-    const handed = (body.subscription as { endpoint: string }).endpoint;
-    if (push.retired(subscription.endpoint) || push.retired(handed)) return subscriptionGone();
+    if (push.retired(subscription.endpoint)) return subscriptionGone();
     push.subscribe(subscription, body.alerts === undefined ? undefined : parseAlerts(body.alerts), deviceId, browserKeys);
     return new Response(null, { status: 204 });
   }
@@ -926,16 +935,12 @@ export async function handlePushRequest(request: Request, pathname: string, push
     return status ? jsonResponse(status, 200, { "cache-control": "no-store" }) : subscriptionNotFound();
   }
   if (typeof body.endpoint !== "string") return badRequest("missing_endpoint", "endpoint is required");
-  // the subscriptions are stored normalized: so is the endpoint they are looked up by
-  const endpoint = normalizeEndpoint(body.endpoint);
   if (route === "DELETE /api/push/subscribe") {
-    push.unsubscribe(endpoint);
-    // and the history recorded, before endpoints were normalized, under the spelling handed over
-    if (body.endpoint !== endpoint) push.unsubscribe(body.endpoint);
+    push.unsubscribe(body.endpoint);
     return new Response(null, { status: 204 });
   }
-  const delivery = await push.sendTest(endpoint);
-  if (!delivery) return push.retired(endpoint) || push.retired(body.endpoint) ? subscriptionGone() : subscriptionNotFound();
+  const delivery = await push.sendTest(body.endpoint);
+  if (!delivery) return push.retired(body.endpoint) ? subscriptionGone() : subscriptionNotFound();
   if (delivery.ok) return new Response(null, { status: 204 });
   if (delivery.gone) return subscriptionGone();
   return jsonResponse({ error: { code: "push_failed", message: `push service answered ${delivery.status ?? "nothing"}` } }, 502);
diff --git a/server/state-merge.test.ts b/server/state-merge.test.ts
index ed5655c61a..be27834c9c 100644
--- a/server/state-merge.test.ts
+++ b/server/state-merge.test.ts
@@ -529,6 +529,21 @@ describe("push subscriptions of the install's other state dir", () => {
     expect(held).toEqual([]);
   });
 
+  // regression: storing endpoints normalized let the other dir's next write respell one taken over, and the next merge took it again
+  it("do not come back after a goodbye here when the other dir writes its list again", async () => {
+    const { active, sibling } = install();
+    const fake = await subscribe(sibling, webpush.generateVAPIDKeys());
+    const legacy = `${fake.subscription.endpoint}#legacy`;
+    writeJson(join(sibling, "push-subscriptions.json"), [{ ...fake.subscription, endpoint: legacy, device_id: null }]);
+    mergeSiblingState(active, sibling, at(0));
+    createPushService({ stateDir: active, loopbackHttp: true }).unsubscribe(legacy);
+    // the other dir writes its list again, for a subscription of its own
+    createPushService({ stateDir: sibling, loopbackHttp: true }).subscribe({ ...fake.subscription, endpoint: `${fake.subscription.endpoint}-other` }, undefined, null);
+    mergeSiblingState(active, sibling, at(1));
+    const held = JSON.parse(readFileSync(join(active, "push-subscriptions.json"), "utf8")) as { endpoint: string }[];
+    expect(held.map((subscription) => subscription.endpoint)).toEqual([`${fake.subscription.endpoint}-other`]);
+  });
+
   it("bring their key along where this dir has none", async () => {
     const { active, sibling } = install();
     const key = webpush.generateVAPIDKeys();
diff --git a/server/state-merge.ts b/server/state-merge.ts
index 89e2c5bb4f..7ce4a7c9d7 100644
--- a/server/state-merge.ts
+++ b/server/state-merge.ts
@@ -467,11 +467,10 @@ function mergePush(active: string, sibling: string, base: MergeRecord | null, st
       const value = readPushFile(join(active, "push-retired.json"));
       retired = (Array.isArray(value) ? value : []).map((entry) => (entry as { hash?: unknown } | null)?.hash).filter((hash): hash is string => typeof hash === "string");
     } catch { /* telemetry: an endpoint it would have kept out only fails its next push */ }
-    // push.ts stores and retires endpoints normalized: another spelling of one held or retired here is that one
-    const canonical = (subscription: Subscription) => normalizeEndpoint(subscription.endpoint);
-    const settled = (subscription: Subscription) => mine.some((held) => canonical(held) === canonical(subscription))
+    // push.ts keys a retired mark by the normalized endpoint (a mark from before #2012 by the spelling handed over)
+    const settled = (subscription: Subscription) => mine.some((held) => held.endpoint === subscription.endpoint)
       || before.includes(endpointHash(subscription.endpoint))
-      || retired.includes(endpointHash(subscription.endpoint)) || retired.includes(endpointHash(canonical(subscription)));
+      || retired.includes(endpointHash(subscription.endpoint)) || retired.includes(endpointHash(normalizeEndpoint(subscription.endpoint)));
     const taken = theirs.filter((subscription) => !settled(subscription) && keyOf(subscription.endpoint) !== null);
     const record = recorded(theirs.filter((subscription) => settled(subscription) || taken.includes(subscription)));
     if (taken.length === 0) return { record, taken: 0 };

From e94ee53913553377c8dbbcbd7385b18db1974e4f Mon Sep 17 00:00:00 2001
From: devswha <devswha@gmail.com>
Date: Wed, 28 Oct 2026 12:05:41 +0900
Subject: [PATCH 3/3] fix(push): keep a mark two owners' spellings share, and
 write a revocation's marks before subscribing again

Two spellings of one endpoint held by different owners share its retired mark, and the
last retirement wrote it for its owner alone, so that owner's revocation reopened the
endpoint. Such a mark now belongs to neither owner.

A revocation whose write of push-retired.json failed removed the mark in memory only. A
subscribe of that endpoint then succeeded, and a restart reloaded the mark and dropped the
subscription. A subscribe now writes the list first while it is unwritten.

The changelog names the 1000-mark bound (CodeRabbit review of #2140).
---
 changelog.d/push-retired-endpoint-checks.md |  6 ++--
 server/push.test.ts                         | 36 +++++++++++++++++++++
 server/push.ts                              | 16 ++++++++-
 3 files changed, 54 insertions(+), 4 deletions(-)

diff --git a/changelog.d/push-retired-endpoint-checks.md b/changelog.d/push-retired-endpoint-checks.md
index b9cf2f994e..018a55cb68 100644
--- a/changelog.d/push-retired-endpoint-checks.md
+++ b/changelog.d/push-retired-endpoint-checks.md
@@ -1,8 +1,8 @@
 ### Fixed
 - A push endpoint its push service retired stays refused, however long ago that was, until its
-  owner's device is revoked. Before, after 30 days another device that knew the endpoint could
-  register it as its own, and so could another spelling of it (a `#fragment`, an uppercase host or
-  `:443`) at any time.
+  owner's device is revoked or a thousand later retirements push its mark out. Before, after 30 days
+  another device that knew the endpoint could register it as its own, and so could another spelling
+  of it (a `#fragment`, an uppercase host or `:443`) at any time.
 - A subscription its push service retired is removed even when `push-retired.json` cannot be
   written (a full disk, a read-only state directory). Before, the failed write left it registered
   and the next alert went to the dead endpoint again.
diff --git a/server/push.test.ts b/server/push.test.ts
index 4edee4ce96..18da7e96b1 100644
--- a/server/push.test.ts
+++ b/server/push.test.ts
@@ -922,6 +922,42 @@ describe("push delivery health (#1039)", () => {
     expect(createPushService({ loopbackHttp: true, stateDir, now: () => clock }).retired(endpoint)).toBe(true);
   });
 
+  // regression: two owners' spellings of one endpoint share its mark, and the last retirement took it for its owner alone
+  it("keeps a mark two owners' spellings were retired under through either owner's revocation", async () => {
+    const push = createPushService({ loopbackHttp: true, stateDir });
+    const endpoint = fake.subscription.endpoint;
+    push.subscribe(fake.subscription, undefined, "device-a");
+    push.subscribe({ ...fake.subscription, endpoint: `${endpoint}#b` }, undefined, "device-b");
+    fake.answerWith(410);
+    expect(await push.sendTest(endpoint)).toEqual({ ok: false, status: 410, gone: true });
+    expect(await push.sendTest(`${endpoint}#b`)).toEqual({ ok: false, status: 410, gone: true });
+    push.revokeDevice("device-b");
+    expect(push.retired(endpoint)).toBe(true);
+    push.revokeDevice("device-a");
+    expect(push.retired(endpoint)).toBe(true);
+    expect(createPushService({ loopbackHttp: true, stateDir }).retired(endpoint)).toBe(true);
+  });
+
+  // regression: a mark a revocation removed in memory only came back at a restart and dropped the endpoint subscribed again
+  it("writes the marks a revocation removed before the endpoint is subscribed again", async () => {
+    const push = createPushService({ loopbackHttp: true, stateDir });
+    push.subscribe(fake.subscription, undefined, "device-a");
+    fake.answerWith(410);
+    expect(await push.sendTest(fake.subscription.endpoint)).toEqual({ ok: false, status: 410, gone: true });
+    // the list cannot be replaced for a while: a directory stands at its temporary path
+    const blocker = join(stateDir, `push-retired.json.${process.pid}.tmp`);
+    mkdirSync(blocker);
+    const quiet = spyOn(console, "error").mockImplementation(() => undefined);
+    try { push.revokeDevice("device-a"); } finally { quiet.mockRestore(); }
+    rmSync(blocker, { recursive: true });
+    const response = await handlePushRequest(new Request("http://localhost/api/push/subscribe", {
+      method: "POST", headers: { "content-type": "application/json" }, body: JSON.stringify({ subscription: fake.subscription }),
+    }), "/api/push/subscribe", push, "device-b");
+    expect(response?.status).toBe(204);
+    fake.answerWith(201);
+    expect(await createPushService({ loopbackHttp: true, stateDir }).sendTest(fake.subscription.endpoint)).toEqual({ ok: true });
+  });
+
   it("revocation takes the device's history and marks when the retired list cannot be written", async () => {
     const push = createPushService({ loopbackHttp: true, stateDir });
     push.subscribe(fake.subscription, undefined, "device-a");
diff --git a/server/push.ts b/server/push.ts
index 1b30b55c0f..58b23b4190 100644
--- a/server/push.ts
+++ b/server/push.ts
@@ -410,12 +410,19 @@ export function createPushService(options: PushServiceOptions): PushService {
     try {
       persistRetired();
     } catch (error) {
+      retiredUnwritten = true;
       console.error(`Cannot write ${retiredPath} (${(error as NodeJS.ErrnoException).code ?? "unknown error"}); retired endpoints are kept in memory until a write succeeds`);
     }
   }
+  /**
+   * The list on disk is not the one in memory (a write failed): a subscribe writes it first, so a mark
+   * a revocation removed in memory only cannot come back at a restart and drop the new subscription.
+   */
+  let retiredUnwritten = false;
 
   function persistRetired(): void {
     writeJsonPrivate(retiredPath, [...retiredStore()].map(([hash, { at, owner }]) => ({ hash, at, ...(owner === undefined ? {} : { owner }) })));
+    retiredUnwritten = false;
   }
 
   function retiredStore(): Map<string, RetiredMark> {
@@ -583,8 +590,14 @@ export function createPushService(options: PushServiceOptions): PushService {
     if (gone) {
       const retired = retiredStore();
       const mark = retiredKey(subscription.endpoint);
+      // two spellings of one endpoint held by different owners share the mark: it then belongs to neither,
+      // so one owner's revocation cannot take a mark the other's subscription was retired under too
+      const previous = retired.get(mark);
+      const shared = (previous !== undefined && previous.owner === undefined)
+        || [...store().values()].some((held) => held.endpoint !== subscription.endpoint && retiredKey(held.endpoint) === mark && held.device_id !== identity.owner);
+      const owner = shared ? undefined : identity.owner;
       retired.delete(mark);
-      retired.set(mark, { at: (options.now ?? Date.now)(), ...(identity.owner === undefined ? {} : { owner: identity.owner }) });
+      retired.set(mark, { at: (options.now ?? Date.now)(), ...(owner === undefined ? {} : { owner }) });
       if (retired.size > RETIRED_LIMIT) retired.delete(retired.keys().next().value!);
       // the list is telemetry: a write that fails keeps the mark in memory, and never keeps the dead subscription (#2013)
       rewriteRetired();
@@ -708,6 +721,7 @@ export function createPushService(options: PushServiceOptions): PushService {
       next.set(subscription.endpoint, { ...subscription, ...(kept === undefined ? {} : { alerts: kept }),
         ...(deviceId === undefined ? {} : { device_id: deviceId }),
         ...(browser === undefined ? {} : { browser_keys: browser }) });
+      if (retiredUnwritten) persistRetired();
       persist(next);
       // the record of the subscription this replaces (another owner, other keys) is not the new one's
       const identity = identityOf({ ...subscription, device_id: deviceId });