From 17485cbcd27440b1fe4563515cad861a89f072a2 Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Tue, 14 Jul 2026 17:46:00 -0700 Subject: [PATCH] fix(cli): sanitize terminal escapes when replaying stored history (/resume recap, /status recap) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- hermes_cli/cli_agent_setup_mixin.py | 8 ++++- hermes_cli/session_recap.py | 6 ++++ tests/cli/test_resume_display.py | 44 +++++++++++++++++++++++++ tests/hermes_cli/test_session_recap.py | 14 ++++++++ tests/tools/test_ansi_strip.py | 45 +++++++++++++++++++++++++- tools/ansi_strip.py | 35 ++++++++++++++++++++ 6 files changed, 150 insertions(+), 2 deletions(-) diff --git a/hermes_cli/cli_agent_setup_mixin.py b/hermes_cli/cli_agent_setup_mixin.py index b0a9e9e2fa1..a2664ddf695 100644 --- a/hermes_cli/cli_agent_setup_mixin.py +++ b/hermes_cli/cli_agent_setup_mixin.py @@ -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: diff --git a/hermes_cli/session_recap.py b/hermes_cli/session_recap.py index 111da117485..407254d2575 100644 --- a/hermes_cli/session_recap.py +++ b/hermes_cli/session_recap.py @@ -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 diff --git a/tests/cli/test_resume_display.py b/tests/cli/test_resume_display.py index 5ccac59ba6e..4a314860291 100644 --- a/tests/cli/test_resume_display.py +++ b/tests/cli/test_resume_display.py @@ -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 diff --git a/tests/hermes_cli/test_session_recap.py b/tests/hermes_cli/test_session_recap.py index 062988525f2..589e247af2b 100644 --- a/tests/hermes_cli/test_session_recap.py +++ b/tests/hermes_cli/test_session_recap.py @@ -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 diff --git a/tests/tools/test_ansi_strip.py b/tests/tools/test_ansi_strip.py index d1585c92bbb..a839a939b90 100644 --- a/tests/tools/test_ansi_strip.py +++ b/tests/tools/test_ansi_strip.py @@ -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" diff --git a/tools/ansi_strip.py b/tools/ansi_strip.py index b1cfb8ecea5..47ff14bfb61 100644 --- a/tools/ansi_strip.py +++ b/tools/ansi_strip.py @@ -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)