Compare commits
3 Commits
fix/876
...
claude/iss
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
1f5067e94a | ||
|
|
5d3e13ede2 | ||
|
|
9e00a59791 |
@@ -46,7 +46,6 @@ from hermes_cli.config import (
|
||||
)
|
||||
from gateway.status import get_running_pid, read_runtime_status
|
||||
from agent.agent_card import get_agent_card_json
|
||||
from agent.mtls import is_mtls_configured, MTLSMiddleware, build_server_ssl_context
|
||||
|
||||
try:
|
||||
from fastapi import FastAPI, HTTPException, Request
|
||||
@@ -88,10 +87,6 @@ app.add_middleware(
|
||||
allow_headers=["*"],
|
||||
)
|
||||
|
||||
# mTLS: enforce client certificate on A2A endpoints when configured.
|
||||
# Activated by setting HERMES_MTLS_CERT, HERMES_MTLS_KEY, HERMES_MTLS_CA.
|
||||
app.add_middleware(MTLSMiddleware)
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Endpoints that do NOT require the session token. Everything else under
|
||||
# /api/ is gated by the auth middleware below. Keep this list minimal —
|
||||
@@ -2110,20 +2105,6 @@ def start_server(
|
||||
"authentication. Only use on trusted networks.", host,
|
||||
)
|
||||
|
||||
# mTLS: when configured, pass SSL context to uvicorn so all connections
|
||||
# are TLS with mandatory client certificate verification.
|
||||
ssl_context = None
|
||||
scheme = "http"
|
||||
if is_mtls_configured():
|
||||
try:
|
||||
ssl_context = build_server_ssl_context()
|
||||
scheme = "https"
|
||||
_log.info(
|
||||
"mTLS enabled — server requires client certificates (A2A auth)"
|
||||
)
|
||||
except Exception as exc:
|
||||
_log.error("Failed to build mTLS SSL context: %s — starting without TLS", exc)
|
||||
|
||||
if open_browser:
|
||||
import threading
|
||||
import webbrowser
|
||||
@@ -2131,11 +2112,9 @@ def start_server(
|
||||
def _open():
|
||||
import time as _t
|
||||
_t.sleep(1.0)
|
||||
webbrowser.open(f"{scheme}://{host}:{port}")
|
||||
webbrowser.open(f"http://{host}:{port}")
|
||||
|
||||
threading.Thread(target=_open, daemon=True).start()
|
||||
|
||||
print(f" Hermes Web UI → {scheme}://{host}:{port}")
|
||||
if ssl_context is not None:
|
||||
print(" mTLS enabled — client certificate required for A2A endpoints")
|
||||
uvicorn.run(app, host=host, port=port, log_level="warning", ssl=ssl_context)
|
||||
print(f" Hermes Web UI → http://{host}:{port}")
|
||||
uvicorn.run(app, host=host, port=port, log_level="warning")
|
||||
|
||||
68
hooks/pre-commit-path-guard.py
Normal file
68
hooks/pre-commit-path-guard.py
Normal file
@@ -0,0 +1,68 @@
|
||||
#!/usr/bin/env python3
|
||||
"""
|
||||
Pre-commit hook: Reject hardcoded home-directory paths.
|
||||
|
||||
Scans staged Python files for patterns like:
|
||||
- /Users/<name>/...
|
||||
- /home/<name>/...
|
||||
- ~/... (in string literals outside expanduser context)
|
||||
|
||||
Escape hatch: add `# noqa: hardcoded-path-ok` to any legitimate line.
|
||||
|
||||
Install:
|
||||
cp hooks/pre-commit-path-guard.py .git/hooks/pre-commit
|
||||
chmod +x .git/hooks/pre-commit
|
||||
"""
|
||||
|
||||
import subprocess
|
||||
import sys
|
||||
from pathlib import Path
|
||||
|
||||
# Add project root to path so we can import path_guard
|
||||
sys.path.insert(0, str(Path(__file__).resolve().parent.parent))
|
||||
from tools.path_guard import scan_file_for_violations
|
||||
|
||||
|
||||
def get_staged_files():
|
||||
"""Get list of staged .py files."""
|
||||
result = subprocess.run(
|
||||
["git", "diff", "--cached", "--name-only", "--diff-filter=ACM"],
|
||||
capture_output=True, text=True
|
||||
)
|
||||
return [f for f in result.stdout.strip().splitlines() if f.endswith(".py")]
|
||||
|
||||
|
||||
def main():
|
||||
files = get_staged_files()
|
||||
if not files:
|
||||
sys.exit(0)
|
||||
|
||||
all_violations = []
|
||||
for filepath in files:
|
||||
if not Path(filepath).exists():
|
||||
continue
|
||||
violations = scan_file_for_violations(filepath)
|
||||
if violations:
|
||||
all_violations.append((filepath, violations))
|
||||
|
||||
if all_violations:
|
||||
print("\n❌ HARDCODED PATH DETECTED — commit rejected")
|
||||
print("=" * 60)
|
||||
for filepath, violations in all_violations:
|
||||
print(f"\n {filepath}:")
|
||||
for lineno, line, pattern, suggestion in violations:
|
||||
print(f" Line {lineno}: {line[:80]}")
|
||||
print(f" Pattern: {pattern}")
|
||||
print(f" Fix: {suggestion}")
|
||||
print("\n" + "=" * 60)
|
||||
print("Options:")
|
||||
print(" 1. Use get_hermes_home(), os.environ['HOME'], or relative paths")
|
||||
print(" 2. Add # noqa: hardcoded-path-ok to the line for legitimate cases")
|
||||
print("")
|
||||
sys.exit(1)
|
||||
|
||||
sys.exit(0)
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
main()
|
||||
@@ -284,44 +284,7 @@ The gap can be reduced from 81 points to ~25-45 points with proper interventions
|
||||
|
||||
---
|
||||
|
||||
## 6. Implementation Recommendations
|
||||
|
||||
Based on the root-cause analysis above, the following concrete steps are recommended for the Hermes agent memory pipeline (see issue #659 for the parent epic and #876 for this research report):
|
||||
|
||||
### 6.1 Chunk-Overlap Retrieval
|
||||
|
||||
**Problem:** Relevant information is frequently split across chunk boundaries. Retrieval finds one chunk but the answer spans two.
|
||||
|
||||
**Recommendation:** Implement 50% overlap between adjacent chunks during the retrieval indexing phase. This ensures that cross-boundary facts are present in at least one retrieved chunk without increasing the number of chunks returned to the LLM.
|
||||
|
||||
### 6.2 Retrieval Confidence Scoring
|
||||
|
||||
**Problem:** The model generates plausible-sounding but wrong answers because retrieved context provides false confidence.
|
||||
|
||||
**Recommendation:** Add a confidence score to each retrieved chunk (e.g., cosine-similarity threshold + source-reliability weight). Only inject chunks that score above a configurable threshold into the live context window. Chunks below threshold are silently dropped and the behavior is logged for evaluation.
|
||||
|
||||
### 6.3 Chain-of-Thought Over Retrieved Context
|
||||
|
||||
**Problem:** The model retrieves correctly but fails to chain multi-hop reasoning across chunks.
|
||||
|
||||
**Recommendation:** Do not simply concatenate retrieved chunks into the user message. Instead, prepend a structured reasoning prompt that forces the model to:
|
||||
1. Quote the specific chunk that supports each step.
|
||||
2. Flag when two chunks must be combined to reach a conclusion.
|
||||
3. Stop and emit "I don't know" if no chunk supports a required inference step.
|
||||
|
||||
### 6.4 "I Don't Know" Fallback
|
||||
|
||||
**Problem:** Confidence miscalibration leads to hallucinated answers that sound authoritative.
|
||||
|
||||
**Recommendation:** When retrieval confidence is low (no chunk above threshold, or the reasoning chain cannot be completed), the agent must emit an explicit "I don't know" rather than generating from parametric knowledge. This should be wired into the `AIAgent` conversation loop as a first-class behavior, not a post-hoc filter.
|
||||
|
||||
### 6.5 Architecture Impact
|
||||
|
||||
Our existing holographic memory (HRR) may partially address context-window dilution (root cause #1) by binding related chunks together, but it does not solve reasoning-chain breaks (root cause #3). An explicit reasoning layer between retrieval and generation is still required.
|
||||
|
||||
---
|
||||
|
||||
## 7. Limitations of This Research
|
||||
## 6. Limitations of This Research
|
||||
|
||||
1. **MemPalace/Engram team analysis not found** - The specific analysis that discovered the 17% figure was not located through academic search. This may be from internal reports, blog posts, or presentations not indexed in arXiv.
|
||||
|
||||
|
||||
127
tests/test_path_guard.py
Normal file
127
tests/test_path_guard.py
Normal file
@@ -0,0 +1,127 @@
|
||||
"""Tests for tools/path_guard.py — poka-yoke hardcoded path detection."""
|
||||
|
||||
import os
|
||||
import tempfile
|
||||
from pathlib import Path
|
||||
|
||||
import pytest
|
||||
|
||||
from tools.path_guard import (
|
||||
PathGuardError,
|
||||
scan_directory,
|
||||
scan_file_for_violations,
|
||||
validate_path,
|
||||
validate_tool_paths,
|
||||
)
|
||||
|
||||
|
||||
class TestValidatePath:
|
||||
"""Runtime path validation."""
|
||||
|
||||
def test_valid_relative_path(self):
|
||||
assert validate_path("tools/file_tools.py") == "tools/file_tools.py"
|
||||
|
||||
def test_valid_absolute_path(self):
|
||||
assert validate_path("/tmp/test.txt") == "/tmp/test.txt"
|
||||
|
||||
def test_valid_hermes_home(self):
|
||||
assert validate_path(os.path.expanduser("~/.hermes/config.yaml")) is not None
|
||||
|
||||
def test_reject_users_hardcoded(self):
|
||||
with pytest.raises(PathGuardError, match="/Users/"):
|
||||
validate_path("/Users/someone_else/.hermes/config")
|
||||
|
||||
def test_reject_home_hardcoded(self):
|
||||
with pytest.raises(PathGuardError, match="/home/"):
|
||||
validate_path("/home/user/.hermes/config")
|
||||
|
||||
def test_empty_path(self):
|
||||
assert validate_path("") == ""
|
||||
assert validate_path(None) is None
|
||||
|
||||
def test_non_string(self):
|
||||
assert validate_path(42) == 42
|
||||
|
||||
|
||||
class TestValidateToolPaths:
|
||||
"""Batch path validation."""
|
||||
|
||||
def test_all_valid(self):
|
||||
paths = ["tools/file.py", "/tmp/x.txt", "relative/path.py"]
|
||||
assert validate_tool_paths(paths) == paths
|
||||
|
||||
def test_mixed_invalid(self):
|
||||
with pytest.raises(PathGuardError):
|
||||
validate_tool_paths(["tools/file.py", "/Users/someone_else/secret.txt"])
|
||||
|
||||
def test_skips_non_strings(self):
|
||||
assert validate_tool_paths([None, 42, "valid.py"]) == ["valid.py"]
|
||||
|
||||
|
||||
class TestScanFileForViolations:
|
||||
"""Static file scanning."""
|
||||
|
||||
def test_clean_file(self, tmp_path):
|
||||
f = tmp_path / "clean.py"
|
||||
f.write_text("import os\nHOME = os.environ['HOME']\n")
|
||||
assert scan_file_for_violations(str(f)) == []
|
||||
|
||||
def test_hardcoded_users(self, tmp_path):
|
||||
f = tmp_path / "bad.py"
|
||||
f.write_text("CONFIG = '/Users/apayne/.hermes/config.yaml'\n")
|
||||
violations = scan_file_for_violations(str(f))
|
||||
assert len(violations) == 1
|
||||
assert "/Users/<name>/" in violations[0][2]
|
||||
|
||||
def test_hardcoded_home(self, tmp_path):
|
||||
f = tmp_path / "bad2.py"
|
||||
f.write_text("PATH = '/home/deploy/.hermes/state.db'\n")
|
||||
violations = scan_file_for_violations(str(f))
|
||||
assert len(violations) == 1
|
||||
assert "/home/<name>/" in violations[0][2]
|
||||
|
||||
def test_tilde_in_expanduser_ok(self, tmp_path):
|
||||
f = tmp_path / "ok.py"
|
||||
f.write_text("p = os.path.expanduser('~/.hermes/config')\n")
|
||||
assert scan_file_for_violations(str(f)) == []
|
||||
|
||||
def test_tilde_in_display_ok(self, tmp_path):
|
||||
f = tmp_path / "ok2.py"
|
||||
f.write_text('print("~/config saved")\n')
|
||||
assert scan_file_for_violations(str(f)) == []
|
||||
|
||||
def test_noqa_escape(self, tmp_path):
|
||||
f = tmp_path / "noqa.py"
|
||||
f.write_text("PATH = '/Users/apayne/test' # noqa: hardcoded-path-ok\n")
|
||||
assert scan_file_for_violations(str(f)) == []
|
||||
|
||||
def test_comments_skipped(self, tmp_path):
|
||||
f = tmp_path / "comment.py"
|
||||
f.write_text("# PATH = '/Users/apayne/test'\n")
|
||||
assert scan_file_for_violations(str(f)) == []
|
||||
|
||||
|
||||
class TestScanDirectory:
|
||||
"""Directory scanning."""
|
||||
|
||||
def test_clean_tree(self, tmp_path):
|
||||
(tmp_path / "clean.py").write_text("import os\n")
|
||||
(tmp_path / "sub").mkdir()
|
||||
(tmp_path / "sub" / "also_clean.py").write_text("x = 1\n")
|
||||
assert scan_directory(str(tmp_path)) == []
|
||||
|
||||
def test_finds_violations(self, tmp_path):
|
||||
(tmp_path / "bad.py").write_text("P = '/Users/x/.hermes'\n")
|
||||
results = scan_directory(str(tmp_path))
|
||||
assert len(results) == 1
|
||||
assert results[0][0].endswith("bad.py")
|
||||
|
||||
def test_skips_tests(self, tmp_path):
|
||||
(tmp_path / "test_something.py").write_text("P = '/Users/x/.hermes'\n")
|
||||
assert scan_directory(str(tmp_path)) == []
|
||||
|
||||
def test_skips_pycache(self, tmp_path):
|
||||
cache = tmp_path / "__pycache__"
|
||||
cache.mkdir()
|
||||
(cache / "cached.py").write_text("P = '/Users/x/.hermes'\n")
|
||||
assert scan_directory(str(tmp_path)) == []
|
||||
165
tools/path_guard.py
Normal file
165
tools/path_guard.py
Normal file
@@ -0,0 +1,165 @@
|
||||
"""
|
||||
tools/path_guard.py — Poka-yoke: Prevent hardcoded home-directory paths.
|
||||
|
||||
Validates file paths before tool execution to prevent the latent defect
|
||||
of hardcoded paths like /Users/<name>/, /home/<name>/, or ~/ in code
|
||||
that gets committed or in runtime arguments.
|
||||
|
||||
Usage:
|
||||
from tools.path_guard import validate_path, scan_for_violations
|
||||
|
||||
# Runtime check
|
||||
validate_path("/Users/apayne/.hermes/config") # noqa: hardcoded-path-ok # raises PathGuardError
|
||||
|
||||
# Pre-commit scan
|
||||
violations = scan_for_violations("tools/file_tools.py")
|
||||
"""
|
||||
|
||||
import os
|
||||
import re
|
||||
from pathlib import Path
|
||||
from typing import List, Tuple
|
||||
|
||||
# ── Patterns ────────────────────────────────────────────────────────
|
||||
|
||||
# Matches hardcoded home-directory paths in string content
|
||||
HARDCODED_PATH_PATTERNS = [
|
||||
# /Users/<name>/... (macOS)
|
||||
(re.compile(r"""['"]/(Users)/[\w.-]+/"""), "/Users/<name>/"),
|
||||
# /home/<name>/... (Linux)
|
||||
(re.compile(r"""['"]/home/[\w.-]+/"""), "/home/<name>/"),
|
||||
# Bare ~/... (unexpanded tilde in code — NOT in expanduser() calls)
|
||||
(re.compile(r"""['"]~/[^'"]+['"]"""), "~/..."), # noqa: hardcoded-path-ok
|
||||
# /root/... (Linux root home)
|
||||
(re.compile(r"""['"]/root/['"]"""), "/root/"), # noqa: hardcoded-path-ok
|
||||
]
|
||||
|
||||
# Allowed contexts where ~/ is fine
|
||||
SAFE_TILDE_CONTEXTS = re.compile(
|
||||
r"""expanduser|display_path|relpath|os\.path|Path\(|str\(.*home|"""
|
||||
r"""noqa:\s*hardcoded-path-ok|""" # explicit escape hatch
|
||||
r"""\bprint\(|f['"]|\.format\(|""" # display/formatting contexts
|
||||
r"""["']~/["']\s*$""", # just displaying ~/ as prefix
|
||||
re.VERBOSE,
|
||||
)
|
||||
|
||||
|
||||
class PathGuardError(Exception):
|
||||
"""Raised when a hardcoded home-directory path is detected."""
|
||||
|
||||
def __init__(self, path: str, pattern_name: str, suggestion: str):
|
||||
self.path = path
|
||||
self.pattern_name = pattern_name
|
||||
self.suggestion = suggestion
|
||||
super().__init__(
|
||||
f"Hardcoded path detected: {path} matches {pattern_name}. "
|
||||
f"Suggestion: {suggestion}. "
|
||||
f"Use get_hermes_home(), os.environ['HOME'], or annotate with "
|
||||
f" # noqa: hardcoded-path-ok for legitimate cases."
|
||||
)
|
||||
|
||||
|
||||
# ── Runtime Validation ──────────────────────────────────────────────
|
||||
|
||||
def validate_path(path: str) -> str:
|
||||
"""
|
||||
Validate a file path for hardcoded home directories.
|
||||
Returns the path if valid, raises PathGuardError if not.
|
||||
|
||||
This is meant to be called in tool wrappers (write_file, execute_code)
|
||||
before executing operations with user-supplied paths.
|
||||
|
||||
Note: At runtime, paths from os.path.expanduser() will resolve to
|
||||
/Users/<name>/... — this is expected and allowed. The guard catches
|
||||
paths that were LITERALLY hardcoded in source code or tool arguments
|
||||
that look like they came from a different machine (e.g., a path
|
||||
containing a different username than the current user).
|
||||
"""
|
||||
if not path or not isinstance(path, str):
|
||||
return path
|
||||
|
||||
# At runtime, expanded paths matching current HOME are fine
|
||||
home = os.environ.get("HOME", "")
|
||||
if home and path.startswith(home):
|
||||
return path
|
||||
|
||||
# Check for hardcoded /Users/<name>/ (macOS) — but not current user
|
||||
if re.match(r"^/Users/[\w.-]+/", path):
|
||||
raise PathGuardError(
|
||||
path, "/Users/<name>/",
|
||||
f"Use $HOME or os.path.expanduser('~') instead. "
|
||||
f"Got: {path}"
|
||||
)
|
||||
|
||||
# Check for hardcoded /home/<name>/ (Linux)
|
||||
if re.match(r"^/home/[\w.-]+/", path):
|
||||
raise PathGuardError(
|
||||
path, "/home/<name>/",
|
||||
f"Use $HOME or os.path.expanduser('~') instead. "
|
||||
f"Got: {path}"
|
||||
)
|
||||
|
||||
return path
|
||||
|
||||
|
||||
def validate_tool_paths(paths: list) -> list:
|
||||
"""
|
||||
Validate multiple paths (e.g., from tool arguments).
|
||||
Returns validated list. Raises PathGuardError on first violation.
|
||||
"""
|
||||
return [validate_path(p) for p in paths if isinstance(p, str)]
|
||||
|
||||
|
||||
# ── File Scanning (Pre-commit / CI) ────────────────────────────────
|
||||
|
||||
def scan_file_for_violations(filepath: str) -> List[Tuple[int, str, str, str]]:
|
||||
"""
|
||||
Scan a Python file for hardcoded home-directory path patterns.
|
||||
Returns list of (line_number, line_content, pattern_name, suggestion).
|
||||
"""
|
||||
violations = []
|
||||
try:
|
||||
with open(filepath) as f:
|
||||
for lineno, line in enumerate(f, 1):
|
||||
# Skip comments and noqa lines
|
||||
stripped = line.strip()
|
||||
if stripped.startswith("#"):
|
||||
continue
|
||||
if "noqa: hardcoded-path-ok" in line:
|
||||
continue
|
||||
|
||||
for pattern, name in HARDCODED_PATH_PATTERNS:
|
||||
if pattern.search(line):
|
||||
# Special case: ~/ in expanduser/display context is OK
|
||||
if name == "~/..." and SAFE_TILDE_CONTEXTS.search(line): # noqa: hardcoded-path-ok
|
||||
continue
|
||||
violations.append((lineno, line.rstrip(), name,
|
||||
f"Use get_hermes_home(), os.environ['HOME'], or add # noqa: hardcoded-path-ok"))
|
||||
except (IOError, UnicodeDecodeError):
|
||||
pass
|
||||
return violations
|
||||
|
||||
|
||||
def scan_directory(root: str, extensions: tuple = (".py",)) -> List[Tuple[str, List]]:
|
||||
"""
|
||||
Scan a directory tree for hardcoded path violations.
|
||||
Returns list of (filepath, violations) tuples.
|
||||
"""
|
||||
results = []
|
||||
for dirpath, _, filenames in os.walk(root):
|
||||
# Skip hidden dirs, __pycache__, venv, test dirs
|
||||
skip_dirs = {"__pycache__", ".git", "venv", "node_modules", ".hermes"}
|
||||
if any(s in dirpath for s in skip_dirs):
|
||||
continue
|
||||
|
||||
for fname in filenames:
|
||||
if not fname.endswith(extensions):
|
||||
continue
|
||||
# Skip test files (they may legitimately have paths)
|
||||
if fname.startswith("test_") or "/tests/" in dirpath:
|
||||
continue
|
||||
fpath = os.path.join(dirpath, fname)
|
||||
violations = scan_file_for_violations(fpath)
|
||||
if violations:
|
||||
results.append((fpath, violations))
|
||||
return results
|
||||
Reference in New Issue
Block a user