diff --git a/CHANGELOG.md b/CHANGELOG.md index 0ab88a8c..fa5d9f90 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,11 @@ ## [Unreleased] +## [v0.51.315] — 2026-06-07 — Release KE (test infra — cross-platform workspace-fallback tests) + +### Changed +- **Workspace-fallback tests are now cross-platform and deterministic.** Three tests simulated an unusable/unwritable workspace path by hard-coding a POSIX path (`/definitely/not/usable`) or by `chmod`-ing a temp dir read-only — which silently no-ops when the suite runs as root and is meaningless on Windows. They now simulate the failure by monkeypatching `_ensure_workspace_dir` / `Path.mkdir`, so the assertions hold identically on every platform and under any uid. Test-harness only; no shipped behavior change. (#3780 / #3771, @rodboev) + ## [v0.51.314] — 2026-06-07 — Release KD (test infra — reliable test-server boot + diagnostics) ### Changed diff --git a/tests/test_default_workspace_fallback.py b/tests/test_default_workspace_fallback.py index c22e40fc..c684a9dd 100644 --- a/tests/test_default_workspace_fallback.py +++ b/tests/test_default_workspace_fallback.py @@ -4,16 +4,30 @@ from pathlib import Path import api.config as config +def _reject_workspace_candidate(monkeypatch, candidate: Path) -> None: + original_ensure = config._ensure_workspace_dir + resolved_candidate = candidate.resolve() + + def wrapped(path: Path) -> bool: + if path.expanduser().resolve() == resolved_candidate: + return False + return original_ensure(path) + + monkeypatch.setattr(config, "_ensure_workspace_dir", wrapped) + + def test_resolve_default_workspace_falls_back_to_existing_home_work(monkeypatch, tmp_path): preferred = tmp_path / "work" preferred.mkdir() state_dir = tmp_path / "state" + bad_candidate = tmp_path / "not-usable" monkeypatch.setattr(config, "HOME", tmp_path) monkeypatch.setattr(config, "STATE_DIR", state_dir) monkeypatch.delenv("HERMES_WEBUI_DEFAULT_WORKSPACE", raising=False) + _reject_workspace_candidate(monkeypatch, bad_candidate) - resolved = config.resolve_default_workspace("/definitely/not/usable") + resolved = config.resolve_default_workspace(str(bad_candidate)) assert resolved == preferred.resolve() @@ -24,14 +38,16 @@ def test_save_settings_rewrites_bad_default_workspace_to_fallback(monkeypatch, t preferred.mkdir() state_dir = tmp_path / "state" settings_file = tmp_path / "settings.json" + bad_candidate = tmp_path / "not-usable" monkeypatch.setattr(config, "HOME", tmp_path) monkeypatch.setattr(config, "STATE_DIR", state_dir) monkeypatch.setattr(config, "SETTINGS_FILE", settings_file) monkeypatch.setattr(config, "DEFAULT_WORKSPACE", preferred) monkeypatch.delenv("HERMES_WEBUI_DEFAULT_WORKSPACE", raising=False) + _reject_workspace_candidate(monkeypatch, bad_candidate) - saved = config.save_settings({"default_workspace": "/definitely/not/usable"}) + saved = config.save_settings({"default_workspace": str(bad_candidate)}) on_disk = json.loads(settings_file.read_text(encoding="utf-8")) assert saved["default_workspace"] == str(preferred.resolve()) @@ -52,18 +68,15 @@ def test_resolve_default_workspace_creates_home_workspace_when_missing(monkeypat def test_resolve_default_workspace_raises_when_all_candidates_fail(monkeypatch, tmp_path): """RuntimeError is raised when every candidate is unwritable.""" - import stat, pytest - # Make tmp_path read-only so mkdir inside it fails - tmp_path.chmod(stat.S_IRUSR | stat.S_IXUSR) + import pytest state_dir = tmp_path / "state" monkeypatch.setattr(config, "HOME", tmp_path) monkeypatch.setattr(config, "STATE_DIR", state_dir) monkeypatch.delenv("HERMES_WEBUI_DEFAULT_WORKSPACE", raising=False) - try: - with pytest.raises(RuntimeError, match="Could not create or access"): - config.resolve_default_workspace(None) - finally: - tmp_path.chmod(stat.S_IRWXU) # restore for cleanup + monkeypatch.setattr(config, "_ensure_workspace_dir", lambda path: False) + + with pytest.raises(RuntimeError, match="Could not create or access"): + config.resolve_default_workspace(None) def test_workspace_candidates_deduplicates_home_workspace(monkeypatch, tmp_path): @@ -94,16 +107,13 @@ def test_env_var_workspace_takes_priority_over_passed_raw(monkeypatch, tmp_path) def test_ensure_workspace_dir_returns_false_for_unwritable_path(monkeypatch, tmp_path): """_ensure_workspace_dir returns False for a path that can't be created.""" - import stat - # Make parent read-only so mkdir fails - parent = tmp_path / "ro_parent" - parent.mkdir() - parent.chmod(stat.S_IRUSR | stat.S_IXUSR) - try: - result = config._ensure_workspace_dir(parent / "child") - assert result is False - finally: - parent.chmod(stat.S_IRWXU) + def fail_mkdir(self, mode=0o777, parents=False, exist_ok=False): + raise PermissionError("simulated create failure") + + monkeypatch.setattr(Path, "mkdir", fail_mkdir) + + result = config._ensure_workspace_dir(tmp_path / "child") + assert result is False def test_env_var_wins_over_settings_json_on_startup(monkeypatch, tmp_path): diff --git a/tests/test_media_inline.py b/tests/test_media_inline.py index b0ae101f..34840111 100644 --- a/tests/test_media_inline.py +++ b/tests/test_media_inline.py @@ -23,6 +23,7 @@ import urllib.parse import urllib.request from tests._pytest_port import BASE, TEST_STATE_DIR +from tests.conftest import TEST_WORKSPACE REPO_ROOT = pathlib.Path(__file__).parent.parent UI_JS = (REPO_ROOT / "static" / "ui.js").read_text(encoding="utf-8") @@ -30,6 +31,18 @@ I18N_JS = (REPO_ROOT / "static" / "i18n.js").read_text(encoding="utf-8") WORKSPACE_JS = (REPO_ROOT / "static" / "workspace.js").read_text(encoding="utf-8") +def _media_fixture_dir() -> pathlib.Path: + # Dot-prefixed so the persistent fixture dir stays OUT of + # /api/workspaces/suggest's default (non-hidden) results — otherwise it + # pollutes the shared TEST_WORKSPACE and breaks sibling tests that assert an + # exact workspace-suggestion set (e.g. test_sprint5 + # test_workspace_suggest_hidden_dirs_only_when_requested). Kept under + # TEST_WORKSPACE (a MEDIA_ALLOWED_ROOT) and Windows-portable (no /tmp). + fixture_dir = TEST_WORKSPACE / ".media-fixtures" + fixture_dir.mkdir(parents=True, exist_ok=True) + return fixture_dir + + # ── Static analysis: renderMd MEDIA stash ──────────────────────────────────── class TestMediaRenderMdStash(unittest.TestCase): @@ -519,6 +532,64 @@ class TestMediaEndpointUnit(unittest.TestCase): h4.status, 403, "profile webui_state/sessions/*.json must be denied") + def test_media_allowed_roots_env_var_serves_outside_hermes_root(self): + """MEDIA_ALLOWED_ROOTS must still allow legitimate outside-root media.""" + from api import routes + + class _Handler: + def __init__(self): + self.status = None + def send_response(self, code): + self.status = code + def send_header(self, *a, **k): + pass + def end_headers(self): + pass + class _W: + def write(self_inner, b): + pass + def flush(self_inner): + pass + wfile = _W() + headers = {} + + png_bytes = ( + b'\x89PNG\r\n\x1a\n\x00\x00\x00\rIHDR\x00\x00\x00\x01\x00\x00\x00\x01' + b'\x08\x06\x00\x00\x00\x1f\x15\xc4\x89\x00\x00\x00\nIDATx\x9cc\x00' + b'\x01\x00\x00\x05\x00\x01\r\n-\xb4\x00\x00\x00\x00IEND\xaeB`\x82' + ) + with tempfile.TemporaryDirectory() as home, tempfile.TemporaryDirectory() as extra: + hermes_home = pathlib.Path(home) / ".hermes" + hermes_home.mkdir(parents=True) + outside_root = pathlib.Path(extra).resolve() + image = outside_root / "settings_artifact.png" + image.write_bytes(png_bytes) + + with mock.patch.dict( + os.environ, + { + "HERMES_HOME": str(hermes_home), + "MEDIA_ALLOWED_ROOTS": str(outside_root), + }, + ), mock.patch.object( + routes, "get_last_workspace", lambda: str(hermes_home / "workspace") + ), mock.patch( + "api.auth.is_auth_enabled", lambda: False + ): + handler = _Handler() + routes._handle_media( + handler, + SimpleNamespace( + query=f"path={urllib.parse.quote(str(image))}&inline=1", + path="/api/media", + ), + ) + + self.assertEqual( + handler.status, 200, + "MEDIA_ALLOWED_ROOTS media outside Hermes roots must still serve", + ) + def test_media_endpoints_advertise_byte_range_support(self): routes_src = (REPO_ROOT / "api" / "routes.py").read_text(encoding="utf-8") self.assertIn("Accept-Ranges", routes_src) @@ -596,7 +667,10 @@ class TestMediaEndpointIntegration(unittest.TestCase): self.assertEqual(status, 400) def test_nonexistent_file_returns_404(self): - _, status, _ = self._get("/api/media?path=/tmp/__hermes_nonexistent_12345.png") + missing = _media_fixture_dir() / "__hermes_nonexistent_12345.png" + _, status, _ = self._get( + "/api/media?path=" + urllib.parse.quote(str(missing)) + ) self.assertEqual(status, 404) def test_path_outside_allowed_root_rejected(self): @@ -605,7 +679,7 @@ class TestMediaEndpointIntegration(unittest.TestCase): self.assertIn(status, {403, 404}) def test_valid_png_served_with_image_mime(self): - """Create a 1-pixel PNG in /tmp and verify it's served correctly.""" + """Create a 1-pixel PNG in the isolated test workspace and verify it serves.""" # Minimal valid 1x1 transparent PNG (67 bytes) png_bytes = ( b'\x89PNG\r\n\x1a\n\x00\x00\x00\rIHDR\x00\x00\x00\x01\x00\x00\x00\x01' @@ -613,13 +687,13 @@ class TestMediaEndpointIntegration(unittest.TestCase): b'\x01\x00\x00\x05\x00\x01\r\n-\xb4\x00\x00\x00\x00IEND\xaeB`\x82' ) with tempfile.NamedTemporaryFile( - suffix=".png", prefix="hermes_test_", dir="/tmp", delete=False + suffix=".png", prefix="hermes_test_", dir=_media_fixture_dir(), delete=False ) as f: f.write(png_bytes) tmp_path = f.name try: body, status, headers = self._get( - f"/api/media?path={urllib.request.quote(tmp_path)}" + f"/api/media?path={urllib.parse.quote(tmp_path)}" ) self.assertEqual(status, 200, f"Expected 200, got {status}") ct = headers.get("Content-Type", "") @@ -632,12 +706,12 @@ class TestMediaEndpointIntegration(unittest.TestCase): """MEDIA: audio paths stream inline and support byte ranges for playback.""" audio_bytes = b"RIFF" + (b"\x00" * 256) with tempfile.NamedTemporaryFile( - suffix=".wav", prefix="hermes_test_", dir="/tmp", delete=False + suffix=".wav", prefix="hermes_test_", dir=_media_fixture_dir(), delete=False ) as f: f.write(audio_bytes) tmp_path = f.name try: - encoded = urllib.request.quote(tmp_path) + encoded = urllib.parse.quote(tmp_path) body, status, headers = self._get(f"/api/media?path={encoded}&inline=1") self.assertEqual(status, 200) self.assertIn("audio/wav", headers.get("Content-Type", "")) @@ -659,12 +733,12 @@ class TestMediaEndpointIntegration(unittest.TestCase): """HTML opens inline only when requested and always carries CSP sandbox.""" html_bytes = b"Hermes" with tempfile.NamedTemporaryFile( - suffix=".html", prefix="hermes_test_", dir="/tmp", delete=False + suffix=".html", prefix="hermes_test_", dir=_media_fixture_dir(), delete=False ) as f: f.write(html_bytes) tmp_path = f.name try: - encoded = urllib.request.quote(tmp_path) + encoded = urllib.parse.quote(tmp_path) body, status, headers = self._get(f"/api/media?path={encoded}") self.assertEqual(status, 200) @@ -690,7 +764,7 @@ class TestMediaEndpointIntegration(unittest.TestCase): def test_path_traversal_rejected(self): _, status, _ = self._get( - "/api/media?path=" + urllib.request.quote("/tmp/../../etc/passwd") + "/api/media?path=" + urllib.parse.quote("/tmp/../../etc/passwd") ) self.assertIn(status, {403, 404}) @@ -709,7 +783,7 @@ class TestMediaEndpointIntegration(unittest.TestCase): settings.write_text('{"secret":"value"}', encoding="utf-8") try: _, status, _ = self._get( - "/api/media?path=" + urllib.request.quote(str(settings.resolve())) + "/api/media?path=" + urllib.parse.quote(str(settings.resolve())) ) self.assertEqual( status, 403, @@ -725,7 +799,7 @@ class TestMediaEndpointIntegration(unittest.TestCase): sess_file.write_text('{"messages":[]}', encoding="utf-8") try: _, status, _ = self._get( - "/api/media?path=" + urllib.request.quote(str(sess_file.resolve())) + "/api/media?path=" + urllib.parse.quote(str(sess_file.resolve())) ) self.assertEqual( status, 403, @@ -734,34 +808,34 @@ class TestMediaEndpointIntegration(unittest.TestCase): finally: sess_file.unlink(missing_ok=True) - def test_deny_list_does_not_overblock_legitimate_media(self): - """#3234 follow-up: the state/secret deny-list must NOT block ordinary - media that merely shares a sensitive basename but lives OUTSIDE any - Hermes state root (e.g. a user artifact in /tmp named settings.json). - - The deny is scoped to files under a Hermes root; a /tmp PNG named - settings.png — or even settings.json — is the user's own content and - must still be served (200), not 403. + def test_deny_list_does_not_overblock_active_workspace_media(self): + """#3234 follow-up: workspace media with a sensitive-looking basename must + still serve when it lives under the active workspace carve-out. """ png_bytes = ( b'\x89PNG\r\n\x1a\n\x00\x00\x00\rIHDR\x00\x00\x00\x01\x00\x00\x00\x01' b'\x08\x06\x00\x00\x00\x1f\x15\xc4\x89\x00\x00\x00\nIDATx\x9cc\x00' b'\x01\x00\x00\x05\x00\x01\r\n-\xb4\x00\x00\x00\x00IEND\xaeB`\x82' ) - # A /tmp artifact whose stem collides with a denied basename — must serve - # because /tmp is not a Hermes state root. + # A test-workspace artifact whose stem collides with a denied basename + # must still serve because the active workspace carve-out allows + # legitimate user media even when the test workspace lives under + # TEST_STATE_DIR. with tempfile.NamedTemporaryFile( - suffix=".png", prefix="settings_artifact_", dir="/tmp", delete=False + suffix=".png", + prefix="settings_artifact_", + dir=_media_fixture_dir(), + delete=False, ) as f: f.write(png_bytes) tmp_path = f.name try: body, status, headers = self._get( - f"/api/media?path={urllib.request.quote(tmp_path)}" + f"/api/media?path={urllib.parse.quote(tmp_path)}" ) self.assertEqual( status, 200, - f"a /tmp PNG outside any Hermes root must serve, got {status}", + f"a test-workspace PNG under the active workspace must serve, got {status}", ) self.assertIn("image/png", headers.get("Content-Type", "")) finally: diff --git a/tests/test_onboarding_existing_config.py b/tests/test_onboarding_existing_config.py index 8c7f56cf..92f700c0 100644 --- a/tests/test_onboarding_existing_config.py +++ b/tests/test_onboarding_existing_config.py @@ -33,14 +33,16 @@ _needs_yaml = pytest.mark.skipif(not _HAS_YAML, reason="PyYAML not installed — # --------------------------------------------------------------------------- -def _make_status(*, config_exists: bool, chat_ready: bool, onboarding_done: bool = False): +def _make_status( + *, tmp_path: pathlib.Path, config_exists: bool, chat_ready: bool, onboarding_done: bool = False +): """Call get_onboarding_status() with a controlled filesystem + settings.""" import importlib # Import fresh copies each call so module-level state doesn't bleed across import api.onboarding as mod - fake_config_path = pathlib.Path("/tmp/_test_config.yaml") + fake_config_path = tmp_path / "_test_config.yaml" settings = {"onboarding_completed": onboarding_done} @@ -55,7 +57,7 @@ def _make_status(*, config_exists: bool, chat_ready: bool, onboarding_done: bool "current_provider": "openrouter" if chat_ready else None, "current_model": "anthropic/claude-sonnet-4.6" if chat_ready else None, "current_base_url": None, - "env_path": "/tmp/.hermes_test/.env", + "env_path": str(tmp_path / ".hermes_test" / ".env"), } with ( @@ -81,42 +83,47 @@ def _make_status(*, config_exists: bool, chat_ready: bool, onboarding_done: bool class TestOnboardingGate: - def test_config_exists_and_chat_ready_returns_completed_true(self): + def test_config_exists_and_chat_ready_returns_completed_true(self, tmp_path): """Primary fix: existing valid config → wizard must NOT fire.""" - result = _make_status(config_exists=True, chat_ready=True) + result = _make_status(tmp_path=tmp_path, config_exists=True, chat_ready=True) assert result["completed"] is True, ( "Wizard fired for existing Hermes user! " "config.yaml + chat_ready must auto-complete onboarding." ) - def test_no_config_returns_completed_false(self): + def test_no_config_returns_completed_false(self, tmp_path): """Fresh install with no config → wizard should fire.""" - result = _make_status(config_exists=False, chat_ready=False) + result = _make_status(tmp_path=tmp_path, config_exists=False, chat_ready=False) assert result["completed"] is False, ( "Fresh install must show the wizard (completed should be False)." ) - def test_config_exists_but_not_chat_ready_still_shows_wizard(self): + def test_config_exists_but_not_chat_ready_still_shows_wizard(self, tmp_path): """Broken/incomplete config (config.yaml exists but chat_ready=False) → still show wizard so the user can fix it.""" - result = _make_status(config_exists=True, chat_ready=False) + result = _make_status(tmp_path=tmp_path, config_exists=True, chat_ready=False) # Should NOT be auto-completed — config is present but broken assert result["completed"] is False, ( "Broken config (chat_ready=False) must still show the wizard." ) - def test_onboarding_done_flag_always_respected(self): + def test_onboarding_done_flag_always_respected(self, tmp_path): """If user already completed onboarding in settings, never show wizard.""" - result = _make_status(config_exists=False, chat_ready=False, onboarding_done=True) + result = _make_status( + tmp_path=tmp_path, + config_exists=False, + chat_ready=False, + onboarding_done=True, + ) assert result["completed"] is True - def test_config_exists_always_exposed_in_system(self): + def test_config_exists_always_exposed_in_system(self, tmp_path): """config_exists must still appear in the response system block.""" - result = _make_status(config_exists=True, chat_ready=True) + result = _make_status(tmp_path=tmp_path, config_exists=True, chat_ready=True) assert "config_exists" in result["system"] assert result["system"]["config_exists"] is True - def test_persist_failure_does_not_break_status_endpoint(self): + def test_persist_failure_does_not_break_status_endpoint(self, tmp_path): """save_settings() failure (read-only FS, disk full) must not turn the status endpoint into a 500. The persistence-across-restart guarantee degrades but `completed` still reflects the live `config_auto_completed` @@ -133,9 +140,9 @@ class TestOnboardingGate: "current_provider": "openrouter", "current_model": "anthropic/claude-sonnet-4.6", "current_base_url": None, - "env_path": "/tmp/.hermes_test/.env", + "env_path": str(tmp_path / ".hermes_test" / ".env"), } - fake_config_path = pathlib.Path("/tmp/_test_config.yaml") + fake_config_path = tmp_path / "_test_config.yaml" with ( mock.patch.object(mod, "load_settings", return_value=settings), @@ -164,10 +171,10 @@ class TestOnboardingGate: class TestApplyOnboardingSetupGuard: """Fix #2: apply_onboarding_setup must not silently overwrite config.yaml.""" - def _call_setup(self, body: dict, config_yaml_exists: bool): + def _call_setup(self, tmp_path: pathlib.Path, body: dict, config_yaml_exists: bool): import api.onboarding as mod - fake_config_path = pathlib.Path("/tmp/_test_config.yaml") + fake_config_path = tmp_path / "_test_config.yaml" with ( mock.patch.object(mod, "_get_config_path", return_value=fake_config_path), @@ -175,9 +182,10 @@ class TestApplyOnboardingSetupGuard: ): return mod.apply_onboarding_setup(body) - def test_setup_blocked_when_config_exists_without_confirm(self): + def test_setup_blocked_when_config_exists_without_confirm(self, tmp_path): """Must return an error dict (not raise) if config.yaml exists and no confirm_overwrite.""" result = self._call_setup( + tmp_path, { "provider": "openrouter", "model": "anthropic/claude-sonnet-4.6", @@ -192,12 +200,12 @@ class TestApplyOnboardingSetupGuard: assert result.get("requires_confirm") is True @_needs_yaml - def test_setup_allowed_with_confirm_overwrite(self): + def test_setup_allowed_with_confirm_overwrite(self, tmp_path): """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 = tmp_path / "_test_config_confirm.yaml" fake_config_path.unlink(missing_ok=True) # start clean try: with tempfile.TemporaryDirectory() as tmp_home: @@ -223,18 +231,18 @@ class TestApplyOnboardingSetupGuard: fake_config_path.unlink(missing_ok=True) @_needs_yaml - def test_setup_allowed_when_no_config_exists(self): + def test_setup_allowed_when_no_config_exists(self, tmp_path): """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 = tmp_path / "_test_config_fresh.yaml" fake_config_path.unlink(missing_ok=True) try: 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. + # Redirect both config path and hermes home into temp dirs so the + # test never touches 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),