hermes-admin/hermes-agent
1
0
Fork 0
Code Issues Pull Requests Actions 4 Packages Projects Releases Wiki Activity

test: check guarded paths with strings, not pathlib

Browse Source 
HomeIOGuard.check built two or three Path objects per filesystem call and
ran is_relative_to() over the interpreter prefixes, the guarded roots and the
resolved target for every guarded open/stat/scandir — ~0.6ms per call. The
autouse fixture wraps every os call in every test, so a path-heavy e2e file
like the cron virtual-clock soak pays it millions of times: one scenario
measured 2.36M guard checks, i.e. ~20 minutes of pure guard CPU across its
threads, which under the e2e job's 3-way parallelism pushed the file past the
900s per-file cap with only the first scenarios complete (the CI hang of jobs
108211558961, 108211601999, 108150838362, 108233411588).

Do the whole check on case-folded strings instead: precomputed prefix
strings, os.path.abspath/realpath (C, no object churn) and explicit
separator-boundary prefix compares that reproduce Path.is_relative_to
semantics, including the filesystem root. os.path.normcase preserves the
case-insensitive compare Path.__eq__ gave on Windows. Decision order and
every exemption are unchanged; measured 611us -> 15us per guarded stat
(40x), and the full soak file drops from ~600s to ~155s in a 3-lane
container run that previously hit the 900s cap in every lane.
This commit is contained in:
Hermes Agent
2026-09-25 18:54:00 -05:00
committed by brooklyn!
parent e2b1c66791
commit 02f57212a4
1 changed files with 42 additions and 20 deletions
Download Patch File Download Diff File Expand all files Collapse all files

62
tests/home_io_guard.py
View File

@@ -25,6 +25,25 @@ _INTERPRETER_PREFIXES = tuple({
# trips on its first traceback.
Path(__file__).resolve().parent.parent,
})
# The same prefixes as plain strings for the check() fast path. PurePath comparison folds case on
# Windows; ``os.path.normcase`` (identity on POSIX) reproduces that for string compares. Prefixes
# resolve once at import, as before: they are fixed for the process lifetime.
_normcase = os.path.normcase
_INTERPRETER_PREFIX_STRS = tuple(_normcase(os.fspath(p)) for p in _INTERPRETER_PREFIXES)
def _within(path: str, prefix: str) -> bool:
"""``Path(path).is_relative_to(prefix)`` for two normalized, case-folded absolute strings."""
if path == prefix:
return True
return path.startswith(prefix) if prefix == os.sep else path.startswith(prefix + os.sep)
def _contains(path: str, prefix: str) -> bool:
"""``Path(prefix).is_relative_to(path)``: *path* is *prefix* or one of its ancestors."""
if path == prefix:
return True
return prefix.startswith(path) if path == os.sep else prefix.startswith(path + os.sep)
class HomeIOGuard:
@@ -38,26 +57,26 @@ class HomeIOGuard:
return
self.checking.active = True
try:
candidate = Path(os.fsdecode(value))
if candidate.parts and candidate.parts[0].startswith("~"):
candidate = os.fsdecode(value)
if candidate.startswith("~"):
# A test may have patched Path.expanduser to fail; the guard must not
# turn that into its own crash — the unexpanded path is checked instead.
try:
candidate = candidate.expanduser()
candidate = os.fspath(Path(candidate).expanduser())
except Exception:
pass
if dir_fd is not None and not candidate.is_absolute():
if dir_fd is not None and not os.path.isabs(candidate):
parent = self.directories.get(dir_fd)
if parent is None:
raise AssertionError("TEST BUG: untracked dir_fd in guarded filesystem I/O")
candidate = parent / candidate
absolute = Path(os.path.abspath(candidate))
candidate = os.path.join(os.fspath(parent), candidate)
absolute = _normcase(os.path.abspath(candidate))
# /proc/<pid>/fd/N is descriptor inspection (deleted-WAL holder scans stat the magic
# link to compare inode identity); resolving it names whatever file that fd holds,
# which is not I/O against the home.
if metadata and absolute.is_relative_to("/proc"):
if metadata and (absolute == "/proc" or absolute.startswith("/proc" + os.sep)):
return
roots = self.roots()
roots = tuple(_normcase(os.fspath(r)) for r in self.roots())
# Resolving the root itself (get_default_hermes_root's relative_to
# probe) reads no state; only its contents are guarded.
if metadata and absolute in roots:
@@ -68,26 +87,29 @@ class HomeIOGuard:
if metadata:
path = os.environ.get("PATH", "")
cwd = os.getcwd() if self._relative_path_entries(path) else None
if absolute.parent in self._path_entries(path, cwd):
if os.path.dirname(absolute) in self._path_entries(path, cwd):
return
# The interpreter's own installation (a PM-managed python under ~/.hermes/tools):
# stdlib source reads (linecache, traceback) are not Hermes state either, nor is
# realpath() walking up through its ancestors.
if any(absolute.is_relative_to(prefix) or (metadata and prefix.is_relative_to(absolute))
for prefix in _INTERPRETER_PREFIXES):
return
for prefix in _INTERPRETER_PREFIX_STRS:
if _within(absolute, prefix) or (metadata and _contains(absolute, prefix)):
return
# Check the lexical path first: resolving must not probe a protected
# tree merely to decide that the original path was forbidden.
if any(absolute.is_relative_to(root) for root in roots):
self.refuse(value)
resolved = absolute.resolve()
for root in roots:
if _within(absolute, root):
self.refuse(value)
resolved = _normcase(os.path.realpath(absolute))
if metadata and resolved in roots:
return
# A fixture symlink to the running interpreter resolves into its installation.
if any(resolved.is_relative_to(prefix) for prefix in _INTERPRETER_PREFIXES):
return
if any(resolved.is_relative_to(root) for root in roots):
self.refuse(value)
for prefix in _INTERPRETER_PREFIX_STRS:
if _within(resolved, prefix):
return
for root in roots:
if _within(resolved, root):
self.refuse(value)
finally:
self.checking.active = False
@@ -108,7 +130,7 @@ class HomeIOGuard:
def _path_entries(path: str, cwd: str | None):
# Relative PATH entries change meaning after chdir; absolute ones need no cwd.
return frozenset(
Path(os.path.normpath(os.path.join(cwd or "", entry)))
_normcase(os.path.normpath(os.path.join(cwd or "", entry)))
for entry in path.split(os.pathsep) if entry
)