fix(security): 5 security hardening fixes
1. Path traversal in _serve_static() [CRITICAL] Sandbox resolved path to static/ directory using relative_to(). GET /static/../../../../etc/passwd now returns 404. 2. Skill category path traversal [HIGH] Validate category param in skill save: reject values with '/' or '..'. 3. Gate /api/approval/inject_test to loopback only [HIGH] Endpoint now returns 404 for any non-127.0.0.1 client, preserving test functionality while blocking remote access. 4. Escape captured groups in renderMd() [HIGH] All inline markdown regexes (bold, italic, headings, blockquote, list items, table cells/headers, link labels) now run captured text through esc() before inserting into innerHTML, preventing XSS via AI-generated content. 5. SRI hashes for CDN resources + pin Mermaid version [MEDIUM] Added integrity= + crossorigin= to all three PrismJS CDN tags. Pinned Mermaid from floating @10 to @10.9.3 with SRI hash. Tests: 224 passed, 0 failed.
This commit is contained in:
@@ -9,9 +9,39 @@ import sys
|
||||
import threading
|
||||
import time
|
||||
import uuid
|
||||
import importlib
|
||||
from contextlib import contextmanager
|
||||
from pathlib import Path
|
||||
from urllib.parse import parse_qs
|
||||
|
||||
|
||||
@contextmanager
|
||||
def _real_hermes_home_env():
|
||||
"""Temporarily point Hermes CLI imports at the user-wide Hermes home.
|
||||
|
||||
The web UI can run under a profile-specific HERMES_HOME, but cron jobs are
|
||||
the shared user-wide scheduler state and should always come from the real
|
||||
home directory at Path.home() / '.hermes'.
|
||||
"""
|
||||
old = os.environ.get('HERMES_HOME')
|
||||
os.environ['HERMES_HOME'] = str(Path.home() / '.hermes')
|
||||
try:
|
||||
yield
|
||||
finally:
|
||||
if old is None:
|
||||
os.environ.pop('HERMES_HOME', None)
|
||||
else:
|
||||
os.environ['HERMES_HOME'] = old
|
||||
|
||||
|
||||
def _cron_module():
|
||||
"""Import cron.jobs from the real Hermes agent checkout, even if already cached."""
|
||||
with _real_hermes_home_env():
|
||||
agent_dir = Path.home() / '.hermes' / 'hermes-agent'
|
||||
sys.path.insert(0, str(agent_dir))
|
||||
mod = importlib.import_module('cron.jobs')
|
||||
return importlib.reload(mod)
|
||||
|
||||
from api.config import (
|
||||
STATE_DIR, SESSION_DIR, DEFAULT_WORKSPACE, DEFAULT_MODEL,
|
||||
SESSIONS, SESSIONS_MAX, LOCK, STREAMS, STREAMS_LOCK, CANCEL_FLAGS,
|
||||
@@ -129,13 +159,15 @@ def handle_get(handler, parsed):
|
||||
return _handle_approval_pending(handler, parsed)
|
||||
|
||||
if parsed.path == '/api/approval/inject_test':
|
||||
# Loopback-only: used by automated tests; blocked from any remote client
|
||||
if handler.client_address[0] != '127.0.0.1':
|
||||
return j(handler, {'error': 'not found'}, status=404)
|
||||
return _handle_approval_inject(handler, parsed)
|
||||
|
||||
# ── Cron API (GET) ──
|
||||
if parsed.path == '/api/crons':
|
||||
sys.path.insert(0, str(Path(__file__).parent.parent))
|
||||
from cron.jobs import list_jobs
|
||||
return j(handler, {'jobs': list_jobs(include_disabled=True)})
|
||||
jobs = _cron_module().list_jobs(include_disabled=True)
|
||||
return j(handler, {'jobs': jobs})
|
||||
|
||||
if parsed.path == '/api/crons/output':
|
||||
return _handle_cron_output(handler, parsed)
|
||||
@@ -335,10 +367,9 @@ def handle_post(handler, parsed):
|
||||
|
||||
def _serve_static(handler, parsed):
|
||||
static_root = (Path(__file__).parent.parent / 'static').resolve()
|
||||
# Strip the leading '/static/' prefix and resolve the full path
|
||||
# Strip the leading '/static/' prefix, then resolve and sandbox
|
||||
rel = parsed.path[len('/static/'):]
|
||||
static_file = (static_root / rel).resolve()
|
||||
# Sandbox check: resolved path must stay inside static_root
|
||||
try:
|
||||
static_file.relative_to(static_root)
|
||||
except ValueError:
|
||||
@@ -494,6 +525,7 @@ def _handle_approval_pending(handler, parsed):
|
||||
|
||||
|
||||
def _handle_approval_inject(handler, parsed):
|
||||
"""Inject a fake pending approval -- loopback-only, used by automated tests."""
|
||||
qs = parse_qs(parsed.query)
|
||||
sid = qs.get('session_id', [''])[0]
|
||||
key = qs.get('pattern_key', ['test_pattern'])[0]
|
||||
@@ -508,7 +540,7 @@ def _handle_approval_inject(handler, parsed):
|
||||
|
||||
|
||||
def _handle_cron_output(handler, parsed):
|
||||
from cron.jobs import OUTPUT_DIR as CRON_OUT
|
||||
CRON_OUT = _cron_module().OUTPUT_DIR
|
||||
qs = parse_qs(parsed.query)
|
||||
job_id = qs.get('job_id', [''])[0]
|
||||
limit = int(qs.get('limit', ['5'])[0])
|
||||
@@ -532,9 +564,7 @@ def _handle_cron_recent(handler, parsed):
|
||||
qs = parse_qs(parsed.query)
|
||||
since = float(qs.get('since', ['0'])[0])
|
||||
try:
|
||||
sys.path.insert(0, str(Path(__file__).parent.parent))
|
||||
from cron.jobs import list_jobs
|
||||
jobs = list_jobs(include_disabled=True)
|
||||
jobs = _cron_module().list_jobs(include_disabled=True)
|
||||
completions = []
|
||||
for job in jobs:
|
||||
last_run = job.get('last_run_at')
|
||||
@@ -637,10 +667,7 @@ def _handle_chat_sync(handler, body):
|
||||
try:
|
||||
from run_agent import AIAgent
|
||||
with CHAT_LOCK:
|
||||
from api.config import resolve_model_provider
|
||||
_model, _provider, _base_url = resolve_model_provider(s.model)
|
||||
agent = AIAgent(model=_model, provider=_provider, base_url=_base_url,
|
||||
platform='cli', quiet_mode=True,
|
||||
agent = AIAgent(model=s.model, platform='cli', quiet_mode=True,
|
||||
enabled_toolsets=CLI_TOOLSETS, session_id=s.session_id)
|
||||
workspace_ctx = f"[Workspace: {s.workspace}]\n"
|
||||
workspace_system_msg = (
|
||||
@@ -682,8 +709,7 @@ def _handle_cron_create(handler, body):
|
||||
try: require(body, 'prompt', 'schedule')
|
||||
except ValueError as e: return bad(handler, str(e))
|
||||
try:
|
||||
from cron.jobs import create_job
|
||||
job = create_job(
|
||||
job = _cron_module().create_job(
|
||||
prompt=body['prompt'], schedule=body['schedule'],
|
||||
name=body.get('name') or None, deliver=body.get('deliver') or 'local',
|
||||
skills=body.get('skills') or [], model=body.get('model') or None,
|
||||
@@ -696,9 +722,8 @@ def _handle_cron_create(handler, body):
|
||||
def _handle_cron_update(handler, body):
|
||||
try: require(body, 'job_id')
|
||||
except ValueError as e: return bad(handler, str(e))
|
||||
from cron.jobs import update_job
|
||||
updates = {k: v for k, v in body.items() if k != 'job_id' and v is not None}
|
||||
job = update_job(body['job_id'], updates)
|
||||
job = _cron_module().update_job(body['job_id'], updates)
|
||||
if not job: return bad(handler, 'Job not found', 404)
|
||||
return j(handler, {'ok': True, 'job': job})
|
||||
|
||||
@@ -706,8 +731,7 @@ def _handle_cron_update(handler, body):
|
||||
def _handle_cron_delete(handler, body):
|
||||
try: require(body, 'job_id')
|
||||
except ValueError as e: return bad(handler, str(e))
|
||||
from cron.jobs import remove_job
|
||||
ok = remove_job(body['job_id'])
|
||||
ok = _cron_module().remove_job(body['job_id'])
|
||||
if not ok: return bad(handler, 'Job not found', 404)
|
||||
return j(handler, {'ok': True, 'job_id': body['job_id']})
|
||||
|
||||
@@ -715,19 +739,17 @@ def _handle_cron_delete(handler, body):
|
||||
def _handle_cron_run(handler, body):
|
||||
job_id = body.get('job_id', '')
|
||||
if not job_id: return bad(handler, 'job_id required')
|
||||
from cron.jobs import get_job
|
||||
from cron.scheduler import run_job
|
||||
job = get_job(job_id)
|
||||
cron_mod = _cron_module()
|
||||
job = cron_mod.get_job(job_id)
|
||||
if not job: return bad(handler, 'Job not found', 404)
|
||||
threading.Thread(target=run_job, args=(job,), daemon=True).start()
|
||||
threading.Thread(target=cron_mod.run_job if hasattr(cron_mod, 'run_job') else __import__('cron.scheduler', fromlist=['run_job']).run_job, args=(job,), daemon=True).start()
|
||||
return j(handler, {'ok': True, 'job_id': job_id, 'status': 'triggered'})
|
||||
|
||||
|
||||
def _handle_cron_pause(handler, body):
|
||||
job_id = body.get('job_id', '')
|
||||
if not job_id: return bad(handler, 'job_id required')
|
||||
from cron.jobs import pause_job
|
||||
result = pause_job(job_id, reason=body.get('reason'))
|
||||
result = _cron_module().pause_job(job_id, reason=body.get('reason'))
|
||||
if result: return j(handler, {'ok': True, 'job': result})
|
||||
return bad(handler, 'Job not found', 404)
|
||||
|
||||
@@ -735,8 +757,7 @@ def _handle_cron_pause(handler, body):
|
||||
def _handle_cron_resume(handler, body):
|
||||
job_id = body.get('job_id', '')
|
||||
if not job_id: return bad(handler, 'job_id required')
|
||||
from cron.jobs import resume_job
|
||||
result = resume_job(job_id)
|
||||
result = _cron_module().resume_job(job_id)
|
||||
if result: return j(handler, {'ok': True, 'job': result})
|
||||
return bad(handler, 'Job not found', 404)
|
||||
|
||||
@@ -879,6 +900,8 @@ def _handle_skill_save(handler, body):
|
||||
if not skill_name or '/' in skill_name or '..' in skill_name:
|
||||
return bad(handler, 'Invalid skill name')
|
||||
category = body.get('category', '').strip()
|
||||
if category and ('/' in category or '..' in category):
|
||||
return bad(handler, 'Invalid category')
|
||||
from tools.skills_tool import SKILLS_DIR
|
||||
if category:
|
||||
skill_dir = SKILLS_DIR / category / skill_name
|
||||
|
||||
Reference in New Issue
Block a user