mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-31 19:16:29 +00:00
fix(cli): sanitize terminal escapes when replaying stored history (/resume recap, /status recap)
Port from openai/codex#31494: user-visible history replay must strip CSI sequences and control characters. Stored conversation history can carry raw terminal escapes (pasted content, gateway-origin text, model output echoing injected tool results). Replaying it via /resume's recap panel or build_recap (/status on CLI + gateway) wrote those bytes straight to the terminal — an injected message could clear the screen, retitle the window, move the cursor, or restyle the recap UI. Rich's Text() does not neutralize raw escape bytes. - tools/ansi_strip.py: add sanitize_display_text() — strip_ansi() plus bare C0/C1 control removal, preserving \n and \t, normalizing \r to \n (adapted to Python from Codex's sanitize_user_text; reuses the existing ECMA-48 stripper instead of transcribing their char-walk) - hermes_cli/cli_agent_setup_mixin.py: sanitize user + assistant text in _display_resumed_history() before building the Rich recap panel - hermes_cli/session_recap.py: sanitize preview lines in build_recap() (_truncate choke point) so /status recaps are clean on every platform - tests: 10 new sanitize_display_text cases (incl. the exact codex#31494 fixture), recap + resume-display leak assertions
This commit is contained in:
parent
b90dbac1d6
commit
17485cbcd2
6 changed files with 150 additions and 2 deletions
|
|
@ -542,6 +542,7 @@ class CLIAgentSetupMixin:
|
|||
an indicator for earlier hidden messages.
|
||||
"""
|
||||
from cli import CLI_CONFIG, _record_output_history_entry, _strip_reasoning_tags, _suspend_output_history
|
||||
from tools.ansi_strip import sanitize_display_text as _sanitize_display_text
|
||||
if not self.conversation_history:
|
||||
return
|
||||
|
||||
|
|
@ -582,13 +583,18 @@ class CLIAgentSetupMixin:
|
|||
elif isinstance(part, dict) and part.get("type") == "image_url":
|
||||
parts.append("[image]")
|
||||
text = " ".join(parts)
|
||||
# Stored history is untrusted for display: strip escape
|
||||
# sequences/control chars so replaying a message can't
|
||||
# clear the screen, retitle the window, or restyle the
|
||||
# recap panel (see tools/ansi_strip.sanitize_display_text).
|
||||
text = _sanitize_display_text(text)
|
||||
if len(text) > MAX_USER_LEN:
|
||||
text = text[:MAX_USER_LEN] + "..."
|
||||
entries.append(("user", text))
|
||||
|
||||
elif role == "assistant":
|
||||
text = "" if content is None else str(content)
|
||||
text = _strip_reasoning_tags(text)
|
||||
text = _sanitize_display_text(_strip_reasoning_tags(text))
|
||||
parts = []
|
||||
full_parts = [] # un-truncated version
|
||||
if text:
|
||||
|
|
|
|||
|
|
@ -23,6 +23,8 @@ import os
|
|||
from collections import Counter
|
||||
from typing import Any, Iterable, List, Mapping, Optional, Sequence, Tuple
|
||||
|
||||
from tools.ansi_strip import sanitize_display_text
|
||||
|
||||
# How many recent user/assistant turns we consider "recent activity".
|
||||
_RECENT_TURN_WINDOW = 20
|
||||
|
||||
|
|
@ -229,6 +231,10 @@ def _summarise_tool_activity(
|
|||
|
||||
|
||||
def _truncate(text: str, limit: int) -> str:
|
||||
# Stored history is untrusted for display — remove escape sequences and
|
||||
# control chars so a recap line can't clear the screen / retitle the
|
||||
# window when echoed to a terminal (openai/codex#31494 bug class).
|
||||
text = sanitize_display_text(text)
|
||||
text = " ".join(text.split()) # collapse newlines for a compact one-liner
|
||||
if len(text) <= limit:
|
||||
return text
|
||||
|
|
|
|||
|
|
@ -728,3 +728,47 @@ class TestResumeDisplayConfig:
|
|||
|
||||
display = config.get("display", {})
|
||||
assert display.get("resume_display") == "full"
|
||||
|
||||
|
||||
class TestResumeDisplaySanitization:
|
||||
"""Stored history replayed by /resume must not carry raw terminal
|
||||
escapes or control chars (openai/codex#31494 bug class)."""
|
||||
|
||||
def _capture_display(self, cli_obj):
|
||||
buf = StringIO()
|
||||
cli_obj.console.file = buf
|
||||
cli_obj._display_resumed_history()
|
||||
return buf.getvalue()
|
||||
|
||||
def test_escape_sequences_stripped_from_user_and_assistant(self):
|
||||
cli = _make_cli()
|
||||
cli.conversation_history = [
|
||||
{"role": "user", "content": "hi \x1b[2J\x1b]0;pwned\x07 there"},
|
||||
{"role": "assistant", "content": "ok \x9b31m fine\x07"},
|
||||
]
|
||||
output = self._capture_display(cli)
|
||||
# Rich adds its own SGR styling escapes when force_terminal is on;
|
||||
# what must NOT survive are the injected non-SGR sequences.
|
||||
assert "\x1b[2J" not in output
|
||||
assert "\x1b]0;pwned" not in output
|
||||
assert "\x9b" not in output
|
||||
assert "\x07" not in output
|
||||
assert "hi" in output and "there" in output
|
||||
assert "fine" in output
|
||||
|
||||
def test_multimodal_text_part_sanitized(self):
|
||||
cli = _make_cli()
|
||||
cli.conversation_history = [
|
||||
{
|
||||
"role": "user",
|
||||
"content": [
|
||||
{"type": "text", "text": "look \x1b[3J\x1b[H at this"},
|
||||
{"type": "image_url", "image_url": {"url": "https://x/y.png"}},
|
||||
],
|
||||
},
|
||||
{"role": "assistant", "content": "sure"},
|
||||
]
|
||||
output = self._capture_display(cli)
|
||||
assert "\x1b[3J" not in output
|
||||
assert "\x1b[H" not in output
|
||||
assert "[image]" in output
|
||||
|
|
|
|||
|
|
@ -177,3 +177,17 @@ def test_ignores_non_mapping_entries_gracefully():
|
|||
# Should not raise.
|
||||
out = build_recap(msgs)
|
||||
assert "Session recap" in out
|
||||
|
||||
|
||||
def test_escape_sequences_sanitized_in_previews():
|
||||
"""Recap previews must not carry raw terminal escapes (codex#31494 class)."""
|
||||
msgs = [
|
||||
_user("please \x1b[2J\x1b]0;pwned\x07 do the thing"),
|
||||
_assistant("done \x9b31m with it\x07"),
|
||||
]
|
||||
out = build_recap(msgs)
|
||||
assert "\x1b" not in out
|
||||
assert "\x9b" not in out
|
||||
assert "\x07" not in out
|
||||
assert "do the thing" in out
|
||||
assert "with it" in out
|
||||
|
|
|
|||
|
|
@ -5,7 +5,7 @@ ANSI codes leaking into the model's context via terminal/execute_code output.
|
|||
It must strip ALL terminal escape sequences while preserving legitimate text.
|
||||
"""
|
||||
|
||||
from tools.ansi_strip import strip_ansi
|
||||
from tools.ansi_strip import sanitize_display_text, strip_ansi
|
||||
|
||||
|
||||
class TestStripAnsiBasicSGR:
|
||||
|
|
@ -166,3 +166,46 @@ class TestStripAnsiPassthrough:
|
|||
"""Array indexing must not be confused with CSI."""
|
||||
code = "arr[0] = arr[31]"
|
||||
assert strip_ansi(code) == code
|
||||
|
||||
|
||||
class TestSanitizeDisplayText:
|
||||
"""sanitize_display_text — escape sequences AND bare control chars.
|
||||
|
||||
Port of the openai/codex#31494 bug class: stored/untrusted text
|
||||
replayed into a terminal UI (e.g. the /resume recap) must not be able
|
||||
to clear the screen, retitle the window, or corrupt adjacent output.
|
||||
"""
|
||||
|
||||
def test_csi_removed(self):
|
||||
assert sanitize_display_text("a\x1b[2Jb") == "ab"
|
||||
|
||||
def test_osc_title_removed(self):
|
||||
assert sanitize_display_text("x\x1b]0;pwned\x07y") == "xy"
|
||||
|
||||
def test_c1_csi_removed(self):
|
||||
assert sanitize_display_text("a\x9b31mb") == "ab"
|
||||
|
||||
def test_bare_controls_removed(self):
|
||||
assert sanitize_display_text("a\x00b\x08c\x07d\x7fe") == "abcde"
|
||||
|
||||
def test_newline_and_tab_preserved(self):
|
||||
assert sanitize_display_text("line1\nline2\tend") == "line1\nline2\tend"
|
||||
|
||||
def test_crlf_normalized_to_newline(self):
|
||||
assert sanitize_display_text("one\r\ntwo\rthree") == "one\ntwo\nthree"
|
||||
|
||||
def test_clean_text_fast_path_identity(self):
|
||||
s = "plain text with unicode 🎉 and [brackets]"
|
||||
assert sanitize_display_text(s) is s
|
||||
|
||||
def test_empty(self):
|
||||
assert sanitize_display_text("") == ""
|
||||
|
||||
def test_codex_31494_fixture(self):
|
||||
"""The exact input shape from openai/codex#31494's test."""
|
||||
raw = "_count_r\x1b[13;2:3uows\tindent\n\x00two\x7f"
|
||||
assert sanitize_display_text(raw) == "_count_rows\tindent\ntwo"
|
||||
|
||||
def test_mixed_escape_and_controls(self):
|
||||
raw = "hello \x1b[2J\x1b]0;pwned\x07 world \x9b31m red\x07"
|
||||
assert sanitize_display_text(raw) == "hello world red"
|
||||
|
|
|
|||
|
|
@ -31,6 +31,17 @@ _ANSI_ESCAPE_RE = re.compile(
|
|||
# Fast-path check — skip full regex when no escape-like bytes are present.
|
||||
_HAS_ESCAPE = re.compile(r"[\x1b\x80-\x9f]")
|
||||
|
||||
# C0 control characters (minus tab/newline/carriage-return, handled
|
||||
# separately) plus DEL. These survive strip_ansi() — it only removes
|
||||
# well-formed escape *sequences* — but are still dangerous or garbled
|
||||
# when echoed back to a terminal (BEL rings, backspace/DEL overwrite,
|
||||
# NUL truncates in some terminals).
|
||||
_CONTROL_CHARS_RE = re.compile(r"[\x00-\x08\x0b\x0c\x0e-\x1f\x7f]")
|
||||
|
||||
# Fast-path check for sanitize_display_text — any C0 control (except
|
||||
# tab/newline), CR, DEL, ESC, or C1 byte triggers the slow path.
|
||||
_HAS_CONTROL = re.compile(r"[\x00-\x08\x0b-\x1f\x7f-\x9f]")
|
||||
|
||||
|
||||
def strip_ansi(text: str) -> str:
|
||||
"""Remove ANSI escape sequences from text.
|
||||
|
|
@ -42,3 +53,27 @@ def strip_ansi(text: str) -> str:
|
|||
if not text or not _HAS_ESCAPE.search(text):
|
||||
return text
|
||||
return _ANSI_ESCAPE_RE.sub("", text)
|
||||
|
||||
|
||||
def sanitize_display_text(text: str) -> str:
|
||||
"""Sanitize stored/untrusted text before echoing it to a terminal.
|
||||
|
||||
Removes ANSI/ECMA-48 escape sequences AND bare control characters,
|
||||
preserving only newlines and tabs (carriage returns are normalized
|
||||
to newlines so ``\\r``-overwrite spoofing can't hide content).
|
||||
|
||||
Use this when re-rendering conversation history or other persisted
|
||||
text in a terminal UI (e.g. the ``/resume`` recap): a message that
|
||||
arrived with embedded escapes — pasted content, gateway-origin
|
||||
text, or model output echoing injected tool results — must not be
|
||||
able to clear the screen, retitle the window, move the cursor, or
|
||||
restyle adjacent UI when replayed. Rich's ``Text()`` does NOT
|
||||
neutralize raw escape bytes, so sanitization has to happen before
|
||||
display. Mirrors openai/codex#31494 (``sanitize_user_text``).
|
||||
"""
|
||||
if not text or not _HAS_CONTROL.search(text):
|
||||
return text
|
||||
text = strip_ansi(text)
|
||||
if "\r" in text:
|
||||
text = text.replace("\r\n", "\n").replace("\r", "\n")
|
||||
return _CONTROL_CHARS_RE.sub("", text)
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue