fix untrained classification models not saving images (#24597)

The mypy cleanup in https://github.com/blakeblackshear/frigate/pull/22658 added a check on the tensor details to the top of process_frame for both state and object models. Those are unset until a model is trained, so untrained models returned early and never saved images to Recent Classifications. Remove the check from process_frame, the classify methods already check the tensor details before using them.

docker/main/rootfs/usr/local/nginx/conf/auth_location.conf

@@ -14,7 +14,7 @@ location /auth {
     proxy_pass_request_headers off;
     # Pass info about the request
     proxy_set_header X-Original-Method $request_method;
-    proxy_set_header X-Original-URL $scheme://$http_host$request_uri;
+    proxy_set_header X-Original-URL $request_uri;
     proxy_set_header X-Server-Port $server_port;
     proxy_set_header Content-Length "";
     # Pass along auth related info

docker/main/rootfs/usr/local/nginx/conf/proxy.conf

@@ -2,7 +2,7 @@
 proxy_set_header Host $host;
 proxy_set_header Upgrade $http_upgrade;
 proxy_set_header Connection "Upgrade";
-proxy_set_header X-Original-URL $scheme://$http_host$request_uri;
+proxy_set_header X-Original-URL $request_uri;
 proxy_set_header X-Forwarded-Proto $scheme;
 proxy_set_header X-Forwarded-Host $http_host;
 proxy_set_header X-Forwarded-URI $request_uri;

frigate/api/export.py

@@ -2,7 +2,6 @@
 
 import datetime
 import logging
-import os
 import random
 import string
 import time
@@ -57,7 +56,6 @@ from frigate.api.defs.response.export_response import (
 )
 from frigate.api.defs.response.generic_response import GenericResponse
 from frigate.api.defs.tags import Tags
-from frigate.api.media_auth import deny_response_for_media_uri
 from frigate.const import CLIPS_DIR, EXPORT_DIR
 from frigate.jobs.export import (
     ExportJob,
@@ -132,7 +130,6 @@ def _validate_export_case(export_case_id: str | None) -> JSONResponse | None:
 
 
 def _sanitize_existing_image(
-    request: Request,
     image_path: str | None,
 ) -> tuple[str | None, JSONResponse | None]:
     if not image_path:
@@ -140,15 +137,6 @@ def _sanitize_existing_image(
 
     existing_image = sanitize_contained_path(image_path, CLIPS_DIR)
 
-    # CLIPS_DIR is shared by every camera, so the caller must also be allowed
-    # to read the image.
-    if existing_image is not None and deny_response_for_media_uri(
-        f"/clips/{quote(os.path.relpath(existing_image, CLIPS_DIR))}",
-        request.headers.get("remote-role"),
-        request.app.frigate_config,
-    ):
-        existing_image = None
-
     if existing_image is None:
         return None, JSONResponse(
             content={"success": False, "message": "Invalid image path"},
@@ -692,7 +680,7 @@ def export_recordings_batch(
     sanitized_images: list[str | None] = []
     for item in body.items:
         existing_image, image_validation_error = _sanitize_existing_image(
-            request, item.image_path
+            item.image_path
         )
         if image_validation_error is not None:
             return image_validation_error
@@ -839,9 +827,7 @@ def export_recording(
 
     playback_source = body.source
     friendly_name = body.name
-    existing_image, image_validation_error = _sanitize_existing_image(
-        request, body.image_path
-    )
+    existing_image, image_validation_error = _sanitize_existing_image(body.image_path)
     if image_validation_error is not None:
         return image_validation_error
 
@@ -979,9 +965,7 @@ def export_recording_custom(
 
     playback_source = body.source
     friendly_name = body.name
-    existing_image, image_validation_error = _sanitize_existing_image(
-        request, body.image_path
-    )
+    existing_image, image_validation_error = _sanitize_existing_image(body.image_path)
     if image_validation_error is not None:
         return image_validation_error
     ffmpeg_input_args = body.ffmpeg_input_args

frigate/comms/webpush.py

@@ -41,6 +41,20 @@ class PushNotification:
     ttl: int = 0
 
 
+def _build_web_pushers(user: str, subs: list[dict[str, Any]]) -> list[WebPusher]:
+    """Build pushers for a user's stored subscriptions, skipping unusable ones."""
+    pushers: list[WebPusher] = []
+
+    for sub in subs:
+        # WebPusher decodes the stored keys and raises on malformed ones
+        try:
+            pushers.append(WebPusher(sub))
+        except Exception:
+            logger.warning("Skipping invalid notification subscription for %s", user)
+
+    return pushers
+
+
 class WebPushClient(Communicator):
     """Frigate wrapper for webpush client."""
 
@@ -82,9 +96,9 @@ class WebPushClient(Communicator):
             User.select(User.username, User.notification_tokens).dicts().iterator()
         )
         for user in users:
-            self.web_pushers[user["username"]] = []
-            for sub in user["notification_tokens"]:
-                self.web_pushers[user["username"]].append(WebPusher(sub))
+            self.web_pushers[user["username"]] = _build_web_pushers(
+                user["username"], user["notification_tokens"]
+            )
 
         # notification and auth config updater
         self.global_config_subscriber = ConfigSubscriber("config/")
@@ -142,10 +156,7 @@ class WebPushClient(Communicator):
                     User.username == user
                 ).execute()
 
-                self.web_pushers[user] = []
-
-                for sub in user_subs:
-                    self.web_pushers[user].append(WebPusher(sub))
+                self.web_pushers[user] = _build_web_pushers(user, user_subs)
 
                 logger.info(
                     f"Cleaned up {len(expired)} notification subscriptions for {user}"

frigate/data_processing/real_time/custom_classification.py

@@ -170,12 +170,7 @@ class CustomStateClassificationProcessor(DeferredRealtimeProcessorApi):
         return None
 
     def process_frame(self, frame_data: dict[str, Any], frame: np.ndarray) -> None:
-        if (
-            not self.model_config.name
-            or not self.model_config.state_config
-            or not self.tensor_input_details
-            or not self.tensor_output_details
-        ):
+        if not self.model_config.name or not self.model_config.state_config:
             return
 
         if self.metrics and self.model_config.name in self.metrics.classification_cps:
@@ -515,12 +510,7 @@ class CustomObjectClassificationProcessor(DeferredRealtimeProcessorApi):
         return best_label, avg_score
 
     def process_frame(self, obj_data: dict[str, Any], frame: np.ndarray) -> None:
-        if (
-            not self.model_config.name
-            or not self.model_config.object_config
-            or not self.tensor_input_details
-            or not self.tensor_output_details
-        ):
+        if not self.model_config.name or not self.model_config.object_config:
             return
 
         if self.metrics and self.model_config.name in self.metrics.classification_cps:

frigate/test/http_api/test_http_export.py

@@ -4,7 +4,6 @@ import tempfile
 import zipfile
 from unittest.mock import patch
 
-from frigate.const import CLIPS_DIR
 from frigate.jobs.export import (
     ExportJob,
     get_export_job_manager,
@@ -950,65 +949,6 @@ class TestHttpExport(BaseTestHttp):
         assert response.status_code == 400
         assert ExportCase.select().count() == 0
 
-    def _batch_export_with_image(self, image_path: str, role: str):
-        with patch(
-            "frigate.api.export.start_export_job",
-            side_effect=lambda _config, job: job.id,
-        ) as start_export_job:
-            with AuthTestClient(self.app) as client:
-                response = client.post(
-                    "/exports/batch",
-                    headers={"remote-user": role, "remote-role": role},
-                    json={
-                        "items": [
-                            {
-                                "camera": "front_door",
-                                "start_time": 110,
-                                "end_time": 150,
-                                "image_path": image_path,
-                            }
-                        ],
-                    },
-                )
-
-        return response, start_export_job
-
-    def test_batch_export_restricted_role_rejects_unreadable_image_path(self):
-        self._insert_recording("rec-front", "front_door", 100, 400)
-        self.app.frigate_config.auth.roles["limited_user"] = ["front_door"]
-
-        for image_path in (
-            f"{CLIPS_DIR}/review/thumb-backyard-123.456-abc.webp",
-            f"{CLIPS_DIR}/thumbs/backyard/123.webp",
-            f"{CLIPS_DIR}/faces/someone/1.webp",
-            f"{CLIPS_DIR}/custom/thumb.jpg",
-        ):
-            response, _ = self._batch_export_with_image(image_path, "limited_user")
-
-            assert response.status_code == 400, image_path
-
-    def test_batch_export_restricted_role_accepts_own_camera_image_path(self):
-        self._insert_recording("rec-front", "front_door", 100, 400)
-        self.app.frigate_config.auth.roles["limited_user"] = ["front_door"]
-        image_path = f"{CLIPS_DIR}/review/thumb-front_door-123.456-abc.webp"
-
-        response, start_export_job = self._batch_export_with_image(
-            image_path, "limited_user"
-        )
-
-        assert response.status_code == 202
-        assert start_export_job.call_args.args[1].image_path == image_path
-
-    def test_batch_export_unrestricted_roles_accept_custom_image_path(self):
-        self._insert_recording("rec-front", "front_door", 100, 400)
-        image_path = f"{CLIPS_DIR}/custom/thumb.jpg"
-
-        for role in ("admin", "viewer"):
-            response, start_export_job = self._batch_export_with_image(image_path, role)
-
-            assert response.status_code == 202, role
-            assert start_export_job.call_args.args[1].image_path == image_path
-
     def test_batch_export_non_admin_can_queue(self):
         self._insert_recording("rec-front", "front_door", 100, 400)
 

frigate/test/test_webpush_registration.py

@@ -1,8 +1,10 @@
 """Tests for push notification subscription validation."""
 
 import unittest
+from base64 import urlsafe_b64encode
 
 from frigate.api.notification import _validate_push_endpoint, _validate_subscription
+from frigate.comms.webpush import _build_web_pushers
 
 VALID_ENDPOINTS = [
     "https://fcm.googleapis.com/fcm/send/dGhpcy1pcy1hLXRva2Vu",
@@ -148,3 +150,16 @@ class TestValidateSubscription(unittest.TestCase):
 
 if __name__ == "__main__":
     unittest.main()
+
+
+class TestBuildWebPushers(unittest.TestCase):
+    def test_skips_subscriptions_with_malformed_keys(self):
+        bad = _subscription(VALID_ENDPOINTS[0])
+        good = _subscription(VALID_ENDPOINTS[0])
+        good["keys"]["p256dh"] = urlsafe_b64encode(b"\x04" + bytes(64)).decode()
+
+        with self.assertLogs("frigate.comms.webpush", level="WARNING"):
+            pushers = _build_web_pushers("viewer", [bad, good])
+
+        self.assertEqual(len(pushers), 1)
+        self.assertEqual(pushers[0].receiver_key, b"\x04" + bytes(64))