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.
This commit is contained in:
1 parent
ffbc91343a
commit
38119fac25
2 files changed
+73
-18
No files matched your search
+35
-18
@@ -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):
|
||||
|
||||
@@ -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
|
||||
Reference in new issue
Block a user