From e7b8ab4d70e3493fe7cd8964b157b3434d517a7d Mon Sep 17 00:00:00 2001 From: nesquena-hermes Date: Thu, 16 Apr 2026 23:03:32 -0700 Subject: [PATCH] =?UTF-8?q?fix:=20harden=20test=20server=20isolation=20?= =?UTF-8?q?=E2=80=94=20HERMES=5FBASE=5FHOME=20+=20strip=20provider=20keys?= =?UTF-8?q?=20+=20mock=20=5Fget=5Factive=5Fhermes=5Fhome=20in=20unit=20tes?= =?UTF-8?q?ts=20(#620)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes the root cause of OPENROUTER_API_KEY being overwritten with test-key-fresh on every pytest run. Three-layer fix: 1. Unit tests: mock _get_active_hermes_home in TestApplyOnboardingSetupGuard so .env writes land in /tmp, never ~/.hermes 2. Test server subprocess: add HERMES_BASE_HOME=TEST_STATE_DIR to hard-lock profile resolution inside the server process 3. Test server subprocess: strip real provider keys (OPENROUTER_API_KEY etc.) from the inherited env before server starts Reviewed and approved by @nesquena. 1373 passed, 0 skipped. --- tests/conftest.py | 16 ++++++++ tests/test_onboarding_existing_config.py | 48 +++++++++++++++--------- 2 files changed, 47 insertions(+), 17 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index b45d4f3..e50eb3c 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -274,6 +274,14 @@ def test_server(): # os.environ already set at module level above; no-op here. env = os.environ.copy() + # Strip real provider keys so test subprocess never inherits production credentials. + # The test server uses a mock/isolated config — no real API calls are made. + for _k in list(env): + if any(_k.startswith(p) for p in ( + 'OPENROUTER_API_KEY', 'OPENAI_API_KEY', 'ANTHROPIC_API_KEY', + 'GOOGLE_API_KEY', 'DEEPSEEK_API_KEY', + )): + del env[_k] env.update({ "HERMES_WEBUI_PORT": str(TEST_PORT), "HERMES_WEBUI_HOST": "127.0.0.1", @@ -281,6 +289,14 @@ def test_server(): "HERMES_WEBUI_DEFAULT_WORKSPACE": str(TEST_WORKSPACE), "HERMES_WEBUI_DEFAULT_MODEL": "openai/gpt-5.4-mini", "HERMES_HOME": str(TEST_STATE_DIR), + # Belt-and-suspenders: HERMES_BASE_HOME hard-locks _DEFAULT_HERMES_HOME + # in api/profiles.py to the test state dir regardless of profile switching + # or any os.environ mutation that happens inside the server process. + # Without this, a profile switch or active_profile file in the real + # ~/.hermes can redirect _get_active_hermes_home() out of the sandbox, + # causing onboarding writes (config.yaml, .env) to land in the production + # ~/.hermes/profiles/webui/ and overwrite real API keys. + "HERMES_BASE_HOME": str(TEST_STATE_DIR), }) # Pass agent dir if discovered so server.py doesn't have to re-discover diff --git a/tests/test_onboarding_existing_config.py b/tests/test_onboarding_existing_config.py index 2947cae..76d831b 100644 --- a/tests/test_onboarding_existing_config.py +++ b/tests/test_onboarding_existing_config.py @@ -151,19 +151,25 @@ class TestApplyOnboardingSetupGuard: def test_setup_allowed_with_confirm_overwrite(self): """With confirm_overwrite=True, setup may proceed (will hit real logic).""" import api.onboarding as mod + import tempfile fake_config_path = pathlib.Path("/tmp/_test_config_confirm.yaml") fake_config_path.unlink(missing_ok=True) # start clean try: - # Without patching Path.exists, use a non-existent path so it won't block - result = mod.apply_onboarding_setup( - { - "provider": "openrouter", - "model": "anthropic/claude-sonnet-4.6", - "api_key": "test-key-confirm", - "confirm_overwrite": True, - } - ) + with tempfile.TemporaryDirectory() as tmp_home: + tmp_home_path = pathlib.Path(tmp_home) + # Without patching Path.exists, use a non-existent path so it won't block. + # Also redirect _get_active_hermes_home so .env writes go to the temp dir, + # never to the real ~/.hermes/.env. + with mock.patch.object(mod, "_get_active_hermes_home", return_value=tmp_home_path): + result = mod.apply_onboarding_setup( + { + "provider": "openrouter", + "model": "anthropic/claude-sonnet-4.6", + "api_key": "test-key-confirm", + "confirm_overwrite": True, + } + ) # Should NOT return config_exists error if isinstance(result, dict): assert result.get("error") != "config_exists", ( @@ -176,18 +182,26 @@ class TestApplyOnboardingSetupGuard: def test_setup_allowed_when_no_config_exists(self): """Fresh install: no config.yaml → setup proceeds normally (no blocking error).""" import api.onboarding as mod + import tempfile fake_config_path = pathlib.Path("/tmp/_test_config_fresh.yaml") fake_config_path.unlink(missing_ok=True) try: - with mock.patch.object(mod, "_get_config_path", return_value=fake_config_path): - result = mod.apply_onboarding_setup( - { - "provider": "openrouter", - "model": "anthropic/claude-sonnet-4.6", - "api_key": "test-key-fresh", - } - ) + with tempfile.TemporaryDirectory() as tmp_home: + tmp_home_path = pathlib.Path(tmp_home) + # Redirect both config path and hermes home so writes stay in /tmp, + # never touching the real ~/.hermes/.env. + with ( + mock.patch.object(mod, "_get_config_path", return_value=fake_config_path), + mock.patch.object(mod, "_get_active_hermes_home", return_value=tmp_home_path), + ): + result = mod.apply_onboarding_setup( + { + "provider": "openrouter", + "model": "anthropic/claude-sonnet-4.6", + "api_key": "test-key-fresh", + } + ) if isinstance(result, dict): assert result.get("error") != "config_exists" finally: