mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-01 12:02:05 +00:00
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
This commit is contained in:
parent
c23f394eb8
commit
6a2958a521
1 changed files with 22 additions and 8 deletions
|
|
@ -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")
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue