From 526a0015fbe1fe8cc9870263909435ce3b59534b Mon Sep 17 00:00:00 2001 From: chaochen Date: Fri, 11 Sep 2026 00:19:21 +0800 Subject: [PATCH] fix(decode): tolerate non-UTF-8 child output in two remaining capture paths Same class as the cron script runner: text=True decodes with the locale codec under strict error handling, so one undecodable byte in a child's output raises UnicodeDecodeError inside subprocess.run. That is a ValueError - neither except (subprocess.SubprocessError, OSError) nor except TimeoutExpired catches it - so it escaped into user-visible paths instead of the intended error handling. tools/bot_mode_dm._run_local_turn: a transport that exits 0 but prints a byte the locale cannot decode crashed the delivery instead of re-emitting the transport's streams (stdout is the reply text the completion notification carries back). agent.command_token_source._mint: a key_cmd printing a non-UTF-8 byte killed token minting with a traceback instead of the documented CommandTokenError path, so provider auth failed in a way the caller could not report. Both decode with errors=replace now: the exit code and the surrounding checks still decide what happens, and damaged bytes appear as U+FFFD in the text we carry. --- agent/command_token_source.py | 2 +- tests/agent/test_command_token_source.py | 12 ++++++++++++ tests/tools/test_bot_mode_dm.py | 15 +++++++++++++++ tools/bot_mode_dm.py | 2 +- 4 files changed, 29 insertions(+), 2 deletions(-) diff --git a/agent/command_token_source.py b/agent/command_token_source.py index 38ace24324..08c6d04763 100644 --- a/agent/command_token_source.py +++ b/agent/command_token_source.py @@ -51,7 +51,7 @@ def _mint(command: str, label: str) -> tuple[str, Optional[float]]: try: completed = subprocess.run( - command, shell=True, capture_output=True, text=True, timeout=_MINT_TIMEOUT_SECONDS, + command, shell=True, capture_output=True, text=True, errors="replace", timeout=_MINT_TIMEOUT_SECONDS, env=served_profile_child_env(inherit_credentials=True), ) except subprocess.TimeoutExpired as exc: diff --git a/tests/agent/test_command_token_source.py b/tests/agent/test_command_token_source.py index 71251e9963..78bd86ec1f 100644 --- a/tests/agent/test_command_token_source.py +++ b/tests/agent/test_command_token_source.py @@ -77,6 +77,18 @@ class TestMinting: assert "dbx" in message # names the provider to fix assert "exited" in message # states what happened + def test_undecodable_output_does_not_raise(self): + """One non-UTF-8 byte from the helper must not crash token minting. + + A strict decode raised UnicodeDecodeError inside subprocess.run — a ValueError, + so neither the TimeoutExpired nor the OSError handler above caught it: provider + auth died with a traceback instead of the documented CommandTokenError path. + """ + token, ttl = _mint("printf 'tok\\377'", "dbx") + + assert ttl is None + assert token.startswith("tok") + class TestNoCredentialLeak: def test_failure_message_excludes_command_output(self): diff --git a/tests/tools/test_bot_mode_dm.py b/tests/tools/test_bot_mode_dm.py index d068c4f12e..2632cede2f 100644 --- a/tests/tools/test_bot_mode_dm.py +++ b/tests/tools/test_bot_mode_dm.py @@ -1102,3 +1102,18 @@ def test_poll_reply_is_persisted_as_a_delivery_row_when_the_runner_exits(tmp_pat assert kw["display_kind"] == "process_complete" assert "PAYLOAD_SENTINEL_42" in kw["content"] assert procs[0].id in kw["content"] + + +def test_local_turn_survives_undecodable_transport_output(tmp_path, capsys): + """A transport that exits 0 while printing a non-UTF-8 byte must still deliver. + + A strict decode raised UnicodeDecodeError inside subprocess.run — a ValueError, so no + handler caught it and the delivery crashed instead of re-emitting the transport's + streams (stdout is the reply text the completion notification carries back). + """ + dm_file = tmp_path / "dm.txt" + dm_file.write_text("hello", encoding="utf-8") + argv = [sys.executable, "-c", "import sys; sys.stdout.buffer.write(b'reply \\377')"] + + assert bot_mode_dm._run_local_turn(argv, str(dm_file)) == 0 + assert "reply" in capsys.readouterr().out diff --git a/tools/bot_mode_dm.py b/tools/bot_mode_dm.py index c6aebeb965..090061d3ba 100644 --- a/tools/bot_mode_dm.py +++ b/tools/bot_mode_dm.py @@ -416,7 +416,7 @@ def _run_local_turn(argv: list[str], dm_file: str, *, env: Optional[dict[str, st def _turn(turn_env=env): return subprocess.run([*argv, "--query-file", dm_file], check=False, stdin=subprocess.DEVNULL, - capture_output=True, text=True, env=turn_env) + capture_output=True, text=True, errors="replace", env=turn_env) proc = _turn() if proc.returncode != 0: