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.
This commit is contained in:
@@ -397,11 +397,19 @@ def handle_post(handler, parsed):
|
|||||||
if parsed.path == '/api/profile/create':
|
if parsed.path == '/api/profile/create':
|
||||||
name = body.get('name', '').strip()
|
name = body.get('name', '').strip()
|
||||||
if not name: return bad(handler, 'name is required')
|
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:
|
try:
|
||||||
from api.profiles import create_profile_api
|
from api.profiles import create_profile_api
|
||||||
result = create_profile_api(
|
result = create_profile_api(
|
||||||
name,
|
name,
|
||||||
clone_from=body.get('clone_from'),
|
clone_from=clone_from,
|
||||||
clone_config=bool(body.get('clone_config', False)),
|
clone_config=bool(body.get('clone_config', False)),
|
||||||
)
|
)
|
||||||
return j(handler, {'ok': True, 'profile': result})
|
return j(handler, {'ok': True, 'profile': result})
|
||||||
@@ -417,6 +425,8 @@ def handle_post(handler, parsed):
|
|||||||
return j(handler, result)
|
return j(handler, result)
|
||||||
except (ValueError, FileNotFoundError) as e:
|
except (ValueError, FileNotFoundError) as e:
|
||||||
return bad(handler, str(e))
|
return bad(handler, str(e))
|
||||||
|
except RuntimeError as e:
|
||||||
|
return bad(handler, str(e), 409)
|
||||||
|
|
||||||
# ── Settings (POST) ──
|
# ── Settings (POST) ──
|
||||||
if parsed.path == '/api/settings':
|
if parsed.path == '/api/settings':
|
||||||
|
|||||||
@@ -477,6 +477,7 @@ function toggleWsDropdown(){
|
|||||||
const open=dd.classList.contains('open');
|
const open=dd.classList.contains('open');
|
||||||
if(open){closeWsDropdown();}
|
if(open){closeWsDropdown();}
|
||||||
else{
|
else{
|
||||||
|
closeProfileDropdown(); // close profile dropdown if open
|
||||||
loadWorkspaceList().then(data=>{
|
loadWorkspaceList().then(data=>{
|
||||||
renderWorkspaceDropdown(data.workspaces, S.session?S.session.workspace:'');
|
renderWorkspaceDropdown(data.workspaces, S.session?S.session.workspace:'');
|
||||||
dd.classList.add('open');
|
dd.classList.add('open');
|
||||||
@@ -642,6 +643,7 @@ function toggleProfileDropdown() {
|
|||||||
const dd = $('profileDropdown');
|
const dd = $('profileDropdown');
|
||||||
if (!dd) return;
|
if (!dd) return;
|
||||||
if (dd.classList.contains('open')) { closeProfileDropdown(); return; }
|
if (dd.classList.contains('open')) { closeProfileDropdown(); return; }
|
||||||
|
closeWsDropdown(); // close workspace dropdown if open
|
||||||
api('/api/profiles').then(data => {
|
api('/api/profiles').then(data => {
|
||||||
renderProfileDropdown(data);
|
renderProfileDropdown(data);
|
||||||
dd.classList.add('open');
|
dd.classList.add('open');
|
||||||
|
|||||||
Reference in New Issue
Block a user