RomanNum3ral
/
hermes-agent
mirror of https://github.com/NousResearch/hermes-agent.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

fix(file-ops): stop read_file blocking forever on non-regular files 

The size probe every read path starts with — `wc -c < path` — opens the
path. On a FIFO with no writer, a socket, or a character device that never
reaches EOF, that read never returns, and read_file/read_file_raw/
read_file_bytes all pass no timeout to _exec. The turn wedges until the
process is killed.

The device blocklist in tools/file_tools.py cannot close this: it matches
literal /dev/* names, so it can only ever cover paths someone thought to
enumerate. A FIFO is a file type and can sit at any path.

Gate the probe behind `[ -f ]`, which stats instead of opening, and report
a path that exists but is not a regular file as such. A missing path keeps
its existing not-found handling.
This commit is contained in:
Drexuxux 2026-08-09 02:29:05 +03:00 committed by Teknium
parent 0514d67fa6
commit e0b5005985
3 changed files with 148 additions and 13 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

12
tests/tools/test_file_operations.py
View File

@ -303,7 +303,9 @@ class TestShellFileOpsHelpers:
def side_effect(command, **kwargs):
commands.append(command)
if command.startswith("wc -c"):
# The size probe gates `wc -c` behind `[ -f ]` so a FIFO or device
# cannot block the read; it still reports a plain byte count.
if command.startswith("if [ -f ") or command.startswith("wc -c"):
return {"output": "5\n", "returncode": 0}
if command.startswith("head -c") and "| base64" in command:
import base64 as b64
@ -321,7 +323,13 @@ class TestShellFileOpsHelpers:
result = ops.read_file(r"C:\Users\alice\notes.txt")
assert result.error is None
assert commands[0] == "wc -c < '/c/Users/alice/notes.txt' 2>/dev/null"
assert commands[0] == (
"if [ -f '/c/Users/alice/notes.txt' ]; "
"then wc -c < '/c/Users/alice/notes.txt' 2>/dev/null; "
"elif [ -e '/c/Users/alice/notes.txt' ]; "
"then echo __hermes_not_regular__; "
"else exit 1; fi"
)
assert commands[1] == "head -c 1000 '/c/Users/alice/notes.txt' 2>/dev/null | base64"
assert commands[2] == "sed -n '1,2000p' '/c/Users/alice/notes.txt'"
assert commands[3] == "wc -l < '/c/Users/alice/notes.txt'"

86
tests/tools/test_file_read_guards.py
View File

@ -211,6 +211,92 @@ class TestDevicePathBlocking(unittest.TestCase):
mock_ops.assert_not_called()
# ---------------------------------------------------------------------------
# Non-regular files (FIFOs, sockets, directories)
# ---------------------------------------------------------------------------
class TestNonRegularFileReads(unittest.TestCase):
"""Blocking paths the device blocklist structurally cannot cover.
The blocklist matches literal ``/dev/*`` names. A FIFO is a file *type*
and can sit at any path, so no name list catches it. Reading one with no
writer blocks in the size probe, and the read helpers pass no timeout, so
the turn wedges until the process is killed.
Each read runs on a worker thread with a wall clock: a thread still alive
at the deadline means the call blocked, which fails as an assertion
instead of hanging the suite.
"""
DEADLINE_SECONDS = 20.0
def _read_within_deadline(self, path, task_id):
import threading
box = {}
def call():
try:
box["raw"] = read_file_tool(path, task_id=task_id)
except BaseException as exc: # noqa: BLE001
box["exc"] = exc
worker = threading.Thread(target=call, daemon=True)
worker.start()
worker.join(self.DEADLINE_SECONDS)
self.assertFalse(
worker.is_alive(),
f"read_file_tool({path!r}) still running after "
f"{self.DEADLINE_SECONDS:.0f}s — the read blocked",
)
if "exc" in box:
raise box["exc"]
return json.loads(box["raw"])
def test_read_file_tool_on_fifo_errors_instead_of_blocking(self):
if not hasattr(os, "mkfifo"):
self.skipTest("platform has no os.mkfifo")
with tempfile.TemporaryDirectory() as tmpdir:
fifo_path = os.path.join(tmpdir, "pipe")
try:
os.mkfifo(fifo_path)
except (OSError, NotImplementedError) as exc:
self.skipTest(f"mkfifo unavailable: {exc}")
result = self._read_within_deadline(fifo_path, "fifo_read_test")
self.assertIn("error", result)
self.assertIn("not a regular file", result["error"])
def test_read_file_tool_on_directory_errors_instead_of_blocking(self):
with tempfile.TemporaryDirectory() as tmpdir:
result = self._read_within_deadline(tmpdir, "dir_read_test")
self.assertIn("error", result)
self.assertIn("not a regular file", result["error"])
def test_regular_file_still_reads(self):
"""The guard must not cost ordinary reads their content."""
with tempfile.TemporaryDirectory() as tmpdir:
target = os.path.join(tmpdir, "notes.txt")
with open(target, "w", encoding="utf-8") as handle:
handle.write("first line\nsecond line\n")
result = self._read_within_deadline(target, "regular_read_test")
self.assertNotIn("error", result)
self.assertIn("second line", result["content"])
def test_missing_file_still_reports_not_found(self):
"""An absent path keeps the not-found wording, not the type error."""
with tempfile.TemporaryDirectory() as tmpdir:
missing = os.path.join(tmpdir, "no-such-file.txt")
result = self._read_within_deadline(missing, "missing_read_test")
self.assertIn("error", result)
self.assertNotIn("not a regular file", result["error"])
# ---------------------------------------------------------------------------
# Character-count limits
# ---------------------------------------------------------------------------

63
tools/file_operations.py
View File

@ -732,6 +732,10 @@ DEFAULT_READ_LIMIT = 2000
DEFAULT_SEARCH_OFFSET = 0
DEFAULT_SEARCH_LIMIT = 50
# Echoed by the size probe when the path exists but is not a regular file.
# `wc -c` prints only digits, so this can never collide with a real size.
NOT_REGULAR_SENTINEL = "__hermes_not_regular__"
def _coerce_int(value: Any, default: int) -> int:
"""Best-effort integer coercion for tool pagination inputs."""
@ -1220,6 +1224,40 @@ class ShellFileOperations(FileOperations):
# READ Implementation
# =========================================================================
def _size_probe_cmd(self, path: str) -> str:
"""Byte size of a regular file, without opening one that never ends.
``wc -c < path`` opens the path. On a FIFO with no writer, a socket,
or a character device like /dev/zero that never reaches EOF, that
read blocks forever — and the read helpers pass no timeout to
:meth:`_exec`, so the turn wedges until the process is killed. The
device blocklist in ``tools/file_tools.py`` cannot cover this: it
matches literal ``/dev/*`` names, while a FIFO is a file *type* and
can sit at any path.
``[ -f ]`` is a stat, not an open — it answers exactly the question
the size probe needs (regular file, symlinks followed) without
touching the contents. Non-regular paths that exist report the
sentinel so callers can say so instead of claiming the file is
missing; a genuinely absent path still exits non-zero.
"""
arg = self._escape_shell_arg(path)
return (
f"if [ -f {arg} ]; then wc -c < {arg} 2>/dev/null; "
f"elif [ -e {arg} ]; then echo {NOT_REGULAR_SENTINEL}; "
f"else exit 1; fi"
)
@staticmethod
def _not_regular_error(path: str) -> ReadResult:
"""Error for a path that exists but would block if read."""
return ReadResult(
error=(
f"Cannot read '{path}': not a regular file (directory, FIFO, "
"socket, or device). Reading it could block indefinitely."
)
)
def read_file(self, path: str, offset: int = 1, limit: int = 2000) -> ReadResult:
"""
Read a file with pagination, binary detection, and line numbers.
@ -1237,10 +1275,9 @@ class ShellFileOperations(FileOperations):
offset, limit = normalize_read_pagination(offset, limit)
# Check if file exists and get size (wc -c is POSIX, works on Linux + macOS)
stat_cmd = f"wc -c < {self._escape_shell_arg(path)} 2>/dev/null"
stat_result = self._exec(stat_cmd)
# Check if file exists and get size (POSIX, works on Linux + macOS)
stat_result = self._exec(self._size_probe_cmd(path))
if stat_result.exit_code != 0:
# File not found. Before failing, try unicode-equivalent
# spellings — NFC/NFD, narrow no-break space, curly quotes
@ -1260,8 +1297,10 @@ class ShellFileOperations(FileOperations):
return result
# No equivalent spelling — suggest similar files
return self._suggest_similar_files(path)
stat_output = _strip_terminal_fence_leaks(stat_result.stdout)
if stat_output.strip() == NOT_REGULAR_SENTINEL:
return self._not_regular_error(path)
try:
file_size = int(stat_output.strip())
except ValueError:
@ -1473,11 +1512,12 @@ class ShellFileOperations(FileOperations):
Uses cat so the full file is returned regardless of size.
"""
path = self._expand_path(path)
stat_cmd = f"wc -c < {self._escape_shell_arg(path)} 2>/dev/null"
stat_result = self._exec(stat_cmd)
stat_result = self._exec(self._size_probe_cmd(path))
if stat_result.exit_code != 0:
return self._suggest_similar_files(path)
stat_output = _strip_terminal_fence_leaks(stat_result.stdout)
if stat_output.strip() == NOT_REGULAR_SENTINEL:
return self._not_regular_error(path)
try:
file_size = int(stat_output.strip())
except ValueError:
@ -1514,13 +1554,14 @@ class ShellFileOperations(FileOperations):
def read_file_bytes(self, path: str, max_bytes: Optional[int] = None) -> ReadResult:
"""Read binary-safe bytes from any shell-backed environment."""
path = self._expand_path(path)
stat_result = self._exec(
f"wc -c < {self._escape_shell_arg(path)} 2>/dev/null"
)
stat_result = self._exec(self._size_probe_cmd(path))
if stat_result.exit_code != 0:
return ReadResult(error=f"File not found: {path}")
stat_output = _strip_terminal_fence_leaks(stat_result.stdout)
if stat_output.strip() == NOT_REGULAR_SENTINEL:
return self._not_regular_error(path)
try:
file_size = int(_strip_terminal_fence_leaks(stat_result.stdout).strip())
file_size = int(stat_output.strip())
except ValueError:
return ReadResult(error=f"Could not determine file size: {path}")
if max_bytes is not None and file_size > max_bytes: