mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-05-29 06:31:32 +00:00
fix: guard yaml.safe_load, flock unlock, TOCTOU races, and atomic writes
1. trajectory_compressor.py: yaml.safe_load() returns None on empty
files, crashing with TypeError on `if 'tokenizer' in data`. Fix by
adding `or {}` fallback. (HIGH — blocks startup with empty config)
2. 6 files with fcntl.flock(LOCK_UN) in finally blocks without
try/except: cron/scheduler.py, hermes_cli/auth.py,
agent/shell_hooks.py, tools/skill_usage.py,
tools/environments/file_sync.py, tools/memory_tool.py. If unlock
raises OSError, fd.close() is skipped and the lock is held forever.
The msvcrt branches already had try/except; the fcntl branches did
not. Fix by wrapping in try/except (OSError, IOError): pass.
3. agent/copilot_acp_client.py line 639: TOCTOU race — path.exists()
followed by path.read_text() with no try/except. If file is deleted
between the check and the read, FileNotFoundError propagates. Fix
by using try/except FileNotFoundError.
4. gateway/sticker_cache.py: non-atomic write via Path.write_text()
can leave truncated JSON on crash, causing JSONDecodeError on next
load. Fix by writing to tempfile + fsync + os.replace (atomic).
This commit is contained in:
parent
d759a67c0f
commit
62573f44cf
9 changed files with 48 additions and 14 deletions
|
|
@ -636,7 +636,10 @@ class CopilotACPClient:
|
|||
block_error = get_read_block_error(str(path))
|
||||
if block_error:
|
||||
raise PermissionError(block_error)
|
||||
content = path.read_text() if path.exists() else ""
|
||||
try:
|
||||
content = path.read_text()
|
||||
except FileNotFoundError:
|
||||
content = ""
|
||||
line = params.get("line")
|
||||
limit = params.get("limit")
|
||||
if isinstance(line, int) and line > 1:
|
||||
|
|
|
|||
|
|
@ -632,7 +632,10 @@ def _locked_update_approvals() -> Iterator[Dict[str, Any]]:
|
|||
yield data
|
||||
save_allowlist(data)
|
||||
finally:
|
||||
fcntl.flock(lock_fh.fileno(), fcntl.LOCK_UN)
|
||||
try:
|
||||
fcntl.flock(lock_fh.fileno(), fcntl.LOCK_UN)
|
||||
except (OSError, IOError):
|
||||
pass
|
||||
|
||||
|
||||
def _prompt_and_record(
|
||||
|
|
|
|||
|
|
@ -1950,7 +1950,10 @@ def tick(verbose: bool = True, adapters=None, loop=None) -> int:
|
|||
return sum(_results)
|
||||
finally:
|
||||
if fcntl:
|
||||
fcntl.flock(lock_fd, fcntl.LOCK_UN)
|
||||
try:
|
||||
fcntl.flock(lock_fd, fcntl.LOCK_UN)
|
||||
except (OSError, IOError):
|
||||
pass
|
||||
elif msvcrt:
|
||||
try:
|
||||
msvcrt.locking(lock_fd.fileno(), msvcrt.LK_UNLCK, 1)
|
||||
|
|
|
|||
|
|
@ -9,6 +9,8 @@ Cache location: ~/.hermes/sticker_cache.json
|
|||
"""
|
||||
|
||||
import json
|
||||
import os
|
||||
import tempfile
|
||||
import time
|
||||
from typing import Optional
|
||||
|
||||
|
|
@ -35,12 +37,23 @@ def _load_cache() -> dict:
|
|||
|
||||
|
||||
def _save_cache(cache: dict) -> None:
|
||||
"""Save the sticker cache to disk."""
|
||||
"""Save the sticker cache to disk atomically."""
|
||||
CACHE_PATH.parent.mkdir(parents=True, exist_ok=True)
|
||||
CACHE_PATH.write_text(
|
||||
json.dumps(cache, indent=2, ensure_ascii=False),
|
||||
encoding="utf-8",
|
||||
fd, tmp_path = tempfile.mkstemp(
|
||||
dir=str(CACHE_PATH.parent), suffix=".tmp"
|
||||
)
|
||||
try:
|
||||
with os.fdopen(fd, "w", encoding="utf-8") as f:
|
||||
json.dump(cache, f, indent=2, ensure_ascii=False)
|
||||
f.flush()
|
||||
os.fsync(f.fileno())
|
||||
os.replace(tmp_path, str(CACHE_PATH))
|
||||
except BaseException:
|
||||
try:
|
||||
os.unlink(tmp_path)
|
||||
except OSError:
|
||||
pass
|
||||
raise
|
||||
|
||||
|
||||
def get_cached_description(file_unique_id: str) -> Optional[dict]:
|
||||
|
|
|
|||
|
|
@ -954,7 +954,10 @@ def _file_lock(
|
|||
finally:
|
||||
holder.depth = 0
|
||||
if fcntl:
|
||||
fcntl.flock(lock_file.fileno(), fcntl.LOCK_UN)
|
||||
try:
|
||||
fcntl.flock(lock_file.fileno(), fcntl.LOCK_UN)
|
||||
except (OSError, IOError):
|
||||
pass
|
||||
elif msvcrt:
|
||||
try:
|
||||
lock_file.seek(0)
|
||||
|
|
|
|||
|
|
@ -289,7 +289,10 @@ class FileSyncManager:
|
|||
fcntl.flock(lock_fd, fcntl.LOCK_EX)
|
||||
self._sync_back_impl()
|
||||
finally:
|
||||
fcntl.flock(lock_fd, fcntl.LOCK_UN)
|
||||
try:
|
||||
fcntl.flock(lock_fd, fcntl.LOCK_UN)
|
||||
except (OSError, IOError):
|
||||
pass
|
||||
lock_fd.close()
|
||||
|
||||
def _sync_back_impl(self) -> None:
|
||||
|
|
|
|||
|
|
@ -166,7 +166,10 @@ class MemoryStore:
|
|||
yield
|
||||
finally:
|
||||
if fcntl:
|
||||
fcntl.flock(fd, fcntl.LOCK_UN)
|
||||
try:
|
||||
fcntl.flock(fd, fcntl.LOCK_UN)
|
||||
except (OSError, IOError):
|
||||
pass
|
||||
elif msvcrt:
|
||||
try:
|
||||
fd.seek(0)
|
||||
|
|
|
|||
|
|
@ -86,7 +86,10 @@ def _usage_file_lock():
|
|||
yield
|
||||
finally:
|
||||
if fcntl:
|
||||
fcntl.flock(fd, fcntl.LOCK_UN)
|
||||
try:
|
||||
fcntl.flock(fd, fcntl.LOCK_UN)
|
||||
except (OSError, IOError):
|
||||
pass
|
||||
elif msvcrt:
|
||||
try:
|
||||
fd.seek(0)
|
||||
|
|
|
|||
|
|
@ -126,10 +126,10 @@ class CompressionConfig:
|
|||
def from_yaml(cls, yaml_path: str) -> "CompressionConfig":
|
||||
"""Load configuration from YAML file."""
|
||||
with open(yaml_path, 'r', encoding="utf-8") as f:
|
||||
data = yaml.safe_load(f)
|
||||
|
||||
data = yaml.safe_load(f) or {}
|
||||
|
||||
config = cls()
|
||||
|
||||
|
||||
# Tokenizer
|
||||
if 'tokenizer' in data:
|
||||
config.tokenizer_name = data['tokenizer'].get('name', config.tokenizer_name)
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue