From a064542df9c066793a3d55ff55ce5ac8a43f2682 Mon Sep 17 00:00:00 2001 From: nesquena-hermes Date: Tue, 7 Apr 2026 22:26:03 -0700 Subject: [PATCH] =?UTF-8?q?release:=20v0.39.0=20=E2=80=94=20security=20har?= =?UTF-8?q?dening,=2012=20fixes=20(#171)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Security: harden auth, CSRF, SSRF, XSS, and env race conditions Twelve fixes from a full security audit: CRITICAL - Add CSRF Origin/Referer validation on all POST endpoints (prevents cross-origin abuse of self-update, settings, file ops) HIGH - Unify password hashing: config.py now uses PBKDF2 (600k iters) instead of single-iteration SHA-256 - Add per-IP rate limiting on login (5 attempts/60s, 429 on excess) MEDIUM - Validate session IDs as hex-only before filesystem operations (prevents path traversal via crafted session ID) - SSRF: resolve DNS before private-IP check in model fetching (prevents DNS rebinding to internal services) - Warn loudly when binding non-loopback without password set - SSE env var mutations: wrap sync chat + streaming restore in _ENV_LOCK - Force Content-Disposition:attachment for HTML/XHTML/SVG uploads (prevents stored XSS via uploaded files) LOW - Extend HMAC session signature from 64 to 128 bits - Add resolve()+relative_to() check on skills path construction - Set Secure flag on session cookie when connection is HTTPS - Sanitize exception messages to strip filesystem paths No breaking changes. All fixes are backward-compatible. * fix: use getattr for Secure cookie SSL detection handler.request.getpeercert raises AttributeError on plain sockets (non-SSL). Use getattr(..., None) to safely check for SSL. Co-Authored-By: Claude Opus 4.6 (1M context) * tests: add sprint 29 security hardening coverage (PR #171) 33 tests covering all 12 security fixes: - CSRF origin/referer validation - Login rate limiting (5 attempts/60s) - Session ID hex validation (path traversal prevention) - Error path sanitization (_sanitize_error) - Secure cookie getattr safety - HMAC signature length (64->128 bit) - Skills path traversal prevention - Content-Disposition for HTML/SVG/XHTML - PBKDF2 password hashing verification - Non-loopback startup warning - SSRF DNS guard code presence - _ENV_LOCK export from streaming module * release: v0.39.0 — security hardening, 12 fixes (#171) --------- Co-authored-by: betamod Co-authored-by: Nathan Esquenazi Co-authored-by: Claude Opus 4.6 (1M context) --- CHANGELOG.md | 22 ++ api/auth.py | 27 ++- api/config.py | 26 ++- api/helpers.py | 9 + api/models.py | 3 + api/routes.py | 85 +++++--- api/streaming.py | 17 +- server.py | 8 + static/index.html | 2 +- tests/test_sprint29.py | 452 +++++++++++++++++++++++++++++++++++++++++ 10 files changed, 612 insertions(+), 39 deletions(-) create mode 100644 tests/test_sprint29.py diff --git a/CHANGELOG.md b/CHANGELOG.md index f0d76a2..3b7af9d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,28 @@ --- +## [v0.39.0] — 2026-04-08 + +### Security (12 fixes — PR #171 by @betamod, reviewed by @nesquena-hermes) + +- **CSRF protection**: all POST endpoints now validate `Origin`/`Referer` against `Host`. Non-browser clients (curl, agent) without these headers are unaffected. +- **PBKDF2 password hashing**: `save_settings()` was using single-iteration SHA-256. Now calls `auth._hash_password()` — PBKDF2-HMAC-SHA256 with 600,000 iterations and a per-installation random salt. +- **Login rate limiting**: 5 failed attempts per 60 seconds per IP returns HTTP 429. +- **Session ID validation**: `Session.load()` rejects any non-hex character before touching the filesystem, preventing path traversal via crafted session IDs. +- **SSRF DNS resolution**: `get_available_models()` resolves DNS before checking private IPs. Prevents DNS rebinding attacks. Known-local providers (Ollama, LM Studio, localhost) are whitelisted. +- **Non-loopback startup warning**: server prints a clear warning when binding to `0.0.0.0` without a password set — a common Docker footgun. +- **ENV_LOCK consistency**: `_ENV_LOCK` now wraps all `os.environ` mutations in both the sync chat and streaming restore blocks, preventing races across concurrent requests. +- **Stored XSS prevention**: files with `text/html`, `application/xhtml+xml`, or `image/svg+xml` MIME types are forced to `Content-Disposition: attachment`, preventing execution in-browser. +- **HMAC signature**: extended from 64 bits to 128 bits (16-char to 32-char hex). +- **Skills path validation**: `resolve().relative_to(SKILLS_DIR)` check added after skill directory construction to prevent traversal. +- **Secure cookie flag**: auto-set when TLS or `X-Forwarded-Proto: https` is detected. Uses `getattr` safely so plain sockets don't raise `AttributeError`. +- **Error path sanitization**: `_sanitize_error()` strips absolute filesystem paths from exception messages before they reach the client. + +### Tests +- Added `tests/test_sprint29.py` — 33 tests covering all 12 security fixes. + +--- + ## [v0.38.6] — 2026-04-07 ### Fixed diff --git a/api/auth.py b/api/auth.py index 94e4438..74cb5a7 100644 --- a/api/auth.py +++ b/api/auth.py @@ -24,6 +24,26 @@ SESSION_TTL = 86400 # 24 hours # Active sessions: token -> expiry timestamp _sessions = {} +# ── Login rate limiter ────────────────────────────────────────────────────── +_login_attempts = {} # ip -> [timestamp, ...] +_LOGIN_MAX_ATTEMPTS = 5 +_LOGIN_WINDOW = 60 # seconds + +def _check_login_rate(ip: str) -> bool: + """Return True if the IP is allowed to attempt login.""" + now = time.time() + attempts = _login_attempts.get(ip, []) + # Prune old attempts + attempts = [t for t in attempts if now - t < _LOGIN_WINDOW] + _login_attempts[ip] = attempts + return len(attempts) < _LOGIN_MAX_ATTEMPTS + +def _record_login_attempt(ip: str) -> None: + now = time.time() + attempts = _login_attempts.get(ip, []) + attempts.append(now) + _login_attempts[ip] = attempts + def _signing_key(): """Return a random signing key, generating and persisting one on first call.""" @@ -84,7 +104,7 @@ def create_session() -> str: """Create a new auth session. Returns signed cookie value.""" token = secrets.token_hex(32) _sessions[token] = time.time() + SESSION_TTL - sig = hmac.new(_signing_key(), token.encode(), hashlib.sha256).hexdigest()[:16] + sig = hmac.new(_signing_key(), token.encode(), hashlib.sha256).hexdigest()[:32] return f"{token}.{sig}" @@ -93,7 +113,7 @@ def verify_session(cookie_value) -> bool: if not cookie_value or '.' not in cookie_value: return False token, sig = cookie_value.rsplit('.', 1) - expected_sig = hmac.new(_signing_key(), token.encode(), hashlib.sha256).hexdigest()[:16] + expected_sig = hmac.new(_signing_key(), token.encode(), hashlib.sha256).hexdigest()[:32] if not hmac.compare_digest(sig, expected_sig): return False expiry = _sessions.get(token) @@ -157,6 +177,9 @@ def set_auth_cookie(handler, cookie_value) -> None: cookie[COOKIE_NAME]['samesite'] = 'Lax' cookie[COOKIE_NAME]['path'] = '/' cookie[COOKIE_NAME]['max-age'] = str(SESSION_TTL) + # Set Secure flag when connection is HTTPS + if getattr(handler.request, 'getpeercert', None) is not None or handler.headers.get('X-Forwarded-Proto', '') == 'https': + cookie[COOKIE_NAME]['secure'] = True handler.send_header('Set-Cookie', cookie[COOKIE_NAME].OutputString()) diff --git a/api/config.py b/api/config.py index 4a26e41..189cafb 100644 --- a/api/config.py +++ b/api/config.py @@ -597,7 +597,23 @@ def get_available_models() -> dict: headers['Authorization'] = f'Bearer {api_key}' break - # Fetch model list from endpoint + # Fetch model list from endpoint (with SSRF protection) + import socket + # Resolve hostname and check against private IPs after DNS lookup + parsed_url = urlparse(endpoint_url if '://' in endpoint_url else f'http://{endpoint_url}') + if parsed_url.hostname: + try: + resolved_ips = socket.getaddrinfo(parsed_url.hostname, None) + for _, _, _, _, addr in resolved_ips: + addr_obj = ipaddress.ip_address(addr[0]) + if addr_obj.is_private or addr_obj.is_loopback or addr_obj.is_link_local: + # Allow known local providers (ollama, lmstudio) + is_known_local = any(k in (parsed_url.hostname or '').lower() + for k in ('ollama', 'localhost', '127.0.0.1', 'lmstudio', 'lm-studio')) + if not is_known_local: + raise ValueError(f'SSRF: resolved hostname to private IP {addr[0]}') + except socket.gaierror: + pass # DNS resolution failed -- let urllib handle it req = urllib.request.Request(endpoint_url, method='GET') for k, v in headers.items(): req.add_header(k, v) @@ -762,7 +778,7 @@ _SETTINGS_DEFAULTS = { 'check_for_updates': True, # check if webui/agent repos are behind upstream 'theme': 'dark', # active UI theme name (no enum gate -- allows custom themes) 'bot_name': os.getenv('HERMES_WEBUI_BOT_NAME', 'Hermes'), # display name for the assistant - 'password_hash': None, # SHA-256 hash; None = auth disabled + 'password_hash': None, # PBKDF2-HMAC-SHA256 hash; None = auth disabled } def load_settings() -> dict: @@ -785,13 +801,13 @@ _SETTINGS_BOOL_KEYS = {'show_token_usage', 'show_cli_sessions', 'sync_to_insight def save_settings(settings: dict) -> dict: """Save settings to disk. Returns the merged settings. Ignores unknown keys.""" - import hashlib as _hl current = load_settings() # Handle _set_password: hash and store as password_hash raw_pw = settings.pop('_set_password', None) if raw_pw and isinstance(raw_pw, str) and raw_pw.strip(): - salt = str(STATE_DIR).encode() - current['password_hash'] = _hl.sha256(salt + raw_pw.strip().encode()).hexdigest() + # Use PBKDF2 from auth module (600k iterations) -- never raw SHA-256 + from api.auth import _hash_password + current['password_hash'] = _hash_password(raw_pw.strip()) # Handle _clear_password: explicitly disable auth if settings.pop('_clear_password', False): current['password_hash'] = None diff --git a/api/helpers.py b/api/helpers.py index e2dc017..fe16fb4 100644 --- a/api/helpers.py +++ b/api/helpers.py @@ -18,6 +18,15 @@ def bad(handler, msg, status: int=400): return j(handler, {'error': msg}, status=status) +def _sanitize_error(e: Exception) -> str: + """Strip filesystem paths from exception messages before returning to client.""" + import re + msg = str(e) + # Remove absolute paths (Unix and Windows) + msg = re.sub(r'(?:(?:/[a-zA-Z0-9_.-]+)+|(?:[A-Z]:\\[^\s]+))', '', msg) + return msg + + def safe_resolve(root: Path, requested: str) -> Path: """Resolve a relative path inside root, raising ValueError on traversal.""" resolved = (root / requested).resolve() diff --git a/api/models.py b/api/models.py index 7420b8a..3ab6cd2 100644 --- a/api/models.py +++ b/api/models.py @@ -73,6 +73,9 @@ class Session: @classmethod def load(cls, sid): + # Validate session ID format to prevent path traversal + if not sid or not all(c in '0123456789abcdef' for c in sid): + return None p = SESSION_DIR / f'{sid}.json' if not p.exists(): return None diff --git a/api/routes.py b/api/routes.py index baea175..ff2ed9e 100644 --- a/api/routes.py +++ b/api/routes.py @@ -20,7 +20,25 @@ from api.config import ( IMAGE_EXTS, MD_EXTS, MIME_MAP, MAX_FILE_BYTES, MAX_UPLOAD_BYTES, CHAT_LOCK, load_settings, save_settings, ) -from api.helpers import require, bad, safe_resolve, j, t, read_body, _security_headers +from api.helpers import require, bad, safe_resolve, j, t, read_body, _security_headers, _sanitize_error + +# ── CSRF: validate Origin/Referer on POST ──────────────────────────────────── +import re as _re +def _check_csrf(handler) -> bool: + """Reject cross-origin POST requests. Returns True if OK.""" + origin = handler.headers.get('Origin', '') + referer = handler.headers.get('Referer', '') + host = handler.headers.get('Host', '') + if not origin and not referer: + return True # non-browser clients (curl, agent) have no Origin + target = origin or referer + # Allow same-origin: Origin must match Host + if host and target: + # Extract host:port from origin/referer + m = _re.match(r'^https?://([^/]+)', target) + if m and m.group(1) == host: + return True + return False from api.models import ( Session, get_session, new_session, all_sessions, title_from, _write_session_index, SESSION_INDEX_FILE, @@ -360,6 +378,9 @@ def handle_get(handler, parsed) -> bool: def handle_post(handler, parsed) -> bool: """Handle all POST routes. Returns True if handled, False for 404.""" + # CSRF: reject cross-origin browser requests + if not _check_csrf(handler): + return j(handler, {'error': 'Cross-origin request rejected'}, status=403) if parsed.path == '/api/upload': return handle_upload(handler) @@ -544,7 +565,7 @@ def handle_post(handler, parsed) -> bool: result = switch_profile(name) return j(handler, result) except (ValueError, FileNotFoundError) as e: - return bad(handler, str(e), 404) + return bad(handler, _sanitize_error(e), 404) except RuntimeError as e: return bad(handler, str(e), 409) @@ -578,7 +599,7 @@ def handle_post(handler, parsed) -> bool: result = delete_profile_api(name) return j(handler, result) except (ValueError, FileNotFoundError) as e: - return bad(handler, str(e)) + return bad(handler, _sanitize_error(e)) except RuntimeError as e: return bad(handler, str(e), 409) @@ -695,10 +716,15 @@ def handle_post(handler, parsed) -> bool: # ── Auth endpoints (POST) ── if parsed.path == '/api/auth/login': from api.auth import verify_password, create_session, set_auth_cookie, is_auth_enabled + from api.auth import _check_login_rate, _record_login_attempt if not is_auth_enabled(): return j(handler, {'ok': True, 'message': 'Auth not enabled'}) + client_ip = handler.client_address[0] + if not _check_login_rate(client_ip): + return j(handler, {'error': 'Too many attempts. Try again in a minute.'}, status=429) password = body.get('password', '') if not verify_password(password): + _record_login_attempt(client_ip) return bad(handler, 'Invalid password', 401) cookie_val = create_session() handler.send_response(200) @@ -810,7 +836,7 @@ def _handle_list_dir(handler, parsed): 'path': qs.get('path', ['.'])[0], }) except (FileNotFoundError, ValueError) as e: - return bad(handler, str(e), 404) + return bad(handler, _sanitize_error(e), 404) def _handle_sse_stream(handler, parsed): @@ -859,9 +885,14 @@ def _handle_file_raw(handler, parsed): handler.send_header('Content-Type', mime) handler.send_header('Content-Length', str(len(raw_bytes))) handler.send_header('Cache-Control', 'no-store') - if force_download: + # Security: force download for dangerous MIME types to prevent XSS + dangerous_types = {'text/html', 'application/xhtml+xml', 'image/svg+xml'} + if force_download or mime in dangerous_types: handler.send_header('Content-Disposition', f'attachment; filename="{target.name}"; filename*=UTF-8\'\'{safe_name}') + else: + handler.send_header('Content-Disposition', + f'inline; filename="{target.name}"; filename*=UTF-8\'\'{safe_name}') handler.end_headers() handler.wfile.write(raw_bytes) return True @@ -876,7 +907,7 @@ def _handle_file_read(handler, parsed): rel = qs.get('path', [''])[0] if not rel: return bad(handler, 'path is required') try: return j(handler, read_file_content(Path(s.workspace), rel)) - except (FileNotFoundError, ValueError) as e: return bad(handler, str(e), 404) + except (FileNotFoundError, ValueError) as e: return bad(handler, _sanitize_error(e), 404) def _handle_approval_pending(handler, parsed): @@ -1027,12 +1058,14 @@ def _handle_chat_sync(handler, body): if not msg: return j(handler, {'error': 'empty message'}, status=400) workspace = Path(body.get('workspace') or s.workspace).expanduser().resolve() s.workspace = str(workspace); s.model = body.get('model') or s.model - old_cwd = os.environ.get('TERMINAL_CWD') - os.environ['TERMINAL_CWD'] = str(workspace) - old_exec_ask = os.environ.get('HERMES_EXEC_ASK') - old_session_key = os.environ.get('HERMES_SESSION_KEY') - os.environ['HERMES_EXEC_ASK'] = '1' - os.environ['HERMES_SESSION_KEY'] = s.session_id + from api.streaming import _ENV_LOCK + with _ENV_LOCK: + old_cwd = os.environ.get('TERMINAL_CWD') + os.environ['TERMINAL_CWD'] = str(workspace) + old_exec_ask = os.environ.get('HERMES_EXEC_ASK') + old_session_key = os.environ.get('HERMES_SESSION_KEY') + os.environ['HERMES_EXEC_ASK'] = '1' + os.environ['HERMES_SESSION_KEY'] = s.session_id try: from run_agent import AIAgent with CHAT_LOCK: @@ -1075,12 +1108,13 @@ def _handle_chat_sync(handler, body): persist_user_message=msg, ) finally: - if old_cwd is None: os.environ.pop('TERMINAL_CWD', None) - else: os.environ['TERMINAL_CWD'] = old_cwd - if old_exec_ask is None: os.environ.pop('HERMES_EXEC_ASK', None) - else: os.environ['HERMES_EXEC_ASK'] = old_exec_ask - if old_session_key is None: os.environ.pop('HERMES_SESSION_KEY', None) - else: os.environ['HERMES_SESSION_KEY'] = old_session_key + with _ENV_LOCK: + if old_cwd is None: os.environ.pop('TERMINAL_CWD', None) + else: os.environ['TERMINAL_CWD'] = old_cwd + if old_exec_ask is None: os.environ.pop('HERMES_EXEC_ASK', None) + else: os.environ['HERMES_EXEC_ASK'] = old_exec_ask + if old_session_key is None: os.environ.pop('HERMES_SESSION_KEY', None) + else: os.environ['HERMES_SESSION_KEY'] = old_session_key s.messages = result.get('messages') or s.messages s.title = title_from(s.messages, s.title); s.save() # Sync to state.db for /insights (opt-in setting) @@ -1180,7 +1214,7 @@ def _handle_file_delete(handler, body): if target.is_dir(): return bad(handler, 'Cannot delete directories via this endpoint') target.unlink() return j(handler, {'ok': True, 'path': body['path']}) - except (ValueError, PermissionError) as e: return bad(handler, str(e)) + except (ValueError, PermissionError) as e: return bad(handler, _sanitize_error(e)) def _handle_file_save(handler, body): @@ -1194,7 +1228,7 @@ def _handle_file_save(handler, body): if target.is_dir(): return bad(handler, 'Cannot save: path is a directory') target.write_text(body.get('content', ''), encoding='utf-8') return j(handler, {'ok': True, 'path': body['path'], 'size': target.stat().st_size}) - except (ValueError, PermissionError) as e: return bad(handler, str(e)) + except (ValueError, PermissionError) as e: return bad(handler, _sanitize_error(e)) def _handle_file_create(handler, body): @@ -1208,7 +1242,7 @@ def _handle_file_create(handler, body): target.parent.mkdir(parents=True, exist_ok=True) target.write_text(body.get('content', ''), encoding='utf-8') return j(handler, {'ok': True, 'path': str(target.relative_to(Path(s.workspace)))}) - except (ValueError, PermissionError) as e: return bad(handler, str(e)) + except (ValueError, PermissionError) as e: return bad(handler, _sanitize_error(e)) def _handle_file_rename(handler, body): @@ -1227,7 +1261,7 @@ def _handle_file_rename(handler, body): source.rename(dest) new_rel = str(dest.relative_to(Path(s.workspace))) return j(handler, {'ok': True, 'old_path': body['path'], 'new_path': new_rel}) - except (ValueError, PermissionError, OSError) as e: return bad(handler, str(e)) + except (ValueError, PermissionError, OSError) as e: return bad(handler, _sanitize_error(e)) def _handle_create_dir(handler, body): @@ -1240,7 +1274,7 @@ def _handle_create_dir(handler, body): if target.exists(): return bad(handler, 'Path already exists') target.mkdir(parents=True) return j(handler, {'ok': True, 'path': str(target.relative_to(Path(s.workspace)))}) - except (ValueError, PermissionError, OSError) as e: return bad(handler, str(e)) + except (ValueError, PermissionError, OSError) as e: return bad(handler, _sanitize_error(e)) def _handle_workspace_add(handler, body): @@ -1314,6 +1348,11 @@ def _handle_skill_save(handler, body): skill_dir = SKILLS_DIR / category / skill_name else: skill_dir = SKILLS_DIR / skill_name + # Validate resolved path stays within SKILLS_DIR + try: + skill_dir.resolve().relative_to(SKILLS_DIR.resolve()) + except ValueError: + return bad(handler, 'Invalid skill path') skill_dir.mkdir(parents=True, exist_ok=True) skill_file = skill_dir / 'SKILL.md' skill_file.write_text(body['content'], encoding='utf-8') diff --git a/api/streaming.py b/api/streaming.py index 6e92ccd..7d5c455 100644 --- a/api/streaming.py +++ b/api/streaming.py @@ -379,14 +379,15 @@ def _run_agent_streaming(session_id, msg_text, model, workspace, stream_id, atta usage['last_prompt_tokens'] = getattr(_cc, 'last_prompt_tokens', 0) or 0 put('done', {'session': s.compact() | {'messages': s.messages, 'tool_calls': tool_calls}, 'usage': usage}) finally: - if old_cwd is None: os.environ.pop('TERMINAL_CWD', None) - else: os.environ['TERMINAL_CWD'] = old_cwd - if old_exec_ask is None: os.environ.pop('HERMES_EXEC_ASK', None) - else: os.environ['HERMES_EXEC_ASK'] = old_exec_ask - if old_session_key is None: os.environ.pop('HERMES_SESSION_KEY', None) - else: os.environ['HERMES_SESSION_KEY'] = old_session_key - if old_hermes_home is None: os.environ.pop('HERMES_HOME', None) - else: os.environ['HERMES_HOME'] = old_hermes_home + with _ENV_LOCK: + if old_cwd is None: os.environ.pop('TERMINAL_CWD', None) + else: os.environ['TERMINAL_CWD'] = old_cwd + if old_exec_ask is None: os.environ.pop('HERMES_EXEC_ASK', None) + else: os.environ['HERMES_EXEC_ASK'] = old_exec_ask + if old_session_key is None: os.environ.pop('HERMES_SESSION_KEY', None) + else: os.environ['HERMES_SESSION_KEY'] = old_session_key + if old_hermes_home is None: os.environ.pop('HERMES_HOME', None) + else: os.environ['HERMES_HOME'] = old_hermes_home except Exception as e: print('[webui] stream error:\n' + traceback.format_exc(), flush=True) diff --git a/server.py b/server.py index f168674..0dde030 100644 --- a/server.py +++ b/server.py @@ -61,6 +61,14 @@ def main() -> None: print_startup_config() + # Security: warn if binding non-loopback without authentication + from api.auth import is_auth_enabled + if HOST not in ('127.0.0.1', '::1', 'localhost') and not is_auth_enabled(): + print(f'[!!] WARNING: Binding to {HOST} with NO PASSWORD SET.', flush=True) + print(f' Anyone on the network can access your filesystem and agent.', flush=True) + print(f' Set a password via Settings or HERMES_WEBUI_PASSWORD env var.', flush=True) + print(f' To suppress: bind to 127.0.0.1 or set a password.', flush=True) + ok, missing, errors = verify_hermes_imports() if not ok and _HERMES_FOUND: print(f'[!!] Warning: Hermes agent found but missing modules: {missing}', flush=True) diff --git a/static/index.html b/static/index.html index 27a57dd..3af9c3a 100644 --- a/static/index.html +++ b/static/index.html @@ -14,7 +14,7 @@