From 15813336cce0749dbf9ee88da6569f1a85be3e4c Mon Sep 17 00:00:00 2001
From: Ben Barclay <ben@nousresearch.com>
Date: Wed, 10 Jun 2026 19:53:07 +1000
Subject: [PATCH] fix(config): preserve original .env file mode in
 remove_env_value too (#43349)

#33699 fixed save_env_value so an operator-set .env mode (e.g. 0640 on a
Docker bind-mount) survives a config write instead of being re-tightened
to 0600 by the unconditional _secure_file() call. The sibling
remove_env_value() had the identical bug: it restores original_mode and
then unconditionally called _secure_file(env_path), clobbering the mode
back to 0600 on every `hermes config remove KEY`.

Apply the same fix: move _secure_file() into the else branch so it only
runs when no original mode was captured (a freshly created .env still
gets 0600 hardening; existing operator-set modes survive).

Added test_remove_env_value_preserves_existing_file_mode_on_posix, which
fails on the unfixed remove path (expected 0o640, got 0o600) and passes
with the fix.
---
 hermes_cli/config.py            |  6 +++++-
 tests/hermes_cli/test_config.py | 22 ++++++++++++++++++++++
 2 files changed, 27 insertions(+), 1 deletion(-)

diff --git a/hermes_cli/config.py b/hermes_cli/config.py
index 2cbb00468b2..933ecadaf7e 100644
--- a/hermes_cli/config.py
+++ b/hermes_cli/config.py
@@ -5808,18 +5808,22 @@ def remove_env_value(key: str) -> bool:
                 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. Mirrors save_env_value().
             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
-        _secure_file(env_path)
 
     os.environ.pop(key, None)
     invalidate_env_cache()
diff --git a/tests/hermes_cli/test_config.py b/tests/hermes_cli/test_config.py
index 5d8cb4436ca..3e3144fdfea 100644
--- a/tests/hermes_cli/test_config.py
+++ b/tests/hermes_cli/test_config.py
@@ -354,6 +354,28 @@ class TestRemoveEnvValue:
             remove_env_value("ORPHAN_KEY")
             assert "ORPHAN_KEY" not in os.environ
 
+    def test_remove_env_value_preserves_existing_file_mode_on_posix(self, tmp_path):
+        """Regression: pre-existing .env mode (e.g. 0640 for a Docker
+        bind-mount the operator chose) survives a remove just as it does a
+        save. Previously _secure_file ran unconditionally after the
+        mode-restore branch and re-tightened to 0600 — the same bug fixed
+        in save_env_value (#33699), in the sibling remove path.
+        """
+        if os.name == "nt":
+            return
+
+        env_path = tmp_path / ".env"
+        env_path.write_text("KEEP=value\nDROP=gone\n")
+        os.chmod(env_path, 0o640)
+
+        with patch.dict(os.environ, {"HERMES_HOME": str(tmp_path), "DROP": "gone"}):
+            removed = remove_env_value("DROP")
+
+        assert removed is True
+        assert "DROP" not in env_path.read_text()
+        env_mode = env_path.stat().st_mode & 0o777
+        assert env_mode == 0o640, f"expected 0o640, got {oct(env_mode)}"
+
 
 class TestSaveConfigAtomicity:
     """Verify save_config uses atomic writes (tempfile + os.replace)."""