From 0acdf1d8c8a27615691d4b526749f299b863264b Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Wed, 22 Jul 2026 08:11:40 -0700 Subject: [PATCH] fix(compression): apply strict redaction at every compaction text boundary (#69294) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Compaction summaries persist across sessions and re-enter every subsequent summarizer prompt, but every redact_sensitive_text() call in context_compressor.py used default mode: a no-op under security.redact_secrets:false, and opaque OAuth-callback / URL-userinfo credentials passed through even when enabled. The stored _previous_summary also re-entered the iterative-update prompt unredacted. Add _redact_compaction_text() — redact_sensitive_text(force=True, redact_url_credentials=True) — and thread it through all compaction text boundaries: serializer input (content + tool args), deterministic fallback summary, summarizer LLM output, manual + auto focus topics, the latest-user task snapshot, and _previous_summary re-entry. Note: force=True at this boundary intentionally overrides security.redact_secrets:false — that opt-out targets live tool output, not persisted summaries. Salvages the compaction half of #49556 (the redact.py strict-URL half landed independently via 75af6dc57/62a00a739). Addresses #43666 item 2. Co-authored-by: AndrewMoryakov --- agent/context_compressor.py | 46 +++- contributors/emails/topazd2@gmail.com | 1 + .../test_compaction_redaction_boundaries.py | 223 ++++++++++++++++++ 3 files changed, 262 insertions(+), 8 deletions(-) create mode 100644 contributors/emails/topazd2@gmail.com create mode 100644 tests/agent/test_compaction_redaction_boundaries.py diff --git a/agent/context_compressor.py b/agent/context_compressor.py index 62fe2d928ed8..d3554bdaac25 100644 --- a/agent/context_compressor.py +++ b/agent/context_compressor.py @@ -342,6 +342,27 @@ _HISTORICAL_TASK_SECTION_RE = re.compile( ) +def _redact_compaction_text(text: Any) -> str: + """Redact text that crosses a compaction summary boundary. + + Compaction summaries persist across sessions and are re-injected into + every subsequent summarizer prompt, so this boundary uses strict mode: + + - ``force=True`` — deliberately overrides ``security.redact_secrets: + false``. That opt-out targets *live tool output* (e.g. working on the + redactor itself); a summary is a persistence boundary where a leaked + credential keeps re-entering prompts indefinitely. + - ``redact_url_credentials=True`` — OAuth callback codes, magic-link + tokens, and URL userinfo never need to survive summarization the way + they must survive live navigation flows. + """ + return redact_sensitive_text( + text or "", + force=True, + redact_url_credentials=True, + ) + + def _dedupe_append(items: list[str], value: str, *, limit: int) -> None: value = value.strip() if value and value not in items and len(items) < limit: @@ -1934,7 +1955,7 @@ class ContextCompressor(ContextEngine): elif isinstance(part, str): text_parts.append(part) content = "\n".join(text_parts) - content = redact_sensitive_text(content or "") + content = _redact_compaction_text(content or "") content = _MEDIA_DIRECTIVE_RE.sub("[media attachment]", content) # Strip inline reasoning blocks (, , etc.) from # assistant content before it reaches the summarizer. Reasoning @@ -1967,7 +1988,7 @@ class ContextCompressor(ContextEngine): if isinstance(tc, dict): fn = tc.get("function", {}) name = fn.get("name", "?") - args = redact_sensitive_text(fn.get("arguments", "")) + args = _redact_compaction_text(fn.get("arguments", "")) # Truncate long arguments but keep enough for context if len(args) > self._TOOL_ARGS_MAX: args = args[:self._TOOL_ARGS_HEAD] + "..." @@ -2010,7 +2031,7 @@ class ContextCompressor(ContextEngine): last_dropped_turns: list[str] = [] def _compact_fallback_turn(value: Any) -> str: - text = redact_sensitive_text(_content_text_for_contains(value)) + text = _redact_compaction_text(_content_text_for_contains(value)) text = re.sub(r"\bgh[pousr]_[A-Za-z0-9_]{8,}\b", "[REDACTED]", text) text = re.sub(r"\s+", " ", text).strip() if len(text) > _FALLBACK_TURN_MAX_CHARS: @@ -2042,7 +2063,7 @@ class ContextCompressor(ContextEngine): if msg.get("role") == "assistant" and msg.get("tool_calls"): for tc in msg.get("tool_calls") or []: name, raw_args = _extract_tool_call_name_and_args(tc) - args = redact_sensitive_text(raw_args) + args = _redact_compaction_text(raw_args) call_id = _extract_tool_call_id(tc) if call_id: call_id_to_tool[call_id] = (name, args) @@ -2180,7 +2201,7 @@ Continue from the most recent unfulfilled user ask and protected tail messages. ## Critical Context Summary generation was unavailable, so this is a best-effort deterministic fallback for {len(turns_to_summarize)} compacted message(s).{reason_text}""" - summary = self._with_summary_prefix(redact_sensitive_text(body.strip())) + summary = self._with_summary_prefix(_redact_compaction_text(body.strip())) if len(summary) > _FALLBACK_SUMMARY_MAX_CHARS: summary = summary[: _FALLBACK_SUMMARY_MAX_CHARS - 42].rstrip() + "\n...[fallback summary truncated]" return summary @@ -2243,6 +2264,15 @@ Summary generation was unavailable, so this is a best-effort deterministic fallb ) return None + # Strict-redact prompt inputs that bypass _serialize_for_summary: + # a manual `/compress ` string, and a previous summary that + # may predate compaction redaction (resumed from a persisted + # handoff message written before this boundary existed). + if focus_topic: + focus_topic = _redact_compaction_text(focus_topic) + if self._previous_summary: + self._previous_summary = _redact_compaction_text(self._previous_summary) + summary_budget = self._compute_summary_budget(turns_to_summarize) content_to_summarize = self._serialize_for_summary(turns_to_summarize) _sanitized_memory_context = sanitize_memory_context(memory_context) @@ -2547,7 +2577,7 @@ This compaction should PRIORITISE preserving all information related to the focu content = stripped # Redact the summary output as well — the summarizer LLM may # ignore prompt instructions and echo back secrets verbatim. - summary = redact_sensitive_text(content.strip()) + summary = _redact_compaction_text(content.strip()) summary = self._ground_historical_task_snapshot(summary, turns_to_summarize) self._validate_summary_user_provenance(summary, has_user_turn) # Store for iterative updates on next compaction @@ -2935,7 +2965,7 @@ This compaction should PRIORITISE preserving all information related to the focu if cls._is_synthetic_compression_user_turn(msg): continue content = msg.get("content") - text = redact_sensitive_text(_content_text_for_contains(content).strip()) + text = _redact_compaction_text(_content_text_for_contains(content).strip()) if not text: continue text = " ".join(text.split()) @@ -2979,7 +3009,7 @@ This compaction should PRIORITISE preserving all information related to the focu if not _is_real_user_message(msg): continue content = msg.get("content") - text = redact_sensitive_text(_content_text_for_contains(content).strip()) + text = _redact_compaction_text(_content_text_for_contains(content).strip()) if not text: continue text = re.sub(r"\s+", " ", text) diff --git a/contributors/emails/topazd2@gmail.com b/contributors/emails/topazd2@gmail.com new file mode 100644 index 000000000000..1143f8df2312 --- /dev/null +++ b/contributors/emails/topazd2@gmail.com @@ -0,0 +1 @@ +AndrewMoryakov diff --git a/tests/agent/test_compaction_redaction_boundaries.py b/tests/agent/test_compaction_redaction_boundaries.py new file mode 100644 index 000000000000..07e93a45873b --- /dev/null +++ b/tests/agent/test_compaction_redaction_boundaries.py @@ -0,0 +1,223 @@ +"""Strict redaction at every compaction text boundary (issue #43666 item 2). + +Compaction summaries persist across sessions and re-enter every subsequent +summarizer prompt, so ``_redact_compaction_text()`` applies strict mode +(``force=True, redact_url_credentials=True``) at each boundary: + +- serializer input (``_serialize_for_summary``: message content + tool args) +- deterministic fallback summary (``_build_static_fallback_summary``) +- summarizer LLM output (``_generate_summary`` return / ``_previous_summary``) +- focus text (manual ``/compress `` and ``_derive_auto_focus_topic``) +- previous-summary re-entry into the iterative-update prompt + +Every test disables the global redaction flag (simulating +``security.redact_secrets: false``) to prove ``force=True`` still redacts at +the persistence boundary, and uses an OAuth-callback-style URL to prove +``redact_url_credentials=True`` strips opaque URL tokens that default-mode +redaction deliberately passes through. +""" + +from unittest.mock import MagicMock, patch + +import pytest + +from agent.context_compressor import ( + ContextCompressor, + SUMMARY_PREFIX, + _redact_compaction_text, +) + +SECRET = "sk-proj-" + ("a" * 40) +OAUTH_URL = ( + "https://localhost/callback?code=opaque-code-123" + "&access_token=opaque-token-456&state=keep" +) + + +@pytest.fixture(autouse=True) +def _redaction_globally_disabled(monkeypatch): + """Simulate security.redact_secrets: false — force=True must still win.""" + monkeypatch.setattr("agent.redact._REDACT_ENABLED", False) + + +def _compressor() -> ContextCompressor: + with patch( + "agent.context_compressor.get_model_context_length", + return_value=100000, + ): + return ContextCompressor(model="test/model", quiet_mode=True) + + +def _response(content: str): + mock_response = MagicMock() + mock_response.choices = [MagicMock()] + mock_response.choices[0].message.content = content + return mock_response + + +def _assert_clean(text: str): + assert SECRET not in text + assert "sk-proj-" not in text + assert "code=opaque-code-123" not in text + assert "access_token=opaque-token-456" not in text + assert "code=***" in text + assert "access_token=***" in text + assert "state=keep" in text + + +def test_helper_is_strict_even_when_redaction_disabled(): + result = _redact_compaction_text(f"key {SECRET} url {OAUTH_URL}") + _assert_clean(result) + # None-safety: helper is used on optional fields. + assert _redact_compaction_text(None) == "" + + +def test_serializer_input_redacts_content_and_tool_args(): + c = _compressor() + messages = [ + {"role": "user", "content": f"token {SECRET} url {OAUTH_URL}"}, + { + "role": "assistant", + "content": None, + "tool_calls": [ + { + "id": "call-1", + "function": { + "name": "terminal", + "arguments": ( + f'{{"command": "curl {OAUTH_URL}",' + f' "note": "{SECRET}"}}' + ), + }, + } + ], + }, + {"role": "tool", "tool_call_id": "call-1", "content": f"got {SECRET}"}, + ] + + serialized = c._serialize_for_summary(messages) + + _assert_clean(serialized) + + +def test_fallback_summary_redacts_secrets(): + c = _compressor() + turns = [ + {"role": "user", "content": f"deploy with {SECRET} via {OAUTH_URL}"}, + {"role": "assistant", "content": f"ran curl {OAUTH_URL}"}, + ] + + summary = c._build_static_fallback_summary(turns, reason="test outage") + + _assert_clean(summary) + + +def test_summary_output_redacts_llm_echoed_secrets(): + c = _compressor() + leaked = f"Summary leaked OPENAI_API_KEY {SECRET} and {OAUTH_URL}" + + with patch( + "agent.context_compressor.call_llm", return_value=_response(leaked) + ): + summary = c._generate_summary([{"role": "user", "content": "hi"}]) + + assert summary is not None + _assert_clean(summary) + # The stored iterative-update seed must be clean too. + _assert_clean(c._previous_summary) + + +def test_manual_focus_topic_redacted_before_summary_prompt(): + c = _compressor() + turns = [ + {"role": "user", "content": "Summarize safely"}, + {"role": "assistant", "content": "OK"}, + ] + + with patch( + "agent.context_compressor.call_llm", + return_value=_response("## Goal\nSafe summary."), + ) as mock_call: + result = c._generate_summary( + turns, focus_topic=f"manual focus {SECRET} {OAUTH_URL}" + ) + + assert result is not None + prompt = mock_call.call_args.kwargs["messages"][0]["content"] + _assert_clean(prompt) + + +def test_auto_focus_topic_redacted(): + c = _compressor() + + focus = c._derive_auto_focus_topic( + [ + {"role": "assistant", "content": "older assistant turn"}, + {"role": "user", "content": f"focus has {SECRET} and {OAUTH_URL}"}, + ] + ) + + assert focus is not None + _assert_clean(focus) + + +def test_previous_summary_redacted_before_iterative_prompt_reentry(): + """Legacy persisted summaries may predate compaction redaction.""" + c = _compressor() + c._previous_summary = f"Old summary leaked {SECRET} and {OAUTH_URL}" + + with patch( + "agent.context_compressor.call_llm", + return_value=_response("updated summary"), + ) as mock_call: + result = c._generate_summary( + [ + {"role": "user", "content": "new turn"}, + {"role": "assistant", "content": "new work"}, + ] + ) + + assert result is not None + prompt = mock_call.call_args.kwargs["messages"][0]["content"] + assert "PREVIOUS SUMMARY:" in prompt + _assert_clean(prompt) + # After generation, _previous_summary holds the new (clean) LLM output — + # the leaked secret must not have survived anywhere in it. + assert SECRET not in c._previous_summary + assert "access_token=opaque-token-456" not in c._previous_summary + + +def test_resumed_handoff_summary_redacted_before_iterative_prompt(): + """Persisted handoff messages may contain pre-fix secrets after resume.""" + with patch( + "agent.context_compressor.get_model_context_length", + return_value=100000, + ): + c = ContextCompressor( + model="test/model", + threshold_percent=0.85, + protect_first_n=1, + protect_last_n=1, + quiet_mode=True, + ) + old_summary = f"RESUMED-SUMMARY leaked {SECRET} and {OAUTH_URL}" + messages = [ + {"role": "system", "content": "system prompt"}, + {"role": "user", "content": f"{SUMMARY_PREFIX}\n{old_summary}"}, + {"role": "assistant", "content": "handoff acknowledged after resume"}, + {"role": "user", "content": "new user turn after resume"}, + {"role": "assistant", "content": "new assistant work after resume"}, + {"role": "user", "content": "more new work after resume"}, + {"role": "assistant", "content": "latest tail response"}, + {"role": "user", "content": "final active request stays in tail"}, + ] + + with patch( + "agent.context_compressor.call_llm", + return_value=_response("updated summary"), + ) as mock_call: + c.compress(messages) + + prompt = mock_call.call_args.kwargs["messages"][0]["content"] + assert "PREVIOUS SUMMARY:" in prompt + _assert_clean(prompt)