From e912d0852b3cd7a12b10365728ff1977422030fe Mon Sep 17 00:00:00 2001 From: Josh Hawkins <32435876+hawkeye217@users.noreply.github.com> Date: Fri, 2 Oct 2026 07:58:57 -0500 Subject: [PATCH 01/18] Fix deleting the only camera when the cameras key has a trailing comment (#24542) --- frigate/api/camera.py | 7 ++++++- frigate/test/test_update_yaml.py | 23 ++++++++++++++++++++++- 2 files changed, 28 insertions(+), 2 deletions(-) diff --git a/frigate/api/camera.py b/frigate/api/camera.py index bc0a318206..6498901b97 100644 --- a/frigate/api/camera.py +++ b/frigate/api/camera.py @@ -35,7 +35,11 @@ from frigate.config.camera.updater import ( ) from frigate.config.env import substitute_frigate_vars from frigate.models import User -from frigate.util.builtin import clean_camera_user_pass, get_record_segment_time +from frigate.util.builtin import ( + clean_camera_user_pass, + clear_orphaned_comments, + get_record_segment_time, +) from frigate.util.camera_cleanup import cleanup_camera_db, cleanup_camera_files from frigate.util.config import find_config_file from frigate.util.image import run_ffmpeg_snapshot @@ -1201,6 +1205,7 @@ async def delete_camera( # Remove camera from config if "cameras" in data and camera_name in data["cameras"]: del data["cameras"][camera_name] + clear_orphaned_comments(data["cameras"], data, "cameras") # Remove camera from auth roles auth = data.get("auth", {}) diff --git a/frigate/test/test_update_yaml.py b/frigate/test/test_update_yaml.py index e9e160c8f4..2f06b480f2 100644 --- a/frigate/test/test_update_yaml.py +++ b/frigate/test/test_update_yaml.py @@ -6,7 +6,7 @@ import unittest from ruamel.yaml import YAML -from frigate.util.builtin import update_yaml_file_bulk +from frigate.util.builtin import clear_orphaned_comments, update_yaml_file_bulk class TestUpdateYaml(unittest.TestCase): @@ -178,6 +178,27 @@ class TestUpdateYaml(unittest.TestCase): assert data["cameras"]["cam1"]["detect"]["fps"] == 5 assert "# tuned for the pi" in self._read() + def test_delete_only_camera_with_comment_on_cameras_key(self): + """Deleting the only camera stays parseable when the cameras key + carries the default config's trailing comment.""" + self._write( + "cameras: # No cameras defined, UI wizard should be used\n" + " cam1:\n" + " enabled: true\n" + "version: 0.18-0\n" + ) + yaml = YAML() + with open(self.config_path) as f: + data = yaml.load(f) + del data["cameras"]["cam1"] + clear_orphaned_comments(data["cameras"], data, "cameras") + with open(self.config_path, "w") as f: + yaml.dump(data, f) + + data = self._load() + assert data["cameras"] == {} + assert data["version"] == "0.18-0" + if __name__ == "__main__": unittest.main(verbosity=2) From cb745d0a504c97cd4c43fd6b6276f41259d7cdb5 Mon Sep 17 00:00:00 2001 From: Josh Hawkins <32435876+hawkeye217@users.noreply.github.com> Date: Mon, 5 Oct 2026 12:26:08 -0500 Subject: [PATCH 02/18] Miscellaneous fixes (0.18) (#24567) * run recording snapshot ffmpeg off the event loop The recording snapshot and Frigate+ recording submit endpoints called get_image_from_recording inline, which is a blocking ffmpeg subprocess with no timeout, so every other request stalled until the frame was decoded. Both calls now run in asyncio.to_thread. * fix camera info dialog overflowing on mobile * adjust test --- frigate/api/media.py | 17 +++- frigate/test/http_api/test_http_media.py | 90 +++++++++++++++++++ .../components/overlay/CameraInfoDialog.tsx | 2 +- 3 files changed, 104 insertions(+), 5 deletions(-) diff --git a/frigate/api/media.py b/frigate/api/media.py index c6fa90c068..d6e2a7f836 100644 --- a/frigate/api/media.py +++ b/frigate/api/media.py @@ -351,8 +351,13 @@ async def get_snapshot_from_recording( mime_type = "png" if format == "png" else "jpeg" config: FrigateConfig = request.app.frigate_config - image_data = get_image_from_recording( - config.ffmpeg, recording.path, time_in_segment, codec, height + image_data = await asyncio.to_thread( + get_image_from_recording, + config.ffmpeg, + recording.path, + time_in_segment, + codec, + height, ) if not image_data: @@ -406,8 +411,12 @@ async def submit_recording_snapshot_to_plus( config: FrigateConfig = request.app.frigate_config recording: Recordings = recording_query.get() time_in_segment = frame_time - recording.start_time - image_data = get_image_from_recording( - config.ffmpeg, recording.path, time_in_segment, "png" + image_data = await asyncio.to_thread( + get_image_from_recording, + config.ffmpeg, + recording.path, + time_in_segment, + "png", ) if not image_data: diff --git a/frigate/test/http_api/test_http_media.py b/frigate/test/http_api/test_http_media.py index b2d83cbc8a..252d8a0af1 100644 --- a/frigate/test/http_api/test_http_media.py +++ b/frigate/test/http_api/test_http_media.py @@ -1,6 +1,9 @@ """Unit tests for recordings/media API endpoints.""" +import threading +from concurrent.futures import ThreadPoolExecutor from datetime import UTC, datetime +from unittest.mock import patch import pytz from fastapi import Request @@ -527,3 +530,90 @@ class TestHttpMedia(BaseTestHttp): assert response.status_code == 200 assert response.json() == [{"start_time": 1010, "end_time": 1030}] + + def _insert_recording(self, id: str, start_time: float, end_time: float) -> None: + Recordings.insert( + id=id, + path=f"/media/recordings/{id}.mp4", + camera="front_door", + start_time=start_time, + end_time=end_time, + duration=end_time - start_time, + motion=0, + ).execute() + + def test_recording_snapshot_does_not_block_event_loop(self): + self._insert_recording("snapshot", 1000, 1010) + started = threading.Event() + release = threading.Event() + finished = threading.Event() + + def slow_image(*args): + started.set() + release.wait(timeout=5) + finished.set() + return b"jpeg" + + with ( + AuthTestClient(self.app) as client, + patch("frigate.api.media.get_image_from_recording", slow_image), + ThreadPoolExecutor(max_workers=1) as executor, + ): + snapshot = executor.submit( + client.get, "/front_door/recordings/1005/snapshot.jpg" + ) + self.assertTrue(started.wait(timeout=2)) + version = client.get("/version") + blocked = finished.is_set() + release.set() + + self.assertEqual(version.status_code, 200) + self.assertFalse(blocked) + self.assertEqual(snapshot.result().status_code, 200) + + def test_recording_snapshot_hit_returns_image(self): + self._insert_recording("snapshot", 1000, 1010) + + with ( + AuthTestClient(self.app) as client, + patch("frigate.api.media.get_image_from_recording", return_value=b"jpeg"), + ): + response = client.get("/front_door/recordings/1005/snapshot.jpg") + + self.assertEqual(response.status_code, 200) + self.assertEqual(response.headers["Content-Type"], "image/jpeg") + self.assertEqual(response.content, b"jpeg") + + def test_recording_snapshot_misses_return_404(self): + self._insert_recording("snapshot", 1000, 1010) + + with AuthTestClient(self.app) as client: + absent = client.get("/front_door/recordings/2000/snapshot.jpg") + rounded_miss = client.get("/front_door/recordings/1010.5/snapshot.jpg") + + self.assertEqual(absent.status_code, 404) + self.assertEqual(rounded_miss.status_code, 404) + + def test_plus_snapshot_miss_returns_404_without_image_lookup(self): + with ( + AuthTestClient(self.app) as client, + patch("frigate.api.media.get_image_from_recording") as image, + ): + response = client.post("/front_door/plus/1005") + + self.assertEqual(response.status_code, 404) + image.assert_not_called() + + def test_plus_snapshot_hit_uploads_image(self): + self._insert_recording("snapshot", 1000, 1010) + + with ( + AuthTestClient(self.app) as client, + patch("frigate.api.media.get_image_from_recording", return_value=b"png"), + patch("frigate.api.media.cv2.imdecode", return_value="frame"), + patch.object(self.app.frigate_config.plus_api, "upload_image") as upload, + ): + response = client.post("/front_door/plus/1005") + + self.assertEqual(response.status_code, 200) + upload.assert_called_once_with("frame", "front_door") diff --git a/web/src/components/overlay/CameraInfoDialog.tsx b/web/src/components/overlay/CameraInfoDialog.tsx index 14755a2015..3696f3175c 100644 --- a/web/src/components/overlay/CameraInfoDialog.tsx +++ b/web/src/components/overlay/CameraInfoDialog.tsx @@ -91,7 +91,7 @@ export default function CameraInfoDialog({ open={showCameraInfoDialog} onOpenChange={setShowCameraInfoDialog} > - + {t("cameras.info.cameraProbeInfo", { From f929134a1b7c23c2dbc25f293def0eaee70d62e0 Mon Sep 17 00:00:00 2001 From: Nicolas Mowen Date: Wed, 7 Oct 2026 09:22:47 -0600 Subject: [PATCH 03/18] Revise embedding model details in config.md (#24586) Updated model descriptions for embeddings in the configuration documentation. --- docs/docs/configuration/genai/config.md | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/docs/docs/configuration/genai/config.md b/docs/docs/configuration/genai/config.md index a3cabcbace..49cc2a4987 100644 --- a/docs/docs/configuration/genai/config.md +++ b/docs/docs/configuration/genai/config.md @@ -73,9 +73,10 @@ You must use a vision-capable model with Frigate. The following models are recom The `embeddings` role needs a different kind of model. Text queries are matched against the stored image embeddings, so the model must be trained to place images and text into the same vector space. A chat or description model will still return vectors when asked, but those vectors are not trained for retrieval and text searches will return poor matches with no error to indicate why. -| Model | Notes | -| -------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| `qwen3-vl-embedding` | Multimodal embeddings for [Semantic Search](/configuration/semantic_search#genai-provider). Must be served by llama.cpp started with `--embeddings` and `--mmproj`. | +| Model | Notes | +| -------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `embeddinggemma-2` | Multimodal embeddings for [Semantic Search](/configuration/semantic_search#genai-provider). Strong semantic search accuracy with efficient inference on a small model. | +| `qwen3-vl-embedding` | Multimodal embeddings for [Semantic Search](/configuration/semantic_search#genai-provider). Good performance, large model that requires strong hardware for inference. | :::info From 2273cf2d502dd7f4182ad13e7f972fdb5fda6907 Mon Sep 17 00:00:00 2001 From: Josh Hawkins <32435876+hawkeye217@users.noreply.github.com> Date: Thu, 8 Oct 2026 07:56:19 -0500 Subject: [PATCH 04/18] remove annke firmware workaround for autotracking (#24594) see https://github.com/blakeblackshear/frigate/discussions/24593 --- docs/docs/configuration/cameras.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/docs/configuration/cameras.md b/docs/docs/configuration/cameras.md index 640b78841f..66bf008ea0 100644 --- a/docs/docs/configuration/cameras.md +++ b/docs/docs/configuration/cameras.md @@ -234,7 +234,7 @@ The FeatureList on the [ONVIF Conformant Products Database](https://www.onvif.or | Amcrest ASH21 | ✅ | ❌ | ONVIF service port: 80 | | Amcrest IP4M-S2112EW-AI | ✅ | ❌ | FOV relative movement not supported. | | Amcrest IP5M-1190EW | ✅ | ❌ | ONVIF Port: 80. FOV relative movement not supported. | -| Annke CZ504 | ✅ | ✅ | Annke support provide specific firmware ([V5.7.1 build 250227](https://github.com/pierrepinon/annke_cz504/raw/refs/heads/main/digicap_V5-7-1_build_250227.dav)) to fix issue with ONVIF "TranslationSpaceFov" | +| Annke CZ504 | ✅ | ❌ | | | Axis Q-6155E | ✅ | ❌ | ONVIF service port: 80; Camera does not support MoveStatus. | | Ctronics PTZ | ✅ | ❌ | | | Dahua | ✅ | ✅ | Some low-end Dahuas (lite series, picoo series (commonly), among others) have been reported to not support autotracking. These models usually don't have a four digit model number with chassis prefix and options postfix (e.g. DH-P5AE-PV vs DH-SD49825GB-HNR). | From 33cb8d987d3b2543378408b227a4a63eec4f150b Mon Sep 17 00:00:00 2001 From: Josh Hawkins <32435876+hawkeye217@users.noreply.github.com> Date: Fri, 9 Oct 2026 07:44:53 -0500 Subject: [PATCH 05/18] pass only the request path in X-Original-URL (#24602) nginx built X-Original-URL from the client's Host header plus the request path, so a Host containing '#' or '?' pushed the real path into the URL fragment or query. The media and go2rtc auth checks then saw an empty path and allowed restricted-role users to read other cameras' clips, recordings, exports and live streams. The header now carries only $request_uri, which is all the auth checks need. --- docker/main/rootfs/usr/local/nginx/conf/auth_location.conf | 2 +- docker/main/rootfs/usr/local/nginx/conf/proxy.conf | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/docker/main/rootfs/usr/local/nginx/conf/auth_location.conf b/docker/main/rootfs/usr/local/nginx/conf/auth_location.conf index 285a3d81b0..619a2b3ff1 100644 --- a/docker/main/rootfs/usr/local/nginx/conf/auth_location.conf +++ b/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 diff --git a/docker/main/rootfs/usr/local/nginx/conf/proxy.conf b/docker/main/rootfs/usr/local/nginx/conf/proxy.conf index a3aacc3095..7ab5b58d15 100644 --- a/docker/main/rootfs/usr/local/nginx/conf/proxy.conf +++ b/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; From c06a97378c50e374936ea8c0810c152f6c732bea Mon Sep 17 00:00:00 2001 From: Josh Hawkins <32435876+hawkeye217@users.noreply.github.com> Date: Fri, 9 Oct 2026 08:05:21 -0500 Subject: [PATCH 06/18] skip malformed webpush subscriptions instead of failing startup (#24604) Registration only checked that the p256dh and auth keys were non-empty strings, so any authenticated user could store a subscription that WebPusher refuses to construct. WebPushClient built one WebPusher per stored subscription with no error handling during init_dispatcher, so a single bad row raised before the camera processes and API started, and it did so again on every restart because the row was reloaded from the database. Building the pushers now logs and skips a subscription that fails to construct, both at startup and when expired subscriptions are cleaned up. --- frigate/comms/webpush.py | 25 ++++++++++++++++------- frigate/test/test_webpush_registration.py | 15 ++++++++++++++ 2 files changed, 33 insertions(+), 7 deletions(-) diff --git a/frigate/comms/webpush.py b/frigate/comms/webpush.py index 34a0e57508..45a6edba64 100644 --- a/frigate/comms/webpush.py +++ b/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}" diff --git a/frigate/test/test_webpush_registration.py b/frigate/test/test_webpush_registration.py index 16e0129152..733659489a 100644 --- a/frigate/test/test_webpush_registration.py +++ b/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)) From e66da62db34653b8d884fbe3b6ebd15a78ea6765 Mon Sep 17 00:00:00 2001 From: Josh Hawkins <32435876+hawkeye217@users.noreply.github.com> Date: Fri, 9 Oct 2026 08:09:31 -0500 Subject: [PATCH 07/18] 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. --- .../real_time/custom_classification.py | 14 ++------------ 1 file changed, 2 insertions(+), 12 deletions(-) diff --git a/frigate/data_processing/real_time/custom_classification.py b/frigate/data_processing/real_time/custom_classification.py index e3b0e23ed8..9ccb7f3233 100644 --- a/frigate/data_processing/real_time/custom_classification.py +++ b/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: From 7786da465fd15ab16ab3d445e1f6365bb8886abd Mon Sep 17 00:00:00 2001 From: Josh Hawkins <32435876+hawkeye217@users.noreply.github.com> Date: Fri, 9 Oct 2026 08:34:55 -0500 Subject: [PATCH 08/18] Check export thumbnail image access (#24603) * require export image_path to belong to the exported camera image_path was only checked for containment under the clips directory, which every camera shares, so a user restricted to one camera could start an export of that camera and have the server copy another camera's image, or a file from an admin-only directory like faces, into the export thumbnail. The path now also has to resolve to the camera being exported, using the same classifier that gates media requests. * check image_path against the caller's read access instead of the exported camera Requiring the image to belong to the exported camera rejected custom images that a script saves under the clips directory for its own export pipeline. The path is now accepted whenever the caller's role could read it as media, so admins and unrestricted roles can use any image there and restricted roles are limited to their own cameras. --- frigate/api/export.py | 22 +++++++-- frigate/test/http_api/test_http_export.py | 60 +++++++++++++++++++++++ 2 files changed, 79 insertions(+), 3 deletions(-) diff --git a/frigate/api/export.py b/frigate/api/export.py index 817dbe7ba8..361300daa7 100644 --- a/frigate/api/export.py +++ b/frigate/api/export.py @@ -2,6 +2,7 @@ import datetime import logging +import os import random import string import time @@ -56,6 +57,7 @@ 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, @@ -130,6 +132,7 @@ 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: @@ -137,6 +140,15 @@ 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"}, @@ -680,7 +692,7 @@ def export_recordings_batch( sanitized_images: list[str | None] = [] for item in body.items: existing_image, image_validation_error = _sanitize_existing_image( - item.image_path + request, item.image_path ) if image_validation_error is not None: return image_validation_error @@ -827,7 +839,9 @@ def export_recording( playback_source = body.source friendly_name = body.name - existing_image, image_validation_error = _sanitize_existing_image(body.image_path) + existing_image, image_validation_error = _sanitize_existing_image( + request, body.image_path + ) if image_validation_error is not None: return image_validation_error @@ -965,7 +979,9 @@ def export_recording_custom( playback_source = body.source friendly_name = body.name - existing_image, image_validation_error = _sanitize_existing_image(body.image_path) + existing_image, image_validation_error = _sanitize_existing_image( + request, body.image_path + ) if image_validation_error is not None: return image_validation_error ffmpeg_input_args = body.ffmpeg_input_args diff --git a/frigate/test/http_api/test_http_export.py b/frigate/test/http_api/test_http_export.py index 44eb0c2c4a..e31a04ad2d 100644 --- a/frigate/test/http_api/test_http_export.py +++ b/frigate/test/http_api/test_http_export.py @@ -4,6 +4,7 @@ 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, @@ -949,6 +950,65 @@ 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) From cc86519603e4a60d0973e4386ed6716442ec6ab4 Mon Sep 17 00:00:00 2001 From: Josh Hawkins <32435876+hawkeye217@users.noreply.github.com> Date: Fri, 9 Oct 2026 14:28:00 -0500 Subject: [PATCH 09/18] Lock ZMQ sockets shared across threads (#24607) * lock zmq sockets shared across threads Several zmq sockets were used from more than one thread at a time, which zmq does not allow. In the embeddings process, GenAI review and object description threads shared one requestor, so overlapping calls silently dropped the second write and could eventually leave a thread waiting forever on a reply that had already arrived, after which nothing was saved until restart. The API's embeddings requestor had the same problem, concurrent config publishes could mix the topic of one update with the payload of another, and the webpush config subscribers were drained from every publishing thread, which could abort the process. Each socket now has a lock. The embeddings requestor lock does not wait, so an overlapping call still returns empty right away instead of stalling the API behind a slow reply. * add test --- frigate/comms/config_updater.py | 7 +- frigate/comms/embeddings_updater.py | 8 + frigate/comms/inter_process.py | 12 +- frigate/comms/webpush.py | 44 +++--- frigate/test/test_comms_thread_safety.py | 193 +++++++++++++++++++++++ 5 files changed, 236 insertions(+), 28 deletions(-) create mode 100644 frigate/test/test_comms_thread_safety.py diff --git a/frigate/comms/config_updater.py b/frigate/comms/config_updater.py index 4552abc111..46ca57cbef 100644 --- a/frigate/comms/config_updater.py +++ b/frigate/comms/config_updater.py @@ -1,6 +1,7 @@ """Facilitates communication between processes.""" import multiprocessing as mp +import threading from _pickle import UnpicklingError from multiprocessing.synchronize import Event as MpEvent from typing import Any @@ -18,11 +19,13 @@ class ConfigPublisher: self.socket = self.context.socket(zmq.PUB) self.socket.bind(SOCKET_PUB_SUB) self.stop_event: MpEvent = mp.Event() + self.lock = threading.Lock() def publish(self, topic: str, payload: Any) -> None: """There is no communication back to the processes.""" - self.socket.send_string(topic, flags=zmq.SNDMORE) - self.socket.send_pyobj(payload) + with self.lock: + self.socket.send_string(topic, flags=zmq.SNDMORE) + self.socket.send_pyobj(payload) def stop(self) -> None: self.stop_event.set() diff --git a/frigate/comms/embeddings_updater.py b/frigate/comms/embeddings_updater.py index cd83709f0b..08ebb96f97 100644 --- a/frigate/comms/embeddings_updater.py +++ b/frigate/comms/embeddings_updater.py @@ -1,6 +1,7 @@ """Facilitates communication between processes.""" import logging +import threading from collections.abc import Callable from enum import Enum from typing import Any @@ -78,14 +79,21 @@ class EmbeddingsRequestor: self.context = zmq.Context() self.socket = self.context.socket(zmq.REQ) self.socket.connect(SOCKET_REP_REQ) + self.lock = threading.Lock() def send_data(self, topic: str, data: Any) -> Any: """Sends data and then waits for reply.""" + # an overlapping call fails fast so a slow reply can't stall the API + if not self.lock.acquire(blocking=False): + return "" + try: self.socket.send_json((topic, data)) return self.socket.recv_json() except zmq.ZMQError: return "" + finally: + self.lock.release() def stop(self) -> None: self.socket.close() diff --git a/frigate/comms/inter_process.py b/frigate/comms/inter_process.py index 6897b12ebf..f90f2d4545 100644 --- a/frigate/comms/inter_process.py +++ b/frigate/comms/inter_process.py @@ -73,14 +73,16 @@ class InterProcessRequestor: self.context = zmq.Context() self.socket = self.context.socket(zmq.REQ) self.socket.connect(SOCKET_REP_REQ) + self.lock = threading.Lock() def send_data(self, topic: str, data: Any) -> Any: """Sends data and then waits for reply.""" - try: - self.socket.send_json((topic, data)) - return self.socket.recv_json() - except zmq.ZMQError: - return "" + with self.lock: + try: + self.socket.send_json((topic, data)) + return self.socket.recv_json() + except zmq.ZMQError: + return "" def stop(self) -> None: self.socket.close(linger=0) diff --git a/frigate/comms/webpush.py b/frigate/comms/webpush.py index 45a6edba64..ea4ee3fd6d 100644 --- a/frigate/comms/webpush.py +++ b/frigate/comms/webpush.py @@ -70,6 +70,7 @@ class WebPushClient(Communicator): for c in self.config.cameras.values() } self.suspension_broadcaster: Callable[[str, Any, bool], None] | None = None + self.config_lock = threading.Lock() self.last_camera_notification_time: dict[str, float] = { c.name: 0 # type: ignore[misc] for c in self.config.cameras.values() @@ -205,29 +206,30 @@ class WebPushClient(Communicator): def publish(self, topic: str, payload: Any, retain: bool = False) -> None: """Wrapper for publishing when client is in valid state.""" - # check for updated global config (notifications, auth) - while True: - config_topic, config_payload = ( - self.global_config_subscriber.check_for_update() - ) - if config_topic is None: - break - if config_topic == "config/notifications" and config_payload: - self.config.notifications = config_payload - elif config_topic == "config/auth": - if isinstance(config_payload, AuthConfig): - self.config.auth = config_payload + with self.config_lock: + # check for updated global config (notifications, auth) + while True: + config_topic, config_payload = ( + self.global_config_subscriber.check_for_update() + ) + if config_topic is None: + break + if config_topic == "config/notifications" and config_payload: + self.config.notifications = config_payload + elif config_topic == "config/auth": + if isinstance(config_payload, AuthConfig): + self.config.auth = config_payload + self._refresh_user_cameras() + + updates = self.config_subscriber.check_for_updates() + + if "add" in updates: + for camera in updates["add"]: + self.suspended_cameras[camera] = 0 + self.last_camera_notification_time[camera] = 0 + self._refresh_user_cameras() - updates = self.config_subscriber.check_for_updates() - - if "add" in updates: - for camera in updates["add"]: - self.suspended_cameras[camera] = 0 - self.last_camera_notification_time[camera] = 0 - - self._refresh_user_cameras() - if topic == "reviews": decoded = json.loads(payload) camera = decoded["before"]["camera"] diff --git a/frigate/test/test_comms_thread_safety.py b/frigate/test/test_comms_thread_safety.py new file mode 100644 index 0000000000..e58be48278 --- /dev/null +++ b/frigate/test/test_comms_thread_safety.py @@ -0,0 +1,193 @@ +"""Tests for sharing one zmq socket wrapper across threads.""" + +import os +import random +import tempfile +import threading +import time +import unittest +from unittest.mock import patch + +import zmq + +from frigate.comms import config_updater, embeddings_updater, inter_process +from frigate.comms.config_updater import ConfigPublisher, ConfigSubscriber +from frigate.comms.embeddings_updater import EmbeddingsRequestor +from frigate.comms.inter_process import InterProcessRequestor +from frigate.comms.webpush import WebPushClient + +THREADS = 8 +CALLS = 100 + +# nothing reads until the end, so the total stays under the default zmq HWM +PUBLISH_CALLS = 100 + + +class TestSharedRequestor(unittest.TestCase): + def setUp(self) -> None: + self.tmp = tempfile.TemporaryDirectory() + self.address = f"ipc://{os.path.join(self.tmp.name, 'comms')}" + self.stop = threading.Event() + self.context = zmq.Context() + self.responder = self.context.socket(zmq.REP) + self.responder.bind(self.address) + self.responder_thread = threading.Thread(target=self._respond) + self.responder_thread.start() + + def tearDown(self) -> None: + self.stop.set() + self.responder_thread.join() + self.responder.close(linger=0) + self.context.destroy(linger=0) + self.tmp.cleanup() + + def _respond(self) -> None: + while not self.stop.is_set(): + ready, _, _ = zmq.select([self.responder], [], [], 0.1) + + if ready: + self.responder.recv_json() + time.sleep(random.uniform(0, 0.002)) + self.responder.send_json(["ok"]) + + def _call_from_threads(self, requestor) -> list: + results: list = [] + + def call() -> None: + for _ in range(CALLS): + # jitter lands some calls on the instant another reply arrives + time.sleep(random.uniform(0, 0.002)) + results.append(requestor.send_data("topic", {"key": "value"})) + + threads = [threading.Thread(target=call, daemon=True) for _ in range(THREADS)] + + for thread in threads: + thread.start() + + for thread in threads: + thread.join(30) + + self.assertFalse(any(thread.is_alive() for thread in threads)) + return results + + def test_inter_process_requestor_waits_for_overlapping_calls(self) -> None: + with patch.object(inter_process, "SOCKET_REP_REQ", self.address): + requestor = InterProcessRequestor() + + results = self._call_from_threads(requestor) + self.assertEqual(results, [["ok"]] * THREADS * CALLS) + requestor.stop() + + def test_embeddings_requestor_survives_overlapping_calls(self) -> None: + with patch.object(embeddings_updater, "SOCKET_REP_REQ", self.address): + requestor = EmbeddingsRequestor() + + # an overlapping call may fail fast, but the socket has to stay usable + results = self._call_from_threads(requestor) + self.assertEqual(len(results), THREADS * CALLS) + self.assertEqual(requestor.send_data("topic", {}), ["ok"]) + requestor.stop() + + +class TestSharedConfigPublisher(unittest.TestCase): + def test_frames_stay_paired_across_threads(self) -> None: + with tempfile.TemporaryDirectory() as tmp: + address = f"ipc://{os.path.join(tmp, 'config')}" + + with patch.object(config_updater, "SOCKET_PUB_SUB", address): + publisher = ConfigPublisher() + subscriber = ConfigSubscriber("config/") + + # a subscription that is still connecting drops messages + subscriber.socket.setsockopt(zmq.RCVTIMEO, 5000) + + while True: + publisher.publish("config/ready", None) + + if zmq.select([subscriber.socket], [], [], 0.05)[0]: + break + + def publish(index: int) -> None: + topic = f"config/cameras/camera_{index}" + + for _ in range(PUBLISH_CALLS): + publisher.publish(topic, topic) + + threads = [ + threading.Thread(target=publish, args=(i,)) for i in range(THREADS) + ] + + for thread in threads: + thread.start() + + for thread in threads: + thread.join() + + publisher.publish("config/done", None) + received = 0 + + while True: + topic = subscriber.socket.recv_string() + payload = subscriber.socket.recv_pyobj() + + if topic == "config/done": + break + + if topic != "config/ready": + self.assertEqual(topic, payload) + received += 1 + + self.assertEqual(received, THREADS * PUBLISH_CALLS) + subscriber.stop() + publisher.stop() + + +class FakeConfigSubscriber: + """Records whether two threads read at the same time.""" + + def __init__(self) -> None: + self.reading = threading.Lock() + self.overlapped = False + + def _read(self) -> None: + if not self.reading.acquire(blocking=False): + self.overlapped = True + return + + time.sleep(0.001) + self.reading.release() + + def check_for_update(self) -> tuple[None, None]: + self._read() + return (None, None) + + def check_for_updates(self) -> dict: + self._read() + return {} + + +class TestSharedWebPushClient(unittest.TestCase): + def test_config_subscribers_read_by_one_thread_at_a_time(self) -> None: + client = WebPushClient.__new__(WebPushClient) + client.config_lock = threading.Lock() + client.global_config_subscriber = FakeConfigSubscriber() + client.config_subscriber = FakeConfigSubscriber() + + def publish() -> None: + for _ in range(20): + client.publish("topic", "payload") + + threads = [threading.Thread(target=publish) for _ in range(THREADS)] + + for thread in threads: + thread.start() + + for thread in threads: + thread.join() + + self.assertFalse(client.global_config_subscriber.overlapped) + self.assertFalse(client.config_subscriber.overlapped) + + +if __name__ == "__main__": + unittest.main() From 2e92251aa593e54ae8b98cd8c4a24c958952721d Mon Sep 17 00:00:00 2001 From: Josh Hawkins <32435876+hawkeye217@users.noreply.github.com> Date: Sat, 10 Oct 2026 07:59:52 -0500 Subject: [PATCH 10/18] save genai debug thumbnails on early requests (#24615) debug_save_thumbnails was only honored when a description was sent at the end of a tracked object, so requests triggered by after_significant_updates never wrote their images to genai-requests. The save now lives in a helper that both paths call. --- .../post/object_descriptions.py | 49 +++++++++++-------- 1 file changed, 29 insertions(+), 20 deletions(-) diff --git a/frigate/data_processing/post/object_descriptions.py b/frigate/data_processing/post/object_descriptions.py index 9612ce7be9..a103b2f929 100644 --- a/frigate/data_processing/post/object_descriptions.py +++ b/frigate/data_processing/post/object_descriptions.py @@ -105,6 +105,10 @@ class ObjectDescriptionProcessor(PostProcessorApi): logger.debug(f"{camera} sending early request to GenAI") self.early_request_sent[data["id"]] = True + + if camera_config.objects.genai.debug_save_thumbnails: + self._save_debug_thumbnails(data["id"]) + # Copy thumbnails to avoid holding references after cleanup thumbnails_copy = [ data["thumbnail"][:] if data.get("thumbnail") else None @@ -297,26 +301,7 @@ class ObjectDescriptionProcessor(PostProcessorApi): ) if camera_config.objects.genai.debug_save_thumbnails and num_thumbnails > 0: - logger.debug(f"Saving {num_thumbnails} thumbnails for event {event_id}") - - Path(os.path.join(CLIPS_DIR, f"genai-requests/{event_id}")).mkdir( - parents=True, exist_ok=True - ) - - for idx, data in enumerate(self.tracked_events[event_id], 1): - jpg_bytes: bytes | None = data["thumbnail"] - - if jpg_bytes is None: - logger.warning(f"Unable to save thumbnail {idx} for {event_id}.") - else: - with open( - os.path.join( - CLIPS_DIR, - f"genai-requests/{event_id}/{idx}.jpg", - ), - "wb", - ) as j: - j.write(jpg_bytes) + self._save_debug_thumbnails(event_id) # Generate the description. Call happens in a thread since it is network bound. threading.Thread( @@ -332,6 +317,30 @@ class ObjectDescriptionProcessor(PostProcessorApi): # Clean up tracked events and early request state self.cleanup_event(event_id) + def _save_debug_thumbnails(self, event_id: str) -> None: + """Write the tracked thumbnails for an event to the genai-requests dir.""" + tracked = self.tracked_events.get(event_id, []) + logger.debug(f"Saving {len(tracked)} thumbnails for event {event_id}") + + Path(os.path.join(CLIPS_DIR, f"genai-requests/{event_id}")).mkdir( + parents=True, exist_ok=True + ) + + for idx, data in enumerate(tracked, 1): + jpg_bytes: bytes | None = data["thumbnail"] + + if jpg_bytes is None: + logger.warning(f"Unable to save thumbnail {idx} for {event_id}.") + else: + with open( + os.path.join( + CLIPS_DIR, + f"genai-requests/{event_id}/{idx}.jpg", + ), + "wb", + ) as j: + j.write(jpg_bytes) + def _genai_embed_description(self, event: Event, thumbnails: list[bytes]) -> None: """Embed the description for an event.""" start = datetime.datetime.now().timestamp() From c75f7c32414ec749ec5a5bcd9de3718718abd365 Mon Sep 17 00:00:00 2001 From: Josh Hawkins <32435876+hawkeye217@users.noreply.github.com> Date: Sat, 10 Oct 2026 09:12:50 -0500 Subject: [PATCH 11/18] Reject dot-only usernames (#24611) * reject usernames made only of dots A username of "." or ".." passed validation, but browsers resolve those as path segments, so requests to /users/{username} never reached the API and the user could not be deleted or edited from the UI. User creation now rejects dot-only names in both the API and the create user dialog. * show a specific error for dot-only usernames Entering "." or ".." in the create user dialog showed the message listing periods as allowed characters, which didn't explain the rejection. The dialog now checks for dot-only names separately and shows its own message. --- frigate/api/auth.py | 2 +- frigate/test/http_api/test_http_users.py | 33 +++++++++++++++++++ web/public/locales/en/views/settings.json | 1 + .../components/overlay/CreateUserDialog.tsx | 3 ++ 4 files changed, 38 insertions(+), 1 deletion(-) create mode 100644 frigate/test/http_api/test_http_users.py diff --git a/frigate/api/auth.py b/frigate/api/auth.py index e0090e32cd..5f4de5204f 100644 --- a/frigate/api/auth.py +++ b/frigate/api/auth.py @@ -912,7 +912,7 @@ def create_user( HASH_ITERATIONS = request.app.frigate_config.auth.hash_iterations config_roles = list(request.app.frigate_config.auth.roles.keys()) - if not re.match("^[A-Za-z0-9._]+$", body.username): + if not re.match(r"^(?!\.+$)[A-Za-z0-9._]+$", body.username): return JSONResponse(content={"message": "Invalid username"}, status_code=400) if body.role not in config_roles: diff --git a/frigate/test/http_api/test_http_users.py b/frigate/test/http_api/test_http_users.py new file mode 100644 index 0000000000..a964e7c0af --- /dev/null +++ b/frigate/test/http_api/test_http_users.py @@ -0,0 +1,33 @@ +"""Tests for user creation.""" + +from unittest.mock import MagicMock + +from frigate.models import User +from frigate.test.http_api.base_http_test import AuthTestClient, BaseTestHttp + +PASSWORD = "a-valid-password-123" + + +class TestCreateUser(BaseTestHttp): + def setUp(self): + super().setUp([User]) + self.app = super().create_app() + self.app.config_publisher = MagicMock() + + def _create(self, username: str): + with AuthTestClient(self.app) as client: + return client.post( + "/users", + json={"username": username, "password": PASSWORD, "role": "viewer"}, + ) + + def test_rejects_dot_only_usernames(self): + # browsers resolve these as URL path segments, so the per-user + # endpoints can never be reached for them + for username in (".", "..", "..."): + assert self._create(username).status_code == 400 + assert User.get_or_none(User.username == username) is None + + def test_accepts_username_containing_dots(self): + assert self._create("john.doe_1").status_code == 200 + assert User.get_or_none(User.username == "john.doe_1") is not None diff --git a/web/public/locales/en/views/settings.json b/web/public/locales/en/views/settings.json index 1f50863e46..0c26619bc7 100644 --- a/web/public/locales/en/views/settings.json +++ b/web/public/locales/en/views/settings.json @@ -1054,6 +1054,7 @@ "title": "Create New User", "desc": "Add a new user account and specify a role for access to areas of the Frigate UI.", "usernameOnlyInclude": "Username may only include letters, numbers, . or _", + "usernameOnlyDots": "Username cannot contain only periods", "confirmPassword": "Please confirm your password" }, "deleteUser": { diff --git a/web/src/components/overlay/CreateUserDialog.tsx b/web/src/components/overlay/CreateUserDialog.tsx index 3b003f2611..2953a29cb9 100644 --- a/web/src/components/overlay/CreateUserDialog.tsx +++ b/web/src/components/overlay/CreateUserDialog.tsx @@ -82,6 +82,9 @@ export default function CreateUserDialog({ .min(1, t("users.dialog.form.usernameIsRequired")) .regex(/^[A-Za-z0-9._]+$/, { message: t("users.dialog.createUser.usernameOnlyInclude"), + }) + .regex(/[^.]/, { + message: t("users.dialog.createUser.usernameOnlyDots"), }), password: z .string() From 9d9de6263e1bbd413db369a0423e95de65721998 Mon Sep 17 00:00:00 2001 From: Josh Hawkins <32435876+hawkeye217@users.noreply.github.com> Date: Sat, 10 Oct 2026 12:24:00 -0500 Subject: [PATCH 12/18] only show camera group edit button on live grid to admins (#24622) The pencil button on the desktop live grid opened the camera group edit dialog for any role, so viewers could change settings and then hit an admin-only error on save. The button now renders only for admins, matching the sidebar and mobile entry points. --- web/src/views/live/DraggableGridLayout.tsx | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/web/src/views/live/DraggableGridLayout.tsx b/web/src/views/live/DraggableGridLayout.tsx index 3428655278..ea587df091 100644 --- a/web/src/views/live/DraggableGridLayout.tsx +++ b/web/src/views/live/DraggableGridLayout.tsx @@ -32,6 +32,7 @@ import { Skeleton } from "@/components/ui/skeleton"; import { useResizeObserver } from "@/hooks/resize-observer"; import { isEqual } from "lodash"; import useSWR from "swr"; +import { useIsAdmin } from "@/hooks/use-is-admin"; import { isDesktop, isMobile } from "react-device-detect"; import BirdseyeLivePlayer from "@/components/player/BirdseyeLivePlayer"; import LivePlayer from "@/components/player/LivePlayer"; @@ -97,6 +98,7 @@ export default function DraggableGridLayout({ }: DraggableGridLayoutProps) { const { t } = useTranslation(["views/live"]); const { data: config } = useSWR("config"); + const isAdmin = useIsAdmin(); const birdseyeConfig = useMemo(() => config?.birdseye, [config]); // preferred live modes per camera @@ -723,7 +725,7 @@ export default function DraggableGridLayout({ {!isEditMode && ( <> - {!fullscreen && ( + {!fullscreen && isAdmin && (
Date: Sun, 11 Oct 2026 06:57:15 -0500 Subject: [PATCH 13/18] clarify intel gpu stats requirements for pre-broadwell gpus (#24632) The i915 driver never publishes per-client usage counters on GPUs older than Broadwell, so Haswell and older users got a warning telling them to upgrade to kernel 6.5 even on 6.12. The warning and docs now state the Broadwell requirement alongside the kernel 6.5 requirement for 12th gen and newer. --- docs/docs/configuration/hardware_acceleration_video.md | 3 ++- frigate/util/services.py | 10 ++++++---- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/docs/docs/configuration/hardware_acceleration_video.md b/docs/docs/configuration/hardware_acceleration_video.md index 360ca1a50c..25cdaf1b12 100644 --- a/docs/docs/configuration/hardware_acceleration_video.md +++ b/docs/docs/configuration/hardware_acceleration_video.md @@ -132,7 +132,8 @@ ffmpeg: Frigate reads Intel GPU utilization directly from the kernel's per-client DRM usage counters exposed at `/proc//fdinfo/`. This requires: -- Linux kernel **5.19 or newer** for the `i915` driver, or any release of the `xe` driver. +- An Intel GPU from Broadwell (5th gen) or newer. Older GPUs do not expose these counters on any kernel. +- Linux kernel **5.19 or newer** for the `i915` driver (**6.5 or newer** on 12th gen and newer), or any release of the `xe` driver. - Frigate running with permission to read other processes' fdinfo. Running as root inside the container (the default) satisfies this; non-root setups may need `CAP_SYS_PTRACE`. No `intel_gpu_top` binary, `CAP_PERFMON`, privileged mode, or `perf_event_paranoid` tuning is required. diff --git a/frigate/util/services.py b/frigate/util/services.py index 38737907ef..de17e29711 100644 --- a/frigate/util/services.py +++ b/frigate/util/services.py @@ -487,8 +487,9 @@ def get_intel_gpu_stats( Each DRM client FD exposes monotonic per-engine busy counters via /proc//fdinfo/. For i915 this requires kernel 6.5 or newer: earlier kernels omit the per-engine counters whenever GuC submission is - active, which is the default on 12th gen and newer. Xe has exposed them - since its first release. We sample twice and divide busy-time deltas by + active, which is the default on 12th gen and newer. i915 never exposes + them on GPUs older than Broadwell (Gen8). Xe has exposed them since its + first release. We sample twice and divide busy-time deltas by wall-clock to derive utilization. Render/3D and Compute are pooled into "compute"; Video and VideoEnhance into "dec". Overall "gpu" is the sum of those pools (clamped to 100%). @@ -557,10 +558,11 @@ def get_intel_gpu_stats( # i915 suppresses per-client engine counters while GuC submission is # active on kernels older than 6.5 (kernel commit 1324680a80eb lifted # this), which covers stock Debian 12 and Ubuntu 22.04 on 12th gen - # and newer. + # and newer. It never publishes them for pre-Broadwell GPUs. logger.warning( "Unable to collect Intel GPU stats: found %d DRM client(s) for %s but " - "no per-engine counters. Kernel 6.5 or newer is required.", + "the kernel reports no usage for them. GPUs older than Broadwell " + "(5th gen) are not supported. 12th gen and newer need kernel 6.5+", len(snapshot_a), "/".join(sorted({client["driver"] for client in snapshot_a.values()})), ) From 7e402d70df6c74a3927f5dc81c7614c1858afa83 Mon Sep 17 00:00:00 2001 From: Josh Hawkins <32435876+hawkeye217@users.noreply.github.com> Date: Sun, 11 Oct 2026 14:21:45 -0500 Subject: [PATCH 14/18] Fix environment_vars not reaching ffmpeg processes (#24635) * start the forkserver after the config applies environment_vars The forkserver started as a side effect of creating the multiprocessing Manager at the top of main(), before the config loaded, so every worker process and the ffmpeg it launched inherited the environment from before environment_vars were applied. Variables like LIBVA_DRIVER_NAME set in config.yml never reached ffmpeg. The Manager now uses the fork context, so the forkserver does not start until the first worker process. * fix test --- frigate/__main__.py | 4 +- frigate/test/test_process.py | 81 ++++++++++++++++++++++++++++++++++++ 2 files changed, 84 insertions(+), 1 deletion(-) create mode 100644 frigate/test/test_process.py diff --git a/frigate/__main__.py b/frigate/__main__.py index df27b42a27..09712cde37 100644 --- a/frigate/__main__.py +++ b/frigate/__main__.py @@ -15,7 +15,9 @@ from frigate.util.config import find_config_file def main() -> None: - manager = mp.Manager() + # Fork this manager so the forkserver does not start until the first + # worker process, after the config has applied environment_vars. + manager = mp.get_context("fork").Manager() faulthandler.enable() # Setup the logging thread diff --git a/frigate/test/test_process.py b/frigate/test/test_process.py new file mode 100644 index 0000000000..7b79929374 --- /dev/null +++ b/frigate/test/test_process.py @@ -0,0 +1,81 @@ +import multiprocessing as mp +import os +import sys +import unittest +from types import SimpleNamespace +from unittest.mock import patch + +import frigate.__main__ as frigate_main +import frigate.log +from frigate.config import FrigateConfig +from frigate.util.process import FrigateProcess + +ENV_KEY = "FRIGATE_TEST_PROCESS_ENV" + + +class EnvReportingProcess(FrigateProcess): + def __init__(self, stop_event, result_queue): + super().__init__(stop_event, 0, name="env_reporter", daemon=True) + self.result_queue = result_queue + + def run(self) -> None: + self.pre_run_setup() + self.result_queue.put(os.environ.get(ENV_KEY)) + + +def _run_main_and_report(result_queue) -> None: + """Run frigate's main() with the config load and app faked out. + + Runs in a fresh interpreter so no earlier test has started the forkserver. + """ + mp.set_start_method("forkserver", force=True) + + def load(**kwargs): + os.environ[ENV_KEY] = "applied" + return SimpleNamespace() + + def setup_logging(manager): + frigate.log.log_listener = SimpleNamespace(queue=manager.Queue()) + + class FakeApp: + def __init__(self, config, manager, stop_event): + self.stop_event = stop_event + + def start(self): + child_queue = mp.Queue() + process = EnvReportingProcess(self.stop_event, child_queue) + process.start() + result_queue.put(child_queue.get(timeout=30)) + process.join(timeout=10) + + try: + with ( + patch.object(sys, "argv", ["frigate"]), + patch.object(FrigateConfig, "load", load), + patch("frigate.__main__.setup_logging", setup_logging), + patch("frigate.__main__.FrigateApp", FakeApp), + ): + frigate_main.main() + except Exception as err: + result_queue.put(f"error: {err!r}") + + +class TestWorkerEnvironment(unittest.TestCase): + def test_env_applied_by_config_load_reaches_worker_processes(self): + ctx = mp.get_context("spawn") + result_queue = ctx.Queue() + runner = ctx.Process(target=_run_main_and_report, args=(result_queue,)) + runner.start() + + try: + self.assertEqual(result_queue.get(timeout=60), "applied") + finally: + runner.join(timeout=10) + + if runner.is_alive(): + runner.terminate() + runner.join() + + +if __name__ == "__main__": + unittest.main() From cf67f089131194d91b0c3590cb5d96fba62413db Mon Sep 17 00:00:00 2001 From: Josh Hawkins <32435876+hawkeye217@users.noreply.github.com> Date: Mon, 12 Oct 2026 11:22:02 -0500 Subject: [PATCH 15/18] fix mse buffer trim spinning in a remove loop (#24643) Subtracting 15s from the buffered end left a floating point remainder, so when Firefox held exactly 15s of buffer the handler called remove() on a near-zero range that removed nothing, and each updateend called it again. The loop pegged a CPU core and kept running after the websocket closed. Trimming now requires more than 1s past the window. --- web/src/components/player/MsePlayer.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/web/src/components/player/MsePlayer.tsx b/web/src/components/player/MsePlayer.tsx index e8daed4437..a8856d671e 100644 --- a/web/src/components/player/MsePlayer.tsx +++ b/web/src/components/player/MsePlayer.tsx @@ -473,7 +473,7 @@ function MSEPlayer({ } else if (sb.buffered && sb.buffered.length) { const end = sb.buffered.end(sb.buffered.length - 1) - 15; const start = sb.buffered.start(0); - if (end > start) { + if (end - start > 1) { sb.remove(start, end); msRef.current?.setLiveSeekableRange(end, end + 15); } From 41d9577d7223fa32ff38fcc0adefb3fb108fda4c Mon Sep 17 00:00:00 2001 From: Josh Hawkins <32435876+hawkeye217@users.noreply.github.com> Date: Mon, 12 Oct 2026 14:51:21 -0500 Subject: [PATCH 16/18] Fix docs shm calculator (#24649) * match the docs shm calculator to the backend's minimum The calculator left out the room Frigate reserves for 2 cameras added at runtime, counted the per-frame overhead once per camera instead of per frame, and reserved 40mb instead of 50mb for logs and the nginx cache, so a setup sized with it got well under 20 frames and the "SHM size is too small" warning. It now computes the same minimum as calculate_shm_requirements() and also shows the size for the full 50 frame cache. * size the docs shm calculator for 25 frames Sizing to the backend's 20 frame minimum could land at 19 frames from float rounding and still show the warning. --- docs/docs/frigate/installation.md | 4 +- docs/src/components/ShmCalculator/index.jsx | 79 +++++++++++++-------- 2 files changed, 51 insertions(+), 32 deletions(-) diff --git a/docs/docs/frigate/installation.md b/docs/docs/frigate/installation.md index 14c85ec4f3..4c6ceec04d 100644 --- a/docs/docs/frigate/installation.md +++ b/docs/docs/frigate/installation.md @@ -78,9 +78,9 @@ Users of the Snapcraft build of Docker cannot use storage locations outside your Frigate utilizes shared memory to store frames during processing. The default `shm-size` provided by Docker is **64MB**. -The default shm size of **128MB** is fine for setups with **2 cameras** detecting at **720p**. If Frigate is exiting with "Bus error" messages, it is likely because you have too many high resolution cameras and you need to specify a higher shm size, using [`--shm-size`](https://docs.docker.com/engine/reference/run/#runtime-constraints-on-resources) (or [`service.shm_size`](https://docs.docker.com/compose/compose-file/compose-file-v2/#shm_size) in Docker Compose). If raising the shm size does not help, check your [process and file limits](#process-and-file-limits) as well. +Frigate needs about **210MB** for **2 cameras** detecting at **720p**. If Frigate is exiting with "Bus error" messages, it is likely because you have too many high resolution cameras and you need to specify a higher shm size, using [`--shm-size`](https://docs.docker.com/engine/reference/run/#runtime-constraints-on-resources) (or [`service.shm_size`](https://docs.docker.com/compose/compose-file/compose-file-v2/#shm_size) in Docker Compose). If raising the shm size does not help, check your [process and file limits](#process-and-file-limits) as well. -The Frigate container also stores logs in shm, which can take up to **40MB**, so make sure to take this into account in your math as well. +The Frigate container also stores logs and an nginx cache in shm, which can take up to **50MB**. The calculator below includes this, along with room for 2 cameras added while Frigate is running. Its result holds about 25 frames per camera, a little above the 20 frame minimum Frigate warns below. Frigate keeps up to 50 when there is room, which gives slow processes more time to read each frame. diff --git a/docs/src/components/ShmCalculator/index.jsx b/docs/src/components/ShmCalculator/index.jsx index b7e13ed79f..afb15365cc 100644 --- a/docs/src/components/ShmCalculator/index.jsx +++ b/docs/src/components/ShmCalculator/index.jsx @@ -2,42 +2,53 @@ import React, { useState, useEffect } from "react"; import Admonition from "@theme/Admonition"; import styles from "./styles.module.css"; +// follows calculate_shm_requirements() in frigate/util/services.py, but sizes +// for 25 frames so float rounding can't land under its 20 frame warning +const MB = 1048576; +const RESERVED_MB = 50; // logs + nginx cache +const TARGET_FRAMES = 25; +const DEFAULT_FRAMES = 50; + +const frameMb = (width, height) => + Math.round(((width * height * 1.5 + 270480) / MB) * 10) / 10; + +// room for 2 cameras added at runtime, sized as 720p +const addedCamerasMb = 2 * frameMb(1280, 720); + const ShmCalculator = () => { const [width, setWidth] = useState(1280); const [height, setHeight] = useState(720); const [cameraCount, setCameraCount] = useState(1); - const [result, setResult] = useState("26.32MB"); - const [singleCameraShm, setSingleCameraShm] = useState("26.32MB"); - const [totalShm, setTotalShm] = useState("26.32MB"); + const [result, setResult] = useState("-"); + const [singleCameraShm, setSingleCameraShm] = useState("-"); + const [addedCamerasShm, setAddedCamerasShm] = useState("-"); + const [fullRingShm, setFullRingShm] = useState("-"); const calculate = () => { if (!width || !height || !cameraCount) { setResult("Please enter valid values"); setSingleCameraShm("-"); - setTotalShm("-"); + setAddedCamerasShm("-"); + setFullRingShm("-"); return; } - // Single camera base SHM calculation (excluding logs) - // Formula: (width * height * 1.5 * 20 + 270480) / 1048576 - const singleCameraBase = - (width * height * 1.5 * 20 + 270480) / 1048576; - setSingleCameraShm(`${singleCameraBase.toFixed(2)}mb`); + const cameraFramesMb = cameraCount * frameMb(width, height) + addedCamerasMb; - // Total SHM calculation (multiple cameras, including logs) - const totalBase = singleCameraBase * cameraCount; - const finalResult = totalBase + 40; // Default includes logs +40mb + setSingleCameraShm(formatMb(frameMb(width, height) * TARGET_FRAMES)); + setAddedCamerasShm(formatMb(addedCamerasMb * TARGET_FRAMES)); + setResult(formatMb(Math.round(RESERVED_MB + cameraFramesMb * TARGET_FRAMES))); + setFullRingShm( + formatMb(Math.round(RESERVED_MB + cameraFramesMb * DEFAULT_FRAMES)), + ); + }; - setTotalShm(`${(totalBase + 40).toFixed(2)}mb`); - - // Format result - if (finalResult < 1) { - setResult(`${(finalResult * 1024).toFixed(2)}kb`); - } else if (finalResult >= 1024) { - setResult(`${(finalResult / 1024).toFixed(2)}gb`); - } else { - setResult(`${finalResult.toFixed(2)}mb`); + const formatMb = (value) => { + if (value >= 1024) { + return `${(value / 1024).toFixed(2)}gb`; } + + return `${Number(value.toFixed(2))}mb`; }; const formatWithUnit = (value) => { @@ -147,19 +158,27 @@ const ShmCalculator = () => {

- Single Camera: {formatWithUnit(singleCameraShm)} + Per Camera: {formatWithUnit(singleCameraShm)}

- Formula: (width × height × 1.5 × 20 + 270480) ÷ - 1048576 + Formula: (width × height × 1.5 + 270480) ÷ + 1048576 × {TARGET_FRAMES} frames

- {cameraCount > 1 && ( -

- Total ({cameraCount} cameras): {formatWithUnit(totalShm)} -

- )}

- With Logs: + 40mb + Room for 2 Added Cameras:{" "} + {formatWithUnit(addedCamerasShm)} +

+

+ Logs and Cache: + {RESERVED_MB} + mb +

+

+ Birdseye Restream: + 8 + mb if enabled +

+

+ For {DEFAULT_FRAMES} Frames:{" "} + {formatWithUnit(fullRingShm)}

From d3b2af932cd828471745c2736dd50e9d02580eb4 Mon Sep 17 00:00:00 2001 From: Josh Hawkins <32435876+hawkeye217@users.noreply.github.com> Date: Tue, 13 Oct 2026 18:07:09 -0500 Subject: [PATCH 17/18] bump version in docs (#24654) --- docs/docs/frigate/updating.md | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/docs/docs/frigate/updating.md b/docs/docs/frigate/updating.md index a4dfb7f0a4..56d8d159c2 100644 --- a/docs/docs/frigate/updating.md +++ b/docs/docs/frigate/updating.md @@ -5,7 +5,7 @@ title: Updating # Updating Frigate -The current stable version of Frigate is **0.18.0**. The release notes and any breaking changes for this version can be found on the [Frigate GitHub releases page](https://github.com/blakeblackshear/frigate/releases/tag/v0.18.0). +The current stable version of Frigate is **0.18.1**. The release notes and any breaking changes for this version can be found on the [Frigate GitHub releases page](https://github.com/blakeblackshear/frigate/releases/tag/v0.18.1). Keeping Frigate up to date ensures you benefit from the latest features, performance improvements, and bug fixes. The update process varies slightly depending on your installation method (Docker, Home Assistant App, etc.). Below are instructions for the most common setups. @@ -31,21 +31,21 @@ If you’re running Frigate via Docker (recommended method), follow these steps: 2. **Update and Pull the Latest Image**: - If using Docker Compose: - - Edit your `docker-compose.yml` file to specify the desired version tag (e.g., `0.18.0` instead of `0.17.1`). For example: + - Edit your `docker-compose.yml` file to specify the desired version tag (e.g., `0.18.1` instead of `0.17.1`). For example: ```yaml services: frigate: - image: ghcr.io/blakeblackshear/frigate:0.18.0 + image: ghcr.io/blakeblackshear/frigate:0.18.1 ``` - Then pull the image: ```bash - docker pull ghcr.io/blakeblackshear/frigate:0.18.0 + docker pull ghcr.io/blakeblackshear/frigate:0.18.1 ``` - **Note for `stable` Tag Users**: If your `docker-compose.yml` uses the `stable` tag (e.g., `ghcr.io/blakeblackshear/frigate:stable`), you don’t need to update the tag manually. The `stable` tag always points to the latest stable release after pulling. - If using `docker run`: - - Pull the image with the appropriate tag (e.g., `0.18.0`, `0.18.0-tensorrt`, or `stable`): + - Pull the image with the appropriate tag (e.g., `0.18.1`, `0.18.1-tensorrt`, or `stable`): ```bash - docker pull ghcr.io/blakeblackshear/frigate:0.18.0 + docker pull ghcr.io/blakeblackshear/frigate:0.18.1 ``` 3. **Start the Container**: @@ -100,7 +100,7 @@ If an update causes issues: 1. Stop Frigate. 2. Restore your backed-up config file and database. 3. Revert to the previous image version: - - For Docker: Specify an older tag (e.g., `ghcr.io/blakeblackshear/frigate:0.17.1`) in your `docker run` command. + - For Docker: Specify an older tag (e.g., `ghcr.io/blakeblackshear/frigate:0.18.0`) in your `docker run` command. - For Docker Compose: Edit your `docker-compose.yml`, specify the older version tag (e.g., `ghcr.io/blakeblackshear/frigate:0.16.4`), and re-run `docker compose up -d`. - For Home Assistant: Restore from the app/addon backup you took before you updated. 4. Verify the old version is running again. From a3ca8742b084c01177b6731399e80b5e9a13d171 Mon Sep 17 00:00:00 2001 From: Blake Blackshear Date: Wed, 14 Oct 2026 14:10:05 -0500 Subject: [PATCH 18/18] Security patches (#24658) * update version * Restrict ffmpeg: go2rtc sources added through the API go2rtc expands ffmpeg: sources into an exec: command line built from the source's #key=value parameters, so the echo/expr/exec prefix check on PUT /go2rtc/streams could be bypassed to run arbitrary ffmpeg arguments. Sources submitted through the API now only accept the ffmpeg: parameters the UI and docs use (video, audio, hardware with preset names, plus numeric width/height/rotate/bitrate/timeout and async). Quotes, whitespace, #raw, #input, #drawtext and unknown parameters are rejected unless GO2RTC_ALLOW_ARBITRARY_EXEC is set. Sources defined in the config file are unchanged. Fixes GHSA-qjjw-3v66-279f Co-Authored-By: Claude Opus 5.5 * Only allow existing streams on proxied go2rtc live paths nginx proxies /live/mse/api/ws, /live/webrtc/api/ws and /api/go2rtc/webrtc straight to go2rtc, which creates a new ad hoc stream from any src that is not an existing stream name and starts the producer it describes. Any authenticated user, including the built-in viewer role, could use this to launch arbitrary go2rtc sources without going through the restricted-source checks on PUT /go2rtc/streams. The /auth subrequest now denies these paths unless every src names an existing stream: a camera or its live stream, a go2rtc stream from the config, or a stream go2rtc reports at runtime. This applies to every role, and the runtime lookup fails closed when go2rtc cannot be reached. Fixes GHSA-93xj-hg88-ffg7 Co-Authored-By: Claude Opus 5.5 * Ignore GO2RTC_ALLOW_ARBITRARY_EXEC in environment_vars environment_vars installs every key into the Frigate process environment at startup, so an admin could enable GO2RTC_ALLOW_ARBITRARY_EXEC from the config and turn off the restricted go2rtc source checks on the API. The override is meant to be set by the operator in the container environment (or the Home Assistant add-on options), so it is now skipped with a warning when it appears in environment_vars. Co-Authored-By: Claude Opus 5.5 * Apply go2rtc source restrictions to config/save and config/set go2rtc.streams written through config/save or config/set were only filtered at startup, which blocks echo/expr/exec and the map form but not the ffmpeg: parameters that PUT /go2rtc/streams now rejects. Saving a stream this way and restarting bypassed the API restriction. Both endpoints now reject a change that adds a go2rtc stream source that fails is_restricted_go2rtc_api_source, or a map-form source, unless GO2RTC_ALLOW_ARBITRARY_EXEC is set. Sources already in the saved config for the same stream are left alone, so streams configured on disk keep working and resubmitting the whole file from the config editor succeeds. config/set restores the previous file when it rejects a change. Follow-up to GHSA-qjjw-3v66-279f Co-Authored-By: Claude Opus 5.5 * Match go2rtc stream-proxy paths the way nginx does /auth compared the raw $request_uri path against the three go2rtc stream-proxy paths exactly, while nginx matches its prefix locations against the normalized URI. Encoded characters, repeated slashes, dot segments or a suffix on the path skipped the stream check even though nginx still proxied the request to go2rtc. The go2rtc stream locations now set a variable that the auth subrequest forwards as X-Go2rtc-Stream-Proxy (client headers are not forwarded, so it cannot be spoofed). As a fallback, /auth also normalizes the path like nginx (percent-decoding, merging slashes, resolving dot segments) and prefix matches it, and splits the raw URI by hand since urlparse reads a leading // as a host. Co-Authored-By: Claude Opus 5.5 * Parse config stream changes with Frigate's YAML loader The go2rtc stream check on config/save and config/set parsed the config with the safe loader and treated a parse error as an empty config, while Frigate and the go2rtc startup script use the round-trip loader. Input that only the round-trip loader accepts, such as a tagged root map, hid new stream sources from the check. The check now uses the same round-trip loader and rejects the request when the new config cannot be parsed. Co-Authored-By: Claude Opus 5.5 * Resolve go2rtc stream placeholders the same way everywhere The go2rtc startup script expanded stream sources with str.format(), which also supports attribute and index access, while the config API check used substitute_frigate_vars and fell back to the raw string when a placeholder could not be resolved. A placeholder the API check could not resolve could therefore expand into a restricted source at startup. The startup script now uses substitute_frigate_vars, like the rest of the config, and the API check rejects a stream source whose placeholders cannot be resolved. The startup script's own copy of the FRIGATE_* env var map is removed; the shared one also honors CREDENTIALS_DIRECTORY. Co-Authored-By: Claude Opus 5.5 * Accept codec sample rates in ffmpeg: go2rtc sources The ffmpeg: parameter allowlist only accepted plain names for video, audio and hardware, so the documented HomeKit source with #audio=opus/16000 was rejected by PUT /go2rtc/streams, config/save and config/set. Names may now carry a numeric /rate suffix. A rate without a name is still rejected. All ffmpeg: sources in the docs pass the check. Co-Authored-By: Claude Opus 5.5 * Restore the environment after go2rtc exec override tests Tests that removed or set GO2RTC_ALLOW_ARBITRARY_EXEC did not restore a value that was already present, so later tests could run with a changed security setting. They now use patch.dict on os.environ. Co-Authored-By: Claude Opus 5.5 * Revert the go2rtc startup script to str.format substitution Restores create_config.py to its state on master. The config API check rejects any stream source whose {FRIGATE_*} placeholders it cannot resolve with substitute_frigate_vars, and for every source it accepts, str.format() at startup either produces the same string or fails and stops go2rtc startup. Changing the startup script is therefore not needed for the fix, and leaving it alone avoids a conflict with dev, which already moved this script to the new env helpers. Co-Authored-By: Claude Opus 5.5 * Explain why a go2rtc stream change was rejected A stream source with an undefined or malformed {FRIGATE_*} placeholder was reported as a restricted source, which reads like a security block when the user made a typo. The config API now reports unknown variables and invalid placeholders separately from restricted sources. Co-Authored-By: Claude Opus 5.5 --------- Co-authored-by: Claude Opus 5.5 --- Makefile | 2 +- .../usr/local/nginx/conf/auth_location.conf | 3 + .../rootfs/usr/local/nginx/conf/nginx.conf | 6 + frigate/api/app.py | 82 ++++++- frigate/api/auth.py | 100 +++++++- frigate/api/camera.py | 10 +- frigate/config/env.py | 14 ++ .../test/http_api/test_http_camera_access.py | 71 ++++++ .../test_http_config_go2rtc_streams.py | 217 ++++++++++++++++++ .../http_api/test_http_go2rtc_proxy_auth.py | 73 ++++++ frigate/test/test_env.py | 31 ++- frigate/test/test_go2rtc_stream_auth.py | 123 +++++++++- frigate/util/services.py | 117 ++++++++++ 13 files changed, 825 insertions(+), 24 deletions(-) create mode 100644 frigate/test/http_api/test_http_config_go2rtc_streams.py create mode 100644 frigate/test/http_api/test_http_go2rtc_proxy_auth.py diff --git a/Makefile b/Makefile index f30bf8680b..22b064aeed 100644 --- a/Makefile +++ b/Makefile @@ -1,7 +1,7 @@ default_target: local COMMIT_HASH := $(shell git log -1 --pretty=format:"%h"|tail -1) -VERSION = 0.18.1 +VERSION = 0.18.2 IMAGE_REPO ?= ghcr.io/blakeblackshear/frigate GITHUB_REF_NAME ?= $(shell git rev-parse --abbrev-ref HEAD) BOARDS= #Initialized empty diff --git a/docker/main/rootfs/usr/local/nginx/conf/auth_location.conf b/docker/main/rootfs/usr/local/nginx/conf/auth_location.conf index 619a2b3ff1..9e2827c3d2 100644 --- a/docker/main/rootfs/usr/local/nginx/conf/auth_location.conf +++ b/docker/main/rootfs/usr/local/nginx/conf/auth_location.conf @@ -15,6 +15,9 @@ location /auth { # Pass info about the request proxy_set_header X-Original-Method $request_method; proxy_set_header X-Original-URL $request_uri; + # set by the go2rtc stream locations; client headers are not forwarded, so + # this cannot be spoofed + proxy_set_header X-Go2rtc-Stream-Proxy $go2rtc_stream_proxy; proxy_set_header X-Server-Port $server_port; proxy_set_header Content-Length ""; # Pass along auth related info diff --git a/docker/main/rootfs/usr/local/nginx/conf/nginx.conf b/docker/main/rootfs/usr/local/nginx/conf/nginx.conf index cad314ae35..34cfe966e5 100644 --- a/docker/main/rootfs/usr/local/nginx/conf/nginx.conf +++ b/docker/main/rootfs/usr/local/nginx/conf/nginx.conf @@ -202,6 +202,8 @@ http { # frigate lovelace card uses this path location /live/mse/api/ws { + # tells /auth to apply the go2rtc stream checks + set $go2rtc_stream_proxy 1; include auth_request.conf; limit_except GET { deny all; @@ -211,6 +213,8 @@ http { } location /live/webrtc/api/ws { + # tells /auth to apply the go2rtc stream checks + set $go2rtc_stream_proxy 1; include auth_request.conf; limit_except GET { deny all; @@ -231,6 +235,8 @@ http { # integration uses this to add webrtc candidate location /api/go2rtc/webrtc { + # tells /auth to apply the go2rtc stream checks + set $go2rtc_stream_proxy 1; include auth_request.conf; limit_except POST { deny all; diff --git a/frigate/api/app.py b/frigate/api/app.py index 0af3bc1aba..00a7b8b5ef 100644 --- a/frigate/api/app.py +++ b/frigate/api/app.py @@ -73,6 +73,8 @@ from frigate.util.config import ( ) from frigate.util.schema import get_config_schema from frigate.util.services import ( + Go2rtcStreamIssue, + find_restricted_go2rtc_stream_changes, get_nvidia_driver_info, process_logs, restart_frigate, @@ -522,6 +524,63 @@ def config_raw(): ) +def _go2rtc_streams(raw_config: str) -> Any: + """Load go2rtc.streams with the same loader Frigate and go2rtc startup use. + + Raises ruamel.yaml.YAMLError when the config cannot be parsed. + """ + loaded = ruamel.yaml.YAML().load(raw_config) + go2rtc = loaded.get("go2rtc") if isinstance(loaded, dict) else None + return go2rtc.get("streams") if isinstance(go2rtc, dict) else None + + +def _restricted_go2rtc_streams_response( + old_raw_config: str, new_raw_config: str +) -> JSONResponse | None: + """Reject config changes that add restricted go2rtc stream sources. + + The same sources are rejected by PUT /go2rtc/streams, and go2rtc would + otherwise load them from the saved config on the next restart. + """ + try: + new_streams = _go2rtc_streams(new_raw_config) + except ruamel.yaml.YAMLError: + logger.warning("Rejected config change that could not be parsed") + return JSONResponse( + content={"success": False, "message": "Unable to parse config"}, + status_code=400, + ) + + try: + old_streams = _go2rtc_streams(old_raw_config) + except ruamel.yaml.YAMLError: + # with no readable saved config, every source is treated as new + old_streams = None + + issues = find_restricted_go2rtc_stream_changes(old_streams, new_streams) + if not issues: + return None + + messages = [] + for issue, label in ( + (Go2rtcStreamIssue.restricted, "Restricted go2rtc stream source in"), + (Go2rtcStreamIssue.unknown_variable, "Unknown variable in go2rtc stream"), + ( + Go2rtcStreamIssue.invalid_placeholder, + "Invalid variable placeholder in go2rtc stream", + ), + ): + names = [name for name, found in issues.items() if found == issue] + if names: + messages.append(f"{label}: {', '.join(names)}") + + logger.warning("Rejected config change: %s", "; ".join(messages)) + return JSONResponse( + content={"success": False, "message": "; ".join(messages)}, + status_code=400, + ) + + @router.post("/config/save", dependencies=[Depends(require_role(["admin"]))]) def config_save(save_option: str, body: Any = Body(media_type="text/plain")): new_config = body.decode() @@ -598,10 +657,22 @@ def config_save(save_option: str, body: Any = Body(media_type="text/plain")): status_code=400, ) + config_file = find_config_file() + + try: + with open(config_file) as f: + old_raw_config = f.read() + except OSError: + old_raw_config = "" + + restricted_response = _restricted_go2rtc_streams_response( + old_raw_config, new_config + ) + if restricted_response is not None: + return restricted_response + # Save the config to file try: - config_file = find_config_file() - with open(config_file, "w") as f: f.write(new_config) f.close() @@ -912,6 +983,13 @@ def config_set(request: Request, body: AppConfigSetBody): ), status_code=400, ) + restricted_response = _restricted_go2rtc_streams_response( + old_raw_config, new_raw_config + ) + if restricted_response is not None: + with open(config_file, "w") as f: + f.write(old_raw_config) + return restricted_response except Exception as e: logging.error(f"Error updating config: {e}") return JSONResponse( diff --git a/frigate/api/auth.py b/frigate/api/auth.py index 5f4de5204f..76d330548e 100644 --- a/frigate/api/auth.py +++ b/frigate/api/auth.py @@ -11,8 +11,9 @@ import secrets import time from datetime import datetime from pathlib import Path -from urllib.parse import parse_qs, urlparse +from urllib.parse import parse_qs, unquote +import requests from fastapi import APIRouter, Depends, HTTPException, Request, Response from fastapi.responses import JSONResponse, RedirectResponse from joserfc import jwt @@ -1138,11 +1139,44 @@ def _get_stream_owner_cameras(request: Request, stream_name: str) -> set[str]: return owner_cameras +def _is_existing_go2rtc_stream(request: Request, stream_name: str) -> bool: + """Check that `stream_name` names a stream go2rtc already knows about. + + go2rtc turns any `src` that is not an existing stream name into a new ad hoc + stream and starts the producer it describes, so the proxied live-stream + paths must only ever reference existing streams. + """ + frigate_config = request.app.frigate_config + + if _get_stream_owner_cameras(request, stream_name): + return True + + config_streams = getattr(frigate_config.go2rtc, "streams", None) or {} + if stream_name in config_streams: + return True + + # streams added at runtime through PUT /go2rtc/streams only exist in go2rtc + try: + r = requests.get("http://127.0.0.1:1984/api/streams", timeout=2) + except requests.RequestException: + logger.warning("Unable to fetch streams from go2rtc to authorize request") + return False + + if not r.ok: + return False + + try: + return stream_name in r.json() + except ValueError: + return False + + # nginx proxies these paths straight to go2rtc with authentication-only checks # (see auth_request.conf). Each names the desired stream via the `src` query # param, so the camera-level check must happen here in the `/auth` subrequest — # `require_go2rtc_stream_access` only guards the REST `/go2rtc/streams/{name}` -# endpoint, not these proxied live-stream paths. +# endpoint, not these proxied live-stream paths. The nginx locations are prefix +# matches, so anything under these paths is proxied too. GO2RTC_STREAM_PROXY_PATHS = frozenset( { "/live/mse/api/ws", @@ -1151,32 +1185,80 @@ GO2RTC_STREAM_PROXY_PATHS = frozenset( } ) +_URI_SCHEME_AND_AUTHORITY = re.compile(r"^[A-Za-z][A-Za-z0-9+.-]*://[^/?]*") + + +def _split_request_uri(request_uri: str) -> tuple[str, str]: + """Split a raw request URI into its path and query string. + + urlparse is not used since it reads a path starting with `//` as a host. + """ + uri = _URI_SCHEME_AND_AUTHORITY.sub("", request_uri, count=1) + path, _, query = uri.partition("?") + return path, query + + +def _normalize_request_path(path: str) -> str: + """Normalize a raw request path the way nginx does before matching locations. + + nginx percent-decodes the path, merges repeated slashes and resolves dot + segments, while `X-Original-URL` carries the raw `$request_uri`. + """ + segments: list[str] = [] + for segment in unquote(path).split("/"): + if segment in ("", "."): + continue + if segment == "..": + if segments: + segments.pop() + continue + segments.append(segment) + + return "/" + "/".join(segments) + + +def _is_go2rtc_stream_proxy_request(path: str, request: Request) -> bool: + # nginx marks requests routed through the go2rtc stream locations, and the + # normalized path catches them if that marker is ever missing + if request.headers.get("x-go2rtc-stream-proxy") == "1": + return True + + normalized = _normalize_request_path(path) + return any(normalized.startswith(p) for p in GO2RTC_STREAM_PROXY_PATHS) + def deny_response_for_go2rtc_stream( original_url: str | None, role: str | None, request: Request ) -> int | None: - """Block role-restricted users from go2rtc live streams they cannot access. + """Block go2rtc live-stream requests the current role may not make. - Returns 403 when any `src` stream named in `original_url` resolves to a - camera outside the role's allow-list (or when no `src` is provided on a - stream-proxy path), otherwise None. Mirrors the resolution logic in + Returns 403 when any `src` named in `original_url` is not an existing go2rtc + stream (for every role), or when it resolves to a camera outside a + restricted role's allow-list (or no `src` is provided for such a role), + otherwise None. Mirrors the resolution logic in `require_go2rtc_stream_access` so substream names map to their owning camera correctly. """ if not original_url: return None - parsed = urlparse(original_url) - if parsed.path not in GO2RTC_STREAM_PROXY_PATHS: + path, query = _split_request_uri(original_url) + if not _is_go2rtc_stream_proxy_request(path, request): return None frigate_config = request.app.frigate_config + sources = parse_qs(query, keep_blank_values=True).get("src", []) + + # an unknown src would make go2rtc create an ad hoc stream from it, which + # bypasses the restricted-source checks on PUT /go2rtc/streams + for src in sources: + if not _is_existing_go2rtc_stream(request, src): + return 403 # admin and full-access roles (no allow-list) bypass the camera check if not role or not is_role_restricted(role, frigate_config): return None - sources = parse_qs(parsed.query).get("src", []) if not sources: # a stream-proxy request naming no stream has nothing legitimate to # show a restricted user diff --git a/frigate/api/camera.py b/frigate/api/camera.py index 6498901b97..a23fe58e60 100644 --- a/frigate/api/camera.py +++ b/frigate/api/camera.py @@ -46,7 +46,7 @@ from frigate.util.image import run_ffmpeg_snapshot from frigate.util.services import ( analyze_record_keyframes, ffprobe_stream, - is_restricted_go2rtc_source, + is_restricted_go2rtc_api_source, ) logger = logging.getLogger(__name__) @@ -152,9 +152,9 @@ def go2rtc_camera_stream(request: Request, stream_name: str): ) def go2rtc_add_stream(request: Request, stream_name: str, src: str = ""): """Add or update a go2rtc stream configuration.""" - if src and is_restricted_go2rtc_source(src): + if src and is_restricted_go2rtc_api_source(src): logger.warning( - "Rejected go2rtc stream '%s' with restricted source type (echo/expr/exec)", + "Rejected go2rtc stream '%s' with restricted source", stream_name, ) return JSONResponse( @@ -173,9 +173,9 @@ def go2rtc_add_stream(request: Request, stream_name: str, src: str = ""): except KeyError: resolved_src = src - if is_restricted_go2rtc_source(resolved_src): + if is_restricted_go2rtc_api_source(resolved_src): logger.warning( - "Rejected go2rtc stream '%s' with restricted source type (echo/expr/exec)", + "Rejected go2rtc stream '%s' with restricted source", stream_name, ) return JSONResponse( diff --git a/frigate/config/env.py b/frigate/config/env.py index 209dda67bf..79d85cfab0 100644 --- a/frigate/config/env.py +++ b/frigate/config/env.py @@ -1,3 +1,4 @@ +import logging import os import re from pathlib import Path @@ -5,6 +6,8 @@ from typing import Annotated from pydantic import AfterValidator, ValidationInfo +logger = logging.getLogger(__name__) + FRIGATE_ENV_VARS = {k: v for k, v in os.environ.items() if k.startswith("FRIGATE_")} secrets_dir = os.environ.get("CREDENTIALS_DIRECTORY", "/run/secrets") # read secret files as env vars too @@ -16,6 +19,10 @@ if os.path.isdir(secrets_dir) and os.access(secrets_dir, os.R_OK): ) +# Security overrides that must come from the container environment, which is +# controlled by the operator, rather than from the config, which admins can edit +_PROTECTED_ENV_VARS = frozenset({"GO2RTC_ALLOW_ARBITRARY_EXEC"}) + # Matches a FRIGATE_* identifier following an opening brace. _FRIGATE_IDENT_RE = re.compile(r"FRIGATE_[A-Za-z0-9_]+") @@ -95,6 +102,13 @@ EnvString = Annotated[str, AfterValidator(validate_env_string)] def validate_env_vars(v: dict[str, str], info: ValidationInfo) -> dict[str, str]: if isinstance(info.context, dict) and info.context.get("install", False): for k, val in v.items(): + if k.upper() in _PROTECTED_ENV_VARS: + logger.warning( + "Ignoring %s in environment_vars, it must be set in the container environment", + k, + ) + continue + os.environ[k] = val if k.startswith("FRIGATE_"): FRIGATE_ENV_VARS[k] = val diff --git a/frigate/test/http_api/test_http_camera_access.py b/frigate/test/http_api/test_http_camera_access.py index 4c74792c14..9d1b57b1cc 100644 --- a/frigate/test/http_api/test_http_camera_access.py +++ b/frigate/test/http_api/test_http_camera_access.py @@ -403,6 +403,77 @@ class TestGo2rtcStreamAccess(BaseTestHttp): forwarded_src = mock_put.call_args.kwargs["params"]["src"] assert forwarded_src == "exec:/tmp/something" + def test_add_stream_rejects_ffmpeg_argument_injection(self): + """PUT /go2rtc/streams must reject ffmpeg: sources whose parameters can + inject arbitrary ffmpeg arguments, since go2rtc expands them to exec:""" + app = self._make_app(_MULTI_CAMERA_CONFIG) + with patch("frigate.api.camera.requests.put") as mock_put: + with AuthTestClient(app) as client: + for src in ( + # shape of the source in GHSA-qjjw-3v66-279f + "ffmpeg:#input='-f''h264''-i''data:placeholder''-c''copy'", + "ffmpeg:#input=-re", + "ffmpeg:front_door#raw=-vn", + "ffmpeg:front_door#video=-vn", + "ffmpeg:front_door#video=out.mp4", + "ffmpeg:front_door#drawtext=x", + "ffmpeg:front_door#width=1x", + "ffmpeg:front_door#audio=/16000", + "ffmpeg:front_door#audio=opus/", + "ffmpeg:front_door#audio=opus/16000/1", + "ffmpeg:'front_door'", + " ffmpeg:#input=-re", + ): + resp = client.put("/go2rtc/streams/injected", params={"src": src}) + assert resp.status_code == 400, ( + f"Expected 400 for ffmpeg src {src!r}; got {resp.status_code}" + ) + assert resp.json().get("success") is False + mock_put.assert_not_called() + + def test_add_stream_allows_standard_ffmpeg_sources(self): + """ffmpeg: sources built by the UI and shown in the docs must still be + forwarded to go2rtc""" + app = self._make_app(_MULTI_CAMERA_CONFIG) + mock_response = type("R", (), {"ok": True, "status_code": 200, "text": "ok"})() + with patch( + "frigate.api.camera.requests.put", return_value=mock_response + ) as mock_put: + with AuthTestClient(app) as client: + for src in ( + "ffmpeg:rtsp://10.0.0.1:554/video", + "ffmpeg:rtsp://10.0.0.1:554/video#video=copy#audio=copy", + "ffmpeg:rtsp://10.0.0.1:554/video#video=h264#hardware", + "ffmpeg:front_door#audio=opus", + "ffmpeg:front_door#video=h264#hardware=vaapi", + "ffmpeg:front_door#video=h264#width=1280#rotate=-90", + # documented HomeKit source + "ffmpeg:front_door#video=h264#width=1280#height=720#audio=opus/16000", + "ffmpeg:http://10.0.0.1/flv?port=1935&app=bcs#video=copy", + ): + resp = client.put("/go2rtc/streams/legit", params={"src": src}) + assert resp.status_code == 200, ( + f"Expected 200 for ffmpeg src {src!r}; got {resp.status_code}" + ) + forwarded_src = mock_put.call_args.kwargs["params"]["src"] + assert forwarded_src == src + + def test_add_stream_allows_ffmpeg_injection_when_override_set(self): + """GO2RTC_ALLOW_ARBITRARY_EXEC also lifts the ffmpeg: parameter check""" + app = self._make_app(_MULTI_CAMERA_CONFIG) + mock_response = type("R", (), {"ok": True, "status_code": 200, "text": "ok"})() + with patch.dict(os.environ, {"GO2RTC_ALLOW_ARBITRARY_EXEC": "true"}): + with patch( + "frigate.api.camera.requests.put", return_value=mock_response + ) as mock_put: + with AuthTestClient(app) as client: + resp = client.put( + "/go2rtc/streams/legit", + params={"src": "ffmpeg:front_door#raw=-vn"}, + ) + assert resp.status_code == 200 + mock_put.assert_called_once() + def test_stream_alias_blocked_when_owning_camera_disallowed(self): """limited_user cannot access a stream alias that belongs to a camera they are not allowed to see.""" diff --git a/frigate/test/http_api/test_http_config_go2rtc_streams.py b/frigate/test/http_api/test_http_config_go2rtc_streams.py new file mode 100644 index 0000000000..c7d95bad82 --- /dev/null +++ b/frigate/test/http_api/test_http_config_go2rtc_streams.py @@ -0,0 +1,217 @@ +"""Tests for go2rtc stream source restrictions on config/save and config/set.""" + +import copy +import os +import tempfile +from unittest.mock import patch + +import ruamel.yaml + +from frigate.config.env import FRIGATE_ENV_VARS +from frigate.models import Event, Recordings, ReviewSegment +from frigate.test.http_api.base_http_test import AuthTestClient, BaseTestHttp + +# shape of the source in GHSA-qjjw-3v66-279f, with the payload replaced +_INJECTED_SRC = "ffmpeg:#input='-f''h264''-i''data:placeholder''-c''copy'" + +# an unrestricted ffmpeg: source the operator already has in the file on disk +_EXISTING_SRC = "ffmpeg:rtsp://10.0.0.1:554/video#raw=-vn" + +_CONFIG = { + "mqtt": {"host": "mqtt"}, + "go2rtc": {"streams": {"front_door": [_EXISTING_SRC]}}, + "cameras": { + "front_door": { + "ffmpeg": { + "inputs": [{"path": "rtsp://10.0.0.1:554/video", "roles": ["detect"]}] + }, + "detect": {"height": 1080, "width": 1920, "fps": 5}, + }, + }, +} + + +def _dump(config: dict) -> str: + yaml = ruamel.yaml.YAML() + stream = tempfile.SpooledTemporaryFile(mode="w+") + yaml.dump(config, stream) + stream.seek(0) + return stream.read() + + +class TestConfigGo2rtcStreamRestrictions(BaseTestHttp): + def setUp(self): + super().setUp([Event, Recordings, ReviewSegment]) + self.minimal_config = _CONFIG + self.app = super().create_app() + + fd, self.config_path = tempfile.mkstemp(suffix=".yml") + with os.fdopen(fd, "w") as f: + f.write(_dump(_CONFIG)) + self.addCleanup(os.unlink, self.config_path) + + find_patch = patch( + "frigate.api.app.find_config_file", return_value=self.config_path + ) + find_patch.start() + self.addCleanup(find_patch.stop) + + env_patch = patch.dict(os.environ) + env_patch.start() + self.addCleanup(env_patch.stop) + os.environ.pop("GO2RTC_ALLOW_ARBITRARY_EXEC", None) + + def _read_config(self) -> str: + with open(self.config_path) as f: + return f.read() + + def _set_streams(self, streams: dict): + with AuthTestClient(self.app) as client: + return client.put( + "/config/set", + json={ + "config_data": {"go2rtc": {"streams": streams}}, + "requires_restart": 1, + }, + ) + + def _save_streams(self, streams: dict, prefix: str = ""): + config = copy.deepcopy(_CONFIG) + config["go2rtc"]["streams"].update(streams) + with AuthTestClient(self.app) as client: + return client.post( + "/config/save?save_option=saveonly", + content=prefix + _dump(config), + headers={"Content-Type": "text/plain"}, + ) + + # --- config/set --- + + def test_set_rejects_injected_ffmpeg_source(self): + before = self._read_config() + resp = self._set_streams({"new_stream": [_INJECTED_SRC]}) + assert resp.status_code == 400 + assert resp.json()["success"] is False + assert self._read_config() == before + + def test_set_rejects_injected_string_source(self): + before = self._read_config() + resp = self._set_streams({"new_stream": "ffmpeg:front_door#raw=-vn"}) + assert resp.status_code == 400 + assert self._read_config() == before + + def test_set_rejects_injected_source_added_to_existing_stream(self): + before = self._read_config() + resp = self._set_streams({"front_door": [_EXISTING_SRC, _INJECTED_SRC]}) + assert resp.status_code == 400 + assert self._read_config() == before + + def test_set_allows_standard_sources(self): + resp = self._set_streams( + { + "new_stream": [ + "rtsp://10.0.0.2:554/video", + "ffmpeg:new_stream#audio=opus", + ] + } + ) + assert resp.status_code == 200, resp.text + assert "ffmpeg:new_stream#audio=opus" in self._read_config() + + def test_set_keeps_existing_source_on_disk(self): + resp = self._set_streams( + {"front_door": [_EXISTING_SRC, "ffmpeg:front_door#audio=opus"]} + ) + assert resp.status_code == 200, resp.text + assert _EXISTING_SRC in self._read_config() + + def test_set_allows_injected_source_when_override_set(self): + with patch.dict(os.environ, {"GO2RTC_ALLOW_ARBITRARY_EXEC": "true"}): + resp = self._set_streams({"new_stream": [_INJECTED_SRC]}) + assert resp.status_code == 200, resp.text + + # --- config/save --- + + def test_save_rejects_injected_ffmpeg_source(self): + before = self._read_config() + resp = self._save_streams({"new_stream": [_INJECTED_SRC]}) + assert resp.status_code == 400 + assert self._read_config() == before + + def test_save_rejects_exec_source(self): + before = self._read_config() + resp = self._save_streams({"new_stream": "exec:/tmp/something"}) + assert resp.status_code == 400 + assert self._read_config() == before + + def test_save_rejects_map_source(self): + before = self._read_config() + resp = self._save_streams({"new_stream": {"url": "rtsp://10.0.0.2/video"}}) + assert resp.status_code == 400 + assert self._read_config() == before + + def test_save_resubmitting_existing_config_allowed(self): + # the UI config editor always sends back the whole file + resp = self._save_streams({"new_stream": "rtsp://10.0.0.2:554/video"}) + assert resp.status_code == 200, resp.text + config = self._read_config() + assert _EXISTING_SRC in config + assert "rtsp://10.0.0.2:554/video" in config + + def test_save_tagged_config_with_standard_source_allowed(self): + # a tag on the root map still passes validation and startup parsing + resp = self._save_streams( + {"new_stream": "rtsp://10.0.0.2:554/video"}, prefix="!config\n" + ) + assert resp.status_code == 200, resp.text + + def test_save_tagged_config_with_injected_source_rejected(self): + before = self._read_config() + resp = self._save_streams({"new_stream": [_INJECTED_SRC]}, prefix="!config\n") + assert resp.status_code == 400 + assert "Restricted go2rtc stream source" in resp.json()["message"] + assert self._read_config() == before + + # --- {FRIGATE_*} placeholders resolve the same way as at go2rtc startup --- + + def test_set_rejects_unresolvable_placeholder(self): + before = self._read_config() + with patch.dict(FRIGATE_ENV_VARS, {"FRIGATE_RTSP_USER": "user"}): + for src, message in ( + ( + "{FRIGATE_RTSP_USER!r}fmpeg:front_door#raw=-vn", + "Invalid variable placeholder in go2rtc stream: new_stream", + ), + ( + "{FRIGATE_RTSP_USER:>5}", + "Invalid variable placeholder in go2rtc stream: new_stream", + ), + ( + "rtsp://{FRIGATE_UNDEFINED}@10.0.0.2:554/video", + "Unknown variable in go2rtc stream: new_stream", + ), + ): + resp = self._set_streams({"new_stream": [src]}) + assert resp.status_code == 400, src + assert resp.json()["message"] == message, src + assert self._read_config() == before + + def test_set_reports_each_issue(self): + resp = self._set_streams( + { + "typo_stream": ["rtsp://{FRIGATE_UNDEFINED}@10.0.0.2:554/video"], + "injected_stream": [_INJECTED_SRC], + } + ) + assert resp.status_code == 400 + assert resp.json()["message"] == ( + "Restricted go2rtc stream source in: injected_stream; " + "Unknown variable in go2rtc stream: typo_stream" + ) + + def test_set_allows_resolvable_placeholder(self): + src = "rtsp://{FRIGATE_RTSP_USER}@10.0.0.2:554/video" + with patch.dict(FRIGATE_ENV_VARS, {"FRIGATE_RTSP_USER": "user"}): + resp = self._set_streams({"new_stream": [src]}) + assert resp.status_code == 200, resp.text + assert src in self._read_config() diff --git a/frigate/test/http_api/test_http_go2rtc_proxy_auth.py b/frigate/test/http_api/test_http_go2rtc_proxy_auth.py new file mode 100644 index 0000000000..c06f174bf3 --- /dev/null +++ b/frigate/test/http_api/test_http_go2rtc_proxy_auth.py @@ -0,0 +1,73 @@ +"""Tests for the /auth subrequest on nginx-proxied go2rtc live-stream paths.""" + +import time +from unittest.mock import patch + +import requests +from fastapi.testclient import TestClient +from joserfc.jwk import OctKey + +from frigate.api.auth import create_encoded_jwt +from frigate.models import Event, Recordings, ReviewSegment, User +from frigate.test.http_api.base_http_test import BaseTestHttp + +_CONFIG = { + "mqtt": {"host": "mqtt"}, + "cameras": { + "front_door": { + "ffmpeg": { + "inputs": [{"path": "rtsp://10.0.0.1:554/video", "roles": ["detect"]}] + }, + "detect": {"height": 1080, "width": 1920, "fps": 5}, + }, + }, +} + +# shape of the source in GHSA-93xj-hg88-ffg7, with the payload replaced +_AD_HOC_SRC = "ffmpeg:%23input%3D'-f''h264''-i''data:placeholder''-c''copy'" + + +class TestGo2rtcProxyAuth(BaseTestHttp): + def setUp(self): + super().setUp([Event, ReviewSegment, Recordings, User]) + self.minimal_config = _CONFIG + self.app = super().create_app() + self.app.jwt_token = OctKey.import_key(b"0" * 32) + + get_patch = patch( + "frigate.api.auth.requests.get", + side_effect=requests.ConnectionError("go2rtc unavailable"), + ) + get_patch.start() + self.addCleanup(get_patch.stop) + + def _auth(self, role: str, original_url: str): + token = create_encoded_jwt( + role, role, int(time.time()) + 3600, self.app.jwt_token + ) + with TestClient(self.app) as client: + return client.get( + "/auth", + headers={ + "authorization": f"Bearer {token}", + "x-original-url": original_url, + }, + ) + + def test_viewer_ad_hoc_mse_source_denied(self): + resp = self._auth("viewer", f"/live/mse/api/ws?src={_AD_HOC_SRC}") + assert resp.status_code == 403 + + def test_viewer_ad_hoc_webrtc_sources_denied(self): + for path in ("/live/webrtc/api/ws", "/api/go2rtc/webrtc"): + resp = self._auth("viewer", f"{path}?src={_AD_HOC_SRC}") + assert resp.status_code == 403, path + + def test_admin_ad_hoc_mse_source_denied(self): + resp = self._auth("admin", f"/live/mse/api/ws?src={_AD_HOC_SRC}") + assert resp.status_code == 403 + + def test_viewer_camera_stream_allowed(self): + resp = self._auth("viewer", "/live/mse/api/ws?src=front_door") + assert resp.status_code == 202 + assert resp.headers["remote-role"] == "viewer" diff --git a/frigate/test/test_env.py b/frigate/test/test_env.py index 37b81a6564..a3cf350ac7 100644 --- a/frigate/test/test_env.py +++ b/frigate/test/test_env.py @@ -49,14 +49,10 @@ class TestGo2RtcAddStreamSubstitution(unittest.TestCase): def test_mixed_localtime_and_frigate_var(self): """%{localtime\\:...} alongside {FRIGATE_USER} substitutes only the var.""" FRIGATE_ENV_VARS["FRIGATE_USER"] = "admin" - src = ( - "ffmpeg:rtsp://host/s#raw=-vf " - "drawtext=text=%{localtime\\:%Y-%m-%d}:user={FRIGATE_USER}" - ) + src = "rtsp://host/s?text=%{localtime\\:%Y-%m-%d}&user={FRIGATE_USER}" self.assertEqual( self._call_route(src), - "ffmpeg:rtsp://host/s#raw=-vf " - "drawtext=text=%{localtime\\:%Y-%m-%d}:user=admin", + "rtsp://host/s?text=%{localtime\\:%Y-%m-%d}&user=admin", ) def test_unknown_var_falls_back_to_raw_src(self): @@ -232,6 +228,29 @@ class TestEnvVars(unittest.TestCase): result = validate_env_string("{FRIGATE_BROKER}") self.assertEqual(result, "mqtt.local") + def test_install_ignores_go2rtc_exec_override(self): + """The go2rtc exec override cannot be enabled from the config.""" + from frigate.util.services import is_go2rtc_arbitrary_exec_allowed + + env_patch = patch.dict(os.environ) + env_patch.start() + self.addCleanup(env_patch.stop) + os.environ.pop("GO2RTC_ALLOW_ARBITRARY_EXEC", None) + ctx = self._make_context(install=True) + with patch("frigate.util.services.os.path.isfile", return_value=False): + for key in ("GO2RTC_ALLOW_ARBITRARY_EXEC", "go2rtc_allow_arbitrary_exec"): + validate_env_vars({key: "true", "OTHER_VAR": "value"}, ctx) + self.assertNotIn(key, os.environ) + self.assertFalse(is_go2rtc_arbitrary_exec_allowed()) + self.assertEqual(os.environ.get("OTHER_VAR"), "value") + + def test_install_keeps_container_go2rtc_exec_override(self): + """An override set in the container environment is left untouched.""" + ctx = self._make_context(install=True) + with patch.dict(os.environ, {"GO2RTC_ALLOW_ARBITRARY_EXEC": "true"}): + validate_env_vars({"GO2RTC_ALLOW_ARBITRARY_EXEC": "false"}, ctx) + self.assertEqual(os.environ.get("GO2RTC_ALLOW_ARBITRARY_EXEC"), "true") + if __name__ == "__main__": unittest.main() diff --git a/frigate/test/test_go2rtc_stream_auth.py b/frigate/test/test_go2rtc_stream_auth.py index b525c94ee1..ddbf579630 100644 --- a/frigate/test/test_go2rtc_stream_auth.py +++ b/frigate/test/test_go2rtc_stream_auth.py @@ -8,6 +8,9 @@ param, which the static-media auth in `media_auth` does not inspect. import types import unittest +from unittest.mock import MagicMock, patch + +import requests from frigate.api.auth import deny_response_for_go2rtc_stream from frigate.config import FrigateConfig @@ -20,6 +23,7 @@ _CONFIG = { "dual_user": ["front_door", "back_door"], } }, + "go2rtc": {"streams": {"doorbell_audio": ["rtsp://10.0.0.4:554/audio"]}}, "cameras": { "front_door": { "ffmpeg": { @@ -46,7 +50,9 @@ _CONFIG = { def _request(config: FrigateConfig) -> types.SimpleNamespace: - return types.SimpleNamespace(app=types.SimpleNamespace(frigate_config=config)) + return types.SimpleNamespace( + app=types.SimpleNamespace(frigate_config=config), headers={} + ) class TestDenyResponseForGo2rtcStream(unittest.TestCase): @@ -54,6 +60,20 @@ class TestDenyResponseForGo2rtcStream(unittest.TestCase): self.config = FrigateConfig(**_CONFIG) self.request = _request(self.config) + # go2rtc is not running in tests; individual tests override this + get_patch = patch( + "frigate.api.auth.requests.get", + side_effect=requests.ConnectionError("go2rtc unavailable"), + ) + self.mock_get = get_patch.start() + self.addCleanup(get_patch.stop) + + def _go2rtc_streams(self, *names: str) -> None: + response = MagicMock(ok=True) + response.json.return_value = {name: {} for name in names} + self.mock_get.side_effect = None + self.mock_get.return_value = response + def _deny(self, url: str, role: str): return deny_response_for_go2rtc_stream(url, role, self.request) @@ -170,6 +190,107 @@ class TestDenyResponseForGo2rtcStream(unittest.TestCase): def test_missing_role_bypasses(self): self.assertIsNone(self._deny("http://host/live/mse/api/ws?src=back_door", None)) + # --- ad hoc sources are denied for every role --- + + def test_ad_hoc_ffmpeg_source_denied_for_all_roles(self): + # shape of the source in GHSA-93xj-hg88-ffg7 + src = "ffmpeg:%23input%3D'-f''h264''-i''data:placeholder''-c''copy'" + for path in ( + "/live/mse/api/ws", + "/live/webrtc/api/ws", + "/api/go2rtc/webrtc", + ): + for role in ("viewer", "admin", "limited_user", None): + self.assertEqual( + self._deny(f"http://host{path}?src={src}", role), + 403, + f"{path} {role}", + ) + + def test_ad_hoc_source_with_name_param_denied(self): + # name= would otherwise let go2rtc replace an existing stream's source + self.assertEqual( + self._deny( + "http://host/live/mse/api/ws?src=ffmpeg:x%23video%3Dh264" + "&name=front_door", + "admin", + ), + 403, + ) + + def test_ad_hoc_source_after_existing_src_denied(self): + self.assertEqual( + self._deny( + "http://host/live/mse/api/ws?src=front_door&src=exec:x", "viewer" + ), + 403, + ) + + def test_unknown_stream_denied_for_admin(self): + self.assertEqual( + self._deny("http://host/live/mse/api/ws?src=nonexistent", "admin"), 403 + ) + + def test_blank_src_denied_for_admin(self): + self.assertEqual(self._deny("http://host/live/mse/api/ws?src=", "admin"), 403) + + def test_go2rtc_config_stream_allowed(self): + self.assertIsNone( + self._deny("http://host/live/mse/api/ws?src=doorbell_audio", "viewer") + ) + self.mock_get.assert_not_called() + + def test_runtime_go2rtc_stream_allowed(self): + # streams added through PUT /go2rtc/streams only exist in go2rtc + self._go2rtc_streams("front_door", "wizard_test") + self.assertIsNone( + self._deny("http://host/live/mse/api/ws?src=wizard_test", "admin") + ) + + def test_runtime_lookup_does_not_admit_ad_hoc_source(self): + self._go2rtc_streams("front_door", "wizard_test") + self.assertEqual( + self._deny("http://host/live/mse/api/ws?src=ffmpeg:wizard_test", "admin"), + 403, + ) + + def test_go2rtc_error_fails_closed(self): + self.mock_get.side_effect = None + self.mock_get.return_value = MagicMock(ok=False) + self.assertEqual( + self._deny("http://host/live/mse/api/ws?src=wizard_test", "admin"), 403 + ) + + # --- path spellings nginx normalizes before matching its location --- + + def test_path_variants_are_checked(self): + for path in ( + "/live/mse/api/%77s", + "/live/mse/api%2Fws", + "//live/mse/api/ws", + "/live//mse/api/ws", + "/live/mse/api/./ws", + "/live/other/../mse/api/ws", + "/live/mse/api/%2e/ws", + "/%6Cive/webrtc/api/ws", + "/api/go2rtc/%77ebrtc", + "/live/mse/api/ws/", + "/live/mse/api/ws/extra", + "http://host//live/mse/api/ws", + ): + self.assertEqual(self._deny(f"{path}?src=nonexistent", "viewer"), 403, path) + + def test_path_variants_allow_existing_stream(self): + self.assertIsNone(self._deny("/live/mse/api/%77s?src=front_door", "viewer")) + + def test_nginx_marker_applies_check(self): + self.request.headers = {"x-go2rtc-stream-proxy": "1"} + self.assertEqual(self._deny("/unexpected?src=nonexistent", "viewer"), 403) + + def test_unrelated_paths_not_checked(self): + for path in ("/live/mse/api", "/api/go2rtc/streams", "/live/jsmpeg/x"): + self.assertIsNone(self._deny(f"{path}?src=nonexistent", "viewer"), path) + if __name__ == "__main__": unittest.main() diff --git a/frigate/util/services.py b/frigate/util/services.py index de17e29711..6ae83aa639 100644 --- a/frigate/util/services.py +++ b/frigate/util/services.py @@ -12,6 +12,7 @@ import subprocess as sp import time import traceback from datetime import datetime +from enum import Enum from typing import Any import cv2 @@ -986,6 +987,122 @@ def is_restricted_go2rtc_source(stream_source: str) -> bool: return not is_go2rtc_arbitrary_exec_allowed() +# go2rtc expands an `ffmpeg:` source into an `exec:` command line built from its +# `#key=value` parameters, so only parameters that map to fixed presets or plain +# numbers are accepted from the API +_GO2RTC_FFMPEG_NAME_PARAMS = frozenset({"video", "audio", "hardware"}) +_GO2RTC_FFMPEG_NUMERIC_PARAMS = frozenset( + {"width", "height", "rotate", "bitrate", "timeout"} +) +_GO2RTC_FFMPEG_FLAG_PARAMS = frozenset({"async"}) +# a preset or codec name, optionally with a sample rate (e.g. opus/16000) +_GO2RTC_FFMPEG_NAME_VALUE = re.compile(r"^([A-Za-z0-9_]*|[A-Za-z0-9_]+/[0-9]{1,6})$") +_GO2RTC_FFMPEG_NUMERIC_VALUE = re.compile(r"^-?[0-9]+[kKmM]?$") + + +def _is_safe_go2rtc_ffmpeg_source(stream_source: str) -> bool: + """Check that an `ffmpeg:` source only uses allowlisted parameters.""" + if any(c in stream_source for c in "'\"") or re.search(r"\s", stream_source): + return False + + _, *params = stream_source.split("#") + for param in params: + key, has_value, value = param.partition("=") + if key in _GO2RTC_FFMPEG_NAME_PARAMS: + if not _GO2RTC_FFMPEG_NAME_VALUE.match(value): + return False + elif key in _GO2RTC_FFMPEG_NUMERIC_PARAMS: + if not _GO2RTC_FFMPEG_NUMERIC_VALUE.match(value): + return False + elif key in _GO2RTC_FFMPEG_FLAG_PARAMS: + if has_value: + return False + else: + return False + + return True + + +def is_restricted_go2rtc_api_source(stream_source: str) -> bool: + """Check if a stream source submitted through the API is restricted. + + In addition to the echo/expr/exec sources, this restricts `ffmpeg:` sources + whose parameters could inject arbitrary ffmpeg arguments. The + GO2RTC_ALLOW_ARBITRARY_EXEC override lifts both restrictions. + """ + source = stream_source.strip() + if is_restricted_go2rtc_source(source): + return True + if not source.startswith("ffmpeg:"): + return False + if _is_safe_go2rtc_ffmpeg_source(source[len("ffmpeg:") :]): + return False + return not is_go2rtc_arbitrary_exec_allowed() + + +def _go2rtc_stream_sources(stream: Any) -> list[Any]: + return stream if isinstance(stream, list) else [stream] + + +class Go2rtcStreamIssue(str, Enum): + """Why a go2rtc stream source was rejected.""" + + restricted = "restricted" + unknown_variable = "unknown_variable" + invalid_placeholder = "invalid_placeholder" + + +def find_restricted_go2rtc_stream_changes( + old_streams: dict[str, Any] | None, new_streams: dict[str, Any] | None +) -> dict[str, Go2rtcStreamIssue]: + """Return the go2rtc streams with new sources that must be rejected. + + Used when the config is changed through the API. A source that is already + in the saved config for the same stream is left alone, so streams the + operator configured on disk keep working; any other source must resolve + its {FRIGATE_*} placeholders and pass `is_restricted_go2rtc_api_source`. + """ + # imported here since frigate.config imports this module + from frigate.config.env import substitute_frigate_vars + + old_streams = old_streams if isinstance(old_streams, dict) else {} + new_streams = new_streams if isinstance(new_streams, dict) else {} + issues: dict[str, Go2rtcStreamIssue] = {} + + for name, stream in new_streams.items(): + existing = _go2rtc_stream_sources(old_streams.get(name)) + + for source in _go2rtc_stream_sources(stream): + if source in existing: + continue + + if not isinstance(source, str): + # the map form is only usable with arbitrary exec enabled + if is_go2rtc_arbitrary_exec_allowed(): + continue + issues[name] = Go2rtcStreamIssue.restricted + break + + # only sources whose placeholders resolve here are accepted, so the + # resolved value checked below is what go2rtc loads at startup + try: + resolved_source = substitute_frigate_vars(source) + except KeyError: + issues[name] = Go2rtcStreamIssue.unknown_variable + break + except ValueError: + issues[name] = Go2rtcStreamIssue.invalid_placeholder + break + + if is_restricted_go2rtc_api_source( + source + ) or is_restricted_go2rtc_api_source(resolved_source): + issues[name] = Go2rtcStreamIssue.restricted + break + + return issues + + def ffprobe_stream(ffmpeg, path: str, detailed: bool = False) -> sp.CompletedProcess: """Run ffprobe on stream.""" clean_path = escape_special_characters(path)