security: fix #2 — add pure mpv options builder helper
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
This commit is contained in:
parent
0aa3d8113d
commit
22744c48af
66
booru_viewer/gui/media/_mpv_options.py
Normal file
66
booru_viewer/gui/media/_mpv_options.py
Normal file
@ -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
|
||||
0
tests/gui/media/__init__.py
Normal file
0
tests/gui/media/__init__.py
Normal file
68
tests/gui/media/test_mpv_options.py
Normal file
68
tests/gui/media/test_mpv_options.py
Normal file
@ -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
|
||||
Loading…
x
Reference in New Issue
Block a user