Harden container startup (#24718)
CI / AMD64 Build (push) Canceled after 0s
CI / AMD64 Smoke Test (push) Canceled after 0s
CI / ARM Build (push) Canceled after 0s
CI / Jetson Jetpack 6 (push) Canceled after 0s
CI / AMD64 Extra Build (push) Canceled after 0s
CI / ARM Extra Build (push) Canceled after 0s
CI / Synaptics Build (push) Canceled after 0s
CI / Assemble and push default build (push) Canceled after 0s

* keep /config out of the root environment in the frigate run script

The script exported HOME=/config and prepended /config/.local/lib to LD_LIBRARY_PATH while still root, then ran python3, ffmpeg, grep, and opt_in_out as root. The runtime user owns /config, so it could plant a .pth file in the user site-packages or a library on the loader path and get code run as root on the next start. Root helpers now run with root's environment (and python3 -I), and HOME, LD_LIBRARY_PATH, and PATH reach only the dropped process through env on the s6-setuidgid line. opt_in_out runs as the runtime user, so the openvino marker no longer gets written into /config by root.

* don't follow a planted symlink when writing /dev/shm/go2rtc.yaml

After the first start the go2rtc user owned /dev/shm/go2rtc.yaml and could swap it for a dangling symlink. The run script's -f test was false for a dangling link, so the link survived, and root's create_config.py then created the link target and the chown handed it to go2rtc. The run script now removes any existing entry, create_config.py creates the file with O_EXCL and O_NOFOLLOW at mode 0640, and the chown uses -h.

* run root python helpers in isolated mode and keep HOME=/config for non-root containers

The previous commit only set HOME=/config when dropping from root, so a container started with --user lost it. The other root-run helpers in the go2rtc, nginx, and certsync scripts now use python3 -I too, so they never load user site-packages or PYTHON* variables.

* validate networking.listen values before rendering them into nginx config

ListenConfig and get_nginx_settings.py only checked that the text after the last colon parsed as an int, and tempio rendered the raw string into the listen directive, so a value like "5000; location /x/ { alias /; } #:5000" injected nginx directives. Both now go through parse_listen_address, which accepts only a port, IPv4:port, or [IPv6]:port with a 1 to 65535 port and rebuilds the value from the parsed parts. get_nginx_settings.py falls back to the default port on an invalid value so nginx still serves the safe-mode UI.

* tighten comments

* accept hostnames, wildcards, and padded values in networking.listen

The stricter parser broke localhost:5000, *:5000, and values with surrounding whitespace that the old int() parsing accepted. parse_listen_address now strips the value and accepts RFC 1123 hostnames and *, still rebuilding the rendered value from the parsed parts. The ListenConfig validator runs in before mode, because pydantic coerced true to port 1 while get_nginx_settings.py rejected it and fell back to 5000, so the auth internal port and nginx disagreed.

* always create go2rtc.yaml after removing it and don't let opt_in_out stop frigate

The go2rtc else-branch was only reachable when a regular file was planted between rm and the test, and then loaded that file as go2rtc's config. create_config.py now runs unconditionally and its exclusive create fails closed. opt_in_out runs after s6-svc -O, so a failure there was terminal under errexit.

* fall back to default listen ports when either is invalid or they collide

* keep valid listen values and refuse to start nginx when the ports collide
This commit is contained in:
Josh Hawkins
2026-10-19 04:57:35 -10:00
committed by GitHub
parent 3d3f6a0678
commit 9a0672a64d
9 changed files with 197 additions and 75 deletions
@@ -36,8 +36,8 @@ else
lefile="/config/tls/fullchain.pem" lefile="/config/tls/fullchain.pem"
fi fi
tls_enabled=`python3 /usr/local/nginx/get_nginx_settings.py | jq -r .tls.enabled` tls_enabled=`python3 -I /usr/local/nginx/get_nginx_settings.py | jq -r .tls.enabled`
listen_external_port=`python3 /usr/local/nginx/get_nginx_settings.py | jq -r .listen.external_port` listen_external_port=`python3 -I /usr/local/nginx/get_nginx_settings.py | jq -r .listen.external_port`
while true while true
do do
@@ -11,30 +11,13 @@ if [[ "$(id -u)" -eq 0 ]]; then
fi fi
fi fi
# /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
# detector runtimes installed at first start (pip install --user) live under
# $HOME/.local; the dynamic loader only reads LD_LIBRARY_PATH at exec time
export LD_LIBRARY_PATH="${LD_LIBRARY_PATH:+${LD_LIBRARY_PATH}:}${HOME}/.local/lib"
export PATH="${PATH}:${HOME}/.local/bin"
# opt out of openvino telemetry
if [ -e /usr/local/bin/opt_in_out ]; then
/usr/local/bin/opt_in_out --opt_out > /dev/null 2>&1
fi
# Logs should be sent to stdout so that s6 can collect them
# Tell S6-Overlay not to restart this service # Tell S6-Overlay not to restart this service
s6-svc -O . s6-svc -O .
# Runs as root with root's HOME and LD_LIBRARY_PATH; -I skips user site-packages
function set_libva_version() { function set_libva_version() {
local ffmpeg_path local ffmpeg_path
ffmpeg_path=$(python3 /usr/local/ffmpeg/get_ffmpeg_path.py) ffmpeg_path=$(python3 -I /usr/local/ffmpeg/get_ffmpeg_path.py)
LIBAVFORMAT_VERSION_MAJOR=$("$ffmpeg_path" -version | grep -Po "libavformat\W+\K\d+") LIBAVFORMAT_VERSION_MAJOR=$("$ffmpeg_path" -version | grep -Po "libavformat\W+\K\d+")
export LIBAVFORMAT_VERSION_MAJOR export LIBAVFORMAT_VERSION_MAJOR
} }
@@ -42,14 +25,32 @@ function set_libva_version() {
echo "[INFO] Preparing Frigate..." echo "[INFO] Preparing Frigate..."
set_libva_version set_libva_version
# HOME=/root survives s6-setuidgid, so any run that is not root gets
# HOME=/config explicitly. The runtime user owns /config, so these paths go
# only to the service process, never to root. Detector runtimes installed at
# first start (pip install --user) live under $HOME/.local.
run_as=()
runtime_home="${HOME}"
if [[ "$runs_as_root" -eq 0 ]]; then
runtime_home=/config
if [[ "$(id -u)" -eq 0 ]]; then
run_as=(s6-setuidgid frigate)
fi
fi
run_as+=(env
"HOME=${runtime_home}"
"LD_LIBRARY_PATH=${LD_LIBRARY_PATH:+${LD_LIBRARY_PATH}:}${runtime_home}/.local/lib"
"PATH=${PATH}:${runtime_home}/.local/bin")
# opt out of openvino telemetry; the marker lands under the service's HOME
if [ -e /usr/local/bin/opt_in_out ]; then
"${run_as[@]}" /usr/local/bin/opt_in_out --opt_out > /dev/null 2>&1 || true
fi
echo "[INFO] Starting Frigate..." echo "[INFO] Starting Frigate..."
cd /opt/frigate || echo "[ERROR] Failed to change working directory to /opt/frigate" cd /opt/frigate || echo "[ERROR] Failed to change working directory to /opt/frigate"
# Replace the bash process with the Frigate process, redirecting stderr to stdout # Replace the bash process with the Frigate process, redirecting stderr to stdout
exec 2>&1 exec 2>&1
if [[ "$(id -u)" -ne 0 || "$runs_as_root" -eq 1 ]]; then exec "${run_as[@]}" python3 -u -m frigate
exec python3 -u -m frigate
else
exec s6-setuidgid frigate python3 -u -m frigate
fi
@@ -59,38 +59,33 @@ function get_ip_and_port_from_supervisor() {
function set_libva_version() { function set_libva_version() {
local ffmpeg_path local ffmpeg_path
ffmpeg_path=$(python3 /usr/local/ffmpeg/get_ffmpeg_path.py) ffmpeg_path=$(python3 -I /usr/local/ffmpeg/get_ffmpeg_path.py)
LIBAVFORMAT_VERSION_MAJOR=$("$ffmpeg_path" -version | grep -Po "libavformat\W+\K\d+") LIBAVFORMAT_VERSION_MAJOR=$("$ffmpeg_path" -version | grep -Po "libavformat\W+\K\d+")
export LIBAVFORMAT_VERSION_MAJOR export LIBAVFORMAT_VERSION_MAJOR
} }
set_libva_version set_libva_version
if [[ -f "/dev/shm/go2rtc.yaml" ]]; then # create_config.py creates the file exclusively and fails if anything
echo "[INFO] Removing stale config from last run..." # reappears at this path
rm /dev/shm/go2rtc.yaml rm -f /dev/shm/go2rtc.yaml
echo "[INFO] Preparing new go2rtc config..."
if [[ -n "${SUPERVISOR_TOKEN:-}" ]]; then
# Running as a Home Assistant Add-on, infer the IP address and port
get_ip_and_port_from_supervisor
fi fi
if [[ ! -f "/dev/shm/go2rtc.yaml" ]]; then python3 -I /usr/local/go2rtc/create_config.py
echo "[INFO] Preparing new go2rtc config..."
if [[ -n "${SUPERVISOR_TOKEN:-}" ]]; then
# Running as a Home Assistant Add-on, infer the IP address and port
get_ip_and_port_from_supervisor
fi
python3 /usr/local/go2rtc/create_config.py
else
echo "[WARNING] Unable to remove existing go2rtc config. Changes made to your frigate config file may not be recognized. Please remove the /dev/shm/go2rtc.yaml from your docker host manually."
fi
# HomeKit persistence. The helper is symlink-safe; hand off to go2rtc only when dropping. # HomeKit persistence. The helper is symlink-safe; hand off to go2rtc only when dropping.
readonly homekit_config_path="/config/go2rtc_homekit.yml" readonly homekit_config_path="/config/go2rtc_homekit.yml"
if [[ "$(id -u)" -eq 0 && "$runs_as_root" -eq 0 ]]; then if [[ "$(id -u)" -eq 0 && "$runs_as_root" -eq 0 ]]; then
python3 /usr/local/go2rtc/prepare_homekit.py "${homekit_config_path}" --chown python3 -I /usr/local/go2rtc/prepare_homekit.py "${homekit_config_path}" --chown
chown go2rtc:go2rtc /dev/shm/go2rtc.yaml 2>/dev/null || true chown -h go2rtc:go2rtc /dev/shm/go2rtc.yaml 2>/dev/null || true
else else
python3 /usr/local/go2rtc/prepare_homekit.py "${homekit_config_path}" python3 -I /usr/local/go2rtc/prepare_homekit.py "${homekit_config_path}"
fi fi
readonly config_path="/config" readonly config_path="/config"
@@ -124,7 +124,7 @@ fi
mkdir -p /etc/letsencrypt/www 2>/dev/null || true mkdir -p /etc/letsencrypt/www 2>/dev/null || true
# nginx settings are read once; both templates consume them # nginx settings are read once; both templates consume them
nginx_settings=$(python3 /usr/local/nginx/get_nginx_settings.py) nginx_settings=$(python3 -I /usr/local/nginx/get_nginx_settings.py)
# build templates for optional FRIGATE_BASE_PATH environment variable # build templates for optional FRIGATE_BASE_PATH environment variable
echo "$nginx_settings" | \ echo "$nginx_settings" | \
@@ -198,9 +198,13 @@ if config.get("birdseye", {}).get("restream", False):
else: else:
go2rtc_config["streams"] = {"birdseye": ffmpeg_cmd} go2rtc_config["streams"] = {"birdseye": ffmpeg_cmd}
# Write go2rtc_config to /dev/shm/go2rtc.yaml # The run script removes the old file first. Exclusive create never follows a
with open("/dev/shm/go2rtc.yaml", "w") as f: # link the go2rtc user planted in sticky /dev/shm. The config contains camera
# credentials, so it is not world-readable.
fd = os.open(
"/dev/shm/go2rtc.yaml",
os.O_WRONLY | os.O_CREAT | os.O_EXCL | os.O_NOFOLLOW,
0o640,
)
with os.fdopen(fd, "w") as f:
yaml.dump(go2rtc_config, f) yaml.dump(go2rtc_config, f)
# config contains camera credentials; do not leave it world-readable
os.chmod("/dev/shm/go2rtc.yaml", 0o640)
@@ -8,7 +8,7 @@ from typing import Any
from ruamel.yaml import YAML from ruamel.yaml import YAML
sys.path.insert(0, "/opt/frigate") sys.path.insert(0, "/opt/frigate")
from frigate.util.config import find_config_file from frigate.util.config import find_config_file, parse_listen_address
sys.path.remove("/opt/frigate") sys.path.remove("/opt/frigate")
@@ -35,20 +35,29 @@ ipv6_config: dict[str, Any] = networking_config.get("ipv6", {})
ipv6_config.setdefault("enabled", False) ipv6_config.setdefault("enabled", False)
listen_config: dict[str, Any] = networking_config.get("listen", {}) listen_config: dict[str, Any] = networking_config.get("listen", {})
listen_config.setdefault("internal", 5000)
listen_config.setdefault("external", 8971)
# handle case where internal port is a string with ip:port # Frigate's own validation runs elsewhere, and a bad value here must not stop nginx
internal_port = listen_config["internal"] # from serving the safe-mode UI. Valid values are kept so a fallback never widens
if type(internal_port) is str: # the unauthenticated internal listener beyond the configured address.
internal_port = int(internal_port.split(":")[-1]) for key, default in (("internal", 5000), ("external", 8971)):
listen_config["internal_port"] = internal_port try:
value, port = parse_listen_address(listen_config.get(key, default))
except ValueError:
print(
f"[WARN] Invalid networking.listen.{key}, using {default}",
file=sys.stderr,
)
value, port = default, default
# handle case where external port is a string with ip:port listen_config[key] = value
external_port = listen_config["external"] listen_config[f"{key}_port"] = port
if type(external_port) is str:
external_port = int(external_port.split(":")[-1]) if listen_config["internal_port"] == listen_config["external_port"]:
listen_config["external_port"] = external_port print(
"[ERROR] networking.listen.internal and external share a port",
file=sys.stderr,
)
sys.exit(1)
base_path = os.environ.get("FRIGATE_BASE_PATH", "") base_path = os.environ.get("FRIGATE_BASE_PATH", "")
+12 -11
View File
@@ -1,18 +1,12 @@
from pydantic import Field, model_validator from pydantic import Field, field_validator, model_validator
from frigate.util.config import parse_listen_address
from .base import FrigateBaseModel from .base import FrigateBaseModel
__all__ = ["IPv6Config", "ListenConfig", "NetworkingConfig"] __all__ = ["IPv6Config", "ListenConfig", "NetworkingConfig"]
def parse_listen_port(value: int | str) -> int:
"""Return the port number from a bare port or an "address:port" value."""
if isinstance(value, str):
return int(value.split(":")[-1])
return value
class IPv6Config(FrigateBaseModel): class IPv6Config(FrigateBaseModel):
enabled: bool = Field( enabled: bool = Field(
default=False, default=False,
@@ -33,13 +27,20 @@ class ListenConfig(FrigateBaseModel):
description="External listening port for Frigate (default 8971).", description="External listening port for Frigate (default 8971).",
) )
# Runs before pydantic coerces a bool to an int port, which
# get_nginx_settings.py rejects when it reads the raw YAML
@field_validator("internal", "external", mode="before")
@classmethod
def validate_listen(cls, value: int | str) -> int | str:
return parse_listen_address(value)[0]
@property @property
def internal_port(self) -> int: def internal_port(self) -> int:
return parse_listen_port(self.internal) return parse_listen_address(self.internal)[1]
@property @property
def external_port(self) -> int: def external_port(self) -> int:
return parse_listen_port(self.external) return parse_listen_address(self.external)[1]
@model_validator(mode="after") @model_validator(mode="after")
def validate_distinct_ports(self) -> "ListenConfig": def validate_distinct_ports(self) -> "ListenConfig":
+67
View File
@@ -36,6 +36,73 @@ class TestListenConfig(unittest.TestCase):
self.assertEqual(listen.internal_port, 5001) self.assertEqual(listen.internal_port, 5001)
self.assertEqual(listen.external_port, 8971) self.assertEqual(listen.external_port, 8971)
def test_valid_listen_values_are_rebuilt(self):
cases = {
5000: 5000,
"5000": 5000,
"127.0.0.1:5000": "127.0.0.1:5000",
"0.0.0.0:5000": "0.0.0.0:5000",
"[::1]:5000": "[::1]:5000",
"[::]:5000": "[::]:5000",
"[2001:DB8:0:0::1]:5000": "[2001:db8::1]:5000",
" 5000 ": 5000,
"5000\n": 5000,
" 127.0.0.1:5000 ": "127.0.0.1:5000",
"localhost:5000": "localhost:5000",
"frigate-host.lan:5000": "frigate-host.lan:5000",
"*:5000": "*:5000",
}
for value, expected in cases.items():
with self.subTest(value=value):
listen = ListenConfig(internal=value)
self.assertEqual(listen.internal, expected)
self.assertEqual(listen.internal_port, 5000)
def test_invalid_listen_values_rejected(self):
cases = [
"5000; location /x/ { alias /; autoindex on; } #:5000",
"127.0.0.1:5000; include /etc/passwd",
"5000 ssl",
"5000;",
"::1:5000",
"[127.0.0.1]:5000",
"[fe80::1%eth0]:5000",
"local host:5000",
"localhost;:5000",
"local{host}:5000",
"host#:5000",
"'localhost':5000",
'"localhost":5000',
"-host:5000",
"host-:5000",
"host..lan:5000",
"**:5000",
"*.lan:5000",
"unix:/tmp/sock:5000",
"5000 5001",
":5000",
"127.0.0.1:",
"127.0.0.1:+5000",
0,
65536,
-1,
]
for value in cases:
with self.subTest(value=value):
with self.assertRaises(ValidationError):
ListenConfig(internal=value)
def test_bool_listen_value_rejected(self):
# get_nginx_settings.py sees the raw bool and falls back to the default,
# so accepting it as port 1 would split nginx and the auth port check
for value in (True, False):
with self.subTest(value=value):
with self.assertRaises(ValidationError):
ListenConfig(internal=value)
if __name__ == "__main__": if __name__ == "__main__":
unittest.main(verbosity=2) unittest.main(verbosity=2)
+45
View File
@@ -1,8 +1,10 @@
"""configuration utils.""" """configuration utils."""
import asyncio import asyncio
import ipaddress
import logging import logging
import os import os
import re
import shutil import shutil
from functools import cache from functools import cache
from typing import Any from typing import Any
@@ -126,6 +128,49 @@ def redact_credential(obj: dict[str, Any], key: str) -> None:
obj.pop(key, None) obj.pop(key, None)
LISTEN_HOSTNAME = re.compile(
r"[A-Za-z0-9](?:[A-Za-z0-9-]{0,61}[A-Za-z0-9])?"
r"(?:\.[A-Za-z0-9](?:[A-Za-z0-9-]{0,61}[A-Za-z0-9])?)*"
)
def parse_listen_address(value: int | str) -> tuple[int | str, int]:
"""Validate a networking.listen value and rebuild it from its parsed parts.
Accepts a port, "host:port", "*:port", or "[IPv6]:port", where host is an
IPv4 address or an RFC 1123 hostname. nginx receives the rebuilt value
verbatim, so anything else raises ValueError.
Returns:
The normalized value (an int for a bare port) and the port
"""
if isinstance(value, bool) or not isinstance(value, int | str):
raise ValueError(f"invalid listen value: {value!r}")
host, sep, port_text = str(value).strip().rpartition(":")
if not re.fullmatch(r"[0-9]{1,5}", port_text) or not (1 <= int(port_text) <= 65535):
raise ValueError(f"invalid listen port: {value!r}")
port = int(port_text)
if not sep:
return port, port
if host.startswith("[") and host.endswith("]"):
address = ipaddress.IPv6Address(host[1:-1])
if address.scope_id:
raise ValueError(f"listen address must not have a scope ID: {value!r}")
return f"[{address}]:{port}", port
if host != "*" and not LISTEN_HOSTNAME.fullmatch(host):
raise ValueError(f"invalid listen address: {value!r}")
return f"{host}:{port}", port
def find_config_file() -> str: def find_config_file() -> str:
"""Return the path of the config file to use. """Return the path of the config file to use.