fix(utils): guard os.fchmod for Windows in atomic_json_write
os.fchmod is Unix-only; the Windows os module has no fchmod (only
chmod). Passing mode= (e.g. 0o600 when saving the Hindsight config
during `hermes memory setup`) crashed on Windows with:
AttributeError: module 'os' has no attribute 'fchmod'
Guard the fchmod fast-path with hasattr(os, "fchmod"). Skipping it on
Windows is safe: mkstemp already creates the temp file as 0o600, and
the existing post-replace os.chmod(real_path, mode) — already wrapped
in try/except — applies the final mode durably (as far as Windows
honors it).
Adds regression tests: one simulating a Windows os module without
fchmod (must not raise), and one asserting the durable 0o600 mode on
POSIX.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@ -1,6 +1,7 @@
|
|||||||
"""Tests for utils.atomic_json_write — crash-safe JSON file writes."""
|
"""Tests for utils.atomic_json_write — crash-safe JSON file writes."""
|
||||||
|
|
||||||
import json
|
import json
|
||||||
|
import os
|
||||||
from pathlib import Path
|
from pathlib import Path
|
||||||
from unittest.mock import patch
|
from unittest.mock import patch
|
||||||
|
|
||||||
@ -132,6 +133,38 @@ class TestAtomicJsonWrite:
|
|||||||
assert result["emoji"] == "🎉"
|
assert result["emoji"] == "🎉"
|
||||||
assert result["japanese"] == "日本語"
|
assert result["japanese"] == "日本語"
|
||||||
|
|
||||||
|
def test_mode_does_not_crash_without_fchmod(self, tmp_path):
|
||||||
|
"""Regression: os.fchmod is Unix-only and absent on Windows. Passing a
|
||||||
|
mode must not raise AttributeError when fchmod is unavailable.
|
||||||
|
|
||||||
|
Simulates the Windows os module by removing fchmod from the namespace.
|
||||||
|
Previously this crashed in `hermes memory setup` while saving the
|
||||||
|
Hindsight config with mode=0o600 (GitHub: Windows setup traceback).
|
||||||
|
"""
|
||||||
|
import utils
|
||||||
|
|
||||||
|
target = tmp_path / "secret.json"
|
||||||
|
no_fchmod = {k: getattr(os, k) for k in dir(os) if k != "fchmod"}
|
||||||
|
fake_os = type("FakeOs", (), no_fchmod)
|
||||||
|
assert not hasattr(fake_os, "fchmod")
|
||||||
|
|
||||||
|
with patch.object(utils, "os", fake_os):
|
||||||
|
atomic_json_write(target, {"api_key": "secret"}, mode=0o600)
|
||||||
|
|
||||||
|
assert json.loads(target.read_text(encoding="utf-8")) == {"api_key": "secret"}
|
||||||
|
|
||||||
|
def test_mode_applied_when_supported(self, tmp_path):
|
||||||
|
import stat as stat_mod
|
||||||
|
|
||||||
|
target = tmp_path / "secret.json"
|
||||||
|
atomic_json_write(target, {"api_key": "secret"}, mode=0o600)
|
||||||
|
|
||||||
|
# os.chmod's effect is platform-dependent (Windows only honors the
|
||||||
|
# write bit), so only assert the durable mode on POSIX.
|
||||||
|
if hasattr(os, "fchmod"):
|
||||||
|
actual = stat_mod.S_IMODE(target.stat().st_mode)
|
||||||
|
assert actual == 0o600
|
||||||
|
|
||||||
def test_concurrent_writes_dont_corrupt(self, tmp_path):
|
def test_concurrent_writes_dont_corrupt(self, tmp_path):
|
||||||
"""Multiple rapid writes should each produce valid JSON."""
|
"""Multiple rapid writes should each produce valid JSON."""
|
||||||
import threading
|
import threading
|
||||||
|
|||||||
5
utils.py
5
utils.py
@ -117,7 +117,10 @@ def atomic_json_write(
|
|||||||
suffix=".tmp",
|
suffix=".tmp",
|
||||||
)
|
)
|
||||||
try:
|
try:
|
||||||
if mode is not None:
|
if mode is not None and hasattr(os, "fchmod"):
|
||||||
|
# fchmod is Unix-only; Windows' os module has no fchmod. Skipping it
|
||||||
|
# here is safe — mkstemp already created the temp file as 0o600, and
|
||||||
|
# the post-replace os.chmod below applies the final mode durably.
|
||||||
os.fchmod(fd, mode)
|
os.fchmod(fd, mode)
|
||||||
with os.fdopen(fd, "w", encoding="utf-8") as f:
|
with os.fdopen(fd, "w", encoding="utf-8") as f:
|
||||||
json.dump(
|
json.dump(
|
||||||
|
|||||||
Reference in New Issue
Block a user