fix(security): redact terminal exception results and ACP stderr logs (#77484)
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.
This commit is contained in:
@@ -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",
|
||||
)
|
||||
|
||||
41
tests/acp_adapter/test_acp_logging_redaction.py
Normal file
41
tests/acp_adapter/test_acp_logging_redaction.py
Normal file
@@ -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)
|
||||
44
tests/tools/test_terminal_tool_exception_redaction.py
Normal file
44
tests/tools/test_terminal_tool_exception_redaction.py
Normal file
@@ -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"]
|
||||
@@ -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)
|
||||
|
||||
|
||||
Reference in New Issue
Block a user