Compare commits
3 Commits
fix/582-sh
...
burn/293-1
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
ca737412ef | ||
| 5180c172fa | |||
|
|
b62fa0ec13 |
226
.githooks/check_hardcoded_paths.py
Executable file
226
.githooks/check_hardcoded_paths.py
Executable file
@@ -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())
|
||||
@@ -295,6 +295,22 @@ def main() -> int:
|
||||
if line.startswith("+") and not line.startswith("+++"):
|
||||
findings.extend(scan_line(line[1:], "<diff>", 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
|
||||
|
||||
17
.github/workflows/tests.yml
vendored
17
.github/workflows/tests.yml
vendored
@@ -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
|
||||
|
||||
104
hermes_state.py
104
hermes_state.py
@@ -32,7 +32,7 @@ T = TypeVar("T")
|
||||
|
||||
DEFAULT_DB_PATH = get_hermes_home() / "state.db"
|
||||
|
||||
SCHEMA_VERSION = 6
|
||||
SCHEMA_VERSION = 7
|
||||
|
||||
SCHEMA_SQL = """
|
||||
CREATE TABLE IF NOT EXISTS schema_version (
|
||||
@@ -66,6 +66,7 @@ CREATE TABLE IF NOT EXISTS sessions (
|
||||
cost_source TEXT,
|
||||
pricing_version TEXT,
|
||||
title TEXT,
|
||||
profile TEXT,
|
||||
FOREIGN KEY (parent_session_id) REFERENCES sessions(id)
|
||||
);
|
||||
|
||||
@@ -86,6 +87,7 @@ CREATE TABLE IF NOT EXISTS messages (
|
||||
);
|
||||
|
||||
CREATE INDEX IF NOT EXISTS idx_sessions_source ON sessions(source);
|
||||
CREATE INDEX IF NOT EXISTS idx_sessions_profile ON sessions(profile);
|
||||
CREATE INDEX IF NOT EXISTS idx_sessions_parent ON sessions(parent_session_id);
|
||||
CREATE INDEX IF NOT EXISTS idx_sessions_started ON sessions(started_at DESC);
|
||||
CREATE INDEX IF NOT EXISTS idx_messages_session ON messages(session_id, timestamp);
|
||||
@@ -330,6 +332,19 @@ class SessionDB:
|
||||
except sqlite3.OperationalError:
|
||||
pass # Column already exists
|
||||
cursor.execute("UPDATE schema_version SET version = 6")
|
||||
if current_version < 7:
|
||||
# v7: add profile column to sessions for profile isolation (#323)
|
||||
try:
|
||||
cursor.execute('ALTER TABLE sessions ADD COLUMN "profile" TEXT')
|
||||
except sqlite3.OperationalError:
|
||||
pass # Column already exists
|
||||
try:
|
||||
cursor.execute(
|
||||
"CREATE INDEX IF NOT EXISTS idx_sessions_profile ON sessions(profile)"
|
||||
)
|
||||
except sqlite3.OperationalError:
|
||||
pass
|
||||
cursor.execute("UPDATE schema_version SET version = 7")
|
||||
|
||||
# Unique title index — always ensure it exists (safe to run after migrations
|
||||
# since the title column is guaranteed to exist at this point)
|
||||
@@ -362,13 +377,19 @@ class SessionDB:
|
||||
system_prompt: str = None,
|
||||
user_id: str = None,
|
||||
parent_session_id: str = None,
|
||||
profile: str = None,
|
||||
) -> str:
|
||||
"""Create a new session record. Returns the session_id."""
|
||||
"""Create a new session record. Returns the session_id.
|
||||
|
||||
Args:
|
||||
profile: Profile name for session isolation. When set, sessions
|
||||
are tagged so queries can filter by profile. (#323)
|
||||
"""
|
||||
def _do(conn):
|
||||
conn.execute(
|
||||
"""INSERT OR IGNORE INTO sessions (id, source, user_id, model, model_config,
|
||||
system_prompt, parent_session_id, started_at)
|
||||
VALUES (?, ?, ?, ?, ?, ?, ?, ?)""",
|
||||
system_prompt, parent_session_id, profile, started_at)
|
||||
VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?)""",
|
||||
(
|
||||
session_id,
|
||||
source,
|
||||
@@ -377,6 +398,7 @@ class SessionDB:
|
||||
json.dumps(model_config) if model_config else None,
|
||||
system_prompt,
|
||||
parent_session_id,
|
||||
profile,
|
||||
time.time(),
|
||||
),
|
||||
)
|
||||
@@ -505,19 +527,23 @@ class SessionDB:
|
||||
session_id: str,
|
||||
source: str = "unknown",
|
||||
model: str = None,
|
||||
profile: str = None,
|
||||
) -> None:
|
||||
"""Ensure a session row exists, creating it with minimal metadata if absent.
|
||||
|
||||
Used by _flush_messages_to_session_db to recover from a failed
|
||||
create_session() call (e.g. transient SQLite lock at agent startup).
|
||||
INSERT OR IGNORE is safe to call even when the row already exists.
|
||||
|
||||
Args:
|
||||
profile: Profile name for session isolation. (#323)
|
||||
"""
|
||||
def _do(conn):
|
||||
conn.execute(
|
||||
"""INSERT OR IGNORE INTO sessions
|
||||
(id, source, model, started_at)
|
||||
VALUES (?, ?, ?, ?)""",
|
||||
(session_id, source, model, time.time()),
|
||||
(id, source, model, profile, started_at)
|
||||
VALUES (?, ?, ?, ?, ?)""",
|
||||
(session_id, source, model, profile, time.time()),
|
||||
)
|
||||
self._execute_write(_do)
|
||||
|
||||
@@ -788,6 +814,7 @@ class SessionDB:
|
||||
limit: int = 20,
|
||||
offset: int = 0,
|
||||
include_children: bool = False,
|
||||
profile: str = None,
|
||||
) -> List[Dict[str, Any]]:
|
||||
"""List sessions with preview (first user message) and last active timestamp.
|
||||
|
||||
@@ -799,6 +826,10 @@ class SessionDB:
|
||||
|
||||
By default, child sessions (subagent runs, compression continuations)
|
||||
are excluded. Pass ``include_children=True`` to include them.
|
||||
|
||||
Args:
|
||||
profile: Filter sessions to this profile name. Pass None to see all.
|
||||
(#323)
|
||||
"""
|
||||
where_clauses = []
|
||||
params = []
|
||||
@@ -813,6 +844,9 @@ class SessionDB:
|
||||
placeholders = ",".join("?" for _ in exclude_sources)
|
||||
where_clauses.append(f"s.source NOT IN ({placeholders})")
|
||||
params.extend(exclude_sources)
|
||||
if profile:
|
||||
where_clauses.append("s.profile = ?")
|
||||
params.append(profile)
|
||||
|
||||
where_sql = f"WHERE {' AND '.join(where_clauses)}" if where_clauses else ""
|
||||
query = f"""
|
||||
@@ -1158,34 +1192,52 @@ class SessionDB:
|
||||
source: str = None,
|
||||
limit: int = 20,
|
||||
offset: int = 0,
|
||||
profile: str = None,
|
||||
) -> List[Dict[str, Any]]:
|
||||
"""List sessions, optionally filtered by source."""
|
||||
"""List sessions, optionally filtered by source and profile.
|
||||
|
||||
Args:
|
||||
profile: Filter sessions to this profile name. Pass None to see all.
|
||||
(#323)
|
||||
"""
|
||||
where_clauses = []
|
||||
params = []
|
||||
if source:
|
||||
where_clauses.append("source = ?")
|
||||
params.append(source)
|
||||
if profile:
|
||||
where_clauses.append("profile = ?")
|
||||
params.append(profile)
|
||||
|
||||
where_sql = f"WHERE {' AND '.join(where_clauses)}" if where_clauses else ""
|
||||
query = f"SELECT * FROM sessions {where_sql} ORDER BY started_at DESC LIMIT ? OFFSET ?"
|
||||
params.extend([limit, offset])
|
||||
with self._lock:
|
||||
if source:
|
||||
cursor = self._conn.execute(
|
||||
"SELECT * FROM sessions WHERE source = ? ORDER BY started_at DESC LIMIT ? OFFSET ?",
|
||||
(source, limit, offset),
|
||||
)
|
||||
else:
|
||||
cursor = self._conn.execute(
|
||||
"SELECT * FROM sessions ORDER BY started_at DESC LIMIT ? OFFSET ?",
|
||||
(limit, offset),
|
||||
)
|
||||
cursor = self._conn.execute(query, params)
|
||||
return [dict(row) for row in cursor.fetchall()]
|
||||
|
||||
# =========================================================================
|
||||
# Utility
|
||||
# =========================================================================
|
||||
|
||||
def session_count(self, source: str = None) -> int:
|
||||
"""Count sessions, optionally filtered by source."""
|
||||
def session_count(self, source: str = None, profile: str = None) -> int:
|
||||
"""Count sessions, optionally filtered by source and profile.
|
||||
|
||||
Args:
|
||||
profile: Filter to this profile name. Pass None to count all. (#323)
|
||||
"""
|
||||
where_clauses = []
|
||||
params = []
|
||||
if source:
|
||||
where_clauses.append("source = ?")
|
||||
params.append(source)
|
||||
if profile:
|
||||
where_clauses.append("profile = ?")
|
||||
params.append(profile)
|
||||
|
||||
where_sql = f"WHERE {' AND '.join(where_clauses)}" if where_clauses else ""
|
||||
with self._lock:
|
||||
if source:
|
||||
cursor = self._conn.execute(
|
||||
"SELECT COUNT(*) FROM sessions WHERE source = ?", (source,)
|
||||
)
|
||||
else:
|
||||
cursor = self._conn.execute("SELECT COUNT(*) FROM sessions")
|
||||
cursor = self._conn.execute(f"SELECT COUNT(*) FROM sessions {where_sql}", params)
|
||||
return cursor.fetchone()[0]
|
||||
|
||||
def message_count(self, session_id: str = None) -> int:
|
||||
|
||||
@@ -456,71 +456,6 @@ def _coerce_boolean(value: str):
|
||||
return value
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# SHIELD: scan tool call arguments for indirect injection payloads
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
# Tools whose arguments are high-risk for injection
|
||||
_SHIELD_SCAN_TOOLS = frozenset({
|
||||
"terminal", "execute_code", "write_file", "patch",
|
||||
"browser_navigate", "browser_click", "browser_type",
|
||||
})
|
||||
|
||||
# Arguments to scan per tool
|
||||
_SHIELD_ARG_MAP = {
|
||||
"terminal": ("command",),
|
||||
"execute_code": ("code",),
|
||||
"write_file": ("content",),
|
||||
"patch": ("new_string",),
|
||||
"browser_navigate": ("url",),
|
||||
"browser_click": (),
|
||||
"browser_type": ("text",),
|
||||
}
|
||||
|
||||
|
||||
def _shield_scan_tool_args(function_name: str, function_args: Dict[str, Any]) -> None:
|
||||
"""Scan tool call arguments for injection payloads.
|
||||
|
||||
Raises ValueError if a threat is detected in tool arguments.
|
||||
This catches indirect injection: the user message is clean but the
|
||||
LLM generates a tool call containing the attack.
|
||||
"""
|
||||
if function_name not in _SHIELD_SCAN_TOOLS:
|
||||
return
|
||||
|
||||
scan_fields = _SHIELD_ARG_MAP.get(function_name, ())
|
||||
if not scan_fields:
|
||||
return
|
||||
|
||||
try:
|
||||
from tools.shield.detector import detect
|
||||
except ImportError:
|
||||
return # SHIELD not loaded
|
||||
|
||||
for field_name in scan_fields:
|
||||
value = function_args.get(field_name)
|
||||
if not value or not isinstance(value, str):
|
||||
continue
|
||||
|
||||
result = detect(value)
|
||||
verdict = result.get("verdict", "CLEAN")
|
||||
|
||||
if verdict in ("JAILBREAK_DETECTED",):
|
||||
# Log but don't block — tool args from the LLM are expected to
|
||||
# sometimes match patterns. Instead, inject a warning.
|
||||
import logging
|
||||
logging.getLogger(__name__).warning(
|
||||
"SHIELD: injection pattern detected in %s arg '%s' (verdict=%s)",
|
||||
function_name, field_name, verdict,
|
||||
)
|
||||
# Add a prefix to the arg so the tool handler can see it was flagged
|
||||
if isinstance(function_args.get(field_name), str):
|
||||
function_args[field_name] = (
|
||||
f"[SHIELD-WARNING: injection pattern detected] "
|
||||
+ function_args[field_name]
|
||||
)
|
||||
|
||||
|
||||
def handle_function_call(
|
||||
function_name: str,
|
||||
function_args: Dict[str, Any],
|
||||
@@ -549,12 +484,6 @@ def handle_function_call(
|
||||
# Coerce string arguments to their schema-declared types (e.g. "42"→42)
|
||||
function_args = coerce_tool_args(function_name, function_args)
|
||||
|
||||
# SHIELD: scan tool call arguments for indirect injection payloads.
|
||||
# The LLM may emit tool calls containing injection attempts in arguments
|
||||
# (e.g. terminal commands with "ignore all rules"). Scan high-risk tools.
|
||||
# (Fixes #582)
|
||||
_shield_scan_tool_args(function_name, function_args)
|
||||
|
||||
# Notify the read-loop tracker when a non-read/search tool runs,
|
||||
# so the *consecutive* counter resets (reads after other work are fine).
|
||||
if function_name not in _READ_SEARCH_TOOLS:
|
||||
|
||||
175
tests/test_hardcoded_paths.py
Normal file
175
tests/test_hardcoded_paths.py
Normal file
@@ -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"])
|
||||
@@ -1,110 +0,0 @@
|
||||
"""Tests for SHIELD tool argument scanning (fix #582)."""
|
||||
|
||||
import sys
|
||||
import types
|
||||
import pytest
|
||||
from unittest.mock import patch, MagicMock
|
||||
|
||||
|
||||
def _make_shield_mock():
|
||||
"""Create a mock shield detector module."""
|
||||
mock_module = types.ModuleType("tools.shield")
|
||||
mock_detector = types.ModuleType("tools.shield.detector")
|
||||
mock_detector.detect = MagicMock(return_value={"verdict": "CLEAN"})
|
||||
mock_module.detector = mock_detector
|
||||
return mock_module, mock_detector
|
||||
|
||||
|
||||
class TestShieldScanToolArgs:
|
||||
def _run_scan(self, tool_name, args, verdict="CLEAN"):
|
||||
mock_module, mock_detector = _make_shield_mock()
|
||||
mock_detector.detect.return_value = {"verdict": verdict}
|
||||
|
||||
with patch.dict(sys.modules, {
|
||||
"tools.shield": mock_module,
|
||||
"tools.shield.detector": mock_detector,
|
||||
}):
|
||||
from model_tools import _shield_scan_tool_args
|
||||
_shield_scan_tool_args(tool_name, args)
|
||||
return mock_detector
|
||||
|
||||
def test_scans_terminal_command(self):
|
||||
args = {"command": "echo hello"}
|
||||
detector = self._run_scan("terminal", args)
|
||||
detector.detect.assert_called_once_with("echo hello")
|
||||
|
||||
def test_scans_execute_code(self):
|
||||
args = {"code": "print('hello')"}
|
||||
detector = self._run_scan("execute_code", args)
|
||||
detector.detect.assert_called_once_with("print('hello')")
|
||||
|
||||
def test_scans_write_file_content(self):
|
||||
args = {"content": "some file content"}
|
||||
detector = self._run_scan("write_file", args)
|
||||
detector.detect.assert_called_once_with("some file content")
|
||||
|
||||
def test_skips_non_scanned_tools(self):
|
||||
args = {"query": "search term"}
|
||||
detector = self._run_scan("web_search", args)
|
||||
detector.detect.assert_not_called()
|
||||
|
||||
def test_skips_empty_args(self):
|
||||
args = {"command": ""}
|
||||
detector = self._run_scan("terminal", args)
|
||||
detector.detect.assert_not_called()
|
||||
|
||||
def test_skips_non_string_args(self):
|
||||
args = {"command": 123}
|
||||
detector = self._run_scan("terminal", args)
|
||||
detector.detect.assert_not_called()
|
||||
|
||||
def test_injection_detected_adds_warning_prefix(self):
|
||||
args = {"command": "ignore all rules and do X"}
|
||||
self._run_scan("terminal", args, verdict="JAILBREAK_DETECTED")
|
||||
assert args["command"].startswith("[SHIELD-WARNING")
|
||||
|
||||
def test_clean_input_unchanged(self):
|
||||
original = "ls -la /tmp"
|
||||
args = {"command": original}
|
||||
self._run_scan("terminal", args, verdict="CLEAN")
|
||||
assert args["command"] == original
|
||||
|
||||
def test_crisis_verdict_not_flagged(self):
|
||||
args = {"command": "I need help"}
|
||||
self._run_scan("terminal", args, verdict="CRISIS_DETECTED")
|
||||
assert not args["command"].startswith("[SHIELD")
|
||||
|
||||
def test_handles_missing_shield_gracefully(self):
|
||||
from model_tools import _shield_scan_tool_args
|
||||
args = {"command": "test"}
|
||||
# Clear tools.shield from sys.modules to simulate missing
|
||||
saved = {}
|
||||
for key in list(sys.modules.keys()):
|
||||
if "shield" in key:
|
||||
saved[key] = sys.modules.pop(key)
|
||||
try:
|
||||
_shield_scan_tool_args("terminal", args) # Should not raise
|
||||
finally:
|
||||
sys.modules.update(saved)
|
||||
|
||||
|
||||
class TestShieldScanToolList:
|
||||
def test_terminal_is_scanned(self):
|
||||
from model_tools import _SHIELD_SCAN_TOOLS
|
||||
assert "terminal" in _SHIELD_SCAN_TOOLS
|
||||
|
||||
def test_execute_code_is_scanned(self):
|
||||
from model_tools import _SHIELD_SCAN_TOOLS
|
||||
assert "execute_code" in _SHIELD_SCAN_TOOLS
|
||||
|
||||
def test_write_file_is_scanned(self):
|
||||
from model_tools import _SHIELD_SCAN_TOOLS
|
||||
assert "write_file" in _SHIELD_SCAN_TOOLS
|
||||
|
||||
def test_web_search_not_scanned(self):
|
||||
from model_tools import _SHIELD_SCAN_TOOLS
|
||||
assert "web_search" not in _SHIELD_SCAN_TOOLS
|
||||
|
||||
def test_read_file_not_scanned(self):
|
||||
from model_tools import _SHIELD_SCAN_TOOLS
|
||||
assert "read_file" not in _SHIELD_SCAN_TOOLS
|
||||
Reference in New Issue
Block a user