fix: harden bot_name — crash guard, XSS escape, sanitization, tests
- Move `import html` to module top (was inside function body) - Fix IndexError crash in /login when bot_name is empty string; use `or 'Hermes'` fallback instead of .get() default which doesn't guard against stored empty string - Add server-side sanitization in POST /api/settings: strip + default empty/whitespace bot_name to 'Hermes' before persisting - Escape _bn initial char in ui.js innerHTML (esc() consistency) - Add maxlength=64 to #settingsBotName input field - Add tests/test_sprint27.py: 9 tests covering API round-trip, empty/whitespace defaults, login page rendering, and XSS escaping
This commit is contained in:
@@ -2,6 +2,7 @@
|
||||
Hermes Web UI -- Route handlers for GET and POST endpoints.
|
||||
Extracted from server.py (Sprint 11) so server.py is a thin shell.
|
||||
"""
|
||||
import html as _html
|
||||
import json
|
||||
import os
|
||||
import queue
|
||||
@@ -116,8 +117,7 @@ def handle_get(handler, parsed) -> bool:
|
||||
content_type='text/html; charset=utf-8')
|
||||
|
||||
if parsed.path == '/login':
|
||||
import html as _html
|
||||
_bn = _html.escape(load_settings().get('bot_name', 'Hermes'))
|
||||
_bn = _html.escape(load_settings().get('bot_name') or 'Hermes')
|
||||
_page = _LOGIN_PAGE_HTML.replace('{{BOT_NAME}}', _bn).replace('{{BOT_NAME_INITIAL}}', _bn[0].upper())
|
||||
return t(handler, _page, content_type='text/html; charset=utf-8')
|
||||
|
||||
@@ -526,6 +526,8 @@ def handle_post(handler, parsed) -> bool:
|
||||
|
||||
# ── Settings (POST) ──
|
||||
if parsed.path == '/api/settings':
|
||||
if 'bot_name' in body:
|
||||
body['bot_name'] = (str(body['bot_name']) or '').strip() or 'Hermes'
|
||||
saved = save_settings(body)
|
||||
saved.pop('password_hash', None) # never expose hash to client
|
||||
return j(handler, saved)
|
||||
|
||||
@@ -375,7 +375,7 @@
|
||||
<div class="settings-field">
|
||||
<label for="settingsBotName">Assistant Name</label>
|
||||
<div style="font-size:11px;color:var(--muted);margin-bottom:6px">Display name for the assistant throughout the UI. Defaults to Hermes.</div>
|
||||
<input type="text" id="settingsBotName" placeholder="Hermes" style="width:100%;padding:8px;background:var(--code-bg);color:var(--text);border:1px solid var(--border2);border-radius:6px;font-size:13px">
|
||||
<input type="text" id="settingsBotName" placeholder="Hermes" maxlength="64" style="width:100%;padding:8px;background:var(--code-bg);color:var(--text);border:1px solid var(--border2);border-radius:6px;font-size:13px">
|
||||
</div>
|
||||
<div class="settings-field" style="border-top:1px solid var(--border);padding-top:12px;margin-top:8px">
|
||||
<label for="settingsPassword">Access Password</label>
|
||||
|
||||
@@ -506,7 +506,7 @@ function renderMessages(){
|
||||
const tsVal=m._ts||m.timestamp;
|
||||
const tsTitle=tsVal?new Date(tsVal*1000).toLocaleString():'';
|
||||
const _bn=window._botName||'Hermes';
|
||||
row.innerHTML=`<div class="msg-role ${m.role}" ${tsTitle?`title="${esc(tsTitle)}"`:''}><div class="role-icon ${m.role}">${isUser?'Y':_bn.charAt(0).toUpperCase()}</div><span style="font-size:12px">${isUser?'You':esc(_bn)}</span>${tsTitle?`<span class="msg-time">${new Date(tsVal*1000).toLocaleTimeString([],{hour:'2-digit',minute:'2-digit'})}</span>`:''}<span class="msg-actions">${editBtn}<button class="msg-copy-btn msg-action-btn" title="Copy" onclick="copyMsg(this)">📋</button>${retryBtn}</span></div>${filesHtml}<div class="msg-body">${bodyHtml}</div>`;
|
||||
row.innerHTML=`<div class="msg-role ${m.role}" ${tsTitle?`title="${esc(tsTitle)}"`:''}><div class="role-icon ${m.role}">${isUser?'Y':esc(_bn.charAt(0).toUpperCase())}</div><span style="font-size:12px">${isUser?'You':esc(_bn)}</span>${tsTitle?`<span class="msg-time">${new Date(tsVal*1000).toLocaleTimeString([],{hour:'2-digit',minute:'2-digit'})}</span>`:''}<span class="msg-actions">${editBtn}<button class="msg-copy-btn msg-action-btn" title="Copy" onclick="copyMsg(this)">📋</button>${retryBtn}</span></div>${filesHtml}<div class="msg-body">${bodyHtml}</div>`;
|
||||
row.dataset.rawText = String(content).trim();
|
||||
inner.appendChild(row);
|
||||
}
|
||||
|
||||
136
tests/test_sprint27.py
Normal file
136
tests/test_sprint27.py
Normal file
@@ -0,0 +1,136 @@
|
||||
"""
|
||||
Sprint 27 Tests: configurable assistant display name (bot_name).
|
||||
Tests cover settings API round-trip, empty/missing input defaults,
|
||||
login page rendering, and server-side sanitization.
|
||||
"""
|
||||
import json
|
||||
import urllib.error
|
||||
import urllib.request
|
||||
|
||||
BASE = "http://127.0.0.1:8788"
|
||||
|
||||
|
||||
def get(path):
|
||||
with urllib.request.urlopen(BASE + path, timeout=10) as r:
|
||||
return json.loads(r.read()), r.status
|
||||
|
||||
|
||||
def get_raw(path):
|
||||
with urllib.request.urlopen(BASE + path, timeout=10) as r:
|
||||
return r.read().decode(), r.status
|
||||
|
||||
|
||||
def post(path, body=None):
|
||||
data = json.dumps(body or {}).encode()
|
||||
req = urllib.request.Request(BASE + path, data=data,
|
||||
headers={"Content-Type": "application/json"})
|
||||
try:
|
||||
with urllib.request.urlopen(req, timeout=10) as r:
|
||||
return json.loads(r.read()), r.status
|
||||
except urllib.error.HTTPError as e:
|
||||
return json.loads(e.read()), e.code
|
||||
|
||||
|
||||
# ── Default value ─────────────────────────────────────────────────────────
|
||||
|
||||
def test_settings_default_bot_name():
|
||||
"""GET /api/settings should return bot_name defaulting to 'Hermes'."""
|
||||
d, status = get("/api/settings")
|
||||
assert status == 200
|
||||
assert "bot_name" in d
|
||||
assert d["bot_name"] == "Hermes"
|
||||
|
||||
|
||||
# ── Round-trip ────────────────────────────────────────────────────────────
|
||||
|
||||
def test_settings_set_bot_name():
|
||||
"""POST /api/settings with bot_name should persist and round-trip."""
|
||||
try:
|
||||
d, status = post("/api/settings", {"bot_name": "TestBot"})
|
||||
assert status == 200
|
||||
assert d.get("bot_name") == "TestBot"
|
||||
d2, _ = get("/api/settings")
|
||||
assert d2.get("bot_name") == "TestBot"
|
||||
finally:
|
||||
post("/api/settings", {"bot_name": "Hermes"})
|
||||
|
||||
|
||||
def test_settings_bot_name_special_chars():
|
||||
"""bot_name with safe special characters should persist correctly."""
|
||||
try:
|
||||
d, status = post("/api/settings", {"bot_name": "My Assistant 2.0"})
|
||||
assert status == 200
|
||||
d2, _ = get("/api/settings")
|
||||
assert d2.get("bot_name") == "My Assistant 2.0"
|
||||
finally:
|
||||
post("/api/settings", {"bot_name": "Hermes"})
|
||||
|
||||
|
||||
# ── Server-side sanitization ──────────────────────────────────────────────
|
||||
|
||||
def test_settings_empty_bot_name_defaults_to_hermes():
|
||||
"""Posting an empty bot_name should default to 'Hermes' server-side."""
|
||||
try:
|
||||
d, status = post("/api/settings", {"bot_name": ""})
|
||||
assert status == 200
|
||||
assert d.get("bot_name") == "Hermes"
|
||||
d2, _ = get("/api/settings")
|
||||
assert d2.get("bot_name") == "Hermes"
|
||||
finally:
|
||||
post("/api/settings", {"bot_name": "Hermes"})
|
||||
|
||||
|
||||
def test_settings_whitespace_bot_name_defaults_to_hermes():
|
||||
"""Posting a whitespace-only bot_name should default to 'Hermes'."""
|
||||
try:
|
||||
d, status = post("/api/settings", {"bot_name": " "})
|
||||
assert status == 200
|
||||
assert d.get("bot_name") == "Hermes"
|
||||
finally:
|
||||
post("/api/settings", {"bot_name": "Hermes"})
|
||||
|
||||
|
||||
# ── Login page rendering ──────────────────────────────────────────────────
|
||||
|
||||
def test_login_page_shows_default_bot_name():
|
||||
"""GET /login should contain 'Hermes' in title and h1 when default."""
|
||||
html, status = get_raw("/login")
|
||||
assert status == 200
|
||||
assert "<title>Hermes" in html
|
||||
assert "<h1>Hermes</h1>" in html
|
||||
|
||||
|
||||
def test_login_page_shows_custom_bot_name():
|
||||
"""GET /login should reflect the configured bot_name."""
|
||||
try:
|
||||
post("/api/settings", {"bot_name": "Aria"})
|
||||
html, status = get_raw("/login")
|
||||
assert status == 200
|
||||
assert "<title>Aria" in html
|
||||
assert "<h1>Aria</h1>" in html
|
||||
finally:
|
||||
post("/api/settings", {"bot_name": "Hermes"})
|
||||
|
||||
|
||||
def test_login_page_empty_name_does_not_crash():
|
||||
"""Login page must not 500 even if somehow bot_name is empty in settings."""
|
||||
# Force an empty value by patching settings file directly — skipped here
|
||||
# because the server-side guard in POST /api/settings prevents storing empty.
|
||||
# Instead, verify that /login returns 200 reliably.
|
||||
html, status = get_raw("/login")
|
||||
assert status == 200
|
||||
assert "Sign in" in html
|
||||
|
||||
|
||||
def test_login_page_xss_escaped():
|
||||
"""bot_name with HTML special chars should be escaped in the login page."""
|
||||
try:
|
||||
post("/api/settings", {"bot_name": "<script>alert(1)</script>"})
|
||||
html, status = get_raw("/login")
|
||||
assert status == 200
|
||||
# Raw tag must not appear unescaped
|
||||
assert "<script>alert(1)</script>" not in html
|
||||
# Escaped form should appear
|
||||
assert "<script>" in html
|
||||
finally:
|
||||
post("/api/settings", {"bot_name": "Hermes"})
|
||||
Reference in New Issue
Block a user