fix(ssh): tolerate non-UTF-8 subprocess output (errors="replace", encoding="utf-8")

tests/tools/test_base_environment.py

@@ -4,9 +4,10 @@
 init_session() failure handling, and the CWD marker contract.
 """
 
+import sys
 from unittest.mock import MagicMock
 
-from tools.environments.base import BaseEnvironment, _BoundedOutputCollector
+from tools.environments.base import BaseEnvironment, _BoundedOutputCollector, _popen_bash
 
 
 class _TestableEnv(BaseEnvironment):
@@ -520,3 +521,21 @@ def test_unique_per_instance(self):
         env1 = _TestableEnv()
         env2 = _TestableEnv()
         assert env1._cwd_marker != env2._cwd_marker
+
+
+class TestPopenBashDecoding:
+    """Regression for issue #37130.
+
+    ``_popen_bash`` reads subprocess stdout in text mode. Without
+    ``errors="replace"`` a non-UTF8 byte (common from Windows OpenSSH /
+    Git Bash targets) raises ``UnicodeDecodeError`` the moment the output
+    is read, instead of being substituted with U+FFFD.
+    """
+
+    def test_non_utf8_output_is_replaced_not_raised(self):
+        proc = _popen_bash(
+            [sys.executable, "-c", "import os; os.write(1, b'\\xff\\xfe')"]
+        )
+        out = proc.stdout.read()
+        proc.wait()
+        assert "�" in out

tests/tools/test_ssh_environment.py

@@ -201,6 +201,121 @@ def _fake_establish(self):
         assert env.user == "alice"
 
 
+class TestNonUTF8SubprocessOutput:
+    """Regression tests for issue #37130.
+
+    Windows OpenSSH targets running Git Bash/MSYS shells can emit non-UTF8
+    bytes on stdout/stderr. The backend decoded subprocess output in strict
+    text mode (``text=True`` without ``errors=``), so a single undecodable
+    byte raised ``UnicodeDecodeError`` during ``_ensure_remote_dirs()`` —
+    before any user command ran — bricking all terminal/file/search tools
+    even though a manual SSH connection to the same host worked fine.
+    """
+
+    _BAD_BYTES = b"setup output \xff\xfe done\n"
+
+    @staticmethod
+    def _decoding_run(stdout=b"", stderr=b"", returncode=0):
+        """A ``subprocess.run`` stand-in that reproduces real text-mode decoding.
+
+        On the unfixed backend (no ``errors="replace"``) this raises
+        ``UnicodeDecodeError`` exactly as the real ``subprocess`` would; once
+        the backend passes ``errors="replace"`` it decodes with U+FFFD.
+        """
+        def _run(cmd, *a, **kwargs):
+            out, err = stdout, stderr
+            if kwargs.get("text") or kwargs.get("universal_newlines"):
+                enc = kwargs.get("encoding") or "utf-8"
+                errs = kwargs.get("errors", "strict")
+                out = stdout.decode(enc, errors=errs)
+                err = stderr.decode(enc, errors=errs)
+            return subprocess.CompletedProcess(cmd, returncode, out, err)
+        return _run
+
+    @pytest.fixture
+    def _ssh_env_factory(self, monkeypatch):
+        monkeypatch.setattr(ssh_env.shutil, "which", lambda _n: "/usr/bin/ssh")
+        monkeypatch.setattr("tools.environments.base.time.sleep", lambda _s: None)
+        from pathlib import Path as _Path
+        monkeypatch.setattr(_Path, "mkdir", lambda *a, **k: None)
+        monkeypatch.setattr(ssh_env, "FileSyncManager", lambda **kw: MagicMock())
+        monkeypatch.setattr(ssh_env.SSHEnvironment, "init_session",
+                            lambda self: None)
+
+        def _factory(stdout=b"", stderr=b"", returncode=0):
+            monkeypatch.setattr(
+                "tools.environments.ssh.subprocess.run",
+                self._decoding_run(stdout, stderr, returncode),
+            )
+            return ssh_env.SSHEnvironment(host="winhost", user="hermes")
+        return _factory
+
+    def test_init_survives_non_utf8_output(self, _ssh_env_factory):
+        """Full __init__ (connect + detect home + _ensure_remote_dirs) must
+        not raise on non-UTF8 subprocess output — the issue #37130 crash."""
+        env = _ssh_env_factory(stdout=self._BAD_BYTES)
+        assert env.host == "winhost"
+
+    def test_init_survives_non_utf8_on_stderr(self, _ssh_env_factory):
+        env = _ssh_env_factory(stderr=self._BAD_BYTES, returncode=0)
+        assert env.user == "hermes"
+
+    def test_detect_home_replaces_bad_bytes(self, _ssh_env_factory):
+        env = _ssh_env_factory(stdout=b"/home/hermes\xff\n")
+        assert env._remote_home == "/home/hermes�"
+
+    def test_ensure_remote_dirs_does_not_raise(self, _ssh_env_factory):
+        """Directly re-exercise the method named in the issue traceback."""
+        env = _ssh_env_factory(stdout=self._BAD_BYTES)
+        env._ensure_remote_dirs()
+
+    def test_every_text_run_call_uses_errors_replace(self, monkeypatch, tmp_path):
+        """All text-mode ``subprocess.run`` calls in the backend — including
+        the file-sync sites that __init__ does not reach — must pass
+        ``errors="replace"`` so non-UTF8 output can never crash them."""
+        calls = []
+
+        def _record(cmd, *a, **kwargs):
+            calls.append(kwargs)
+            return subprocess.CompletedProcess(cmd, 0, "", "")
+
+        def _fake_popen(*a, **k):
+            m = MagicMock()
+            m.returncode = 0
+            m.poll.return_value = 0
+            m.communicate.return_value = (b"", b"")
+            m.stderr.read.return_value = b""
+            return m
+
+        monkeypatch.setattr(ssh_env.shutil, "which", lambda _n: "/usr/bin/ssh")
+        monkeypatch.setattr("tools.environments.base.time.sleep", lambda _s: None)
+        from pathlib import Path as _Path
+        monkeypatch.setattr(_Path, "mkdir", lambda *a, **k: None)
+        monkeypatch.setattr(ssh_env, "FileSyncManager", lambda **kw: MagicMock())
+        monkeypatch.setattr(ssh_env.SSHEnvironment, "init_session",
+                            lambda self: None)
+        monkeypatch.setattr("tools.environments.ssh.subprocess.run", _record)
+        monkeypatch.setattr("tools.environments.ssh.subprocess.Popen", _fake_popen)
+
+        env = ssh_env.SSHEnvironment(host="winhost", user="hermes")
+
+        src = tmp_path / "payload.txt"
+        src.write_text("x")
+        env._ssh_delete(["~/.hermes/a"])
+        env._scp_upload(str(src), "~/.hermes/b")
+        env._ssh_bulk_upload([(str(src), f"{env._remote_home}/.hermes/sub/c")])
+
+        text_calls = [k for k in calls if k.get("text")]
+        assert text_calls, "expected text-mode subprocess.run calls"
+        for kwargs in text_calls:
+            assert kwargs.get("errors") == "replace", (
+                f"text-mode subprocess.run missing errors='replace': {kwargs}"
+            )
+            assert kwargs.get("encoding") == "utf-8", (
+                f"text-mode subprocess.run missing encoding='utf-8': {kwargs}"
+            )
+
+
 def _setup_ssh_env(monkeypatch, persistent: bool):
     monkeypatch.setenv("TERMINAL_ENV", "ssh")
     monkeypatch.setenv("TERMINAL_SSH_HOST", _SSH_HOST)

tools/environments/base.py

@@ -249,6 +249,8 @@ def _popen_bash(
         stderr=subprocess.STDOUT,
         stdin=subprocess.PIPE if stdin_data is not None else subprocess.DEVNULL,
         text=True,
+        encoding="utf-8",
+        errors="replace",
         **kwargs,
     )
     if stdin_data is not None:

tools/environments/ssh.py

@@ -105,6 +105,8 @@ def _establish_connection(self):
                 cmd,
                 capture_output=True,
                 text=True,
+                encoding="utf-8",
+                errors="replace",
                 timeout=15,
                 stdin=subprocess.DEVNULL,
             )
@@ -123,6 +125,8 @@ def _detect_remote_home(self) -> str:
                 cmd,
                 capture_output=True,
                 text=True,
+                encoding="utf-8",
+                errors="replace",
                 timeout=10,
                 stdin=subprocess.DEVNULL,
             )
@@ -150,6 +154,8 @@ def _ensure_remote_dirs(self) -> None:
             cmd,
             capture_output=True,
             text=True,
+            encoding="utf-8",
+            errors="replace",
             timeout=10,
             stdin=subprocess.DEVNULL,
         )
@@ -165,6 +171,8 @@ def _scp_upload(self, host_path: str, remote_path: str) -> None:
             mkdir_cmd,
             capture_output=True,
             text=True,
+            encoding="utf-8",
+            errors="replace",
             timeout=10,
             stdin=subprocess.DEVNULL,
         )
@@ -179,6 +187,8 @@ def _scp_upload(self, host_path: str, remote_path: str) -> None:
             scp_cmd,
             capture_output=True,
             text=True,
+            encoding="utf-8",
+            errors="replace",
             timeout=30,
             stdin=subprocess.DEVNULL,
         )
@@ -208,6 +218,8 @@ def _ssh_bulk_upload(self, files: list[tuple[str, str]]) -> None:
                 cmd,
                 capture_output=True,
                 text=True,
+                encoding="utf-8",
+                errors="replace",
                 timeout=30,
                 stdin=subprocess.DEVNULL,
             )
@@ -326,6 +338,8 @@ def _ssh_delete(self, remote_paths: list[str]) -> None:
             cmd,
             capture_output=True,
             text=True,
+            encoding="utf-8",
+            errors="replace",
             timeout=10,
             stdin=subprocess.DEVNULL,
         )