From 2fa67ad699cf3953957fd74d783dce13355401fa Mon Sep 17 00:00:00 2001 From: nesquena-hermes Date: Mon, 15 Jun 2026 21:55:31 -0700 Subject: [PATCH] Release PL (v0.51.451): show named-profile external sessions in the all-profiles list (#4067) (#4296) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Release PL (v0.51.451): show named-profile external sessions in the all-profiles list (#4067, #4065) All-profiles session enumeration now scans across profiles only when the request explicitly asks (all-profiles view); cross-profile import resolves to the active profile's state.db for unqualified requests (foreign id -> 404), mirroring the #3982/#3991 detail/export profile-scoping gate. Co-authored-by: rodboev * gate fix (Codex CORE + Opus): close cross-profile leak on the import_cli refresh/existing-session branch The already-imported branch of _handle_session_import_cli forced allow_all_profiles=True whenever the globally-stored session carried a profile, with no active-profile visibility gate — so an unqualified request from profile A could read/refresh a session imported under profile B's live state.db (both advisors reproduced the foreign-transcript leak). Now gated exactly like the /api/session detail + export endpoints: unqualified requests require _session_visible_to_active_profile; explicit all_profiles requires a matching profile. + 3 regression tests. * gate fix (Codex SILENT, re-gate): thread source_filter into the all-profiles CLI scan (models.py:4113) The all-profiles branch keys its cache on source_filter but called _load_cli_sessions_uncached without it, so /api/sessions?all_profiles=1 with a source filter returned every-source sessions under a filtered key. + regression test. --------- Co-authored-by: nesquena-hermes --- CHANGELOG.md | 6 + api/models.py | 274 +++++++++++------- api/routes.py | 135 ++++++--- static/sessions.js | 17 +- tests/test_claude_code_session_import.py | 16 +- tests/test_cli_session_tool_metadata.py | 4 +- tests/test_gateway_sync.py | 117 +++++++- ...est_issue1611_session_profile_filtering.py | 126 ++++++++ .../test_issue3930_source_filter_pushdown.py | 44 ++- ...ssue4067_import_cli_cross_profile_guard.py | 133 +++++++++ tests/test_session_cli_scan_fast_path.py | 6 +- .../test_session_import_cli_fallback_model.py | 65 ++--- 12 files changed, 737 insertions(+), 206 deletions(-) create mode 100644 tests/test_issue4067_import_cli_cross_profile_guard.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 79de4625..fbde39a1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,12 @@ - **Windows pytest-harness compatibility (#3664).** Hardened the test suite to run on Windows: profile-home fallback paths are path-normalized, strict POSIX file-mode (`0o600`) assertions are gated behind `os.name != "nt"` (Linux still asserts them at full strictness), the conftest cleanup handles Windows process-tree/port teardown and the Py3.12+ `shutil.rmtree` `onexc` shim, and tests that require `fork`/`fcntl` carry `@requires_fork` / `@requires_fcntl` markers (which never skip on Linux). Test-only — no runtime or app behavior change, no Linux CI behavior change. (#4254, #4255, #4256, #4257, #4259, #4263, #4266, #4274) +## [v0.51.451] — 2026-06-16 — Release PL (show named-profile external sessions in the all-profiles list) + +### Fixed + +- **The "all profiles" session list now includes CLI / Telegram / API sessions that live under named profiles.** Previously the all-profiles view enumerated only the active profile's external (non-WebUI) sessions, so a session started by a named-profile CLI, Telegram, or API client never appeared even with "all profiles" selected. Enumeration now scans across profiles when (and only when) the request explicitly asks for the all-profiles view. The cross-profile boundary is preserved: an unqualified single-profile request still resolves to the active profile's `state.db` (a foreign session id returns 404, the same not-found shape as a missing session), and the client only sends the cross-profile import fields from the all-profiles view — mirroring the active-profile scoping already applied to session detail loads and exports. (#4067, #4065) + ## [v0.51.450] — 2026-06-16 — Release PK (preserve custom provider namespace prefixes in model normalization) ### Fixed diff --git a/api/models.py b/api/models.py index f5b409ce..7a928526 100644 --- a/api/models.py +++ b/api/models.py @@ -3825,14 +3825,70 @@ def _resolve_cli_sessions_context(source_filter=None): return hermes_home, db_path, cli_profile, cache_key +def _all_profiles_cli_contexts() -> tuple[list[tuple[Path, Path, str | None]], tuple]: + """Return per-profile CLI scan contexts plus a cache key fragment.""" + try: + from api.profiles import ( + _profiles_root, + get_active_profile_name, + get_hermes_home_for_profile, + list_profiles_api, + ) + except Exception: + return [], () + + contexts: list[tuple[Path, Path, str | None]] = [] + cache_entries: list[tuple[str, str, object]] = [] + seen_homes: set[str] = set() + + def _add_context(profile_name) -> None: + try: + hermes_home = Path(get_hermes_home_for_profile(profile_name)).expanduser().resolve() + except Exception: + return + home_key = _path_cache_key(hermes_home) + if not home_key or home_key in seen_homes: + return + seen_homes.add(home_key) + db_path = hermes_home / 'state.db' + profile_value = str(profile_name or 'default').strip() or 'default' + contexts.append((hermes_home, db_path, profile_value)) + cache_entries.append((home_key, profile_value, _sqlite_file_stat_cache_key(db_path))) + + try: + _add_context(get_active_profile_name()) + except Exception: + pass + try: + for row in list_profiles_api(): + if not isinstance(row, dict): + continue + _add_context(row.get('name')) + except Exception: + logger.debug("All-profiles CLI context enumeration failed", exc_info=True) + try: + for entry in _profiles_root().iterdir(): + if not entry.is_dir(): + continue + _add_context(entry.name) + except Exception: + logger.debug("All-profiles CLI directory enumeration failed", exc_info=True) + + return contexts, tuple(cache_entries) + + def _load_cli_sessions_uncached( hermes_home: Path, db_path: Path, _cli_profile, source_filter=None, + *, + visible_session_limit: int | None = None, + cron_project_limit: int | None | bool = CRON_PROJECT_CHIP_LIMIT, + include_claude_code: bool = True, ) -> list: cli_sessions = [] - if source_filter in (None, CLAUDE_CODE_SOURCE): + if source_filter in (None, CLAUDE_CODE_SOURCE) and include_claude_code: try: cli_sessions.extend(get_claude_code_sessions()) except Exception: @@ -3841,6 +3897,7 @@ def _load_cli_sessions_uncached( if source_filter == CLAUDE_CODE_SOURCE: return cli_sessions + if not db_path.exists(): return cli_sessions @@ -3853,9 +3910,10 @@ def _load_cli_sessions_uncached( _cron_pid_cache[0] = ensure_cron_project() return _cron_pid_cache[0] + profile_value = _cli_profile or 'default' for row in read_importable_agent_session_rows( db_path, - limit=CRON_PROJECT_CHIP_LIMIT if source_filter == 'cron' else CLI_VISIBLE_SESSION_LIMIT, + limit=visible_session_limit if visible_session_limit is not None else (CRON_PROJECT_CHIP_LIMIT if source_filter == 'cron' else CLI_VISIBLE_SESSION_LIMIT), log=logger, exclude_sources=("cron",) if source_filter is None else None, include_sources=None if source_filter is None else (source_filter,), @@ -3864,7 +3922,7 @@ def _load_cli_sessions_uncached( raw_ts = row['last_activity'] or row['started_at'] # Prefer the CLI session's own profile from the DB; fall back to # the active CLI profile so sidebar filtering works either way. - profile = _cli_profile # CLI DB has no profile column; use active profile + profile = profile_value # CLI DB has no profile column; use active profile _source = row['source'] or 'cli' _title = row['title'] @@ -3942,88 +4000,89 @@ def _load_cli_sessions_uncached( # before _include_project_hidden_background_sidebar_sessions can rescue # them (#3172). A separate, higher-capped cron-only pass ensures they # stay addressable under their project chip. - existing_sids = {s['session_id'] for s in cli_sessions} - try: - for row in read_importable_agent_session_rows( - db_path, - limit=CRON_PROJECT_CHIP_LIMIT, - log=logger, - exclude_sources=None, - include_sources=("cron",), - ): - sid = row['id'] - if sid in existing_sids: - continue - _source = row['source'] or 'cli' - if _source != 'cron': - continue - raw_ts = row['last_activity'] or row['started_at'] - _title = row['title'] - if not _title and sid.startswith('cron_'): - parts = sid.split('_') - if len(parts) >= 3: - _job_id = parts[1] - try: - _jobs_path = hermes_home / 'cron' / 'jobs.json' - if _jobs_path.exists(): - import json as _json - _jobs_data = _json.loads(_jobs_path.read_text()) - for _j in _jobs_data.get('jobs', []): - if _j.get('id') == _job_id: - _title = _j.get('name') or _title - break - except Exception: - pass - try: - _webui_meta = Session.load_metadata_only(sid) - if _webui_meta and getattr(_webui_meta, 'title', None): - _title = _webui_meta.title - except Exception: - pass - _display_title = _title or 'Cron Session' - cli_sessions.append({ - 'session_id': sid, - 'title': _display_title, - 'workspace': str(get_last_workspace()), - 'model': row['model'] or None, - 'message_count': row['message_count'] or row['actual_message_count'] or 0, - 'created_at': row['started_at'], - 'updated_at': raw_ts, - 'pinned': False, - 'archived': False, - 'project_id': _cron_pid(), - 'profile': _cli_profile, - 'source_tag': 'cron', - 'raw_source': row.get('raw_source'), - 'user_id': row.get('user_id'), - 'chat_id': row.get('chat_id') or row.get('origin_chat_id'), - 'chat_type': row.get('chat_type'), - 'thread_id': row.get('thread_id'), - 'session_key': row.get('session_key'), - 'platform': row.get('platform'), - 'session_source': row.get('session_source'), - 'source_label': row.get('source_label'), - 'parent_session_id': row.get('parent_session_id'), - 'parent_title': row.get('parent_title'), - 'parent_source': row.get('parent_source'), - 'relationship_type': row.get('relationship_type'), - '_parent_lineage_root_id': row.get('_parent_lineage_root_id'), - 'end_reason': row.get('end_reason'), - 'actual_message_count': row.get('actual_message_count'), - 'user_message_count': row.get('actual_user_message_count'), - '_lineage_root_id': row.get('_lineage_root_id'), - '_lineage_tip_id': row.get('_lineage_tip_id'), - '_compression_segment_count': row.get('_compression_segment_count'), - 'is_cli_session': is_cli_session_row(row), - }) - existing_sids.add(sid) - except Exception: - logger.debug("Cron project-chip second pass failed", exc_info=True) + if cron_project_limit is not False: + existing_sids = {s['session_id'] for s in cli_sessions} + try: + for row in read_importable_agent_session_rows( + db_path, + limit=cron_project_limit, + log=logger, + exclude_sources=None, + include_sources=("cron",), + ): + sid = row['id'] + if sid in existing_sids: + continue + _source = row['source'] or 'cli' + if _source != 'cron': + continue + raw_ts = row['last_activity'] or row['started_at'] + _title = row['title'] + if not _title and sid.startswith('cron_'): + parts = sid.split('_') + if len(parts) >= 3: + _job_id = parts[1] + try: + _jobs_path = hermes_home / 'cron' / 'jobs.json' + if _jobs_path.exists(): + import json as _json + _jobs_data = _json.loads(_jobs_path.read_text()) + for _j in _jobs_data.get('jobs', []): + if _j.get('id') == _job_id: + _title = _j.get('name') or _title + break + except Exception: + pass + try: + _webui_meta = Session.load_metadata_only(sid) + if _webui_meta and getattr(_webui_meta, 'title', None): + _title = _webui_meta.title + except Exception: + pass + _display_title = _title or 'Cron Session' + cli_sessions.append({ + 'session_id': sid, + 'title': _display_title, + 'workspace': str(get_last_workspace()), + 'model': row['model'] or None, + 'message_count': row['message_count'] or row['actual_message_count'] or 0, + 'created_at': row['started_at'], + 'updated_at': raw_ts, + 'pinned': False, + 'archived': False, + 'project_id': _cron_pid(), + 'profile': profile_value, + 'source_tag': 'cron', + 'raw_source': row.get('raw_source'), + 'user_id': row.get('user_id'), + 'chat_id': row.get('chat_id') or row.get('origin_chat_id'), + 'chat_type': row.get('chat_type'), + 'thread_id': row.get('thread_id'), + 'session_key': row.get('session_key'), + 'platform': row.get('platform'), + 'session_source': row.get('session_source'), + 'source_label': row.get('source_label'), + 'parent_session_id': row.get('parent_session_id'), + 'parent_title': row.get('parent_title'), + 'parent_source': row.get('parent_source'), + 'relationship_type': row.get('relationship_type'), + '_parent_lineage_root_id': row.get('_parent_lineage_root_id'), + 'end_reason': row.get('end_reason'), + 'actual_message_count': row.get('actual_message_count'), + 'user_message_count': row.get('actual_user_message_count'), + '_lineage_root_id': row.get('_lineage_root_id'), + '_lineage_tip_id': row.get('_lineage_tip_id'), + '_compression_segment_count': row.get('_compression_segment_count'), + 'is_cli_session': is_cli_session_row(row), + }) + existing_sids.add(sid) + except Exception: + logger.debug("Cron project-chip second pass failed", exc_info=True) return cli_sessions -def get_cli_sessions(source_filter=None) -> list: +def get_cli_sessions(source_filter=None, *, all_profiles: bool = False) -> list: """Read CLI sessions from the agent's SQLite store and return them as dicts in a format the WebUI sidebar can render alongside local sessions. @@ -4031,10 +4090,39 @@ def get_cli_sessions(source_filter=None) -> list: bridge is purely additive and never crashes the WebUI. """ source_filter = _normalize_cli_session_source_filter(source_filter) - hermes_home, db_path, cli_profile, cache_key = _resolve_cli_sessions_context(source_filter) + if all_profiles: + contexts, context_cache_key = _all_profiles_cli_contexts() + cache_key = ( + 'all_profiles', + source_filter or '', + context_cache_key, + _path_cache_key(_default_claude_code_projects_dir()), + _path_stat_cache_key(_default_claude_code_projects_dir()), + _path_stat_cache_key(SESSION_INDEX_FILE), + ) + else: + hermes_home, db_path, cli_profile, cache_key = _resolve_cli_sessions_context(source_filter) ttl = _cli_sessions_cache_ttl_seconds() now = time.monotonic() + def _load_sessions(): + if all_profiles: + merged: list[dict] = [] + for idx, (ctx_home, ctx_db_path, ctx_profile) in enumerate(contexts): + merged.extend( + _load_cli_sessions_uncached( + ctx_home, + ctx_db_path, + ctx_profile, + source_filter=source_filter, + visible_session_limit=None, + cron_project_limit=None, + include_claude_code=(idx == 0), + ) + ) + return merged + return _load_cli_sessions_uncached(hermes_home, db_path, cli_profile, source_filter=source_filter) + if ttl > 0: with _CLI_SESSIONS_CACHE_LOCK: cached = _CLI_SESSIONS_CACHE.get(cache_key) @@ -4044,16 +4132,11 @@ def get_cli_sessions(source_filter=None) -> list: return _copy_cli_sessions(cached_sessions) _CLI_SESSIONS_CACHE.pop(cache_key, None) try: - sessions = _load_cli_sessions_uncached( - hermes_home, - db_path, - cli_profile, - source_filter=source_filter, - ) + sessions = _load_sessions() except Exception as _cli_err: logger.warning( "get_cli_sessions() failed — check state.db schema or path (%s): %s", - db_path, _cli_err, + "all profiles" if all_profiles else db_path, _cli_err, ) return [] _CLI_SESSIONS_CACHE[cache_key] = ( @@ -4063,16 +4146,11 @@ def get_cli_sessions(source_filter=None) -> list: return _copy_cli_sessions(sessions) try: - return _load_cli_sessions_uncached( - hermes_home, - db_path, - cli_profile, - source_filter=source_filter, - ) + return _load_sessions() except Exception as _cli_err: logger.warning( "get_cli_sessions() failed — check state.db schema or path (%s): %s", - db_path, _cli_err, + "all profiles" if all_profiles else db_path, _cli_err, ) return [] @@ -4720,7 +4798,7 @@ def reconciled_state_db_messages_for_session( ) -def get_cli_session_messages(sid) -> list: +def get_cli_session_messages(sid, *, profile=None) -> list: """Read messages for a single CLI/external-agent session. Preserve tool-call/result and reasoning metadata from the agent state.db so @@ -4731,7 +4809,7 @@ def get_cli_session_messages(sid) -> list: """ if str(sid or '').startswith(f'{CLAUDE_CODE_SOURCE}_'): return get_claude_code_session_messages(sid) - return get_state_db_session_messages(sid, stitch_continuations=True) + return get_state_db_session_messages(sid, stitch_continuations=True, profile=profile) def count_conversation_rounds(sid: str, since: float | None = None) -> int: diff --git a/api/routes.py b/api/routes.py index 4e41e115..dde8764c 100644 --- a/api/routes.py +++ b/api/routes.py @@ -1592,7 +1592,7 @@ def _build_session_list_cache_payload( webui_sessions = [_normalize_sidebar_source_flags(s) for s in webui_sessions] if show_cli_sessions: diag_stage("get_cli_sessions") - cli = get_cli_sessions(source_filter=source_filter) + cli = get_cli_sessions(source_filter=source_filter, all_profiles=all_profiles) diag_stage("merge_cli_sessions") cli_by_id = {s["session_id"]: s for s in cli} # #3238: reconcile orphaned imported-CLI sidecars. When a CLI @@ -3886,11 +3886,11 @@ def _lookup_gateway_session_identity(session_id: str) -> dict: return metadata if isinstance(metadata, dict) else {} -def _lookup_cli_session_metadata(session_id: str) -> dict: +def _lookup_cli_session_metadata(session_id: str, *, all_profiles: bool = False) -> dict: if not session_id: return {} try: - for row in get_cli_sessions(): + for row in get_cli_sessions(all_profiles=all_profiles): if row.get("session_id") == session_id: return row except Exception: @@ -3898,6 +3898,40 @@ def _lookup_cli_session_metadata(session_id: str) -> dict: return {} +def _request_wants_all_profiles_import(body) -> bool: + if not isinstance(body, dict): + return False + value = body.get("all_profiles") + if isinstance(value, str): + return value.strip().lower() in {"1", "true", "yes", "on"} + return bool(value) + + +def _normalize_import_profile_value(value): + profile = str(value or "").strip() + if not profile: + return None + try: + from api.profiles import _PROFILE_ID_RE + if profile != "default" and not _PROFILE_ID_RE.fullmatch(profile): + return "" + except Exception: + pass + return profile + + +def _resolve_cli_import_metadata(session_id: str, *, requested_profile=None, allow_all_profiles: bool = False) -> dict: + cli_meta = _lookup_cli_session_metadata(session_id) + if cli_meta and (not requested_profile or _profiles_match(cli_meta.get("profile"), requested_profile)): + return cli_meta + if not allow_all_profiles: + return {} + cli_meta = _lookup_cli_session_metadata(session_id, all_profiles=True) + if cli_meta and requested_profile and not _profiles_match(cli_meta.get("profile"), requested_profile): + return {} + return cli_meta or {} + + def _messaging_session_identity(session: dict, raw_source: str) -> str: metadata = _lookup_gateway_session_identity(session.get("session_id")) session_key = _safe_first( @@ -16997,17 +17031,39 @@ def _handle_session_import_cli(handler, body): return bad(handler, str(e)) sid = str(body["session_id"]) + requested_profile = _normalize_import_profile_value((body or {}).get("profile")) + if requested_profile == "": + return bad(handler, "invalid profile", 400) + allow_all_profiles = _request_wants_all_profiles_import(body) + if allow_all_profiles and not requested_profile: + return bad(handler, "profile is required for all_profiles import", 400) # Check if already imported — refresh messages from CLI store if new ones arrived existing = Session.load(sid) if existing: - fresh_msgs = get_cli_session_messages(sid) + # Cross-profile boundary: an unqualified (non-all-profiles) request must not + # read or refresh a session that belongs to another profile, even though the + # WebUI session store (SESSION_DIR) is a single global directory. This mirrors + # the /api/session detail and /api/session/export profile-scoping gates. + # An explicit all_profiles import is still allowed, but only when the request's + # profile matches the stored session's profile. + existing_profile = getattr(existing, "profile", None) + if allow_all_profiles: + if requested_profile and not _profiles_match(existing_profile, requested_profile): + return bad(handler, "Session not found in CLI store", 404) + elif not _session_visible_to_active_profile(existing_profile, handler): + return bad(handler, "Session not found in CLI store", 404) + refresh_profile = requested_profile or existing_profile + cli_meta = _resolve_cli_import_metadata( + sid, + requested_profile=refresh_profile, + allow_all_profiles=allow_all_profiles, + ) + fresh_msgs = get_cli_session_messages( + sid, + profile=(cli_meta or {}).get("profile") or refresh_profile, + ) changed = False - cli_meta = None - for cs in list(get_cli_sessions()): - if cs["session_id"] == sid: - cli_meta = cs - break if fresh_msgs and len(fresh_msgs) > len(existing.messages): # Prefix-equality guard: only extend if existing messages are a prefix of # the fresh CLI messages. Prevents silently dropping WebUI-added messages @@ -17053,48 +17109,33 @@ def _handle_session_import_cli(handler, body): ) # Fetch messages from CLI store - msgs = get_cli_session_messages(sid) + cli_meta = _resolve_cli_import_metadata( + sid, + requested_profile=requested_profile, + allow_all_profiles=allow_all_profiles, + ) + profile = cli_meta.get("profile") if cli_meta else (requested_profile if allow_all_profiles else None) + msgs = get_cli_session_messages(sid, profile=profile) if not msgs: return bad(handler, "Session not found in CLI store", 404) # Get profile, model, timestamps, and title from CLI session metadata - profile = None - created_at = None - updated_at = None - cli_title = None - cli_source_tag = None - model = "unknown" - cli_raw_source = None - cli_session_source = None - cli_source_label = None - cli_user_id = None - cli_chat_id = None - cli_chat_type = None - cli_thread_id = None - cli_session_key = None - cli_platform = None - cli_parent_session_id = None - cli_read_only = False - for cs in get_cli_sessions(): - if cs["session_id"] == sid: - profile = cs.get("profile") - model = cs.get("model", "unknown") - created_at = cs.get("created_at") - updated_at = cs.get("updated_at") - cli_title = cs.get("title") - cli_source_tag = cs.get("source_tag") - cli_raw_source = cs.get("raw_source") - cli_session_source = cs.get("session_source") - cli_source_label = cs.get("source_label") - cli_user_id = cs.get("user_id") - cli_chat_id = cs.get("chat_id") - cli_chat_type = cs.get("chat_type") - cli_thread_id = cs.get("thread_id") - cli_session_key = cs.get("session_key") - cli_platform = cs.get("platform") - cli_parent_session_id = cs.get("parent_session_id") - cli_read_only = bool(cs.get("read_only")) - break + created_at = cli_meta.get("created_at") if cli_meta else None + updated_at = cli_meta.get("updated_at") if cli_meta else None + cli_title = cli_meta.get("title") if cli_meta else None + cli_source_tag = cli_meta.get("source_tag") if cli_meta else None + model = cli_meta.get("model", "unknown") if cli_meta else "unknown" + cli_raw_source = cli_meta.get("raw_source") if cli_meta else None + cli_session_source = cli_meta.get("session_source") if cli_meta else None + cli_source_label = cli_meta.get("source_label") if cli_meta else None + cli_user_id = cli_meta.get("user_id") if cli_meta else None + cli_chat_id = cli_meta.get("chat_id") if cli_meta else None + cli_chat_type = cli_meta.get("chat_type") if cli_meta else None + cli_thread_id = cli_meta.get("thread_id") if cli_meta else None + cli_session_key = cli_meta.get("session_key") if cli_meta else None + cli_platform = cli_meta.get("platform") if cli_meta else None + cli_parent_session_id = cli_meta.get("parent_session_id") if cli_meta else None + cli_read_only = bool((cli_meta or {}).get("read_only")) # Use the CLI session title if available (e.g., cron job name), otherwise derive from messages title = cli_title or title_from(msgs, "CLI Session") diff --git a/static/sessions.js b/static/sessions.js index cdd3fc44..5d4812bc 100644 --- a/static/sessions.js +++ b/static/sessions.js @@ -1417,6 +1417,15 @@ function _isExternalSession(session) { return !!(session && (session.is_cli_session || _isMessagingSession(session))); } +function _externalImportPayload(session) { + const payload = {session_id: session.session_id}; + if (_showAllProfiles && session && typeof session.profile === 'string' && session.profile) { + payload.all_profiles = true; + payload.profile = session.profile; + } + return payload; +} + function _isReadOnlySession(session) { return !!(session && (session.read_only || session.is_read_only)); } @@ -4001,7 +4010,7 @@ function startGatewaySSE(){ // Capture active session ID before async fetch — race guard. // If the user switches sessions while the fetch is in-flight, discard the result. const activeSid = S.session.session_id; - api('/api/session/import_cli',{method:'POST',body:JSON.stringify({session_id:activeSid})}) + api('/api/session/import_cli',{method:'POST',body:JSON.stringify(_externalImportPayload(S.session))}) .then(res=>{ if(!S.session || S.session.session_id !== activeSid) return; if(res && res.session && Array.isArray(res.session.messages)){ @@ -5658,7 +5667,7 @@ function renderSessionListFromCache(){ row.onclick=async(e)=>{ e.stopPropagation(); if(_isExternalSession(seg)){ - try{await api('/api/session/import_cli',{method:'POST',body:JSON.stringify({session_id:seg.session_id})});} + try{await api('/api/session/import_cli',{method:'POST',body:JSON.stringify(_externalImportPayload(seg))});} catch(_e){ /* read-only fallback */ } } await loadSession(seg.session_id, {skipLineageResolve:true}); @@ -5675,7 +5684,7 @@ function renderSessionListFromCache(){ const sortedChildren=[...s._child_sessions].sort((a,b)=>_sessionTimestampMs(b)-_sessionTimestampMs(a)); const openChildSession=async(childSession)=>{ if(_isExternalSession(childSession)){ - try{await api('/api/session/import_cli',{method:'POST',body:JSON.stringify({session_id:childSession.session_id})});} + try{await api('/api/session/import_cli',{method:'POST',body:JSON.stringify(_externalImportPayload(childSession))});} catch(_e){ /* read-only fallback */ } } await loadSession(childSession.session_id, {skipLineageResolve:true}); @@ -6305,7 +6314,7 @@ function renderSessionListFromCache(){ // WebUI store first so /api/chat/start finds a persisted session. if(_isExternalSession(s)){ try{ - await api('/api/session/import_cli',{method:'POST',body:JSON.stringify({session_id:s.session_id})}); + await api('/api/session/import_cli',{method:'POST',body:JSON.stringify(_externalImportPayload(s))}); }catch(e){ /* import failed -- fall through to read-only view */ } } try{ diff --git a/tests/test_claude_code_session_import.py b/tests/test_claude_code_session_import.py index 38073e25..e9c69266 100644 --- a/tests/test_claude_code_session_import.py +++ b/tests/test_claude_code_session_import.py @@ -198,8 +198,8 @@ def test_session_import_cli_returns_read_only_claude_code_payload(monkeypatch, t monkeypatch.setattr(routes, "require", lambda body, *keys: None) monkeypatch.setattr(routes, "bad", lambda _handler, msg, status=400: {"ok": False, "error": msg, "status": status}) monkeypatch.setattr(routes, "j", lambda _handler, payload, status=200, extra_headers=None: payload) - monkeypatch.setattr(routes, "get_cli_session_messages", lambda _sid: messages if _sid == sid else []) - monkeypatch.setattr(routes, "get_cli_sessions", lambda source_filter=None: [meta]) + monkeypatch.setattr(routes, "get_cli_session_messages", lambda _sid, profile=None: messages if _sid == sid else []) + monkeypatch.setattr(routes, "get_cli_sessions", lambda source_filter=None, all_profiles=False: [meta]) monkeypatch.setattr(routes, "get_last_workspace", lambda: tmp_path / "workspace") monkeypatch.setattr(routes, "import_cli_session", lambda *args, **kwargs: (_ for _ in ()).throw(AssertionError("read-only import must not persist"))) @@ -263,8 +263,16 @@ def test_session_import_cli_queues_generated_title_for_writable_default_cli_titl monkeypatch.setattr(routes, "require", lambda body, *keys: None) monkeypatch.setattr(routes, "bad", lambda _handler, msg, status=400: {"ok": False, "error": msg, "status": status}) monkeypatch.setattr(routes, "j", lambda _handler, payload, status=200, extra_headers=None: payload) - monkeypatch.setattr(routes, "get_cli_session_messages", lambda _sid: messages if _sid == sid else []) - monkeypatch.setattr(routes, "get_cli_sessions", lambda: [cli_meta]) + monkeypatch.setattr( + routes, + "get_cli_session_messages", + lambda _sid, profile=None: messages if _sid == sid else [], + ) + monkeypatch.setattr( + routes, + "get_cli_sessions", + lambda source_filter=None, all_profiles=False: [cli_meta], + ) monkeypatch.setattr(routes, "import_cli_session", lambda *args, **kwargs: imported) monkeypatch.setattr(routes, "publish_session_list_changed", lambda reason, profile=None: published.append((reason, profile))) monkeypatch.setattr(routes, "_queue_generated_title_for_imported_session", lambda session, meta: queued.append((session, meta.copy()))) diff --git a/tests/test_cli_session_tool_metadata.py b/tests/test_cli_session_tool_metadata.py index 93c2afb9..be314595 100644 --- a/tests/test_cli_session_tool_metadata.py +++ b/tests/test_cli_session_tool_metadata.py @@ -141,8 +141,8 @@ def test_existing_cli_import_refreshes_same_length_tool_metadata(monkeypatch): monkeypatch.setattr(routes.Session, "load", classmethod(lambda _cls, sid: existing if sid == session_id else None)) monkeypatch.setattr(routes, "require", lambda body, *keys: None) monkeypatch.setattr(routes, "j", lambda _handler, payload, status=200, extra_headers=None: payload) - monkeypatch.setattr(routes, "get_cli_session_messages", lambda sid: enriched if sid == session_id else []) - monkeypatch.setattr(routes, "get_cli_sessions", lambda source_filter=None: [{"session_id": session_id, "source_tag": "cli", "raw_source": "cli", "session_source": "cli", "source_label": "CLI"}]) + monkeypatch.setattr(routes, "get_cli_session_messages", lambda sid, profile=None: enriched if sid == session_id else []) + monkeypatch.setattr(routes, "get_cli_sessions", lambda source_filter=None, all_profiles=False: [{"session_id": session_id, "source_tag": "cli", "raw_source": "cli", "session_source": "cli", "source_label": "CLI"}]) response = routes._handle_session_import_cli(object(), {"session_id": session_id}) diff --git a/tests/test_gateway_sync.py b/tests/test_gateway_sync.py index 51efe2aa..ca40f81a 100644 --- a/tests/test_gateway_sync.py +++ b/tests/test_gateway_sync.py @@ -22,15 +22,21 @@ REPO_ROOT = pathlib.Path(__file__).parent.parent.resolve() from tests._pytest_port import BASE -def get(path): - with urllib.request.urlopen(BASE + path, timeout=10) as r: +def get(path, *, profile=None): + headers = {} + if profile: + headers["Cookie"] = f"hermes_profile={profile}" + req = urllib.request.Request(BASE + path, headers=headers) + with urllib.request.urlopen(req, timeout=10) as r: return json.loads(r.read()), r.status -def post(path, body=None): +def post(path, body=None, *, profile=None): data = json.dumps(body or {}).encode() - req = urllib.request.Request(BASE + path, data=data, - headers={"Content-Type": "application/json"}) + headers = {"Content-Type": "application/json"} + if profile: + headers["Cookie"] = f"hermes_profile={profile}" + req = urllib.request.Request(BASE + path, data=data, headers=headers) try: with urllib.request.urlopen(req, timeout=10) as r: return json.loads(r.read()), r.status @@ -55,16 +61,23 @@ def _get_test_state_dir(): return _ptsd -def _get_state_db_path(): +def _get_profile_state_dir(profile=None): + state_dir = _get_test_state_dir() + if isinstance(profile, str) and profile.strip(): + return state_dir / 'profiles' / profile.strip() + return state_dir + + +def _get_state_db_path(profile=None): """Return path to the test state.db.""" - return _get_test_state_dir() / 'state.db' + return _get_profile_state_dir(profile) / 'state.db' -def _ensure_state_db(): +def _ensure_state_db(profile=None): """Create state.db with sessions and messages tables if it doesn't exist. Returns a connection. Does NOT delete existing data (safe for parallel tests). """ - db_path = _get_state_db_path() + db_path = _get_state_db_path(profile) db_path.parent.mkdir(parents=True, exist_ok=True) conn = sqlite3.connect(str(db_path)) conn.row_factory = sqlite3.Row @@ -1221,6 +1234,92 @@ def test_import_cli_preserves_messaging_source_metadata(cleanup_test_sessions): pass +def test_import_cli_named_profile_requires_explicit_all_profiles_opt_in(cleanup_test_sessions): + """Unqualified import_cli should not read a named profile's state.db.""" + named_profile = 'issue1611-import' + conn = _ensure_state_db(profile=named_profile) + sid = 'gw_named_profile_import_001' + cleanup_test_sessions.append(sid) + try: + _insert_gateway_session( + conn, + session_id=sid, + source='telegram', + title='Named Profile Telegram Session', + ) + + data, status = post('/api/session/import_cli', {'session_id': sid}) + assert status == 404, data + finally: + try: + _remove_test_sessions(conn, sid) + conn.close() + except Exception: + pass + + +def test_import_cli_all_profiles_opt_in_reads_named_profile_state_db(cleanup_test_sessions): + """Explicit all-profiles imports should resolve messages from the named profile store.""" + named_profile = 'issue1611-import' + conn = _ensure_state_db(profile=named_profile) + sid = 'gw_named_profile_import_001' + cleanup_test_sessions.append(sid) + try: + _insert_gateway_session( + conn, + session_id=sid, + source='telegram', + title='Named Profile Telegram Session', + ) + + data, status = post('/api/session/import_cli', { + 'session_id': sid, + 'all_profiles': True, + 'profile': named_profile, + }) + assert status == 200, data + session = data.get('session', {}) + messages = session.get('messages', []) + + assert session.get('session_id') == sid + assert session.get('profile') == named_profile + assert session.get('source_tag') == 'telegram' + assert session.get('session_source') == 'messaging' + assert [m.get('content') for m in messages] == ['Hello from Telegram', 'Hi there!'] + finally: + try: + _remove_test_sessions(conn, sid) + conn.close() + except Exception: + pass + + +def test_all_profiles_cli_contexts_normalizes_default_profile_name(tmp_path, monkeypatch): + from api import models, profiles + + profiles_root = tmp_path / "profiles" + profiles_root.mkdir() + (profiles_root / "research").mkdir() + + monkeypatch.setattr(profiles, "_profiles_root", lambda: profiles_root) + monkeypatch.setattr(profiles, "get_active_profile_name", lambda: None) + monkeypatch.setattr( + profiles, + "get_hermes_home_for_profile", + lambda profile_name: tmp_path / (profile_name or "default-home"), + ) + monkeypatch.setattr( + profiles, + "list_profiles_api", + lambda: [{"name": None}, {"name": "research"}], + ) + + contexts, _cache_key = models._all_profiles_cli_contexts() + + assert ("default" in [profile for _home, _db_path, profile in contexts]) + assert ("research" in [profile for _home, _db_path, profile in contexts]) + + def test_sessions_response_backfills_imported_messaging_source_metadata(cleanup_test_sessions): """Old imported messaging sessions should still expose source metadata in /api/sessions.""" from api.models import Session diff --git a/tests/test_issue1611_session_profile_filtering.py b/tests/test_issue1611_session_profile_filtering.py index 3595a7a0..4f0a0696 100644 --- a/tests/test_issue1611_session_profile_filtering.py +++ b/tests/test_issue1611_session_profile_filtering.py @@ -14,12 +14,20 @@ all_profiles=1 opt-in path. End-to-end HTTP-level tests live separately under tests/test_sessions_endpoint.py if/when added. """ +import json +import os +import sqlite3 +import time from types import SimpleNamespace from unittest.mock import patch +import urllib.request +from pathlib import Path from urllib.parse import urlparse import pytest +from tests._pytest_port import BASE + # ── _profiles_match helper ───────────────────────────────────────────────── @@ -144,6 +152,19 @@ def test_static_sessions_js_uses_all_profiles_query_when_toggle_on(): ) +def test_static_sessions_js_marks_all_profiles_imports_with_profile(): + """All-profiles row opens must opt into cross-profile import explicitly.""" + from pathlib import Path + + repo_root = Path(__file__).parent.parent + src = (repo_root / 'static' / 'sessions.js').read_text(encoding='utf-8') + + assert "function _externalImportPayload(session)" in src + assert "payload.all_profiles = true;" in src + assert "payload.profile = session.profile;" in src + assert "JSON.stringify(_externalImportPayload(s))" in src + + # ── SHOULD-FIX #2: profile filter must run BEFORE messaging-source dedupe ── # Bug shape (Opus pre-release advisor): _messaging_source_key is profile-blind, # so if profiles A and B both have a session for the same Slack identity, a @@ -431,6 +452,111 @@ def test_session_export_allows_session_from_active_profile(): assert ("Cache-Control", "no-store") in handler.sent_headers +def _profile_state_db_path(profile: str | None = None) -> Path: + root = Path(os.environ["HERMES_WEBUI_TEST_STATE_DIR"]) + if profile: + return root / "profiles" / profile / "state.db" + return root / "state.db" + + +def _ensure_agent_state_db(profile: str | None = None) -> sqlite3.Connection: + db_path = _profile_state_db_path(profile) + db_path.parent.mkdir(parents=True, exist_ok=True) + conn = sqlite3.connect(str(db_path)) + conn.executescript(""" + CREATE TABLE IF NOT EXISTS sessions ( + id TEXT PRIMARY KEY, + source TEXT NOT NULL, + user_id TEXT, + model TEXT, + started_at REAL NOT NULL, + message_count INTEGER DEFAULT 0, + title TEXT + ); + CREATE TABLE IF NOT EXISTS messages ( + id INTEGER PRIMARY KEY AUTOINCREMENT, + session_id TEXT NOT NULL, + role TEXT NOT NULL, + content TEXT, + timestamp REAL NOT NULL + ); + """) + conn.commit() + return conn + + +def _insert_agent_session(conn: sqlite3.Connection, session_id: str, *, source: str, title: str) -> None: + started_at = time.time() + conn.execute( + "INSERT OR REPLACE INTO sessions (id, source, title, model, started_at, message_count) " + "VALUES (?, ?, ?, ?, ?, ?)", + (session_id, source, title, "openai/gpt-5", started_at, 2), + ) + conn.execute("DELETE FROM messages WHERE session_id = ?", (session_id,)) + conn.execute( + "INSERT INTO messages (session_id, role, content, timestamp) VALUES (?, 'user', ?, ?)", + (session_id, "Hello from other profile", started_at), + ) + conn.execute( + "INSERT INTO messages (session_id, role, content, timestamp) VALUES (?, 'assistant', ?, ?)", + (session_id, "Reply from other profile", started_at + 1), + ) + conn.commit() + + +def _delete_agent_session(conn: sqlite3.Connection, session_id: str) -> None: + conn.execute("DELETE FROM messages WHERE session_id = ?", (session_id,)) + conn.execute("DELETE FROM sessions WHERE id = ?", (session_id,)) + conn.commit() + + +def _get_json(path: str) -> tuple[dict, int]: + req = urllib.request.Request(BASE + path) + with urllib.request.urlopen(req, timeout=10) as resp: + return json.loads(resp.read()), resp.status + + +def _post_json(path: str, body: dict) -> tuple[dict, int]: + req = urllib.request.Request( + BASE + path, + data=json.dumps(body).encode("utf-8"), + headers={"Content-Type": "application/json"}, + ) + with urllib.request.urlopen(req, timeout=10) as resp: + return json.loads(resp.read()), resp.status + + +def test_all_profiles_query_includes_named_profile_cli_sessions(): + """all_profiles=1 should aggregate agent sessions from non-active named profiles.""" + conn = _ensure_agent_state_db("issue1611-named") + sid = "issue1611_named_profile_cli_001" + try: + _insert_agent_session( + conn, + sid, + source="telegram", + title="Named Profile Telegram Session", + ) + _post_json("/api/settings", {"show_cli_sessions": True}) + + scoped, scoped_status = _get_json("/api/sessions") + assert scoped_status == 200 + assert sid not in {row.get("session_id") for row in scoped.get("sessions", [])} + + aggregate, aggregate_status = _get_json("/api/sessions?all_profiles=1") + assert aggregate_status == 200 + session = next( + row for row in aggregate.get("sessions", []) + if row.get("session_id") == sid + ) + assert session.get("profile") == "issue1611-named" + assert aggregate.get("all_profiles") is True + finally: + try: + _post_json("/api/settings", {"show_cli_sessions": False}) + finally: + _delete_agent_session(conn, sid) + conn.close() # ── Cleanup ──────────────────────────────────────────────────────────────── diff --git a/tests/test_issue3930_source_filter_pushdown.py b/tests/test_issue3930_source_filter_pushdown.py index bdf85728..fc4c35ce 100644 --- a/tests/test_issue3930_source_filter_pushdown.py +++ b/tests/test_issue3930_source_filter_pushdown.py @@ -183,6 +183,39 @@ def test_get_cli_sessions_source_filter_uses_distinct_cache_key(monkeypatch, tmp assert filtered_again[0]["session_id"] == "session-tui" +def test_get_cli_sessions_all_profiles_pushes_source_filter_to_every_context(monkeypatch, tmp_path): + """#4067 regression: the all_profiles CLI scan must thread source_filter into the + per-context _load_cli_sessions_uncached calls. The all-profiles branch keys the cache + on source_filter, so omitting it from the loader returned every-source sessions under + a filtered key (Codex SILENT finding on the #4067 re-gate).""" + hermes_home = tmp_path / "hermes" + hermes_home.mkdir() + monkeypatch.setattr(profiles, "get_active_hermes_home", lambda: str(hermes_home)) + monkeypatch.setattr(profiles, "get_active_profile_name", lambda: "default") + monkeypatch.setattr(models, "_CLI_SESSIONS_CACHE_TTL_SECONDS", 60.0, raising=False) + models.clear_cli_sessions_cache() + + # Two profile contexts; cache key derived from a stable token. + contexts = [ + (hermes_home, hermes_home / "state.db", "default"), + (hermes_home / "p2", hermes_home / "p2" / "state.db", "haku"), + ] + monkeypatch.setattr(models, "_all_profiles_cli_contexts", lambda: (contexts, "ctx-key")) + + seen = [] + + def fake_loader(_hermes_home, _db_path, _cli_profile, source_filter=None, **kwargs): + seen.append(source_filter) + return [{"session_id": f"s-{_cli_profile}-{source_filter or 'all'}", "title": "x"}] + + monkeypatch.setattr(models, "_load_cli_sessions_uncached", fake_loader) + + models.get_cli_sessions(source_filter="tui", all_profiles=True) + + # Every context must receive the "tui" filter, not None. + assert seen == ["tui", "tui"], seen + + def test_load_cli_sessions_uncached_pushes_specific_source_into_state_db_scan(monkeypatch, tmp_path): db = tmp_path / "state.db" db.write_text("", encoding="utf-8") @@ -269,8 +302,8 @@ def test_cron_source_filter_uses_cron_rescue_limit(monkeypatch, tmp_path): def test_api_sessions_passes_source_filter_only_on_sidebar_path(monkeypatch): captured = [] - def fake_get_cli_sessions(source_filter=None): - captured.append(source_filter) + def fake_get_cli_sessions(source_filter=None, *, all_profiles=False): + captured.append({"source_filter": source_filter, "all_profiles": all_profiles}) return [] monkeypatch.setattr(routes, "all_sessions", lambda diag=None: []) @@ -290,17 +323,20 @@ def test_api_sessions_passes_source_filter_only_on_sidebar_path(monkeypatch): assert handler.status == 200 assert handler.json_body()["sessions"] == [] - assert captured == [" TUI "] + assert captured == [{"source_filter": " TUI ", "all_profiles": False}] def test_non_sidebar_cli_session_callers_keep_default_get_cli_sessions_signature(monkeypatch): + captured = [] + monkeypatch.setattr( routes, "get_cli_sessions", - lambda: [{"session_id": "cli-session", "title": "CLI Session"}], + lambda *, all_profiles=False: captured.append(all_profiles) or [{"session_id": "cli-session", "title": "CLI Session"}], ) assert routes._lookup_cli_session_metadata("cli-session") == { "session_id": "cli-session", "title": "CLI Session", } + assert captured == [False] diff --git a/tests/test_issue4067_import_cli_cross_profile_guard.py b/tests/test_issue4067_import_cli_cross_profile_guard.py new file mode 100644 index 00000000..669604ef --- /dev/null +++ b/tests/test_issue4067_import_cli_cross_profile_guard.py @@ -0,0 +1,133 @@ +"""Regression test for the #4067 cross-profile import_cli boundary. + +The all-profiles session enumeration (#4067) must NOT relax the cross-profile +isolation boundary on the import_cli refresh path. Both release-gate advisors +(Codex CORE + Opus) reproduced the same leak on PR head b16586f0: the +already-imported branch of `_handle_session_import_cli` forced +`allow_all_profiles=True` whenever the (globally-stored) session carried a +profile, and had no `_session_visible_to_active_profile` gate — so an +unqualified request from profile A could read/refresh a session imported under +profile B's live `state.db` (Codex returned the foreign transcript + a planted +secret on an unqualified request). + +This pins the fix: the existing-session branch is profile-scoped exactly like +the /api/session detail and /api/session/export endpoints. +""" + +from __future__ import annotations + +import io + +import api.routes as routes + + +class _FakeHandler: + def __init__(self, cookie_profile: str | None = None): + self.status = None + self.headers = {} + if cookie_profile is not None: + self.headers["Cookie"] = f"hermes_profile={cookie_profile}" + self.wfile = io.BytesIO() + + def send_response(self, status): + self.status = status + + def send_header(self, key, value): + self.headers[key] = value + + def end_headers(self): + pass + + +def _capture(monkeypatch): + cap = {} + monkeypatch.setattr(routes, "j", lambda h, o: (cap.__setitem__("ok", o), True)[1]) + monkeypatch.setattr( + routes, + "bad", + lambda h, m, c=400: (cap.__setitem__("bad", (m, c)), True)[1], + ) + return cap + + +class _FakeSession: + """Minimal stand-in for a globally-stored Session imported under a profile.""" + + def __init__(self, sid, profile): + self.session_id = sid + self.profile = profile + self.messages = [{"role": "user", "content": "FOREIGN_SECRET"}] + self.source_tag = "cli" + self.raw_source = "cli" + self.session_source = "cli" + self.source_label = "CLI" + self.parent_session_id = None + + def compact(self): + return {"id": self.session_id, "profile": self.profile} + + def save(self, touch_updated_at=False): + raise AssertionError("must not save/refresh a foreign-profile session") + + +def test_import_cli_existing_foreign_profile_unqualified_request_404(monkeypatch): + """Unqualified request (active profile=default) for a session stored under a + foreign profile must 404 — not read or refresh the foreign session.""" + foreign = _FakeSession("foreign_existing_001", "other") + monkeypatch.setattr(routes.Session, "load", staticmethod(lambda sid: foreign)) + monkeypatch.setattr(routes, "_get_active_profile_name", lambda: "default") + # If the gate fails open, these would be reached — make them loud. + monkeypatch.setattr( + routes, "get_cli_session_messages", + lambda *a, **k: (_ for _ in ()).throw(AssertionError("foreign read attempted")), + ) + cap = _capture(monkeypatch) + + routes._handle_session_import_cli( + _FakeHandler(cookie_profile="default"), + {"session_id": "foreign_existing_001"}, + ) + + assert "bad" in cap, f"expected 404, got {cap}" + assert cap["bad"][1] == 404 + + +def test_import_cli_existing_same_profile_still_refreshes(monkeypatch): + """A session stored under the active profile is still served/refreshed.""" + own = _FakeSession("own_001", "default") + own.save = lambda touch_updated_at=False: None # allow refresh + monkeypatch.setattr(routes.Session, "load", staticmethod(lambda sid: own)) + monkeypatch.setattr(routes, "_get_active_profile_name", lambda: "default") + monkeypatch.setattr(routes, "_resolve_cli_import_metadata", lambda *a, **k: {}) + monkeypatch.setattr(routes, "get_cli_session_messages", lambda *a, **k: []) + cap = _capture(monkeypatch) + + routes._handle_session_import_cli( + _FakeHandler(cookie_profile="default"), + {"session_id": "own_001"}, + ) + + assert "ok" in cap, f"expected success, got {cap}" + assert cap["ok"]["session"]["id"] == "own_001" + + +def test_import_cli_all_profiles_requires_matching_profile(monkeypatch): + """An explicit all_profiles import for a foreign session must still match the + requested profile — a mismatched profile 404s rather than reading.""" + foreign = _FakeSession("foreign_002", "other") + monkeypatch.setattr(routes.Session, "load", staticmethod(lambda sid: foreign)) + monkeypatch.setattr(routes, "_get_active_profile_name", lambda: "default") + monkeypatch.setattr( + routes, "get_cli_session_messages", + lambda *a, **k: (_ for _ in ()).throw(AssertionError("foreign read attempted")), + ) + cap = _capture(monkeypatch) + + # all_profiles=1 but requested profile "haku" != stored "other" → 404. + routes._handle_session_import_cli( + _FakeHandler(cookie_profile="default"), + {"session_id": "foreign_002", "all_profiles": 1, "profile": "haku"}, + ) + + assert "bad" in cap, f"expected 404, got {cap}" + assert cap["bad"][1] == 404 diff --git a/tests/test_session_cli_scan_fast_path.py b/tests/test_session_cli_scan_fast_path.py index 70069318..7cb7545c 100644 --- a/tests/test_session_cli_scan_fast_path.py +++ b/tests/test_session_cli_scan_fast_path.py @@ -46,7 +46,7 @@ def test_read_only_session_metadata_load_preserves_cli_metadata_lookup(monkeypat monkeypatch.setattr(routes, "get_session", lambda sid, metadata_only=False: session) monkeypatch.setattr(routes, "_clear_stale_stream_state", lambda _session: None) - monkeypatch.setattr(routes, "get_cli_session_messages", lambda _sid: []) + monkeypatch.setattr(routes, "get_cli_session_messages", lambda _sid, profile=None: []) monkeypatch.setattr(routes, "redact_session_data", lambda payload: payload) monkeypatch.setattr(routes, "j", lambda _handler, payload, status=200, extra_headers=None: payload) @@ -85,7 +85,7 @@ def test_messaging_session_metadata_load_preserves_cli_metadata_lookup(monkeypat monkeypatch.setattr(routes, "get_session", lambda sid, metadata_only=False: session) monkeypatch.setattr(routes, "_clear_stale_stream_state", lambda _session: None) - monkeypatch.setattr(routes, "get_cli_session_messages", lambda _sid: []) + monkeypatch.setattr(routes, "get_cli_session_messages", lambda _sid, profile=None: []) monkeypatch.setattr(routes, "redact_session_data", lambda payload: payload) monkeypatch.setattr(routes, "j", lambda _handler, payload, status=200, extra_headers=None: payload) @@ -118,7 +118,7 @@ def test_messaging_session_metadata_matches_full_display_merge(monkeypatch): monkeypatch.setattr(routes, "get_session", lambda sid, metadata_only=False: session) monkeypatch.setattr(routes, "_clear_stale_stream_state", lambda _session: None) monkeypatch.setattr(routes, "_lookup_cli_session_metadata", lambda sid: {"session_id": sid, "session_source": "messaging", "raw_source": "telegram"}) - monkeypatch.setattr(routes, "get_cli_session_messages", lambda _sid: cli) + monkeypatch.setattr(routes, "get_cli_session_messages", lambda _sid, profile=None: cli) monkeypatch.setattr(routes, "redact_session_data", lambda payload: payload) monkeypatch.setattr(routes, "j", lambda _handler, payload, status=200, extra_headers=None: payload) full = routes.handle_get(object(), urlparse("/api/session?session_id=telegram_resume&messages=1&resolve_model=0"))["session"] diff --git a/tests/test_session_import_cli_fallback_model.py b/tests/test_session_import_cli_fallback_model.py index 73c276ad..a053eb87 100644 --- a/tests/test_session_import_cli_fallback_model.py +++ b/tests/test_session_import_cli_fallback_model.py @@ -1,15 +1,16 @@ """Regression test for #1386: CLI session import must not crash when the session is missing from `get_cli_sessions()` metadata at the time of import. -Before the fix, `_handle_session_import_cli` only assigned `model` inside -the `for cs in get_cli_sessions(): if cs["session_id"] == sid` loop. If -the session existed in the messages store but had no metadata row (or had -been pruned after `get_cli_session_messages()` was called), `model` was -unbound and `import_cli_session(sid, title, msgs, model, ...)` raised +Before the fix, `_handle_session_import_cli` only assigned `model` while +walking `get_cli_sessions()` rows inline. If the session existed in the +messages store but had no metadata row (or had been pruned after +`get_cli_session_messages()` was called), `model` was unbound and +`import_cli_session(sid, title, msgs, model, ...)` raised `UnboundLocalError`. -The fix initializes `model = "unknown"` before the loop so the import -proceeds with a sensible default rather than crashing. +The fix centralizes metadata lookup and still defaults to `"unknown"` when +that lookup misses, so the import proceeds with a sensible fallback rather +than crashing. """ from __future__ import annotations @@ -52,27 +53,21 @@ def _extract_handler(name: str) -> str: return ROUTES_PY[idx : next_def if next_def != -1 else len(ROUTES_PY)] -def test_import_cli_initializes_model_before_metadata_loop(): - """The fallback `model = 'unknown'` must be set BEFORE the - `for cs in get_cli_sessions()` loop so that a metadata-less session - cannot leave `model` unbound.""" +def test_import_cli_initializes_model_from_lookup_with_unknown_fallback(): + """The metadata lookup path must still provide an explicit unknown model fallback.""" handler = _extract_handler("_handle_session_import_cli") - init_idx = handler.find('model = "unknown"') - if init_idx == -1: - # Allow single quotes too. - init_idx = handler.find("model = 'unknown'") - assert init_idx != -1, ( - "Expected `model = \"unknown\"` initialization in " - "_handle_session_import_cli before the metadata loop. Without it, " - "import crashes when the session has messages but no metadata row." + lookup_idx = handler.find('cli_meta = _resolve_cli_import_metadata(') + if lookup_idx == -1: + lookup_idx = handler.find('cli_meta = _lookup_cli_session_metadata(sid)') + model_idx = handler.find('model = cli_meta.get("model", "unknown") if cli_meta else "unknown"') + assert lookup_idx != -1, "Expected metadata lookup in _handle_session_import_cli" + assert model_idx != -1, ( + "Expected `_handle_session_import_cli` to derive `model` from cli_meta " + "with an explicit `'unknown'` fallback." ) - loop_idx = handler.find("for cs in get_cli_sessions()") - assert loop_idx != -1, "Expected `for cs in get_cli_sessions()` loop" - assert init_idx < loop_idx, ( - "`model` must be initialized BEFORE the `for cs in get_cli_sessions()` " - "loop, otherwise a session without a metadata row leaves `model` " - "unbound and `import_cli_session(..., model, ...)` raises " - "UnboundLocalError." + assert lookup_idx < model_idx, ( + "`model` must be derived after metadata lookup and still keep the " + "`'unknown'` fallback for metadata-less imports." ) @@ -132,8 +127,8 @@ def test_session_import_cli_refresh_matches_messages_despite_timestamp_type_diff monkeypatch.setattr(routes, "require", lambda body, *keys: None) monkeypatch.setattr(routes, "bad", lambda _handler, msg, status=400: {"ok": False, "error": msg, "status": status}) monkeypatch.setattr(routes, "j", lambda _handler, payload, status=200, extra_headers=None: payload) - monkeypatch.setattr(routes, "get_cli_session_messages", lambda sid: fresh if sid == session_id else []) - monkeypatch.setattr(routes, "get_cli_sessions", lambda source_filter=None: [{"session_id": session_id, "source_tag": "weixin", "raw_source": "weixin", "session_source": "messaging", "source_label": "WeChat"}]) + monkeypatch.setattr(routes, "get_cli_session_messages", lambda sid, profile=None: fresh if sid == session_id else []) + monkeypatch.setattr(routes, "get_cli_sessions", lambda source_filter=None, all_profiles=False: [{"session_id": session_id, "source_tag": "weixin", "raw_source": "weixin", "session_source": "messaging", "source_label": "WeChat"}]) response = routes._handle_session_import_cli(object(), {"session_id": session_id}) @@ -184,8 +179,8 @@ def test_session_import_cli_refresh_rejects_prefix_if_non_timing_content_diverge monkeypatch.setattr(routes, "require", lambda body, *keys: None) monkeypatch.setattr(routes, "bad", lambda _handler, msg, status=400: {"ok": False, "error": msg, "status": status}) monkeypatch.setattr(routes, "j", lambda _handler, payload, status=200, extra_headers=None: payload) - monkeypatch.setattr(routes, "get_cli_session_messages", lambda sid: fresh if sid == session_id else []) - monkeypatch.setattr(routes, "get_cli_sessions", lambda source_filter=None: [{"session_id": session_id, "source_tag": "telegram", "raw_source": "telegram", "session_source": "messaging", "source_label": "Telegram"}]) + monkeypatch.setattr(routes, "get_cli_session_messages", lambda sid, profile=None: fresh if sid == session_id else []) + monkeypatch.setattr(routes, "get_cli_sessions", lambda source_filter=None, all_profiles=False: [{"session_id": session_id, "source_tag": "telegram", "raw_source": "telegram", "session_source": "messaging", "source_label": "Telegram"}]) response = routes._handle_session_import_cli(object(), {"session_id": session_id}) @@ -224,11 +219,11 @@ def test_session_import_cli_preserves_parent_metadata_on_existing_import(monkeyp monkeypatch.setattr(routes.Session, "load", classmethod(lambda _cls, sid: existing if sid == session_id else None)) monkeypatch.setattr(routes, "require", lambda body, *keys: None) monkeypatch.setattr(routes, "j", lambda _handler, payload, status=200, extra_headers=None: payload) - monkeypatch.setattr(routes, "get_cli_session_messages", lambda sid: existing.messages if sid == session_id else []) + monkeypatch.setattr(routes, "get_cli_session_messages", lambda sid, profile=None: existing.messages if sid == session_id else []) monkeypatch.setattr( routes, "get_cli_sessions", - lambda source_filter=None: [{ + lambda source_filter=None, all_profiles=False: [{ "session_id": session_id, "source_tag": "telegram", "raw_source": "telegram", @@ -258,11 +253,11 @@ def test_read_only_import_payload_includes_parent_session_id(monkeypatch): monkeypatch.setattr(routes, "require", lambda body, *keys: None) monkeypatch.setattr(routes, "bad", lambda _handler, msg, status=400: {"ok": False, "error": msg, "status": status}) monkeypatch.setattr(routes, "j", lambda _handler, payload, status=200, extra_headers=None: payload) - monkeypatch.setattr(routes, "get_cli_session_messages", lambda sid: messages if sid == session_id else []) + monkeypatch.setattr(routes, "get_cli_session_messages", lambda sid, profile=None: messages if sid == session_id else []) monkeypatch.setattr( routes, "get_cli_sessions", - lambda source_filter=None: [{ + lambda source_filter=None, all_profiles=False: [{ "session_id": session_id, "title": "Read-only child", "model": "test-model", @@ -377,7 +372,7 @@ def test_sessions_endpoint_suppresses_duplicate_webui_state_projection(monkeypat } monkeypatch.setattr(routes, "all_sessions", lambda diag=None: [webui_row]) - monkeypatch.setattr(routes, "get_cli_sessions", lambda source_filter=None: [duplicate_webui_projection, external_projection]) + monkeypatch.setattr(routes, "get_cli_sessions", lambda source_filter=None, all_profiles=False: [duplicate_webui_projection, external_projection]) handler = _FakeHandler() routes.handle_get(handler, urlparse("http://example.com/api/sessions"))