Compare commits

...

1 Commits

Author SHA1 Message Date
Ben Barclay
53f21eb0ae fix(config): preserve original .env file mode instead of unconditionally tightening to 0600
`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>
2026-05-28 16:29:57 +10:00
2 changed files with 23 additions and 2 deletions

View File

@@ -5106,19 +5106,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()

View File

@@ -227,6 +227,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):