hermes-agent/tests/hermes_cli/test_update_zip_symlink_reject.py
Teknium 46c1ae8b24
fix(tests): four pre-existing flakes from the security cluster merge (#32072)
All four failures were broken by the security cluster (#10082 / #10133 /
#4609 / symlink-reject batch) merging on May 25. They were red on
origin/main HEAD when #32042 and #32061 ran, gating PRs that touched
unrelated code.

1) tests/hermes_cli/test_update_zip_symlink_reject.py
   test_update_via_zip_accepts_normal_member called the real
   _update_via_zip without sandboxing PROJECT_ROOT — so the function's
   shutil.copytree() actually copied the fake README from the test ZIP
   over the real repo's README.md, which then made
   test_readme_mentions_powershell_installer fail in any test run that
   happened to pick this test up earlier. Mock PROJECT_ROOT to an
   isolated tmp_path / install_dir, stub subprocess so pip/uv reinstall
   doesn't actually run, and assert the fake README lands in the
   sandbox (not the real tree).

2) tests/tools/test_windows_native_support.py
   test_readme_mentions_powershell_installer was the victim of (1) —
   nothing wrong with the test itself, the fix in (1) clears it.

3) tests/tools/test_file_read_guards.py
   test_proc_fd_other_not_blocked called _is_blocked_device('/proc/self/fd/3')
   expecting False. But _is_blocked_device runs realpath() and on
   pytest xdist workers fd 3 happens to be dup'd to /dev/urandom
   (because the worker subprocess inherits open fds from pytest's
   collection pipe machinery). Switch to the lower-level
   _is_blocked_device_path which is the path-pattern check the test
   actually means to exercise; realpath-resolution coverage already
   lives in test_symlink_to_blocked_device_is_blocked.

4) tests/tools/test_transcription_tools.py
   Module installed a faster_whisper stub via sys.modules without
   setting __spec__, then later @pytest.mark.skipif called
   importlib.util.find_spec('faster_whisper') which raises
   'ValueError: __spec__ is None' for modules with a None spec attr.
   Set __spec__ on the stub to a real ModuleSpec.

Validation: 195/195 green across the 4 affected files.
2026-05-25 05:50:29 -07:00

132 lines
5.1 KiB
Python

"""Regression: _update_via_zip must reject ZIP members with symlink mode.
A symlink member in a downloaded update ZIP would let an attacker who can
serve / MITM the update mirror plant a symlink that extractall() then
follows, writing arbitrary file content outside the staging directory.
The Linux mode bits live in the upper 16 bits of ``ZipInfo.external_attr``;
we explicitly reject any member whose type bits are S_IFLNK.
"""
import io
import os
import stat
import tempfile
import zipfile
from unittest.mock import patch
import pytest
def _build_zip_with_symlink_member(zip_path: str, link_name: str, target: str) -> None:
"""Write a ZIP containing a single member with S_IFLNK mode bits set."""
with zipfile.ZipFile(zip_path, "w") as zf:
info = zipfile.ZipInfo(link_name)
# Upper 16 bits = Unix mode; mark as symlink (0o120000) + 0o777 perms.
info.external_attr = (stat.S_IFLNK | 0o777) << 16
# The "data" of a symlink ZIP member is the link target string.
zf.writestr(info, target)
def _build_normal_zip(zip_path: str) -> None:
"""Write a regular ZIP with a normal file member (no symlink)."""
with zipfile.ZipFile(zip_path, "w") as zf:
zf.writestr("hermes-agent-main/README.md", "ok\n")
def test_update_via_zip_rejects_symlink_member(tmp_path, monkeypatch):
"""A symlink member in the update ZIP must raise before extractall."""
zip_path = tmp_path / "evil.zip"
_build_zip_with_symlink_member(
str(zip_path),
link_name="hermes-agent-main/evil-link",
target="/etc/passwd",
)
from hermes_cli.main import _update_via_zip
args = type("Args", (), {})()
# Patch urlretrieve to "download" our pre-built malicious ZIP into the
# _update_via_zip tempdir. Capture the tempdir so we can prove no
# extraction happened.
captured = {}
original_mkdtemp = tempfile.mkdtemp
def capturing_mkdtemp(*args, **kwargs):
d = original_mkdtemp(*args, **kwargs)
captured["tmp_dir"] = d
return d
def fake_urlretrieve(url, dest):
# Copy our malicious zip into the destination dest path.
with open(zip_path, "rb") as src, open(dest, "wb") as dst:
dst.write(src.read())
return dest, None
with patch("tempfile.mkdtemp", side_effect=capturing_mkdtemp), \
patch("urllib.request.urlretrieve", side_effect=fake_urlretrieve):
# _update_via_zip catches ValueError, prints the message, and exits 1.
# That's the contract: a malicious ZIP must fail the update, not
# silently materialize a symlink.
with pytest.raises(SystemExit) as exc_info:
_update_via_zip(args)
assert exc_info.value.code == 1
# Belt: confirm extractall never produced the link.
tmp_dir = captured.get("tmp_dir")
if tmp_dir:
evil_path = os.path.join(tmp_dir, "hermes-agent-main", "evil-link")
assert not os.path.lexists(evil_path), (
"symlink member should never be materialized"
)
def test_update_via_zip_accepts_normal_member(tmp_path, monkeypatch, capsys):
"""A ZIP with only regular file members must extract without raising.
Sanity check that the symlink reject didn't break the happy path. We
point ``PROJECT_ROOT`` at an isolated tmp dir so the function's
``shutil.copytree(src, dst)`` over PROJECT_ROOT lands in a sandbox, NOT
the real repo checkout (which previously stomped on README.md whenever
this test ran, leaving 'ok\\n' there and breaking
``test_readme_mentions_powershell_installer`` for everyone else).
"""
zip_path = tmp_path / "normal.zip"
_build_normal_zip(str(zip_path))
# Sandbox PROJECT_ROOT so the file-copy phase can't escape the test's
# tmp tree. The function only reads PROJECT_ROOT to derive dst paths.
fake_root = tmp_path / "install_dir"
fake_root.mkdir()
from hermes_cli import main as hermes_main
monkeypatch.setattr(hermes_main, "PROJECT_ROOT", fake_root)
args = type("Args", (), {})()
def fake_urlretrieve(url, dest):
with open(zip_path, "rb") as src, open(dest, "wb") as dst:
dst.write(src.read())
return dest, None
# Stub the post-extract pip/uv reinstall so we don't actually run pip.
# The function may sys.exit(1) when those commands fail; that's fine —
# we only care that ZIP validation + extraction completed without
# raising "symlink member".
with patch("urllib.request.urlretrieve", side_effect=fake_urlretrieve), \
patch("subprocess.run") as fake_run, \
patch("subprocess.check_call"):
fake_run.return_value = type("R", (), {"returncode": 0, "stdout": "", "stderr": ""})()
try:
hermes_main._update_via_zip(args)
except SystemExit:
pass
captured = capsys.readouterr()
assert "symlink member" not in captured.out
assert "symlink member" not in captured.err
# The fake README from the ZIP should have landed in our sandbox root,
# confirming the extraction + copy phases ran past the validation gate.
assert (fake_root / "README.md").exists()
assert (fake_root / "README.md").read_text() == "ok\n"