diff --git a/api/helpers.py b/api/helpers.py index 9a8e7f36..c92a4518 100644 --- a/api/helpers.py +++ b/api/helpers.py @@ -472,10 +472,31 @@ _redact_fn_lru = functools.lru_cache(maxsize=4096)(_redact_fn_uncached) # thousands of small recurring strings that actually benefit, or balloon RSS. _REDACT_CACHE_MAX_TEXT_LEN = 16384 +# Strings above that threshold used to bypass memoization entirely and re-run +# the full ~15-pass redactor on every request. On a real 22MB session that was +# 59 large strings (29 unique, 0.76MB) costing 1.68s per request — 99.9% of the +# recurring redaction cost once the small-string cache is warm, paid again by +# every tab and every poll for a byte-identical result. +# +# They get their own small cache instead of sharing the 4096-entry one, so a +# large blob can never evict the thousands of small recurring strings. Both the +# entry count and the per-entry size are capped, which bounds retained bytes to +# roughly _REDACT_LARGE_CACHE_SIZE * _REDACT_LARGE_CACHE_MAX_TEXT_LEN (key plus +# value). Anything larger stays uncached: unbounded growth is the failure mode +# this threshold exists to prevent. +_REDACT_LARGE_CACHE_MAX_TEXT_LEN = 131072 +_REDACT_LARGE_CACHE_SIZE = 64 + +_redact_fn_large_lru = functools.lru_cache(maxsize=_REDACT_LARGE_CACHE_SIZE)( + _redact_fn_uncached +) + def _redact_fn_cached(text): if len(text) > _REDACT_CACHE_MAX_TEXT_LEN: - return _redact_fn_uncached(text) + if len(text) > _REDACT_LARGE_CACHE_MAX_TEXT_LEN: + return _redact_fn_uncached(text) + return _redact_fn_large_lru(text) return _redact_fn_lru(text) @@ -956,13 +977,43 @@ def _redact_value(v, *, _enabled: bool | None = None): ``_enabled`` is threaded through so a single response-level redact pass only reads settings.json once. (Opus pre-release perf fix.) + + Containers are rebuilt only when a descendant actually changed. The + overwhelming majority of a transcript carries no credential marker, so the + previous unconditional dict/list comprehension deep-copied the entire + payload — tens of MB per response — to reproduce an identical structure. + That copy is pure CPU under the GIL (allocation and refcounting never + release it), which serialized concurrent tab loads on a threaded server. + + Returning the original object when nothing changed is safe because callers + treat redacted output as read-only: ``_public_message_projection`` builds a + fresh ``item`` dict per message, and ``redact_session_data`` builds a fresh + ``result``. Nothing mutates a value returned from here in place, so sharing + an unmodified subtree cannot leak a later mutation back into session state. + The redacting path is unchanged: as soon as one string is masked, every + container on the path to it is rebuilt and the caller's original is left + untouched. """ if isinstance(v, str): return _redact_text(v, _enabled=_enabled) if isinstance(v, dict): - return {key: _redact_value(value, _enabled=_enabled) for key, value in v.items()} + changed = False + out = {} + for key, value in v.items(): + redacted = _redact_value(value, _enabled=_enabled) + out[key] = redacted + if redacted is not value: + changed = True + return out if changed else v if isinstance(v, list): - return [_redact_value(item, _enabled=_enabled) for item in v] + changed = False + out = [] + for item in v: + redacted = _redact_value(item, _enabled=_enabled) + out.append(redacted) + if redacted is not item: + changed = True + return out if changed else v return v diff --git a/tests/test_redact_large_string_cache.py b/tests/test_redact_large_string_cache.py new file mode 100644 index 00000000..d125adc1 --- /dev/null +++ b/tests/test_redact_large_string_cache.py @@ -0,0 +1,114 @@ +"""Grosses chaines: memoiser au lieu de repayer ~15 regex a chaque requete. + +``_redact_fn_cached`` excluait du LRU toute chaine > 16 384 caracteres, pour +eviter d'evincer les milliers de petites chaines recurrentes et de gonfler la +RSS. Consequence mesuree sur une session reelle de 22 Mo: 59 grosses chaines +(29 uniques, 0,76 Mo) repassaient l'integralite des regex a CHAQUE requete, +pour un resultat toujours identique — 1,68 s par requete, soit 99,9% du cout +recurrent de la redaction une fois le cache des petites chaines chaud. + +Le correctif ajoute un SECOND cache, dedie aux grosses chaines, borne en +nombre d'entrees ET par une taille maximale par entree, pour que la RSS reste +plafonnee. Les chaines geantes (> plafond) restent volontairement non cachees. + +Contrat verifie ici: + 1. une grosse chaine est redigee exactement comme avant; + 2. la meme grosse chaine n'est pas recalculee au second appel; + 3. les deux caches restent separes (pas d'eviction croisee); + 4. une chaine au-dela du plafond n'est jamais retenue. +""" +import sys +from pathlib import Path + +import pytest + +sys.path.insert(0, str(Path(__file__).resolve().parents[1])) + +from api import helpers # noqa: E402 +from api.helpers import ( # noqa: E402 + _REDACT_CACHE_MAX_TEXT_LEN, + _REDACT_LARGE_CACHE_MAX_TEXT_LEN, + _redact_fn_cached, + _redact_fn_uncached, +) + +SECRET = "ghp_0123456789abcdefghijklmnopqrstuvwxyzAB" + + +@pytest.fixture(autouse=True) +def _isolate_large_cache(): + """Ne pas laisser ce fichier polluer l'etat global du process. + + Le cache est un singleton de module partage par toute la suite. Le vider + sans le restaurer laisse les tests suivants demarrer a froid, ce qui + perturbe ceux qui dependent de fenetres de fraicheur ou de temps de + reponse. On repart d'un cache vide ET on le revide en sortie, pour que ce + fichier n'ait aucun effet observable en dehors de lui-meme. + """ + helpers._redact_fn_large_lru.cache_clear() + yield + helpers._redact_fn_large_lru.cache_clear() + + +def _big(secret: str, size: int) -> str: + """Chaine > seuil du petit cache, porteuse d'un secret. + + Le remplissage doit etre assez long pour atteindre ``size`` meme au-dela + du plafond du grand cache, sinon la troncature produit une chaine trop + courte et le test ne verifie plus ce qu'il annonce. + """ + unit = "lorem ipsum dolor sit amet " + filler = unit * (size // len(unit) + 1) + text = (filler + secret + filler)[:size] + assert len(text) == size + return text + + +def test_large_string_redaction_matches_uncached(): + text = _big(SECRET, _REDACT_CACHE_MAX_TEXT_LEN + 5000) + assert len(text) > _REDACT_CACHE_MAX_TEXT_LEN + assert _redact_fn_cached(text) == _redact_fn_uncached(text) + assert SECRET not in _redact_fn_cached(text) + + +def test_large_string_is_memoized(): + helpers._redact_fn_large_lru.cache_clear() + text = _big(SECRET, _REDACT_CACHE_MAX_TEXT_LEN + 7000) + + first = _redact_fn_cached(text) + misses_after_first = helpers._redact_fn_large_lru.cache_info().misses + + second = _redact_fn_cached(text) + info = helpers._redact_fn_large_lru.cache_info() + + assert second == first + assert info.hits >= 1, "grosse chaine recalculee au second appel" + assert info.misses == misses_after_first, "miss supplementaire inattendu" + + +def test_small_strings_do_not_use_the_large_cache(): + helpers._redact_fn_large_lru.cache_clear() + small = f"petit texte {SECRET}" + assert len(small) <= _REDACT_CACHE_MAX_TEXT_LEN + _redact_fn_cached(small) + info = helpers._redact_fn_large_lru.cache_info() + assert info.hits == 0 and info.misses == 0, "petite chaine routee vers le grand cache" + + +def test_giant_string_is_not_retained(): + """Au-dela du plafond: correct, mais jamais mis en cache (RSS bornee).""" + helpers._redact_fn_large_lru.cache_clear() + giant = _big(SECRET, _REDACT_LARGE_CACHE_MAX_TEXT_LEN + 10000) + assert len(giant) > _REDACT_LARGE_CACHE_MAX_TEXT_LEN + + out = _redact_fn_cached(giant) + + assert out == _redact_fn_uncached(giant) + assert SECRET not in out + info = helpers._redact_fn_large_lru.cache_info() + assert info.currsize == 0, "chaine geante retenue en cache" + + +def test_large_cache_is_bounded(): + assert helpers._redact_fn_large_lru.cache_info().maxsize is not None + assert helpers._redact_fn_large_lru.cache_info().maxsize <= 128 diff --git a/tests/test_redact_value_no_copy_when_clean.py b/tests/test_redact_value_no_copy_when_clean.py new file mode 100644 index 00000000..ffb8f875 --- /dev/null +++ b/tests/test_redact_value_no_copy_when_clean.py @@ -0,0 +1,105 @@ +"""Redaction: ne pas reconstruire ce qui n'a pas change. + +``_redact_value`` reconstruisait integralement chaque dict/liste, soit une +copie profonde de tout le payload a chaque reponse, meme quand rien n'est +redige. Sur une session de 22 Mo, 97,4% des chaines ne contiennent aucun +marqueur sensible: la copie est donc du travail pur perte, et elle tient le +GIL (json/allocations ne le liberent pas), ce qui serialise les onglets. + +Contrat verifie ici: + 1. Quand rien n'est redige, la valeur RENVOYEE EST l'objet d'origine + (identite, pas seulement egalite) -> zero allocation. + 2. Quand quelque chose est redige, un NOUVEL objet est renvoye et + l'original n'est PAS mute (fail-closed: pas de fuite par aliasing). + 3. Le resultat reste egal a celui de l'ancienne implementation. +""" +import sys +from pathlib import Path + +sys.path.insert(0, str(Path(__file__).resolve().parents[1])) + +from api.helpers import _redact_value # noqa: E402 + + +SECRET = "authorization=Bearer sk-live-abcdef0123456789" + + +def _reference_redact(v, *, _enabled): + """Implementation d'origine (copie systematique), pour comparaison.""" + from api.helpers import _redact_text + + if isinstance(v, str): + return _redact_text(v, _enabled=_enabled) + if isinstance(v, dict): + return {k: _reference_redact(x, _enabled=_enabled) for k, x in v.items()} + if isinstance(v, list): + return [_reference_redact(x, _enabled=_enabled) for x in v] + return v + + +def test_clean_payload_is_returned_by_identity(): + """Rien de sensible -> aucun dict/liste ne doit etre reconstruit.""" + payload = { + "messages": [ + {"role": "user", "content": "bonjour, ou en est la commande 4512 ?"}, + {"role": "assistant", "content": [{"type": "text", "text": "elle part demain"}]}, + ], + "meta": {"workspace": "/a0/usr/projects/MES", "count": 3, "ok": True}, + } + + out = _redact_value(payload, _enabled=True) + + assert out is payload, "payload propre reconstruit inutilement" + assert out["messages"] is payload["messages"] + assert out["messages"][0] is payload["messages"][0] + assert out["meta"] is payload["meta"] + + +def test_sensitive_payload_is_copied_and_original_untouched(): + """Quelque chose est redige -> nouvel objet, original intact.""" + inner = {"role": "user", "content": SECRET} + payload = {"messages": [inner], "meta": {"workspace": "/tmp"}} + + out = _redact_value(payload, _enabled=True) + + # un nouvel objet est renvoye sur le chemin modifie + assert out is not payload + assert out["messages"] is not payload["messages"] + assert out["messages"][0] is not inner + + # l'original n'est pas mute (pas de fuite par aliasing) + assert inner["content"] == SECRET + assert payload["messages"][0]["content"] == SECRET + + # le secret est bien masque dans la sortie + assert out["messages"][0]["content"] != SECRET + + # les branches NON modifiees restent partagees (c'est tout l'interet) + assert out["meta"] is payload["meta"] + + +def test_matches_reference_implementation(): + """Le resultat doit rester identique a l'implementation d'origine.""" + payload = { + "messages": [ + {"role": "user", "content": "texte normal"}, + {"role": "assistant", "content": SECRET}, + {"role": "user", "content": ["propre", SECRET, 42, None]}, + ], + "nested": {"a": {"b": {"c": SECRET}}, "d": {"e": "rien"}}, + "scalars": [1, 2.5, True, None], + } + + assert _redact_value(payload, _enabled=True) == _reference_redact(payload, _enabled=True) + + +def test_disabled_redaction_still_shares(): + """Redaction desactivee -> rien ne doit etre copie.""" + payload = {"messages": [{"role": "user", "content": SECRET}]} + out = _redact_value(payload, _enabled=False) + assert out is payload + + +def test_non_container_values_pass_through(): + for v in (42, 2.5, True, None): + assert _redact_value(v, _enabled=True) is v