Fix /media/verify apostrophe false-positive #60
3 changed files with 35 additions and 5 deletions
|
|
@ -98,6 +98,10 @@ Integration Compose definition: `tests/compose.integration.yaml` (binds only to
|
||||||
|
|
||||||
## Changelog
|
## Changelog
|
||||||
|
|
||||||
|
### 2.4.8 — 17.09.2026
|
||||||
|
|
||||||
|
- Fixed `POST /media/verify` false-positive rejection: filenames containing a legitimate apostrophe (e.g. "La'An" in a Star Trek episode title) were blocked as "unsafe characters". Replaced the naive single-quote wrapping + apostrophe blocklist with proper `shlex.quote()` escaping for the remote ffprobe command; only newline/NUL byte injection is still rejected.
|
||||||
|
|
||||||
### 2.4.7 — 17.09.2026
|
### 2.4.7 — 17.09.2026
|
||||||
|
|
||||||
- Added `POST /media/verify`: probes a media file's real duration via `ffprobe` (running inside the existing `bazarrUHD` container on `arrapps`, which already mounts `/data` and ships ffmpeg — no new service needed) and flags it as `suspect` when it deviates from an `expected_minutes` value beyond `tolerance_pct`. Catches truncated Sonarr/Radarr imports (e.g. a re-grab that lands as 1.8 GB instead of the expected 8 GB for a UHD episode) after the file is already on the NAS.
|
- Added `POST /media/verify`: probes a media file's real duration via `ffprobe` (running inside the existing `bazarrUHD` container on `arrapps`, which already mounts `/data` and ships ffmpeg — no new service needed) and flags it as `suspect` when it deviates from an `expected_minutes` value beyond `tolerance_pct`. Catches truncated Sonarr/Radarr imports (e.g. a re-grab that lands as 1.8 GB instead of the expected 8 GB for a UHD episode) after the file is already on the NAS.
|
||||||
|
|
|
||||||
8
app.py
8
app.py
|
|
@ -1,7 +1,7 @@
|
||||||
"""Homelab Butler v2.1 – Unified API proxy for Pfannkuchen homelab.
|
"""Homelab Butler v2.1 – Unified API proxy for Pfannkuchen homelab.
|
||||||
Reads service config from butler.yaml, credentials from Vaultwarden cache with flat-file fallback."""
|
Reads service config from butler.yaml, credentials from Vaultwarden cache with flat-file fallback."""
|
||||||
|
|
||||||
import os, json, asyncio, logging, time, base64, re, subprocess, ipaddress, secrets, sqlite3, math, hashlib
|
import os, json, asyncio, logging, time, base64, re, subprocess, ipaddress, secrets, sqlite3, math, hashlib, shlex
|
||||||
from datetime import datetime, timezone
|
from datetime import datetime, timezone
|
||||||
import httpx, yaml
|
import httpx, yaml
|
||||||
from typing import Literal
|
from typing import Literal
|
||||||
|
|
@ -12,7 +12,7 @@ from contextlib import asynccontextmanager
|
||||||
from contextvars import ContextVar
|
from contextvars import ContextVar
|
||||||
|
|
||||||
log = logging.getLogger("butler")
|
log = logging.getLogger("butler")
|
||||||
VERSION = "2.4.7"
|
VERSION = "2.4.8"
|
||||||
|
|
||||||
API_DIR = os.environ.get("API_KEY_DIR", "/data/api")
|
API_DIR = os.environ.get("API_KEY_DIR", "/data/api")
|
||||||
VAULT_CACHE_DIR = os.environ.get("VAULT_CACHE_DIR", "/data/vault-cache")
|
VAULT_CACHE_DIR = os.environ.get("VAULT_CACHE_DIR", "/data/vault-cache")
|
||||||
|
|
@ -1577,7 +1577,7 @@ async def media_verify(payload: MediaVerifyRequest, _=Depends(_verify)):
|
||||||
raise HTTPException(400, f"No ffprobe container configured for host {payload.host}")
|
raise HTTPException(400, f"No ffprobe container configured for host {payload.host}")
|
||||||
if not re.fullmatch(r"/data/[^\x00]+\.(mkv|mp4|avi|m4v|ts)", payload.path):
|
if not re.fullmatch(r"/data/[^\x00]+\.(mkv|mp4|avi|m4v|ts)", payload.path):
|
||||||
raise HTTPException(400, "Path must be an absolute /data media file")
|
raise HTTPException(400, "Path must be an absolute /data media file")
|
||||||
if ".." in payload.path or "'" in payload.path:
|
if ".." in payload.path or "\n" in payload.path or "\x00" in payload.path:
|
||||||
raise HTTPException(400, "Path contains unsafe characters")
|
raise HTTPException(400, "Path contains unsafe characters")
|
||||||
|
|
||||||
container, _default_user = MEDIA_VERIFY_PROBES[payload.host]
|
container, _default_user = MEDIA_VERIFY_PROBES[payload.host]
|
||||||
|
|
@ -1590,7 +1590,7 @@ async def media_verify(payload: MediaVerifyRequest, _=Depends(_verify)):
|
||||||
|
|
||||||
probe_cmd = (
|
probe_cmd = (
|
||||||
f"docker exec {container} ffprobe -v error "
|
f"docker exec {container} ffprobe -v error "
|
||||||
f"-show_entries format=duration -of default=noprint_wrappers=1:nokey=1 '{payload.path}'"
|
f"-show_entries format=duration -of default=noprint_wrappers=1:nokey=1 {shlex.quote(payload.path)}"
|
||||||
)
|
)
|
||||||
rc, out, err = await asyncio.to_thread(
|
rc, out, err = await asyncio.to_thread(
|
||||||
_ssh, f'{target["user"]}@{target["ip"]}', f"sudo -n {probe_cmd} || {probe_cmd}", 30
|
_ssh, f'{target["user"]}@{target["ip"]}', f"sudo -n {probe_cmd} || {probe_cmd}", 30
|
||||||
|
|
|
||||||
|
|
@ -1,6 +1,7 @@
|
||||||
import os
|
import os
|
||||||
import asyncio
|
import asyncio
|
||||||
import json
|
import json
|
||||||
|
import shlex
|
||||||
import time
|
import time
|
||||||
from datetime import datetime, timedelta, timezone
|
from datetime import datetime, timedelta, timezone
|
||||||
|
|
||||||
|
|
@ -951,13 +952,38 @@ def test_media_verify_rejects_path_outside_data_before_ssh(monkeypatch):
|
||||||
assert response.status_code == 400
|
assert response.status_code == 400
|
||||||
|
|
||||||
|
|
||||||
|
def test_media_verify_allows_apostrophe_in_filename_and_quotes_it_safely(monkeypatch):
|
||||||
|
monkeypatch.setattr(app, "_find_inventory_host", lambda host: {"user": "sascha", "ip": "10.2.1.100"})
|
||||||
|
calls = []
|
||||||
|
|
||||||
|
def fake_ssh(host, command, timeout=30):
|
||||||
|
calls.append((host, command, timeout))
|
||||||
|
return 0, "2967.0\n", ""
|
||||||
|
|
||||||
|
monkeypatch.setattr(app, "_ssh", fake_ssh)
|
||||||
|
path = "/data/FHD/serien/Star Trek - Strange New Worlds (2022)/Season 04/Once La'An a Time.mkv"
|
||||||
|
with TestClient(app.app) as client:
|
||||||
|
response = client.post(
|
||||||
|
"/media/verify",
|
||||||
|
headers={"Authorization": "Bearer test-token"},
|
||||||
|
json={"host": "arrapps", "path": path, "expected_minutes": 49.5},
|
||||||
|
)
|
||||||
|
assert response.status_code == 200
|
||||||
|
# shlex.quote must produce a command the remote shell parses as ONE argument,
|
||||||
|
# i.e. no unescaped apostrophe breaks out of quoting.
|
||||||
|
executed_cmd = calls[0][1]
|
||||||
|
quoted = shlex.quote(path)
|
||||||
|
assert quoted in executed_cmd
|
||||||
|
assert shlex.split(executed_cmd)[-1] == path
|
||||||
|
|
||||||
|
|
||||||
def test_media_verify_rejects_shell_metacharacters_before_ssh(monkeypatch):
|
def test_media_verify_rejects_shell_metacharacters_before_ssh(monkeypatch):
|
||||||
monkeypatch.setattr(app, "_ssh", lambda *_args, **_kwargs: (_ for _ in ()).throw(AssertionError("must not SSH")))
|
monkeypatch.setattr(app, "_ssh", lambda *_args, **_kwargs: (_ for _ in ()).throw(AssertionError("must not SSH")))
|
||||||
with TestClient(app.app) as client:
|
with TestClient(app.app) as client:
|
||||||
response = client.post(
|
response = client.post(
|
||||||
"/media/verify",
|
"/media/verify",
|
||||||
headers={"Authorization": "Bearer test-token"},
|
headers={"Authorization": "Bearer test-token"},
|
||||||
json={"host": "arrapps", "path": "/data/UHD/serien/a'; rm -rf /'.mkv"},
|
json={"host": "arrapps", "path": "/data/UHD/serien/a\x00.mkv"},
|
||||||
)
|
)
|
||||||
assert response.status_code == 400
|
assert response.status_code == 400
|
||||||
|
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue