diff --git a/hermes_cli/desktop_console.py b/hermes_cli/desktop_console.py index 5af13ea526..b311b4863e 100644 --- a/hermes_cli/desktop_console.py +++ b/hermes_cli/desktop_console.py @@ -4,6 +4,7 @@ import logging import os import sys import threading +import time from contextlib import contextmanager logger = logging.getLogger("hermes_cli.desktop") @@ -31,19 +32,39 @@ def desktop_console_output(*, source_mode: bool): yield {} return - import subprocess - - read_fd, write_fd = os.pipe() - - def drain(): + def drain(read_fd, level): with os.fdopen(read_fd, "rb") as stream: - while data := stream.readline(8192): - logger.info("[desktop] %s", data.decode("utf-8", errors="replace").rstrip()) + # Decode/redact complete records, not arbitrary pipe-read fragments. + # Discard an oversized line in full: even its prefix may be a secret. + limit = 128 * 1024 + while data := stream.readline(limit + 1): + if len(data) > limit: + while data and not data.endswith(b"\n"): + data = stream.readline(8192) + logger.log(level, "[desktop] [oversized line omitted]") + else: + logger.log(level, "[desktop] %s", data.decode("utf-8", errors="replace").rstrip()) - reader = threading.Thread(target=drain, name="desktop-console", daemon=True) - reader.start() + streams = {} + readers = [] try: - yield {"stdout": write_fd, "stderr": subprocess.STDOUT} + # Keep diagnostics visible even when ordinary INFO output is disabled. + for name, level in (("stdout", logging.INFO), ("stderr", logging.ERROR)): + read_fd, write_fd = os.pipe() + streams[name] = write_fd + reader = threading.Thread( + target=drain, args=(read_fd, level), name=f"desktop-console-{name}", daemon=True, + ) + try: + reader.start() + except BaseException: + os.close(read_fd) + raise + readers.append(reader) + yield streams finally: - os.close(write_fd) - reader.join(timeout=1) + for write_fd in streams.values(): + os.close(write_fd) + deadline = time.monotonic() + 1 + for reader in readers: + reader.join(timeout=max(0, deadline - time.monotonic())) diff --git a/hermes_cli/desktop_source_cache.py b/hermes_cli/desktop_source_cache.py deleted file mode 100644 index 0413487f68..0000000000 --- a/hermes_cli/desktop_source_cache.py +++ /dev/null @@ -1,111 +0,0 @@ -"""Reuse the desktop content hash while all source metadata is unchanged. - -This is a build-freshness cache, not an integrity/security check. Normal launches -must not read thousands of source/test files (and trigger on-access AV scans). -Changed metadata always falls back to the existing content hash, so checkout -mtime churn alone still does not force a rebuild. -""" - -import ctypes -import contextlib -import hashlib -import json -import os -import tempfile -from functools import lru_cache -from pathlib import Path -from typing import Callable - - -@lru_cache(maxsize=1) -def _windows_file_api(): - from ctypes import wintypes - - class FileBasicInfo(ctypes.Structure): - _fields_ = [(name, ctypes.c_longlong) for name in ( - "creation", "access", "write", "change" - )] + [("attributes", wintypes.DWORD)] - - kernel = ctypes.WinDLL("kernel32", use_last_error=True) - kernel.CreateFileW.argtypes = [wintypes.LPCWSTR, wintypes.DWORD, wintypes.DWORD, - ctypes.c_void_p, wintypes.DWORD, wintypes.DWORD, wintypes.HANDLE] - kernel.CreateFileW.restype = wintypes.HANDLE - kernel.GetFileInformationByHandleEx.argtypes = [wintypes.HANDLE, ctypes.c_int, - ctypes.c_void_p, wintypes.DWORD] - kernel.GetFileInformationByHandleEx.restype = wintypes.BOOL - kernel.CloseHandle.argtypes = [wintypes.HANDLE] - kernel.CloseHandle.restype = wintypes.BOOL - return kernel, FileBasicInfo - - -def _change_time(path: Path, st: os.stat_result) -> int: - if os.name != "nt": - return st.st_ctime_ns - # Windows stat().st_ctime is CREATION time on supported Python versions. - # FILE_BASIC_INFO.ChangeTime also catches same-size edits with restored mtime. - # Request metadata only: never FILE_READ_DATA or a read of source contents. - kernel, info_type = _windows_file_api() - handle = kernel.CreateFileW(str(path), 0x80, 7, None, 3, 0x02000000, None) - if handle == ctypes.c_void_p(-1).value: - raise ctypes.WinError(ctypes.get_last_error()) - try: - info = info_type() - if not kernel.GetFileInformationByHandleEx(handle, 0, ctypes.byref(info), ctypes.sizeof(info)): - raise ctypes.WinError(ctypes.get_last_error()) - return info.change - finally: - kernel.CloseHandle(handle) - - -def _source_metadata(project_root: Path, tree_dir: Path) -> str: - from hermes_cli.main_web_build import _source_tree_files - - digest = hashlib.sha256() - for path in _source_tree_files(project_root, tree_dir): - st = path.stat() - record = (str(path.relative_to(project_root)), st.st_dev, st.st_ino, - st.st_size, st.st_mtime_ns, _change_time(path, st)) - digest.update(json.dumps(record, ensure_ascii=True).encode()) - digest.update(b"\0") - return digest.hexdigest() - - -def cached_desktop_source_hash(project_root: Path, compute: Callable[[], str]) -> str: - from hermes_constants import get_hermes_home - - tree_dir = project_root / "apps" / "desktop" - cache_file = get_hermes_home() / "desktop-source-cache.json" - scope = str(project_root.resolve()) - try: - before = _source_metadata(project_root, tree_dir) - except OSError: - return compute() # Metadata unavailable: keep the full content check. - try: - cache = json.loads(cache_file.read_text(encoding="utf-8")) - if (isinstance(cache, dict) and cache.get("version") == 1 - and cache.get("root") == scope and cache.get("metadata") == before - and isinstance(cache.get("contentHash"), str) and len(cache["contentHash"]) == 64): - return cache["contentHash"] - except (OSError, ValueError): - pass - - content_hash = compute() - temporary = None - try: - # Do not cache a digest if an editor/update changed inputs while we read. - if before != _source_metadata(project_root, tree_dir): - return content_hash - cache_file.parent.mkdir(parents=True, exist_ok=True) - with tempfile.NamedTemporaryFile(mode="w", encoding="utf-8", dir=cache_file.parent, - prefix=".desktop-source-", delete=False) as stream: - temporary = Path(stream.name) - json.dump({"version": 1, "root": scope, "metadata": before, - "contentHash": content_hash}, stream) - temporary.replace(cache_file) - except OSError: - pass # Read-only cache locations must not prevent building/launching. - finally: - if temporary is not None: - with contextlib.suppress(OSError): - temporary.unlink(missing_ok=True) - return content_hash diff --git a/hermes_cli/main_desktop.py b/hermes_cli/main_desktop.py index 670e24cc70..8c3a25b89b 100644 --- a/hermes_cli/main_desktop.py +++ b/hermes_cli/main_desktop.py @@ -37,9 +37,7 @@ def _desktop_dist_exists(desktop_dir: Path) -> bool: def _compute_desktop_content_hash(project_root: Path) -> str: """SHA-256 of ``apps/desktop/`` (minus .gitignore matches) plus root workspace config.""" - from hermes_cli.desktop_source_cache import cached_desktop_source_hash - return cached_desktop_source_hash( - project_root, lambda: _hash_source_tree(project_root, project_root / "apps" / "desktop")) + return _hash_source_tree(project_root, project_root / "apps" / "desktop") def _desktop_stamp_path() -> Path: diff --git a/hermes_logging.py b/hermes_logging.py index a2f786f9bb..9142759181 100644 --- a/hermes_logging.py +++ b/hermes_logging.py @@ -165,7 +165,7 @@ COMPONENT_PREFIXES = { "tools": ("tools",), "cli": ("hermes_cli", "cli"), "cron": ("cron",), - "gui": ("hermes_cli.web_server", "hermes_cli.pty_bridge", "tui_gateway", "uvicorn"), + "gui": ("hermes_cli.web_server", "hermes_cli.pty_bridge", "hermes_cli.desktop", "tui_gateway", "uvicorn"), } diff --git a/tests/hermes_cli/test_desktop_console.py b/tests/hermes_cli/test_desktop_console.py index 0bfa99c1a0..e3d48794df 100644 --- a/tests/hermes_cli/test_desktop_console.py +++ b/tests/hermes_cli/test_desktop_console.py @@ -30,6 +30,66 @@ def test_packaged_console_output_drains_both_streams(caplog, monkeypatch): assert any("diagnostic" in message for message in messages) +@pytest.mark.windows_only +@pytest.mark.parametrize("level", [logging.WARNING, logging.ERROR]) +def test_stderr_survives_logging_threshold(caplog, level): + caplog.set_level(level, logger="hermes_cli.desktop") + with desktop_console_output(source_mode=False) as streams: + result = subprocess.run( + [sys.executable, "-c", "import sys; print('ordinary-output'); " + "sys.stderr.write('fatal-diagnostic\\n'); sys.exit(7)"], + timeout=15, check=False, **streams, + ) + assert result.returncode == 7 + messages = [record.getMessage() for record in caplog.records] + assert any("fatal-diagnostic" in message for message in messages) + assert not any("ordinary-output" in message for message in messages) + + +@pytest.mark.windows_only +@pytest.mark.parametrize("stream", ["stdout", "stderr"]) +def test_complete_records_reach_redacted_logs(tmp_path, monkeypatch, caplog, stream): + import hermes_logging + + monkeypatch.setenv("HERMES_HOME", str(tmp_path)) + hermes_logging._reset_queued_handlers() + caplog.set_level(logging.INFO) + caplog.set_level(logging.INFO, logger="hermes_cli.desktop") + secret = "synthetic-boundary-credential" + payload = ( + b" " * (8192 - len(b"API_KEY=")) + b"API_KEY=" + secret.encode() + b"\n" + + b"u" * 8191 + "\u20ac\n".encode() + + b"oversized-start API_KEY=" + secret.encode() + b"z" * 262144 + + b"oversized-end\nrecovered\ntail " + "\u20ac".encode() + ) + payload_path = tmp_path / "child-output.bin" + payload_path.write_bytes(payload) + try: + log_dir = hermes_logging.setup_logging(hermes_home=tmp_path, log_level="INFO", mode="gui") + with desktop_console_output(source_mode=False) as streams: + result = subprocess.run( + [sys.executable, "-c", "import pathlib, sys; " + "data = pathlib.Path(sys.argv[1]).read_bytes(); " + "out = getattr(sys, sys.argv[2]).buffer; " + "[(out.write(data[i:i+997]), out.flush()) for i in range(0, len(data), 997)]", + str(payload_path), stream], + timeout=15, check=False, **streams, + ) + assert result.returncode == 0 + hermes_logging.flush_log_queue() + text = (log_dir / "agent.log").read_text(encoding="utf-8") + assert secret not in text + assert "u" * 8191 + "\u20ac" in text + assert "\ufffd" not in text + assert "oversized-start" not in text and "oversized-end" not in text + assert "recovered" in text and "tail \u20ac" in text + assert "omitted" in text.lower() + gui_text = (log_dir / "gui.log").read_text(encoding="utf-8") + assert "recovered" in gui_text and secret not in gui_text + finally: + hermes_logging._reset_queued_handlers() + + def test_source_launch_keeps_interactive_streams(): with desktop_console_output(source_mode=True) as streams: assert streams == {} diff --git a/tests/hermes_cli/test_desktop_startup_cost.py b/tests/hermes_cli/test_desktop_startup_cost.py index 5f038e83b3..8a27553b50 100644 --- a/tests/hermes_cli/test_desktop_startup_cost.py +++ b/tests/hermes_cli/test_desktop_startup_cost.py @@ -1,7 +1,6 @@ """Startup regressions exercised against real source trees on the current host.""" import argparse -import builtins import os import subprocess from pathlib import Path @@ -25,28 +24,21 @@ def _tree(tmp_path): return root, app, source -def test_unchanged_desktop_hash_does_not_reopen_sources(tmp_path): - root, app, source = _tree(tmp_path) +def test_desktop_hash_prunes_ignored_build_directories(tmp_path): + root, app, _ = _tree(tmp_path) ignored = app / "dist" ignored.mkdir() (ignored / "bundle.js").write_text("built output") expected = desktop._compute_desktop_content_hash(root) - original_open = builtins.open original_scandir = os.scandir - reads = [] - - def track(file, *args, **kwargs): - if isinstance(file, (str, os.PathLike)) and Path(file) == source: - reads.append(file) - return original_open(file, *args, **kwargs) def scan(directory): assert Path(directory) != ignored, "ignored build output must be pruned before traversal" return original_scandir(directory) - with patch("builtins.open", side_effect=track), patch("os.scandir", side_effect=scan): + (ignored / "bundle.js").write_text("different build output") + with patch("os.scandir", side_effect=scan): assert desktop._compute_desktop_content_hash(root) == expected - assert reads == [], "launch should use source metadata, not read every source file again" def test_desktop_hash_invalidates_for_edits_even_with_restored_mtime(tmp_path): @@ -67,6 +59,41 @@ def test_desktop_hash_invalidates_for_edits_even_with_restored_mtime(tmp_path): assert desktop._compute_desktop_content_hash(root) != original +@pytest.mark.windows_only +def test_desktop_hash_detects_memory_mapped_edits(tmp_path): + import mmap + + root, _, source = _tree(tmp_path) + original = desktop._compute_desktop_content_hash(root) + with source.open("r+b") as stream: + with mmap.mmap(stream.fileno(), 0, access=mmap.ACCESS_WRITE) as view: + view[-1:] = b"2" + view.flush() + assert source.read_bytes().endswith(b"2") + assert desktop._compute_desktop_content_hash(root) != original + + +@pytest.mark.windows_only +def test_desktop_hash_recovers_after_a_temporary_read_lock(tmp_path): + import msvcrt + + root, _, source = _tree(tmp_path) + source.write_bytes(b"") + original = desktop._compute_desktop_content_hash(root) + payload = b"export const x = 2" + source.write_bytes(payload) + with source.open("r+b") as locked: + msvcrt.locking(locked.fileno(), msvcrt.LK_NBLCK, len(payload)) + try: + with pytest.raises(OSError): + source.read_bytes() + desktop._compute_desktop_content_hash(root) + finally: + locked.seek(0) + msvcrt.locking(locked.fileno(), msvcrt.LK_UNLCK, len(payload)) + assert desktop._compute_desktop_content_hash(root) != original + + def test_current_packaged_launch_does_not_require_npm(tmp_path, monkeypatch): root, app, _ = _tree(tmp_path) monkeypatch.setattr(cli_main, "PROJECT_ROOT", root)