Never let root run a file the bot can write

deploy/install-units.sh

@@ -16,6 +16,7 @@ APP_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)"
 VENV_DIR="$APP_DIR/.venv"
 SERVICE_NAME="loopify-bot"
 UPDATER_NAME="loopify-ytdlp-update"
+UPDATER_BIN="/usr/local/sbin/$UPDATER_NAME"
 # Whoever owns the checkout is who the bot runs as.
 APP_USER="$(stat -c '%U' "$APP_DIR")"
 
@@ -78,7 +79,10 @@ NoNewPrivileges=true
 PrivateTmp=true
 ProtectSystem=strict
 ProtectHome=read-only
-ReadWritePaths=$APP_DIR
+# Only the caches, and the cookies yt-dlp writes back. Never the code: the bot
+# parses untrusted media, and a bot that can rewrite main.py keeps whatever got
+# in across restarts.
+ReadWritePaths=$APP_DIR/.cache -$APP_DIR/cookies.txt
 ProtectKernelTunables=true
 ProtectKernelModules=true
 ProtectControlGroups=true
@@ -97,9 +101,16 @@ TasksMax=256
 WantedBy=multi-user.target
 UNIT
 
+# Owned by $APP_USER even when this runs as root: the bot writes nowhere else.
+sudo install -d -o "$APP_USER" -g "$APP_USER" -m 0755 "$APP_DIR/.cache"
+
 # yt-dlp is the one dependency that must NOT be pinned: YouTube changes its
 # player constantly and a stale build stops resolving videos within weeks.
-chmod +x "$APP_DIR/deploy/update-ytdlp.sh"
+#
+# The updater runs as root, so root runs a copy root owns. Run from the
+# checkout, it was a file $APP_USER can write, and so one step from anything
+# that ever runs as $APP_USER.
+sudo install -o root -g root -m 0755 "$APP_DIR/deploy/update-ytdlp.sh" "$UPDATER_BIN"
 
 sudo tee "/etc/systemd/system/${UPDATER_NAME}.service" >/dev/null <<UNIT
 [Unit]
@@ -110,7 +121,22 @@ Wants=network-online.target
 [Service]
 Type=oneshot
 # Runs as root to restart the service; drops to $APP_USER for the pip install.
-ExecStart=$APP_DIR/deploy/update-ytdlp.sh
+ExecStart=$UPDATER_BIN $APP_DIR
+# pip runs as $APP_USER, whose home is read-only here; without this it warns
+# on every run that its cache is unusable.
+Environment=PIP_NO_CACHE_DIR=1
+NoNewPrivileges=true
+PrivateTmp=true
+ProtectSystem=strict
+ProtectHome=read-only
+ReadWritePaths=$VENV_DIR
+ProtectKernelTunables=true
+ProtectKernelModules=true
+ProtectControlGroups=true
+RestrictSUIDSGID=true
+RestrictNamespaces=true
+LockPersonality=true
+RestrictAddressFamilies=AF_INET AF_INET6 AF_UNIX
 UNIT
 
 sudo tee "/etc/systemd/system/${UPDATER_NAME}.timer" >/dev/null <<UNIT

deploy/update-ytdlp.sh

@@ -8,11 +8,14 @@
 # else in the venv is upgraded here — the rest is pinned on purpose.
 #
 # Runs as root so it can restart the service, but drops to the app user for the
-# pip install so the venv does not end up owned by root.
+# pip install so the venv does not end up owned by root. install-units.sh
+# installs it as a root-owned copy and passes the checkout as the argument.
+#
+# Usage: loopify-ytdlp-update APP_DIR
 #
 set -euo pipefail
 
-APP_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)"
+APP_DIR="${1:?usage: $0 APP_DIR}"
 VENV_DIR="$APP_DIR/.venv"
 SERVICE_NAME="loopify-bot"
 # The venv's owner is the user the bot runs as.

tests/test_service_sandbox.py

@@ -74,3 +74,26 @@ def test_only_the_dns_resolvers_are_let_back_in():
     allows = [line for line in bot_unit().splitlines() if line.startswith("IPAddressAllow=")]
     assert allows == ["IPAddressAllow=$RESOLVERS"]
     assert re.search(r'^RESOLVERS="\$\(awk .*/etc/resolv\.conf', script(), re.MULTILINE)
+
+
+def updater_unit() -> str:
+    blocks = re.findall(r"\$\{UPDATER_NAME\}\.service\" >/dev/null <<UNIT\n(.*?)\nUNIT",
+                        script(), re.DOTALL)
+    assert len(blocks) == 1, "the loopify-ytdlp-update unit was not found"
+    return blocks[0]
+
+
+def test_the_bot_cannot_write_its_own_code():
+    """A compromised bot that can rewrite the checkout survives restarts."""
+    writable = [line for line in bot_unit().splitlines() if line.startswith("ReadWritePaths=")]
+    assert writable == ["ReadWritePaths=$APP_DIR/.cache -$APP_DIR/cookies.txt"]
+
+
+def test_root_never_runs_a_file_from_the_checkout():
+    """The updater runs as root; anything it executes must be root-owned."""
+    unit = updater_unit()
+    assert "User=" not in unit, "the updater is expected to run as root"
+    assert "ExecStart=$UPDATER_BIN $APP_DIR" in unit.splitlines()
+    assert re.search(r'^UPDATER_BIN="/usr/local/sbin/', script(), re.MULTILINE)
+    assert re.search(r'sudo install -o root -g root -m 0755 .*update-ytdlp\.sh" "\$UPDATER_BIN"',
+                     script())