mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-28 18:19:28 +00:00
fix(compression): apply strict redaction at every compaction text boundary (#69294)
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 <topazd2@gmail.com>
This commit is contained in:
parent
2ee50c69d3
commit
0acdf1d8c8
3 changed files with 262 additions and 8 deletions
|
|
@ -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 (<think>, <reasoning>, 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 <focus>` 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)
|
||||
|
|
|
|||
1
contributors/emails/topazd2@gmail.com
Normal file
1
contributors/emails/topazd2@gmail.com
Normal file
|
|
@ -0,0 +1 @@
|
|||
AndrewMoryakov
|
||||
223
tests/agent/test_compaction_redaction_boundaries.py
Normal file
223
tests/agent/test_compaction_redaction_boundaries.py
Normal file
|
|
@ -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 <focus>`` 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)
|
||||
Loading…
Add table
Add a link
Reference in a new issue