diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index ed1df8fb55..2a96aac1b1 100644 --- a/.github/workflows/ci.yml +++ b/.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 diff --git a/docker/main/Dockerfile b/docker/main/Dockerfile index cc551429c0..0059425ea2 100644 --- a/docker/main/Dockerfile +++ b/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 diff --git a/docker/main/rootfs/etc/s6-overlay/s6-rc.d/certsync/run b/docker/main/rootfs/etc/s6-overlay/s6-rc.d/certsync/run index 4ec679bca4..d734aa9a73 100755 --- a/docker/main/rootfs/etc/s6-overlay/s6-rc.d/certsync/run +++ b/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 diff --git a/docker/main/rootfs/etc/s6-overlay/s6-rc.d/frigate/run b/docker/main/rootfs/etc/s6-overlay/s6-rc.d/frigate/run index 7d060e1723..465fea136e 100755 --- a/docker/main/rootfs/etc/s6-overlay/s6-rc.d/frigate/run +++ b/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 diff --git a/docker/main/rootfs/etc/s6-overlay/s6-rc.d/go2rtc/run b/docker/main/rootfs/etc/s6-overlay/s6-rc.d/go2rtc/run index 6c5012feec..50fe9a30de 100755 --- a/docker/main/rootfs/etc/s6-overlay/s6-rc.d/go2rtc/run +++ b/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 diff --git a/docker/main/rootfs/etc/s6-overlay/s6-rc.d/init-usermod/run b/docker/main/rootfs/etc/s6-overlay/s6-rc.d/init-usermod/run index 744ded7039..98000f1917 100755 --- a/docker/main/rootfs/etc/s6-overlay/s6-rc.d/init-usermod/run +++ b/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 diff --git a/docker/main/rootfs/etc/s6-overlay/s6-rc.d/nginx/run b/docker/main/rootfs/etc/s6-overlay/s6-rc.d/nginx/run index bed1a8cb85..ce9805a36a 100755 --- a/docker/main/rootfs/etc/s6-overlay/s6-rc.d/nginx/run +++ b/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 \ diff --git a/docker/main/rootfs/etc/s6-overlay/s6-rc.d/prepare/run b/docker/main/rootfs/etc/s6-overlay/s6-rc.d/prepare/run index 8372054b85..343b2e1082 100755 --- a/docker/main/rootfs/etc/s6-overlay/s6-rc.d/prepare/run +++ b/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 diff --git a/docker/main/rootfs/usr/local/bin/fix-ownership b/docker/main/rootfs/usr/local/bin/fix-ownership index c6c6d6bc8c..26ca7bf07d 100755 --- a/docker/main/rootfs/usr/local/bin/fix-ownership +++ b/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 } diff --git a/docker/main/rootfs/usr/local/bin/service-runs-as-root b/docker/main/rootfs/usr/local/bin/service-runs-as-root index 69944cb7d8..d40a817544 100755 --- a/docker/main/rootfs/usr/local/bin/service-runs-as-root +++ b/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 diff --git a/docker/main/rootfs/usr/local/nginx/conf/nginx.conf b/docker/main/rootfs/usr/local/nginx/conf/nginx.conf index 8235392452..11333f0c1c 100644 --- a/docker/main/rootfs/usr/local/nginx/conf/nginx.conf +++ b/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; diff --git a/frigate/app.py b/frigate/app.py index 01440508b4..892978049f 100644 --- a/frigate/app.py +++ b/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", diff --git a/frigate/record/maintainer.py b/frigate/record/maintainer.py index f949979ec8..7bb90ae823 100644 --- a/frigate/record/maintainer.py +++ b/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) diff --git a/frigate/test/test_ownership.py b/frigate/test/test_ownership.py index 495e40a72e..529c6e2cab 100644 --- a/frigate/test/test_ownership.py +++ b/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")