From 22744c48afee84842a076e35fa979b249ca2d29c Mon Sep 17 00:00:00 2001 From: pax Date: Sat, 11 Apr 2026 16:06:33 -0500 Subject: [PATCH] =?UTF-8?q?security:=20fix=20#2=20=E2=80=94=20add=20pure?= =?UTF-8?q?=20mpv=20options=20builder=20helper?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Extracts the mpv.MPV() kwargs into a Qt-free pure function so the security-relevant options can be unit-tested on CI (which lacks PySide6 and libmpv). The builder embeds the audit #2 hardening — ytdl="no", load_scripts="no", and a lavf protocol whitelist of file,http,https,tls,tcp — alongside the existing playback tuning. Not yet wired into _MpvGLWidget; that lands in the next commit. Audit-Ref: SECURITY_AUDIT.md finding #2 Severity: High --- booru_viewer/gui/media/_mpv_options.py | 66 +++++++++++++++++++++++++ tests/gui/media/__init__.py | 0 tests/gui/media/test_mpv_options.py | 68 ++++++++++++++++++++++++++ 3 files changed, 134 insertions(+) create mode 100644 booru_viewer/gui/media/_mpv_options.py create mode 100644 tests/gui/media/__init__.py create mode 100644 tests/gui/media/test_mpv_options.py diff --git a/booru_viewer/gui/media/_mpv_options.py b/booru_viewer/gui/media/_mpv_options.py new file mode 100644 index 0000000..b4ded47 --- /dev/null +++ b/booru_viewer/gui/media/_mpv_options.py @@ -0,0 +1,66 @@ +"""Pure helper that builds the kwargs dict passed to ``mpv.MPV``. + +Kept free of any Qt or mpv imports so the options dict can be audited +from a CI test that only installs the stdlib. +""" + +from __future__ import annotations + + +def build_mpv_kwargs(is_windows: bool) -> dict[str, object]: + """Return the kwargs dict for constructing ``mpv.MPV``. + + The playback, audio, and network options are unchanged from + pre-audit v0.2.5. The security hardening added by SECURITY_AUDIT.md + finding #2 is: + + - ``ytdl="no"``: refuse to delegate URL handling to yt-dlp. mpv's + default enables a yt-dlp hook script that matches ~1500 hosts + and shells out to ``yt-dlp`` on any URL it recognizes. A + compromised booru returning ``file_url: "https://youtube.com/..."`` + would pull the user through whatever extractor CVE is current. + + - ``load_scripts="no"``: do not auto-load Lua scripts from + ``~/.config/mpv/scripts``. These scripts run in mpv's context + every time the widget is created. + + - ``demuxer_lavf_o="protocol_whitelist=file,http,https,tls,tcp"``: + restrict ffmpeg's lavf demuxer to HTTP(S) and local file reads. + The default accepts ``concat:``, ``subfile:``, ``data:``, + ``udp://``, etc. — ``concat:/etc/passwd|/dev/zero`` is the + canonical local-file-read gadget when a hostile container is + piped through lavf. ``file`` must stay in the whitelist because + cached local files (``.part``, promoted cache paths) rely on it. + ``crypto`` is intentionally omitted; it's an FFmpeg pseudo- + protocol for AES-decrypted streams that boorus do not serve. + + - ``input_conf="/dev/null"`` (POSIX only): skip loading + ``~/.config/mpv/input.conf``. The existing + ``input_default_bindings=False`` + ``input_vo_keyboard=False`` + are the primary lockdown; this is defense-in-depth. Windows + uses a different null-device path and the load behavior varies + by mpv build, so it is skipped there. + """ + kwargs: dict[str, object] = { + "vo": "libmpv", + "hwdec": "auto", + "keep_open": "yes", + "ao": "pulse,wasapi,", + "audio_client_name": "booru-viewer", + "input_default_bindings": False, + "input_vo_keyboard": False, + "osc": False, + "vd_lavc_fast": "yes", + "vd_lavc_skiploopfilter": "nonkey", + "cache": "yes", + "cache_pause": "no", + "demuxer_max_bytes": "50MiB", + "demuxer_readahead_secs": "20", + "network_timeout": "10", + "ytdl": "no", + "load_scripts": "no", + "demuxer_lavf_o": "protocol_whitelist=file,http,https,tls,tcp", + } + if not is_windows: + kwargs["input_conf"] = "/dev/null" + return kwargs diff --git a/tests/gui/media/__init__.py b/tests/gui/media/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/tests/gui/media/test_mpv_options.py b/tests/gui/media/test_mpv_options.py new file mode 100644 index 0000000..0abc278 --- /dev/null +++ b/tests/gui/media/test_mpv_options.py @@ -0,0 +1,68 @@ +"""Tests for the pure mpv kwargs builder. + +Pure Python. No Qt, no mpv, no network. The helper is importable +from the CI environment that installs only httpx + Pillow + pytest. +""" + +from __future__ import annotations + +from booru_viewer.gui.media._mpv_options import build_mpv_kwargs + + +def test_ytdl_disabled(): + """Finding #2 — mpv must not delegate URLs to yt-dlp.""" + kwargs = build_mpv_kwargs(is_windows=False) + assert kwargs["ytdl"] == "no" + + +def test_load_scripts_disabled(): + """Finding #2 — no auto-loading of ~/.config/mpv/scripts.""" + kwargs = build_mpv_kwargs(is_windows=False) + assert kwargs["load_scripts"] == "no" + + +def test_protocol_whitelist_restricts_to_file_and_http(): + """Finding #2 — lavf demuxer must only accept file + HTTP(S) + TLS/TCP.""" + kwargs = build_mpv_kwargs(is_windows=False) + value = kwargs["demuxer_lavf_o"] + assert isinstance(value, str) + assert value.startswith("protocol_whitelist=") + allowed = set(value.split("=", 1)[1].split(",")) + # `file` must be present — cached local clips and .part files use it. + assert "file" in allowed + # HTTP(S) + supporting protocols for network videos. + assert "http" in allowed + assert "https" in allowed + assert "tls" in allowed + assert "tcp" in allowed + # Dangerous protocols must NOT appear. + for banned in ("concat", "subfile", "data", "udp", "rtp", "crypto"): + assert banned not in allowed + + +def test_input_conf_nulled_on_posix(): + """Finding #2 — on POSIX, skip loading ~/.config/mpv/input.conf.""" + kwargs = build_mpv_kwargs(is_windows=False) + assert kwargs["input_conf"] == "/dev/null" + + +def test_input_conf_skipped_on_windows(): + """Finding #2 — input_conf gate is POSIX-only; Windows omits the key.""" + kwargs = build_mpv_kwargs(is_windows=True) + assert "input_conf" not in kwargs + + +def test_existing_options_preserved(): + """Regression: pre-audit playback/audio tuning must remain.""" + kwargs = build_mpv_kwargs(is_windows=False) + # Discord screen-share audio fix (see mpv_gl.py comment). + assert kwargs["ao"] == "pulse,wasapi," + assert kwargs["audio_client_name"] == "booru-viewer" + # Network tuning from the uncached-video fast path. + assert kwargs["cache"] == "yes" + assert kwargs["cache_pause"] == "no" + assert kwargs["demuxer_max_bytes"] == "50MiB" + assert kwargs["network_timeout"] == "10" + # Existing input lockdown (primary — input_conf is defense-in-depth). + assert kwargs["input_default_bindings"] is False + assert kwargs["input_vo_keyboard"] is False