From 571a5a40f1b0676ad531b56279c64caf80d118e9 Mon Sep 17 00:00:00 2001 From: Nathan Esquenazi Date: Fri, 3 Apr 2026 18:06:18 +0000 Subject: [PATCH] fix(review): 3 issues found in agent review of PR #41 BUG-3 (high): /api/profile/delete missing RuntimeError catch. When deleting the active profile while an agent was running, delete_profile_api() called switch_profile('default') which raises RuntimeError('Cannot switch profiles while agent is running'). This propagated to the 500 handler giving the user 'Internal server error' with no context. Added the same except RuntimeError -> 409 pattern that /api/profile/switch already uses. INFO-1 (defense-in-depth): /api/profile/create had no server-side name validation before delegating to hermes_cli.validate_profile_name. Added server-side ^[a-z0-9][a-z0-9_-]{0,63}$ check, consistent with client-side regex in submitProfileCreate(). Prevents path-traversal-ish names from reaching hermes_cli even if the client-side guard is bypassed. INFO-2 (defense-in-depth): clone_from parameter was passed directly to hermes_cli with no validation. Applied the same name regex check to clone_from before delegating. BUG-11 (low): toggleProfileDropdown() and toggleWsDropdown() could both be open simultaneously. Added cross-dropdown close calls: opening the profile dropdown now closes the workspace dropdown, and vice versa. Tests: 415 passed, 0 failed. --- api/routes.py | 12 +++++++++++- static/panels.js | 2 ++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/api/routes.py b/api/routes.py index b90c8e9..f0bafc5 100644 --- a/api/routes.py +++ b/api/routes.py @@ -397,11 +397,19 @@ def handle_post(handler, parsed): if parsed.path == '/api/profile/create': name = body.get('name', '').strip() if not name: return bad(handler, 'name is required') + import re as _re + if not _re.match(r'^[a-z0-9][a-z0-9_-]{0,63}$', name): + return bad(handler, 'Invalid profile name: lowercase letters, numbers, hyphens, underscores only') + clone_from = body.get('clone_from') + if clone_from is not None: + clone_from = str(clone_from).strip() + if not _re.match(r'^[a-z0-9][a-z0-9_-]{0,63}$', clone_from): + return bad(handler, 'Invalid clone_from name') try: from api.profiles import create_profile_api result = create_profile_api( name, - clone_from=body.get('clone_from'), + clone_from=clone_from, clone_config=bool(body.get('clone_config', False)), ) return j(handler, {'ok': True, 'profile': result}) @@ -417,6 +425,8 @@ def handle_post(handler, parsed): return j(handler, result) except (ValueError, FileNotFoundError) as e: return bad(handler, str(e)) + except RuntimeError as e: + return bad(handler, str(e), 409) # ── Settings (POST) ── if parsed.path == '/api/settings': diff --git a/static/panels.js b/static/panels.js index fbfd9b5..6718679 100644 --- a/static/panels.js +++ b/static/panels.js @@ -477,6 +477,7 @@ function toggleWsDropdown(){ const open=dd.classList.contains('open'); if(open){closeWsDropdown();} else{ + closeProfileDropdown(); // close profile dropdown if open loadWorkspaceList().then(data=>{ renderWorkspaceDropdown(data.workspaces, S.session?S.session.workspace:''); dd.classList.add('open'); @@ -642,6 +643,7 @@ function toggleProfileDropdown() { const dd = $('profileDropdown'); if (!dd) return; if (dd.classList.contains('open')) { closeProfileDropdown(); return; } + closeWsDropdown(); // close workspace dropdown if open api('/api/profiles').then(data => { renderProfileDropdown(data); dd.classList.add('open');