fix(env): concurrent .env writes cause token clobbering

hermes_cli/config.py

@@ -26,6 +26,7 @@
 import sys
 import tempfile
 import threading
+import portalocker
 import time
 from dataclasses import dataclass
 from pathlib import Path
@@ -3894,63 +3895,62 @@ def save_env_value(key: str, value: str):
     read_kw = {"encoding": "utf-8-sig", "errors": "replace"}
     write_kw = {"encoding": "utf-8"}
 
-    lines = []
-    if env_path.exists():
-        with open(env_path, **read_kw) as f:
-            lines = f.readlines()
-        # Normalize safe line formatting without interpreting values as syntax.
-        lines = _sanitize_env_lines(lines)
+    # Cross-process lock around the entire read-modify-write on ~/.hermes/.env
+    # so concurrent Hermes processes can't clobber each other's writes.
+    with portalocker.Lock(env_path, timeout=5, fail_when_locked=False):
+        lines = []
+        if env_path.exists():
+            with open(env_path, **read_kw) as f:
+                lines = f.readlines()
+            lines = _sanitize_env_lines(lines)
+
+        serialized_value = _quote_env_value(value)
+
+        # Find and update or append. Match both ``KEY=`` and the bash-compatible
+        # ``export KEY=`` form — load_env() parses export lines (#6659), so a
+        # user-added ``export GITHUB_TOKEN=...`` shows as set in every UI. If the
+        # writer didn't match it, a save would append a SECOND line and a later
+        # delete of that line would silently resurrect the old exported value
+        # (#40041: "token detected but cannot be replaced through the UI").
+        found = False
+        for i, line in enumerate(lines):
+            if _env_line_defines_key(line, key):
+                lines[i] = f"{key}={serialized_value}\n"
+                found = True
+                break
 
-    serialized_value = _quote_env_value(value)
-
-    # Find and update or append. Match both ``KEY=`` and the bash-compatible
-    # ``export KEY=`` form — load_env() parses export lines (#6659), so a
-    # user-added ``export GITHUB_TOKEN=...`` shows as set in every UI. If the
-    # writer didn't match it, a save would append a SECOND line and a later
-    # delete of that line would silently resurrect the old exported value
-    # (#40041: "token detected but cannot be replaced through the UI").
-    found = False
-    for i, line in enumerate(lines):
-        if _env_line_defines_key(line, key):
-            lines[i] = f"{key}={serialized_value}\n"
-            found = True
-            break
+        if not found:
+            # Ensure there's a newline at the end of the file before appending
+            if lines and not lines[-1].endswith("\n"):
+                lines[-1] += "\n"
+            lines.append(f"{key}={serialized_value}\n")
 
-    if not found:
-        # Ensure there's a newline at the end of the file before appending
-        if lines and not lines[-1].endswith("\n"):
-            lines[-1] += "\n"
-        lines.append(f"{key}={serialized_value}\n")
-
-    fd, tmp_path = tempfile.mkstemp(dir=str(env_path.parent), suffix='.tmp', prefix='.env_')
-    # Preserve original permissions so Docker volume mounts aren't clobbered.
-    original_mode = None
-    if env_path.exists():
-        try:
-            original_mode = stat.S_IMODE(env_path.stat().st_mode)
-        except OSError:
-            pass
-    try:
-        with os.fdopen(fd, 'w', **write_kw) as f:
-            f.writelines(lines)
-            f.flush()
-            os.fsync(f.fileno())
-        atomic_replace(tmp_path, env_path)
-        # Preserve the original file mode (e.g. 0640 for Docker volume mounts)
-        # instead of letting _secure_file unconditionally tighten to 0600.
-        if original_mode is not None:
+        fd, tmp_path = tempfile.mkstemp(dir=str(env_path.parent), suffix='.tmp', prefix='.env_')
+        original_mode = None
+        if env_path.exists():
             try:
-                os.chmod(env_path, original_mode)
+                original_mode = stat.S_IMODE(env_path.stat().st_mode)
             except OSError:
                 pass
-        else:
-            _secure_file(env_path)
-    except BaseException:
         try:
-            os.unlink(tmp_path)
-        except OSError:
-            pass
-        raise
+            with os.fdopen(fd, 'w', **write_kw) as f:
+                f.writelines(lines)
+                f.flush()
+                os.fsync(f.fileno())
+            atomic_replace(tmp_path, env_path)
+            if original_mode is not None:
+                try:
+                    os.chmod(env_path, original_mode)
+                except OSError:
+                    pass
+            else:
+                _secure_file(env_path)
+        except BaseException:
+            try:
+                os.unlink(tmp_path)
+            except OSError:
+                pass
+            raise
 
     os.environ[key] = value
     invalidate_env_cache()
@@ -4005,9 +4005,10 @@ def remove_env_value(key: str) -> bool:
     read_kw = {"encoding": "utf-8-sig", "errors": "replace"}
     write_kw = {"encoding": "utf-8"}
 
-    with open(env_path, **read_kw) as f:
-        lines = f.readlines()
-    lines = _sanitize_env_lines(lines)
+    with portalocker.Lock(env_path, timeout=5, fail_when_locked=False):
+        with open(env_path, **read_kw) as f:
+            lines = f.readlines()
+        lines = _sanitize_env_lines(lines)
 
     new_lines = [line for line in lines if not _env_line_defines_key(line, key)]
     found = len(new_lines) < len(lines)