mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-06-11 08:42:11 +00:00
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.
This commit is contained in:
parent
183d86b3e0
commit
15813336cc
2 changed files with 27 additions and 1 deletions
|
|
@ -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()
|
||||
|
|
|
|||
|
|
@ -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)."""
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue