mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-06-27 11:22:03 +00:00
fix(tools): catch mkdtemp OSError in tirith install to prevent unbounded retry and temp-dir leak (#51826)
When tempfile.mkdtemp() raises OSError (e.g. disk full), the exception propagated past the try/finally block, so _mark_install_failed() was never called. The 24h backoff marker never engaged, causing unbounded retry on every command -- each attempt leaked a tirith-install-* temp directory, eventually filling /tmp completely. Fix: wrap mkdtemp in its own try/except OSError, returning (None, "no_space") so the caller's normal failure path (including _mark_install_failed) executes. Salvaged from #51831 by @liuhao1024. Closes #51826
This commit is contained in:
parent
77d2b50751
commit
dbf0797335
2 changed files with 55 additions and 1 deletions
|
|
@ -1427,3 +1427,53 @@ class TestIsAppTldFinding:
|
|||
|
||||
def test_case_insensitive_match(self):
|
||||
assert self.fn({"rule_id": "lookalike_tld", "value": ".APP"})
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# mkdtemp OSError → no_space (disk-full leak prevention)
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
class TestMkdtempOSErrorNoSpace:
|
||||
"""When tempfile.mkdtemp raises OSError (e.g. disk full), _install_tirith
|
||||
must return (None, "no_space") instead of propagating the exception.
|
||||
This prevents the unbounded retry + temp-dir leak described in #51826.
|
||||
"""
|
||||
|
||||
def test_mkdtemp_oserror_returns_no_space(self):
|
||||
from tools.tirith_security import _install_tirith
|
||||
|
||||
with patch("tools.tirith_security.tempfile.mkdtemp",
|
||||
side_effect=OSError(28, "No space left on device")):
|
||||
result, reason = _install_tirith(log_failures=False)
|
||||
assert result is None
|
||||
assert reason == "no_space"
|
||||
|
||||
def test_mkdtemp_oserror_does_not_leak_tempdir(self):
|
||||
"""No temp directory should remain after a mkdtemp failure."""
|
||||
import glob
|
||||
from tools.tirith_security import _install_tirith
|
||||
|
||||
before = set(glob.glob("/tmp/tirith-install-*"))
|
||||
with patch("tools.tirith_security.tempfile.mkdtemp",
|
||||
side_effect=OSError(28, "No space left on device")):
|
||||
_install_tirith(log_failures=False)
|
||||
after = set(glob.glob("/tmp/tirith-install-*"))
|
||||
assert after - before == set()
|
||||
|
||||
def test_mkdtemp_oserror_propagates_to_ensure_installed(self):
|
||||
"""ensure_installed should cache the failure via _mark_install_failed."""
|
||||
from tools.tirith_security import _resolve_tirith_path, _INSTALL_FAILED
|
||||
|
||||
_tirith_mod._resolved_path = None
|
||||
with patch("tools.tirith_security.tempfile.mkdtemp",
|
||||
side_effect=OSError(28, "No space left on device")), \
|
||||
patch("tools.tirith_security.shutil.which",
|
||||
return_value=None), \
|
||||
patch("tools.tirith_security._hermes_bin_dir",
|
||||
return_value="/nonexistent"), \
|
||||
patch("tools.tirith_security._is_install_failed_on_disk",
|
||||
return_value=False), \
|
||||
patch("tools.tirith_security._mark_install_failed") as mock_mark:
|
||||
result = _resolve_tirith_path("tirith")
|
||||
assert _tirith_mod._resolved_path is _INSTALL_FAILED
|
||||
mock_mark.assert_called_once_with("no_space")
|
||||
|
|
|
|||
|
|
@ -372,7 +372,11 @@ def _install_tirith(*, log_failures: bool = True) -> tuple[str | None, str]:
|
|||
archive_name = f"tirith-{target}.tar.gz"
|
||||
base_url = f"https://github.com/{_REPO}/releases/latest/download"
|
||||
|
||||
tmpdir = tempfile.mkdtemp(prefix="tirith-install-")
|
||||
try:
|
||||
tmpdir = tempfile.mkdtemp(prefix="tirith-install-")
|
||||
except OSError as exc:
|
||||
log("tirith install failed: cannot create temp dir: %s", exc)
|
||||
return None, "no_space"
|
||||
try:
|
||||
archive_path = os.path.join(tmpdir, archive_name)
|
||||
checksums_path = os.path.join(tmpdir, "checksums.txt")
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue