diff --git a/CHANGELOG.md b/CHANGELOG.md index 7902db5d..b55e252d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -42,6 +42,8 @@ The bundled TTS model package (`pyproject.toml`) is versioned independently. ### Fixed - Backend journal, dictation reset, voice-catalog, and crash-notification failures are now visible and retryable instead of being silently ignored. (#1459) +- Failed gallery, batch-video, and desktop-log cleanup is now reported instead of silently claiming success, and diagnostic redaction fails closed if a scrubber breaks. (#1458) +- Remote backends can no longer probe or overwrite arbitrary host files through native-only tools, and imported or persisted paths cannot escape their VoiceStudio data folders. (#1455) - Linux releases now verify that the AppImage actually contains the compatibility launcher, instead of silently shipping Tauri's stock launcher and opening as a blank window on newer Mesa systems. (#1464) - Patched dependency releases now cover 35 Python and Rust security advisories without weakening VoiceStudio's GPU or offline-runtime compatibility. (#1456, #1472, #1473, #1474, #1475, #1476, #1477) - Curated models now install and repair from reviewed, immutable revisions; custom MOSS remote code requires an explicit safety opt-in. (#1453) diff --git a/backend/api/dependencies.py b/backend/api/dependencies.py index 2c8bc394..d3217d6e 100644 --- a/backend/api/dependencies.py +++ b/backend/api/dependencies.py @@ -7,6 +7,8 @@ composed at the route or router level without surprises. Currently exposed: - `require_loopback`: 403 unless the request came from a loopback origin (bypassed in explicit server mode — see `_server_mode`). +- `require_native_access`: true-loopback-only access to the host filesystem; + unlike `require_loopback`, it is never bypassed by server mode. - `ws_remote_authorized`: whether a WebSocket handshake from a non-loopback client carries the remote API key (Wave 2.3) — used by WS endpoints that keep their own inline loopback guards. @@ -241,6 +243,19 @@ def require_local(request: Request) -> None: raise HTTPException(status_code=403, detail="loopback origin required") +def require_native_access(request: Request) -> None: + """Protect capabilities that read or write operator-chosen host paths. + + Docker server mode deliberately relaxes the ordinary admin gate because a + bridge makes even local traffic appear remote. That exception is unsafe for + native file pickers: a remote API caller must never probe or overwrite an + arbitrary path on the backend host, even with the server API key. + """ + host = request.client.host if request.client else None + if not is_loopback(host): + raise HTTPException(status_code=403, detail="native filesystem access requires loopback origin") + + def remote_api_key() -> str | None: """The remote-backend bearer key (Wave 2.3), or None when remote mode is off. Read at call time so tests can monkeypatch the env.""" diff --git a/backend/api/routers/batch.py b/backend/api/routers/batch.py index a65ed2c8..b8885727 100644 --- a/backend/api/routers/batch.py +++ b/backend/api/routers/batch.py @@ -20,6 +20,7 @@ from pydantic import BaseModel from core.config import DATA_DIR from core import failure +from core.file_cleanup import FileCleanupError, unlink_if_present router = APIRouter() logger = logging.getLogger("omnivoice.batch") @@ -573,14 +574,18 @@ def cancel_batch_job(job_id: str): @router.delete("/batch/jobs/{job_id}") def delete_batch_job(job_id: str): """Delete a batch job record and its video file.""" - job = _jobs.pop(job_id, None) + job = _jobs.get(job_id) if not job: raise HTTPException(404, "Job not found") - if job.get("video_path") and os.path.exists(job["video_path"]): + if job.get("video_path"): try: - os.remove(job["video_path"]) - except Exception: - pass + unlink_if_present(job["video_path"]) + except FileCleanupError as exc: + raise HTTPException( + status_code=500, + detail="Could not delete the batch video file. Close any app using it and retry.", + ) from exc + _jobs.pop(job_id, None) return {"deleted": True} diff --git a/backend/api/routers/dub_export.py b/backend/api/routers/dub_export.py index 6f5f253e..5217cc20 100644 --- a/backend/api/routers/dub_export.py +++ b/backend/api/routers/dub_export.py @@ -1,19 +1,21 @@ -import os +import asyncio import io -import re import json +import logging +import ntpath +import os +import re import time import uuid -import asyncio -import logging +from pathlib import Path, PureWindowsPath from typing import Optional -from fastapi import APIRouter, HTTPException, Query, Response -from fastapi.responses import FileResponse, StreamingResponse -from core.config import DUB_DIR, dub_seg_path -from core.tasks import task_manager +from core.config import DUB_DIR from core.http_headers import content_disposition -from api.routers.dub_core import _get_job +from core.path_security import UnsafePath, resolve_within +from core.tasks import task_manager +from fastapi import APIRouter, Header, HTTPException, Query, Response +from fastapi.responses import FileResponse, StreamingResponse from services.ffmpeg_utils import ( bed_mix_filter, explain_ffmpeg_failure, @@ -28,6 +30,8 @@ from services.video_retime import ( prepare_smart_fit_video, ) +from api.routers.dub_core import _get_job + router = APIRouter() logger = logging.getLogger("omnivoice.api") @@ -40,6 +44,143 @@ def _unique_stamp() -> str: _SAFE_LANG = re.compile(r"^[A-Za-z0-9_-]{1,32}$") +def _job_dir_or_400(job_id: str) -> str: + if not re.fullmatch(r"[A-Za-z0-9_-]{1,64}", job_id or ""): + raise HTTPException(status_code=400, detail="Invalid job id") + try: + return str(resolve_within(DUB_DIR, job_id)) + except UnsafePath as exc: + raise HTTPException(status_code=400, detail="Invalid job id") from exc + + +def _existing_job_dir_or_404(job_id: str) -> str: + """Discover a real job directory without passing request data to a path sink.""" + _job_dir_or_400(job_id) + try: + for entry in os.scandir(DUB_DIR): + if entry.name == job_id and not entry.is_symlink() and entry.is_dir(follow_symlinks=False): + return entry.path + except OSError as exc: + raise HTTPException(status_code=404, detail="Job directory not found") from exc + raise HTTPException(status_code=404, detail="Job directory not found") + + +def _resolve_dub_artifact(value: object, job_id: str) -> Path: + """Resolve current or safely rebased pre-relocation dub artifact paths.""" + raw = str(value or "") + try: + resolved = resolve_within(DUB_DIR, raw) + relative = resolved.relative_to(Path(DUB_DIR).resolve()) + if not relative.parts or relative.parts[0] != job_id: + raise UnsafePath("Artifact does not belong to the requested job") + return resolved + except UnsafePath: + # Older job rows store absolute paths. After the user relocates the + # data directory, preserve only the suffix rooted at the exact + # ``dub_jobs`` boundary; never touch the old host path itself. + if ntpath.isabs(raw): + parts = PureWindowsPath(raw).parts + elif os.path.isabs(raw): + parts = Path(raw).parts + else: + raise + anchor = Path(DUB_DIR).name + positions = [index for index, part in enumerate(parts) if part == anchor] + if not positions: + raise + relative_parts = parts[positions[-1] + 1:] + if ( + not relative_parts + or relative_parts[0] != job_id + or any( + part in {"", ".", ".."} + or "/" in part + or "\\" in part + or ":" in part + for part in relative_parts + ) + ): + raise + return resolve_within(DUB_DIR, Path(*relative_parts)) + + +def _discover_job_artifact(path: Path, job_id: str) -> Path | None: + """Return an existing artifact by walking the validated job directory. + + Persisted paths select names but never reach a filesystem sink. Each + returned path comes from ``os.scandir`` beneath the validated job root, + and symlinks are rejected so a post-validation swap cannot escape. + """ + job_root = Path(_existing_job_dir_or_404(job_id)).resolve() + try: + parts = path.relative_to(job_root).parts + except ValueError: + return None + if not parts: + return None + current = job_root + for index, requested in enumerate(parts): + if os.path.basename(requested) != requested or requested in {"", ".", ".."}: + return None + try: + entry = next( + ( + item + for item in os.scandir(current) + if item.name == requested and not item.is_symlink() + ), + None, + ) + except OSError: + return None + if entry is None: + return None + if index < len(parts) - 1 and not entry.is_dir(follow_symlinks=False): + return None + current = Path(entry.path) + return current if current.is_file() else None + + +def _dub_artifact(value: object, job_id: str, *, missing_detail: str = "File not found") -> str: + """Resolve a persisted job artifact inside the global dub-data boundary.""" + try: + resolved = _resolve_dub_artifact(value, job_id) + except UnsafePath as exc: + raise HTTPException(status_code=400, detail="Invalid job artifact path") from exc + path = _discover_job_artifact(resolved, job_id) + if path is None: + raise HTTPException(status_code=404, detail=missing_detail) + return str(path) + + +def _optional_dub_artifact(value: object, job_id: str) -> str | None: + if not value: + return None + try: + resolved = _resolve_dub_artifact(value, job_id) + except UnsafePath as exc: + raise HTTPException(status_code=400, detail="Invalid job artifact path") from exc + path = _discover_job_artifact(resolved, job_id) + return str(path) if path is not None else None + + +def _safe_lang_or_400(lang: str | None) -> str | None: + if lang is not None and not _SAFE_LANG.fullmatch(lang): + raise HTTPException(status_code=400, detail="Invalid language code") + return lang + + +def _consume_native_save(authorization: str) -> str | None: + if not authorization: + return None + from core.path_authorization import PathAuthorizationError, consume + + try: + return consume(authorization, "dub_export") + except PathAuthorizationError as exc: + raise HTTPException(status_code=403, detail=str(exc)) from exc + + def _native_save(source: str, destination: str, display_name: str, media_type: str): """Copy a generated export file to a user-chosen destination and return JSON.""" import shutil @@ -432,7 +573,7 @@ async def dub_download( preserve_bg: bool = Query(True, description="Mix background noise into dubbed tracks"), default_track: str = Query("original"), include_tracks: str = Query("", description="Comma-separated list of tracks to include (e.g. 'original,de,es'). Empty = include all."), - save_path: str = Query("", description="Absolute destination path. If set, mux output is copied there and JSON returned instead of FileResponse."), + save_authorization: str = Header("", alias="X-VoiceStudio-Path-Authorization"), burn_subs: bool = Query(False, description="Burn subtitles into the video stream (forces re-encode). Uses dual-subtitle layout when dual=1."), dual: bool = Query(False, description="When burn_subs=1, render translated on top of italicised original."), out_format: str = Query("m4a", description="Audio-only jobs (#119): output container — wav, m4a, mp3, or flac. Ignored for video jobs."), @@ -440,8 +581,7 @@ async def dub_download( # Strict allowlist on the path param BEFORE it reaches any filesystem # path or ffmpeg argv (export dir, retime work path, slice paths). Real # job ids are short uuid slices — alnum/hyphen/underscore only. - if not re.fullmatch(r"[A-Za-z0-9_-]{1,64}", job_id): - raise HTTPException(status_code=400, detail="Invalid job id") + job_dir = _job_dir_or_400(job_id) job = _get_job(job_id) if not job: raise HTTPException(status_code=404, detail="Job not found") @@ -458,12 +598,20 @@ async def dub_download( else: filtered_tracks = dict(tracks) + filtered_tracks = { + key: { + **value, + "path": _dub_artifact(value.get("path"), job_id, missing_detail="Dubbed track not found"), + } + for key, value in filtered_tracks.items() + } + if not filtered_tracks and not include_original: raise HTTPException(status_code=400, detail="No tracks selected for export") - video_path = job["video_path"] + video_path = _dub_artifact(job["video_path"], job_id, missing_detail="Source video not found") stamp = _unique_stamp() - exports_dir = os.path.join(DUB_DIR, job_id, "exports") + exports_dir = os.path.join(job_dir, "exports") os.makedirs(exports_dir, exist_ok=True) output_path = os.path.join(exports_dir, f"dubbed_video_{stamp}.mp4") ffmpeg = find_ffmpeg() @@ -488,8 +636,7 @@ async def dub_download( # safe_name below). safe_lang = "".join(c for c in lang_code if c.isalnum() or c in "-_") or "track" out_path = os.path.join(exports_dir, f"dubbed_audio_{safe_lang}_{stamp}.{fmt}") - bg = job.get("no_vocals_path") if preserve_bg else None - bg = bg if (bg and os.path.exists(bg)) else None + bg = _optional_dub_artifact(job.get("no_vocals_path"), job_id) if preserve_bg else None cmd = _build_audio_export_cmd(ffmpeg, track_info["path"], bg, out_path, fmt) try: rc, _, stderr = await run_ffmpeg(cmd, timeout=1800.0) @@ -512,6 +659,7 @@ async def dub_download( safe_name = "".join(c for c in base_name if c.isalnum() or c in "-_ ").strip() or "output" dl_name = f"dubbed_{safe_name}_{safe_lang}_{stamp}.{fmt}" media_type = _MEDIA_TYPES.get(f".{fmt}", "audio/mp4") + save_path = _consume_native_save(save_authorization) if save_path: return _native_save(out_path, save_path, dl_name, media_type=media_type) return FileResponse( @@ -612,9 +760,9 @@ async def dub_download( retimed_idx = input_idx input_idx += 1 - bg_audio = job.get("no_vocals_path") if preserve_bg else None + bg_audio = _optional_dub_artifact(job.get("no_vocals_path"), job_id) if preserve_bg else None bg_idx = None - if bg_audio and os.path.exists(bg_audio) and filtered_tracks: + if bg_audio and filtered_tracks: cmd += ["-i", bg_audio] bg_idx = input_idx input_idx += 1 @@ -791,6 +939,7 @@ async def dub_download( if retime_warning is not None: extra_headers["X-Dub-Export-Warning"] = "video-retime-fallback" + save_path = _consume_native_save(save_authorization) if save_path: result = _native_save(output_path, save_path, dl_name, media_type="video/mp4") if retime_warning is not None: @@ -819,12 +968,11 @@ _MEDIA_TYPES = { @router.get("/dub/media/{job_id}") async def dub_get_media(job_id: str): + _job_dir_or_400(job_id) job = _get_job(job_id) if not job: raise HTTPException(status_code=404, detail="Job not found") - video_path = job["video_path"] - if not os.path.exists(video_path): - raise HTTPException(status_code=404, detail="Media file not found") + video_path = _dub_artifact(job["video_path"], job_id, missing_detail="Media file not found") # Pass an explicit media_type. Without this Starlette falls back to # mimetypes.guess_type, which on some platforms returns the wrong # MIME (e.g. "application/octet-stream" for .mkv), and the Tauri @@ -863,8 +1011,8 @@ async def dub_preview_video( # Strict allowlist on the path param BEFORE it reaches any filesystem # path or ffmpeg argv (exports dir, preview/retime work paths) — same # boundary check as dub_download. - if not re.fullmatch(r"[A-Za-z0-9_-]{1,64}", job_id): - raise HTTPException(status_code=400, detail="Invalid job id") + job_dir = _job_dir_or_400(job_id) + lang = _safe_lang_or_400(lang) job = _get_job(job_id) if not job: raise HTTPException(status_code=404, detail="Job not found") @@ -874,25 +1022,19 @@ async def dub_preview_video( if not track_info: raise HTTPException(status_code=404, detail=f"No dubbed track for lang={lang}") - track_path = track_info.get("path") - if not track_path or not os.path.exists(track_path): - raise HTTPException(status_code=404, detail="Dubbed track file missing") + track_path = _dub_artifact(track_info.get("path"), job_id, missing_detail="Dubbed track file missing") - video_path = job.get("video_path") - if not video_path or not os.path.exists(video_path): - raise HTTPException(status_code=404, detail="Source video missing") + video_path = _dub_artifact(job.get("video_path"), job_id, missing_detail="Source video missing") - bg_audio = job.get("no_vocals_path") if preserve_bg else None - has_bg = bool(bg_audio and os.path.exists(bg_audio)) + bg_audio = _optional_dub_artifact(job.get("no_vocals_path"), job_id) if preserve_bg else None + has_bg = bool(bg_audio) - if not _SAFE_LANG.match(lang): - raise HTTPException(status_code=400, detail="Invalid lang") # realpath-normalised + containment-checked inline BEFORE any filesystem # access so the guard dominates every sink (the file's established # pattern — see dub_preview_segment; CodeQL does not track the guard # through a helper's return value). _base = os.path.realpath(DUB_DIR) - exports_dir = os.path.realpath(os.path.join(_base, job_id, "exports")) + exports_dir = os.path.realpath(os.path.join(job_dir, "exports")) if not exports_dir.startswith(_base + os.sep): raise HTTPException(status_code=400, detail="Invalid job id") os.makedirs(exports_dir, exist_ok=True) @@ -1112,16 +1254,16 @@ async def dub_get_onsets(job_id: str): ``onsets.json`` in the job directory; recomputed if the source audio is newer than the cache (e.g. re-ingest into the same job dir). """ - import json + job_dir = _job_dir_or_400(job_id) job = _get_job(job_id) if not job: raise HTTPException(status_code=404, detail="Job not found") - vocals = job.get("vocals_path") - mix = job.get("audio_path") - if vocals and os.path.exists(vocals): + vocals = _optional_dub_artifact(job.get("vocals_path"), job_id) + mix = _optional_dub_artifact(job.get("audio_path"), job_id) + if vocals: src_path, source = vocals, "vocals" - elif mix and os.path.exists(mix): + elif mix: src_path, source = mix, "mix" else: raise HTTPException(status_code=404, detail="No audio track available for onset analysis") @@ -1129,7 +1271,7 @@ async def dub_get_onsets(job_id: str): # Containment inlined (not via _safe_job_path): CodeQL can't track the # sanitizer through a helper's return — the file's established idiom. base = os.path.realpath(DUB_DIR) - cache_path = os.path.realpath(os.path.join(base, job_id, "onsets.json")) + cache_path = os.path.realpath(os.path.join(job_dir, "onsets.json")) if not cache_path.startswith(base + os.sep): raise HTTPException(status_code=400, detail="Invalid job id") try: @@ -1159,31 +1301,31 @@ async def dub_get_onsets(job_id: str): with open(tmp_path, "w", encoding="utf-8") as f: json.dump(payload, f) os.replace(tmp_path, cache_path) - except OSError as e: - logger.warning("onsets cache write failed for %s: %s", job_id, e) + except OSError: + logger.warning("onsets cache write failed") return payload @router.get("/dub/thumb/{job_id}") async def dub_get_thumb(job_id: str): """Serve the extracted dub video thumbnail (jpg). 404 if not generated.""" + job_dir = _job_dir_or_400(job_id) job = _get_job(job_id) if not job: raise HTTPException(status_code=404, detail="Job not found") # Resolve under DUB_DIR to prevent traversal. - thumb = os.path.join(DUB_DIR, job_id, "thumb.jpg") + thumb = os.path.join(job_dir, "thumb.jpg") if not os.path.exists(thumb): raise HTTPException(status_code=404, detail="Thumbnail not available") return FileResponse(thumb, media_type="image/jpeg", headers={"Cache-Control": "public, max-age=3600"}) @router.get("/dub/audio/{job_id}") async def dub_get_audio(job_id: str): + _job_dir_or_400(job_id) job = _get_job(job_id) if not job: raise HTTPException(status_code=404, detail="Job not found") - audio = job.get("audio_path") - if not audio or not os.path.exists(audio): - raise HTTPException(status_code=404, detail="Audio file not found") + audio = _dub_artifact(job.get("audio_path"), job_id, missing_detail="Audio file not found") return FileResponse(audio, media_type="audio/wav") def _seg_wav_candidates(job: dict, lang: "str | None", seg_keys: tuple) -> list: @@ -1205,8 +1347,33 @@ def _seg_wav_candidates(job: dict, lang: "str | None", seg_keys: tuple) -> list: return keys +def _existing_segment_artifact(job_id: str, candidate_ids: list) -> str | None: + """Discover an existing, non-symlink segment WAV inside one job root.""" + job_root = Path(_existing_job_dir_or_404(job_id)) + wanted: list[str] = [] + for value in candidate_ids: + safe = re.sub(r"[^A-Za-z0-9._-]", "_", str(value)) + if safe: + wanted.append(f"seg_{safe}.wav") + try: + entries = { + entry.name: entry + for entry in os.scandir(job_root) + if not entry.is_symlink() and entry.is_file(follow_symlinks=False) + } + except OSError: + return None + for name in wanted: + entry = entries.get(name) + if entry is not None: + return entry.path + return None + + @router.get("/dub/preview/{job_id}/{segment_index}") async def dub_preview_segment(job_id: str, segment_index: int, lang: str = Query(None)): + _job_dir_or_400(job_id) + lang = _safe_lang_or_400(lang) job = _get_job(job_id) if not job: raise HTTPException(status_code=404, detail="Job not found") @@ -1214,16 +1381,12 @@ async def dub_preview_segment(job_id: str, segment_index: int, lang: str = Query # name first (P1.3), then the legacy id/index names for jobs rendered # before per-language (and before id-based, #185) naming. Each candidate # is realpath-normalised and containment-checked BEFORE any filesystem - # access, so the guard dominates every path sink. + # access, and discovery returns only a non-symlink entry from that root. order = job.get("seg_order") or [] seg_id = order[segment_index] if 0 <= segment_index < len(order) else segment_index - base = os.path.realpath(DUB_DIR) - seg_path = None - for _sid in _seg_wav_candidates(job, lang, (seg_id, segment_index)): - cand = os.path.realpath(dub_seg_path(job_id, _sid)) - if cand.startswith(base + os.sep) and os.path.exists(cand): - seg_path = cand - break + seg_path = _existing_segment_artifact( + job_id, _seg_wav_candidates(job, lang, (seg_id, segment_index)) + ) if not seg_path: raise HTTPException(status_code=404, detail="Segment not generated yet") return FileResponse(seg_path, media_type="audio/wav") @@ -1243,19 +1406,18 @@ async def dub_qc_pass(job_id: str, lang: str = Query(None), drift_threshold: flo from services import dub_qc from services.dub_pipeline import put_job, save_job + _job_dir_or_400(job_id) + lang = _safe_lang_or_400(lang) job = _get_job(job_id) if not job: raise HTTPException(status_code=404, detail="Job not found") tracks = job.get("dubbed_tracks", {}) if lang and lang in tracks: - wav_path = tracks[lang]["path"] + wav_path = _dub_artifact(tracks[lang].get("path"), job_id, missing_detail="Dubbed audio file not found") elif tracks: - wav_path = list(tracks.values())[0]["path"] + wav_path = _dub_artifact(list(tracks.values())[0].get("path"), job_id, missing_detail="Dubbed audio file not found") else: raise HTTPException(status_code=400, detail="No dubbed audio track generated yet") - if not os.path.exists(wav_path): - raise HTTPException(status_code=404, detail="Dubbed audio file not found") - segments = job.get("segments") or [] if not segments: raise HTTPException(status_code=400, detail="Job has no segments") @@ -1277,17 +1439,17 @@ async def dub_qc_pass(job_id: str, lang: str = Query(None), drift_threshold: flo return result.get("segments", []), backend.id try: - from services.model_manager import _get_gpu_pool from services.asr_backend import ASRTimeoutError, run_transcribe_guarded + from services.model_manager import _get_gpu_pool recognized, engine_id = await run_transcribe_guarded( _get_gpu_pool(), _recognize, what="QC", ) except ASRTimeoutError as e: # Backend is alive; ASR just couldn't finish in time. 504, not 500/connection. - logger.warning("dub QC ASR pass timed out for %s: %s", job_id, e) + logger.warning("dub QC ASR pass timed out") raise HTTPException(status_code=504, detail=str(e)) except Exception as e: - logger.exception("dub QC ASR pass failed for %s", job_id) + logger.exception("dub QC ASR pass failed") raise HTTPException(status_code=500, detail=f"QC transcription failed: {e}") seg_ids = job.get("seg_order") or [s.get("id", i) for i, s in enumerate(segments)] @@ -1315,9 +1477,9 @@ async def dub_qc_pass(job_id: str, lang: str = Query(None), drift_threshold: flo try: from core import job_store job_store.append_event(job_id, f"data: {payload}\n\n") - except Exception as e: + except Exception: # QC event fan-out is best-effort; the scores are already in the response. - logger.debug("QC event append failed: %s", e) + logger.debug("QC event append failed") return { "engine": engine_id, @@ -1335,31 +1497,36 @@ async def dub_qc_pass(job_id: str, lang: str = Query(None), drift_threshold: flo @router.get("/dub/download-audio/{job_id}") @router.get("/dub/download-audio/{job_id}/{filename}") -async def dub_download_audio(job_id: str, lang: str = Query(None), preserve_bg: bool = Query(True), save_path: str = Query("")): +async def dub_download_audio( + job_id: str, + lang: str = Query(None), + preserve_bg: bool = Query(True), + save_authorization: str = Header("", alias="X-VoiceStudio-Path-Authorization"), +): + job_dir = _existing_job_dir_or_404(job_id) + lang = _safe_lang_or_400(lang) job = _get_job(job_id) if not job: raise HTTPException(status_code=404, detail="Job not found") tracks = job.get("dubbed_tracks", {}) if lang and lang in tracks: - wav_path = tracks[lang]["path"] + wav_path = _dub_artifact(tracks[lang].get("path"), job_id, missing_detail="Audio file not found") elif tracks: - wav_path = list(tracks.values())[0]["path"] + wav_path = _dub_artifact(list(tracks.values())[0].get("path"), job_id, missing_detail="Audio file not found") else: raise HTTPException(status_code=400, detail="No dubbed audio track generated yet") - if not os.path.exists(wav_path): - raise HTTPException(status_code=404, detail="Audio file not found") - lang_label = lang or list(tracks.keys())[0] + _safe_lang_or_400(lang_label) stamp = _unique_stamp() - exports_dir = os.path.join(DUB_DIR, job_id, "exports") + exports_dir = os.path.join(job_dir, "exports") os.makedirs(exports_dir, exist_ok=True) - bg_audio = job.get("no_vocals_path") if preserve_bg else None - if bg_audio and os.path.exists(bg_audio): + bg_audio = _optional_dub_artifact(job.get("no_vocals_path"), job_id) if preserve_bg else None + if bg_audio: ffmpeg = find_ffmpeg() - final_audio_path = os.path.join(exports_dir, f"mixed_dub_{lang_label}_{stamp}.wav") + final_audio_path = os.path.join(exports_dir, f"mixed_dub_{stamp}.wav") cmd = [ ffmpeg, "-i", bg_audio, "-i", wav_path, "-filter_complex", bed_mix_filter("0:a", "1:a"), @@ -1372,13 +1539,14 @@ async def dub_download_audio(job_id: str, lang: str = Query(None), preserve_bg: if not os.path.exists(final_audio_path) or os.path.getsize(final_audio_path) == 0: raise Exception("ffmpeg mix produced no output file") wav_path = final_audio_path - logger.info("Dub audio mix wrote %s (%d bytes)", final_audio_path, os.path.getsize(final_audio_path)) + logger.info("Dub audio mix completed") except Exception: logger.exception("Failed to mix audio") base_name = os.path.splitext(job.get('filename', 'audio'))[0] safe_name = ''.join(c for c in base_name if c.isalnum() or c in '-_ ').strip() or 'audio' dl_name = f"dubbed_audio_{lang_label}_{safe_name}_{stamp}.wav" + save_path = _consume_native_save(save_authorization) if save_path: return _native_save(wav_path, save_path, dl_name, media_type="audio/wav") return FileResponse( @@ -1435,6 +1603,8 @@ async def dub_export_srt( dual: bool = False, lang: str = Query(None, description="Track language code. Emits that track's text (segments_i18n) when the job carries it; when that track was generated under Smart Fit or stretch_video, cue times come from the fitted timeline."), ): + _job_dir_or_400(job_id) + lang = _safe_lang_or_400(lang) job = _get_job(job_id) if not job: raise HTTPException(status_code=404, detail="Job not found") @@ -1487,6 +1657,8 @@ async def dub_export_vtt( dual: bool = False, lang: str = Query(None, description="Track language code. Emits that track's text (segments_i18n) when the job carries it; when that track was generated under Smart Fit or stretch_video, cue times come from the fitted timeline."), ): + _job_dir_or_400(job_id) + lang = _safe_lang_or_400(lang) job = _get_job(job_id) if not job: raise HTTPException(status_code=404, detail="Job not found") @@ -1524,6 +1696,8 @@ async def dub_export_vtt( @router.get("/dub/export-segments/{job_id}") async def dub_export_segments_zip(job_id: str, lang: str = Query(None)): import zipfile + _job_dir_or_400(job_id) + lang = _safe_lang_or_400(lang) job = _get_job(job_id) if not job: raise HTTPException(status_code=404, detail="Job not found") @@ -1534,17 +1708,13 @@ async def dub_export_segments_zip(job_id: str, lang: str = Query(None)): zip_buffer = io.BytesIO() order = job.get("seg_order") or [] - base = os.path.realpath(DUB_DIR) with zipfile.ZipFile(zip_buffer, "w", zipfile.ZIP_DEFLATED) as zf: for i, seg in enumerate(segments): seg_id = order[i] if i < len(order) else i - # realpath + containment guard before any filesystem access. - seg_path = None - for _sid in _seg_wav_candidates(job, lang, (seg_id, i)): - cand = os.path.realpath(dub_seg_path(job_id, _sid)) - if cand.startswith(base + os.sep) and os.path.exists(cand): - seg_path = cand - break + # Discovery returns only a non-symlink entry from the validated job root. + seg_path = _existing_segment_artifact( + job_id, _seg_wav_candidates(job, lang, (seg_id, i)) + ) if seg_path: speaker = seg.get("speaker_id", "Speaker1").replace(" ", "") start_str = f"{seg['start']:.2f}" @@ -1563,32 +1733,38 @@ async def dub_export_segments_zip(job_id: str, lang: str = Query(None)): @router.get("/dub/download-mp3/{job_id}") @router.get("/dub/download-mp3/{job_id}/{filename}") -async def dub_download_mp3(job_id: str, lang: str = Query(None), preserve_bg: bool = Query(True), save_path: str = Query(""), bitrate: str = Query("192k")): +async def dub_download_mp3( + job_id: str, + lang: str = Query(None), + preserve_bg: bool = Query(True), + save_authorization: str = Header("", alias="X-VoiceStudio-Path-Authorization"), + bitrate: str = Query("192k"), +): + job_dir = _existing_job_dir_or_404(job_id) + lang = _safe_lang_or_400(lang) job = _get_job(job_id) if not job: raise HTTPException(status_code=404, detail="Job not found") tracks = job.get("dubbed_tracks", {}) if lang and lang in tracks: - wav_path = tracks[lang]["path"] + wav_path = _dub_artifact(tracks[lang].get("path"), job_id, missing_detail="Audio file not found") elif tracks: - wav_path = list(tracks.values())[0]["path"] + wav_path = _dub_artifact(list(tracks.values())[0].get("path"), job_id, missing_detail="Audio file not found") else: raise HTTPException(status_code=400, detail="No dubbed audio track generated yet") - if not os.path.exists(wav_path): - raise HTTPException(status_code=404, detail="Audio file not found") - lang_label = lang or list(tracks.keys())[0] + _safe_lang_or_400(lang_label) ffmpeg = find_ffmpeg() stamp = _unique_stamp() - exports_dir = os.path.join(DUB_DIR, job_id, "exports") + exports_dir = os.path.join(job_dir, "exports") os.makedirs(exports_dir, exist_ok=True) source_path = wav_path - bg_audio = job.get("no_vocals_path") if preserve_bg else None - if bg_audio and os.path.exists(bg_audio): - mixed_path = os.path.join(exports_dir, f"mixed_mp3_{lang_label}_{stamp}.wav") + bg_audio = _optional_dub_artifact(job.get("no_vocals_path"), job_id) if preserve_bg else None + if bg_audio: + mixed_path = os.path.join(exports_dir, f"mixed_mp3_{stamp}.wav") cmd_mix = [ ffmpeg, "-i", bg_audio, "-i", wav_path, "-filter_complex", bed_mix_filter("0:a", "1:a"), @@ -1601,7 +1777,7 @@ async def dub_download_mp3(job_id: str, lang: str = Query(None), preserve_bg: bo except Exception: logger.exception("Failed to mix audio for MP3") - mp3_path = os.path.join(exports_dir, f"dubbed_{lang_label}_{stamp}.mp3") + mp3_path = os.path.join(exports_dir, f"dubbed_{stamp}.mp3") # Accept '128', '192k' etc. — normalize to ffmpeg's 'Nk' form and clamp # to a sensible range so a malformed value can't stall encoding. _br = str(bitrate or "192k").lower().rstrip("k") or "192" @@ -1629,11 +1805,12 @@ async def dub_download_mp3(job_id: str, lang: str = Query(None), preserve_bg: bo if not os.path.exists(mp3_path) or os.path.getsize(mp3_path) == 0: raise HTTPException(status_code=500, detail="MP3 encoding produced no output file") - logger.info("Dub MP3 encoded %s (%d bytes)", mp3_path, os.path.getsize(mp3_path)) + logger.info("Dub MP3 encoding completed") base_name = os.path.splitext(job.get('filename', 'audio'))[0] safe_name = ''.join(c for c in base_name if c.isalnum() or c in '-_ ').strip() or 'audio' dl_name = f"dubbed_{lang_label}_{safe_name}_{stamp}.mp3" + save_path = _consume_native_save(save_authorization) if save_path: return _native_save(mp3_path, save_path, dl_name, media_type="audio/mpeg") return FileResponse( @@ -1644,6 +1821,8 @@ async def dub_download_mp3(job_id: str, lang: str = Query(None), preserve_bg: bo @router.get("/dub/export-stems/{job_id}") async def dub_export_stems(job_id: str, lang: str = Query(None)): import zipfile + _job_dir_or_400(job_id) + lang = _safe_lang_or_400(lang) job = _get_job(job_id) if not job: raise HTTPException(status_code=404, detail="Job not found") @@ -1653,22 +1832,22 @@ async def dub_export_stems(job_id: str, lang: str = Query(None)): raise HTTPException(status_code=400, detail="No dubbed tracks generated yet") if lang and lang in tracks: - vocals_path = tracks[lang]["path"] + vocals_path = _dub_artifact(tracks[lang].get("path"), job_id, missing_detail="Dubbed audio file not found") lang_label = lang elif tracks: first_key = list(tracks.keys())[0] - vocals_path = tracks[first_key]["path"] + _safe_lang_or_400(first_key) + vocals_path = _dub_artifact(tracks[first_key].get("path"), job_id, missing_detail="Dubbed audio file not found") lang_label = first_key else: raise HTTPException(status_code=400, detail="No dubbed audio track") - bg_path = job.get("no_vocals_path") + bg_path = _optional_dub_artifact(job.get("no_vocals_path"), job_id) zip_buffer = io.BytesIO() with zipfile.ZipFile(zip_buffer, "w", zipfile.ZIP_DEFLATED) as zf: - if os.path.exists(vocals_path): - zf.write(vocals_path, f"vocals_dubbed_{lang_label}.wav") - if bg_path and os.path.exists(bg_path): + zf.write(vocals_path, f"vocals_dubbed_{lang_label}.wav") + if bg_path: zf.write(bg_path, "background_original.wav") zip_buffer.seek(0) diff --git a/backend/api/routers/exports.py b/backend/api/routers/exports.py index fde7e9ef..75bee35f 100644 --- a/backend/api/routers/exports.py +++ b/backend/api/routers/exports.py @@ -4,34 +4,28 @@ import time import shutil import subprocess import platform -from fastapi import APIRouter, HTTPException +from fastapi import APIRouter, Depends, HTTPException +from api.dependencies import require_native_access from core.db import db_conn -from core.config import OUTPUTS_DIR +from core.config import DATA_DIR, OUTPUTS_DIR from core import event_bus +from core.path_authorization import PathAuthorizationError, consume +from core.path_security import UnsafePath, resolve_within, safe_filename from schemas.requests import ExportRequest, ExportRecordRequest, RevealRequest router = APIRouter() -def _safe_destination(raw: str) -> str: - """Resolve + validate an export destination. Rejects relative/empty paths.""" - if not raw or not raw.strip(): - raise HTTPException( - status_code=400, - detail="Export needs a destination folder. Pick where the file should go and try again.", - ) - expanded = os.path.expanduser(raw) - # Check BEFORE realpath(): realpath absolutizes a relative path against - # the server's cwd, which made this check dead code — a relative - # destination silently exported to a cwd-dependent location instead of - # the documented 400 (regression-tested in tests/test_exports_api.py). - if not os.path.isabs(expanded): - raise HTTPException( - status_code=400, - detail="The destination needs to be a full path (e.g. /Users/you/Movies/VoiceStudio) — not relative.", - ) - dest = os.path.realpath(expanded) +def _authorized_destination(token: str) -> str: + """Consume a native save-dialog capability and validate its destination.""" + try: + raw = consume(token, "dub_export") + except PathAuthorizationError as exc: + raise HTTPException(status_code=403, detail=str(exc)) from exc + if not raw or not raw.strip() or not os.path.isabs(os.path.expanduser(raw)): + raise HTTPException(status_code=400, detail="The selected destination is invalid.") + dest = os.path.realpath(os.path.expanduser(raw)) parent = os.path.dirname(dest) if not parent or not os.path.isdir(parent): raise HTTPException( @@ -43,30 +37,30 @@ def _safe_destination(raw: str) -> str: def _safe_source(filename: str) -> str: """Resolve a source filename against OUTPUTS_DIR / dub outputs, blocking traversal.""" - base = os.path.basename(filename or "") - # "." and ".." are their own basename, so they'd slip past the - # base != filename check and only die later on realpath containment — - # reject them up front with the same 400 as any other malformed name. - if not base or base != filename or base in (".", ".."): + try: + base = safe_filename(filename) + except UnsafePath as exc: raise HTTPException( status_code=400, detail="The file to export has an unexpected name. Try re-generating the audio and exporting again.", - ) + ) from exc for root in (OUTPUTS_DIR, os.path.join("dub", "outputs")): - candidate = os.path.realpath(os.path.join(root, base)) - root_real = os.path.realpath(root) - if candidate.startswith(root_real + os.sep) and os.path.exists(candidate): - return candidate + try: + candidate = resolve_within(root, base) + except UnsafePath: + continue + if candidate.is_file(): + return str(candidate) raise HTTPException( status_code=404, detail="That file isn't on disk anymore — it may have been cleaned up. Regenerate and try again.", ) -@router.post("/export") +@router.post("/export", dependencies=[Depends(require_native_access)]) def export_file(req: ExportRequest): src = _safe_source(req.source_filename) - dest = _safe_destination(req.destination_path) + dest = _authorized_destination(req.authorization) try: # Video exports: overlay VoiceStudio logo if visible watermark is enabled if src.lower().endswith(".mp4"): @@ -126,31 +120,38 @@ def get_export_history(): return [dict(r) for r in rows] -@router.post("/export/reveal") +@router.post("/export/reveal", dependencies=[Depends(require_native_access)]) def reveal_in_folder(req: RevealRequest): - # Tauri/native dialog-provided path; subprocess uses list args (no shell interpolation). + # Desktop clients reveal arbitrary user-selected export destinations in + # the native Tauri process. This HTTP fallback is deliberately limited to + # server-owned data so a remote/browser caller cannot make the host open + # an attacker-chosen path. if not req.path or not req.path.strip(): raise HTTPException( status_code=400, detail="No path was provided — nothing to reveal.", ) - target = os.path.realpath(os.path.expanduser(req.path)) - if not os.path.exists(target): + try: + target_path = resolve_within(DATA_DIR, req.path) + except UnsafePath as exc: + raise HTTPException(status_code=403, detail="That path cannot be opened remotely.") from exc + if not target_path.exists(): raise HTTPException( status_code=404, detail="That file or folder is no longer on disk. It may have been moved or deleted.", ) - folder = target if os.path.isdir(target) else os.path.dirname(target) + target = str(target_path) + folder = target if target_path.is_dir() else str(target_path.parent) system = platform.system() try: if system == "Darwin": - if os.path.isfile(target): + if target_path.is_file(): subprocess.Popen(["open", "-R", target]) else: subprocess.Popen(["open", folder]) elif system == "Windows": - if os.path.isfile(target): + if target_path.is_file(): subprocess.Popen(["explorer", "/select,", target.replace("/", "\\")]) else: subprocess.Popen(["explorer", folder.replace("/", "\\")]) diff --git a/backend/api/routers/gallery.py b/backend/api/routers/gallery.py index dcab06f9..217914a6 100644 --- a/backend/api/routers/gallery.py +++ b/backend/api/routers/gallery.py @@ -13,6 +13,7 @@ from pydantic import BaseModel from core.db import db_conn from core.config import VOICES_DIR, OUTPUTS_DIR from core import event_bus +from core.file_cleanup import FileCleanupError, unlink_if_present from services.ffmpeg_utils import spawn_subprocess logger = logging.getLogger("omnivoice.gallery") @@ -139,11 +140,14 @@ def delete_voice(voice_id: str): raise HTTPException(status_code=404, detail="Voice not found") audio_path = row["audio_path"] - if audio_path and os.path.exists(audio_path): + if audio_path: try: - os.remove(audio_path) - except Exception: - pass + unlink_if_present(audio_path) + except FileCleanupError as exc: + raise HTTPException( + status_code=500, + detail="Could not delete the voice audio file. Close any app using it and retry.", + ) from exc conn.execute("DELETE FROM voice_gallery WHERE id = ?", (voice_id,)) return {"success": True} @@ -478,19 +482,22 @@ def batch_delete_voices(body: dict): return {"deleted": 0} deleted = 0 + failed = 0 with db_conn() as conn: for vid in ids: row = conn.execute("SELECT audio_path FROM voice_gallery WHERE id = ?", (vid,)).fetchone() if row: audio_path = row["audio_path"] - if audio_path and os.path.exists(audio_path): + if audio_path: try: - os.remove(audio_path) - except Exception: - pass + unlink_if_present(audio_path) + except FileCleanupError: + logger.warning("Voice audio cleanup failed for a gallery item") + failed += 1 + continue conn.execute("DELETE FROM voice_gallery WHERE id = ?", (vid,)) deleted += 1 - return {"deleted": deleted} + return {"deleted": deleted, "failed": failed} @router.post("/gallery/voices/{voice_id}/to-profile") @@ -526,4 +533,3 @@ def voice_to_profile(voice_id: str): event_bus.emit("profiles", {"action": "created", "id": profile_id}) return {"success": True, "profile_id": profile_id, "name": voice["name"]} - diff --git a/backend/api/routers/marketplace.py b/backend/api/routers/marketplace.py index b99131da..1af53347 100644 --- a/backend/api/routers/marketplace.py +++ b/backend/api/routers/marketplace.py @@ -40,6 +40,7 @@ from core.db import db_conn from core import event_bus from core.version import APP_VERSION from core.http_headers import content_disposition +from core.path_security import UnsafePath, resolve_within, safe_filename logger = logging.getLogger("omnivoice.marketplace") @@ -56,6 +57,26 @@ BUNDLE_VERSION = 1 MAX_BUNDLE_BYTES = 100 * 1024 * 1024 +def _contained_path(root, value, *, detail="Invalid file path") -> Path: + try: + return resolve_within(root, value) + except UnsafePath as exc: + raise HTTPException(status_code=400, detail=detail) from exc + + +def _voice_asset(value) -> Path | None: + """Resolve a DB-stored voice asset without trusting the database value.""" + if not value: + return None + try: + resolved = resolve_within(VOICES_DIR, value) + except UnsafePath as exc: + raise HTTPException(status_code=400, detail="Voice profile contains an invalid asset path") from exc + if not resolved.is_file(): + raise HTTPException(status_code=400, detail="Voice profile reference audio is missing") + return resolved + + # ── Export ────────────────────────────────────────────────────────────────── @@ -109,18 +130,18 @@ def export_profile(profile_id: str): # Reference audio ref_path = profile.get("ref_audio_path") if ref_path: - full_ref = os.path.join(VOICES_DIR, ref_path) - if os.path.isfile(full_ref): + full_ref = _voice_asset(ref_path) + if full_ref and full_ref.is_file(): ext = os.path.splitext(ref_path)[1] or ".wav" - zf.write(full_ref, f"ref_audio{ext}") + zf.write(str(full_ref), f"ref_audio{ext}") # Locked audio (if profile is locked) locked_path = profile.get("locked_audio_path") if locked_path: - full_locked = os.path.join(VOICES_DIR, locked_path) - if os.path.isfile(full_locked): + full_locked = _voice_asset(locked_path) + if full_locked and full_locked.is_file(): ext = os.path.splitext(locked_path)[1] or ".wav" - zf.write(full_locked, f"locked_audio{ext}") + zf.write(str(full_locked), f"locked_audio{ext}") buf.seek(0) safe_name = "".join( @@ -270,7 +291,11 @@ def publish_to_marketplace( safe_name = "".join( c if c.isalnum() or c in "-_ " else "" for c in profile.get("name", "voice") ).strip().replace(" ", "_")[:40] - bundle_path = MARKETPLACE_DIR / f"{safe_name}_{profile_id}.omnivoice" + bundle_path = _contained_path( + MARKETPLACE_DIR, + f"{safe_name}_{profile_id}.omnivoice", + detail="Invalid profile id", + ) # Build the bundle with zipfile.ZipFile(str(bundle_path), "w", zipfile.ZIP_DEFLATED) as zf: @@ -283,17 +308,17 @@ def publish_to_marketplace( ref_path = profile.get("ref_audio_path") if ref_path: - full_ref = os.path.join(VOICES_DIR, ref_path) - if os.path.isfile(full_ref): + full_ref = _voice_asset(ref_path) + if full_ref and full_ref.is_file(): ext = os.path.splitext(ref_path)[1] or ".wav" - zf.write(full_ref, f"ref_audio{ext}") + zf.write(str(full_ref), f"ref_audio{ext}") locked_path = profile.get("locked_audio_path") if locked_path: - full_locked = os.path.join(VOICES_DIR, locked_path) - if os.path.isfile(full_locked): + full_locked = _voice_asset(locked_path) + if full_locked and full_locked.is_file(): ext = os.path.splitext(locked_path)[1] or ".wav" - zf.write(full_locked, f"locked_audio{ext}") + zf.write(str(full_locked), f"locked_audio{ext}") logger.info("Published voice %r to marketplace: %s", profile.get("name"), bundle_path) return { @@ -353,7 +378,13 @@ def browse_marketplace( @router.post("/install/{filename}") async def install_from_marketplace(filename: str): """Import a voice profile from a bundle in the local marketplace directory.""" - bundle_path = MARKETPLACE_DIR / filename + try: + filename = safe_filename(filename) + except UnsafePath as exc: + raise HTTPException(status_code=400, detail="Invalid bundle filename") from exc + if not filename.endswith(".omnivoice"): + raise HTTPException(status_code=400, detail="Invalid bundle filename") + bundle_path = _contained_path(MARKETPLACE_DIR, filename, detail="Invalid bundle filename") if not bundle_path.is_file(): raise HTTPException(status_code=404, detail=f"Bundle not found: {filename}") @@ -426,7 +457,13 @@ async def install_from_marketplace(filename: str): @router.delete("/{filename}") def remove_from_marketplace(filename: str): """Remove a bundle from the local marketplace directory.""" - bundle_path = MARKETPLACE_DIR / filename + try: + filename = safe_filename(filename) + except UnsafePath as exc: + raise HTTPException(status_code=400, detail="Invalid bundle filename") from exc + if not filename.endswith(".omnivoice"): + raise HTTPException(status_code=400, detail="Invalid bundle filename") + bundle_path = _contained_path(MARKETPLACE_DIR, filename, detail="Invalid bundle filename") if not bundle_path.is_file(): raise HTTPException(status_code=404, detail=f"Bundle not found: {filename}") try: diff --git a/backend/api/routers/profiles.py b/backend/api/routers/profiles.py index b27b8f57..af563a45 100644 --- a/backend/api/routers/profiles.py +++ b/backend/api/routers/profiles.py @@ -13,6 +13,7 @@ from core.config import VOICES_DIR, OUTPUTS_DIR from core import event_bus from core.personalities import get_personalities from omnivoice.utils.voice_design import heal_design_instruct, sanitize_instruct +from core.path_security import UnsafePath, resolve_within router = APIRouter() @@ -377,13 +378,18 @@ async def lock_profile( if not history or not history["audio_path"]: raise HTTPException(status_code=404, detail="History item not found or has no audio") - src_path = os.path.join(OUTPUTS_DIR, history["audio_path"]) - if not os.path.exists(src_path): + try: + src_path = resolve_within(OUTPUTS_DIR, history["audio_path"]) + except UnsafePath as exc: + raise HTTPException(status_code=400, detail="Invalid history audio path") from exc + if not src_path.is_file(): raise HTTPException(status_code=404, detail="Audio file not found on disk") locked_filename = f"{profile_id}_locked.wav" - locked_path = os.path.join(VOICES_DIR, locked_filename) - shutil.copy2(src_path, locked_path) + locked_path = _voices_path(locked_filename) + if locked_path is None: + raise HTTPException(status_code=400, detail="Invalid profile id") + shutil.copy2(str(src_path), locked_path) ref_text = history["text"][:100] if history["text"] else "" @@ -405,8 +411,8 @@ async def unlock_profile(profile_id: str): ) if profile["locked_audio_path"]: - locked_path = os.path.join(VOICES_DIR, profile["locked_audio_path"]) - if os.path.exists(locked_path): + locked_path = _voices_path(profile["locked_audio_path"]) + if locked_path and os.path.exists(locked_path): os.remove(locked_path) conn.execute( diff --git a/backend/api/routers/sonitranslate.py b/backend/api/routers/sonitranslate.py index 658490a8..cdaedeec 100644 --- a/backend/api/routers/sonitranslate.py +++ b/backend/api/routers/sonitranslate.py @@ -5,10 +5,11 @@ SoniTranslate sidecar integration. """ import logging -from fastapi import APIRouter, HTTPException -from pydantic import BaseModel -from typing import Optional +from fastapi import APIRouter, Depends, HTTPException +from pydantic import BaseModel + +from api.dependencies import require_native_access from services import sonitranslate as soni router = APIRouter(prefix="/engines/sonitranslate", tags=["SoniTranslate"]) @@ -63,15 +64,15 @@ async def sonitranslate_stop(): class DubRequest(BaseModel): - video_path: str + video_authorization: str target_language: str = "Spanish (es)" source_language: str = "Automatic detection" tts_voice: str = "es-ES-AlvaroNeural-Male" max_speakers: int = 1 - output_dir: Optional[str] = None + output_authorization: str | None = None -@router.post("/dub") +@router.post("/dub", dependencies=[Depends(require_native_access)]) async def sonitranslate_dub(body: DubRequest): """Run full dubbing pipeline via SoniTranslate. @@ -89,15 +90,28 @@ async def sonitranslate_dub(body: DubRequest): AudioSeal provenance mark that every built-in synthesis path carries. """ try: + from core.path_authorization import PathAuthorizationError, consume + + try: + video_path = consume(body.video_authorization, "soni_input") + output_dir = ( + consume(body.output_authorization, "soni_output_dir") + if body.output_authorization + else None + ) + except PathAuthorizationError as exc: + raise HTTPException(status_code=403, detail=str(exc)) from exc result = await soni.dub_video( - video_path=body.video_path, + video_path=video_path, target_language=body.target_language, source_language=body.source_language, tts_voice=body.tts_voice, max_speakers=body.max_speakers, - output_dir=body.output_dir, + output_dir=output_dir, ) return result + except HTTPException: + raise except Exception as e: logger.exception("SoniTranslate dub failed") raise HTTPException(status_code=500, detail=str(e)) diff --git a/backend/api/routers/system.py b/backend/api/routers/system.py index c0ed7f41..2d1df597 100644 --- a/backend/api/routers/system.py +++ b/backend/api/routers/system.py @@ -476,14 +476,20 @@ def _truncate_file(path: str): async def clear_tauri_logs(): """Truncate whichever Tauri-side log files we know about. OS-level rotation may recreate them.""" cleared = [] + failed = 0 for p in _tauri_log_candidates(): if os.path.exists(p): try: await asyncio.to_thread(_truncate_file, p) cleared.append(p) - except Exception: - pass - return {"cleared": cleared} + except OSError: + failed += 1 + if failed: + raise HTTPException( + status_code=500, + detail="One or more desktop log files could not be cleared. Close any app using them and retry.", + ) + return {"cleared": cleared, "failed": 0} @router.get("/sysinfo", response_model=SysinfoResponse) def get_sys_info(): diff --git a/backend/api/routers/tools.py b/backend/api/routers/tools.py index 70001957..6f706b42 100644 --- a/backend/api/routers/tools.py +++ b/backend/api/routers/tools.py @@ -21,13 +21,16 @@ import asyncio import json import logging import os +import re from typing import Optional -from fastapi import APIRouter, HTTPException +from fastapi import APIRouter, Depends, HTTPException from pydantic import BaseModel, Field from services import director, speech_rate, incremental from services.ffmpeg_utils import find_ffprobe, spawn_subprocess +from api.dependencies import require_native_access +from core.path_security import UnsafePath, resolve_within logger = logging.getLogger("omnivoice.tools") router = APIRouter() @@ -40,7 +43,7 @@ class ProbeReq(BaseModel): path: str -@router.post("/tools/probe") +@router.post("/tools/probe", dependencies=[Depends(require_native_access)]) async def probe(req: ProbeReq): target = os.path.realpath(os.path.expanduser(req.path)) if not os.path.exists(target): @@ -174,18 +177,26 @@ async def analyse_video_context(job_id: str): from core.config import DUB_DIR from services.video_context import analyse_video + if not re.fullmatch(r"[A-Za-z0-9_-]{1,64}", job_id or ""): + raise HTTPException(status_code=400, detail="Invalid job id") + try: + job_dir = resolve_within(DUB_DIR, job_id) + except UnsafePath as exc: + raise HTTPException(status_code=400, detail="Invalid job id") from exc job = _get_job(job_id) if not job: - from fastapi import HTTPException raise HTTPException(status_code=404, detail="Job not found") - video_path = os.path.join(DUB_DIR, job_id, "source.mp4") - if not os.path.exists(video_path): - video_path = job.get("video_path", "") + video_path = resolve_within(DUB_DIR, job_dir / "source.mp4") + if not video_path.is_file(): + try: + video_path = resolve_within(DUB_DIR, job.get("video_path", "")) + except UnsafePath: + return {"error": "Source video not found", "segments": {}} - if not video_path or not os.path.exists(video_path): + if not video_path.is_file(): return {"error": "Source video not found", "segments": {}} segments = job.get("segments") or [] - ctx = await analyse_video(video_path, segments) + ctx = await analyse_video(str(video_path), segments) return ctx.to_dict() diff --git a/backend/core/file_cleanup.py b/backend/core/file_cleanup.py new file mode 100644 index 00000000..37efab1e --- /dev/null +++ b/backend/core/file_cleanup.py @@ -0,0 +1,24 @@ +"""Reliable file deletion for user-visible destructive operations.""" +from __future__ import annotations + +import os + + +class FileCleanupError(OSError): + """A requested file exists but could not be removed.""" + + +def unlink_if_present(path: str | os.PathLike[str]) -> bool: + """Delete *path*, returning whether it existed. + + Missing files make delete operations idempotent. Other failures must reach + the caller so it cannot discard the only record from which cleanup can be + retried. + """ + try: + os.unlink(path) + except FileNotFoundError: + return False + except OSError as exc: + raise FileCleanupError("file cleanup failed") from exc + return True diff --git a/backend/core/path_authorization.py b/backend/core/path_authorization.py index 86d88620..46a71e24 100644 --- a/backend/core/path_authorization.py +++ b/backend/core/path_authorization.py @@ -15,7 +15,14 @@ import stat from core.config import DATA_DIR _TOKEN_RE = re.compile(r"[0-9a-f]{64}\Z") -_KINDS = {"models_dir", "ffmpeg", "ffprobe"} +_KINDS = { + "models_dir", + "ffmpeg", + "ffprobe", + "dub_export", + "soni_input", + "soni_output_dir", +} _AUTH_DIR = os.path.join(DATA_DIR, ".path-authorizations") diff --git a/backend/core/path_security.py b/backend/core/path_security.py new file mode 100644 index 00000000..b821cbe1 --- /dev/null +++ b/backend/core/path_security.py @@ -0,0 +1,79 @@ +"""Filesystem trust-boundary helpers. + +Paths persisted in SQLite are still untrusted: older clients and imported job +records can contain absolute paths, traversal components, or symlink escapes. +Keep containment checks at the filesystem boundary instead of relying on the +route or database layer to have sanitised a value earlier. +""" + +from __future__ import annotations + +import ntpath +import os +from pathlib import Path + + +class UnsafePath(ValueError): + """Raised when a path crosses its allowed filesystem boundary.""" + + +def safe_filename(value: object) -> str: + """Return a portable bare filename, rejecting traversal and drive paths.""" + name = str(value or "") + if ( + not name + or name in {".", ".."} + or "/" in name + or "\\" in name + or os.path.isabs(name) + or ntpath.isabs(name) + or ntpath.basename(name) != name + ): + raise UnsafePath("expected a bare filename") + return name + + +def resolve_within(root: os.PathLike[str] | str, value: os.PathLike[str] | str) -> Path: + """Resolve *value* beneath *root*, rejecting traversal and symlink escapes. + + Absolute values are accepted only when they already resolve inside the + root. This preserves existing database rows, which historically stored a + mixture of relative filenames and absolute job-artifact paths. + """ + raw = os.fspath(value) if value is not None else "" + if not isinstance(raw, str) or not raw: + raise UnsafePath("path is empty") + # Treat both separator families as structural on every host. Otherwise a + # Windows traversal string is an innocent-looking filename when validated + # on Linux (and can become dangerous after persisted data is moved). + if os.sep != "\\" and ("\\" in raw or bool(ntpath.splitdrive(raw)[0])): + raise UnsafePath("path uses a foreign separator or drive") + root_path = Path(root).expanduser().resolve(strict=False) + root_text = str(root_path) + if os.path.isabs(raw): + prefix = root_text.rstrip(os.sep) + os.sep + if not os.path.normcase(raw).startswith(os.path.normcase(prefix)): + raise UnsafePath("path escapes its allowed root") + raw = raw[len(prefix):] + + # Rebuild from individually sanitized basenames. Besides making the + # containment proof explicit to static analysis, this rejects empty, + # dot, parent, drive, and separator-bearing components before Path sees + # any persisted/request-derived string. + parts = raw.split(os.sep) + clean_parts: list[str] = [] + for part in parts: + clean = os.path.basename(part) + if not clean or clean in {".", ".."} or clean != part: + raise UnsafePath("path contains an unsafe component") + clean_parts.append(clean) + candidate = root_path.joinpath(*clean_parts) + resolved = candidate.resolve(strict=False) + try: + if os.path.commonpath((str(root_path), str(resolved))) != str(root_path): + raise UnsafePath("path escapes its allowed root") + except ValueError as exc: # Windows paths on different drives + raise UnsafePath("path escapes its allowed root") from exc + if resolved == root_path: + raise UnsafePath("path must name an item below its allowed root") + return resolved diff --git a/backend/core/scrub.py b/backend/core/scrub.py index 7b3b3d68..152198b2 100644 --- a/backend/core/scrub.py +++ b/backend/core/scrub.py @@ -91,9 +91,9 @@ def _env_secret_values() -> list[str]: def scrub_text(text: str | None) -> str: """Return ``text`` with secrets and home paths redacted. - Never raises — scrubbing failure must not block a bug report, and a - partially-scrubbed string is still better than an unscrubbed one, so - each pass is independent. + Never raises. If a redaction pass itself fails, return only the redaction + marker: diagnostic detail is less important than keeping credentials and + private paths out of a report. """ if not text: return "" if text is None else str(text) @@ -104,18 +104,18 @@ def scrub_text(text: str | None) -> str: for val in _env_secret_values(): s = s.replace(val, REDACTED) except Exception: - pass + return REDACTED # 2. Credential-shaped substrings + URL query secrets. for pat in _TOKEN_PATTERNS: try: s = pat.sub(REDACTED, s) except Exception: - pass + return REDACTED try: s = _URL_SECRET_RE.sub(lambda m: m.group(1) + REDACTED, s) except Exception: - pass + return REDACTED # 3. This process's real home dir (covers symlinked/nonstandard homes # the generic patterns miss), then the per-OS shapes. Boundary-aware so @@ -126,12 +126,12 @@ def scrub_text(text: str | None) -> str: if home and home not in ("/", "~"): s = re.sub(re.escape(home) + r"(?=[/\\\s\"']|$)", "~", s) except Exception: - pass + return REDACTED for pat in _HOME_PATTERNS: try: s = pat.sub("~", s) except Exception: - pass + return REDACTED return s @@ -153,5 +153,5 @@ def scrub_provider_error(detail: object, api_key: str | None = None) -> str: if api_key and api_key != "local" and len(api_key) >= _MIN_SECRET_LEN: s = s.replace(api_key, REDACTED) except Exception: - pass + return REDACTED return scrub_text(s) diff --git a/backend/schemas/requests.py b/backend/schemas/requests.py index 8d501b8a..020bc64e 100644 --- a/backend/schemas/requests.py +++ b/backend/schemas/requests.py @@ -5,7 +5,7 @@ from services.audio_dsp import EFFECT_PRESETS class ExportRequest(BaseModel): source_filename: str - destination_path: str + authorization: str mode: str = "history" class ExportRecordRequest(BaseModel): diff --git a/backend/services/ffmpeg_utils.py b/backend/services/ffmpeg_utils.py index dad6e8a0..9cc59119 100644 --- a/backend/services/ffmpeg_utils.py +++ b/backend/services/ffmpeg_utils.py @@ -9,6 +9,7 @@ import sys # Leaf module (stdlib-only) — safe to import at module top, unlike # services.dub_pipeline which imports this module and would cycle. from services.proc_registry import register_proc, unregister_proc +from core.path_security import UnsafePath, resolve_within logger = logging.getLogger("omnivoice.api") @@ -550,21 +551,25 @@ async def _pitch_preserving_stretch(wav, target_samples: int, sr: int): return torch.from_numpy(out_arr.copy()).unsqueeze(0).to(wav.device) -async def probe_duration(path: str) -> float | None: +async def probe_duration(path: str, *, allowed_root: str) -> float | None: """Return a media file's duration in seconds via ffprobe, or None. Used by the Smart Fit pipeline to sanity-check source/track lengths without loading the media. Never raises — probing is best-effort. """ ffprobe = find_ffprobe() - if not ffprobe or not os.path.isfile(path): + try: + media_path = resolve_within(allowed_root, path) + except UnsafePath: + return None + if not ffprobe or not media_path.is_file(): return None try: proc = await spawn_subprocess( ffprobe, "-v", "error", "-show_entries", "format=duration", "-of", "default=noprint_wrappers=1:nokey=1", - path, + str(media_path), stdout=asyncio.subprocess.PIPE, stderr=asyncio.subprocess.PIPE, ) @@ -573,7 +578,7 @@ async def probe_duration(path: str) -> float | None: return None return float(stdout.decode().strip()) except Exception as e: - logger.debug("probe_duration failed for %s: %s", os.path.basename(str(path)), e) + logger.debug("probe_duration failed for %s: %s", media_path.name, e) return None diff --git a/backend/services/sonitranslate.py b/backend/services/sonitranslate.py index 2c668d2d..04da9387 100644 --- a/backend/services/sonitranslate.py +++ b/backend/services/sonitranslate.py @@ -15,6 +15,7 @@ from pathlib import Path from typing import Optional from services.ffmpeg_utils import spawn_subprocess +from core.path_security import UnsafePath, resolve_within, safe_filename logger = logging.getLogger("omnivoice.sonitranslate") @@ -308,9 +309,16 @@ async def dub_video( output_file = result if output_file and output_dir: - dest = os.path.join(output_dir, os.path.basename(output_file)) - shutil.copy2(output_file, dest) - output_file = dest + output_root = Path(output_dir).expanduser() + if not output_root.is_absolute() or not output_root.is_dir(): + raise ValueError("output_dir must be an existing absolute directory") + try: + output_name = safe_filename(os.path.basename(output_file)) + dest = resolve_within(output_root, output_name) + except UnsafePath as exc: + raise ValueError("SoniTranslate returned an invalid output filename") from exc + shutil.copy2(output_file, str(dest)) + output_file = str(dest) logger.info("SoniTranslate dub complete: %s", output_file) return { diff --git a/backend/services/text_normalization.py b/backend/services/text_normalization.py index a264c6b5..274124d8 100644 --- a/backend/services/text_normalization.py +++ b/backend/services/text_normalization.py @@ -181,10 +181,29 @@ def _num2words_lang(language: Optional[str]) -> Optional[str]: # ── Universal safety filters (all languages) ───────────────────────────────── # Zero-width & bidi controls, C0/C1 controls (except \t \n \r), BOM, U+FFFD. -_ZW_CONTROL_RE = re.compile( - "[\u0000-\u0008\u000b\u000c\u000e-\u001f\u007f-\u009f" - "\u200b-\u200f\u202a-\u202e\u2060-\u2064\ufeff\ufffd]" +# Keep the set explicit rather than encoding it as regex character ranges: the +# latter is easy to widen accidentally and obscures the intentionally preserved +# whitespace controls at the C0 boundaries. +_UNSAFE_CONTROL_CODEPOINTS = frozenset( + ( + *range(0x00, 0x09), + 0x0B, + 0x0C, + *range(0x0E, 0x20), + *range(0x7F, 0xA0), + *range(0x200B, 0x2010), + *range(0x202A, 0x202F), + *range(0x2060, 0x2065), + 0xFEFF, + 0xFFFD, + ) ) +_UNSAFE_CONTROL_TRANSLATION = dict.fromkeys(_UNSAFE_CONTROL_CODEPOINTS) + + +def _strip_unsafe_controls(text: str) -> str: + """Delete only the explicitly enumerated unsafe Unicode controls.""" + return text.translate(_UNSAFE_CONTROL_TRANSLATION) # A tiny, unambiguous HTML-entity leftover set. `&` is decoded only when # NOT followed by a letter/`#` — so double-encoded junk ("&nbsp;") is left @@ -212,7 +231,7 @@ _NEWLINE_RE = re.compile(r"\n{3,}") # blank-line floods → one blank line def _safety_filters(text: str) -> str: - out = _ZW_CONTROL_RE.sub("", text) + out = _strip_unsafe_controls(text) out = _ENTITY_RE.sub(lambda m: _ENTITIES.get(m.group(0), "&"), out) out = _REPEAT_RE.sub(lambda m: m.group(1) * 3, out) out = _HSPACE_RE.sub(" ", out) diff --git a/backend/services/video_retime.py b/backend/services/video_retime.py index e5f492ac..2da604a3 100644 --- a/backend/services/video_retime.py +++ b/backend/services/video_retime.py @@ -471,7 +471,7 @@ async def prepare_smart_fit_video( video_dur=expected + tail_pad, ) raise - actual = await probe_duration(work_path) + actual = await probe_duration(work_path, allowed_root=_base) return RetimeDecision( mode="file", file_path=work_path, video_dur=float(actual) if actual else expected + tail_pad, diff --git a/docs/api-auth.md b/docs/api-auth.md index 8d5a9e18..cc68ae84 100644 --- a/docs/api-auth.md +++ b/docs/api-auth.md @@ -197,7 +197,7 @@ time, so in production **restart the backend** to apply a change. Default empty Admin routes — `/system/*` (including `set-env`, **RCE-class**), `/api/settings/*`, engine install/uninstall, media tools, MCP bindings — sit on -a stricter gate (`require_loopback`, `backend/api/dependencies.py`) than +a stricter gate (`require_admin`, `backend/api/dependencies.py`) than consumption. On the desktop build they are **true-loopback-only**: no PIN, key, or trusted network reaches them from another machine. @@ -219,8 +219,11 @@ requirement is dropped (issue #261, else the operator is 403'd out of their own loopback or a caller already authenticated with the API key can read it. Host paths are never selected through HTTP. The native Tauri process validates -model-cache destinations and custom FFmpeg/FFprobe binaries, writes a private -one-shot capability, and only that opaque authorization reaches the backend. +model-cache and export destinations plus custom FFmpeg/FFprobe binaries, writes +a private one-shot capability, and only that opaque authorization reaches the +backend. `/export` therefore accepts an `authorization` token, never a +`destination_path`; revealing an arbitrary exported path runs in the native +process, while the HTTP fallback is limited to the server-owned data root. `/system/set-env` does not accept executable-path keys at all. Server mode and an API key do not weaken that native boundary. @@ -263,7 +266,7 @@ same origin.) If you only moved the Vite dev server's port, set | Code | Meaning | What to do | |---|---|---| | **401** | Consumption auth failed — `{"detail": "PIN required"}` or `{"detail": "API key required"}`. | Supply the PIN / key (header, cookie, or query param above). A WebSocket surfaces this as close code **1008**. | -| **403** | `{"detail": "loopback origin required"}` — you reached a **loopback-gated** route (admin: `/system/*`, `/api/settings/*`; or a `require_local` route from outside a trusted network) from a non-loopback origin. | A PIN won't help. Run the request from the box itself; for `require_local` routes add the caller to `OMNIVOICE_TRUSTED_NETWORKS`; for **admin** routes use `OMNIVOICE_SERVER_MODE=1` **and** present the **API key** (the PIN/trusted-network don't reach admin). | +| **403** | Authorization failed: loopback/native access was required, a server-mode mutation lacked the API key, or a native path capability was invalid, expired, or for a different operation. | A PIN cannot grant admin or filesystem access. Run native operations from the desktop app; configure and present the API key for remote server-mode mutations; reopen the native picker if a one-shot capability expired. | | **429** | **Not an auth failure.** The GPU pool is saturated (admission control) or a model download is rate-limited. Ships with `Retry-After` and `X-VoiceStudio-Retryable: true`. | Back off for `Retry-After` seconds and retry the identical request. | --- diff --git a/frontend/src-tauri/src/commands.rs b/frontend/src-tauri/src/commands.rs index 4be42a13..1575a2ec 100644 --- a/frontend/src-tauri/src/commands.rs +++ b/frontend/src-tauri/src/commands.rs @@ -3,10 +3,11 @@ use std::fs; use std::path::{Path, PathBuf}; use std::sync::atomic::Ordering; -use std::time::Duration; +use std::time::{Duration, Instant}; use serde::{Deserialize, Serialize}; use tauri::image::Image; +use tauri_plugin_dialog::DialogExt; use crate::{AppFlags, TrayHandle, DictationShortcutState}; use crate::{TRAY_ICON_DEFAULT, TRAY_ICON_RECORDING}; @@ -21,44 +22,146 @@ struct AuthorizedHostPath { path: String, } +#[derive(Serialize)] +pub struct AuthorizedPathSelection { + authorization: String, + path: String, +} + pub fn path_authorization_dir(app: &tauri::AppHandle) -> PathBuf { crate::setup::resolved_data_dir(app) .unwrap_or_else(crate::setup::default_data_dir) .join(".path-authorizations") } -fn validate_host_path(kind: &str, raw: &str) -> Result { - if !matches!(kind, "models_dir" | "ffmpeg" | "ffprobe") { +fn remember_reveal_path( + app: &tauri::AppHandle, + path: &Path, +) -> Result<(), String> { + let dir = path_authorization_dir(app); + fs::create_dir_all(&dir).map_err(|e| format!("Could not create authorization store: {e}"))?; + let ledger = dir.join("revealed-paths"); + let selected = path.to_string_lossy().into_owned(); + let mut paths: Vec = fs::read_to_string(&ledger) + .unwrap_or_default() + .lines() + .map(str::to_owned) + .collect(); + paths.retain(|item| item != &selected); + paths.push(selected); + if paths.len() > 1024 { + paths.drain(..paths.len() - 1024); + } + fs::write(&ledger, format!("{}\n", paths.join("\n"))) + .map_err(|e| format!("Could not remember selected path: {e}"))?; + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + fs::set_permissions(&ledger, fs::Permissions::from_mode(0o600)) + .map_err(|e| format!("Could not protect selected paths: {e}"))?; + } + Ok(()) +} + +fn reveal_path_is_authorized( + app: &tauri::AppHandle, + target: &Path, +) -> bool { + if let Ok(data_root) = fs::canonicalize( + crate::setup::resolved_data_dir(app).unwrap_or_else(crate::setup::default_data_dir), + ) { + if target.starts_with(data_root) { + return true; + } + } + let Ok(ledger) = fs::read_to_string(path_authorization_dir(app).join("revealed-paths")) else { + return false; + }; + ledger.lines().any(|selected| { + fs::canonicalize(selected) + .map(|remembered| remembered == target) + .unwrap_or(false) + }) +} + +fn validate_host_path(kind: &str, path: PathBuf) -> Result { + if !matches!( + kind, + "models_dir" + | "ffmpeg" + | "ffprobe" + | "dub_export" + | "soni_input" + | "soni_output_dir" + ) { return Err("Unsupported host-path capability".into()); } - if raw.chars().any(|c| c.is_control()) { + if path.to_string_lossy().chars().any(|c| c.is_control()) { return Err("Path contains invalid control characters".into()); } - if kind == "models_dir" && raw.is_empty() { + if kind == "models_dir" && path.as_os_str().is_empty() { return Ok(PathBuf::new()); // explicit reset to the platform default } - let path = PathBuf::from(raw); if !path.is_absolute() { return Err("Path must be absolute".into()); } - if kind == "models_dir" { + if matches!(kind, "models_dir" | "soni_output_dir") { fs::create_dir_all(&path).map_err(|e| format!("Directory is not writable: {e}"))?; let probe = path.join(".voicestudio-write-test"); fs::write(&probe, b"ok").map_err(|e| format!("Directory is not writable: {e}"))?; let _ = fs::remove_file(probe); + } else if kind == "dub_export" { + let parent = path + .parent() + .ok_or_else(|| "Save destination must have a parent directory".to_string())?; + if !parent.is_dir() { + return Err("Save destination directory does not exist".into()); + } + } else if kind == "soni_input" { + if !path.is_file() { + return Err("Selected media input is not a file".into()); + } } else { if !path.is_file() { return Err("Selected media tool is not a file".into()); } - let status = crate::tools::no_window( + let mut child = crate::tools::no_window( std::process::Command::new(&path) .arg("-version") - .stdout(std::process::Stdio::null()) - .stderr(std::process::Stdio::null()), + .stdout(std::process::Stdio::piped()) + .stderr(std::process::Stdio::piped()), ) - .status() + .spawn() .map_err(|e| format!("Selected media tool could not run: {e}"))?; - if !status.success() { + let deadline = Instant::now() + Duration::from_secs(5); + loop { + match child.try_wait() { + Ok(Some(_)) => break, + Ok(None) if Instant::now() < deadline => { + std::thread::sleep(Duration::from_millis(25)); + } + Ok(None) => { + let _ = child.kill(); + let _ = child.wait(); + return Err("Selected media tool did not respond within 5 seconds".into()); + } + Err(e) => { + let _ = child.kill(); + let _ = child.wait(); + return Err(format!("Selected media tool could not be checked: {e}")); + } + } + } + let output = child + .wait_with_output() + .map_err(|e| format!("Selected media tool output could not be read: {e}"))?; + let version_text = format!( + "{}{}", + String::from_utf8_lossy(&output.stdout), + String::from_utf8_lossy(&output.stderr) + ) + .to_ascii_lowercase(); + if !output.status.success() || !version_text.contains(kind) { return Err("Selected media tool failed its version check".into()); } } @@ -66,12 +169,34 @@ fn validate_host_path(kind: &str, raw: &str) -> Result { } #[tauri::command] -pub fn authorize_host_path( +pub async fn authorize_host_path( app: tauri::AppHandle, kind: String, - path: String, -) -> Result { - let validated = validate_host_path(&kind, path.trim())?; + suggested_name: Option, + reset: Option, +) -> Result, String> { + let selected = if kind == "models_dir" && reset.unwrap_or(false) { + Some(PathBuf::new()) + } else { + let dialog = app.dialog().file(); + let picked = match kind.as_str() { + "models_dir" | "soni_output_dir" => dialog.blocking_pick_folder(), + "ffmpeg" | "ffprobe" | "soni_input" => dialog.blocking_pick_file(), + "dub_export" => { + let mut save = app.dialog().file(); + if let Some(name) = suggested_name.as_deref() { + save = save.set_file_name(name); + } + save.blocking_save_file() + } + _ => return Err("Unsupported host-path capability".into()), + }; + picked.and_then(|value| value.into_path().ok()) + }; + let Some(selected) = selected else { + return Ok(None); + }; + let validated = validate_host_path(&kind, selected)?; let mut random = [0_u8; 32]; getrandom::fill(&mut random).map_err(|e| format!("Secure randomness unavailable: {e}"))?; let token: String = random.iter().map(|b| format!("{b:02x}")).collect(); @@ -97,7 +222,13 @@ pub fn authorize_host_path( fs::set_permissions(&target, fs::Permissions::from_mode(0o600)) .map_err(|e| format!("Could not protect authorization: {e}"))?; } - Ok(token) + if payload.kind == "dub_export" { + remember_reveal_path(&app, &validated)?; + } + Ok(Some(AuthorizedPathSelection { + authorization: token, + path: validated.to_string_lossy().into_owned(), + })) } #[cfg(test)] @@ -107,14 +238,30 @@ mod host_path_authorization_tests { #[test] fn rejects_unknown_relative_and_control_character_paths() { - assert!(validate_host_path("shell", "/tmp/tool").is_err()); - assert!(validate_host_path("models_dir", "relative/models").is_err()); - assert!(validate_host_path("models_dir", "/tmp/bad\npath").is_err()); + assert!(validate_host_path("shell", PathBuf::from("/tmp/tool")).is_err()); + assert!(validate_host_path("models_dir", PathBuf::from("relative/models")).is_err()); + assert!(validate_host_path("models_dir", PathBuf::from("/tmp/bad\npath")).is_err()); } #[test] fn empty_models_path_is_the_authorized_default_reset() { - assert_eq!(validate_host_path("models_dir", "").unwrap(), PathBuf::new()); + assert_eq!(validate_host_path("models_dir", PathBuf::new()).unwrap(), PathBuf::new()); + } + + #[test] + fn dub_export_accepts_only_absolute_paths_in_existing_directories() { + let parent = std::env::temp_dir(); + let destination = parent.join("voicestudio-authorized-export.wav"); + assert_eq!( + validate_host_path("dub_export", destination.clone()).unwrap(), + destination, + ); + assert!(validate_host_path("dub_export", PathBuf::from("relative/export.wav")).is_err()); + assert!(validate_host_path( + "dub_export", + parent.join("missing-directory/export.wav"), + ) + .is_err()); } } @@ -832,6 +979,53 @@ pub fn save_text_file(path: String, contents: String) -> Result<(), String> { std::fs::write(p, contents).map_err(|e| format!("write: {e}")) } +#[tauri::command] +pub fn reveal_host_path(app: tauri::AppHandle, path: String) -> Result<(), String> { + // Revealing is a native-shell action, never a loopback HTTP authority. + // Canonicalization rejects missing paths and removes traversal/symlinks; + // argv-only spawning avoids shell interpretation on every platform. + let target = std::fs::canonicalize(&path) + .map_err(|_| "That file or folder is no longer on disk".to_string())?; + if !reveal_path_is_authorized(&app, &target) { + return Err("That path was not selected by VoiceStudio".into()); + } + let folder = if target.is_dir() { + target.clone() + } else { + target.parent() + .ok_or_else(|| "That path has no containing folder".to_string())? + .to_path_buf() + }; + let mut command = if cfg!(target_os = "macos") { + let mut command = std::process::Command::new("open"); + if target.is_file() { + command.arg("-R").arg(&target); + } else { + command.arg(&folder); + } + command + } else if cfg!(target_os = "windows") { + let mut command = std::process::Command::new("explorer"); + if target.is_file() { + command.arg("/select,").arg(&target); + } else { + command.arg(&folder); + } + command + } else { + let mut command = std::process::Command::new("xdg-open"); + command.arg(&folder); + command + }; + let mut child = crate::tools::no_window(&mut command) + .spawn() + .map_err(|e| format!("Could not open the containing folder: {e}"))?; + std::thread::spawn(move || { + let _ = child.wait(); + }); + Ok(()) +} + // ── WebView cache repair (issue #879) ───────────────────────────────────── // // After an unclean shutdown (e.g. a Windows BSOD), WebView2's profile cache diff --git a/frontend/src-tauri/src/lib.rs b/frontend/src-tauri/src/lib.rs index 71c7897e..f599320b 100644 --- a/frontend/src-tauri/src/lib.rs +++ b/frontend/src-tauri/src/lib.rs @@ -435,6 +435,7 @@ pub fn run() { commands::set_tray_recording, commands::quit_app, commands::save_text_file, + commands::reveal_host_path, commands::get_dictation_shortcut, commands::set_dictation_shortcut, commands::get_launch_as_widget, diff --git a/frontend/src/App.jsx b/frontend/src/App.jsx index 2b6b58fe..a4c0a241 100644 --- a/frontend/src/App.jsx +++ b/frontend/src/App.jsx @@ -808,17 +808,16 @@ function App() { return; } try { - const { save } = await import('@tauri-apps/plugin-dialog'); - const ext = fallbackName.includes('.') ? fallbackName.split('.').pop() : 'wav'; - const destPath = await save({ - defaultPath: fallbackName, - filters: [{ name: 'Media', extensions: [ext] }], + const { invoke } = await import('@tauri-apps/api/core'); + const selection = await invoke('authorize_host_path', { + kind: 'dub_export', + suggestedName: fallbackName, }); - if (!destPath) return; // User cancelled + if (!selection) return; // User cancelled await exportAction({ source_filename: sourceIdentifier, - destination_path: destPath, + authorization: selection.authorization, mode, }); toast.success(i18n.t('app.toast_exported', { name: fallbackName })); @@ -839,7 +838,7 @@ function App() { toast.error(i18n.t('app.toast_open_folder_failed', { message: err.message })); } }; - // Save a dynamic (save_path-aware) export — dub video/audio/subtitles — to + // Save a dynamic export — dub video/audio/subtitles — to // disk. The parity-safe dialog + server-side copy vs browser-blob branch now // lives in the shared `downloadMedia` util (#1218) so audiobook/story exports // reuse the exact same path and never fall back to a webview-hijacking diff --git a/frontend/src/api/client.ts b/frontend/src/api/client.ts index e5528cdc..7057aa9e 100644 --- a/frontend/src/api/client.ts +++ b/frontend/src/api/client.ts @@ -240,7 +240,9 @@ const STARTUP_GRACE_MS = 120_000; const RECONCILE_MS = 12_000; const RECONCILE_INTERVAL_MS = 1000; -export async function apiFetch(path: string, opts: RequestInit = {}): Promise { +export type ApiFetchOptions = RequestInit & { retryTransport?: boolean }; + +export async function apiFetch(path: string, opts: ApiFetchOptions = {}): Promise { const pin = typeof sessionStorage !== 'undefined' ? sessionStorage.getItem('ov_pin') : null; const key = _apiKey(); // Only modify the request when a PIN/API key is set, so the default call @@ -249,9 +251,10 @@ export async function apiFetch(path: string, opts: RequestInit = {}): Promise = {}; if (pin) extra['X-OmniVoice-Pin'] = pin; if (key) extra['Authorization'] = `Bearer ${key}`; + const { retryTransport = true, ...requestOpts } = opts; const finalOpts: RequestInit = Object.keys(extra).length - ? { ...opts, headers: { ...(opts.headers as Record), ...extra } } - : opts; + ? { ...requestOpts, headers: { ...(requestOpts.headers as Record), ...extra } } + : requestOpts; const signal = finalOpts.signal as AbortSignal | null | undefined; let lastDetail = ''; // The shell's last word on the backend. When it still says `ready` after we've @@ -283,7 +286,7 @@ export async function apiFetch(path: string, opts: RequestInit = {}): Promise setTimeout(r, TRANSPORT_RETRY_BACKOFF_MS[attempt])); continue; } @@ -292,7 +295,7 @@ export async function apiFetch(path: string, opts: RequestInit = {}): Promise): Promise): Promise { + if (typeof window !== 'undefined' && '__TAURI_INTERNALS__' in window) { + const { invoke } = await import('@tauri-apps/api/core'); + return invoke('reveal_host_path', { path: body.path }); + } return apiPost('/export/reveal', body); } diff --git a/frontend/src/components/MediaEngineCard.jsx b/frontend/src/components/MediaEngineCard.jsx index c64444b4..eaf23a68 100644 --- a/frontend/src/components/MediaEngineCard.jsx +++ b/frontend/src/components/MediaEngineCard.jsx @@ -19,8 +19,6 @@ export default function MediaEngineCard() { const { t } = useTranslation(); const [status, setStatus] = useState(null); const [detectError, setDetectError] = useState(null); - const [customPath, setCustomPath] = useState(''); - const [showPathInput, setShowPathInput] = useState(false); const [busy, setBusy] = useState(false); const refresh = useCallback(async () => { @@ -81,19 +79,22 @@ export default function MediaEngineCard() { const chooseFile = async () => { try { if ('__TAURI_INTERNALS__' in window) { - const { open } = await import('@tauri-apps/plugin-dialog'); - const picked = await open({ multiple: false, directory: false, title: 'FFmpeg' }); - if (typeof picked === 'string') { - await post('/media-tools/ffmpeg/custom-path', { path: picked }); + const { invoke } = await import('@tauri-apps/api/core'); + const selection = await invoke('authorize_host_path', { kind: 'ffmpeg' }); + if (selection) { + await post('/media-tools/ffmpeg/custom-path', { + authorization: selection.authorization, + }); return; } } - } catch { - /* picker unavailable — fall through to the inline input */ + } catch (e) { + setDetectError(e?.message || String(e)); } - setShowPathInput(true); }; + const isDesktop = '__TAURI_INTERNALS__' in window; + if (!status || status.ready) return null; // the ideal outcome: nothing. const op = status.ops?.acquire || {}; @@ -151,29 +152,10 @@ export default function MediaEngineCard() { > {t('setup.media_engine_use_system', { defaultValue: 'Use a system copy' })} - - {showPathInput && ( - <> - setCustomPath(e.target.value)} - placeholder="/usr/bin/ffmpeg" - className="min-w-[220px] flex-1 rounded border border-border bg-transparent px-2 py-1 font-mono text-xs text-fg" - aria-label={t('settings.ffmpeg_input_aria', { defaultValue: 'FFmpeg path' })} - data-testid="media-engine-path" - /> - - + {isDesktop && ( + )} diff --git a/frontend/src/components/MediaEngineCard.test.jsx b/frontend/src/components/MediaEngineCard.test.jsx index 998b07ef..1946a868 100644 --- a/frontend/src/components/MediaEngineCard.test.jsx +++ b/frontend/src/components/MediaEngineCard.test.jsx @@ -6,6 +6,8 @@ vi.mock('../api/client', () => ({ apiJson: vi.fn(), apiFetch: vi.fn(), })); +const invoke = vi.fn(); +vi.mock('@tauri-apps/api/core', () => ({ invoke: (...args) => invoke(...args) })); import { apiJson, apiFetch } from '../api/client'; import MediaEngineCard from './MediaEngineCard'; @@ -19,6 +21,7 @@ const statusWith = (ready, acquire) => ({ describe('MediaEngineCard — invisible-by-default media engine', () => { beforeEach(() => { vi.clearAllMocks(); + delete window.__TAURI_INTERNALS__; apiFetch.mockResolvedValue({ ok: true, json: async () => ({}) }); }); @@ -83,18 +86,28 @@ describe('MediaEngineCard — invisible-by-default media engine', () => { ); }); - it('Choose file… falls back to an inline path input outside Tauri and saves it', async () => { + it('Choose file uses only a native picker authorization', async () => { apiJson.mockResolvedValue(statusWith(false, { state: 'error', error: 'boom' })); + window.__TAURI_INTERNALS__ = {}; + invoke.mockResolvedValue({ authorization: 'e'.repeat(64), path: '/usr/local/bin/ffmpeg' }); render(); fireEvent.click(await screen.findByText('Choose file…')); - const input = await screen.findByTestId('media-engine-path'); - fireEvent.change(input, { target: { value: '/usr/local/bin/ffmpeg' } }); - fireEvent.click(screen.getByText('Save')); + await waitFor(() => + expect(invoke).toHaveBeenCalledWith('authorize_host_path', { kind: 'ffmpeg' }), + ); await waitFor(() => expect(apiFetch).toHaveBeenCalledWith( '/media-tools/ffmpeg/custom-path', - expect.objectContaining({ body: JSON.stringify({ path: '/usr/local/bin/ffmpeg' }) }), + expect.objectContaining({ body: JSON.stringify({ authorization: 'e'.repeat(64) }) }), ), ); + expect(apiFetch.mock.calls.flat().join(' ')).not.toContain('/usr/local/bin/ffmpeg'); + }); + + it('does not offer native file selection in a browser', async () => { + apiJson.mockResolvedValue(statusWith(false, { state: 'error', error: 'boom' })); + render(); + await screen.findByTestId('media-engine-card'); + expect(screen.queryByText('Choose file…')).not.toBeInTheDocument(); }); }); diff --git a/frontend/src/components/settings/AudioToolsPanel.jsx b/frontend/src/components/settings/AudioToolsPanel.jsx index 4d1bc522..4369b438 100644 --- a/frontend/src/components/settings/AudioToolsPanel.jsx +++ b/frontend/src/components/settings/AudioToolsPanel.jsx @@ -5,7 +5,7 @@ * One row per tool: * • FFmpeg / FFprobe — version + origin badge (Bundled / System / Custom / * App package) + path; actions: Use system copy (auto-detect), - * Choose file… (picker in Tauri, inline path input everywhere), + * Choose file… (native picker in Tauri), * Restore bundled (always-safe revert). The section header carries * "Update bundled build" (one download covers both binaries). * • yt-dlp — module version + Update (fetches the newest wheel into an @@ -19,9 +19,8 @@ import { toast } from 'react-hot-toast'; import { useTranslation } from 'react-i18next'; import { AudioLines, Film, ScanSearch, DownloadCloud } from 'lucide-react'; import { Button, Badge } from '../../ui'; -import { SettingsSection, SettingRow, SettingsInput } from './primitives'; +import { SettingsSection, SettingRow } from './primitives'; import RestartBadge from './RestartBadge'; -import { isTauri } from './native'; const ORIGIN_TONE = { bundled: 'success', @@ -46,33 +45,11 @@ function OriginBadge({ origin }) { ); } -/** Open the OS file picker in Tauri; return the chosen path or null. */ -async function pickBinary(title) { - if (!isTauri()) return null; - try { - const { open } = await import('@tauri-apps/plugin-dialog'); - const picked = await open({ multiple: false, directory: false, title }); - return typeof picked === 'string' ? picked : null; - } catch { - return null; - } -} - function BinaryRow({ tool, info, onAction, busy }) { const { t } = useTranslation(); - const [path, setPath] = useState(''); - const [showInput, setShowInput] = useState(false); const label = tool === 'ffmpeg' ? 'FFmpeg' : 'FFprobe'; - const chooseFile = async () => { - const picked = await pickBinary(label); - if (picked) { - onAction(`/media-tools/${tool}/custom-path`, { path: picked }); - } else { - // Web preview / picker unavailable — fall back to the inline input. - setShowInput(true); - } - }; + const chooseFile = () => onAction(`/media-tools/${tool}/custom-path`); return ( {t('settings.audio_tools_use_system', { defaultValue: 'Use system copy' })} - + {'__TAURI_INTERNALS__' in window && ( + + )} - {showInput && ( - <> - setPath(e.target.value)} - onKeyDown={(e) => - e.key === 'Enter' && - path.trim() && - onAction(`/media-tools/${tool}/custom-path`, { path: path.trim() }) - } - aria-label={t('settings.audio_tools_path_input_aria', { - tool: label, - defaultValue: '{{tool}} binary path', - })} - /> - - - )} } /> @@ -268,15 +221,15 @@ export default function AudioToolsPanel() { const onToolAction = useCallback( async (path, body) => { let requestBody = body; + let selectedPath = body?.path; if (path.endsWith('/custom-path')) { try { const { invoke } = await import('@tauri-apps/api/core'); const tool = path.includes('/ffprobe/') ? 'ffprobe' : 'ffmpeg'; - const authorization = await invoke('authorize_host_path', { - kind: tool, - path: body?.path || '', - }); - requestBody = { authorization }; + const selection = await invoke('authorize_host_path', { kind: tool }); + if (!selection) return; + requestBody = { authorization: selection.authorization }; + selectedPath = selection.path; } catch (e) { toast.error( t('settings.audio_tools_path_failed', { @@ -292,7 +245,8 @@ export default function AudioToolsPanel() { toast.success( t('settings.audio_tools_path_set', { tool: path.includes('ffprobe') ? 'FFprobe' : 'FFmpeg', - path: body?.path || t('settings.audio_tools_origin_system', { defaultValue: 'System' }), + path: + selectedPath || t('settings.audio_tools_origin_system', { defaultValue: 'System' }), defaultValue: '{{tool}} now uses {{path}}', }), ); diff --git a/frontend/src/components/settings/AudioToolsPanel.test.jsx b/frontend/src/components/settings/AudioToolsPanel.test.jsx index a596fcec..8051a830 100644 --- a/frontend/src/components/settings/AudioToolsPanel.test.jsx +++ b/frontend/src/components/settings/AudioToolsPanel.test.jsx @@ -57,9 +57,17 @@ const okResponse = { ok: true, json: async () => ({}) }; describe('AudioToolsPanel — power-user surface for the media tools', () => { beforeEach(() => { vi.clearAllMocks(); + window.__TAURI_INTERNALS__ = {}; apiJson.mockResolvedValue(JSON.parse(JSON.stringify(STATUS))); apiFetch.mockResolvedValue(okResponse); - invoke.mockResolvedValue('c'.repeat(64)); + invoke.mockResolvedValue({ authorization: 'c'.repeat(64), path: '/opt/tools/ffmpeg' }); + }); + + it('does not offer native file selection in a browser', async () => { + delete window.__TAURI_INTERNALS__; + render(); + await screen.findByText('FFmpeg'); + expect(screen.queryByLabelText('FFmpeg: Choose file…')).not.toBeInTheDocument(); }); it('renders one row per tool with version, path, and origin badge', async () => { @@ -91,14 +99,10 @@ describe('AudioToolsPanel — power-user surface for the media tools', () => { it('sends only a native one-shot authorization for a custom executable', async () => { render(); fireEvent.click(await screen.findByLabelText('FFmpeg: Choose file…')); - const input = await screen.findByLabelText('FFmpeg binary path'); - fireEvent.change(input, { target: { value: '/opt/tools/ffmpeg' } }); - fireEvent.click(screen.getByRole('button', { name: 'Save' })); await waitFor(() => expect(invoke).toHaveBeenCalledWith('authorize_host_path', { kind: 'ffmpeg', - path: '/opt/tools/ffmpeg', }), ); expect(apiFetch).toHaveBeenCalledWith( @@ -106,6 +110,7 @@ describe('AudioToolsPanel — power-user surface for the media tools', () => { expect.objectContaining({ body: JSON.stringify({ authorization: 'c'.repeat(64) }) }), ); expect(apiFetch.mock.calls.flat().join(' ')).not.toContain('/opt/tools/ffmpeg'); + expect(toast.success).toHaveBeenCalledWith(expect.stringContaining('/opt/tools/ffmpeg')); }); it('Restore bundled is per-tool and always available (safe revert)', async () => { diff --git a/frontend/src/components/settings/StoragePanel.jsx b/frontend/src/components/settings/StoragePanel.jsx index 7cf29dec..e30c38e7 100644 --- a/frontend/src/components/settings/StoragePanel.jsx +++ b/frontend/src/components/settings/StoragePanel.jsx @@ -48,16 +48,17 @@ export default function StoragePanel() { refresh(); }, [refresh]); - const save = async (path) => { + const save = async (reset = false) => { setSaving(true); setError(null); try { const { invoke } = await import('@tauri-apps/api/core'); - const authorization = await invoke('authorize_host_path', { kind: 'models_dir', path }); + const selection = await invoke('authorize_host_path', { kind: 'models_dir', reset }); + if (!selection) return; const res = await apiFetch('/api/settings/storage/models-dir', { method: 'PUT', headers: { 'Content-Type': 'application/json' }, - body: JSON.stringify({ authorization }), + body: JSON.stringify({ authorization: selection.authorization }), }); if (!res.ok) { const b = await res.json().catch(() => ({})); @@ -67,7 +68,7 @@ export default function StoragePanel() { setConfigured(b?.configured || ''); setRestart(Boolean(b?.restart_required)); toast.success( - path + selection.path ? 'Models directory saved — restart to apply' : 'Reverted to default — restart to apply', ); @@ -116,7 +117,7 @@ export default function StoragePanel() { type="text" value={input} placeholder={def || '~/.cache/huggingface'} - onChange={(e) => setInput(e.target.value)} + readOnly disabled={saving || loading} spellCheck={false} aria-label="Models directory" @@ -124,7 +125,7 @@ export default function StoragePanel() { />