From 9c9e059e2d14619d89ef77a3bb0a005ceeabfbec Mon Sep 17 00:00:00 2001 From: imSp4rky Date: Sat, 19 Sep 2026 21:42:01 -0600 Subject: [PATCH] fix(remote): forward nested service cache files to and from the server Cache keys are now the path relative to the service cache dir without .json, so Cacher keys like session_web/ reach the remote session. The credential filter runs on every path segment, the suffix is appended rather than substituted, and both sides cap the map at 64 keys. --- docs/dev/rest-api/endpoints.md | 6 +- docs/dev/rest-api/remote-sessions.md | 34 +++- tests/core/test_sanitize.py | 27 ++- .../unit/test_remote_service_helpers.py | 181 ++++++++++++++++++ unshackle/core/api/handlers.py | 24 ++- unshackle/core/api/sanitize.py | 29 ++- unshackle/core/remote_service.py | 29 ++- 7 files changed, 292 insertions(+), 38 deletions(-) diff --git a/docs/dev/rest-api/endpoints.md b/docs/dev/rest-api/endpoints.md index 45a658a9..f4f196f4 100644 --- a/docs/dev/rest-api/endpoints.md +++ b/docs/dev/rest-api/endpoints.md @@ -857,7 +857,7 @@ DELETE /api/session/{id} → tear down, harvest updated cache ### `POST /api/session/create` -Make a remote session for a service and title. Authentication runs asynchronously in the background. This call returns immediately with `status: "authenticating"`, and you then poll the prompt endpoint. The body accepts `service` and `title_id` (both required). It also accepts a broad set of optional keys, because the body allows `additionalProperties`. These are `credentials` (`{username, password, extra?}`), `cookies` (base64 of zlib-compressed Netscape cookie file), `proxy`, `no_proxy`, `profile`, `cache` (map of forwarded cache files), `client_region`, `proxy_region`, `cdm_type`, `range_`, `vcodec`, `quality`, `best_available`, `client`, and any service CLI options. `proxy_region` is the country code the client resolved `proxy` from; the server matches it against its own accounts. `client` is a freeform object the dashboard shows as sent (the CLI puts `version`, `code_hash`, `platform` and a redacted `argv` in it); the server ignores it above 4096 bytes of JSON. +Make a remote session for a service and title. Authentication runs asynchronously in the background. This call returns immediately with `status: "authenticating"`, and you then poll the prompt endpoint. The body accepts `service` and `title_id` (both required). It also accepts a broad set of optional keys, because the body allows `additionalProperties`. These are `credentials` (`{username, password, extra?}`), `cookies` (base64 of zlib-compressed Netscape cookie file), `proxy`, `no_proxy`, `profile`, `cache` (map of forwarded cache files, keyed by the file path relative to the service cache directory with `/` separators and no `.json` suffix, so a nested key such as `session_web/` lands in a subdirectory), `client_region`, `proxy_region`, `cdm_type`, `range_`, `vcodec`, `quality`, `best_available`, `client`, and any service CLI options. `proxy_region` is the country code the client resolved `proxy` from; the server matches it against its own accounts. `client` is a freeform object the dashboard shows as sent (the CLI puts `version`, `code_hash`, `platform` and a redacted `argv` in it); the server ignores it above 4096 bytes of JSON. Service CLI options also travel in a nested `service_params` object, which wins over a flat key with the same name. `profile` at the top level always means the credentials profile, never a service's own `--profile` option. @@ -1130,11 +1130,11 @@ Tear down a remote session. The server cancels its input bridge, harvests any up ```json { "status": "ok", - "cache": { "tokens.json": "" } + "cache": { "tokens": "", "session_web/": "" } } ``` -The `cache` field is present only if the remote session produced cache files and the login belongs to the client. `404 SESSION_NOT_FOUND` if the remote session does not exist. +The `cache` field is present only if the remote session produced cache files and the login belongs to the client. Each cache key is the file path relative to the session cache directory, with `/` separators and no `.json` suffix, so a file in a subdirectory keeps its place on the client. The server rejects a cache key with a drive letter, a root, a `..` segment, or an empty segment, and so does the client. `404 SESSION_NOT_FOUND` if the remote session does not exist. --- diff --git a/docs/dev/rest-api/remote-sessions.md b/docs/dev/rest-api/remote-sessions.md index 30cbbcfc..52b6b8b0 100644 --- a/docs/dev/rest-api/remote-sessions.md +++ b/docs/dev/rest-api/remote-sessions.md @@ -124,11 +124,13 @@ hood the client walks a remote session through its lifecycle. the client region itself - Track-selection hints (`range_`, `vcodec`, `quality`, `best_available`) so the server fetches the right manifests - - Your local per-service **cache files** (e.g. refreshed tokens), but only the - files for the active profile. The client withholds a file whose name embeds - a hash of another credential, or the name of another profile. One profile's - tokens therefore never reach the server while you use a different profile. - At worst, a withheld file makes the server authenticate again + - Your local per-service **cache files** (e.g. refreshed tokens), including + files in subdirectories of the service cache directory, but only the files + for the active profile. The client withholds a file whose path, in any + directory or file name, embeds a hash of another credential or the name of + another profile. One profile's tokens + therefore never reach the server while you use a different profile. At + worst, a withheld file makes the server authenticate again The server uses its own accounts only when the operator lists the service in `serve.server_accounts` and gives your API key `server_accounts` in `serve.users`. @@ -430,14 +432,28 @@ _sessions/": "...base64(zlib(bytes))..." + } +} ``` +Both sides reject a cache key with a drive letter, a root, a `..` segment, an empty +segment, or more than 8 segments before it becomes a path, so a peer cannot write +outside the cache directory. Both sides also refuse a `cache` map with more than +64 entries. The server skips a cache key it cannot write, such as one that names an +existing file as a directory, and logs a warning. + ### Remote session info response `GET /api/session/{id}`: diff --git a/tests/core/test_sanitize.py b/tests/core/test_sanitize.py index 0d1ddeba..90434d2b 100644 --- a/tests/core/test_sanitize.py +++ b/tests/core/test_sanitize.py @@ -10,16 +10,37 @@ def test_safe_cache_key_rejects_backslash_traversal(): # "..\\..\\secret" as one filename; written as ".json" it escapes the # cache dir on Windows. PureWindowsPath handling must reject it everywhere. assert safe_cache_key("..\\..\\secret") is None - assert safe_cache_key("sub\\dir") is None + assert safe_cache_key("sub\\..\\secret") is None -def test_safe_cache_key_rejects_forward_slash_and_dotdot(): +def test_safe_cache_key_rejects_dotdot_and_empty_segments(): assert safe_cache_key("../secret") is None - assert safe_cache_key("a/b") is None + assert safe_cache_key("a/../b") is None + assert safe_cache_key("a/./b") is None assert safe_cache_key("..") is None assert safe_cache_key(".") is None assert safe_cache_key("") is None + assert safe_cache_key("a//b") is None + + +def test_safe_cache_key_rejects_absolute_paths_and_drives(): + assert safe_cache_key("/etc/passwd") is None + assert safe_cache_key("\\x") is None + assert safe_cache_key("C:/x") is None + assert safe_cache_key("C:x") is None + assert safe_cache_key("//server/share/x") is None + assert safe_cache_key("a/b:stream") is None + assert safe_cache_key("a\x00b") is None def test_safe_cache_key_accepts_plain_filename(): assert safe_cache_key("valid-key_123") == "valid-key_123" + + +def test_safe_cache_key_accepts_nested_key_and_normalises_separators(): + # A Cacher key with a subdirectory, as NF uses ("session_web/"), keeps its + # posix form; a backslash from a Windows peer names the same nested file. + assert safe_cache_key("session_web/abc") == "session_web/abc" + assert safe_cache_key("MSL/a/b") == "MSL/a/b" + assert safe_cache_key("session_web\\abc") == "session_web/abc" + assert safe_cache_key("/".join(["d"] * 9)) is None diff --git a/tests/remote/unit/test_remote_service_helpers.py b/tests/remote/unit/test_remote_service_helpers.py index d866e7fd..0319df2f 100644 --- a/tests/remote/unit/test_remote_service_helpers.py +++ b/tests/remote/unit/test_remote_service_helpers.py @@ -318,6 +318,149 @@ def test_load_cache_files_wiring(tmp_path, monkeypatch) -> None: assert set(sent) == {"tokens_default", "session_guid"} +def test_load_cache_files_forwards_nested_keys(tmp_path, monkeypatch) -> None: + """A service that keeps its cache in subfolders (NF: session_web/.json) must have + those files forwarded under the Cacher key, and the credential filter must still apply + to the basename of a nested file.""" + import base64 + import json + import logging + import zlib + from types import SimpleNamespace + + from unshackle.core.config import config + from unshackle.core.credential import Credential + from unshackle.core.remote_service import RemoteService + + tag = "TESTSVC" + cache_dir = tmp_path / tag + active = Credential("a@example.com", "pw") + other = Credential("other@example.com", "pw2") + for rel in ( + "tokens", + f"session_web/{active.sha1}", + f"session_web/{other.sha1}", + "session_androidtv/default", + "MSL/web/keys", + "MSL/web/keys.v2", + f"{other.sha1}/state", + "titles_abc", + "sub/titles_abc", + ): + path = cache_dir / f"{rel}.json" + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text(json.dumps({"k": rel})) + + monkeypatch.setattr(config.directories, "cache", tmp_path) + monkeypatch.setattr(config, "credentials", {tag: {"default": f"{active.username}:{active.password}"}}) + stub = SimpleNamespace(service_tag=tag, log=logging.getLogger("test")) + + sent = RemoteService.load_cache_files(stub, None) + assert set(sent) == { + "tokens", + f"session_web/{active.sha1}", + "session_androidtv/default", + "MSL/web/keys", + "MSL/web/keys.v2", + } + payload = json.loads(zlib.decompress(base64.b64decode(sent[f"session_web/{active.sha1}"]))) + assert payload == {"k": f"session_web/{active.sha1}"} + + +def test_save_returned_cache_writes_nested_keys_and_rejects_escapes(tmp_path, monkeypatch) -> None: + import base64 + import logging + import zlib + from types import SimpleNamespace + + from unshackle.core.config import config + from unshackle.core.remote_service import RemoteService + + tag = "TESTSVC" + monkeypatch.setattr(config.directories, "cache", tmp_path) + stub = SimpleNamespace(service_tag=tag, log=logging.getLogger("test")) + + def blob(text: str) -> str: + return base64.b64encode(zlib.compress(text.encode())).decode("ascii") + + RemoteService.save_returned_cache( + stub, + { + "tokens": blob("flat"), + "session_web/abc": blob("nested"), + "session_web\\def": blob("windows"), + "MSL/keys.v2": blob("dotted"), + "../escape": blob("bad"), + "/abs": blob("bad"), + "C:/x": blob("bad"), + }, + ) + + assert (tmp_path / tag / "tokens.json").read_text() == "flat" + assert (tmp_path / tag / "session_web" / "abc.json").read_text() == "nested" + assert (tmp_path / tag / "session_web" / "def.json").read_text() == "windows" + assert (tmp_path / tag / "MSL" / "keys.v2.json").read_text() == "dotted" + assert not (tmp_path / "escape.json").exists() + written = sorted(p.relative_to(tmp_path).as_posix() for p in tmp_path.rglob("*.json")) + assert written == [ + f"{tag}/MSL/keys.v2.json", + f"{tag}/session_web/abc.json", + f"{tag}/session_web/def.json", + f"{tag}/tokens.json", + ] + + +def test_save_returned_cache_ignores_oversized_payload(tmp_path, monkeypatch) -> None: + import logging + from types import SimpleNamespace + + from unshackle.core.api.sanitize import MAX_SESSION_CACHE_KEYS + from unshackle.core.config import config + from unshackle.core.remote_service import RemoteService + + monkeypatch.setattr(config.directories, "cache", tmp_path) + stub = SimpleNamespace(service_tag="TESTSVC", log=logging.getLogger("test")) + RemoteService.save_returned_cache(stub, {f"k{i}": "x" for i in range(MAX_SESSION_CACHE_KEYS + 1)}) + assert not list(tmp_path.rglob("*.json")) + + +@pytest.mark.asyncio +async def test_session_delete_returns_nested_cache_keys(tmp_path, monkeypatch) -> None: + """The server harvests a session's whole cache tree and keys each file by its path + relative to the session cache dir, so it round-trips to the same Cacher key.""" + import base64 + import json + import zlib + from types import SimpleNamespace + + from unshackle.core.api import handlers, session_store + + cache_tag = "_sessions/abc/sid/TESTSVC" + cache_dir = tmp_path / cache_tag + for rel in ("tokens", "session_web/abc", "titles_x", "MSL/web/keys", "MSL/keys.v2"): + path = cache_dir / f"{rel}.json" + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text(rel) + monkeypatch.setattr(handlers.config.directories, "cache", tmp_path) + + session = SimpleNamespace(cache_tag=cache_tag, client_auth=True, input_bridge=None) + + async def fake_validated(session_id, request): + return session + + class FakeStore: + async def delete(self, session_id): + return True + + monkeypatch.setattr(handlers, "get_validated_session", fake_validated) + monkeypatch.setattr(session_store, "get_session_store", lambda: FakeStore()) + + response = await handlers.session_delete_handler("sid", None) + body = json.loads(response.body) + assert set(body["cache"]) == {"tokens", "session_web/abc", "MSL/web/keys", "MSL/keys.v2"} + assert zlib.decompress(base64.b64decode(body["cache"]["session_web/abc"])) == b"session_web/abc" + + def test_resolve_manifest_data_matches_across_per_range_manifests() -> None: """One ISM manifest per range. The HDR10 track must take the quality level from its own manifest, not the same-sized one in the SDR manifest, in any manifest order.""" @@ -355,3 +498,41 @@ def test_resolve_manifest_data_matches_across_per_range_manifests() -> None: resolve_manifest_data(tracks, manifests) assert remote_hdr.data["ism"]["quality_level"].get("CodecPrivateData") == VIDEO_HEVC_PQ_CPD + + +def test_session_create_writes_nested_cache_and_skips_colliding_keys(tmp_path, monkeypatch) -> None: + """The server writes a nested key under the session cache dir with the ``.json`` + suffix appended, not substituted, and a key that collides with an existing file + (``x`` then ``x.json/y``) is skipped instead of failing the whole create.""" + import asyncio + import base64 + import logging + import zlib + from types import SimpleNamespace + + from unshackle.core.api import handlers + + class Stop(Exception): + pass + + def blob(text: str) -> str: + return base64.b64encode(zlib.compress(text.encode())).decode("ascii") + + monkeypatch.setattr(handlers.config.directories, "cache", tmp_path) + monkeypatch.setattr(handlers, "validate_service", lambda service, request=None: service) + monkeypatch.setattr(handlers, "resolve_handler_proxy", lambda *args: (None, [])) + monkeypatch.setattr(handlers, "server_account_for", lambda *args: False) + monkeypatch.setattr(handlers.Services, "load", lambda service: type("Svc", (), {})) + monkeypatch.setattr( + handlers, + "create_service_instance", + lambda *args, **kwargs: (SimpleNamespace(log=logging.getLogger("Svc")), None, None), + ) + monkeypatch.setattr(handlers, "InputBridge", lambda: (_ for _ in ()).throw(Stop())) + + cache = {"x": blob("flat"), "x.json/y": blob("collides"), "MSL/keys.v2": blob("dotted")} + response = asyncio.run(handlers.session_create_handler({"service": "TESTSVC", "title_id": "t", "cache": cache})) + assert response.status == 500 # the Stop stand-in fired after the cache write + + written = {p.name: p.read_text() for p in tmp_path.rglob("*.json") if p.is_file()} + assert written == {"x.json": "flat", "keys.v2.json": "dotted"} diff --git a/unshackle/core/api/handlers.py b/unshackle/core/api/handlers.py index 313e931c..87d403f1 100644 --- a/unshackle/core/api/handlers.py +++ b/unshackle/core/api/handlers.py @@ -21,7 +21,7 @@ from aiohttp import web from unshackle.core.api.compression import safe_inflate from unshackle.core.api.errors import APIError, APIErrorCode, categorize_exception, handle_api_exception from unshackle.core.api.input_bridge import AuthStatus, InputBridge -from unshackle.core.api.sanitize import safe_cache_key, sanitize_log +from unshackle.core.api.sanitize import MAX_SESSION_CACHE_KEYS, safe_cache_key, sanitize_log from unshackle.core.api.session_log import SessionLogBuffer, SessionLogMirror, capture_service_logs from unshackle.core.api.session_store import SessionStore from unshackle.core.cacher import Cacher @@ -1111,7 +1111,6 @@ def drm_preference_name(track: Any) -> Optional[str]: MANIFEST_DATA_KEYS = ("hls", "dash", "ism") -MAX_SESSION_CACHE_KEYS = 64 def serialize_track_data(track: Any) -> Optional[Dict[str, Any]]: @@ -3016,7 +3015,12 @@ async def session_create_handler(data: Dict[str, Any], request: Optional[web.Req if not isinstance(content, str): raise APIError(APIErrorCode.INVALID_INPUT, "cache values must be base64 strings") decompressed = safe_inflate(base64.b64decode(content)).decode("utf-8") - (cache_dir / safe_name).with_suffix(".json").write_text(decompressed, encoding="utf-8") + target = cache_dir / f"{safe_name}.json" + try: + target.parent.mkdir(parents=True, exist_ok=True) + target.write_text(decompressed, encoding="utf-8") + except OSError as e: + log.warning(f"Skipping session cache key {sanitize_log(key)}: {e}") bridge = InputBridge() service_instance._input_bridge = bridge @@ -4452,12 +4456,14 @@ async def session_delete_handler(session_id: str, request: Optional[web.Request] if cache_tag and session.client_auth: cache_dir = app_config.directories.cache / cache_tag if cache_dir.is_dir(): - for f in cache_dir.glob("*.json"): - if not f.stem.startswith("titles_"): - try: - cache_data[f.stem] = base64.b64encode(zlib.compress(f.read_bytes())).decode("ascii") - except OSError: - pass + for f in sorted(cache_dir.rglob("*.json")): + key = f.relative_to(cache_dir).as_posix()[: -len(".json")] + if f.stem.startswith("titles_") or not safe_cache_key(key): + continue + try: + cache_data[key] = base64.b64encode(zlib.compress(f.read_bytes())).decode("ascii") + except OSError: + pass await store.delete(session_id) diff --git a/unshackle/core/api/sanitize.py b/unshackle/core/api/sanitize.py index 2a82b3c1..570eb013 100644 --- a/unshackle/core/api/sanitize.py +++ b/unshackle/core/api/sanitize.py @@ -2,9 +2,13 @@ from __future__ import annotations +import re from pathlib import PureWindowsPath from typing import Optional +MAX_CACHE_KEY_DEPTH = 8 +MAX_SESSION_CACHE_KEYS = 64 + def sanitize_log(value: object) -> str: """Sanitise a value for safe logging by removing newlines and control characters.""" @@ -12,17 +16,28 @@ def sanitize_log(value: object) -> str: def safe_cache_key(key: object) -> Optional[str]: - """Return a bare filename for a peer-supplied cache key, or None if it escapes its directory. + """Return a relative posix path for a peer-supplied cache key, or None if it escapes its directory. - unshackle writes each cache key as ``.json`` inside a fixed directory. A cache key - that holds a path separator, ``..``, or an absolute path would let the peer write outside - that directory, so this function rejects anything that is not already a plain filename. + unshackle writes each cache key as ``.json`` inside a fixed directory, and a key may + hold ``/`` to place the file in a subdirectory (``Cacher`` keys such as ``session_web/``). + A cache key with a drive letter, a root, a ``..`` or ``.`` segment, an empty segment, a + control character, or more than ``MAX_CACHE_KEY_DEPTH`` segments would let the peer write + outside that directory or build an unbounded tree, so this function rejects it. PureWindowsPath treats both ``/`` and ``\\`` as separators, so this function rejects a cache key like ``..\\..\\secret`` even on POSIX (where backslash is a plain character). + The function turns a surviving backslash into ``/`` so the same cache key names the + same file on every platform. """ text = str(key) - name = PureWindowsPath(text).name - if not name or name != text or name in {".", ".."}: + if not text or any(ord(ch) < 32 or ch == "\x7f" for ch in text): return None - return name + path = PureWindowsPath(text) + if path.drive or path.root or path.anchor: + return None + parts = re.split(r"[\\/]", text) + if len(parts) > MAX_CACHE_KEY_DEPTH: + return None + if any(not part or part in {".", ".."} or part.endswith((" ", ".")) or ":" in part for part in parts): + return None + return "/".join(parts) diff --git a/unshackle/core/remote_service.py b/unshackle/core/remote_service.py index f8384af9..136a67da 100644 --- a/unshackle/core/remote_service.py +++ b/unshackle/core/remote_service.py @@ -1408,7 +1408,11 @@ class RemoteService: return from unshackle.core.api.compression import safe_inflate - from unshackle.core.api.sanitize import safe_cache_key + from unshackle.core.api.sanitize import MAX_SESSION_CACHE_KEYS, safe_cache_key + + if len(cache_data) > MAX_SESSION_CACHE_KEYS: + self.log.warning(f"Ignoring {len(cache_data)} cache files from server: more than {MAX_SESSION_CACHE_KEYS}") + return cache_dir = config.directories.cache / self.service_tag cache_dir.mkdir(parents=True, exist_ok=True) @@ -1420,7 +1424,10 @@ class RemoteService: continue try: decompressed = safe_inflate(base64.b64decode(content)) - (cache_dir / safe_name).with_suffix(".json").write_bytes(decompressed) + # Not with_suffix: that would turn the key "keys.v2" into "keys.json". + target = cache_dir / f"{safe_name}.json" + target.parent.mkdir(parents=True, exist_ok=True) + target.write_bytes(decompressed) except Exception as e: self.log.warning(f"Failed to save returned cache file '{safe_name}': {e}") @@ -1432,10 +1439,17 @@ class RemoteService: The client cannot rely on the server to filter, so it sends only the files it can tie to the active credential or profile, plus service-global state. At worst, a withheld file makes the server authenticate again. + + Each cache key is the file path relative to the service cache directory, + with posix separators and no ``.json`` suffix, so it equals the ``Cacher`` + cache key the service reads it with (``session_web/``). The relevance check + runs on every path segment, because a service may embed the credential + digest or profile name in a directory name as well as in the basename. """ import zlib from unshackle.commands.dl import dl + from unshackle.core.api.sanitize import safe_cache_key cache_dir = config.directories.cache / self.service_tag if not cache_dir.is_dir(): @@ -1450,13 +1464,14 @@ class RemoteService: foreign = set(profiles) - {active} if isinstance(profiles, dict) else set() files: Dict[str, str] = {} - for f in cache_dir.glob("*.json"): - if f.stem.startswith("titles_"): + for f in sorted(cache_dir.rglob("*.json")): + key = f.relative_to(cache_dir).as_posix()[: -len(".json")] + if f.stem.startswith("titles_") or not safe_cache_key(key): continue - if not cache_stem_is_relevant(f.stem, allowed, active, foreign): - self.log.debug(f"Withholding cache file from the remote server: {f.stem}") + if not all(cache_stem_is_relevant(part, allowed, active, foreign) for part in key.split("/")): + self.log.debug(f"Withholding cache file from the remote server: {key}") continue - files[f.stem] = base64.b64encode(zlib.compress(f.read_bytes())).decode("ascii") + files[key] = base64.b64encode(zlib.compress(f.read_bytes())).decode("ascii") return files