diff --git a/.githooks/check_hardcoded_paths.py b/.githooks/check_hardcoded_paths.py new file mode 100755 index 000000000..74db9621e --- /dev/null +++ b/.githooks/check_hardcoded_paths.py @@ -0,0 +1,226 @@ +#!/usr/bin/env python3 +""" +Pre-commit hook for detecting hardcoded ~/.hermes paths. + +This is a poka-yoke (error-proofing) measure to prevent profile isolation +failures. All code should use get_hermes_home() from hermes_constants instead +of hardcoding ~/.hermes or Path.home() / ".hermes". + +Installation: + git config core.hooksPath .githooks + +To bypass: + git commit --no-verify +""" + +from __future__ import annotations + +import re +import subprocess +import sys +from pathlib import Path +from typing import Iterable, List + +# ANSI color codes +RED = "\033[0;31m" +YELLOW = "\033[1;33m" +GREEN = "\033[0;32m" +NC = "\033[0m" + + +class Finding: + """Represents a single hardcoded path finding.""" + + def __init__(self, filename: str, line: int, message: str, suggestion: str = "") -> None: + self.filename = filename + self.line = line + self.message = message + self.suggestion = suggestion + + def __repr__(self) -> str: + return f"Finding({self.filename!r}, {self.line}, {self.message!r})" + + +# --------------------------------------------------------------------------- +# Regex patterns for hardcoded paths +# --------------------------------------------------------------------------- + +# Pattern 1: Path.home() / ".hermes" or Path.home() / '.hermes' +_RE_PATH_HOME_HERMES = re.compile( + r"""Path\.home\(\)\s*/\s*['"]\.hermes['"]""" +) + +# Pattern 2: Path.home() / ".hermes" / something +_RE_PATH_HOME_HERMES_SUB = re.compile( + r"""Path\.home\(\)\s*/\s*['"]\.hermes['"]\s*/""" +) + +# Pattern 3: ~/.hermes in strings (but not in comments or docs) +_RE_TILDE_HERMES = re.compile( + r"""['"]~/?\.hermes(/|['"])""" +) + +# Pattern 4: os.path.expanduser("~/.hermes") +_RE_EXPANDUSER_HERMES = re.compile( + r"""os\.path\.expanduser\(\s*['"]~/?\.hermes""" +) + +# Pattern 5: os.path.join(os.path.expanduser("~"), ".hermes") +_RE_JOIN_EXPANDUSER = re.compile( + r"""os\.path\.join\(\s*os\.path\.expanduser\(\s*['"]~['"]\s*\)\s*,\s*['"]\.hermes['"]""" +) + +# All patterns combined +_ALL_PATTERNS = [ + (_RE_PATH_HOME_HERMES, "Path.home() / '.hermes' — use get_hermes_home() instead"), + (_RE_PATH_HOME_HERMES_SUB, "Path.home() / '.hermes' / ... — use get_hermes_home() / '...' instead"), + (_RE_TILDE_HERMES, "'~/.hermes' — use get_hermes_home() for paths, display_hermes_home() for display"), + (_RE_EXPANDUSER_HERMES, "os.path.expanduser('~/.hermes') — use get_hermes_home() instead"), + (_RE_JOIN_EXPANDUSER, "os.path.join(expanduser('~'), '.hermes') — use get_hermes_home() instead"), +] + +# Safe contexts (don't flag these) +_SAFE_CONTEXTS = [ + # hermes_constants.py is allowed (it's the source of truth) + "hermes_constants.py", + # Test files can mock/test the behavior + "test_", + "_test.py", + "/tests/", + # Documentation files + ".md", + "README", + "CHANGELOG", + "AGENTS.md", + # Example/template files + ".example", + "template", +] + + +def _is_safe_context(filename: str) -> bool: + """Check if the file is in a safe context where hardcoded paths are OK.""" + for safe in _SAFE_CONTEXTS: + if safe in filename: + return True + return False + + +def _is_comment_or_doc(line: str) -> bool: + """Check if the line is a comment or documentation.""" + stripped = line.strip() + if stripped.startswith("#"): + return True + if stripped.startswith('"""') or stripped.startswith("'''"): + return True + if '"""' in stripped or "'''" in stripped: + return True + return False + + +def scan_line_for_hardcoded_paths(line: str, filename: str, line_no: int) -> Iterable[Finding]: + """Scan a single line for hardcoded ~/.hermes paths.""" + if _is_safe_context(filename): + return + + stripped = line.rstrip("\n") + if not stripped: + return + + # Skip comments and docstrings + if _is_comment_or_doc(stripped): + return + + for pattern, message in _ALL_PATTERNS: + if pattern.search(stripped): + yield Finding( + filename, + line_no, + message, + "Use get_hermes_home() from hermes_constants for paths, display_hermes_home() for display", + ) + return # One finding per line is enough + + +def get_staged_files() -> List[str]: + """Get list of staged files in the git index.""" + try: + result = subprocess.run( + ["git", "diff", "--cached", "--name-only", "--diff-filter=ACM"], + capture_output=True, + text=True, + check=True, + ) + return [f.strip() for f in result.stdout.splitlines() if f.strip()] + except subprocess.CalledProcessError: + return [] + + +def get_staged_content(filename: str) -> str: + """Get the staged content of a file.""" + try: + result = subprocess.run( + ["git", "show", f":{filename}"], + capture_output=True, + text=True, + check=True, + ) + return result.stdout + except subprocess.CalledProcessError: + return "" + + +def scan_file(filename: str) -> List[Finding]: + """Scan a file for hardcoded ~/.hermes paths.""" + if _is_safe_context(filename): + return [] + + # Only scan Python files + if not filename.endswith(".py"): + return [] + + content = get_staged_content(filename) + if not content: + return [] + + findings = [] + for line_no, line in enumerate(content.splitlines(), start=1): + for finding in scan_line_for_hardcoded_paths(line, filename, line_no): + findings.append(finding) + + return findings + + +def main() -> int: + """Main entry point for the pre-commit hook.""" + staged_files = get_staged_files() + if not staged_files: + return 0 + + all_findings = [] + for filename in staged_files: + findings = scan_file(filename) + all_findings.extend(findings) + + if not all_findings: + return 0 + + # Print findings + print(f"\n{RED}✗ Hardcoded ~/.hermes paths detected:{NC}\n") + for finding in all_findings: + print(f" {YELLOW}{finding.filename}:{finding.line}{NC}") + print(f" {finding.message}") + if finding.suggestion: + print(f" {GREEN}Fix: {finding.suggestion}{NC}") + print() + + print(f"{RED}Found {len(all_findings)} hardcoded path(s).{NC}") + print(f"{YELLOW}Use get_hermes_home() from hermes_constants for paths.{NC}") + print(f"{YELLOW}Use display_hermes_home() for user-facing display.{NC}") + print(f"\n{YELLOW}To bypass: git commit --no-verify{NC}\n") + + return 1 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/.githooks/pre-commit.py b/.githooks/pre-commit.py index a48ade0af..da4156bac 100755 --- a/.githooks/pre-commit.py +++ b/.githooks/pre-commit.py @@ -295,6 +295,22 @@ def main() -> int: if line.startswith("+") and not line.startswith("+++"): findings.extend(scan_line(line[1:], "", line_no)) + # Also check for hardcoded ~/.hermes paths + print(f"{GREEN}🔍 Scanning for hardcoded ~/.hermes paths...{NC}") + try: + import subprocess as sp + result = sp.run( + [sys.executable, str(Path(__file__).parent / "check_hardcoded_paths.py")], + capture_output=True, + text=True, + ) + if result.returncode != 0: + # Print the output from the hardcoded path check + print(result.stdout) + return 1 + except Exception as e: + print(f"{YELLOW}Warning: Could not run hardcoded path check: {e}{NC}") + if not findings: print(f"{GREEN}✓ No potential secret leaks detected{NC}") return 0 diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 120cf01bf..054373f3a 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -12,6 +12,23 @@ concurrency: cancel-in-progress: true jobs: + check-hardcoded-paths: + runs-on: ubuntu-latest + timeout-minutes: 5 + steps: + - name: Checkout code + uses: actions/checkout@v4 + + - name: Set up Python 3.11 + uses: actions/setup-python@v5 + with: + python-version: '3.11' + + - name: Check for hardcoded ~/.hermes paths + run: | + python .githooks/check_hardcoded_paths.py + # This will fail if any hardcoded paths are found + test: runs-on: ubuntu-latest container: catthehacker/ubuntu:act-22.04 diff --git a/tests/test_hardcoded_paths.py b/tests/test_hardcoded_paths.py new file mode 100644 index 000000000..789f33868 --- /dev/null +++ b/tests/test_hardcoded_paths.py @@ -0,0 +1,175 @@ +""" +Tests for hardcoded ~/.hermes path detection (poka-yoke). + +These tests verify that the pre-commit hook correctly detects hardcoded +paths and that the codebase uses get_hermes_home() correctly. +""" + +import os +import tempfile +from pathlib import Path +from unittest.mock import patch + +import pytest + +# Import the scanner +import sys +sys.path.insert(0, str(Path(__file__).parent.parent / ".githooks")) +from check_hardcoded_paths import scan_line_for_hardcoded_paths, Finding + + +class TestHardcodedPathDetection: + """Test the hardcoded path detection logic.""" + + def test_detects_path_home_hermes(self): + """Detect Path.home() / '.hermes' pattern.""" + line = ' home = Path.home() / ".hermes"' + findings = list(scan_line_for_hardcoded_paths(line, "test.py", 1)) + assert len(findings) == 1 + assert "Path.home()" in findings[0].message + + def test_detects_path_home_hermes_subpath(self): + """Detect Path.home() / '.hermes' / 'subdir' pattern.""" + line = ' config_dir = Path.home() / ".hermes" / "config"' + findings = list(scan_line_for_hardcoded_paths(line, "test.py", 1)) + assert len(findings) == 1 + + def test_detects_tilde_hermes_in_string(self): + """Detect '~/.hermes' in string literals.""" + line = ' path = "~/.hermes/config.yaml"' + findings = list(scan_line_for_hardcoded_paths(line, "test.py", 1)) + assert len(findings) == 1 + + def test_detects_expanduser_hermes(self): + """Detect os.path.expanduser('~/.hermes') pattern.""" + line = ' home = os.path.expanduser("~/.hermes")' + findings = list(scan_line_for_hardcoded_paths(line, "test.py", 1)) + assert len(findings) == 1 + + def test_detects_join_expanduser(self): + """Detect os.path.join(expanduser('~'), '.hermes') pattern.""" + line = ' home = os.path.join(os.path.expanduser("~"), ".hermes")' + findings = list(scan_line_for_hardcoded_paths(line, "test.py", 1)) + assert len(findings) == 1 + + def test_ignores_comments(self): + """Ignore hardcoded paths in comments.""" + line = ' # This is ~/.hermes in a comment' + findings = list(scan_line_for_hardcoded_paths(line, "test.py", 1)) + assert len(findings) == 0 + + def test_ignores_docstrings(self): + """Ignore hardcoded paths in docstrings.""" + line = ' """This mentions ~/.hermes in a docstring."""' + findings = list(scan_line_for_hardcoded_paths(line, "test.py", 1)) + assert len(findings) == 0 + + def test_ignores_hermes_constants(self): + """hermes_constants.py is allowed to have hardcoded paths.""" + line = ' return Path.home() / ".hermes"' + findings = list(scan_line_for_hardcoded_paths(line, "hermes_constants.py", 1)) + assert len(findings) == 0 + + def test_ignores_test_files(self): + """Test files can have hardcoded paths for testing.""" + line = ' home = Path.home() / ".hermes"' + findings = list(scan_line_for_hardcoded_paths(line, "test_something.py", 1)) + assert len(findings) == 0 + + def test_ignores_markdown_files(self): + """Markdown files can have hardcoded paths in examples.""" + line = ' home = Path.home() / ".hermes"' + findings = list(scan_line_for_hardcoded_paths(line, "README.md", 1)) + assert len(findings) == 0 + + def test_ignores_empty_lines(self): + """Empty lines should not produce findings.""" + line = "" + findings = list(scan_line_for_hardcoded_paths(line, "test.py", 1)) + assert len(findings) == 0 + + +class TestHermesHomeUsage: + """Test that the codebase uses get_hermes_home() correctly.""" + + def test_hermes_constants_has_get_hermes_home(self): + """hermes_constants.py should export get_hermes_home().""" + from hermes_constants import get_hermes_home + assert callable(get_hermes_home) + + def test_hermes_constants_has_display_hermes_home(self): + """hermes_constants.py should export display_hermes_home().""" + from hermes_constants import display_hermes_home + assert callable(display_hermes_home) + + def test_get_hermes_home_returns_path(self): + """get_hermes_home() should return a Path object.""" + from hermes_constants import get_hermes_home + result = get_hermes_home() + assert isinstance(result, Path) + + def test_get_hermes_home_honors_env_var(self): + """get_hermes_home() should honor HERMES_HOME env var.""" + from hermes_constants import get_hermes_home + + with tempfile.TemporaryDirectory() as tmpdir: + with patch.dict(os.environ, {"HERMES_HOME": tmpdir}): + result = get_hermes_home() + assert result == Path(tmpdir) + + def test_display_hermes_home_returns_string(self): + """display_hermes_home() should return a string.""" + from hermes_constants import display_hermes_home + result = display_hermes_home() + assert isinstance(result, str) + + def test_display_hermes_home_uses_tilde_shorthand(self): + """display_hermes_home() should use ~/ shorthand for home directory.""" + from hermes_constants import display_hermes_home, get_hermes_home + + # If HERMES_HOME is under home directory, should use ~/ + home = get_hermes_home() + if home.is_relative_to(Path.home()): + result = display_hermes_home() + assert result.startswith("~/") + + def test_profile_isolation_with_env_var(self): + """Each profile should have its own HERMES_HOME.""" + from hermes_constants import get_hermes_home + + with tempfile.TemporaryDirectory() as tmpdir1, tempfile.TemporaryDirectory() as tmpdir2: + # Profile 1 + with patch.dict(os.environ, {"HERMES_HOME": tmpdir1}): + home1 = get_hermes_home() + + # Profile 2 + with patch.dict(os.environ, {"HERMES_HOME": tmpdir2}): + home2 = get_hermes_home() + + assert home1 != home2 + assert home1 == Path(tmpdir1) + assert home2 == Path(tmpdir2) + + +class TestPreCommitHookIntegration: + """Integration tests for the pre-commit hook.""" + + def test_hook_script_exists(self): + """The check_hardcoded_paths.py script should exist.""" + hook_path = Path(__file__).parent.parent / ".githooks" / "check_hardcoded_paths.py" + assert hook_path.exists() + + def test_hook_script_is_executable(self): + """The check_hardcoded_paths.py script should be executable.""" + hook_path = Path(__file__).parent.parent / ".githooks" / "check_hardcoded_paths.py" + assert hook_path.stat().st_mode & 0o111 # Check executable bits + + def test_pre_commit_calls_hardcoded_check(self): + """pre-commit.py should call the hardcoded path check.""" + pre_commit_path = Path(__file__).parent.parent / ".githooks" / "pre-commit.py" + content = pre_commit_path.read_text() + assert "check_hardcoded_paths.py" in content + + +if __name__ == "__main__": + pytest.main([__file__, "-v"])