diff --git a/ytsage/core/ytsage_downloader.py b/ytsage/core/ytsage_downloader.py index 3799143..0d9ee54 100644 --- a/ytsage/core/ytsage_downloader.py +++ b/ytsage/core/ytsage_downloader.py @@ -244,8 +244,12 @@ class DownloadThread(QThread): def _build_yt_dlp_command(self) -> List[str]: """Build the yt-dlp command line with all options for direct execution.""" yt_dlp_path: str = get_yt_dlp_path() + if str(yt_dlp_path) == "yt-dlp": + # Sentinel: no managed binary and no opted-in system binary. + # Never exec a bare command name from PATH. + raise FileNotFoundError("yt-dlp is not installed - run the yt-dlp setup first") # Build the command line array - cmd: List[str] = [yt_dlp_path] + cmd: List[str] = [str(yt_dlp_path)] logger.debug(f"Using yt-dlp from: {yt_dlp_path}") # Add concurrent fragments setting diff --git a/ytsage/core/ytsage_utils.py b/ytsage/core/ytsage_utils.py index 52bdd5e..b5164f1 100644 --- a/ytsage/core/ytsage_utils.py +++ b/ytsage/core/ytsage_utils.py @@ -432,8 +432,9 @@ def save_path(main_window_instance: Any, path: Union[str, Path]) -> bool: def update_yt_dlp() -> bool: """Check for yt-dlp updates and update if a newer version is available.""" try: - # Get the yt-dlp path - yt_dlp_path: Path = get_yt_dlp_path() + # Get the yt-dlp path (may be the bare "yt-dlp" sentinel string when + # not installed - normalize to Path so .exists()/.samefile() work) + yt_dlp_path: Path = Path(get_yt_dlp_path()) # Extra logic moved to src\utils\ytsage_constants.py diff --git a/ytsage/core/ytsage_yt_dlp.py b/ytsage/core/ytsage_yt_dlp.py index 4eceebb..d7bf780 100644 --- a/ytsage/core/ytsage_yt_dlp.py +++ b/ytsage/core/ytsage_yt_dlp.py @@ -612,19 +612,30 @@ def check_ytdlp_installed() -> bool: def get_yt_dlp_path() -> Path: """ - Get the yt-dlp path, either from the app's bin directory or system PATH. - This replaces the function in ytsage_utils.py. + Get the yt-dlp path. Prefers the app-managed, SHA256-verified binary; + a system-installed yt-dlp is only used when the user explicitly opts in + via the advanced.allow_system_ytdlp config key (resolved to an absolute + path so Windows' implicit CWD lookup can never pick up a planted binary). Returns: - str: Path to yt-dlp binary + Path to yt-dlp binary, or the bare string "yt-dlp" sentinel meaning + "not installed" (triggers the setup dialog). """ - # First check if we have yt-dlp in our app's bin directory or system PATH ytdlp_path = check_ytdlp_binary() if ytdlp_path: logger.info(f"Using yt-dlp from: {ytdlp_path}") return ytdlp_path - # If not found anywhere, fall back to the command name as a last resort - logger.info("yt-dlp not found in app directory or PATH, falling back to command name") + from ..utils.ytsage_config_manager import ConfigManager + + if ConfigManager.get("advanced.allow_system_ytdlp"): + system_ytdlp = shutil.which("yt-dlp") + if system_ytdlp: + resolved = Path(system_ytdlp).resolve() + logger.info(f"Using system yt-dlp (advanced.allow_system_ytdlp): {resolved}") + return resolved + + # Not installed - return the sentinel that triggers the setup dialog + logger.info("yt-dlp not found in app directory, returning setup sentinel") return "yt-dlp" # type: ignore[return-value] diff --git a/ytsage/gui/ytsage_gui_analysis.py b/ytsage/gui/ytsage_gui_analysis.py index 5bdd095..edf87c6 100644 --- a/ytsage/gui/ytsage_gui_analysis.py +++ b/ytsage/gui/ytsage_gui_analysis.py @@ -1,7 +1,10 @@ from typing import TYPE_CHECKING, Any, Dict, List, Optional, cast import json +import os +import signal import subprocess +import sys from PySide6.QtCore import QMetaObject, Qt, Q_ARG, QThread, Signal, QTimer from PySide6.QtWidgets import QMessageBox @@ -95,13 +98,56 @@ class AnalysisThread(QThread): if self.geo_proxy_url: cmd.extend(["--geo-verification-proxy", self.geo_proxy_url]) + @staticmethod + def _run_ytdlp(cmd: list, timeout: int) -> subprocess.CompletedProcess: + """Run yt-dlp capturing output, killing the whole process group on timeout. + + subprocess.run() only kills the direct child on TimeoutExpired, leaking + grandchildren (deno); it also loses any stderr produced before the + timeout. Use Popen with a new session so the entire group can be + reaped, and surface the partial stderr in the raised exception. + """ + popen_kwargs: Dict[str, Any] = { + "stdout": subprocess.PIPE, + "stderr": subprocess.PIPE, + "text": True, + "encoding": "utf-8", + "errors": "replace", + } + if sys.platform == "win32": + popen_kwargs["creationflags"] = SUBPROCESS_CREATIONFLAGS + else: + popen_kwargs["start_new_session"] = True + + proc = subprocess.Popen(cmd, **popen_kwargs) + try: + stdout, stderr = proc.communicate(timeout=timeout) + except subprocess.TimeoutExpired as exc: + if sys.platform == "win32": + subprocess.run( + ["taskkill", "/F", "/T", "/PID", str(proc.pid)], + stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL, + creationflags=SUBPROCESS_CREATIONFLAGS, + ) + else: + try: + os.killpg(os.getpgid(proc.pid), signal.SIGKILL) + except (ProcessLookupError, PermissionError, OSError): + pass + stdout, stderr = proc.communicate() + exc.stdout, exc.stderr = stdout, stderr + raise + return subprocess.CompletedProcess(cmd, proc.returncode, stdout, stderr) + def _analyze_url_with_subprocess(self, url: str) -> None: """Analyze URL using yt-dlp executable.""" if self._cancelled: return yt_dlp_path = get_yt_dlp_path() - if not yt_dlp_path: + if not yt_dlp_path or str(yt_dlp_path) == "yt-dlp": + # Sentinel: no managed binary and no opted-in system binary. + # Never exec a bare command name from PATH. logger.error("yt-dlp executable not found. Please install yt-dlp first.") self.analysis_error.emit(_("errors.ytdlp_not_found")) self.playlist_info_visible.emit(False) @@ -118,12 +164,10 @@ class AnalysisThread(QThread): logger.debug(f"Executing yt-dlp command: {cmd}") try: - result = subprocess.run( - cmd, capture_output=True, text=True, timeout=300, - creationflags=SUBPROCESS_CREATIONFLAGS - ) - except subprocess.TimeoutExpired: - logger.error("Analysis timed out") + result = self._run_ytdlp(cmd, timeout=300) + except subprocess.TimeoutExpired as e: + stderr_tail = (e.stderr or "")[-500:] if isinstance(e.stderr, str) else "" + logger.error(f"Analysis timed out. Partial stderr: {stderr_tail}") self.analysis_error.emit(_("errors.timeout")) return @@ -181,7 +225,8 @@ class AnalysisThread(QThread): if first_info.get("_type") == "playlist": result_data["is_playlist"] = True result_data["playlist_info"] = first_info - playlist_entries = first_info.get("entries", []) + # Private/deleted videos can appear as None entries in flat playlists + playlist_entries = [e for e in first_info.get("entries", []) if e] result_data["playlist_entries"] = playlist_entries if not playlist_entries: @@ -201,15 +246,12 @@ class AnalysisThread(QThread): self._add_auth_options(cmd_single) try: - result_single = subprocess.run( - cmd_single, capture_output=True, text=True, timeout=60, - creationflags=SUBPROCESS_CREATIONFLAGS - ) + result_single = self._run_ytdlp(cmd_single, timeout=60) if result_single.returncode == 0: result_data["video_info"] = json.loads(result_single.stdout) else: result_data["video_info"] = first_video_entry - except subprocess.TimeoutExpired: + except (subprocess.TimeoutExpired, json.JSONDecodeError): result_data["video_info"] = first_video_entry if self._cancelled: