From 38119fac25f6bf92961766fc9d68cb6d6a317885 Mon Sep 17 00:00:00 2001 From: nesquena-hermes Date: Wed, 15 Jul 2026 21:06:36 +0000 Subject: [PATCH] fix(#6056): extend sidebar title-field redaction to /api/sessions/search (Codex class-fix) Codex re-gate found the same derived-title leak on the sibling /api/sessions/search endpoint (3 branches redacted only 'title'). Extract _redact_sidebar_title_fields() shared helper, apply it in both _sidebar_session_response_item and all 3 search branches, read api_redact_enabled once per search response (mirrors #4662). +helper test + source-guard that every search branch calls the redactor. --- api/routes.py | 53 ++++++++++++------- ...t_issue4662_sidebar_redaction_read_once.py | 38 +++++++++++++ 2 files changed, 73 insertions(+), 18 deletions(-) diff --git a/api/routes.py b/api/routes.py index af8a4a3f..80b08b4b 100644 --- a/api/routes.py +++ b/api/routes.py @@ -9632,24 +9632,30 @@ def _sidebar_session_response_item(session: dict, *, redact_enabled: bool | None } if isinstance(item.get("title"), str): item["title"] = _redact_text(item["title"], _enabled=redact_enabled) - # display_title / _state_db_title can carry raw user-message-derived text - # (e.g. #6056 derives a delegated subagent's sidebar title from its first - # user message), so they must go through the SAME redaction as title — a - # credential-shaped value in a delegated goal would otherwise leak to the - # sidebar even with api_redact_enabled=True. - if isinstance(item.get("display_title"), str): - item["display_title"] = _redact_text(item["display_title"], _enabled=redact_enabled) - if isinstance(item.get("_state_db_title"), str): - item["_state_db_title"] = _redact_text(item["_state_db_title"], _enabled=redact_enabled) - # parent_title is a copied parent-session title that surfaces in the sidebar - # lineage collapse; redact it on the same path so a parent whose title was - # itself user-content-derived can't leak through the child row. - if isinstance(item.get("parent_title"), str): - item["parent_title"] = _redact_text(item["parent_title"], _enabled=redact_enabled) + _redact_sidebar_title_fields(item, redact_enabled) item["attention"] = _session_attention_summary(str(item.get("session_id") or "")) return item +def _redact_sidebar_title_fields(item: dict, redact_enabled: bool | None = None) -> None: + """Redact every user-content-derived title field on a sidebar/search row in place. + + `title` is redacted by the callers directly (they special-case it), but + `display_title`, `_state_db_title`, and `parent_title` can ALSO carry raw + user-message-derived text — e.g. #6056 derives a delegated subagent's + `display_title` from its first user message, and `parent_title` copies a + parent session's (possibly derived) title. Without this a credential-shaped + value in a delegated goal would surface in the sidebar / search results even + with `api_redact_enabled=True`. Shared by `_sidebar_session_response_item` + (`/api/sessions`) and every `/api/sessions/search` response branch so the two + endpoints can never drift on which fields get redacted. + """ + for field in ("display_title", "_state_db_title", "parent_title"): + value = item.get(field) + if isinstance(value, str): + item[field] = _redact_text(value, _enabled=redact_enabled) + + # ── Login page locale strings ───────────────────────────────────────────────── # Add entries here to support more languages on the login page. # The key must match the 'language' setting value (from static/i18n.js LOCALES). @@ -16581,12 +16587,21 @@ def _handle_sessions_search(handler, parsed): depth = max(0, int(qs.get("depth", ["5"])[0])) except (ValueError, TypeError): depth = 5 + # Read the redaction setting ONCE for the whole response (mirrors the + # /api/sessions read-once optimization, #4662) and thread it through every + # branch + the shared title-field redactor so search rows redact the same + # fields as the sidebar list. + try: + _search_redact_enabled = bool(load_settings().get("api_redact_enabled", True)) + except Exception: + _search_redact_enabled = True # fail safe: redact when settings unreadable if not q: safe_sessions = [] for s in sessions: item = dict(s) if isinstance(item.get("title"), str): - item["title"] = _redact_text(item["title"]) + item["title"] = _redact_text(item["title"], _enabled=_search_redact_enabled) + _redact_sidebar_title_fields(item, _search_redact_enabled) safe_sessions.append(item) return j(handler, { "sessions": safe_sessions, @@ -16599,7 +16614,8 @@ def _handle_sessions_search(handler, parsed): if title_match: item = dict(s, match_type="title") if isinstance(item.get("title"), str): - item["title"] = _redact_text(item["title"]) + item["title"] = _redact_text(item["title"], _enabled=_search_redact_enabled) + _redact_sidebar_title_fields(item, _search_redact_enabled) results.append(item) continue if content_search: @@ -16612,9 +16628,10 @@ def _handle_sessions_search(handler, parsed): item = dict(s, match_type="content") preview = _session_search_preview(c, q) if preview: - item["match_preview"] = _redact_text(preview) + item["match_preview"] = _redact_text(preview, _enabled=_search_redact_enabled) if isinstance(item.get("title"), str): - item["title"] = _redact_text(item["title"]) + item["title"] = _redact_text(item["title"], _enabled=_search_redact_enabled) + _redact_sidebar_title_fields(item, _search_redact_enabled) results.append(item) break except (KeyError, Exception): diff --git a/tests/test_issue4662_sidebar_redaction_read_once.py b/tests/test_issue4662_sidebar_redaction_read_once.py index 6a33729b..cb0c2ab8 100644 --- a/tests/test_issue4662_sidebar_redaction_read_once.py +++ b/tests/test_issue4662_sidebar_redaction_read_once.py @@ -90,3 +90,41 @@ def test_sidebar_payload_redacts_display_and_parent_titles(monkeypatch): assert secret not in str(row.get(field, "")), ( f"credential leaked into sidebar {field}: {row.get(field)!r}" ) + + +def test_redact_sidebar_title_fields_helper(): + """The shared helper redacts every user-content-derived title field, honors the + enabled flag, and never raises on missing/non-str fields.""" + secret = "sk-" + ("B" * 44) + item = { + "title": "kept-by-caller", + "display_title": f"a {secret}", + "_state_db_title": f"b {secret}", + "parent_title": f"c {secret}", + "session_id": "s1", + } + routes._redact_sidebar_title_fields(item, True) + for field in ("display_title", "_state_db_title", "parent_title"): + assert secret not in item[field], f"{field} not redacted" + # Disabled → pass through unchanged. + item2 = {"display_title": f"a {secret}"} + routes._redact_sidebar_title_fields(item2, False) + assert item2["display_title"] == f"a {secret}" + # Missing / non-str fields must not raise. + routes._redact_sidebar_title_fields({"display_title": None, "parent_title": 42}, True) + + +def test_sessions_search_branches_redact_derived_titles(): + """Every /api/sessions/search response branch must call the shared title-field + redactor so search rows can't leak a derived display_title the sidebar hides. + Source-guard: the #6056 class-fix is easy to regress by adding a 4th branch.""" + import inspect + src = inspect.getsource(routes._handle_sessions_search) + # 3 response branches (empty-query list, title-match, content-match) each build + # an `item` and must pass it through _redact_sidebar_title_fields. + assert src.count("_redact_sidebar_title_fields(item") >= 3, ( + "a /api/sessions/search branch builds a row without redacting the derived " + "title fields (display_title/_state_db_title/parent_title)" + ) + # And it must read the setting once, not per-_redact_text call. + assert "_search_redact_enabled" in src