Isolate demo chat history per visitor

deeptutor/api/main.py

@@ -375,9 +375,16 @@ async def demo_rate_limit(request, call_next):
 
 # All other routers require a valid session when AUTH_ENABLED=true.
 # require_auth is a no-op when AUTH_ENABLED=false, so this is safe for local use.
-from deeptutor.api.routers.auth import require_admin, require_auth  # noqa: E402
+from deeptutor.api.routers.auth import (  # noqa: E402
+    bind_demo_visitor,
+    require_admin,
+    require_auth,
+)
 
-_auth = [Depends(require_auth)]
+# require_auth installs the current user; bind_demo_visitor partitions the demo
+# session store per visitor (no-op unless DEMO_MODE is on). Both run on every
+# authenticated router below.
+_auth = [Depends(require_auth), Depends(bind_demo_visitor)]
 # Partner data is anchored at the admin workspace (data/partners) and shared
 # process-wide, so management is admin-gated in multi-user deployments
 # (single-user local runs are implicitly admin — no behaviour change there).

deeptutor/api/routers/auth.py

@@ -263,6 +263,27 @@ async def require_auth(
     return payload
 
 
+async def bind_demo_visitor(
+    x_demo_visitor: str | None = Header(default=None, alias="X-Demo-Visitor"),
+) -> None:
+    """Bind the per-visitor demo id from the ``X-Demo-Visitor`` header.
+
+    A no-op unless ``DEMO_MODE`` is on, so private deployments are unaffected.
+    In demo mode this partitions the shared in-memory session store per visitor
+    (see ``get_sqlite_session_store``) so concurrent visitors don't see each
+    other's chat history.
+
+    ``async def`` for the same reason as ``require_auth``: the ContextVar set
+    must run on the event loop to stay visible to the endpoint. HTTP callers
+    ignore the reset token — the request task's context is discarded when it
+    ends. Registered globally via the ``_auth`` dependency list in ``main``.
+    """
+    from deeptutor.services.demo import is_demo_mode, set_visitor_id
+
+    if is_demo_mode():
+        set_visitor_id(x_demo_visitor)
+
+
 class _WsAuthFailed:
     """Sentinel: ws_require_auth failed and closed the WebSocket."""
 
@@ -291,6 +312,16 @@ async def ws_require_auth(ws: WebSocket) -> _CtxToken | _WsAuthFailed:
         finally:
             reset_current_user(user_token)
     """
+    # Demo per-visitor isolation: read the visitor id from the WS query param
+    # (cookies aren't reliably set in the demo's proxy setup, so the client
+    # sends it explicitly). Set before the AUTH_ENABLED branch so it applies
+    # whether or not auth is on. Not reset here — the connection's task context
+    # is discarded when the handler task ends, same as the HTTP path.
+    from deeptutor.services.demo import is_demo_mode, set_visitor_id
+
+    if is_demo_mode():
+        set_visitor_id(ws.query_params.get("visitor"))
+
     if not AUTH_ENABLED:
         return _install_current_user(None)
 

deeptutor/services/demo/__init__.py

@@ -7,11 +7,21 @@
     get_demo_limiter,
     is_demo_mode,
 )
+from .visitor import (
+    get_visitor_id,
+    reset_visitor_id,
+    sanitize_visitor_id,
+    set_visitor_id,
+)
 
 __all__ = [
     "DemoRateLimiter",
     "RateDecision",
     "client_ip",
     "get_demo_limiter",
+    "get_visitor_id",
     "is_demo_mode",
+    "reset_visitor_id",
+    "sanitize_visitor_id",
+    "set_visitor_id",
 ]

deeptutor/services/demo/visitor.py

@@ -0,0 +1,64 @@
+"""Per-visitor identity for the public demo.
+
+Why this exists
+---------------
+DeepTutor is single-user by design: sessions carry no owner column, and every
+anonymous demo visitor resolves to the same ``local_admin_user``. In demo mode
+the session store is one shared in-memory DB for the whole process, so a naive
+``list_sessions`` surfaces *every* visitor's chats to *everyone* — reproduced by
+opening the demo in a second browser or incognito window.
+
+This module carries a per-visitor id (minted client-side and sent as the
+``X-Demo-Visitor`` header on HTTP and the ``visitor`` query param on the chat
+WebSocket) through a request-local :class:`~contextvars.ContextVar`, mirroring
+``multi_user.context``'s ``_current_user``. ``get_sqlite_session_store`` reads
+it to hand each visitor a physically separate in-memory store, so history stays
+both isolated between visitors and ephemeral (in-memory, gone on restart).
+
+It is **isolation, not authentication**: the id is an opaque client-chosen
+token, not a security boundary. It only partitions the demo's ephemeral store.
+Non-demo deployments never set it, so the value is inert outside the demo.
+"""
+
+from __future__ import annotations
+
+from contextvars import ContextVar, Token
+import re
+
+# Request-local visitor id. Empty string means "no visitor id supplied" — such
+# callers share a single default bucket (see ``get_sqlite_session_store``).
+_visitor_id: ContextVar[str] = ContextVar("deeptutor_demo_visitor_id", default="")
+
+# Client-chosen ids are opaque, but they become dict keys for live in-memory
+# stores, so bound both the character set and the length to keep a hostile or
+# buggy client from minting pathological keys.
+_MAX_LEN = 64
+_ALLOWED = re.compile(r"[^A-Za-z0-9_-]")
+
+
+def sanitize_visitor_id(raw: str | None) -> str:
+    """Normalise an untrusted visitor id to ``[A-Za-z0-9_-]{1,64}`` or ``""``."""
+    if not raw:
+        return ""
+    cleaned = _ALLOWED.sub("", raw)[:_MAX_LEN]
+    return cleaned
+
+
+def set_visitor_id(value: str | None) -> Token[str]:
+    """Bind the current visitor id (sanitized). Returns a reset token.
+
+    Follows the same lifetime rules as ``multi_user.context.set_current_user``:
+    HTTP callers may ignore the token (the request task's context is discarded
+    when it ends), while long-lived WebSocket handlers keep it and call
+    :func:`reset_visitor_id` in their ``finally`` block.
+    """
+    return _visitor_id.set(sanitize_visitor_id(value))
+
+
+def reset_visitor_id(token: Token[str]) -> None:
+    _visitor_id.reset(token)
+
+
+def get_visitor_id() -> str:
+    """The current visitor id, or ``""`` when none was supplied."""
+    return _visitor_id.get()

deeptutor/services/session/sqlite_store.py

@@ -106,26 +106,31 @@ def __init__(self, db_path: Path | None = None, *, in_memory: bool = False) -> N
             # destroyed the instant no connection is open. The keep-alive
             # connection below stays open for the store's lifetime to pin it.
             # A unique name keeps two in-memory stores from sharing one cache.
-            self.db_path = f"file:deeptutor-mem-{uuid.uuid4().hex}?mode=memory&cache=shared"
+            # ``db_path`` is a ``file:`` URI here and a filesystem ``Path`` on
+            # the on-disk branch below; both are accepted by ``sqlite3.connect``.
+            self.db_path: str | Path = (
+                f"file:deeptutor-mem-{uuid.uuid4().hex}?mode=memory&cache=shared"
+            )
             self._keepalive = sqlite3.connect(self.db_path, uri=True)
             self._lock = asyncio.Lock()
             self._initialize()
             return
 
         path_service = get_path_service()
-        self.db_path = db_path or path_service.get_chat_history_db()
-        self.db_path.parent.mkdir(parents=True, exist_ok=True)
-        self._migrate_legacy_db(path_service)
+        disk_path = db_path or path_service.get_chat_history_db()
+        disk_path.parent.mkdir(parents=True, exist_ok=True)
+        self.db_path = disk_path
+        self._migrate_legacy_db(path_service, disk_path)
         self._lock = asyncio.Lock()
         self._initialize()
 
-    def _migrate_legacy_db(self, path_service) -> None:
+    def _migrate_legacy_db(self, path_service, db_path: Path) -> None:
         """Move the legacy ``data/chat_history.db`` into ``data/user/`` once."""
         legacy_path = path_service.project_root / "data" / "chat_history.db"
-        if self.db_path.exists() or not legacy_path.exists() or legacy_path == self.db_path:
+        if db_path.exists() or not legacy_path.exists() or legacy_path == db_path:
             return
         try:
-            os.replace(legacy_path, self.db_path)
+            os.replace(legacy_path, db_path)
         except OSError:
             # Fall back to leaving the legacy DB in place if an OS-level move
             # is not possible; the new DB path will be initialized empty.
@@ -1847,22 +1852,30 @@ async def get_entry_categories(self, entry_id: int) -> list[dict[str, Any]]:
 
 _instances: dict[str, SQLiteSessionStore] = {}
 
-# Stable memoisation key for the demo-mode ephemeral store. Distinct from any
-# real on-disk path so demo and non-demo stores never collide, and fixed so a
-# process reuses one in-memory DB (each ``SQLiteSessionStore(in_memory=True)``
-# otherwise mints a fresh, isolated shared-cache namespace).
+# Memoisation-key prefix for demo-mode ephemeral stores. Distinct from any real
+# on-disk path so demo and non-demo stores never collide. The current visitor
+# id is appended (see ``get_sqlite_session_store``) so each visitor reuses one
+# in-memory DB across requests while staying isolated from other visitors; the
+# bare prefix is the shared bucket for callers with no visitor id.
 _DEMO_INSTANCE_KEY = "__demo_in_memory__"
 
 
 def get_sqlite_session_store() -> SQLiteSessionStore:
     # Demo mode: chat history must stay ephemeral (never written to disk), so
-    # serve a shared in-memory store instead of the on-disk one.
-    from deeptutor.services.demo import get_demo_limiter
+    # serve an in-memory store instead of the on-disk one. It is keyed per
+    # visitor (a client-supplied id in a ContextVar) so concurrent demo
+    # visitors never see each other's history; a visitor with no id shares one
+    # default bucket. Each in-memory store mints a unique ``db_path`` (see
+    # ``SQLiteSessionStore.__init__``), so ``get_turn_runtime_manager`` — which
+    # caches by ``db_path`` — stays isolated per visitor automatically.
+    from deeptutor.services.demo import get_demo_limiter, get_visitor_id
 
     if get_demo_limiter().enabled:
-        if _DEMO_INSTANCE_KEY not in _instances:
-            _instances[_DEMO_INSTANCE_KEY] = SQLiteSessionStore(in_memory=True)
-        return _instances[_DEMO_INSTANCE_KEY]
+        visitor = get_visitor_id()
+        key = f"{_DEMO_INSTANCE_KEY}{visitor}" if visitor else _DEMO_INSTANCE_KEY
+        if key not in _instances:
+            _instances[key] = SQLiteSessionStore(in_memory=True)
+        return _instances[key]
 
     db_path = get_path_service().get_chat_history_db().resolve()
     key = str(db_path)

tests/api/test_demo_visitor_isolation.py

@@ -0,0 +1,96 @@
+"""Integration test: the demo visitor id survives the request lifecycle.
+
+The store-level isolation is covered by
+``tests/services/demo/test_demo_visitor_isolation.py``. This test pins the part
+that unit tests can't: that the real ``bind_demo_visitor`` dependency actually
+sets the visitor ContextVar in a way the endpoint sees. That is exactly the
+regression class behind issue #481 (a sync dependency's ContextVar set runs in a
+worker thread and is discarded), so we exercise it through the ASGI stack with
+``TestClient``, mirroring the minimal-app style of ``test_demo_rate_limit.py``.
+"""
+
+from __future__ import annotations
+
+from pathlib import Path
+
+import pytest
+
+pytest.importorskip("fastapi")
+
+from fastapi import Depends, FastAPI
+from fastapi.testclient import TestClient
+
+from deeptutor.api.routers.auth import bind_demo_visitor
+import deeptutor.services.demo.rate_limiter as rate_limiter
+from deeptutor.services.path_service import PathService
+from deeptutor.services.session import sqlite_store
+from deeptutor.services.session.sqlite_store import get_sqlite_session_store
+
+
+@pytest.fixture
+def demo_client(monkeypatch: pytest.MonkeyPatch, tmp_path: Path):
+    monkeypatch.setenv("DEMO_MODE", "true")
+    monkeypatch.setattr(rate_limiter, "_limiter", None)
+    monkeypatch.setattr(sqlite_store, "_instances", {})
+
+    service = PathService.get_instance()
+    saved = (service._project_root, service._user_data_dir)
+    service._project_root = tmp_path.resolve()
+    service._user_data_dir = (tmp_path / "data" / "user").resolve()
+
+    app = FastAPI(dependencies=[Depends(bind_demo_visitor)])
+
+    @app.post("/sessions")
+    async def create(title: str):
+        await get_sqlite_session_store().create_session(title=title)
+        return {"ok": True}
+
+    @app.get("/sessions")
+    async def listing():
+        rows = await get_sqlite_session_store().list_sessions()
+        return {"titles": [r["title"] for r in rows]}
+
+    try:
+        yield TestClient(app)
+    finally:
+        (service._project_root, service._user_data_dir) = saved
+
+
+def _h(visitor: str) -> dict[str, str]:
+    return {"X-Demo-Visitor": visitor}
+
+
+def test_visitors_do_not_see_each_others_history(demo_client):
+    demo_client.post("/sessions", params={"title": "alice chat"}, headers=_h("alice"))
+
+    # Bob (a second browser / incognito) sees none of Alice's history.
+    assert demo_client.get("/sessions", headers=_h("bob")).json()["titles"] == []
+    # Alice still sees her own.
+    assert demo_client.get("/sessions", headers=_h("alice")).json()["titles"] == ["alice chat"]
+
+
+def test_missing_header_does_not_inherit_previous_visitor(demo_client):
+    """A request with no visitor id must not leak into another's bucket."""
+    demo_client.post("/sessions", params={"title": "alice chat"}, headers=_h("alice"))
+
+    # No header → default bucket, which is empty (not Alice's).
+    assert demo_client.get("/sessions").json()["titles"] == []
+
+
+@pytest.mark.asyncio
+async def test_ws_require_auth_binds_visitor_from_query_param(monkeypatch):
+    """The chat WebSocket carries the visitor id as a query param (WS upgrades
+    can't set custom headers), and ``ws_require_auth`` must bind it."""
+    monkeypatch.setenv("DEMO_MODE", "true")
+    monkeypatch.setattr(rate_limiter, "_limiter", None)
+
+    from deeptutor.api.routers.auth import ws_require_auth
+    from deeptutor.services.demo import get_visitor_id
+
+    class _FakeWS:
+        # AUTH is disabled by default, so ws_require_auth only touches query_params.
+        query_params = {"visitor": "alice"}
+        cookies: dict[str, str] = {}
+
+    await ws_require_auth(_FakeWS())
+    assert get_visitor_id() == "alice"