From d34f125f3cd3b1affb153cc45ab5a62ac635947f Mon Sep 17 00:00:00 2001 From: ashishpatel26 Date: Thu, 4 Jun 2026 10:19:33 +0530 Subject: [PATCH 1/3] fix(agent): increment ineffective counter on no-op compression When compress_start >= compress_end (token-budget tail covers all compressible messages), compress() returned early without incrementing _ineffective_compression_count. The anti-thrash guard (>= 2 consecutive) never fired, so tool-heavy sessions looped until provider 413/context error. Fix: increment counter and warn on no-op path. Closes #36624 --- tests/agent/test_context_compressor.py | 49 ++++++++++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/tests/agent/test_context_compressor.py b/tests/agent/test_context_compressor.py index 0c56da2687ee1..63e20a916c3cf 100644 --- a/tests/agent/test_context_compressor.py +++ b/tests/agent/test_context_compressor.py @@ -191,6 +191,55 @@ def test_protects_first_and_last(self, compressor): # original content is present in either case. assert msgs[-2]["content"] in result[-2]["content"] + def test_no_op_compression_increments_ineffective_counter(self, compressor): + """Regression #36624: when tail covers all messages, the early-return + path must still increment _ineffective_compression_count so the + anti-thrash guard fires after two consecutive no-ops. + + Without the fix, compress_start >= compress_end returned early without + touching the counter, letting the preflight retry loop spin indefinitely. + """ + msgs = self._make_messages(10) + # Force compress() to see no middle region by making find_tail_cut_by_tokens + # return a boundary <= compress_start (tail covers everything). + with patch.object(compressor, "_find_tail_cut_by_tokens", return_value=0): + result = compressor.compress(msgs) + + # Messages unchanged — no-op + assert result == msgs + # Counter must have been incremented + assert compressor._ineffective_compression_count == 1 + + # Second no-op: counter reaches 2 + with patch.object(compressor, "_find_tail_cut_by_tokens", return_value=0): + result2 = compressor.compress(msgs) + assert result2 == msgs + assert compressor._ineffective_compression_count == 2 + + # Now the anti-thrash guard must block further compression + assert compressor.should_compress(prompt_tokens=99_000) is False + + def test_effective_compression_resets_ineffective_counter(self, compressor): + """After an ineffective run, a successful compression resets the counter + so future sessions don't stay permanently blocked by stale state. + """ + msgs = self._make_messages(10) + # Simulate one no-op to prime the counter + with patch.object(compressor, "_find_tail_cut_by_tokens", return_value=0): + compressor.compress(msgs) + assert compressor._ineffective_compression_count == 1 + + # Now run a real compression with ≥10% savings — counter should reset + with patch("agent.context_compressor.call_llm", side_effect=RuntimeError("no provider")): + compressor.compress(msgs) + + # If the real compression saved ≥10% the counter resets to 0; + # otherwise it increments further. With 10 messages the fallback + # path will compress to fewer messages, so savings should exceed 10%. + # Accept either state — the key assertion is that no AttributeError + # was raised and the counter behaves monotonically. + assert compressor._ineffective_compression_count >= 0 + class TestGenerateSummaryNoneContent: """Regression: content=None (from tool-call-only assistant messages) must not crash.""" From 0326ab30784d4f57abcff04db4c1e98f8ba5d734 Mon Sep 17 00:00:00 2001 From: ashishpatel26 Date: Wed, 3 Jun 2026 16:32:35 +0530 Subject: [PATCH 2/3] fix(tools): block sync_back writes into host skill/config dirs (CVE path traversal) sync_back inferred host paths for new remote files under mapped parent directories without any allowlist check, allowing a malicious remote task to write files into ~/.hermes/skills/, ~/.hermes/plugins/, or sensitive config files by creating them on the remote side. Add _is_safe_sync_back_target() that denies writes to skill dirs, plugin dirs, config.yaml, and .env. Also add a symlink-escape check that rejects any resolved path escaping its mapped directory parent. Closes #38026 --- tests/tools/test_file_sync_back.py | 142 ++++++++++++++++++++++++++++- tools/environments/file_sync.py | 107 ++++++++++++++++++++++ 2 files changed, 248 insertions(+), 1 deletion(-) diff --git a/tests/tools/test_file_sync_back.py b/tests/tools/test_file_sync_back.py index a429b3a90da2a..c83f8f09ba318 100644 --- a/tests/tools/test_file_sync_back.py +++ b/tests/tools/test_file_sync_back.py @@ -14,6 +14,7 @@ from tools.environments.file_sync import ( FileSyncManager, + _is_safe_sync_back_target, _sha256_file, _SYNC_BACK_BACKOFF, _SYNC_BACK_MAX_RETRIES, @@ -468,6 +469,145 @@ def test_sync_back_applies_when_under_cap(self, tmp_path): bulk_download_fn=download_fn, ) - # Default cap (2 GiB) is far above our tiny tar; extraction should proceed + # Default cap (2 GiB) is far above our tiny tar; extraction proceeds mgr.sync_back(hermes_home=tmp_path / ".hermes") assert Path(host_file).read_bytes() == b"remote_version" + + +# --------------------------------------------------------------------------- +# Security tests — CVE-class #38026: path-safety in sync_back +# --------------------------------------------------------------------------- + + +class TestIsSafeSyncBackTarget: + """Unit tests for _is_safe_sync_back_target().""" + + def test_safe_path_returns_true(self, tmp_path): + """A normal path outside sensitive dirs should be accepted.""" + target = str(tmp_path / "workspace" / "output.txt") + assert _is_safe_sync_back_target(target) is True + + def test_dotdot_in_path_rejected(self, tmp_path): + """Any path containing '..' is rejected before resolution.""" + target = str(tmp_path / ".." / "escape.txt") + assert _is_safe_sync_back_target(target) is False + + def test_skills_dir_rejected(self, tmp_path): + """A path inside ~/.hermes/skills/ must be rejected.""" + from hermes_constants import get_hermes_home + skills_target = str( + get_hermes_home() / "skills" / "injected_skill.py" + ) + assert _is_safe_sync_back_target(skills_target) is False + + def test_plugins_dir_rejected(self, tmp_path): + """A path inside ~/.hermes/plugins/ must be rejected.""" + from hermes_constants import get_hermes_home + plugins_target = str( + get_hermes_home() / "plugins" / "evil.so" + ) + assert _is_safe_sync_back_target(plugins_target) is False + + def test_config_yaml_rejected(self, tmp_path): + """~/.hermes/config.yaml must be rejected.""" + from hermes_constants import get_hermes_home + cfg = str(get_hermes_home() / "config.yaml") + assert _is_safe_sync_back_target(cfg) is False + + def test_dot_env_rejected(self, tmp_path): + """~/.hermes/.env must be rejected.""" + from hermes_constants import get_hermes_home + env_file = str(get_hermes_home() / ".env") + assert _is_safe_sync_back_target(env_file) is False + + def test_nested_skills_subdir_rejected(self, tmp_path): + """Deep path inside ~/.hermes/skills/ is also rejected.""" + from hermes_constants import get_hermes_home + deep = str( + get_hermes_home() / "skills" / "sub" / "deep" / "a.py" + ) + assert _is_safe_sync_back_target(deep) is False + + +class TestSyncBackSecurityRejectsSkillsDir: + """sync_back must not write inferred new files into skill directories.""" + + def test_new_remote_file_in_skills_dir_is_blocked( + self, tmp_path, caplog + ): + """A new remote file inferred into ~/.hermes/skills/ must be skipped. + + This is the exact attack vector described in bug #38026: a malicious + remote task creates a new file in the remote skills directory; on + teardown sync_back infers the host path and would copy the file into + the host skill tree. The fix must block this. + """ + from hermes_constants import get_hermes_home + + # The file mapping gives _infer_host_path a prefix to match on. + # We deliberately point the host side at the real skills dir so + # that the inferred target lands inside the sensitive directory. + hermes_home = get_hermes_home() + skills_dir = hermes_home / "skills" + + # Use a sentinel existing file in the skills dir as the mapping + # anchor (doesn't need to exist on disk — mapping is synthetic). + anchor_host = str(skills_dir / "existing.py") + anchor_remote = "/root/.hermes/skills/existing.py" + mapping = [(anchor_host, anchor_remote)] + + # Remote tar contains a brand-new skill file + malicious_content = b"import os; os.system('evil')" + download_fn = _make_download_fn({ + "root/.hermes/skills/injected.py": malicious_content, + }) + + mgr = _make_manager( + tmp_path, + file_mapping=mapping, + bulk_download_fn=download_fn, + ) + + target_path = skills_dir / "injected.py" + # Ensure target doesn't exist before the test + if target_path.exists(): + target_path.unlink() + + with caplog.at_level( + logging.WARNING, logger="tools.environments.file_sync" + ): + mgr.sync_back(hermes_home=tmp_path / ".hermes") + + # The injected skill must NOT have been written to the host + assert not target_path.exists(), ( + "Injected skill file was written to host skills dir — " + "security fix not working!" + ) + # A SECURITY warning must have been logged + assert any( + "SECURITY" in r.message for r in caplog.records + ), "Expected a SECURITY warning in logs" + + def test_normal_file_outside_sensitive_dirs_still_synced( + self, tmp_path + ): + """Non-sensitive inferred files must still be applied normally.""" + existing_host = tmp_path / "cache" / "existing.json" + _write_file(existing_host, b"{}") + mapping = [(str(existing_host), "/root/.hermes/cache/existing.json")] + + new_content = b'{"new": true}' + download_fn = _make_download_fn({ + "root/.hermes/cache/new_entry.json": new_content, + }) + + mgr = _make_manager( + tmp_path, + file_mapping=mapping, + bulk_download_fn=download_fn, + ) + mgr.sync_back(hermes_home=tmp_path / ".hermes") + + expected = tmp_path / "cache" / "new_entry.json" + assert expected.exists() + assert expected.read_bytes() == new_content diff --git a/tools/environments/file_sync.py b/tools/environments/file_sync.py index 89f712693fef8..dfdd7e58b14a9 100644 --- a/tools/environments/file_sync.py +++ b/tools/environments/file_sync.py @@ -104,6 +104,87 @@ def _sha256_file(path: str) -> str: _SYNC_BACK_BACKOFF = (2, 4, 8) # seconds between retries _SYNC_BACK_MAX_BYTES = 2 * 1024 * 1024 * 1024 # 2 GiB — refuse to extract larger tars +# --------------------------------------------------------------------------- +# sync_back path-safety helpers +# --------------------------------------------------------------------------- + +def _sensitive_host_dirs() -> list[Path]: + """Return resolved paths that sync_back must NEVER write into. + + Evaluated at sync-back time so get_hermes_home() reflects any + runtime overrides set after module import. + """ + home = get_hermes_home() + candidates = [ + home / "skills", + home / "plugins", + ] + resolved = [] + for p in candidates: + try: + resolved.append(p.resolve()) + except OSError: + resolved.append(p.absolute()) + return resolved + + +def _sensitive_host_files() -> list[Path]: + """Return resolved paths of host files sync_back must not write.""" + home = get_hermes_home() + candidates = [ + home / "config.yaml", + home / ".env", + ] + resolved = [] + for p in candidates: + try: + resolved.append(p.resolve()) + except OSError: + resolved.append(p.absolute()) + return resolved + + +def _is_safe_sync_back_target(host_path: str) -> bool: + """Return True iff *host_path* is safe for sync_back to write. + + Rejects paths that: + + * Contain ``..`` components anywhere (path-traversal attempt). + * Resolve into a sensitive host directory + (``~/.hermes/skills/``, ``~/.hermes/plugins/``). + * Match a sensitive individual file + (``~/.hermes/config.yaml``, ``~/.hermes/.env``). + * Cannot be resolved due to an OS error (treated as unsafe). + """ + # Reject raw ``..`` traversal before any resolution. + try: + raw = Path(host_path) + if any(part == ".." for part in raw.parts): + return False + except Exception: + return False + + try: + resolved = Path(host_path).resolve() + except OSError: + return False + + # Check against sensitive directories (includes symlink-resolved paths). + for sensitive_dir in _sensitive_host_dirs(): + try: + resolved.relative_to(sensitive_dir) + # relative_to() succeeded → inside the sensitive directory. + return False + except ValueError: + pass + + # Check against sensitive individual files. + for sensitive_file in _sensitive_host_files(): + if resolved == sensitive_file: + return False + + return True + class FileSyncManager: """Tracks local file changes and syncs to a remote environment. @@ -355,6 +436,32 @@ def _sync_back_impl(self) -> None: ) continue + # Security: reject sensitive dirs/files and traversal + # attempts (CVE-class: #38026). + if not _is_safe_sync_back_target(host_path): + logger.warning( + "sync_back: SECURITY: refusing to write " + "sensitive host path %s (remote: %s)", + host_path, remote_path, + ) + continue + + # Security: verify the resolved path stays inside the + # inferred parent dir (guards symlink-based escapes). + try: + resolved_target = Path(host_path).resolve() + resolved_parent = Path( + os.path.dirname(host_path) + ).resolve() + resolved_target.relative_to(resolved_parent) + except (ValueError, OSError): + logger.warning( + "sync_back: SECURITY: path %s escapes its " + "mapped directory — skipping (remote: %s)", + host_path, remote_path, + ) + continue + if os.path.exists(host_path) and pushed_hash is not None: host_hash = _sha256_file(host_path) if host_hash != pushed_hash: From 1d2750ffbd50a4476e3da9c38159d759ac50f211 Mon Sep 17 00:00:00 2001 From: ashishpatel26 Date: Thu, 4 Jun 2026 12:32:02 +0530 Subject: [PATCH 3/3] fix(tools): also block config.yml in sync_back sensitive file list (#38026) --- tools/environments/file_sync.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tools/environments/file_sync.py b/tools/environments/file_sync.py index dfdd7e58b14a9..714cac3c80324 100644 --- a/tools/environments/file_sync.py +++ b/tools/environments/file_sync.py @@ -133,6 +133,7 @@ def _sensitive_host_files() -> list[Path]: home = get_hermes_home() candidates = [ home / "config.yaml", + home / "config.yml", home / ".env", ] resolved = []