From 6a2958a5216b9d58e15ff6399256f2e99235e3c4 Mon Sep 17 00:00:00 2001 From: kyssta-exe Date: Wed, 3 Jun 2026 15:01:07 +0000 Subject: [PATCH] fix(environments): use atomic file replacement for snapshot writes Fix race condition in terminal environment snapshots that could corrupt PATH with declare -x entries. When concurrent terminal calls share the same snapshot file, the non-atomic 'export -p > snapshot.sh' write could be read mid-write by another process, causing partial/corrupted env vars to be sourced and mixed into PATH. The fix uses atomic file replacement: - Write to a temp file: export -p > snapshot.sh.tmp.303651 - Atomically replace: mv -f snapshot.sh.tmp.303651 snapshot.sh On POSIX, mv within the same filesystem is atomic, so source() will either see the old complete snapshot or the new complete one, never a partial/truncated file. Fixes #38249 --- tools/environments/base.py | 30 ++++++++++++++++++++++-------- 1 file changed, 22 insertions(+), 8 deletions(-) diff --git a/tools/environments/base.py b/tools/environments/base.py index dd2dd8bcdaa..d782e5691d2 100644 --- a/tools/environments/base.py +++ b/tools/environments/base.py @@ -371,13 +371,22 @@ class BaseEnvironment(ABC): # backends) into every terminal-tool response. _quoted_snap = shlex.quote(self._snapshot_path) _quoted_cwd_file = shlex.quote(self._cwd_file) + # Use atomic file replacement: write to a temp file, then mv to the + # final path. This prevents concurrent source() calls from reading a + # half-written snapshot when another terminal command finishes and + # rewrites the env vars (issue #38249). `mv` is atomic on POSIX + # when src and dest are on the same filesystem, so source() will + # either see the old complete snapshot or the new complete one — + # never a partial/truncated file. + _snap_tmp = f"{self._snapshot_path}.tmp.$$" bootstrap = ( - f"export -p > {_quoted_snap}\n" - f"declare -f | grep -vE '^_[^_]' >> {_quoted_snap}\n" - f"alias -p >> {_quoted_snap}\n" - f"echo 'shopt -s expand_aliases' >> {_quoted_snap}\n" - f"echo 'set +e' >> {_quoted_snap}\n" - f"echo 'set +u' >> {_quoted_snap}\n" + f"export -p > {_snap_tmp}\n" + f"declare -f | grep -vE '^_[^_]' >> {_snap_tmp}\n" + f"alias -p >> {_snap_tmp}\n" + f"echo 'shopt -s expand_aliases' >> {_snap_tmp}\n" + f"echo 'set +e' >> {_snap_tmp}\n" + f"echo 'set +u' >> {_snap_tmp}\n" + f"mv -f {_snap_tmp} {_quoted_snap}\n" f"builtin cd {_quoted_cwd} 2>/dev/null || true\n" f"pwd -P > {_quoted_cwd_file} 2>/dev/null || true\n" f"printf '\\n{self._cwd_marker}%s{self._cwd_marker}\\n' \"$(pwd -P)\"\n" @@ -427,6 +436,10 @@ class BaseEnvironment(ABC): # :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) + # Use atomic file replacement for env snapshot updates (issue #38249). + # Write to a temp file, then mv to atomically replace the snapshot so + # concurrent source() calls never read a truncated/half-written file. + _snap_tmp = f"{self._snapshot_path}.tmp.$$" parts = [] @@ -451,9 +464,10 @@ class BaseEnvironment(ABC): parts.append(f"eval '{escaped}'") parts.append("__hermes_ec=$?") - # Re-dump env vars to snapshot (last-writer-wins for concurrent calls) + # Re-dump env vars to snapshot (atomic replacement to avoid races) if self._snapshot_ready: - parts.append(f"export -p > {_quoted_snap} 2>/dev/null || true") + parts.append(f"export -p > {_snap_tmp} 2>/dev/null || true") + parts.append(f"mv -f {_snap_tmp} {_quoted_snap} 2>/dev/null || true") # Write CWD to file (local reads this) and stdout marker (remote parses this) parts.append(f"pwd -P > {_quoted_cwd_file} 2>/dev/null || true")