diff --git a/tests/conftest.py b/tests/conftest.py index c36942512b5..788e5cbafd8 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -20,9 +20,12 @@ test runner at ``scripts/run_tests.sh``. """ import asyncio +import atexit import os +import shutil import sqlite3 import sys +import tempfile from pathlib import Path import pytest @@ -33,6 +36,42 @@ if str(PROJECT_ROOT) not in sys.path: sys.path.insert(0, str(PROJECT_ROOT)) +# ── Sandbox HERMES_HOME before ANY test module is imported ────────────────── +# `hermes_cli/main.py` calls `setup_logging()` at MODULE level, which resolves +# `get_hermes_home()` and attaches rotating file handlers to the ROOT logger. +# So merely importing it - which many test modules do, directly or +# transitively - points the whole pytest session's logging at the operator's +# real `~/.hermes/logs/agent.log` and `errors.log`. +# +# The `_isolate_env` fixture below also sandboxes HERMES_HOME, but fixtures run +# AFTER collection imports test modules, by which point the handler already +# holds an absolute path to the real log. Measured on a live install: 126 +# warnings in the operator's agent.log came from test runs, not the gateway - +# enough noise to make genuine warnings hard to find. +# +# conftest is imported before any test module, so setting it here closes that +# window. The per-test fixture still applies for everything after import. +# +# ORDER MATTERS: the kanban write guard's deny-list (further down) must know +# the REAL Hermes root — capture it BEFORE the sandbox rewires HERMES_HOME, +# otherwise the deny-list would point at the throwaway tempdir and the guard +# would silently stop protecting the operator's actual ~/.hermes (#69385). +_PRE_SANDBOX_KANBAN_OVERRIDE = os.environ.get("HERMES_KANBAN_HOME", "").strip() +_PRE_SANDBOX_HERMES_HOME = os.environ.get("HERMES_HOME", "") +if not os.environ.get("HERMES_HOME"): + _SESSION_HERMES_HOME = tempfile.mkdtemp(prefix="hermes-test-home-") + os.environ["HERMES_HOME"] = _SESSION_HERMES_HOME + atexit.register(shutil.rmtree, _SESSION_HERMES_HOME, True) + +#: HERMES_HOME as it stood when conftest was imported - i.e. before any test +#: module could import code that configures logging. Recorded so the guard in +#: tests/test_log_isolation.py can assert the sandbox existed AT THAT MOMENT. +#: Reading os.environ from inside a test is useless here: the per-test +#: `_isolate_env` fixture has sandboxed it by then, so the check would pass +#: even with this block removed. +HERMES_HOME_AT_CONFTEST_IMPORT = os.environ.get("HERMES_HOME", "") + + # ── Per-file process isolation ────────────────────────────────────────────── # Tests run via ``scripts/run_tests_parallel.py``, which spawns a fresh # ``python -m pytest `` subprocess per test file. Cross-file state @@ -516,16 +555,23 @@ def _neutralize_macos_keychain_creds(request, monkeypatch): def _capture_real_kanban_root() -> Path: """Resolve the REAL kanban root from the pre-test environment. - Runs at conftest import time, before any fixture rewires HERMES_HOME. - Mirrors ``kanban_db.kanban_home()`` resolution order: + Uses the pre-sandbox environment snapshot taken at the very top of this + file (before the session HERMES_HOME sandbox rewired the env), so the + deny-list keeps pointing at the operator's actual root. Mirrors + ``kanban_db.kanban_home()`` resolution order: 1. ``HERMES_KANBAN_HOME`` env var when set and non-empty - 2. ``get_default_hermes_root()`` otherwise + 2. the real (pre-sandbox) Hermes root otherwise """ - override = os.environ.get("HERMES_KANBAN_HOME", "").strip() - if override: - return Path(override).expanduser().resolve() - from hermes_constants import get_default_hermes_root - return get_default_hermes_root().resolve() + if _PRE_SANDBOX_KANBAN_OVERRIDE: + return Path(_PRE_SANDBOX_KANBAN_OVERRIDE).expanduser().resolve() + if _PRE_SANDBOX_HERMES_HOME: + # HERMES_HOME was genuinely set before the sandbox — honor it via the + # normal resolver (it may be a profile dir whose root matters). + from hermes_constants import get_default_hermes_root + return get_default_hermes_root().resolve() + # No pre-existing HERMES_HOME: the real root is the platform default, + # NOT the sandbox tempdir now sitting in the env. + return (Path.home() / ".hermes").resolve() _REAL_KANBAN_ROOT = _capture_real_kanban_root() diff --git a/tests/test_log_isolation.py b/tests/test_log_isolation.py new file mode 100644 index 00000000000..08e0a6cb7f7 --- /dev/null +++ b/tests/test_log_isolation.py @@ -0,0 +1,88 @@ +"""The test suite must never write into the operator's real Hermes logs. + +`hermes_cli/main.py` calls `setup_logging()` at module scope, which resolves +`get_hermes_home()` and attaches rotating file handlers to the ROOT logger. +Importing it - which many test modules do, directly or transitively - wires +the whole pytest session's logging to `/logs/agent.log`. + +If HERMES_HOME is not already sandboxed at that moment, that is the +operator's real log. Measured on a live install, 126 warnings in a personal +`agent.log` came from test runs rather than the running gateway: phantom +`FakeTree` Discord failures and `rejected invalid API key` entries from +`test_api_server_runs.py`. Noise like that makes genuine warnings hard to +find precisely when someone is debugging. + +The per-test env fixture cannot close this: fixtures run after collection has +imported the test modules, and by then the handler holds an absolute path. +`tests/conftest.py` sets HERMES_HOME at module scope for that reason - this +guards the property so a refactor cannot quietly undo it. +""" + +import logging +import os +from pathlib import Path + +import pytest + + +def _real_hermes_home() -> Path: + """Where the operator's logs live, ignoring any test sandboxing.""" + return Path.home() / ".hermes" + + +def _all_file_destinations() -> list[str]: + """Every file path the root logger can reach, including via a QueueHandler. + + Logging is routed through a queue, so the file handlers hang off the + listener rather than the root logger - checking `root.handlers` alone + reports nothing and looks falsely clean. + """ + seen: list[str] = [] + + def collect(handlers) -> None: + for handler in handlers or (): + path = getattr(handler, "baseFilename", None) + if path: + seen.append(str(path)) + listener = getattr(handler, "listener", None) + if listener is not None: + collect(getattr(listener, "handlers", ())) + + collect(logging.getLogger().handlers) + + try: + import hermes_logging + + listener = getattr(hermes_logging, "_queue_listener", None) + if listener is not None: + collect(getattr(listener, "handlers", ())) + except Exception: + pass + + return seen + + +class TestLogIsolation: + def test_hermes_home_is_sandboxed_before_imports(self): + # Deliberately NOT os.environ: by test time the per-test `_isolate_env` + # fixture has sandboxed HERMES_HOME, so reading it here would pass even + # with the conftest block deleted. Assert the value captured at conftest + # import, which is the moment that actually matters. + from tests.conftest import HERMES_HOME_AT_CONFTEST_IMPORT as home + + assert home, "conftest must set HERMES_HOME before test modules import" + assert Path(home).resolve() != _real_hermes_home().resolve(), ( + f"HERMES_HOME pointed at the operator's real home ({home}) when " + "conftest loaded; import-time setup_logging() writes to their agent.log" + ) + + def test_importing_the_cli_does_not_target_the_real_logs(self): + pytest.importorskip("hermes_cli.main") + + real_logs = str(_real_hermes_home() / "logs") + offenders = [p for p in _all_file_destinations() if p.startswith(real_logs)] + + assert offenders == [], ( + "the test session is writing into the operator's real Hermes logs:\n " + + "\n ".join(offenders) + )