fix(windows): bash-safe snapshot paths after #63113

#63113 rewrote native drive paths in ShellFileOperations, but init_session
/_wrap_command still embedded C:/... hermes-snap paths from get_temp_dir.
MSYS arg-converts those during bash -l and surfaces Directory \drivers\etc
— including for relative write_file targets, since the wrapper is the fault.

Add _bash_safe_path, override BaseEnvironment._quote_shell_path on
LocalEnvironment (no base→local import), and normalize mixed /c/Users\...
paths in file ops.

Co-authored-by: xxxigm <tuancanhnguyen706@gmail.com>
This commit is contained in:
Brooklyn Nicholson 2026-07-13 01:59:58 -04:00
parent acb3bde097
commit f2fcf89c1f
5 changed files with 181 additions and 28 deletions

View file

@ -367,15 +367,14 @@ class BaseEnvironment(ABC):
# Without this the snapshot bootstrap ``cd`` below fails on Windows and
# ``pwd -P`` captures the login shell's directory, not ``terminal.cwd``.
_quoted_cwd = self._quote_cwd_for_cd(self.cwd)
# Quote the snapshot / cwd-file paths so Git Bash on Windows handles
# ``C:/Users/...``-shaped paths without glob-splitting the colon or
# tripping on drive letters. On POSIX this is a no-op (no colons /
# special chars in a /tmp path). Previously unquoted interpolation
# caused ``C:/Users/.../hermes-snap-*.sh: No such file or directory``
# errors on Windows, leaking via stderr (merged into stdout on Linux
# backends) into every terminal-tool response.
_quoted_snap = shlex.quote(self._snapshot_path)
_quoted_cwd_file = shlex.quote(self._cwd_file)
# Quote snapshot / cwd-file paths via ``_quote_shell_path`` so the
# LocalEnvironment override can rewrite ``C:/...`` (and mixed
# ``/c/Users\\...``) to ``/c/...`` before quoting — bare drive paths
# in the bootstrap script trip MSYS into the
# ``Directory \\drivers\\etc does not exist`` failure class.
# On POSIX this is plain ``shlex.quote``.
_quoted_snap = self._quote_shell_path(self._snapshot_path)
_quoted_cwd_file = self._quote_shell_path(self._cwd_file)
# Use atomic file replacement: assemble the snapshot in a temp file,
# then mv it over the final path. This prevents concurrent source()
# calls from reading a half-written snapshot when another terminal
@ -391,9 +390,9 @@ class BaseEnvironment(ABC):
# mid-write, and mv would then publish a torn file (the corruption is
# only narrowed, not closed). ``$BASHPID`` is the actual subshell PID
# and is genuinely unique per writer, which closes the race. The
# static path is shlex-quoted (Windows/Git-Bash drive letters, spaces)
# static path is shell-quoted (Windows/Git-Bash drive letters, spaces)
# with ``$BASHPID`` left outside the quotes so it still expands.
_snap_tmp = shlex.quote(self._snapshot_path + ".tmp.") + "$BASHPID"
_snap_tmp = self._quote_shell_path(self._snapshot_path + ".tmp.") + "$BASHPID"
bootstrap = (
f"umask 077\n"
f"export -p > {_snap_tmp}\n"
@ -461,25 +460,32 @@ class BaseEnvironment(ABC):
return f"$HOME/{shlex.quote(cwd[2:])}"
return shlex.quote(cwd)
def _quote_shell_path(self, path: str) -> str:
"""Quote *path* for interpolation into a bash script.
LocalEnvironment overrides this to rewrite native/mixed Windows
paths to ``/c/...`` before quoting. Remote backends leave paths
as-is (they already speak POSIX).
"""
return shlex.quote(path)
def _wrap_command(self, command: str, cwd: str) -> str:
"""Build the full bash script that sources snapshot, cd's, runs command,
re-dumps env vars, and emits CWD markers."""
escaped = command.replace("'", "'\\''")
# Quote the snapshot / cwd-file paths so Git Bash on Windows handles
# ``C:/Users/...``-shaped paths without glob-splitting the colon or
# tripping on drive letters. POSIX paths are unaffected. See
# :meth:`init_session` for the same fix on the bootstrap block.
_quoted_snap = shlex.quote(self._snapshot_path)
_quoted_cwd_file = shlex.quote(self._cwd_file)
# Quote snapshot/cwd-file paths (see init_session — LocalEnvironment
# rewrites ``C:/...`` to ``/c/...`` so MSYS doesn't mangle them).
_quoted_snap = self._quote_shell_path(self._snapshot_path)
_quoted_cwd_file = self._quote_shell_path(self._cwd_file)
# Use atomic file replacement for env snapshot updates (issue #38249).
# Assemble into a per-writer-unique temp file, then mv to atomically
# replace the snapshot so concurrent source() calls never read a
# truncated/half-written file. ``$BASHPID`` (not ``$$``) is the actual
# subshell PID — unique per concurrent ``&``-launched writer — so two
# writers never share a temp name and clobber each other before the mv.
# Static path shlex-quoted (Windows/spaces); ``$BASHPID`` left to expand.
_snap_tmp = shlex.quote(self._snapshot_path + ".tmp.") + "$BASHPID"
# Static path shell-quoted (Windows/spaces); ``$BASHPID`` left to expand.
_snap_tmp = self._quote_shell_path(self._snapshot_path + ".tmp.") + "$BASHPID"
parts = []

View file

@ -66,6 +66,35 @@ def _windows_to_msys_path(cwd: str) -> str:
return f"/{drive}/{tail}" if tail else f"/{drive}/"
def _bash_safe_path(path: str) -> str:
"""Return *path* in a form safe to embed in a Git Bash script.
Native ``C:\\Users\\x`` / ``C:/Users/x`` ``/c/Users/x`` via
:func:`_windows_to_msys_path`. Mixed MSYS leftovers
(``/c/Users\\Alexander\\Documents``) get backslashes normalized so
bash does not eat ``\\U`` and trip the ``Directory \\drivers\\etc``
failure class. No-op off Windows and for empty input.
``get_temp_dir`` already emits forward-slash ``C:/...`` forms for
Python compatibility; those still need the ``/c/...`` rewrite
MSYS argument conversion treats ``C:/...`` as a Windows path and
can corrupt the login-shell ``drivers\\etc`` lookup.
"""
if not _IS_WINDOWS or not path:
return path
path = _windows_to_msys_path(path)
if "\\" in path:
path = path.replace("\\", "/")
return path
def _quote_bash_path(path: str) -> str:
"""Quote *path* for safe interpolation into a Git Bash script on Windows."""
import shlex
return shlex.quote(_bash_safe_path(path))
def _resolve_safe_cwd(cwd: str) -> str:
"""Return ``cwd`` if it exists as a directory, else the nearest existing
ancestor. Falls back to ``tempfile.gettempdir()`` only if walking up the
@ -994,6 +1023,10 @@ class LocalEnvironment(BaseEnvironment):
"""Use native paths for Python, but Git Bash-friendly paths for cd."""
return BaseEnvironment._quote_cwd_for_cd(_windows_to_msys_path(cwd))
def _quote_shell_path(self, path: str) -> str:
"""Rewrite native/mixed Windows paths before quoting for Git Bash."""
return _quote_bash_path(path)
def _run_bash(self, cmd_string: str, *, login: bool = False,
timeout: int = 120,
stdin_data: str | None = None) -> subprocess.Popen:

View file

@ -960,16 +960,17 @@ class ShellFileOperations(FileOperations):
def _escape_shell_arg(self, arg: str) -> str:
"""Escape a string for safe use in shell commands.
On Windows a native drive path (``C:\\Users\\x``) is first rewritten to
the Git Bash / MSYS ``/c/Users/x`` form: bash eats the backslashes and
MSYS otherwise mangles ``C:\\...`` (the "Directory \\drivers\\etc does not
exist" class of failures). Reuses the env-layer translator so shell file
ops and the terminal ``cd`` agree on the path form. No-op off Windows and
for non-drive-qualified paths.
On Windows native drive paths (``C:\\Users\\x`` / ``C:/Users/x``)
and mixed MSYS leftovers (``/c/Users\\x``) are rewritten to the
Git Bash ``/c/Users/x`` form via ``_bash_safe_path``: bash eats
backslashes and MSYS otherwise mangles drive paths into the
``Directory \\drivers\\etc does not exist`` failure class. Reuses
the env-layer translator so shell file ops and the terminal ``cd``
agree on the path form. No-op off Windows and for plain POSIX paths.
"""
from tools.environments.local import _windows_to_msys_path
from tools.environments.local import _bash_safe_path
arg = _windows_to_msys_path(arg)
arg = _bash_safe_path(arg)
# Use single quotes and escape any single quotes in the string
return "'" + arg.replace("'", "'\"'\"'") + "'"