mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-05-30 06:41:51 +00:00
f4953bc648
SubdirectoryHintTracker was scanning directories outside the active working directory, allowing files like ~/.codex/AGENTS.md or ~/.claude/CLAUDE.md to be loaded and injected into the agent context. This causes cross-agent context contamination and instruction mixup. Add _is_ancestor_or_same() helper and a path boundary check in _is_valid_subdir(): only directories within the working directory tree (i.e. path.is_relative_to(working_dir)) are allowed. Also add exist_ok=True to mkdir() calls in new tests to prevent pytest-xdist race conditions when workers share the same tmp_path parent. Tests added: - test_outside_working_dir_rejected: verifies sibling dirs are blocked - test_outside_working_dir_absolute_path_rejected: verifies ~/.codex paths blocked - test_inside_workspace_subdir_allowed: verifies normal subdir access unaffected - test_sibling_repo_not_loaded_via_ancestor_walk: ancestor walk stays within workspace
328 lines
14 KiB
Python
328 lines
14 KiB
Python
"""Tests for progressive subdirectory hint discovery."""
|
|
|
|
import os
|
|
import pytest
|
|
from pathlib import Path
|
|
from unittest.mock import patch
|
|
|
|
from agent.subdirectory_hints import SubdirectoryHintTracker
|
|
|
|
|
|
@pytest.fixture
|
|
def project(tmp_path):
|
|
"""Create a mock project tree with hint files in subdirectories."""
|
|
# Root — already loaded at startup
|
|
(tmp_path / "AGENTS.md").write_text("Root project instructions")
|
|
|
|
# backend/ — has its own AGENTS.md
|
|
backend = tmp_path / "backend"
|
|
backend.mkdir()
|
|
(backend / "AGENTS.md").write_text("Backend-specific instructions:\n- Use FastAPI\n- Always add type hints")
|
|
|
|
# backend/src/ — no hints
|
|
(backend / "src").mkdir()
|
|
(backend / "src" / "main.py").write_text("print('hello')")
|
|
|
|
# frontend/ — has CLAUDE.md
|
|
frontend = tmp_path / "frontend"
|
|
frontend.mkdir()
|
|
(frontend / "CLAUDE.md").write_text("Frontend rules:\n- Use TypeScript\n- No any types")
|
|
|
|
# docs/ — no hints
|
|
(tmp_path / "docs").mkdir()
|
|
(tmp_path / "docs" / "README.md").write_text("Documentation")
|
|
|
|
# deep/nested/path/ — has .cursorrules
|
|
deep = tmp_path / "deep" / "nested" / "path"
|
|
deep.mkdir(parents=True)
|
|
(deep / ".cursorrules").write_text("Cursor rules for nested path")
|
|
|
|
return tmp_path
|
|
|
|
|
|
class TestSubdirectoryHintTracker:
|
|
"""Unit tests for SubdirectoryHintTracker."""
|
|
|
|
def test_working_dir_not_loaded(self, project):
|
|
"""Working dir is pre-marked as loaded (startup handles it)."""
|
|
tracker = SubdirectoryHintTracker(working_dir=str(project))
|
|
# Reading a file in the root should NOT trigger hints
|
|
result = tracker.check_tool_call("read_file", {"path": str(project / "AGENTS.md")})
|
|
assert result is None
|
|
|
|
def test_discovers_agents_md_via_ancestor_walk(self, project):
|
|
"""Reading backend/src/main.py discovers backend/AGENTS.md via ancestor walk."""
|
|
tracker = SubdirectoryHintTracker(working_dir=str(project))
|
|
result = tracker.check_tool_call(
|
|
"read_file", {"path": str(project / "backend" / "src" / "main.py")}
|
|
)
|
|
# backend/src/ has no hints, but ancestor walk finds backend/AGENTS.md
|
|
assert result is not None
|
|
assert "Backend-specific instructions" in result
|
|
# Second read in same subtree should not re-trigger
|
|
result2 = tracker.check_tool_call(
|
|
"read_file", {"path": str(project / "backend" / "AGENTS.md")}
|
|
)
|
|
assert result2 is None # backend/ already loaded
|
|
|
|
def test_discovers_claude_md(self, project):
|
|
"""Frontend CLAUDE.md should be discovered."""
|
|
tracker = SubdirectoryHintTracker(working_dir=str(project))
|
|
result = tracker.check_tool_call(
|
|
"read_file", {"path": str(project / "frontend" / "index.ts")}
|
|
)
|
|
assert result is not None
|
|
assert "Frontend rules" in result
|
|
|
|
def test_no_duplicate_loading(self, project):
|
|
"""Same directory should not be loaded twice."""
|
|
tracker = SubdirectoryHintTracker(working_dir=str(project))
|
|
result1 = tracker.check_tool_call(
|
|
"read_file", {"path": str(project / "frontend" / "a.ts")}
|
|
)
|
|
assert result1 is not None
|
|
|
|
result2 = tracker.check_tool_call(
|
|
"read_file", {"path": str(project / "frontend" / "b.ts")}
|
|
)
|
|
assert result2 is None # already loaded
|
|
|
|
def test_no_hints_in_empty_directory(self, project):
|
|
"""Directories without hint files return None."""
|
|
tracker = SubdirectoryHintTracker(working_dir=str(project))
|
|
result = tracker.check_tool_call(
|
|
"read_file", {"path": str(project / "docs" / "README.md")}
|
|
)
|
|
assert result is None
|
|
|
|
def test_terminal_command_path_extraction(self, project):
|
|
"""Paths extracted from terminal commands."""
|
|
tracker = SubdirectoryHintTracker(working_dir=str(project))
|
|
result = tracker.check_tool_call(
|
|
"terminal", {"command": f"cat {project / 'frontend' / 'index.ts'}"}
|
|
)
|
|
assert result is not None
|
|
assert "Frontend rules" in result
|
|
|
|
def test_terminal_cd_command(self, project):
|
|
"""cd into a directory with hints."""
|
|
tracker = SubdirectoryHintTracker(working_dir=str(project))
|
|
result = tracker.check_tool_call(
|
|
"terminal", {"command": f"cd {project / 'backend'} && ls"}
|
|
)
|
|
assert result is not None
|
|
assert "Backend-specific instructions" in result
|
|
|
|
def test_relative_path(self, project):
|
|
"""Relative paths resolved against working_dir."""
|
|
tracker = SubdirectoryHintTracker(working_dir=str(project))
|
|
result = tracker.check_tool_call(
|
|
"read_file", {"path": "frontend/index.ts"}
|
|
)
|
|
assert result is not None
|
|
assert "Frontend rules" in result
|
|
|
|
def test_outside_working_dir_rejected(self, tmp_path, project):
|
|
"""Paths outside working_dir are rejected — no hints from outside workspace.
|
|
|
|
Note: project fixture returns tmp_path, so we need a path whose ancestor
|
|
is outside project. We simulate this by creating a directory at the same
|
|
level as project but not inside it — which requires creating a parent
|
|
tree. Since tmp_path / "other" IS inside tmp_path (=project), we need
|
|
a different approach: use tmp_path.parent as the reference for "outside".
|
|
"""
|
|
# Create a directory at the same level as tmp_path (project),
|
|
# which means it's a sibling of project — not a child.
|
|
# Since tmp_path IS project, tmp_path.parent / "other" is a sibling.
|
|
parent = tmp_path.parent
|
|
other_project = parent / "other"
|
|
other_project.mkdir(exist_ok=True)
|
|
(other_project / "AGENTS.md").write_text("Other project rules")
|
|
tracker = SubdirectoryHintTracker(working_dir=str(project))
|
|
result = tracker.check_tool_call(
|
|
"read_file", {"path": str(other_project / "file.py")}
|
|
)
|
|
# Outside workspace — should NOT load hints
|
|
assert result is None
|
|
|
|
def test_outside_working_dir_absolute_path_rejected(self, tmp_path, project):
|
|
"""Absolute paths like ~/.codex/AGENTS.md are rejected."""
|
|
# Create a directory at the parent level of project, simulating ~/.codex
|
|
parent = tmp_path.parent
|
|
outside_dir = parent / ".test-codex"
|
|
outside_dir.mkdir(exist_ok=True)
|
|
(outside_dir / "AGENTS.md").write_text("Codex contamination rules")
|
|
tracker = SubdirectoryHintTracker(working_dir=str(project))
|
|
result = tracker.check_tool_call(
|
|
"read_file", {"path": str(outside_dir / "AGENTS.md")}
|
|
)
|
|
# Reading a hint file outside working_dir — should NOT load hints
|
|
assert result is None
|
|
|
|
def test_inside_workspace_subdir_allowed(self, project):
|
|
"""Paths inside working_dir are still allowed."""
|
|
tracker = SubdirectoryHintTracker(working_dir=str(project))
|
|
result = tracker.check_tool_call(
|
|
"read_file", {"path": str(project / "backend" / "src" / "main.py")}
|
|
)
|
|
assert result is not None
|
|
assert "Backend-specific instructions" in result
|
|
|
|
def test_sibling_repo_not_loaded_via_ancestor_walk(self, tmp_path, project):
|
|
"""Ancestor walk from inside working_dir should NOT discover sibling repo hints."""
|
|
# Create a nested structure inside working_dir
|
|
deep_dir = project / "deep" / "nested" / "very" / "deep"
|
|
deep_dir.mkdir(parents=True)
|
|
(deep_dir / "file.py").write_text("deep file")
|
|
# Also create a sibling directory at the parent level
|
|
parent = tmp_path.parent
|
|
sibling = parent / "sibling-repo"
|
|
sibling.mkdir(exist_ok=True)
|
|
(sibling / "AGENTS.md").write_text("Sibling repo rules")
|
|
# Create a .cursorrules in the deep/nested/very dir so ancestor walk
|
|
# discovers it (fixture's deep/nested/path is NOT an ancestor of very/deep)
|
|
(deep_dir / ".cursorrules").write_text("Deep cursorrules")
|
|
tracker = SubdirectoryHintTracker(working_dir=str(project))
|
|
result = tracker.check_tool_call(
|
|
"read_file", {"path": str(deep_dir / "file.py")}
|
|
)
|
|
# Should discover deep cursorrules from the file's own directory
|
|
# but NOT sibling repo hints
|
|
assert result is not None
|
|
assert "Deep cursorrules" in result
|
|
assert "Sibling repo rules" not in result
|
|
|
|
def test_workdir_arg(self, project):
|
|
"""The workdir argument from terminal tool is checked."""
|
|
tracker = SubdirectoryHintTracker(working_dir=str(project))
|
|
result = tracker.check_tool_call(
|
|
"terminal", {"command": "ls", "workdir": str(project / "frontend")}
|
|
)
|
|
assert result is not None
|
|
assert "Frontend rules" in result
|
|
|
|
def test_deeply_nested_cursorrules(self, project):
|
|
"""Deeply nested .cursorrules should be discovered."""
|
|
tracker = SubdirectoryHintTracker(working_dir=str(project))
|
|
result = tracker.check_tool_call(
|
|
"read_file", {"path": str(project / "deep" / "nested" / "path" / "file.py")}
|
|
)
|
|
assert result is not None
|
|
assert "Cursor rules for nested path" in result
|
|
|
|
def test_hint_format_includes_path(self, project):
|
|
"""Discovered hints should indicate which file they came from."""
|
|
tracker = SubdirectoryHintTracker(working_dir=str(project))
|
|
result = tracker.check_tool_call(
|
|
"read_file", {"path": str(project / "backend" / "file.py")}
|
|
)
|
|
assert result is not None
|
|
assert "Subdirectory context discovered:" in result
|
|
assert "AGENTS.md" in result
|
|
|
|
def test_truncation_of_large_hints(self, tmp_path):
|
|
"""Hint files over the limit are truncated."""
|
|
sub = tmp_path / "bigdir"
|
|
sub.mkdir()
|
|
(sub / "AGENTS.md").write_text("x" * 20_000)
|
|
|
|
tracker = SubdirectoryHintTracker(working_dir=str(tmp_path))
|
|
result = tracker.check_tool_call(
|
|
"read_file", {"path": str(sub / "file.py")}
|
|
)
|
|
assert result is not None
|
|
assert "truncated" in result.lower()
|
|
# Should be capped
|
|
assert len(result) < 20_000
|
|
|
|
def test_empty_args(self, project):
|
|
"""Empty args should not crash."""
|
|
tracker = SubdirectoryHintTracker(working_dir=str(project))
|
|
assert tracker.check_tool_call("read_file", {}) is None
|
|
assert tracker.check_tool_call("terminal", {"command": ""}) is None
|
|
|
|
def test_url_in_command_ignored(self, project):
|
|
"""URLs in shell commands should not be treated as paths."""
|
|
tracker = SubdirectoryHintTracker(working_dir=str(project))
|
|
result = tracker.check_tool_call(
|
|
"terminal", {"command": "curl https://example.com/frontend/api"}
|
|
)
|
|
assert result is None
|
|
|
|
|
|
class TestPermissionErrorHandling:
|
|
"""Regression tests for PermissionError in filesystem checks (ref #6214)."""
|
|
|
|
def test_is_valid_subdir_permission_error(self, tmp_path):
|
|
"""_is_valid_subdir should return False when is_dir() raises PermissionError."""
|
|
tracker = SubdirectoryHintTracker(working_dir=str(tmp_path))
|
|
restricted = tmp_path / "restricted"
|
|
restricted.mkdir()
|
|
with patch.object(Path, "is_dir", side_effect=PermissionError("Permission denied")):
|
|
assert tracker._is_valid_subdir(restricted) is False
|
|
|
|
def test_load_hints_permission_error_on_is_file(self, tmp_path):
|
|
"""_load_hints_for_directory should skip files when is_file() raises PermissionError."""
|
|
tracker = SubdirectoryHintTracker(working_dir=str(tmp_path))
|
|
restricted = tmp_path / "restricted"
|
|
restricted.mkdir()
|
|
original_is_file = Path.is_file
|
|
def patched_is_file(self):
|
|
if "restricted" in str(self):
|
|
raise PermissionError("Permission denied")
|
|
return original_is_file(self)
|
|
with patch.object(Path, "is_file", patched_is_file):
|
|
result = tracker._load_hints_for_directory(restricted)
|
|
assert result is None
|
|
|
|
def test_check_tool_call_survives_inaccessible_path(self, project):
|
|
"""Full check_tool_call should not crash when a path is inaccessible."""
|
|
tracker = SubdirectoryHintTracker(working_dir=str(project))
|
|
original_is_dir = Path.is_dir
|
|
def patched_is_dir(self):
|
|
if "backend" in str(self) and "src" not in str(self):
|
|
raise PermissionError("Permission denied")
|
|
return original_is_dir(self)
|
|
with patch.object(Path, "is_dir", patched_is_dir):
|
|
# Should not raise — gracefully skip the inaccessible directory
|
|
result = tracker.check_tool_call(
|
|
"read_file", {"path": str(project / "backend" / "src" / "main.py")}
|
|
)
|
|
# Result may be None (backend skipped) — the key point is no crash
|
|
assert result is None or isinstance(result, str)
|
|
|
|
|
|
class TestOutsideWorkspaceRejection:
|
|
"""Direct tests for _is_valid_subdir rejecting outside-workspace paths."""
|
|
|
|
def test_is_valid_subdir_rejects_outside_path(self, tmp_path, project):
|
|
"""_is_valid_subdir should return False for paths outside working_dir.
|
|
|
|
Note: tmp_path / "other" is inside tmp_path (=project), so we use
|
|
tmp_path.parent / "other" to create a true outside-path sibling.
|
|
"""
|
|
parent = tmp_path.parent
|
|
other_project = parent / "other"
|
|
other_project.mkdir(exist_ok=True)
|
|
tracker = SubdirectoryHintTracker(working_dir=str(project))
|
|
assert tracker._is_valid_subdir(other_project) is False
|
|
|
|
def test_is_valid_subdir_allows_inside_path(self, project):
|
|
"""_is_valid_subdir should return True for paths inside working_dir."""
|
|
tracker = SubdirectoryHintTracker(working_dir=str(project))
|
|
backend = project / "backend"
|
|
assert tracker._is_valid_subdir(backend) is True
|
|
|
|
def test_is_valid_subdir_rejects_parent_dir(self, tmp_path, project):
|
|
"""_is_valid_subdir should reject parent directories outside working_dir."""
|
|
parent = tmp_path.parent
|
|
tracker = SubdirectoryHintTracker(working_dir=str(project))
|
|
assert tracker._is_valid_subdir(parent) is False
|
|
|
|
def test_is_valid_subdir_rejects_sibling_dir(self, tmp_path, project):
|
|
"""_is_valid_subdir should reject a sibling directory (simulating ~/.codex)."""
|
|
parent = tmp_path.parent
|
|
outside = parent / ".test-codex"
|
|
outside.mkdir(exist_ok=True)
|
|
tracker = SubdirectoryHintTracker(working_dir=str(project))
|
|
assert tracker._is_valid_subdir(outside) is False
|