fix(api): refuse peer cache files that name types other than datetime

The server and the client both check a forwarded cache file before they write it, because Cacher decodes it with jsonpickle. The session-create cache write now runs off the event loop.

docs/dev/rest-api/remote-sessions.md

@@ -398,7 +398,8 @@ serve:
   cannot read each other's cached tokens. The server deletes the directory when the
   remote session ends. A remote session that uses a server account (`serve.server_accounts`)
   gets a persistent per-account cache instead, so the account's tokens outlive the
-  remote session.
+  remote session. The server and the client both refuse a forwarded cache file that is not
+  JSON or that names a type other than a date or time, and the service then authenticates again.
 - **The server redacts secrets in logs.** It redacts remote session IDs, service tags,
   and other user values before it writes them to the log.
 

tests/remote/unit/test_cache_payload_safety.py

@@ -0,0 +1,74 @@
+import base64
+import datetime as dt
+import logging
+import zlib
+from types import SimpleNamespace
+
+import pytest
+
+from unshackle.core.api import handlers
+from unshackle.core.api.sanitize import safe_cache_payload
+from unshackle.core.cacher import Cacher
+from unshackle.core.config import config
+from unshackle.core.remote_service import RemoteService
+
+pytestmark = pytest.mark.unit
+
+TAG = "TESTSVC"
+
+
+def b64z(text: str) -> str:
+    return base64.b64encode(zlib.compress(text.encode())).decode("ascii")
+
+
+@pytest.mark.parametrize("text", ['{"a": [1, "b", null]}', '"token"'])
+def test_plain_json_is_safe(text) -> None:
+    assert safe_cache_payload(text)
+
+
+@pytest.mark.parametrize(
+    "text",
+    [
+        "not json",
+        '{"py/object": "collections.OrderedDict"}',
+        '{"py/type": "builtins.dict"}',
+        '{"py/function": "builtins.print"}',
+        '{"py/repr": "builtins/1"}',
+        '{"py/reduce": [{"py/function": "builtins.print"}, {"py/tuple": ["x"]}]}',
+        '{"py/object": "datetime.datetime", "__reduce__": [{"py/type": "collections.OrderedDict"}, []]}',
+        '{"py/object": "datetime.datetime", "py/state": {}}',
+        "[" * 100000 + "]" * 100000,
+    ],
+)
+def test_unsafe_payload_is_refused(text) -> None:
+    assert safe_cache_payload(text) is False
+
+
+def save_returned(cache_map: dict) -> None:
+    RemoteService.save_returned_cache(SimpleNamespace(service_tag=TAG, log=logging.getLogger("test")), cache_map)
+
+
+@pytest.mark.parametrize(
+    "write", [lambda m: handlers.write_client_cache(m, TAG), save_returned], ids=["server", "client"]
+)
+def test_writer_skips_unsafe_entry(tmp_path, monkeypatch, write) -> None:
+    monkeypatch.setattr(config.directories, "cache", tmp_path)
+    write({"good": b64z('{"a": 1}'), "bad": b64z('{"py/function": "builtins.print"}')})
+    assert (tmp_path / TAG / "good.json").exists()
+    assert not (tmp_path / TAG / "bad.json").exists()
+
+
+@pytest.mark.parametrize(
+    "expiration",
+    [None, dt.datetime(2030, 1, 1, 12, 0, 0), dt.datetime(2030, 1, 1, 12, 0, 0, tzinfo=dt.timezone.utc)],
+    ids=["none", "naive", "aware"],
+)
+def test_real_cacher_file_round_trips(tmp_path, monkeypatch, expiration) -> None:
+    monkeypatch.setattr(config.directories, "cache", tmp_path / "src")
+    cache = Cacher(TAG, "tok")
+    cache.set({"token": "abc", "n": [1, 2]}, expiration=expiration)
+    text = cache.path.read_text(encoding="utf8")
+
+    monkeypatch.setattr(config.directories, "cache", tmp_path / "dst")
+    save_returned({"tok": b64z(text)})
+    assert Cacher(TAG).get("tok").data == {"token": "abc", "n": [1, 2]}

tests/remote/unit/test_remote_service_helpers.py

@@ -395,20 +395,20 @@ def test_save_returned_cache_writes_nested_keys_and_rejects_escapes(tmp_path, mo
     RemoteService.save_returned_cache(
         stub,
         {
-            "tokens": blob("flat"),
+            "tokens": blob('"flat"'),
-            "session_web/abc": blob("nested"),
+            "session_web/abc": blob('"nested"'),
-            "session_web\\def": blob("windows"),
+            "session_web\\def": blob('"windows"'),
-            "MSL/keys.v2": blob("dotted"),
+            "MSL/keys.v2": blob('"dotted"'),
-            "../escape": blob("bad"),
+            "../escape": blob('"bad"'),
-            "/abs": blob("bad"),
+            "/abs": blob('"bad"'),
-            "C:/x": blob("bad"),
+            "C:/x": blob('"bad"'),
         },
     )
 
-    assert (tmp_path / tag / "tokens.json").read_text() == "flat"
+    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" / "abc.json").read_text() == '"nested"'
-    assert (tmp_path / tag / "session_web" / "def.json").read_text() == "windows"
+    assert (tmp_path / tag / "session_web" / "def.json").read_text() == '"windows"'
-    assert (tmp_path / tag / "MSL" / "keys.v2.json").read_text() == "dotted"
+    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 == [
@@ -433,7 +433,7 @@ def test_save_returned_cache_drops_other_profiles_files(tmp_path, monkeypatch) -
     monkeypatch.setattr(config.directories, "cache", tmp_path)
     monkeypatch.setattr(config, "credentials", {tag: {"default": "a@example.com:pw", "work": "b@example.com:pw"}})
     stub = SimpleNamespace(service_tag=tag, log=logging.getLogger("test"))
-    blob = base64.b64encode(zlib.compress(b"x")).decode("ascii")
+    blob = base64.b64encode(zlib.compress(b'"x"')).decode("ascii")
 
     RemoteService.save_returned_cache(stub, {"tokens_work": blob, "tokens_default": blob, "tokens": blob}, "work")
 
@@ -560,12 +560,12 @@ def test_session_create_writes_nested_cache_and_skips_colliding_keys(tmp_path, m
     )
     monkeypatch.setattr("unshackle.core.api.session_store.get_session_store", lambda: (_ for _ in ()).throw(Stop()))
 
-    cache = {"x": blob("flat"), "x.json/y": blob("collides"), "MSL/keys.v2": blob("dotted")}
+    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"}
+    assert written == {"x.json": '"flat"', "keys.v2.json": '"dotted"'}
 
 
 class _SentCreate(Exception):

unshackle/core/api/handlers.py

@@ -23,7 +23,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, NoUserBridge
-from unshackle.core.api.sanitize import MAX_SESSION_CACHE_KEYS, safe_cache_key, sanitize_log
+from unshackle.core.api.sanitize import MAX_SESSION_CACHE_KEYS, safe_cache_key, safe_cache_payload, 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
@@ -316,9 +316,9 @@ def write_client_cache(cache_data: Any, cache_tag: str) -> None:
     """Write a client-sent ``cache`` map into the cache directory ``cache_tag``.
 
     Each cache key is a file path relative to that directory, and each value is the file as
-    base64 of zlib-compressed bytes. The function skips an unsafe key or a file it cannot write,
+    base64 of zlib-compressed bytes. The function skips an unsafe key, a file that names a type
-    and raises INVALID_INPUT for a map that is not an object, has too many entries, or holds a
+    other than a datetime, or a file it cannot write, and raises INVALID_INPUT for a map that is
-    value that is not a string.
+    not an object, has too many entries, or holds a value that is not a string.
     """
     if not isinstance(cache_data, dict) or len(cache_data) > MAX_SESSION_CACHE_KEYS:
         raise APIError(APIErrorCode.INVALID_INPUT, f"cache must hold at most {MAX_SESSION_CACHE_KEYS} entries")
@@ -332,6 +332,9 @@ def write_client_cache(cache_data: Any, cache_tag: str) -> None:
         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")
+        if not safe_cache_payload(decompressed):
+            log.warning(f"Rejecting session cache file {sanitize_log(key)}: it holds a type a peer may not send")
+            continue
         target = cache_dir / f"{safe_name}.json"
         try:
             target.parent.mkdir(parents=True, exist_ok=True)
@@ -3895,7 +3898,7 @@ async def session_create_handler(data: Dict[str, Any], request: Optional[web.Req
 
         cache_data = data.get("cache", {})
         if cache_data and session_cache_tag:
-            write_client_cache(cache_data, session_cache_tag)
+            await asyncio.to_thread(write_client_cache, cache_data, session_cache_tag)
 
         service_instance._input_bridge = bridge
 

unshackle/core/api/sanitize.py

@@ -2,6 +2,7 @@
 
 from __future__ import annotations
 
+import json
 import re
 from pathlib import PureWindowsPath
 from typing import Optional
@@ -41,3 +42,38 @@ def safe_cache_key(key: object) -> Optional[str]:
     if any(not part or part in {".", ".."} or part.endswith((" ", ".")) or ":" in part for part in parts):
         return None
     return "/".join(parts)
+
+
+SAFE_PICKLE_TAGS = frozenset({"py/object", "py/type", "py/reduce", "py/tuple", "py/set", "py/b64", "py/id"})
+SAFE_PICKLE_TYPES = frozenset(
+    {"datetime.datetime", "datetime.date", "datetime.time", "datetime.timedelta", "datetime.timezone"}
+)
+
+
+def safe_cache_payload(text: str) -> bool:
+    """True when a peer-supplied cache file is JSON that jsonpickle decodes to plain data and datetimes only.
+
+    ``Cacher`` decodes cache files with jsonpickle, which imports and calls any type a file names,
+    so a file from another machine may name the datetime types and nothing else.
+    """
+    try:
+        root = json.loads(text)
+    except (ValueError, RecursionError):
+        return False
+    stack = [root]
+    while stack:
+        node = stack.pop()
+        if isinstance(node, list):
+            stack.extend(node)
+        elif isinstance(node, dict):
+            for key, value in node.items():
+                if key.startswith("py/") and key not in SAFE_PICKLE_TAGS:
+                    return False
+                if key in ("py/object", "py/type") and not (isinstance(value, str) and value in SAFE_PICKLE_TYPES):
+                    return False
+                if key in ("py/reduce", "__reduce__") and not (
+                    isinstance(value, list) and value and isinstance(value[0], dict) and value[0].keys() == {"py/type"}
+                ):
+                    return False
+                stack.append(value)
+    return True

unshackle/core/remote_service.py

@@ -1977,12 +1977,13 @@ class RemoteService:
         can forward them back, skipping interactive auth. The same filter that
         :meth:`load_cache_files` applies drops a file that belongs to another
         credential or profile, so it cannot overwrite that profile's local file.
+        The client also refuses a file that names a type other than a datetime.
         """
         if not cache_data:
             return
 
         from unshackle.core.api.compression import safe_inflate
-        from unshackle.core.api.sanitize import MAX_SESSION_CACHE_KEYS, safe_cache_key
+        from unshackle.core.api.sanitize import MAX_SESSION_CACHE_KEYS, safe_cache_key, safe_cache_payload
 
         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}")
@@ -2003,6 +2004,11 @@ class RemoteService:
                 continue
             try:
                 decompressed = safe_inflate(base64.b64decode(content))
+                if not safe_cache_payload(decompressed.decode("utf-8")):
+                    self.log.warning(
+                        f"Rejecting cache file '{safe_name}' from the server: it holds a type a peer may not send"
+                    )
+                    continue
                 # 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)