clean up

.github/workflows/ci.yml

@@ -97,11 +97,9 @@ jobs:
             echo "response carries frame-ancestors, which breaks cross-origin iframe embedding"
             exit 1
           fi
-          # -t must NOT run as root: ngx_create_paths would chown the live cache
-          # and temp dirs to the `user root` directive user, breaking the workers.
-          # stdout goes to /dev/null because -t reopens the config's
-          # error_log/access_log /dev/stdout by path, and the docker exec pipe
-          # is root-owned; -t reports on stderr, so nothing is lost
+          # -t as root would chown the live cache and temp dirs to the `user`
+          # directive user; stdout discarded because -t reopens the config's
+          # /dev/stdout logs and the docker exec pipe is root-owned
           docker exec frigate /command/s6-setuidgid frigate bash -c '/usr/local/nginx/sbin/nginx -e stderr -t -c /tmp/nginx/conf/nginx.conf >/dev/null'
           docker exec frigate stat -c %a /etc/letsencrypt/live/frigate/privkey.pem | grep -qx 600
           docker exec frigate stat -c %a /dev/shm/go2rtc.yaml | grep -qx 640
@@ -125,8 +123,7 @@ jobs:
           code=$(curl -s -o /dev/null -w '%{http_code}' -X POST http://127.0.0.1:5000/api/login \
             -H 'content-type: application/json' -d '{"user":"admin","password":"definitely-wrong"}')
           [ "$code" = "401" ] || { echo "login endpoint returned $code"; exit 1; }
-          # nginx runtime state must belong to the runtime user (a root nginx -t
-          # in the step above would have chowned it to root)
+          # a root nginx -t above would have chowned the runtime dirs to root
           owners=$(docker exec frigate stat -c %U /tmp/nginx /dev/shm/nginx_cache)
           echo "$owners"
           if echo "$owners" | grep -qvx frigate; then
@@ -135,12 +132,10 @@ jobs:
           # runtime user can write recordings storage
           docker exec frigate /command/s6-setuidgid frigate touch /media/frigate/.write-probe
           docker exec frigate rm /media/frigate/.write-probe
-          # /tmp/cache is a tmpfs mount here, as the docs recommend: it arrives
-          # root-owned, and the ZMQ IPC sockets live in it
+          # tmpfs mount per the docs: arrives root-owned, holds the ZMQ IPC sockets
           docker exec frigate /command/s6-setuidgid frigate touch /tmp/cache/.write-probe
           docker exec frigate rm /tmp/cache/.write-probe
-          # bundled models must be readable by the runtime user: they are baked
-          # in as root, and archive members can carry root-only modes
+          # models are baked in as root and archive members can carry root-only modes
           docker exec frigate /command/s6-setuidgid frigate sh -c '
             for f in /cpu_model.tflite /edgetpu_model.tflite /cpu_audio_model.tflite \
                      /labelmap.txt /audio-labelmap.txt /openvino-model/*; do
@@ -151,9 +146,7 @@ jobs:
         run: |
           mkdir -p /tmp/frigate-config-root
           printf 'mqtt:\n  enabled: false\ncameras: {}\n' > /tmp/frigate-config-root/config.yml
-          # pre-seed a sentinel: the assertion below is that the escape hatch
-          # DELETES it. Against a fresh dir the absence check passes vacuously
-          # and proves nothing about the rm -f in the prepare script.
+          # pre-seed so the absence check proves the rm -f, not a vacuous pass
           echo "2:1000:1000" > /tmp/frigate-config-root/.permissions_version
           docker run -d --name frigate-root --shm-size 256m \
             -e FRIGATE_RUN_AS_ROOT=true \
@@ -170,8 +163,7 @@ jobs:
           echo "$ps_out" | grep -w python3 | grep -q '^root'
           echo "$ps_out" | grep -w go2rtc  | grep -q '^root'
           echo "$ps_out" | grep -w nginx   | grep -q '^root'
-          # escape hatch must have DELETED the pre-seeded sentinel. written as
-          # an if because bash exempts a negated command from set -e
+          # an if, not ! test: bash exempts negated commands from set -e
           if docker exec frigate-root test -f /config/.permissions_version; then
             echo "escape hatch did not delete the sweep sentinel"; exit 1
           fi
@@ -195,8 +187,7 @@ jobs:
           echo "$ps_out"
           # the listed service runs as root
           echo "$ps_out" | grep -w python3 | grep -q '^root'
-          # unlisted services still drop. written as ifs because bash exempts
-          # a negated command from set -e
+          # unlisted services still drop; ifs because set -e exempts negated commands
           if echo "$ps_out" | grep -w go2rtc | grep -q '^root'; then
             echo "go2rtc is unexpectedly running as root"; exit 1
           fi
@@ -207,8 +198,7 @@ jobs:
           docker exec frigate-granular cat /config/.permissions_version | grep -qx "2:1000:1000:frigate"
           # the root frigate process chowns the db it creates (first-boot immediacy)
           docker exec frigate-granular stat -c %u /config/frigate.db | grep -qx 1000
-          # per-boot sweep guard: plant a root-owned straggler, restart, and
-          # it must come back owned by the runtime user
+          # plant a root-owned straggler; the per-boot sweep must reclaim it on restart
           docker exec frigate-granular sh -c 'mkdir -p /media/frigate/clips && touch /media/frigate/clips/straggler.webp'
           docker restart frigate-granular
           up=0
@@ -235,8 +225,7 @@ jobs:
           if [ "$found" -ne 1 ]; then
             echo "no fail-fast error for an unknown service name"; docker logs frigate-badsvc; exit 1
           fi
-          # the failed oneshot must have blocked startup (prepare and every
-          # service depend on init-usermod transitively)
+          # the failed oneshot blocks startup through the dependency chain
           if docker exec frigate-badsvc curl -fs http://127.0.0.1:5000/api/version; then
             echo "container came up despite an invalid FRIGATE_ROOT_SERVICES"; exit 1
           fi

docker/main/Dockerfile

@@ -146,8 +146,6 @@ RUN wget -q https://github.com/openvinotoolkit/open_model_zoo/raw/master/data/da
 RUN wget -qO - https://www.kaggle.com/api/v1/models/google/yamnet/tfLite/classification-tflite/1/download | tar xvz && mv 1.tflite cpu_audio_model.tflite
 COPY audio-labelmap.txt .
 
-# tar restores the archive's modes and the yamnet member ships 0700, so without
-# this the unprivileged runtime user cannot read the audio model
 RUN chmod -R a+rX /rootfs
 
 

docker/main/rootfs/etc/s6-overlay/s6-rc.d/certsync/run

@@ -6,10 +6,8 @@ set -o errexit -o nounset -o pipefail
 
 # Logs should be sent to stdout so that s6 can collect them
 
-# Signal the master directly rather than running `nginx -s reload`. That would
-# have root parse /tmp/nginx/conf, which the unprivileged nginx user can
-# rewrite, and nginx acts on path directives while loading a config: it creates
-# them and chowns them to the configured user.
+# Not `nginx -s reload`: that has root parse /tmp/nginx/conf, which the
+# unprivileged nginx user can rewrite, and nginx chowns path directives on load.
 function reload_nginx() {
     local pid
 

docker/main/rootfs/etc/s6-overlay/s6-rc.d/frigate/run

@@ -11,10 +11,8 @@ if [[ "$(id -u)" -eq 0 ]]; then
     fi
 fi
 
-# $HOME is /root from the container env and survives s6-setuidgid, so cache
-# and telemetry writes (huggingface, openvino) fail after the drop. Set it
-# before opt_in_out so the opt-out marker lands where the service will look.
-# When the service keeps root, /root stays correct.
+# /root survives s6-setuidgid and breaks cache writes after the drop; set
+# before opt_in_out so the opt-out marker lands where the service will look
 if [[ "$runs_as_root" -eq 0 ]]; then
     export HOME=/config
 fi

docker/main/rootfs/etc/s6-overlay/s6-rc.d/go2rtc/run

@@ -119,10 +119,8 @@ setup_homekit_config "${homekit_config_path}"
 
 if [[ "$(id -u)" -eq 0 && "$runs_as_root" -eq 0 ]]; then
     chown go2rtc:go2rtc /dev/shm/go2rtc.yaml 2>/dev/null || true
-    # go2rtc rewrites this in place (os.WriteFile, no rename), so owning the
-    # file is enough; /config grants frigate-data traverse only. Tolerated so
-    # a chown-refusing mount (NFS root_squash) degrades pairing persistence
-    # instead of crash-looping the service
+    # go2rtc rewrites this in place, so owning the file is enough. Tolerated so
+    # a chown-refusing mount (NFS root_squash) degrades pairing, not the service
     chown go2rtc:frigate-data "${homekit_config_path}" 2>/dev/null && chmod 664 "${homekit_config_path}" 2>/dev/null || \
         echo "[WARN] Could not hand ${homekit_config_path} to the go2rtc user; HomeKit pairing changes may not persist"
 fi

docker/main/rootfs/etc/s6-overlay/s6-rc.d/init-usermod/run

@@ -19,8 +19,7 @@ if [[ "${FRIGATE_RUN_AS_ROOT:-false}" == "true" ]]; then
     exit 0
 fi
 
-# Validate before anything consumes the list: a typo silently dropping a
-# service to non-root would defeat the reason the user set it.
+# a typo must fail the boot, not silently drop a service to non-root
 if [[ -n "${FRIGATE_ROOT_SERVICES:-}" ]]; then
     IFS=',' read -ra root_services <<< "${FRIGATE_ROOT_SERVICES}"
     for entry in "${root_services[@]}"; do

docker/main/rootfs/etc/s6-overlay/s6-rc.d/nginx/run

@@ -69,11 +69,9 @@ function set_worker_processes() {
     sed -i "s/worker_processes auto;/worker_processes ${cpus};/" /tmp/nginx/conf/nginx.conf
 }
 
-# Rebuilt root-owned and fresh every start. A previous unprivileged nginx owned
-# this tree, and the cp and tempio writes below run as root: without wiping it
-# first, a planted symlink here would let those writes land on any root file.
-# rm does not traverse symlinks, and the bare mkdir fails closed if /tmp/nginx
-# is raced into a symlink before we can create it.
+# Rebuilt root-owned every start: a symlink planted by the previously
+# unprivileged nginx would redirect the root cp/tempio writes below onto any
+# root file. rm does not traverse symlinks; the bare mkdir fails closed if raced.
 rm -rf /tmp/nginx
 mkdir /tmp/nginx
 mkdir -p /tmp/nginx/conf /tmp/nginx/client_body /tmp/nginx/proxy \
@@ -113,14 +111,12 @@ echo "$nginx_settings" | \
 
 if [[ "$(id -u)" -eq 0 && "$runs_as_root" -eq 0 ]]; then
     chown -R frigate:frigate /tmp/nginx
-    # heal the cache if a root `nginx -t` chowned it (ngx_create_paths chowns
-    # every cycle path to the `user` directive user when run as root)
+    # heal the cache: a root `nginx -t` chowns every cycle path to the `user` directive user
     if [ -d /dev/shm/nginx_cache ]; then
         chown -R frigate:frigate /dev/shm/nginx_cache
     fi
-    # error_log/access_log /dev/stdout make nginx REOPEN the s6 log pipe by
-    # path, and s6 created it root-owned 0600; without this the non-root
-    # master exits with "open() /dev/stdout failed (13: Permission denied)"
+    # nginx reopens /dev/stdout by path for its logs, and s6 made the pipe
+    # root-owned 0600; without this the non-root master exits EACCES
     chown frigate /dev/stdout
     # self-signed certs are root-generated; tolerant because mounted certs may be :ro
     if [ -f "$letsencrypt_path/privkey.pem" ]; then
@@ -130,8 +126,7 @@ fi
 
 # Replace the bash process with the NGINX process, redirecting stderr to stdout
 exec 2>&1
-# -e stderr: the compile-time default error log under /usr/local/nginx/logs
-# is not writable by the runtime user and would alert before config load
+# -e stderr: the compiled-in error log path is not writable by the runtime user
 if [[ "$(id -u)" -ne 0 || "$runs_as_root" -eq 1 ]]; then
     exec \
         s6-notifyoncheck -t 30000 -n 1 \

docker/main/rootfs/etc/s6-overlay/s6-rc.d/prepare/run

@@ -153,14 +153,12 @@ if [[ "$(id -u)" -eq 0 ]]; then
     if [[ "${FRIGATE_RUN_AS_ROOT:-false}" == "true" ]]; then
         rm -f /config/.permissions_version
     else
-        # Only bless the sentinel when a mount backs /media/frigate itself or
-        # below it. A parent /media mount doesn't count: a dedicated
-        # /media/frigate volume added later would be shadowed and skipped.
+        # Only when a mount backs /media/frigate itself: under a parent /media
+        # mount, a dedicated volume added later would be shadowed and skipped
         sentinel_args=(--sentinel /config/.permissions_version)
         root_services_mode=""
         if [[ -n "${FRIGATE_ROOT_SERVICES:-}" ]]; then
-            # || true: grep -v exits non-zero on an all-empty list (e.g. ",")
-            # and errexit+pipefail would otherwise abort the boot over it
+            # || true: an all-empty list (",") fails grep -v and errexit would kill the boot
             root_services_mode=$(tr ',' '\n' <<< "${FRIGATE_ROOT_SERVICES//[[:space:]]/}" | grep -v '^$' | sort -u | paste -sd, - || true)
             if [[ -n "$root_services_mode" ]]; then
                 sentinel_args+=(--mode "$root_services_mode")
@@ -172,13 +170,10 @@ if [[ "$(id -u)" -eq 0 ]]; then
         /usr/local/bin/fix-ownership "${sentinel_args[@]}" \
             "${PUID:-1000}" "${PGID:-1000}" /config /media/frigate
 
-        # Root services write some files as root while running (clips
-        # stragglers, caches, stale db journals). These trees are small, so
-        # realign them on every boot. Recordings are chowned at create and
-        # stay behind the sentinel sweep above.
+        # Root services write clips stragglers and caches mid-run; realign the
+        # small trees every boot. Recordings are chowned at create instead.
         if [[ -n "$root_services_mode" ]]; then
-            # clips and exports don't exist until the frigate service has run
-            # once; a WARN about them on a normal first boot invites reports
+            # only sweep what exists; clips and exports appear after the first run
             boot_sweep_paths=(/config)
             for extra in /media/frigate/clips /media/frigate/exports; do
                 if [[ -d "$extra" ]]; then
@@ -191,8 +186,7 @@ if [[ "$(id -u)" -eq 0 ]]; then
     fi
 fi
 
-# Not in the image, and the runtime user cannot create it under root-owned
-# /media. Must stay after the sweep, which reads an absent /media/frigate as an
+# Must stay after the sweep, which reads an absent /media/frigate as an
 # unmounted volume rather than a swept one
 if [[ "$(id -u)" -eq 0 && ! -d /media/frigate ]]; then
     mkdir -p /media/frigate
@@ -201,8 +195,7 @@ if [[ "$(id -u)" -eq 0 && ! -d /media/frigate ]]; then
     fi
 fi
 
-# Usually a tmpfs mount, so it arrives root-owned and is outside the swept
-# volumes. The runtime user binds its ZMQ IPC sockets in here.
+# usually a tmpfs mount: root-owned on arrival and outside the swept volumes
 if [[ "$(id -u)" -eq 0 && "${FRIGATE_RUN_AS_ROOT:-false}" != "true" ]]; then
     mkdir -p /tmp/cache
     chown "${PUID:-1000}:${PGID:-1000}" /tmp/cache

docker/main/rootfs/usr/local/bin/fix-ownership

@@ -11,8 +11,7 @@
 #   --mode       append STRING to the sentinel, so changing it re-sweeps once
 #
 # Only files whose uid OR gid differs are touched, so re-runs are cheap.
-# lost+found is skipped: it belongs to the filesystem, not to Frigate, and
-# fsck puts recovered fragments of arbitrary files there under root-only 0700.
+# lost+found is skipped: fsck fills it with root-only recovered fragments.
 # Top-level /config additionally grants group frigate-data TRAVERSE ONLY
 # (g+rx) so the separate go2rtc user can reach its pre-created HomeKit file
 # on hosts where /config is mounted 0700. Never g+w: directory write means
@@ -64,10 +63,8 @@ if [[ "$(id -u)" -ne 0 ]]; then
     exit 0
 fi
 
-# The mode string folds the FRIGATE_ROOT_SERVICES list into the sentinel, so
-# entering or leaving a granular root mode re-sweeps once. That is the
-# backstop for anything a root service created that the per-boot sweep and
-# chown-at-create did not cover.
+# The list folds into the sentinel so entering or leaving a granular root mode
+# re-sweeps once, catching whatever the other ownership mechanisms missed.
 sentinel_content="${schema}:${target_uid}:${target_gid}"
 if [[ -n "$mode" ]]; then
     sentinel_content="${sentinel_content}:${mode}"
@@ -125,8 +122,7 @@ for path in "$@"; do
         continue
     fi
 
-    # -print feeds the progress counter while -exec {} + keeps the chown
-    # batched; the scan above is what makes a real percentage possible
+    # -print feeds the progress counter; -exec {} + keeps the chown batched
     started=$SECONDS
     if find "$path" -name lost+found -prune -o \( -not -uid "$target_uid" -o -not -gid "$target_gid" \) \
         -print -exec chown -h "${target_uid}:${target_gid}" {} + \
@@ -137,8 +133,8 @@ for path in "$@"; do
                 if (pct > 100) pct = 100
                 if (pct >= next_pct) {
                     printf "[INFO] fix-ownership: %s %d%% (%d/%d entries)\n", path, pct, NR, total
-                    # mawk block-buffers to a pipe; without this the whole
-                    # progress log arrives at once when the sweep ends
+                    # mawk block-buffers to a pipe; without fflush the whole
+                    # progress log arrives at once
                     fflush()
                     while (next_pct <= pct) next_pct += 5
                 }

docker/main/rootfs/usr/local/bin/service-runs-as-root

@@ -1,7 +1,6 @@
 #!/bin/bash
-# Exit 0 when FRIGATE_ROOT_SERVICES names the given service.
-# Membership only: FRIGATE_RUN_AS_ROOT precedence and the euid check stay in
-# the callers, which each combine them differently.
+# Exit 0 when FRIGATE_ROOT_SERVICES names the given service. Membership only:
+# the euid and FRIGATE_RUN_AS_ROOT checks stay in the callers.
 #
 # Usage: service-runs-as-root SERVICE
 

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

@@ -1,10 +1,8 @@
-# Copied to /tmp/nginx/conf at startup and loaded with -c from there. Relative
-# includes resolve against the -c file, but every other path directive resolves
-# against the compile-time --prefix, so non-include paths must stay absolute.
+# Loaded with -c from the /tmp/nginx/conf copy: relative includes follow the -c
+# file, all other path directives follow --prefix and must stay absolute.
 
 daemon off;
-# Ignored by a non-root master; required by FRIGATE_RUN_AS_ROOT so workers
-# stay root instead of the compiled-in default user
+# ignored by a non-root master; keeps workers root under FRIGATE_RUN_AS_ROOT
 user root;
 worker_processes auto;
 

frigate/app.py

@@ -231,9 +231,8 @@ class FrigateApp:
 
         migrate_db.close()
 
-        # A root frigate service (FRIGATE_ROOT_SERVICES) creates these as
-        # root. The wal and shm journals can be recreated as root later in
-        # the run; the per-boot sweep of /config realigns those on restart.
+        # a root frigate service creates these as root; wal and shm recreated
+        # later in the run are realigned by the per-boot /config sweep
         for db_file in (
             self.config.database.path,
             f"{self.config.database.path}-wal",

frigate/record/maintainer.py

@@ -929,8 +929,7 @@ class RecordingMaintainer(threading.Thread):
         )
 
         os.makedirs(directory, exist_ok=True)
-        # makedirs creates the date and hour levels too; own every level so
-        # the host user can prune old recordings
+        # own every level makedirs creates so the host user can prune recordings
         level = directory
         while level != RECORD_DIR:
             chown_to_runtime(level)

frigate/test/test_ownership.py

@@ -16,8 +16,7 @@ class FakePwEntry:
 class TestGetRuntimeIds(unittest.TestCase):
     def setUp(self) -> None:
         ownership.get_runtime_ids.cache_clear()
-        # and again on the way out, so a value cached under this test's
-        # patches never leaks into later test modules in the same process
+        # a value cached under this test's patches must not leak into later modules
         self.addCleanup(ownership.get_runtime_ids.cache_clear)
 
     @patch("frigate.util.ownership.os.geteuid", return_value=1000)
@@ -53,8 +52,7 @@ class TestGetRuntimeIds(unittest.TestCase):
 class TestChownToRuntime(unittest.TestCase):
     def setUp(self) -> None:
         ownership.get_runtime_ids.cache_clear()
-        # and again on the way out, so a value cached under this test's
-        # patches never leaks into later test modules in the same process
+        # a value cached under this test's patches must not leak into later modules
         self.addCleanup(ownership.get_runtime_ids.cache_clear)
 
     @patch("frigate.util.ownership.os.chown")