Compare commits
3 Commits
fix/924
...
fix/921-ha
| Author | SHA1 | Date | |
|---|---|---|---|
| 4cdda8701d | |||
| a80d30b342 | |||
| f098cf8c4a |
78
.githooks/pre-commit-hardcoded-path.py
Normal file
78
.githooks/pre-commit-hardcoded-path.py
Normal file
@@ -0,0 +1,78 @@
|
||||
#!/usr/bin/env python3
|
||||
"""
|
||||
Pre-commit hook: Reject hardcoded home-directory paths.
|
||||
|
||||
Install:
|
||||
cp pre-commit-hardcoded-path.py .git/hooks/pre-commit-hardcoded-path
|
||||
chmod +x .git/hooks/pre-commit-hardcoded-path
|
||||
|
||||
Or add to .pre-commit-config.yaml
|
||||
"""
|
||||
|
||||
import sys
|
||||
import subprocess
|
||||
import re
|
||||
|
||||
PATTERNS = [
|
||||
(r"/Users/[\w.\-]+/", "macOS home directory"),
|
||||
(r"/home/[\w.\-]+/", "Linux home directory"),
|
||||
(r"(?<![\w/])~/", "unexpanded tilde"),
|
||||
]
|
||||
|
||||
NOQA = re.compile(r"#\s*noqa:?\s*hardcoded-path-ok")
|
||||
|
||||
def get_staged_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().split("\n") if f.endswith(".py")]
|
||||
|
||||
def check_file(filepath):
|
||||
try:
|
||||
result = subprocess.run(
|
||||
["git", "show", f":{filepath}"],
|
||||
capture_output=True, text=True
|
||||
)
|
||||
content = result.stdout
|
||||
except Exception:
|
||||
return []
|
||||
|
||||
violations = []
|
||||
for i, line in enumerate(content.split("\n"), 1):
|
||||
if line.strip().startswith("#"):
|
||||
continue
|
||||
if line.strip().startswith(("import ", "from ")):
|
||||
continue
|
||||
if NOQA.search(line):
|
||||
continue
|
||||
for pattern, desc in PATTERNS:
|
||||
if re.search(pattern, line):
|
||||
violations.append((filepath, i, line.strip(), desc))
|
||||
break
|
||||
return violations
|
||||
|
||||
def main():
|
||||
files = get_staged_files()
|
||||
if not files:
|
||||
sys.exit(0)
|
||||
|
||||
all_violations = []
|
||||
for f in files:
|
||||
all_violations.extend(check_file(f))
|
||||
|
||||
if all_violations:
|
||||
print("ERROR: Hardcoded home directory paths detected:")
|
||||
print()
|
||||
for filepath, line_no, line, desc in all_violations:
|
||||
print(f" {filepath}:{line_no}: {desc}")
|
||||
print(f" {line[:100]}")
|
||||
print()
|
||||
print("Fix: Use $HOME, relative paths, or get_hermes_home().")
|
||||
print("Override: Add '# noqa: hardcoded-path-ok' to the line.")
|
||||
sys.exit(1)
|
||||
|
||||
sys.exit(0)
|
||||
|
||||
if __name__ == "__main__":
|
||||
main()
|
||||
@@ -1,146 +0,0 @@
|
||||
"""Provider Preflight — Poka-yoke validation of provider/model config.
|
||||
|
||||
Validates provider and model configuration before session start.
|
||||
Prevents wasted context on misconfigured providers.
|
||||
|
||||
Usage:
|
||||
from agent.provider_preflight import preflight_check
|
||||
result = preflight_check(provider="openrouter", model="xiaomi/mimo-v2-pro")
|
||||
if not result["valid"]:
|
||||
print(result["error"])
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import logging
|
||||
import os
|
||||
from typing import Any, Dict, Optional
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
|
||||
# Provider -> required env var
|
||||
PROVIDER_KEYS = {
|
||||
"openrouter": "OPENROUTER_API_KEY",
|
||||
"anthropic": "ANTHROPIC_API_KEY",
|
||||
"openai": "OPENAI_API_KEY",
|
||||
"nous": "NOUS_API_KEY",
|
||||
"ollama": None, # Local, no key needed
|
||||
"local": None,
|
||||
}
|
||||
|
||||
|
||||
def check_provider_key(provider: str) -> Dict[str, Any]:
|
||||
"""Check if provider has a valid API key configured."""
|
||||
provider_lower = provider.lower().strip()
|
||||
|
||||
env_var = None
|
||||
for known, key in PROVIDER_KEYS.items():
|
||||
if known in provider_lower:
|
||||
env_var = key
|
||||
break
|
||||
|
||||
if env_var is None:
|
||||
# Unknown provider — assume OK (custom/local)
|
||||
return {"valid": True, "provider": provider, "key_status": "unknown"}
|
||||
|
||||
if env_var is None:
|
||||
# Local provider, no key needed
|
||||
return {"valid": True, "provider": provider, "key_status": "not_required"}
|
||||
|
||||
key_value = os.getenv(env_var, "").strip()
|
||||
if not key_value:
|
||||
return {
|
||||
"valid": False,
|
||||
"provider": provider,
|
||||
"key_status": "missing",
|
||||
"error": f"{env_var} is not set. Provider '{provider}' will fail.",
|
||||
"fix": f"Set {env_var} in ~/.hermes/.env",
|
||||
}
|
||||
|
||||
if len(key_value) < 10:
|
||||
return {
|
||||
"valid": False,
|
||||
"provider": provider,
|
||||
"key_status": "too_short",
|
||||
"error": f"{env_var} is suspiciously short ({len(key_value)} chars). May be invalid.",
|
||||
"fix": f"Verify {env_var} value in ~/.hermes/.env",
|
||||
}
|
||||
|
||||
return {"valid": True, "provider": provider, "key_status": "set"}
|
||||
|
||||
|
||||
def check_model_availability(model: str, provider: str) -> Dict[str, Any]:
|
||||
"""Check if model is likely available for provider."""
|
||||
if not model:
|
||||
return {"valid": False, "error": "No model specified"}
|
||||
|
||||
# Basic sanity checks
|
||||
model_lower = model.lower()
|
||||
|
||||
# Anthropic models should use anthropic provider
|
||||
if "claude" in model_lower and "anthropic" not in provider.lower():
|
||||
return {
|
||||
"valid": True, # Allow but warn
|
||||
"warning": f"Model '{model}' usually runs on Anthropic provider, not '{provider}'",
|
||||
}
|
||||
|
||||
# Ollama models
|
||||
ollama_indicators = ["llama", "mistral", "qwen", "gemma", "phi", "hermes"]
|
||||
if any(x in model_lower for x in ollama_indicators) and ":" not in model:
|
||||
return {
|
||||
"valid": True,
|
||||
"warning": f"Model '{model}' may need a version tag for Ollama (e.g., {model}:latest)",
|
||||
}
|
||||
|
||||
return {"valid": True}
|
||||
|
||||
|
||||
def preflight_check(
|
||||
provider: str = "",
|
||||
model: str = "",
|
||||
fallback_provider: str = "",
|
||||
fallback_model: str = "",
|
||||
) -> Dict[str, Any]:
|
||||
"""Full pre-flight check for provider/model configuration.
|
||||
|
||||
Returns:
|
||||
Dict with valid (bool), errors (list), warnings (list).
|
||||
"""
|
||||
errors = []
|
||||
warnings = []
|
||||
|
||||
# Check primary provider
|
||||
if provider:
|
||||
result = check_provider_key(provider)
|
||||
if not result["valid"]:
|
||||
errors.append(result.get("error", f"Provider {provider} invalid"))
|
||||
|
||||
# Check primary model
|
||||
if model:
|
||||
result = check_model_availability(model, provider)
|
||||
if not result["valid"]:
|
||||
errors.append(result.get("error", f"Model {model} invalid"))
|
||||
elif result.get("warning"):
|
||||
warnings.append(result["warning"])
|
||||
|
||||
# Check fallback
|
||||
if fallback_provider:
|
||||
result = check_provider_key(fallback_provider)
|
||||
if not result["valid"]:
|
||||
warnings.append(f"Fallback provider {fallback_provider} also invalid: {result.get('error','')}")
|
||||
|
||||
if fallback_model:
|
||||
result = check_model_availability(fallback_model, fallback_provider)
|
||||
if not result["valid"]:
|
||||
warnings.append(f"Fallback model {fallback_model} invalid")
|
||||
elif result.get("warning"):
|
||||
warnings.append(result["warning"])
|
||||
|
||||
return {
|
||||
"valid": len(errors) == 0,
|
||||
"errors": errors,
|
||||
"warnings": warnings,
|
||||
"provider": provider,
|
||||
"model": model,
|
||||
}
|
||||
@@ -28,6 +28,7 @@ from typing import Dict, Any, List, Optional, Tuple
|
||||
|
||||
from tools.registry import discover_builtin_tools, registry
|
||||
from tools.tool_pokayoke import validate_tool_call, reset_circuit_breaker, get_hallucination_stats
|
||||
from tools.hardcoded_path_guard import guard_tool_dispatch as _guard_hardcoded_paths
|
||||
from toolsets import resolve_toolset, validate_toolset
|
||||
from agent.tool_orchestrator import orchestrator
|
||||
|
||||
@@ -501,6 +502,12 @@ def handle_function_call(
|
||||
# Prefer the caller-provided list so subagents can't overwrite
|
||||
# the parent's tool set via the process-global.
|
||||
sandbox_enabled = enabled_tools if enabled_tools is not None else _last_resolved_tool_names
|
||||
# Poka-yoke #921: guard against hardcoded home-directory paths
|
||||
_hardcoded_err = _guard_hardcoded_paths(function_name, function_args)
|
||||
if _hardcoded_err:
|
||||
logger.warning(f"Hardcoded path blocked: {function_name}")
|
||||
return _hardcoded_err
|
||||
|
||||
# Poka-yoke: validate tool call before dispatch
|
||||
is_valid, corrected_name, corrected_params, pokayoke_messages = validate_tool_call(function_name, function_args)
|
||||
if not is_valid:
|
||||
|
||||
113
tools/hardcoded_path_guard.py
Normal file
113
tools/hardcoded_path_guard.py
Normal file
@@ -0,0 +1,113 @@
|
||||
#!/usr/bin/env python3
|
||||
"""
|
||||
Hardcoded Path Guard — Poka-Yoke #921
|
||||
|
||||
Detects and blocks hardcoded home-directory paths in tool arguments.
|
||||
These paths work on one machine but break on others, VPS deployments,
|
||||
or when HOME changes.
|
||||
|
||||
Usage:
|
||||
from tools.hardcoded_path_guard import check_path, validate_tool_args
|
||||
|
||||
# Check a single path
|
||||
err = check_path("/Users/apayne/.hermes/config.yaml")
|
||||
|
||||
# Validate all path-like args in a tool call
|
||||
clean_args, warnings = validate_tool_args("read_file", {"path": "/home/user/file.txt"})
|
||||
"""
|
||||
|
||||
import os
|
||||
import re
|
||||
import json as _json
|
||||
from typing import Dict, List, Optional, Tuple, Any
|
||||
|
||||
# Patterns that indicate hardcoded home directories
|
||||
HARDCODED_PATTERNS = [
|
||||
(r"/Users/[\w.\-]+/", "macOS home directory (/Users/...)"),
|
||||
(r"/home/[\w.\-]+/", "Linux home directory (/home/...)"),
|
||||
(r"(?<![\w/])~/", "unexpanded tilde (~/)"),
|
||||
(r"/root/", "root home directory (/root/)"),
|
||||
]
|
||||
|
||||
_COMPILED_PATTERNS = [(re.compile(p), desc) for p, desc in HARDCODED_PATTERNS]
|
||||
_NOQA_PATTERN = re.compile(r"#\s*noqa:?\s*hardcoded-path-ok")
|
||||
|
||||
_PATH_ARG_NAMES = frozenset({
|
||||
"path", "file_path", "filepath", "dir", "directory", "dest", "source",
|
||||
"input", "output", "src", "dst", "target", "location", "file",
|
||||
"image_path", "script", "config", "log_file",
|
||||
})
|
||||
|
||||
|
||||
def has_hardcoded_path(text: str) -> Optional[str]:
|
||||
if _NOQA_PATTERN.search(text):
|
||||
return None
|
||||
for pattern, desc in _COMPILED_PATTERNS:
|
||||
if pattern.search(text):
|
||||
return desc
|
||||
return None
|
||||
|
||||
|
||||
def check_path(path_value: str) -> Optional[str]:
|
||||
if not isinstance(path_value, str):
|
||||
return None
|
||||
match_desc = has_hardcoded_path(path_value)
|
||||
if match_desc:
|
||||
return (
|
||||
f"Path contains hardcoded home directory ({match_desc}): '{path_value}'. "
|
||||
f"Use $HOME, relative paths, or get_hermes_home(). "
|
||||
f"Add '# noqa: hardcoded-path-ok' if intentional."
|
||||
)
|
||||
return None
|
||||
|
||||
|
||||
def validate_tool_args(tool_name: str, args: Dict[str, Any]) -> Tuple[Dict[str, Any], List[str]]:
|
||||
warnings = []
|
||||
for key, value in args.items():
|
||||
if key.lower() not in _PATH_ARG_NAMES:
|
||||
continue
|
||||
if isinstance(value, str):
|
||||
err = check_path(value)
|
||||
if err:
|
||||
warnings.append(err)
|
||||
elif isinstance(value, list):
|
||||
for item in value:
|
||||
if isinstance(item, str):
|
||||
err = check_path(item)
|
||||
if err:
|
||||
warnings.append(err)
|
||||
return args, warnings
|
||||
|
||||
|
||||
def scan_source_for_violations(source_code: str, filename: str = "") -> List[Tuple[int, str, str]]:
|
||||
violations = []
|
||||
lines = source_code.split("\n")
|
||||
for i, line in enumerate(lines, 1):
|
||||
stripped = line.strip()
|
||||
if stripped.startswith("#"):
|
||||
if _NOQA_PATTERN.search(line):
|
||||
continue
|
||||
continue
|
||||
if stripped.startswith("import ") or stripped.startswith("from "):
|
||||
continue
|
||||
for pattern, desc in _COMPILED_PATTERNS:
|
||||
match = pattern.search(line)
|
||||
if match:
|
||||
if _NOQA_PATTERN.search(line):
|
||||
continue
|
||||
violations.append((i, line.strip(), desc))
|
||||
break
|
||||
return violations
|
||||
|
||||
|
||||
def guard_tool_dispatch(tool_name: str, args: Dict[str, Any]) -> Optional[str]:
|
||||
_, warnings = validate_tool_args(tool_name, args)
|
||||
if warnings:
|
||||
return _json.dumps({
|
||||
"error": "Hardcoded home directory path detected",
|
||||
"details": warnings,
|
||||
"suggestion": "Use $HOME, relative paths, or get_hermes_home() instead of hardcoded paths.",
|
||||
"pokayoke": True,
|
||||
"rule": "hardcoded-path-guard"
|
||||
})
|
||||
return None
|
||||
Reference in New Issue
Block a user