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.
This commit is contained in:
@@ -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:
|
||||
|
||||
@@ -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):
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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:
|
||||
|
||||
Reference in New Issue
Block a user