From 72eda946be949a5932923df3325037a0d6c5da49 Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Sat, 8 Aug 2026 04:03:04 -0700 Subject: [PATCH] fix(security): redact terminal exception results and ACP stderr logs (#77484) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes the last two emission gaps from #77484: - tools/terminal_tool.py: both exception paths (generic except and TERMINAL_DEGRADED_MODE=fail) returned raw str(e) + traceback.format_exc() to the model — only the logger copy was redacted. Exception text can embed the failing command line and any secrets inline in it; both fields now pass through redact_sensitive_text. - acp_adapter/entry.py: _setup_logging cleared root handlers and installed a plain logging.Formatter, bypassing redaction entirely on ACP stderr. Now uses RedactingFormatter like every other logging surface. The other three gaps from #77484 (process(list), *_KEY regex variants, control-char splits) were fixed in #80964/#80965. --- acp_adapter/entry.py | 4 +- .../acp_adapter/test_acp_logging_redaction.py | 41 +++++++++++++++++ .../test_terminal_tool_exception_redaction.py | 44 +++++++++++++++++++ tools/terminal_tool.py | 14 ++++-- 4 files changed, 98 insertions(+), 5 deletions(-) create mode 100644 tests/acp_adapter/test_acp_logging_redaction.py create mode 100644 tests/tools/test_terminal_tool_exception_redaction.py diff --git a/acp_adapter/entry.py b/acp_adapter/entry.py index 7b549006bc..40ad336abc 100644 --- a/acp_adapter/entry.py +++ b/acp_adapter/entry.py @@ -80,9 +80,11 @@ class _BenignProbeMethodFilter(logging.Filter): def _setup_logging() -> None: """Route all logging to stderr so stdout stays clean for ACP stdio.""" + from agent.redact import RedactingFormatter + handler = logging.StreamHandler(sys.stderr) handler.setFormatter( - logging.Formatter( + RedactingFormatter( "%(asctime)s [%(levelname)s] %(name)s: %(message)s", datefmt="%Y-%m-%d %H:%M:%S", ) diff --git a/tests/acp_adapter/test_acp_logging_redaction.py b/tests/acp_adapter/test_acp_logging_redaction.py new file mode 100644 index 0000000000..1a99cd6ac4 --- /dev/null +++ b/tests/acp_adapter/test_acp_logging_redaction.py @@ -0,0 +1,41 @@ +"""ACP adapter stderr logging must go through RedactingFormatter. + +``_setup_logging`` clears root handlers and installs its own stderr handler; +before the fix it used a plain ``logging.Formatter`` — zero redaction on a +surface that logs request/response internals. See issue #77484. +""" + +import logging + +from acp_adapter.entry import _setup_logging + +SECRET = "sk-proj-AbCdEf1234567890SecretValue999" + + +def test_acp_stderr_handler_redacts_secrets(): + saved_handlers = logging.getLogger().handlers[:] + saved_level = logging.getLogger().level + try: + _setup_logging() + root = logging.getLogger() + assert root.handlers, "ACP logging setup installed no handler" + handler = root.handlers[0] + assert isinstance(handler, logging.StreamHandler) + record = logging.LogRecord( + name="acp.test", + level=logging.ERROR, + pathname=__file__, + lineno=1, + msg="request failed: OPENROUTER_API_KEY=%s", + args=(SECRET,), + exc_info=None, + ) + out = handler.format(record) + assert SECRET not in out + assert "OPENROUTER_API_KEY=" in out + finally: + root = logging.getLogger() + root.handlers.clear() + for h in saved_handlers: + root.addHandler(h) + root.setLevel(saved_level) diff --git a/tests/tools/test_terminal_tool_exception_redaction.py b/tests/tools/test_terminal_tool_exception_redaction.py new file mode 100644 index 0000000000..5862adb882 --- /dev/null +++ b/tests/tools/test_terminal_tool_exception_redaction.py @@ -0,0 +1,44 @@ +"""Terminal-tool exception paths must redact secrets before returning to the model. + +The logger copy of the traceback already goes through RedactingFormatter, but +the JSON result returned to the model previously carried raw ``str(e)`` and +``traceback.format_exc()`` — exception text can embed the failing command line +(and any secrets inline in it). See issue #77484. +""" + +import json + +import tools.terminal_tool as terminal_tool + +SECRET = "sk-proj-AbCdEf1234567890SecretValue999" + + +def _force_exception(monkeypatch, exc): + def boom(): + raise exc + monkeypatch.setattr(terminal_tool, "_get_env_config", boom) + + +def test_generic_exception_result_redacts_error_and_traceback(monkeypatch): + _force_exception(monkeypatch, RuntimeError(f"connect failed OPENAI_API_KEY={SECRET}")) + result = json.loads(terminal_tool.terminal_tool("echo hi")) + assert result["status"] == "error" + assert SECRET not in result["error"] + assert SECRET not in result["traceback"] + # The redaction must mask the value, not drop the message entirely. + assert "OPENAI_API_KEY=" in result["error"] + assert "Failed to execute command" in result["error"] + + +def test_degraded_fail_mode_result_redacts_error_and_traceback(monkeypatch): + from tools.environments.base import EnvironmentConnectionError + + monkeypatch.setenv("TERMINAL_DEGRADED_MODE", "fail") + exc = EnvironmentConnectionError( + f"ssh auth failed TOKEN={SECRET}", retry_hint="retry later" + ) + _force_exception(monkeypatch, exc) + result = json.loads(terminal_tool.terminal_tool("echo hi")) + assert result["status"] == "error" + assert SECRET not in result["error"] + assert SECRET not in result["traceback"] diff --git a/tools/terminal_tool.py b/tools/terminal_tool.py index fc5315db55..00f50bba99 100644 --- a/tools/terminal_tool.py +++ b/tools/terminal_tool.py @@ -3305,13 +3305,16 @@ def terminal_tool( degraded_mode = os.getenv("TERMINAL_DEGRADED_MODE", "warn").strip().lower() if degraded_mode == "fail": import traceback + from agent.redact import redact_sensitive_text tb_str = traceback.format_exc() logger.error("terminal_tool exception:\n%s", tb_str) + # Exception text can embed the failing command line (and any + # secrets inline in it) — redact before returning to the model. return json.dumps({ "output": "", "exit_code": -1, - "error": f"Failed to execute command: {str(e)}", - "traceback": tb_str, + "error": redact_sensitive_text(f"Failed to execute command: {str(e)}"), + "traceback": redact_sensitive_text(tb_str), "status": "error" }, ensure_ascii=False) @@ -3334,13 +3337,16 @@ def terminal_tool( except Exception as e: import traceback + from agent.redact import redact_sensitive_text tb_str = traceback.format_exc() logger.error("terminal_tool exception:\n%s", tb_str) + # Exception text can embed the failing command line (and any + # secrets inline in it) — redact before returning to the model. return json.dumps({ "output": "", "exit_code": -1, - "error": f"Failed to execute command: {str(e)}", - "traceback": tb_str, + "error": redact_sensitive_text(f"Failed to execute command: {str(e)}"), + "traceback": redact_sensitive_text(tb_str), "status": "error" }, ensure_ascii=False)