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)."""