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 instead of unconditionally tightening to 0600 (#33699)
`save_env_value()` captures the original .env file mode (e.g. 0640 for Docker volume mounts) and restores it via `os.chmod` — but then unconditionally calls `_secure_file(env_path)` on the next line, which re-tightens the mode to 0600 and defeats the entire preservation logic. The intent (preserve when `original_mode` is captured, secure otherwise) was already in the code but got short-circuited. Move `_secure_file()` into the `else` branch so it only runs when no original mode was captured — fresh `.env` files written for the first time still get the 0600 hardening treatment, but operator-set modes survive subsequent writes. Salvages #31518 by @blut-agent (config.py portion only). Their PR also bundled unrelated lowercase-lookup changes in `hermes_cli/commands.py`; this salvage takes only the focused config fix. The commands.py changes are reasonable on their own merits but belong in a separate PR. Co-authored-by: blut-agent <278569635+blut-agent@users.noreply.github.com>
This commit is contained in:
parent
ea7981eba7
commit
e4a1b35a39
2 changed files with 23 additions and 2 deletions
|
|
@ -5719,19 +5719,21 @@ def save_env_value(key: str, value: str):
|
|||
f.flush()
|
||||
os.fsync(f.fileno())
|
||||
atomic_replace(tmp_path, env_path)
|
||||
# Restore original permissions before _secure_file may tighten them.
|
||||
# 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:
|
||||
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[key] = value
|
||||
invalidate_env_cache()
|
||||
|
|
|
|||
|
|
@ -292,6 +292,25 @@ class TestSaveEnvValueSecure:
|
|||
env_mode = (tmp_path / ".env").stat().st_mode & 0o777
|
||||
assert env_mode == 0o600
|
||||
|
||||
def test_save_env_value_preserves_existing_file_mode_on_posix(self, tmp_path):
|
||||
"""Regression for #31518: pre-existing .env mode (e.g. 0640 for a
|
||||
Docker bind-mount that the operator chose) survives subsequent
|
||||
writes. Previously _secure_file ran unconditionally after the
|
||||
mode-restore branch and re-tightened to 0600.
|
||||
"""
|
||||
if os.name == "nt":
|
||||
return
|
||||
|
||||
env_path = tmp_path / ".env"
|
||||
env_path.write_text("EXISTING=value\n")
|
||||
os.chmod(env_path, 0o640)
|
||||
|
||||
with patch.dict(os.environ, {"HERMES_HOME": str(tmp_path)}):
|
||||
save_env_value("TENOR_API_KEY", "sk-test-secret")
|
||||
|
||||
env_mode = env_path.stat().st_mode & 0o777
|
||||
assert env_mode == 0o640, f"expected 0o640, got {oct(env_mode)}"
|
||||
|
||||
|
||||
class TestRemoveEnvValue:
|
||||
def test_removes_key_from_env_file(self, tmp_path):
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue