From f333e9308757db983b99d02b21a36b9dc621278e Mon Sep 17 00:00:00 2001 From: chen Date: Thu, 9 Jul 2026 23:29:42 +0800 Subject: [PATCH 1/3] refactor: shared session load path for session and export APIs --- api/_session_handlers.py | 95 +++++++++++++++++++++++++ api/export_api.py | 118 ++++++++++++-------------------- api/sessions.py | 116 +++++++++---------------------- tests/test_api_routes.py | 2 +- tests/test_error_propagation.py | 4 +- utils/export_engine.py | 9 +-- utils/session_errors.py | 11 +++ 7 files changed, 186 insertions(+), 169 deletions(-) create mode 100644 api/_session_handlers.py create mode 100644 utils/session_errors.py diff --git a/api/_session_handlers.py b/api/_session_handlers.py new file mode 100644 index 0000000..cbb2f5b --- /dev/null +++ b/api/_session_handlers.py @@ -0,0 +1,95 @@ +"""Shared resolve/load/exclude/error helpers for session API handlers.""" + +from __future__ import annotations + +import os +from collections.abc import Callable +from dataclasses import dataclass + +from flask import current_app + +from api._flask_types import FlaskReturn +from api.error_codes import ErrorCode, error_response +from models.session import SessionDict +from models.stats import SessionStatsDict +from utils.exclusion_rules import is_session_excluded +from utils.session_cache import get_cached_session +from utils.session_errors import SESSION_LOAD_ERRORS +from utils.session_path import get_claude_projects_dir, safe_join +from utils.session_stats import compute_stats + +__all__ = [ + "SESSION_LOAD_ERRORS", + "LoadedSession", + "resolve_loaded_session", + "compute_stats_or_error", +] + + +@dataclass(frozen=True) +class LoadedSession: + session: SessionDict + filepath: str + + +def resolve_loaded_session( + project_name: str, + session_id: str, + *, + missing_file_message: str | Callable[[str], str], + parse_log_action: str = "Failed to parse session %s", +) -> LoadedSession | FlaskReturn: + """Resolve path, load session, and apply exclusion rules. + + Returns ``LoadedSession`` on success or an ``error_response`` tuple/Response. + """ + base = current_app.config.get("CLAUDE_PROJECTS_DIR") or get_claude_projects_dir() + try: + filepath = safe_join(base, project_name, f"{session_id}.jsonl") + except ValueError: + return error_response(ErrorCode.INVALID_PATH, "Invalid path", 400) + + if not os.path.isfile(filepath): + msg = ( + missing_file_message(session_id) + if callable(missing_file_message) + else missing_file_message + ) + return error_response(ErrorCode.SESSION_NOT_FOUND, msg, 404) + + try: + session = get_cached_session(filepath) + except SESSION_LOAD_ERRORS: + current_app.logger.exception(parse_log_action, session_id) + return error_response( + ErrorCode.PARSE_ERROR, + "Failed to parse session", + 500, + ) + + rules = current_app.config.get("EXCLUSION_RULES") or [] + if is_session_excluded(rules, session, project_name): + return error_response( + ErrorCode.SESSION_NOT_FOUND, + "Session not found", + 404, + ) + + return LoadedSession(session=session, filepath=filepath) + + +def compute_stats_or_error( + session: SessionDict, + session_id: str, + *, + log_action: str, +) -> SessionStatsDict | FlaskReturn: + try: + return compute_stats(session) + except SESSION_LOAD_ERRORS: + current_app.logger.exception(log_action, session_id) + return error_response( + ErrorCode.INTERNAL_ERROR, + "Failed to compute session stats", + 500, + ) diff --git a/api/export_api.py b/api/export_api.py index 15b62c0..f643edd 100644 --- a/api/export_api.py +++ b/api/export_api.py @@ -2,7 +2,6 @@ import io import json -import os import zipfile from datetime import datetime from typing import Any @@ -10,15 +9,14 @@ from flask import Blueprint, current_app, request, send_file from api._flask_types import FlaskReturn, json_response +from api._session_handlers import ( + LoadedSession, + compute_stats_or_error, + resolve_loaded_session, +) from api.error_codes import ErrorCode, error_response from models.export import ExportStateDict -from utils.exclusion_rules import is_session_excluded -from utils.export_engine import ( - EXPORT_ERRORS as _EXPORT_ERRORS, - ExportFailure, - ZipSink, - run_bulk_export, -) +from utils.export_engine import ExportFailure, ZipSink, run_bulk_export from utils.export_state_store import ( EXPORT_STATE_FILE, atomic_write_export_state, @@ -27,9 +25,7 @@ ) from utils.json_exporter import session_to_json from utils.md_exporter import session_to_markdown -from utils.session_cache import get_cached_session -from utils.session_path import get_claude_projects_dir, list_projects, safe_join -from utils.session_stats import compute_stats +from utils.session_path import get_claude_projects_dir, list_projects from utils.slugify import slugify export_bp = Blueprint("export", __name__) @@ -222,67 +218,41 @@ def _on_export_error(sid: str, exc: Exception) -> None: @export_bp.route("/api/export/session//") def export_session(project_name: str, session_id: str) -> FlaskReturn: - base = current_app.config.get("CLAUDE_PROJECTS_DIR") or get_claude_projects_dir() - try: - filepath = safe_join(base, project_name, f"{session_id}.jsonl") - except ValueError: - return error_response(ErrorCode.INVALID_PATH, "Invalid path", 400) - - if not os.path.isfile(filepath): - return error_response( - ErrorCode.SESSION_NOT_FOUND, - "Session not found", - 404, - ) - - fmt = request.args.get("format", "md") - try: - session = get_cached_session(filepath) - except _EXPORT_ERRORS: - current_app.logger.exception("Failed to parse session %s for export", session_id) - return error_response( - ErrorCode.PARSE_ERROR, - "Failed to parse session", - 500, - ) - - rules = current_app.config.get("EXCLUSION_RULES") or [] - if is_session_excluded(rules, session, project_name): - return error_response( - ErrorCode.SESSION_NOT_FOUND, - "Session not found", - 404, - ) - - try: - stats = compute_stats(session) - except _EXPORT_ERRORS: - current_app.logger.exception("Failed to compute stats for export %s", session_id) - return error_response( - ErrorCode.INTERNAL_ERROR, - "Failed to compute session stats", - 500, - ) - - title_slug = slugify(session["title"], default="session") - - if fmt == "json": - content = session_to_json(session, stats) - buf = io.BytesIO(content.encode("utf-8")) - buf.seek(0) - return send_file( - buf, - mimetype="application/json", - as_attachment=True, - download_name=f"{title_slug}.json", # type: ignore[call-arg] - ) - - md = session_to_markdown(session, stats) - buf = io.BytesIO(md.encode("utf-8")) - buf.seek(0) - return send_file( - buf, - mimetype="text/markdown", - as_attachment=True, - download_name=f"{title_slug}.md", # type: ignore[call-arg] + loaded = resolve_loaded_session( + project_name, + session_id, + missing_file_message="Session not found", + parse_log_action="Failed to parse session %s for export", ) + if isinstance(loaded, LoadedSession): + fmt = request.args.get("format", "md") + stats = compute_stats_or_error( + loaded.session, + session_id, + log_action="Failed to compute stats for export %s", + ) + if isinstance(stats, dict): + title_slug = slugify(loaded.session["title"], default="session") + + if fmt == "json": + content = session_to_json(loaded.session, stats) + buf = io.BytesIO(content.encode("utf-8")) + buf.seek(0) + return send_file( + buf, + mimetype="application/json", + as_attachment=True, + download_name=f"{title_slug}.json", # type: ignore[call-arg] + ) + + md = session_to_markdown(loaded.session, stats) + buf = io.BytesIO(md.encode("utf-8")) + buf.seek(0) + return send_file( + buf, + mimetype="text/markdown", + as_attachment=True, + download_name=f"{title_slug}.md", # type: ignore[call-arg] + ) + return stats + return loaded diff --git a/api/sessions.py b/api/sessions.py index 0e42848..2922270 100644 --- a/api/sessions.py +++ b/api/sessions.py @@ -1,101 +1,47 @@ """Session detail and stats endpoints.""" -import json -import os - -from flask import Blueprint, current_app +from flask import Blueprint from api._flask_types import FlaskReturn, json_response -from api.error_codes import ErrorCode, error_response -from utils.exclusion_rules import is_session_excluded -from utils.session_cache import get_cached_session -from utils.session_path import get_claude_projects_dir, safe_join -from utils.session_stats import compute_stats +from api._session_handlers import ( + LoadedSession, + compute_stats_or_error, + resolve_loaded_session, +) sessions_bp = Blueprint("sessions", __name__) -_PARSE_ERRORS = ( - json.JSONDecodeError, - KeyError, - ValueError, - OSError, - FileNotFoundError, -) + +def _missing_session_message(session_id: str) -> str: + return f"Session {session_id} not found" @sessions_bp.route("/api/sessions//") def get_session(project_name: str, session_id: str) -> FlaskReturn: - base = current_app.config.get("CLAUDE_PROJECTS_DIR") or get_claude_projects_dir() - try: - filepath = safe_join(base, project_name, f"{session_id}.jsonl") - except ValueError: - return error_response(ErrorCode.INVALID_PATH, "Invalid path", 400) - - if not os.path.isfile(filepath): - return error_response( - ErrorCode.SESSION_NOT_FOUND, - f"Session {session_id} not found", - 404, - ) - - try: - session = get_cached_session(filepath) - rules = current_app.config.get("EXCLUSION_RULES") or [] - if is_session_excluded(rules, session, project_name): - return error_response( - ErrorCode.SESSION_NOT_FOUND, - "Session not found", - 404, - ) - return json_response(session) - except _PARSE_ERRORS: - current_app.logger.exception("Failed to parse session %s", session_id) - return error_response( - ErrorCode.PARSE_ERROR, - "Failed to parse session", - 500, - ) + loaded = resolve_loaded_session( + project_name, + session_id, + missing_file_message=_missing_session_message, + ) + if isinstance(loaded, LoadedSession): + return json_response(loaded.session) + return loaded @sessions_bp.route("/api/sessions///stats") def get_session_stats(project_name: str, session_id: str) -> FlaskReturn: - base = current_app.config.get("CLAUDE_PROJECTS_DIR") or get_claude_projects_dir() - try: - filepath = safe_join(base, project_name, f"{session_id}.jsonl") - except ValueError: - return error_response(ErrorCode.INVALID_PATH, "Invalid path", 400) - - if not os.path.isfile(filepath): - return error_response( - ErrorCode.SESSION_NOT_FOUND, - f"Session {session_id} not found", - 404, - ) - - try: - session = get_cached_session(filepath) - rules = current_app.config.get("EXCLUSION_RULES") or [] - if is_session_excluded(rules, session, project_name): - return error_response( - ErrorCode.SESSION_NOT_FOUND, - "Session not found", - 404, - ) - except _PARSE_ERRORS: - current_app.logger.exception("Failed to parse session %s", session_id) - return error_response( - ErrorCode.PARSE_ERROR, - "Failed to parse session", - 500, - ) - - try: - stats = compute_stats(session) - return json_response(stats) - except _PARSE_ERRORS: - current_app.logger.exception("Failed to compute stats for %s", session_id) - return error_response( - ErrorCode.INTERNAL_ERROR, - "Failed to compute session stats", - 500, + loaded = resolve_loaded_session( + project_name, + session_id, + missing_file_message=_missing_session_message, + ) + if isinstance(loaded, LoadedSession): + stats = compute_stats_or_error( + loaded.session, + session_id, + log_action="Failed to compute stats for %s", ) + if isinstance(stats, dict): + return json_response(stats) + return stats + return loaded diff --git a/tests/test_api_routes.py b/tests/test_api_routes.py index 3e99020..7a33b4f 100644 --- a/tests/test_api_routes.py +++ b/tests/test_api_routes.py @@ -66,7 +66,7 @@ def test_session_detail_parse_failure_returns_500_without_leak(client, monkeypat def _boom(*_args, **_kwargs): raise KeyError("internal_secret_field_id") - monkeypatch.setattr("api.sessions.get_cached_session", _boom) + monkeypatch.setattr("api._session_handlers.get_cached_session", _boom) resp = client.get("/api/sessions/test-project/session_abc123") assert resp.status_code == 500 body_text = resp.get_data(as_text=True) diff --git a/tests/test_error_propagation.py b/tests/test_error_propagation.py index 02119d6..c2515f8 100644 --- a/tests/test_error_propagation.py +++ b/tests/test_error_propagation.py @@ -110,7 +110,7 @@ def test_500_on_parse_failure_does_not_leak_class_name(self, tmp_path, client, m def _boom(*args, **kwargs): raise KeyError("internal_secret_field_id") - monkeypatch.setattr("api.sessions.get_cached_session", _boom) + monkeypatch.setattr("api._session_handlers.get_cached_session", _boom) resp = client.get("/api/sessions/proj/abc") assert resp.status_code == 500 @@ -152,7 +152,7 @@ def test_500_on_parse_failure_does_not_leak_class_name(self, tmp_path, client, m def _boom(*args, **kwargs): raise ValueError("invalid literal: '/private/path/secret.json'") - monkeypatch.setattr("api.sessions.get_cached_session", _boom) + monkeypatch.setattr("api._session_handlers.get_cached_session", _boom) resp = client.get("/api/sessions/proj/abc/stats") assert resp.status_code == 500 diff --git a/utils/export_engine.py b/utils/export_engine.py index 2f7bf52..8a28a08 100644 --- a/utils/export_engine.py +++ b/utils/export_engine.py @@ -18,17 +18,12 @@ from utils.json_exporter import session_to_json from utils.jsonl_parser import parse_session from utils.md_exporter import session_to_markdown +from utils.session_errors import SESSION_LOAD_ERRORS from utils.session_path import list_sessions from utils.session_stats import compute_stats from utils.slugify import slugify -EXPORT_ERRORS = ( - json.JSONDecodeError, - KeyError, - ValueError, - OSError, - FileNotFoundError, -) +EXPORT_ERRORS = SESSION_LOAD_ERRORS PathLayout = Literal["api", "cli"] ManifestStyle = Literal["api", "cli"] diff --git a/utils/session_errors.py b/utils/session_errors.py new file mode 100644 index 0000000..e607d3b --- /dev/null +++ b/utils/session_errors.py @@ -0,0 +1,11 @@ +"""Exception types treated as session load/parse failures.""" + +import json + +SESSION_LOAD_ERRORS: tuple[type[BaseException], ...] = ( + json.JSONDecodeError, + KeyError, + ValueError, + OSError, + FileNotFoundError, +) From b6915bacb29190a445933512749f0f55eb9f85a0 Mon Sep 17 00:00:00 2001 From: chen Date: Fri, 10 Jul 2026 04:10:14 +0800 Subject: [PATCH 2/3] fix: handle exclusion failures in session load try block --- api/_session_handlers.py | 15 ++++---- tests/test_error_propagation.py | 61 +++++++++++++++++++++++++++++++++ utils/session_errors.py | 4 --- 3 files changed, 68 insertions(+), 12 deletions(-) diff --git a/api/_session_handlers.py b/api/_session_handlers.py index cbb2f5b..a87c68e 100644 --- a/api/_session_handlers.py +++ b/api/_session_handlers.py @@ -59,6 +59,13 @@ def resolve_loaded_session( try: session = get_cached_session(filepath) + rules = current_app.config.get("EXCLUSION_RULES") or [] + if is_session_excluded(rules, session, project_name): + return error_response( + ErrorCode.SESSION_NOT_FOUND, + "Session not found", + 404, + ) except SESSION_LOAD_ERRORS: current_app.logger.exception(parse_log_action, session_id) return error_response( @@ -67,14 +74,6 @@ def resolve_loaded_session( 500, ) - rules = current_app.config.get("EXCLUSION_RULES") or [] - if is_session_excluded(rules, session, project_name): - return error_response( - ErrorCode.SESSION_NOT_FOUND, - "Session not found", - 404, - ) - return LoadedSession(session=session, filepath=filepath) diff --git a/tests/test_error_propagation.py b/tests/test_error_propagation.py index c2515f8..a711fdd 100644 --- a/tests/test_error_propagation.py +++ b/tests/test_error_propagation.py @@ -32,6 +32,7 @@ class names from a defensive blocklist. from flask import Flask +from api.export_api import export_bp from api.projects import projects_bp from api.search import _IndexSearchOutcome from api.sessions import sessions_bp @@ -94,6 +95,13 @@ def _write_session(tmp_path, project: str, session_id: str, content: str): return p +def _patch_exclusion_raises(monkeypatch): + def _boom(*_args, **_kwargs): + raise ValueError("internal_exclusion_secret") + + monkeypatch.setattr("api._session_handlers.is_session_excluded", _boom) + + # --------------------------------------------------------------------------- # /api/sessions// # --------------------------------------------------------------------------- @@ -122,6 +130,18 @@ def _boom(*args, **kwargs): assert "internal_secret_field_id" not in json.dumps(body) _assert_no_class_name_leak(json.dumps(body)) + def test_500_when_exclusion_raises_does_not_leak(self, tmp_path, client, monkeypatch): + _write_session(tmp_path, "proj", "abc", '{"type":"user","message":{}}') + _patch_exclusion_raises(monkeypatch) + + resp = client.get("/api/sessions/proj/abc") + assert resp.status_code == 500 + body = resp.get_json() + assert body.get("error") == "Failed to parse session" + assert body.get("code") == "PARSE_ERROR" + assert "internal_exclusion_secret" not in json.dumps(body) + _assert_no_class_name_leak(json.dumps(body)) + def test_404_on_missing_file_keeps_session_id_safe(self, tmp_path, client): # Session ID is part of the URL so it appears in the 404 message — # that's fine; what we're guarding is exception-class leakage, which @@ -163,6 +183,47 @@ def _boom(*args, **kwargs): assert "/private/path" not in json.dumps(body) _assert_no_class_name_leak(json.dumps(body)) + def test_500_when_exclusion_raises_does_not_leak(self, tmp_path, client, monkeypatch): + _write_session(tmp_path, "proj", "abc", '{"type":"user","message":{}}') + _patch_exclusion_raises(monkeypatch) + + resp = client.get("/api/sessions/proj/abc/stats") + assert resp.status_code == 500 + body = resp.get_json() + assert body.get("error") == "Failed to parse session" + assert body.get("code") == "PARSE_ERROR" + assert "internal_exclusion_secret" not in json.dumps(body) + _assert_no_class_name_leak(json.dumps(body)) + + +# --------------------------------------------------------------------------- +# /api/export/session +# --------------------------------------------------------------------------- + + +class TestExportSessionErrorBody: + @pytest.fixture + def export_client(self, tmp_path): + app = Flask(__name__) + app.config["TESTING"] = True + app.config["CLAUDE_PROJECTS_DIR"] = str(tmp_path) + app.register_blueprint(export_bp) + return app.test_client() + + def test_500_when_exclusion_raises_does_not_leak( + self, tmp_path, export_client, monkeypatch + ): + _write_session(tmp_path, "proj", "abc", '{"type":"user","message":{}}') + _patch_exclusion_raises(monkeypatch) + + resp = export_client.get("/api/export/session/proj/abc") + assert resp.status_code == 500 + body = resp.get_json() + assert body.get("error") == "Failed to parse session" + assert body.get("code") == "PARSE_ERROR" + assert "internal_exclusion_secret" not in json.dumps(body) + _assert_no_class_name_leak(json.dumps(body)) + # --------------------------------------------------------------------------- # /api/projects (per-session card) diff --git a/utils/session_errors.py b/utils/session_errors.py index e607d3b..e28f209 100644 --- a/utils/session_errors.py +++ b/utils/session_errors.py @@ -1,11 +1,7 @@ """Exception types treated as session load/parse failures.""" -import json - SESSION_LOAD_ERRORS: tuple[type[BaseException], ...] = ( - json.JSONDecodeError, KeyError, ValueError, OSError, - FileNotFoundError, ) From 1b29cca4e8ac15affb0692d07eb136fe834c685c Mon Sep 17 00:00:00 2001 From: chen Date: Fri, 10 Jul 2026 04:14:25 +0800 Subject: [PATCH 3/3] style: ruff format test_error_propagation.py --- tests/test_error_propagation.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/tests/test_error_propagation.py b/tests/test_error_propagation.py index a711fdd..8463a3b 100644 --- a/tests/test_error_propagation.py +++ b/tests/test_error_propagation.py @@ -210,9 +210,7 @@ def export_client(self, tmp_path): app.register_blueprint(export_bp) return app.test_client() - def test_500_when_exclusion_raises_does_not_leak( - self, tmp_path, export_client, monkeypatch - ): + def test_500_when_exclusion_raises_does_not_leak(self, tmp_path, export_client, monkeypatch): _write_session(tmp_path, "proj", "abc", '{"type":"user","message":{}}') _patch_exclusion_raises(monkeypatch)