v1.0.0 — production-quality cleanup pass
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.
This commit is contained in:
parent
659e7cf613
commit
24be9497e9
5 changed files with 184 additions and 41 deletions
|
|
@ -1,7 +1,7 @@
|
|||
<?xml version="1.0" encoding="UTF-8" standalone="yes"?>
|
||||
<addon id="plugin.video.torttube"
|
||||
name="torttube"
|
||||
version="0.0.17"
|
||||
version="1.0.0"
|
||||
provider-name="Sulkta-Coop">
|
||||
<requires>
|
||||
<import addon="xbmc.python" version="3.0.0"/>
|
||||
|
|
|
|||
|
|
@ -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":
|
||||
|
|
|
|||
|
|
@ -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 <cobb@sulkta.com>"]
|
||||
|
|
|
|||
|
|
@ -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).
|
||||
|
|
|
|||
|
|
@ -66,7 +66,7 @@ pub(crate) async fn fetch(id: &str, categories: &[String]) -> anyhow::Result<ser
|
|||
// Stream the body and bail as soon as accumulated bytes exceed the cap —
|
||||
// do NOT buffer the entire response first, otherwise a hostile mirror or
|
||||
// compromised DNS returning a multi-GB response would OOM the sidecar
|
||||
// before any cap check fires. Audit HIGH-1 (2nd pass).
|
||||
// before any cap check fires.
|
||||
const SPONSORBLOCK_MAX_BYTES: usize = 1024 * 1024;
|
||||
let mut bytes: Vec<u8> = Vec::with_capacity(64 * 1024);
|
||||
while let Some(chunk) = resp.chunk().await? {
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue