mirror of
https://github.com/calibrain/shelfmark.git
synced 2026-11-10 12:28:27 +00:00
fix(docker): drop ffmpeg from the default image; install on demand in Debug mode (#1631)
ffmpeg is in the default image for one thing, the Debug mode screen recording (`_start_ffmpeg_recording`). Nothing else calls it, SeleniumBase included, and it brings 93 packages with it. On real builds that's 406 Trivy findings (149 High) and 78 MB of the gzipped amd64 image. This takes it out of the image and installs it when Debug mode needs it. It came out of the base image discussion on #1616 and helps on any base, though on Wolfi the install line would need `apk`. ## What changed - **Dockerfile:** ffmpeg is gone from the full image's install. - **Guard:** `_start_ffmpeg_recording` checks `shutil.which("ffmpeg")` first. If it's missing it logs a warning and returns, and the bypass carries on. Without this, Debug on with no ffmpeg fails every bypass with `FileNotFoundError`. There's a new test for it. - **Entrypoint:** when Debug is on, the internal bypasser is in use and ffmpeg isn't already there, it installs ffmpeg in the background so the app doesn't wait for it. The download and the install each run under a 120 s `timeout` with `DEBIAN_FRONTEND=noninteractive`. Downloading first means a hung mirror can only be killed before dpkg starts, and the next start runs `dpkg --configure -a` in case a restart cut dpkg off. It logs when it starts, succeeds, times out or fails. - **Docs:** a short paragraph in `docs/troubleshooting.md` covering the install and the non-root limit. ## Debug has two sources Debug can come from the `DEBUG` env var or from the Debug Mode setting in the UI, which is saved to `/config/plugins/advanced.json`. The existing block in `entrypoint.sh` only reads the env var, so an install keyed on `$DEBUG` would miss anyone who turned it on in the UI. The new check asks the app instead (`shelfmark.config.env.DEBUG`), which reads both. The UI setting is `requires_restart`, so the restart it already asks for is what brings it through the entrypoint. ## Non-root mode A container started as `1000:1000` can't run `apt-get`, so the entrypoint logs that it can't install ffmpeg, the guard warns on each bypass, and there's no recording. Everything else works. The workaround for a debugging session is to start as root with `PUID`/`PGID`. ## Startup cost None that I could measure. With Debug on, `/api/health` answers in the same time as main (3-4 s with Debug only in config, 9-10 s from env), and ffmpeg was ready 6-8 s later on the runners. The trade-off is that the first bypass after startup may be unrecorded while the install runs. It logs the warning and passes. I saw both outcomes on a bypass started right after health. From reading the code: the install lives in the container, so recreating the container installs it again. It also runs after `tor.sh` and `wireguard.sh`, so apt goes through whichever egress is active. ## Testing Real Docker builds on GitHub Actions on my fork, amd64 and arm64, with the same result on both for every case below. | Case | Result | |---|---| | Debug off | No install, `/api/health` in 3-4 s, same as main | | Debug only in config | Health in 3-4 s, ffmpeg ready 6-8 s later, real mp4 after a bypass | | Debug from env | Health in 9-10 s (same as main), real mp4 after a bypass | | Started as root, app dropped to uid 1000 | Smoke and bypass pass | | Started with `--user 1000`, Debug on | Logs both messages, bypass passes, no mp4 | | apt blocked (closed proxy) | Logs `Could not install ffmpeg`, bypass passes, no mp4 | | apt's proxy on a dropped address, so no TCP reply at all | Health in 9-10 s, a bypass during the install passes with the warning and no mp4. apt gives up by itself in under a minute. Nothing left running, `dpkg --audit` clean | | DNS server on a dropped address | Health in 9-10 s, `Timed out installing ffmpeg after 120s`. Nothing left running, `dpkg --audit` clean | | `--network none`, Debug on | No hang, health 200 from inside the container | | Container restarted while dpkg was running | Next start finishes the install, `dpkg --audit` clean, real mp4 | | Fresh named volume, Debug only in config | Recording made, uid 1000 can write | | Existing uid-1000 volume, Debug only in config | Same, nothing root-owned afterwards | | `find /config /var/log/shelfmark -user root` | 0 files, same as main | | `find /app -user root` | 245, same count as main | | `tests/e2e/platform/run-e2e.sh` with `dns-blocked` and `baseline` | Pass | The bypass checks ran against example.com. A real Cloudflare challenge was served by a demo page in the same runs, and no build solved it, including main, so that part doesn't say anything about this change. Locally: 5494 Python tests pass, and `ruff`, `basedpyright`, `vulture` and `shellcheck` are clean. The full results are in [the base image report](https://github.com/splitsec2/shelfmark/blob/7afcecfb76727724a71561cbc078f091569b58dc/docs/research/2026-11-01-base-image-report.md).
This commit is contained in:
1 parent
34c9e6dbd7
commit
c6685feba5
6 files changed
+178
-2
No files matched your search
@@ -158,8 +158,6 @@ RUN echo "deb [check-valid-until=no] https://snapshot.debian.org/archive/debian-
|
||||
apt-get install -y --no-install-recommends -o Acquire::Retries=5 \
|
||||
# For dumb display
|
||||
xvfb \
|
||||
# For screen recording
|
||||
ffmpeg \
|
||||
chromium=${CHROMIUM_VERSION} \
|
||||
chromium-common=${CHROMIUM_VERSION} \
|
||||
# For tkinter (pyautogui)
|
||||
|
||||
@@ -12,6 +12,8 @@ Shelfmark writes its log to the container output, so `docker logs shelfmark` sho
|
||||
|
||||
Set `DEBUG=true` on the container and restart it. A **DEBUG** button then appears in the interface. It generates a zip with logs, configuration and environment details, with sensitive values redacted. Have a look through it before you post it publicly, then attach it when you open an issue.
|
||||
|
||||
The full image also records the browser's screen during a bypass while Debug mode is on. ffmpeg isn't in the image, so the container installs it in the background at startup, which needs network access. Bypasses that run before it finishes aren't recorded. The log says when the install succeeds, fails or gives up after 120 seconds. In non-root mode it can't install anything, so the log warns that bypasses won't be recorded and everything else works as normal.
|
||||
|
||||
## Reporting an issue
|
||||
|
||||
Open an issue on [GitHub](https://github.com/calibrain/shelfmark/issues) and fill in the bug report template. A description of what you expected, the steps to reproduce it and the debug bundle are what make a problem findable.
|
||||
@@ -498,6 +498,50 @@ else
|
||||
fi
|
||||
command="${GUNICORN_BIN} --log-level ${gunicorn_loglevel} --access-logfile - --error-logfile - --worker-class geventwebsocket.gunicorn.workers.GeventWebSocketWorker --workers 1 -t 300 -b ${FLASK_HOST:-0.0.0.0}:${FLASK_PORT:-8084} shelfmark.main:app"
|
||||
|
||||
# Debug mode's screen recording needs ffmpeg, which the image leaves out because nothing
|
||||
# else uses it. This runs in the background so a slow or silent mirror can't hold up
|
||||
# startup. A mirror can only hang the download, and killing that leaves nothing half
|
||||
# installed. dpkg then works from what's on disk, and the next start's --configure -a
|
||||
# clears up if a restart or the time limit cuts it off.
|
||||
install_ffmpeg() {
|
||||
local limit="${FFMPEG_INSTALL_TIMEOUT:-120}" status=0
|
||||
export DEBIAN_FRONTEND=noninteractive
|
||||
umask 0022
|
||||
timeout -k 10 "$limit" dpkg --configure -a || status=$?
|
||||
if [ "$status" = 0 ]; then
|
||||
# No && after update: a partial index failure should not rule out the install.
|
||||
timeout -k 10 "$limit" sh -c 'apt-get update -o Acquire::Retries=3
|
||||
apt-get install -y --no-install-recommends --download-only ffmpeg' || status=$?
|
||||
fi
|
||||
if [ "$status" = 0 ]; then
|
||||
timeout -k 10 "$limit" apt-get install -y --no-install-recommends --no-download ffmpeg || status=$?
|
||||
fi
|
||||
case "$status" in
|
||||
0)
|
||||
if ffmpeg -version >/dev/null 2>&1; then
|
||||
echo "ffmpeg installed; bypasses will be recorded from now on."
|
||||
else
|
||||
echo "Could not install ffmpeg (apt finished but ffmpeg does not run); bypasses will not be recorded."
|
||||
fi
|
||||
;;
|
||||
124 | 137) echo "Timed out installing ffmpeg after ${limit}s; bypasses will not be recorded." ;;
|
||||
*) echo "Could not install ffmpeg (exit $status); bypasses will not be recorded." ;;
|
||||
esac
|
||||
}
|
||||
|
||||
# Ask the app rather than reading $DEBUG: the Debug Mode setting lives in the config file
|
||||
# and needs a restart, so this is where it takes effect.
|
||||
INSTALL_FFMPEG="false"
|
||||
if [ "$USING_EXTERNAL_BYPASSER" != "true" ] && ! ffmpeg -version >/dev/null 2>&1 &&
|
||||
"$PYTHON_BIN" -c 'from shelfmark.config.env import DEBUG; raise SystemExit(0 if DEBUG else 1)' 2>/dev/null; then
|
||||
if [ "$RUN_AS_NON_ROOT" = "true" ]; then
|
||||
echo "Debug mode is on but ffmpeg is not installed and non-root mode cannot install it; bypasses will not be recorded."
|
||||
else
|
||||
echo "Debug mode is on: installing ffmpeg in the background; bypasses before it finishes will not be recorded."
|
||||
INSTALL_FFMPEG="true"
|
||||
fi
|
||||
fi
|
||||
|
||||
# If DEBUG and not using an external bypass
|
||||
if [ "$DEBUG" = "true" ] && [ "$USING_EXTERNAL_BYPASSER" != "true" ]; then
|
||||
set +e
|
||||
@@ -589,4 +633,13 @@ echo "Setting umask to $UMASK_VALUE"
|
||||
umask $UMASK_VALUE
|
||||
|
||||
stop_file_logging
|
||||
# Started only now because stop_file_logging waits for the log pipe to close, which a
|
||||
# background job holding it would delay. The outer subshell leaves it to dumb-init to reap.
|
||||
if [ "$INSTALL_FFMPEG" = "true" ]; then
|
||||
if [ "$FILE_LOGGING_ENABLED" = "true" ]; then
|
||||
( (install_ffmpeg 2>&1 | tee -a "$LOG_FILE") </dev/null & )
|
||||
else
|
||||
( (install_ffmpeg 2>&1) </dev/null & )
|
||||
fi
|
||||
fi
|
||||
exec_as_target_user env HOME="$RUNTIME_HOME" $command
|
||||
@@ -5,6 +5,7 @@ import asyncio
|
||||
import json
|
||||
import os
|
||||
import random
|
||||
import shutil
|
||||
import signal
|
||||
import socket
|
||||
import stat
|
||||
@@ -1499,6 +1500,11 @@ async def _close_websocket_connection(conn: Any) -> None:
|
||||
def _start_ffmpeg_recording(display: str) -> None:
|
||||
"""Start FFmpeg screen recording for debug mode."""
|
||||
global DISPLAY
|
||||
# Debug mode can be on when ffmpeg is not installed. A recording is a diagnostic, so
|
||||
# going without one must not fail the bypass it was meant to help diagnose.
|
||||
if shutil.which("ffmpeg") is None:
|
||||
logger.warning("ffmpeg is not installed; no recording for this bypass.")
|
||||
return
|
||||
RECORDING_DIR.mkdir(parents=True, exist_ok=True)
|
||||
timestamp = datetime.now(UTC).strftime("%y%m%d-%H%M%S")
|
||||
output_file = RECORDING_DIR / f"screen_recording_{timestamp}.mp4"
|
||||
|
||||
@@ -24,6 +24,12 @@ def _clean_display():
|
||||
ib.DISPLAY.update(before)
|
||||
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
def _ffmpeg_on_path(monkeypatch):
|
||||
# The recorder checks for the binary first; these tests stub the process itself.
|
||||
monkeypatch.setattr(ib.shutil, "which", lambda _name: "/usr/bin/ffmpeg")
|
||||
|
||||
|
||||
class _Proc:
|
||||
def __init__(self, returncode):
|
||||
self.returncode = returncode
|
||||
@@ -133,3 +139,23 @@ def test_summary_survives_a_missing_log():
|
||||
ib.DISPLAY["ffmpeg_error_log"] = None
|
||||
|
||||
assert "No FFmpeg error log" in ib._ffmpeg_error_summary()
|
||||
|
||||
|
||||
def test_debug_mode_without_ffmpeg_skips_the_recording_and_says_so(monkeypatch, tmp_path):
|
||||
"""Debug mode can be on with no ffmpeg in the image; that must not fail the bypass."""
|
||||
monkeypatch.setattr(ib, "RECORDING_DIR", tmp_path / "recording")
|
||||
monkeypatch.setattr(ib.shutil, "which", lambda _name: None)
|
||||
|
||||
def popen_must_not_run(*_args, **_kwargs):
|
||||
raise AssertionError("ffmpeg should not be started when it is not installed")
|
||||
|
||||
monkeypatch.setattr(ib.subprocess, "Popen", popen_must_not_run)
|
||||
|
||||
warnings: list[str] = []
|
||||
monkeypatch.setattr(ib.logger, "warning", lambda msg, *_args: warnings.append(msg))
|
||||
|
||||
ib._start_ffmpeg_recording(display=":99")
|
||||
|
||||
assert ib.DISPLAY["ffmpeg"] is None
|
||||
assert any("ffmpeg is not installed" in message for message in warnings)
|
||||
assert not (tmp_path / "recording").exists()
|
||||
@@ -0,0 +1,91 @@
|
||||
"""The Debug mode ffmpeg install: what it runs, what it logs, and what it can't block."""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import os
|
||||
import shutil
|
||||
import subprocess
|
||||
from pathlib import Path
|
||||
|
||||
ENTRYPOINT_PATH = Path(__file__).resolve().parents[2] / "entrypoint.sh"
|
||||
BASH_PATH = shutil.which("bash") or "/bin/bash"
|
||||
|
||||
|
||||
def _stub(bin_dir: Path, name: str, body: str) -> None:
|
||||
path = bin_dir / name
|
||||
path.write_text(f"#!/bin/sh\n{body}\n")
|
||||
path.chmod(0o755)
|
||||
|
||||
|
||||
def _install_ffmpeg_source() -> str:
|
||||
text = ENTRYPOINT_PATH.read_text()
|
||||
start = text.index("install_ffmpeg() {")
|
||||
return text[start : text.index("\n}\n", start) + 3]
|
||||
|
||||
|
||||
def _run(
|
||||
tmp_path: Path, *, apt_body: str, ffmpeg_works: bool = True
|
||||
) -> tuple[subprocess.CompletedProcess[str], list[str]]:
|
||||
bin_dir = tmp_path / "bin"
|
||||
bin_dir.mkdir()
|
||||
calls = tmp_path / "apt-calls.txt"
|
||||
_stub(
|
||||
bin_dir,
|
||||
"apt-get",
|
||||
f'echo "frontend=$DEBIAN_FRONTEND $*" >> "{calls}"\n{apt_body}',
|
||||
)
|
||||
_stub(bin_dir, "dpkg", "exit 0")
|
||||
_stub(bin_dir, "ffmpeg", "exit 0" if ffmpeg_works else "exit 1")
|
||||
|
||||
env = os.environ.copy()
|
||||
env.update({"PATH": f"{bin_dir}:{env.get('PATH', '')}", "FFMPEG_INSTALL_TIMEOUT": "1"})
|
||||
result = subprocess.run(
|
||||
[BASH_PATH, "-c", f"{_install_ffmpeg_source()}\ninstall_ffmpeg"],
|
||||
capture_output=True,
|
||||
env=env,
|
||||
text=True,
|
||||
check=False,
|
||||
timeout=30,
|
||||
)
|
||||
return result, calls.read_text().splitlines() if calls.exists() else []
|
||||
|
||||
|
||||
def test_install_downloads_first_then_installs_without_the_network(tmp_path):
|
||||
result, calls = _run(tmp_path, apt_body="exit 0")
|
||||
|
||||
assert "ffmpeg installed" in result.stdout
|
||||
assert [call.split()[1] for call in calls] == ["update", "install", "install"]
|
||||
assert "--download-only" in calls[1]
|
||||
assert "--no-download" in calls[2]
|
||||
assert all(call.startswith("frontend=noninteractive ") for call in calls)
|
||||
|
||||
|
||||
def test_install_gives_up_on_a_mirror_that_never_answers(tmp_path):
|
||||
result, calls = _run(tmp_path, apt_body="exec sleep 60")
|
||||
|
||||
assert "Timed out installing ffmpeg after 1s" in result.stdout
|
||||
assert "ffmpeg installed" not in result.stdout
|
||||
# Killed during the download, so dpkg's install step never ran.
|
||||
assert not any("--no-download" in call for call in calls)
|
||||
|
||||
|
||||
def test_install_reports_a_failure(tmp_path):
|
||||
result, calls = _run(tmp_path, apt_body="exit 100")
|
||||
|
||||
assert "Could not install ffmpeg (exit 100)" in result.stdout
|
||||
assert not any("--no-download" in call for call in calls)
|
||||
|
||||
|
||||
def test_install_does_not_claim_success_when_ffmpeg_does_not_run(tmp_path):
|
||||
result, _ = _run(tmp_path, apt_body="exit 0", ffmpeg_works=False)
|
||||
|
||||
assert "ffmpeg does not run" in result.stdout
|
||||
assert "ffmpeg installed" not in result.stdout
|
||||
|
||||
|
||||
def test_install_starts_after_the_log_pipe_is_released():
|
||||
"""stop_file_logging waits for the log pipe to close, so the job must not hold it."""
|
||||
text = ENTRYPOINT_PATH.read_text()
|
||||
|
||||
assert text.rindex("stop_file_logging\n") < text.index('if [ "$INSTALL_FFMPEG" = "true" ]')
|
||||
assert text.index('if [ "$INSTALL_FFMPEG" = "true" ]') < text.rindex("exec_as_target_user env")
|
||||
Reference in new issue
Block a user