only skip stored push rows that break the vapid audience

Running the full registration validator over stored rows dropped self-hosted endpoints registered before 0.18 with a non-443 port or a bare / path. _build_web_pushers now skips only non-dict rows, rows with no endpoint, unparsable endpoints and hosts with empty labels, and the warning names the host so the row can be found. Registration still uses the full validator.

frigate/comms/webpush.py

@@ -133,14 +133,29 @@ def _build_web_pushers(user: str, subs: list[dict[str, Any]]) -> list[WebPusher]
     pushers: list[WebPusher] = []
 
     for sub in subs:
-        # Rows stored before registration validation can hold any endpoint
-        reason = validate_push_endpoint(
-            sub.get("endpoint") if isinstance(sub, dict) else None
-        )
+        # Stored rows may predate registration validation, so only skip the
+        # endpoints that would break building the VAPID audience
+        endpoint = sub.get("endpoint") if isinstance(sub, dict) else None
 
-        if reason:
+        if not isinstance(endpoint, str):
             logger.warning(
-                "Skipping notification subscription for %s: %s", user, reason
+                "Skipping notification subscription for %s: no endpoint", user
+            )
+            continue
+
+        try:
+            hostname = urlparse(endpoint).hostname or ""
+        except ValueError:
+            logger.warning(
+                "Skipping notification subscription for %s: unparsable endpoint", user
+            )
+            continue
+
+        if "" in hostname.split("."):
+            logger.warning(
+                "Skipping notification subscription for %s: invalid endpoint host %r",
+                user,
+                hostname,
             )
             continue
 

frigate/test/test_webpush_registration.py

@@ -165,18 +165,41 @@ class TestBuildWebPushers(unittest.TestCase):
         self.assertEqual(len(pushers), 1)
         self.assertEqual(pushers[0].receiver_key, b"\x04" + bytes(64))
 
-    def test_skips_subscriptions_with_invalid_endpoints(self):
-        good = _subscription(VALID_ENDPOINTS[0])
-        good["keys"]["p256dh"] = urlsafe_b64encode(b"\x04" + bytes(64)).decode()
-        bad = dict(good, endpoint="https://./x")
+    def _valid_keys_subscription(self, endpoint: str) -> dict:
+        sub = _subscription(endpoint)
+        sub["keys"]["p256dh"] = urlsafe_b64encode(b"\x04" + bytes(64)).decode()
+        return sub
 
-        with self.assertLogs("frigate.comms.webpush", level="WARNING"):
-            pushers = _build_web_pushers("viewer", [bad, "not-a-dict", good])
+    def test_skips_stored_rows_that_break_the_audience(self):
+        good = self._valid_keys_subscription(VALID_ENDPOINTS[0])
+        rows = [
+            self._valid_keys_subscription("https://./secret-token"),
+            self._valid_keys_subscription("https://[::1/secret-token"),
+            {"keys": good["keys"]},
+            "not-a-dict",
+            good,
+        ]
 
+        with self.assertLogs("frigate.comms.webpush", level="WARNING") as logs:
+            pushers = _build_web_pushers("viewer", rows)
+
+        self.assertEqual(len(logs.output), 4)
+        self.assertIn("'.'", logs.output[0])
+        self.assertNotIn("secret-token", "".join(logs.output))
         self.assertEqual(len(pushers), 1)
         self.assertEqual(pushers[0].subscription_info["endpoint"], VALID_ENDPOINTS[0])
         self.assertIsInstance(pushers[0].mod_or_session, _NoRedirectSession)
 
+    def test_keeps_stored_self_hosted_endpoints(self):
+        rows = [
+            self._valid_keys_subscription("https://ntfy.example.com:8443/up123"),
+            self._valid_keys_subscription("https://ntfy.example.com/"),
+        ]
+
+        pushers = _build_web_pushers("viewer", rows)
+
+        self.assertEqual(len(pushers), 2)
+
 
 class TestVapidAudience(unittest.TestCase):
     def test_uses_scheme_and_host(self):