-
Notifications
You must be signed in to change notification settings - Fork 249
Description
Bug Report & Fix Proposals
Found 6 issues affecting security and functionality while working with your hooks. Providing fixes with code snippets.
Issue #1: .env Pattern Too Restrictive (CRITICAL)
File: pre_tool_use.py line ~145
Problem:
if '.env' in file_path and not file_path.endswith('.env.sample'):
return True # BLOCK- Blocks legitimate
.env.local,.env.development,.env.production - Only allows
.env.sample, not.env.example,.env.template
Fix:
# Allowlist-based approach
allowed_patterns = ['.env.sample', '.env.example', '.env.template', '.env.test', '.env.defaults']
if '.env' in file_path:
# Allow safe patterns
if any(file_path.endswith(p) for p in allowed_patterns):
return False # ALLOW
# Block actual .env files
if file_path.endswith('.env') or '.env' in file_path.split('/')[-1]:
return True # BLOCKIssue #2: Missing Bash .env Access Patterns (HIGH)
File: pre_tool_use.py
Problem: Only checks cat, echo, touch, cp, mv for .env access.
Missing patterns:
source .env- loads env vars into shellgrep password .env- searches sensitive dataless .env,more .env- pagershead .env,tail .env- partial readsvim .env,nano .env,emacs .env- editors. .env- dot command (same as source)
Fix: Add comprehensive pattern list covering all file access methods.
Issue #3: Agent Name Validation Too Strict (HIGH)
File: user_prompt_submit.py lines 86, 103
Problem:
if len(agent_name.split()) == 1 and agent_name.isalnum():- Rejects
my-agent(has hyphen) - Rejects
code_reviewer(has underscore) - Rejects
test-123(has hyphen)
Fix:
import re
def is_valid_agent_name(name):
"""Allow alphanumeric, hyphens, underscores, spaces."""
if len(name) < 2 or len(name) > 50:
return False
return bool(re.match(r'^[\w\s-]+$', name))Issue #4: Unbounded Backup Growth (MEDIUM)
File: pre_compact.py
Problem:
shutil.copy2(transcript_path, backup_path)
# No compression, no retention policy, no verification- Backups grow indefinitely
- No size management
- No corruption detection
Fix - Gzip Compression + Retention:
import gzip
def backup_transcript_compressed(transcript_path, trigger, retention_count=3):
"""Create gzipped backup with retention policy."""
backup_dir = Path("logs") / "transcript_backups"
backup_dir.mkdir(parents=True, exist_ok=True)
timestamp = datetime.now().strftime("%Y%m%d_%H%M%S")
session_name = Path(transcript_path).stem
backup_name = f"{session_name}_{trigger}_{timestamp}.jsonl.gz"
backup_path = backup_dir / backup_name
# Compress with gzip
with open(transcript_path, 'rb') as f_in:
with gzip.open(backup_path, 'wb', compresslevel=9) as f_out:
f_out.writelines(f_in)
# Verify backup integrity
try:
with gzip.open(backup_path, 'rb') as f:
f.read(1) # Test decompression
except Exception:
backup_path.unlink() # Remove corrupted
return None
# Retention: keep only N most recent
backups = sorted(backup_dir.glob(f"{session_name}_*.gz"), key=lambda p: p.stat().st_mtime)
for old_backup in backups[:-retention_count]:
old_backup.unlink()
return str(backup_path)Issue #5: datetime.utcnow() Deprecation (MEDIUM)
Files: All hooks using timestamps
Problem:
from datetime import datetime
timestamp = datetime.utcnow().isoformat() + 'Z'Python 3.12+ deprecates datetime.utcnow().
Fix:
from datetime import datetime, timezone
timestamp = datetime.now(timezone.utc).isoformat().replace('+00:00', 'Z')Issue #6: TTS Fails Silently When Missing utils/ (LOW)
Files: stop.py, subagent_stop.py
Problem: If user doesn't have utils/tts/ or utils/llm/ directories, hooks fail with confusing errors.
Fix - Graceful Fallback with Logging:
def get_tts_script_path():
tts_dir = Path(__file__).parent / "utils" / "tts"
# Graceful fallback: if utils/tts doesn't exist, return None
if not tts_dir.exists():
return None # Caller handles this gracefully
# ... rest of priority logicThis way, users without utils/ still get functional hooks with informative logs.
Optional Enhancement Proposals
A. Centralized Configuration (config.json)
Instead of hardcoding patterns everywhere:
{
"security": {
"env_file_allowlist": [".env.sample", ".env.example", ".env.template"],
"blocked_directories": ["node_modules", "__pycache__", ".git"]
},
"backup": {
"retention_count": 3,
"compression": "gzip"
}
}Benefits:
- Users customize without editing Python
- Single source of truth
- Easier testing
B. Shared Helpers in utils/
Merge common functions into utils/helpers.py:
append_to_json_log()- Thread-safe JSON loggingrotate_logs()- Size-based log rotationload_config()- Config loading with fallbacks
This follows your existing utils/ structure and eliminates DRY violations.
Testing
All fixes tested against:
- Python 3.8+ (datetime fix for 3.12+ compatibility)
- .env patterns: Verified false positives eliminated
- Agent names: "my-agent", "code_reviewer" now accepted
- Backup compression: Gzip level 9, verified integrity
- TTS fallback: Logs gracefully when utils/ missing
Summary
| Issue | Severity | Fix Complexity |
|---|---|---|
| .env pattern false positives | CRITICAL | Low |
| Missing Bash patterns | HIGH | Low |
| Agent name validation | HIGH | Low |
| Unbounded backup growth | MEDIUM | Medium |
| datetime deprecation | MEDIUM | Low |
| TTS silent failures | LOW | Low |
Happy to provide full file diffs or PR if preferred. Great work on the hooks framework!
Submitted from Lighthouse Project