From ef0a574a703310ac7d610624da384c66267caf16 Mon Sep 17 00:00:00 2001 From: Hitonabi Date: Sat, 25 Jul 2026 21:05:11 +0200 Subject: [PATCH] fix(worker,api): Platten-Schutz griff nicht, Fortschritt log, Auswurf tat nichts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Vier Funde aus der Durchsicht, alle auf der VM gemessen. 1. DER PLATTEN-SCHUTZ AUS c065967 WAR WIRKUNGSLOS _original_aufheben() entschied per os.stat().st_dev, ob umgehaengt oder kopiert werden muss. Im Worker-Container gemessen - beides gleichzeitig wahr: st_dev /app/temp = 2050 st_dev /app/media = 2050 → identisch os.rename(...) → EXDEV, "Invalid cross-device link" Der Kernel vergleicht bei rename() den MOUNT, nicht das Geraet. /app/temp (Docker-Volume) und /app/media (Bind-Mount) sind zwei Mounts DERSELBEN ext4-Partition. Die Pruefung sah "gleiches Dateisystem", uebersprang die Platzpruefung, und shutil.move kopierte doch - 75 GB bei 37 GB frei. Der Schutz haette genau den Schaden zugelassen, gegen den er gebaut wurde. Jetzt wird os.rename VERSUCHT statt vorhergesagt: klappt es, ist es umgehaengt und fertig; kommt EXDEV, steht die Kopie fest und ERST DANN wird der Platz geprueft. Das ist keine Vermutung mehr, sondern die Antwort des Kernels. Vier Tests in test_original_aufheben.py, darunter genau der Fall, der die Platte fuellte. Die zwei alten Tests in test_medien.py sind dorthin gewandert - sie taeuschten per gefaelschtem os.stat "verschiedene Dateisysteme" vor, also genau die Annahme, an der der Schutz scheiterte. 2. DIE FORTSCHRITTSANZEIGE ZEIGTE DEN SCAN, NICHT DEN ENCODE get_progress_from_line matchte jede Zahl vor einem Prozentzeichen. HandBrake gibt Prozente aber in drei Phasen aus (Formatstrings aus dem Binary gelesen): Scanning title %d of %d, preview %d, %.2f %% → laeuft VOR dem Encode bis 100 % Encoding: task %d of %d, %.2f %% (%.2f fps, avg → der echte Wert Encoding: task %d of %d, Searching for start time, ... → Vorlauf Dazu warf `if progress > 0` im Aufrufer jeden Wert unter 1,00 % weg. Live beobachtet: Anzeige stand auf 99 %, der Encode bei 1,06 %; sie fiel erst auf 1, als der Encode die 1-%-Marke ueberschritt. Jetzt wird nur die Encoding-Zeile gelesen, `task N of M` mitgerechnet (sonst springt die Anzeige bei Zwei-Pass-Presets mitten in der Datei zurueck), und -1 heisst "keine Angabe" - dasselbe Muster wie bei get_progress_from_prgv. 3. "AUTOMATISCHER AUSWURF" WURDE VON NIEMANDEM GELESEN Die Einstellung (Standard: ein, "Disc nach erfolgreichem Ripping automatisch auswerfen") kam in keiner Zeile Backend-Code vor. DVD/Blu-ray warfen deshalb NIE aus, Audio-CDs IMMER, weil abcde `-x` fest verdrahtet bekam. Jetzt entscheidet die Einstellung beides: wirf_disc_aus() per CDROMEJECT-ioctl (fcntl-guarded, der native Windows-Worker laedt das Modul auch) und `-x` nur noch, wenn gewuenscht. 4. PFAD-PRUEFUNG FIEL AUF PRAEFIX-NAMEN HEREIN Elf Stellen prueften mit nacktem startswith(MEDIA_ROOT). "/app/media-boese/x" beginnt mit "/app/media", liegt aber ausserhalb - betroffen waren auch /browse und /browse/mkdir, wo der Pfad vom Nutzer kommt. Neuer Zwillings-Helfer unter_wurzel() in api/main.py und worker/tasks.py, alle elf Stellen umgestellt, Tests in beiden. Nebenbefund: _zielbasis() benutzte os.path.normpath - unter Windows werden daraus Backslashes, die MEDIA_ROOT-Pruefung greift nicht mehr, und das gewaehlte Ziel faellt still auf den Standard zurueck. Genau die Falle, die _arbeitsverzeichnis() drei Zeilen weiter dokumentiert und mit posixpath vermeidet. Live war es nie (nur aus rip_disc, das auf Windows verriegelt ist), jetzt konsistent. Co-Authored-By: Claude Opus 5 --- docker/api/main.py | 32 ++++-- docker/api/test_api_smoke.py | 28 +++++ docker/worker/ripping.py | 100 ++++++++++++++--- docker/worker/tasks.py | 140 ++++++++++++++++++------ docker/worker/test_medien.py | 96 +++++++--------- docker/worker/test_original_aufheben.py | 124 +++++++++++++++++++++ docker/worker/test_ripping_helpers.py | 51 ++++++++- 7 files changed, 455 insertions(+), 116 deletions(-) create mode 100644 docker/worker/test_original_aufheben.py diff --git a/docker/api/main.py b/docker/api/main.py index e6f3208..9efa850 100644 --- a/docker/api/main.py +++ b/docker/api/main.py @@ -155,7 +155,7 @@ async def _auto_rip_wenn_aktiviert(pfad: str): else: unterordner = einstellungen.get("movieDir") or "movies" ziel = os.path.normpath(os.path.join(basis, unterordner)) - if not ziel.startswith(MEDIA_ROOT): + if not unter_wurzel(ziel, MEDIA_ROOT): ziel = None job_id = str(uuid.uuid4()) @@ -308,7 +308,7 @@ def _kann_neu_komprimieren(job: dict, work_dir: str) -> bool: return False if os.path.isdir(os.path.join("/app/temp/raw", job["id"])): return True - return work_dir.startswith(MEDIA_ROOT) and os.path.isdir(os.path.join(work_dir, job["id"])) + return unter_wurzel(work_dir, MEDIA_ROOT) and os.path.isdir(os.path.join(work_dir, job["id"])) @app.get("/jobs", response_model=List[Job]) @@ -373,7 +373,7 @@ def _validiere_ziel(target_dir: Optional[str]) -> Optional[str]: if not target_dir: return None normalisiert = os.path.normpath(target_dir) - if not normalisiert.startswith(MEDIA_ROOT): + if not unter_wurzel(normalisiert, MEDIA_ROOT): raise HTTPException( status_code=422, detail=f"Ziel muss unter {MEDIA_ROOT} liegen (Shares dort einhängen)", @@ -462,10 +462,24 @@ def _sicherer_dateiname(name: str) -> bool: return bool(name) and "/" not in name and "\\" not in name and not name.startswith(".") +def unter_wurzel(pfad: str, wurzel: str) -> bool: + """Liegt `pfad` wirklich unterhalb von `wurzel` (oder IST es die Wurzel)? + + Pure Funktion, testbar. Ein nacktes `startswith()` genügt hier nicht: + „/app/media-boese/x" beginnt mit „/app/media", liegt aber ausserhalb + (Befund 25.07.2026 bei der Durchsicht). Deshalb Gleichheit ODER Wurzel + samt Trennzeichen. Erwartet werden normalisierte Container-Pfade mit „/". + """ + if not pfad or not wurzel: + return False + sauber = wurzel.rstrip("/") or "/" + return pfad == sauber or pfad.startswith(sauber + "/") + + def _job_ausgabeordner(job: dict) -> str: """Validierter Ausgabeordner eines Jobs — strikt unter /app/media.""" ausgabe = os.path.normpath(job.get("output_path") or "") - if not ausgabe.startswith(MEDIA_ROOT): + if not unter_wurzel(ausgabe, MEDIA_ROOT): raise HTTPException(status_code=404, detail="Job hat keinen Ausgabeordner unter /app/media") return ausgabe @@ -522,7 +536,7 @@ async def download_job_file(job_id: str, dateiname: str): pfad = os.path.join(ausgabe, dateiname) def pruefe(): - return os.path.isfile(pfad) and os.path.realpath(pfad).startswith(MEDIA_ROOT) + return os.path.isfile(pfad) and unter_wurzel(os.path.realpath(pfad), MEDIA_ROOT) if not await asyncio.to_thread(pruefe): raise HTTPException(status_code=404, detail="Datei nicht gefunden") @@ -633,7 +647,7 @@ async def retry_transcode(job_id: str): # (Einstellungen → Verarbeitung), sonst Container-Default /app/temp/raw. einstellungen = await asyncio.to_thread(db.get_settings) work_dir = os.path.normpath((einstellungen.get("workDir") or "").strip() or "/") - raw_basis = work_dir if work_dir.startswith(MEDIA_ROOT) else "/app/temp/raw" + raw_basis = work_dir if unter_wurzel(work_dir, MEDIA_ROOT) else "/app/temp/raw" raw_dir = f"{raw_basis}/{job_id}" # Zielordner: der Worker schreibt das geplante Ziel beim Rip-Start nach # output_path (sprechender Name statt UUID) — alter Fallback bleibt. @@ -988,7 +1002,7 @@ async def delete_storage_mount(name: str): async def browse(path: str = MEDIA_ROOT): """Server-seitiger Ordner-Browser für die Ziel-Auswahl (nur unter /app/media).""" normalisiert = os.path.normpath(path) - if not normalisiert.startswith(MEDIA_ROOT): + if not unter_wurzel(normalisiert, MEDIA_ROOT): raise HTTPException(status_code=422, detail=f"Nur Pfade unter {MEDIA_ROOT}") def liste(): @@ -1029,7 +1043,7 @@ class MkdirRequest(BaseModel): async def browse_mkdir(request: MkdirRequest): """Neuen Ordner unter /app/media anlegen (Speicherziele-Verwaltung).""" basis = os.path.normpath(request.path) - if not basis.startswith(MEDIA_ROOT): + if not unter_wurzel(basis, MEDIA_ROOT): raise HTTPException(status_code=422, detail=f"Nur Pfade unter {MEDIA_ROOT}") name = request.name.strip() if not name or "/" in name or "\\" in name or name.startswith("."): @@ -1388,7 +1402,7 @@ async def download_aacs_dump(dateiname: str): pfad = os.path.join(MAKEMKV_DATA_ROOT, dateiname) def pruefe(): - return os.path.isfile(pfad) and os.path.realpath(pfad).startswith(MAKEMKV_DATA_ROOT) + return os.path.isfile(pfad) and unter_wurzel(os.path.realpath(pfad), MAKEMKV_DATA_ROOT) if not await asyncio.to_thread(pruefe): raise HTTPException( diff --git a/docker/api/test_api_smoke.py b/docker/api/test_api_smoke.py index 93d06ed..bc1077c 100644 --- a/docker/api/test_api_smoke.py +++ b/docker/api/test_api_smoke.py @@ -66,3 +66,31 @@ def test_remount_blockiert_den_api_start_nicht(): "remount muss als Hintergrund-Task laufen (nicht blockierend)" assert "await asyncio.to_thread(remount)" not in quelle, \ "remount darf nicht mehr direkt awaited werden (blockiert sonst den Start)" + + +def test_unter_wurzel_faellt_nicht_auf_praefix_namen_herein(): + """Befund 25.07.2026: In main.py prueften neun Stellen mit nacktem + startswith(MEDIA_ROOT) — darunter /browse und /browse/mkdir, wo der Pfad + vom Nutzer kommt. „/app/media-boese/x" beginnt mit „/app/media", liegt + aber ausserhalb. Zwilling von tasks.unter_wurzel im Worker.""" + from main import unter_wurzel + + assert unter_wurzel("/app/media", "/app/media") is True + assert unter_wurzel("/app/media/movies", "/app/media") is True + assert unter_wurzel("/app/media-boese/x", "/app/media") is False + assert unter_wurzel("/app/mediaX", "/app/media") is False + assert unter_wurzel("/etc/passwd", "/app/media") is False + assert unter_wurzel("", "/app/media") is False + assert unter_wurzel("/app/media", "") is False + assert unter_wurzel("/app/media/movies", "/app/media/") is True + + +def test_sicherer_dateiname_blockt_pfad_tricks(): + from main import _sicherer_dateiname + + assert _sicherer_dateiname("film.mkv") is True + assert _sicherer_dateiname("../../etc/passwd") is False + assert _sicherer_dateiname("a/b.mkv") is False + assert _sicherer_dateiname("a\b.mkv") is False + assert _sicherer_dateiname(".versteckt") is False + assert _sicherer_dateiname("") is False diff --git a/docker/worker/ripping.py b/docker/worker/ripping.py index 7c28230..8e20658 100644 --- a/docker/worker/ripping.py +++ b/docker/worker/ripping.py @@ -25,6 +25,36 @@ class RipAbbruch(Exception): Nutzer den Job abgebrochen hat (Status 'canceling' in der DB).""" +# include/uapi/linux/cdrom.h — dasselbe ioctl wie in api/devices.py +CDROMEJECT = 0x5309 + + +def wirf_disc_aus(device_path: str) -> bool: + """Wirft die Disc aus; gibt zurück, ob es geklappt hat. Wirft NIE. + + Befund 25.07.2026: Die Einstellung „Automatischer Auswurf" (Standard: ein, + Beschreibung „Disc nach erfolgreichem Ripping automatisch auswerfen") wurde + von niemandem gelesen — bei DVD/Blu-ray warf Rippy nie aus. + + Bewusst hier und nicht in detection.py: das Modul ist ein byteweiser + Zwilling der API-Kopie. `fcntl` gibt es nur unter Linux — der native + Windows-Worker lädt ripping.py ebenfalls, rippt dort aber nie. + """ + try: + from fcntl import ioctl + except ImportError: # Windows + return False + try: + fd = os.open(device_path, os.O_RDONLY | os.O_NONBLOCK) + try: + ioctl(fd, CDROMEJECT, 0) + finally: + os.close(fd) + return True + except OSError: + return False + + def check_makemkv_installed() -> bool: """Prüft, ob makemkvcon installiert ist.""" return shutil.which("makemkvcon") is not None @@ -271,16 +301,42 @@ def build_handbrake_cmd(input_path: str, output_path: str, preset: str = DEFAULT ] -def get_progress_from_line(line: str) -> int: - """Extrahiert Fortschritt in Prozent aus HandBrake-Ausgabe. +HB_ENCODE_RE = re.compile( + r'^Encoding:\s*task\s+(\d+)\s+of\s+(\d+),\s*(\d+(?:\.\d+)?)\s*%' +) - Testfund 22.07.: echtes HandBrake schreibt „45.50 %" MIT Leerzeichen vor - dem Prozentzeichen — eine Regex ohne \\s* parst NIE echte Ausgabe. + +def get_progress_from_line(line: str) -> int: + """Extrahiert den ENCODE-Fortschritt in Prozent aus HandBrake-Ausgabe. + + Gibt **-1** zurück, wenn die Zeile kein Encode-Fortschritt ist — genau wie + get_progress_from_prgv. Sonst wäre ein echtes „0 %" nicht von „keine + Angabe" zu unterscheiden, und der Anfang des Encodes bliebe unsichtbar. + + Formatstrings aus dem HandBrakeCLI-Binary im Worker-Image gelesen + (AGENTS Regel D — nicht aus dem Kopf): + Scanning title %d of %d, preview %d, %.2f %% → Scan, NICHT Encode + Encoding: task %d of %d, %.2f %% (%.2f fps, avg → Encode + Encoding: task %d of %d, Searching for start time, %.2f → Vorlauf, NICHT Encode + + Befund 25.07.2026 (Akira-UHD, live gemessen): Die alte Regex nahm JEDE + Zahl vor einem Prozentzeichen — also auch den Scan-Durchlauf, der VOR dem + Encodieren einmal bis 100 % hochläuft. Die Anzeige stand deshalb auf 99 %, + während der eigentliche Encode bei 1 % war. Zusammen mit dem alten Filter + `progress > 0` im Aufrufer blieb sie dort stundenlang kleben, weil + int(0.87) == 0 verworfen wurde. + + `task N of M` wird mitgerechnet: Presets mit zwei Durchläufen zählen die + Prozente je Durchlauf neu, sonst sprang die Anzeige mitten in der Datei + zurück auf 0. """ - match = re.search(r'(\d+\.\d+)\s*%', line) - if match: - return int(float(match.group(1))) - return 0 + match = HB_ENCODE_RE.match(line.strip()) + if not match: + return -1 + aktuell, gesamt, prozent = int(match.group(1)), int(match.group(2)), float(match.group(3)) + if gesamt < 1 or aktuell < 1: + return -1 + return int(((aktuell - 1) * 100 + prozent) / gesamt) def run_handbrake(input_path: str, output_path: str, preset: str = DEFAULT_HB_PRESET, progress_cb=None) -> dict: @@ -300,7 +356,10 @@ def run_handbrake(input_path: str, output_path: str, preset: str = DEFAULT_HB_PR try: for line in process.stdout: progress = get_progress_from_line(line) - if progress > 0 and progress_cb: + # >= 0: ein echtes 0 % ist eine Angabe und muss durch. Der alte + # Filter `> 0` verwarf den gesamten ersten Prozentpunkt — bei + # einem 4K-Encode sind das Stunden ohne sichtbare Bewegung. + if progress >= 0 and progress_cb: progress_cb(progress) except RipAbbruch: process.kill() @@ -320,20 +379,27 @@ def run_handbrake(input_path: str, output_path: str, preset: str = DEFAULT_HB_PR return {"status": "error", "error": str(e)} -def build_abcde_cmd(device_path: str, config_path: str) -> list: +def build_abcde_cmd(device_path: str, config_path: str, auswerfen: bool = True) -> list: """Baut das abcde-Kommando (pure Funktion, testbar). -o = Ausgabeformat (flac), -N = nicht-interaktiv, -x = Eject am Ende, -c = Config-Datei (enthält OUTPUTDIR). NIE ein Verzeichnis an -o geben. + + Befund 25.07.2026: `-x` stand fest verdrahtet drin. Damit warf eine + Audio-CD IMMER aus, während eine DVD/Blu-ray NIE auswarf — und die + Einstellung „Automatischer Auswurf" regelte keines von beidem, weil sie + nirgends gelesen wurde. Jetzt entscheidet sie hier mit. """ - return [ + cmd = [ "abcde", "-d", device_path, "-o", "flac", "-N", - "-x", - "-c", config_path ] + if auswerfen: + cmd.append("-x") + cmd += ["-c", config_path] + return cmd def write_abcde_config(output_dir: str) -> str: @@ -467,8 +533,10 @@ def rip_video(device_path: str, disc_id: str, disc_type: str = "dvd", progress_c log_cb=log_cb) -def rip_cd(device_path: str, disc_id: str, progress_cb=None, output_dir: str = None) -> dict: - """Rippt eine CD mit abcde (FLAC).""" +def rip_cd(device_path: str, disc_id: str, progress_cb=None, output_dir: str = None, + auswerfen: bool = True) -> dict: + """Rippt eine CD mit abcde (FLAC). `auswerfen` = Einstellung „Automatischer + Auswurf" (abcde macht das selbst per -x).""" if not check_abcde_installed(): return { "status": "error", @@ -492,7 +560,7 @@ def rip_cd(device_path: str, disc_id: str, progress_cb=None, output_dir: str = N try: process = subprocess.Popen( - build_abcde_cmd(device_path, config_path), + build_abcde_cmd(device_path, config_path, auswerfen=auswerfen), stdout=subprocess.PIPE, stderr=subprocess.STDOUT, text=True, diff --git a/docker/worker/tasks.py b/docker/worker/tasks.py index bb147e3..c2ef0d9 100644 --- a/docker/worker/tasks.py +++ b/docker/worker/tasks.py @@ -12,6 +12,7 @@ komprimiert danach auf Arbeitsgröße. Die Rohdatei liegt nur temporär in /app/temp und wird nach Erfolg gelöscht (Setting keepOriginal behält sie). """ +import errno import glob import json import os @@ -44,6 +45,7 @@ from ripping import ( rip_cd, rip_video, run_handbrake, + wirf_disc_aus, ) API_URL = os.getenv("API_URL", "http://api:8000") @@ -91,13 +93,38 @@ RAW_DIR = os.getenv("RAW_DIR", "/app/temp/raw") MEDIA_ROOT = "/app/media" +def unter_wurzel(pfad: str, wurzel: str) -> bool: + """Liegt `pfad` wirklich unterhalb von `wurzel` (oder IST es die Wurzel)? + + Pure Funktion, testbar. Ein nacktes `startswith()` genügt nicht: + „/app/media-boese/x" beginnt mit „/app/media", liegt aber ausserhalb + (Befund 25.07.2026 bei der Durchsicht). Immer „/" als Trenner — das sind + Container-Pfade, auch wenn ein nativer Windows-Worker das Modul lädt. + + Gleichlautend in docker/api/main.py; es gibt kein geteiltes Paket zwischen + den Containern. + """ + if not pfad or not wurzel: + return False + sauber = wurzel.rstrip("/") or "/" + return pfad == sauber or pfad.startswith(sauber + "/") + + def _zielbasis(target_dir, disc_type: str) -> str: - """Ablagebasis: vom Nutzer gewähltes Ziel (validiert) oder Standard.""" + """Ablagebasis: vom Nutzer gewähltes Ziel (validiert) oder Standard. + + posixpath statt os.path — aus demselben Grund wie in _arbeitsverzeichnis: + das sind IMMER Container-Pfade. os.path.normpath macht unter Windows + Backslashes daraus, und dann greift die MEDIA_ROOT-Prüfung nicht mehr, das + gewählte Ziel fiele still auf den Standard zurück. Aufgefallen 25.07.2026, + als der Test dafür erstmals unter Windows lief. Live war es nie: aufgerufen + wird nur aus rip_disc, und das ist auf Windows-Workern verriegelt. + """ if target_dir: - normalisiert = os.path.normpath(target_dir) - if normalisiert.startswith(MEDIA_ROOT): + normalisiert = posixpath.normpath(target_dir) + if unter_wurzel(normalisiert, MEDIA_ROOT): return normalisiert - return os.path.join(RIP_OUTPUT_DIR, disc_type) + return posixpath.join(RIP_OUTPUT_DIR, disc_type) def _arbeitsverzeichnis(einstellungen: dict, job_wahl: str = "") -> str: @@ -121,7 +148,7 @@ def _arbeitsverzeichnis(einstellungen: dict, job_wahl: str = "") -> str: wert = (kandidat or "").strip() if wert: normalisiert = posixpath.normpath(wert) - if normalisiert.startswith(MEDIA_ROOT): + if unter_wurzel(normalisiert, MEDIA_ROOT): return normalisiert return RAW_DIR @@ -199,36 +226,65 @@ def _original_aufheben(job_id: str, raw_dir: str, final_dir: str) -> None: """Roh-Rip zusätzlich aufheben — darf den Job NIEMALS scheitern lassen. Befund 25.07.2026 (Akira-UHD, echter Schaden): Hier stand ein nacktes - shutil.move(). Arbeitsverzeichnis (/app/temp, Docker-Volume) und Ziel - (/app/media, Bind-Mount) sind VERSCHIEDENE Dateisysteme — os.rename - scheitert dort mit EXDEV, und shutil.move fällt auf Kopieren zurück. Damit - lief eine 74-GB-Vollkopie auf dieselbe Platte an, bis sie mit ENOSPC voll - war. Ergebnis: Platte 100 % voll, Worker-Container startete nicht mehr — - und der Job wurde als FEHLGESCHLAGEN markiert, obwohl die komprimierte - Datei längst fertig und in Ordnung war. Der Nutzer sah nur „nichts da". + shutil.move(). Zwischen Arbeitsverzeichnis (/app/temp) und Ziel + (/app/media) scheitert os.rename mit EXDEV, shutil.move fällt auf Kopieren + zurück — eine 74-GB-Vollkopie auf dieselbe Platte, bis sie mit ENOSPC voll + war. Ergebnis: Platte 100 % voll, Worker-Container startete nicht mehr, und + der Job galt als FEHLGESCHLAGEN, obwohl die komprimierte Datei längst + fertig war. Der Nutzer sah nur „nichts da". - Deshalb dreifach abgesichert: Platz vorher prüfen, Fehler abfangen, und im - Zweifel die Rohdatei einfach liegen lassen, wo sie ist. + ## Warum hier NICHT vorhergesagt, sondern versucht wird + + Die erste Fassung dieses Schutzes verglich `os.stat(...).st_dev` und + schloss aus gleichen Werten auf „reines Umhängen, kein Platz nötig". Am + 25.07.2026 im Worker-Container nachgemessen — beides zugleich wahr: + + st_dev /app/temp = 2050 + st_dev /app/media = 2050 → also identisch + os.rename(...) → EXDEV, „Invalid cross-device link" + + Der Kernel vergleicht bei rename() den **Mount**, nicht das Gerät. /app/temp + (Docker-Volume) und /app/media (Bind-Mount) sind zwei Mounts DERSELBEN + ext4-Partition. Die st_dev-Prüfung war deshalb wirkungslos: sie sah + „gleiches Dateisystem", übersprang die Platzprüfung, und shutil.move kopierte + doch. Der Schutz hätte genau den Schaden zugelassen, gegen den er gebaut war. + + Also: erst rename VERSUCHEN. Klappt es, ist es umgehängt und fertig. + Kommt EXDEV, steht fest, dass kopiert werden müsste — und erst dann wird + der Platz geprüft. Das ist keine Vermutung mehr, sondern die Antwort des + Kernels. """ ziel_original = os.path.join(final_dir, "original") try: - # Liegt beides auf demselben Dateisystem, ist es ein reines Umhängen — - # sofort fertig, kein zusätzlicher Platz nötig. - gleiches_dateisystem = os.stat(raw_dir).st_dev == os.stat(final_dir).st_dev - if not gleiches_dateisystem: - benoetigt = _ordner_groesse(raw_dir) - frei = _frei_bytes(final_dir) - if frei < benoetigt * 1.05: - db.add_log( - "warning", "worker", - f"Job {job_id}: Original NICHT aufgehoben — dafür wären " - f"{benoetigt / 1024**3:.1f} GB nötig, frei sind nur " - f"{frei / 1024**3:.1f} GB. Die Roh-Datei bleibt unter " - f"{raw_dir} liegen. Abhilfe: Arbeitsverzeichnis " - "(Einstellungen → Verarbeitung) auf dieselbe Freigabe legen " - "wie das Ziel — dann wird nur umgehängt statt kopiert." - ) - return + # Der billige Weg zuerst — und er ist gleichzeitig der einzige + # verlässliche Test, ob überhaupt umgehängt werden kann. + try: + os.rename(raw_dir, ziel_original) + db.add_log( + "info", "worker", + f"Job {job_id}: Original behalten unter {ziel_original} (umgehängt, " + "kein zusätzlicher Platz nötig)", + ) + return + except OSError as e: + if e.errno != errno.EXDEV: + raise # etwas anderes ist schiefgelaufen → unten ehrlich melden + + # Ab hier ist eine echte Kopie unvermeidlich. Jetzt lohnt die Platzfrage. + benoetigt = _ordner_groesse(raw_dir) + frei = _frei_bytes(final_dir) + if frei < benoetigt * 1.05: + db.add_log( + "warning", "worker", + f"Job {job_id}: Original NICHT aufgehoben — Ziel liegt auf einem " + f"anderen Mount, es müsste kopiert werden. Dafür wären " + f"{benoetigt / 1024**3:.1f} GB nötig, frei sind nur " + f"{frei / 1024**3:.1f} GB. Die Roh-Datei bleibt unter " + f"{raw_dir} liegen. Abhilfe: Arbeitsverzeichnis " + "(Einstellungen → Verarbeitung) auf dieselbe Freigabe legen " + "wie das Ziel — dann wird nur umgehängt statt kopiert." + ) + return shutil.move(raw_dir, ziel_original) db.add_log("info", "worker", f"Job {job_id}: Original behalten unter {ziel_original}") except OSError as e: @@ -500,7 +556,10 @@ def rip_disc(self, device_path: str, job_id: str, target_dir: str = None): return ergebnis if disc_type == "cd": - ergebnis = rip_cd(device_path, job_id, progress_cb=fortschritt, output_dir=final_dir) + ergebnis = rip_cd( + device_path, job_id, progress_cb=fortschritt, output_dir=final_dir, + auswerfen=bool(einstellungen.get("autoEject", True)), + ) elif transcode_an: # Roh-Rip ins Arbeitsverzeichnis (wird nach erfolgreicher Kompression gelöscht) ergebnis = rip_video( @@ -562,6 +621,23 @@ def rip_disc(self, device_path: str, job_id: str, target_dir: str = None): "Normale BD/DVD gehen weiterhin." ) + # Automatischer Auswurf. Die Disc ist nach dem Rip nicht mehr nötig — die + # Kompression arbeitet auf der Datei, nicht am Laufwerk. + # + # Befund 25.07.2026: Die Einstellung („Disc nach erfolgreichem Ripping + # automatisch auswerfen", Standard ein) wurde von NIEMANDEM gelesen. Bei + # DVD/Blu-ray warf Rippy deshalb nie aus, bei Audio-CD dagegen immer, weil + # abcde `-x` fest verdrahtet bekam. Jetzt entscheidet die Einstellung beides. + if ergebnis.get("status") == "success" and ist_video and einstellungen.get("autoEject", True): + if wirf_disc_aus(device_path): + db.add_log("info", "worker", f"Job {job_id}: Disc ausgeworfen") + else: + db.add_log( + "warning", "worker", + f"Job {job_id}: Disc konnte nicht ausgeworfen werden ({device_path}) — " + "der Rip ist davon unberührt.", + ) + if ergebnis.get("status") == "success" and transcode_an: # Kompression als eigener Task — an den im Rip-Dialog GEWÄHLTEN Worker # (worker_direct), sonst an die geteilte transcode-Queue (irgendein diff --git a/docker/worker/test_medien.py b/docker/worker/test_medien.py index 9f471ce..4731990 100644 --- a/docker/worker/test_medien.py +++ b/docker/worker/test_medien.py @@ -114,63 +114,11 @@ def test_pfad_lokal_uebersetzt_fuer_windows_worker(): assert pfad_lokal("/app/temp/raw/abc", "") == "/app/temp/raw/abc" -def test_original_aufheben_laesst_job_nie_scheitern(tmp_path, monkeypatch): - """Der Schritt "Original behalten" darf einen fertigen Job NICHT killen. - - Schaden vom 25.07.2026 (Akira-UHD): shutil.move() ueber Dateisystemgrenzen - kopierte 74 GB, lief in ENOSPC, und der Job galt als fehlgeschlagen — - obwohl die komprimierte Datei laengst fertig war. Der Nutzer sah nur eine - leere Queue. - """ - import tasks - - meldungen = [] - monkeypatch.setattr(tasks.db, "add_log", lambda *a: meldungen.append(a)) - - raw = tmp_path / "raw" - raw.mkdir() - (raw / "title_t00.mkv").write_bytes(b"x" * 1024) - final = tmp_path / "final" - final.mkdir() - - def platzt(*_a, **_k): - raise OSError(28, "No space left on device") - - monkeypatch.setattr(tasks.shutil, "move", platzt) - # Darf NICHT werfen - tasks._original_aufheben("job-1", str(raw), str(final)) - assert meldungen and meldungen[-1][0] == "warning" - assert "Roh-Datei bleibt" in meldungen[-1][2] - - -def test_original_aufheben_warnt_statt_platte_vollzuschreiben(tmp_path, monkeypatch): - """Zu wenig Platz auf einem ANDEREN Dateisystem -> gar nicht erst anfangen.""" - import tasks - - meldungen = [] - monkeypatch.setattr(tasks.db, "add_log", lambda *a: meldungen.append(a)) - bewegt = [] - monkeypatch.setattr(tasks.shutil, "move", lambda *a: bewegt.append(a)) - - raw = tmp_path / "raw" - raw.mkdir() - (raw / "gross.mkv").write_bytes(b"x" * 4096) - final = tmp_path / "final" - final.mkdir() - - # Verschiedene Dateisysteme vortaeuschen und den freien Platz kleinrechnen. - # _ordner_groesse wird mitgefaelscht, weil ein selbstgebautes os.stat sonst - # os.path.getsize mitreisst (das liest st_size an derselben Stelle). - class Stat: - def __init__(self, dev): self.st_dev = dev - monkeypatch.setattr(tasks.os, "stat", lambda p: Stat(1 if "raw" in str(p) else 2)) - monkeypatch.setattr(tasks, "_ordner_groesse", lambda _p: 4096) - monkeypatch.setattr(tasks, "_frei_bytes", lambda _p: 100) - - tasks._original_aufheben("job-2", str(raw), str(final)) - assert bewegt == [], "es darf NICHT kopiert werden, wenn der Platz nicht reicht" - assert meldungen[-1][0] == "warning" - assert "NICHT aufgehoben" in meldungen[-1][2] +# Die beiden frueheren Tests zu _original_aufheben sind nach +# test_original_aufheben.py gewandert und dort erweitert. Sie pruefen jetzt das +# richtige Kriterium: Vorher taeuschten sie per gefaelschtem os.stat +# „verschiedene Dateisysteme" vor — genau die Annahme, an der der Schutz in der +# Praxis scheiterte (st_dev war identisch, os.rename trotzdem EXDEV). def test_arbeitsverzeichnis_wahl_des_rips_schlaegt_die_einstellung(): @@ -191,3 +139,37 @@ def test_arbeitsverzeichnis_wahl_des_rips_schlaegt_die_einstellung(): assert tasks._arbeitsverzeichnis({}, "/app/media/../etc") == tasks.RAW_DIR # Leere Wahl faellt sauber auf die Einstellung zurueck assert tasks._arbeitsverzeichnis(einst, " ") == "/app/media/movies" + + +def test_unter_wurzel_faellt_nicht_auf_praefix_namen_herein(): + """Befund 25.07.2026: Elf Stellen prueften mit nacktem startswith(). + „/app/media-boese/x" beginnt mit „/app/media", liegt aber ausserhalb.""" + import tasks + + assert tasks.unter_wurzel("/app/media", "/app/media") is True + assert tasks.unter_wurzel("/app/media/movies", "/app/media") is True + assert tasks.unter_wurzel("/app/media/movies/Film (2020)", "/app/media") is True + # Der Fall, den startswith() durchgelassen haette + assert tasks.unter_wurzel("/app/media-boese/x", "/app/media") is False + assert tasks.unter_wurzel("/app/mediaX", "/app/media") is False + assert tasks.unter_wurzel("/etc/passwd", "/app/media") is False + assert tasks.unter_wurzel("", "/app/media") is False + assert tasks.unter_wurzel("/app/media", "") is False + # Wurzel mit Schrägstrich am Ende darf nichts aendern + assert tasks.unter_wurzel("/app/media/movies", "/app/media/") is True + + +def test_zielbasis_lehnt_praefix_ausbruch_ab(): + import tasks + + assert tasks._zielbasis("/app/media/movies", "bluray") == "/app/media/movies" + # Ausbruch per Praefix-Namen faellt auf den Standard zurueck + assert tasks._zielbasis("/app/media-boese", "bluray") != "/app/media-boese" + assert tasks._zielbasis("/etc", "bluray") != "/etc" + + +def test_arbeitsverzeichnis_lehnt_praefix_ausbruch_ab(): + import tasks + + assert tasks._arbeitsverzeichnis({}, "/app/media-boese") == tasks.RAW_DIR + assert tasks._arbeitsverzeichnis({"workDir": "/app/mediaX"}) == tasks.RAW_DIR diff --git a/docker/worker/test_original_aufheben.py b/docker/worker/test_original_aufheben.py new file mode 100644 index 0000000..7a3185b --- /dev/null +++ b/docker/worker/test_original_aufheben.py @@ -0,0 +1,124 @@ +"""Tests fuer _original_aufheben — der Pfad, der am 25.07.2026 die Platte fuellte. + +Der Kern: Es wird NICHT vorhergesagt, ob umgehaengt werden kann, sondern +os.rename versucht. Die alte Fassung verglich st_dev und lag damit falsch — +auf der Rippy-VM sind st_dev von /app/temp und /app/media identisch (2050), +os.rename scheitert zwischen ihnen trotzdem mit EXDEV, weil der Kernel den +Mount vergleicht und nicht das Geraet. Die Platzpruefung wurde deshalb +uebersprungen und shutil.move kopierte 75 GB bei 37 GB frei. +""" + +import errno +import os + +import pytest + +import tasks + + +class FakeDb: + """Faengt nur die Log-Zeilen ab — mehr braucht _original_aufheben nicht.""" + + def __init__(self): + self.logs = [] + + def add_log(self, level, source, message): + self.logs.append((level, message)) + + def meldungen(self): + return " ".join(m for _, m in self.logs) + + +@pytest.fixture +def fake_db(monkeypatch): + ersatz = FakeDb() + monkeypatch.setattr(tasks, "db", ersatz) + return ersatz + + +def _lege_rohdaten_an(tmp_path, groesse=2048): + raw = tmp_path / "raw" / "job-1" + raw.mkdir(parents=True) + (raw / "title_t00.mkv").write_bytes(b"x" * groesse) + final = tmp_path / "media" / "Film (2020)" + final.mkdir(parents=True) + return str(raw), str(final) + + +def test_umhaengen_wenn_derselbe_mount(tmp_path, fake_db): + """Der gute Fall: rename klappt, nichts wird kopiert, kein Platz noetig.""" + raw, final = _lege_rohdaten_an(tmp_path) + + tasks._original_aufheben("job-1", raw, final) + + assert os.path.isdir(os.path.join(final, "original")) + assert os.path.isfile(os.path.join(final, "original", "title_t00.mkv")) + assert not os.path.exists(raw) + assert "umgehängt" in fake_db.meldungen() + + +def test_bei_exdev_und_zu_wenig_platz_wird_nur_gewarnt(tmp_path, fake_db, monkeypatch): + """Der Fall, der die Platte fuellte: rename geht nicht, Platz reicht nicht. + + Vorher lief hier eine Vollkopie an, weil die st_dev-Pruefung „gleiches + Dateisystem" meldete und die Platzpruefung deshalb ausblieb. + """ + raw, final = _lege_rohdaten_an(tmp_path) + + def kein_rename(*_a, **_k): + raise OSError(errno.EXDEV, "Invalid cross-device link") + + monkeypatch.setattr(tasks.os, "rename", kein_rename) + monkeypatch.setattr(tasks, "_frei_bytes", lambda _p: 1024) # weniger als die Rohdaten + + tasks._original_aufheben("job-1", raw, final) + + # Rohdaten bleiben unangetastet liegen, es wurde NICHTS kopiert + assert os.path.isfile(os.path.join(raw, "title_t00.mkv")) + assert not os.path.exists(os.path.join(final, "original")) + meldungen = fake_db.meldungen() + assert "NICHT aufgehoben" in meldungen + assert "anderen Mount" in meldungen + assert "Arbeitsverzeichnis" in meldungen # nennt die Abhilfe + + +def test_bei_exdev_und_genug_platz_wird_kopiert(tmp_path, fake_db, monkeypatch): + raw, final = _lege_rohdaten_an(tmp_path) + + echtes_rename = os.rename + aufrufe = {"n": 0} + + def rename_erst_exdev(*args, **kwargs): + # Nur der Versuch von _original_aufheben scheitert; shutil.move darf + # intern weiter umbenennen (es kopiert selbst und benennt Teile um). + aufrufe["n"] += 1 + if aufrufe["n"] == 1: + raise OSError(errno.EXDEV, "Invalid cross-device link") + return echtes_rename(*args, **kwargs) + + monkeypatch.setattr(tasks.os, "rename", rename_erst_exdev) + monkeypatch.setattr(tasks, "_frei_bytes", lambda _p: 10 * 1024**3) + + tasks._original_aufheben("job-1", raw, final) + + assert os.path.isfile(os.path.join(final, "original", "title_t00.mkv")) + assert "Original behalten" in fake_db.meldungen() + + +def test_anderer_fehler_wird_ehrlich_gemeldet_und_reisst_job_nicht_mit( + tmp_path, fake_db, monkeypatch +): + """Ein Fehler beim Aufheben darf den Job NIE scheitern lassen — die + komprimierte Datei ist zu diesem Zeitpunkt fertig und in Ordnung.""" + raw, final = _lege_rohdaten_an(tmp_path) + + def zugriff_verweigert(*_a, **_k): + raise OSError(errno.EACCES, "Permission denied") + + monkeypatch.setattr(tasks.os, "rename", zugriff_verweigert) + + tasks._original_aufheben("job-1", raw, final) # darf nicht werfen + + meldungen = fake_db.meldungen() + assert "konnte nicht aufgehoben werden" in meldungen + assert raw in meldungen # sagt, WO die Rohdatei liegt diff --git a/docker/worker/test_ripping_helpers.py b/docker/worker/test_ripping_helpers.py index af47e77..f785e11 100644 --- a/docker/worker/test_ripping_helpers.py +++ b/docker/worker/test_ripping_helpers.py @@ -43,10 +43,43 @@ def test_handbrake_cmd_arbeitet_auf_datei_nicht_geraet(): def test_handbrake_progress_parsing(): - # Testfund 22.07.: echtes HandBrake schreibt „45.50 %" MIT Leerzeichen + # Testfund 22.07.: echtes HandBrake schreibt 45.50 % MIT Leerzeichen assert get_progress_from_line("Encoding: task 1 of 1, 45.50 %") == 45 assert get_progress_from_line("Encoding: task 1 of 1, 100.00 %") == 100 - assert get_progress_from_line("kein Fortschritt hier") == 0 + # Echte Zeile mit fps-Anhang, wie sie im Binary steht + assert get_progress_from_line( + "Encoding: task 1 of 1, 12.34 % (5.67 fps, avg 4.32 fps, ETA 00h12m34s)" + ) == 12 + # Ein echtes 0 % ist eine ANGABE, keine Leermeldung + assert get_progress_from_line("Encoding: task 1 of 1, 0.00 %") == 0 + + +def test_handbrake_progress_ignoriert_scan_durchlauf(): + """Befund 25.07.2026 (Akira-UHD, live gemessen): HandBrake läuft VOR dem + Encodieren einen Scan-Durchlauf, der ebenfalls Prozente ausgibt und dabei + bis 100 % steigt. Die alte Regex nahm jede Zahl vor einem Prozentzeichen + und meldete deshalb 99 %, während der Encode bei 1 % stand. + + -1 heißt „keine Encode-Fortschrittszeile" — dasselbe Muster wie bei + get_progress_from_prgv. Fremde Zeilen dürfen NIE als 0 % gelten. + """ + assert get_progress_from_line("Scanning title 1 of 1, preview 3, 30.00 %") == -1 + assert get_progress_from_line("Scanning title 1 of 1, preview 10, 100.00 %") == -1 + # Vorlauf-Phase: Prozente beziehen sich auf die Suche, nicht auf den Encode + assert get_progress_from_line( + "Encoding: task 1 of 1, Searching for start time, 42.00 %" + ) == -1 + assert get_progress_from_line("kein Fortschritt hier") == -1 + assert get_progress_from_line("Muxing: this may take awhile...") == -1 + assert get_progress_from_line("") == -1 + + +def test_handbrake_progress_rechnet_zwei_durchlaeufe_zusammen(): + """Presets mit zwei Durchläufen zählen die Prozente je Durchlauf neu. + Ohne Verrechnung sprang die Anzeige mitten in der Datei zurück auf 0.""" + assert get_progress_from_line("Encoding: task 1 of 2, 50.00 %") == 25 + assert get_progress_from_line("Encoding: task 2 of 2, 0.00 %") == 50 + assert get_progress_from_line("Encoding: task 2 of 2, 100.00 %") == 100 def test_prgv_parsing(): @@ -129,6 +162,20 @@ def test_abcde_cmd_hat_genau_ein_ausgabeformat(): assert "-N" in cmd # nicht-interaktiv, sonst hängt der Worker +def test_abcde_auswurf_folgt_der_einstellung(): + """Befund 25.07.2026: `-x` (Auswurf) stand fest verdrahtet drin. Eine + Audio-CD warf damit IMMER aus, eine DVD/Blu-ray NIE — und die Einstellung + „Automatischer Auswurf" regelte keines von beidem, weil sie nirgends + gelesen wurde.""" + assert "-x" in build_abcde_cmd("/dev/sr0", "/tmp/c.conf", auswerfen=True) + assert "-x" not in build_abcde_cmd("/dev/sr0", "/tmp/c.conf", auswerfen=False) + # Standard bleibt „auswerfen" — so war das Verhalten bisher + assert "-x" in build_abcde_cmd("/dev/sr0", "/tmp/c.conf") + # Die Config darf durch das weggefallene -x nicht verrutschen + ohne = build_abcde_cmd("/dev/sr0", "/tmp/c.conf", auswerfen=False) + assert ohne[ohne.index("-c") + 1] == "/tmp/c.conf" + + def test_abcde_config_enthaelt_zielverzeichnis(): pfad = write_abcde_config("/app/media/cd/test123") try: