mirror of
https://github.com/nesquena/hermes-webui.git
synced 2026-05-26 03:30:36 +00:00
Merge pull request #1564 from nesquena/stage-286
v0.50.286 — Settings password field env-var lock UI (closes #1560)
This commit is contained in:
@@ -1,5 +1,24 @@
|
||||
# Hermes Web UI -- Changelog
|
||||
|
||||
## [v0.50.286] — 2026-05-03
|
||||
|
||||
### Fixed (1 PR — closes #1560)
|
||||
|
||||
- **Settings password field silently no-ops when `HERMES_WEBUI_PASSWORD` env var is set** (#1561, @dutchaiagency; closes #1560 — resurfaced from #1139) — when `HERMES_WEBUI_PASSWORD` was exported, `api/auth.py:get_password_hash()` already returned the env-var hash and ignored `settings.json["password_hash"]`. But the Settings → System pane never knew this, so the password field accepted input, called the API, returned 200, and showed a green "Saved" toast — every subsequent login still required the env-var password. Same for "Disable Auth" / clearing the password. The save genuinely succeeded; it was just unreachable. **Fix — three layers:** (1) `GET /api/settings` now includes `password_env_var: bool(env)` so the UI can detect the locked state. Hash still stripped from response (existing invariant). (2) `POST /api/settings` refuses `_set_password` and `_clear_password` with **HTTP 409** + an explanatory message naming `HERMES_WEBUI_PASSWORD` when the env var is set. The 409 short-circuits BEFORE `save_settings()`, so the on-disk hash is never touched. Whitespace-only env values are not treated as set (matches `api/auth.py` `.strip()` guard). (3) Frontend (`static/index.html`, `static/panels.js`, `static/i18n.js`) — added `#settingsPasswordEnvLock` banner div in the System pane (hidden by default). When `password_env_var` is true: password input is `disabled`, value cleared, placeholder swapped to a localized "Locked: HERMES_WEBUI_PASSWORD env var is set" string; banner revealed; Disable Auth button hidden (its POST would 409 anyway); Sign Out stays available since it only clears the session cookie. 2 new i18n keys (`password_env_var_locked`, `password_env_var_locked_placeholder`) added to all 9 shipped locales (en, ja, ru, es, de, zh, zh-Hant, pt, ko). Each locale's banner string literally names `HERMES_WEBUI_PASSWORD` so users can grep their environment. 23 new regression tests in `tests/test_issue1560_password_env_var_lock.py` (12 tests) and `tests/test_1560_password_env_var_no_op.py` (11 tests) covering both the surfacing flag, the 409 refusal on both write paths, frontend lock behavior, and 9-locale parity. Pre-release Opus advisor pass. Maintainer-rebased from contributor's v0.50.283 base onto current master cleanly.
|
||||
|
||||
### Tests
|
||||
|
||||
4028 → **4051 passing** (+23 from PR #1561). 0 regressions. Full suite in 115s.
|
||||
|
||||
### Pre-release verification
|
||||
|
||||
- All 23 PR-1561 tests pass standalone in 3.6s.
|
||||
- All 4051 tests pass in the full suite (110s).
|
||||
- Browser sanity (HTTP API checks against port 8789): 11/11 endpoints verified.
|
||||
- All modified JS files (`static/i18n.js`, `static/panels.js`) pass `node -c` syntax check.
|
||||
- PR rebase verified clean: `git diff origin/master --stat` shows ONLY the 6 files PR #1561 touches (no spurious deletions of v0.50.284/v0.50.285 test files that the older PR base would have dropped).
|
||||
|
||||
|
||||
## [v0.50.285] — 2026-05-03
|
||||
|
||||
### Fixed (1 PR — same-day hotfix-of-hotfix)
|
||||
|
||||
+1
-1
@@ -2,7 +2,7 @@
|
||||
|
||||
> Web companion to the Hermes Agent CLI. Same workflows, browser-native.
|
||||
>
|
||||
> Last updated: v0.50.285 (May 03, 2026) — 4028 tests collected
|
||||
> Last updated: v0.50.286 (May 03, 2026) — 4051 tests collected
|
||||
> Test source: `pytest tests/ --collect-only -q`
|
||||
> Per-version detail: see [CHANGELOG.md](./CHANGELOG.md)
|
||||
|
||||
|
||||
+2
-2
@@ -1835,8 +1835,8 @@ Bridged CLI sessions:
|
||||
|
||||
---
|
||||
|
||||
*Last updated: v0.50.285, May 03, 2026*
|
||||
*Total automated tests collected: 4028*
|
||||
*Last updated: v0.50.286, May 03, 2026*
|
||||
*Total automated tests collected: 4051*
|
||||
*Regression gate: tests/test_regressions.py*
|
||||
*Run: pytest tests/ -v --timeout=60*
|
||||
*Source: <repo>/*
|
||||
|
||||
@@ -1705,6 +1705,13 @@ def handle_get(handler, parsed) -> bool:
|
||||
settings = load_settings()
|
||||
# Never expose the stored password hash to clients
|
||||
settings.pop("password_hash", None)
|
||||
# Surface env-var precedence so the UI can disable the password field
|
||||
# instead of silently no-oping the save (#1560). The setting takes
|
||||
# precedence in api.auth.get_password_hash(), but until now the UI
|
||||
# had no way to know — see issue #1139 / #1560.
|
||||
settings["password_env_var"] = bool(
|
||||
os.getenv("HERMES_WEBUI_PASSWORD", "").strip()
|
||||
)
|
||||
# Inject the running version so the UI badge stays in sync with git tags
|
||||
# without any manual release step.
|
||||
try:
|
||||
@@ -3049,6 +3056,21 @@ def handle_post(handler, parsed) -> bool:
|
||||
isinstance(body.get("_set_password"), str)
|
||||
and body.get("_set_password", "").strip()
|
||||
)
|
||||
requested_clear_password = bool(body.get("_clear_password"))
|
||||
|
||||
# #1560: HERMES_WEBUI_PASSWORD env var takes precedence in
|
||||
# api.auth.get_password_hash(), so writing password_hash to settings.json
|
||||
# has no effect on auth. Refuse loudly with 409 instead of silently
|
||||
# succeeding — the previous behaviour returned 200 + a green save toast
|
||||
# while every subsequent login still required the env-var password.
|
||||
if requested_password or requested_clear_password:
|
||||
if os.getenv("HERMES_WEBUI_PASSWORD", "").strip():
|
||||
return bad(
|
||||
handler,
|
||||
"HERMES_WEBUI_PASSWORD env var is set — it overrides the settings password. "
|
||||
"Unset the env var and restart the server before changing the password here.",
|
||||
409,
|
||||
)
|
||||
|
||||
saved = save_settings(body)
|
||||
saved.pop("password_hash", None) # never expose hash to client
|
||||
|
||||
@@ -529,6 +529,8 @@ const LOCALES = {
|
||||
settings_desc_bot_name: 'Display name for the assistant throughout the UI. Defaults to Hermes.',
|
||||
settings_desc_password: 'Enter a new password to set or change it. Leave blank to keep current setting.',
|
||||
password_placeholder: 'Enter new password…',
|
||||
password_env_var_locked: 'The HERMES_WEBUI_PASSWORD environment variable is currently set and takes precedence. Unset it and restart the server to manage the password from here.',
|
||||
password_env_var_locked_placeholder: 'Locked: HERMES_WEBUI_PASSWORD env var is set',
|
||||
disable_auth: 'Disable Auth',
|
||||
sign_out: 'Sign Out',
|
||||
// Providers panel
|
||||
@@ -1427,6 +1429,8 @@ const LOCALES = {
|
||||
settings_desc_bot_name: 'UI 全体で表示されるアシスタントの名前。デフォルトは Hermes。',
|
||||
settings_desc_password: '新しいパスワードを入力すると設定または変更します。空欄なら現在の設定を維持。',
|
||||
password_placeholder: '新しいパスワードを入力…',
|
||||
password_env_var_locked: '現在 HERMES_WEBUI_PASSWORD 環境変数が設定されており優先されます。ここで管理するには変数を解除してサーバーを再起動してください。',
|
||||
password_env_var_locked_placeholder: 'ロック中: HERMES_WEBUI_PASSWORD 環境変数が設定されています',
|
||||
disable_auth: '認証を無効化',
|
||||
sign_out: 'サインアウト',
|
||||
// Providers panel
|
||||
@@ -2134,6 +2138,8 @@ const LOCALES = {
|
||||
settings_desc_bot_name: 'Отображаемое имя помощника во всём интерфейсе. По умолчанию Hermes.',
|
||||
settings_desc_password: 'Введите новый пароль, чтобы задать или изменить его. Оставьте пустым, чтобы сохранить текущую настройку.',
|
||||
password_placeholder: 'Введите новый пароль…',
|
||||
password_env_var_locked: 'Переменная окружения HERMES_WEBUI_PASSWORD сейчас задана и имеет приоритет. Сбросьте её и перезапустите сервер, чтобы управлять паролем отсюда.',
|
||||
password_env_var_locked_placeholder: 'Заблокировано: задана переменная HERMES_WEBUI_PASSWORD',
|
||||
disable_auth: 'Отключить авторизацию',
|
||||
sign_out: 'Выйти',
|
||||
// Providers panel (English fallback — native translations welcome in follow-up PRs)
|
||||
@@ -2969,6 +2975,8 @@ const LOCALES = {
|
||||
settings_desc_bot_name: 'Nombre visible del asistente en toda la UI. Por defecto es Hermes.',
|
||||
settings_desc_password: 'Introduce una nueva contraseña para establecerla o cambiarla. Déjalo en blanco para mantener la configuración actual.',
|
||||
password_placeholder: 'Introduce una contraseña nueva…',
|
||||
password_env_var_locked: 'La variable de entorno HERMES_WEBUI_PASSWORD está definida y tiene prioridad. Quítala y reinicia el servidor para gestionar la contraseña desde aquí.',
|
||||
password_env_var_locked_placeholder: 'Bloqueado: la variable HERMES_WEBUI_PASSWORD está definida',
|
||||
disable_auth: 'Desactivar autenticación',
|
||||
sign_out: 'Cerrar sesión',
|
||||
// Providers panel (English fallback — native translations welcome in follow-up PRs)
|
||||
@@ -3783,6 +3791,8 @@ const LOCALES = {
|
||||
settings_desc_bot_name: 'Anzeigename für den Assistenten in der UI. Standardmäßig Hermes.',
|
||||
settings_desc_password: 'Geben Sie ein neues Passwort ein, um es zu setzen oder zu ändern. Leer lassen, um die aktuelle Einstellung beizubehalten.',
|
||||
password_placeholder: 'Neues Passwort eingeben…',
|
||||
password_env_var_locked: 'Die Umgebungsvariable HERMES_WEBUI_PASSWORD ist gesetzt und hat Vorrang. Entferne sie und starte den Server neu, um das Passwort hier zu verwalten.',
|
||||
password_env_var_locked_placeholder: 'Gesperrt: HERMES_WEBUI_PASSWORD-Umgebungsvariable ist gesetzt',
|
||||
disable_auth: 'Authentifizierung deaktivieren',
|
||||
sign_out: 'Abmelden',
|
||||
// Providers panel (English fallback — native translations welcome in follow-up PRs)
|
||||
@@ -4648,6 +4658,8 @@ const LOCALES = {
|
||||
providers_key_placeholder_new: 'sk-...',
|
||||
providers_key_placeholder_replace: 'Enter new key to replace…',
|
||||
password_placeholder: '输入新密码…',
|
||||
password_env_var_locked: '当前已设置 HERMES_WEBUI_PASSWORD 环境变量并具有优先级。请取消该变量并重启服务器,才能在此管理密码。',
|
||||
password_env_var_locked_placeholder: '已锁定:已设置 HERMES_WEBUI_PASSWORD 环境变量',
|
||||
disable_auth: '停用认证',
|
||||
settings_label_sound: '通知声音',
|
||||
settings_label_notifications: '浏览器通知',
|
||||
@@ -5466,6 +5478,8 @@ const LOCALES = {
|
||||
suggest_files: '這個工作區有哪些檔案?',
|
||||
sign_out: '\u767b\u51fa',
|
||||
password_placeholder: '\u5bc6\u78bc',
|
||||
password_env_var_locked: '\u76ee\u524d\u5df2\u8a2d\u5b9a HERMES_WEBUI_PASSWORD \u74b0\u5883\u8b8a\u6578\u4e14\u512a\u5148\u751f\u6548\u3002\u8acb\u53d6\u6d88\u8a2d\u5b9a\u4e26\u91cd\u65b0\u555f\u52d5\u4f3a\u670d\u5668\uff0c\u624d\u80fd\u5728\u6b64\u7ba1\u7406\u5bc6\u78bc\u3002',
|
||||
password_env_var_locked_placeholder: '\u5df2\u9396\u5b9a\uff1a\u5df2\u8a2d\u5b9a HERMES_WEBUI_PASSWORD \u74b0\u5883\u8b8a\u6578',
|
||||
disable_auth: '\u505c\u7528\u9a57\u8b49',
|
||||
settings_label_sound: '\u901a\u77e5\u8072\u97f3',
|
||||
settings_label_notifications: '\u700f\u89bd\u901a\u77e5',
|
||||
@@ -6474,6 +6488,8 @@ const LOCALES = {
|
||||
settings_desc_bot_name: 'Nome de exibição do assistente. Padrão: Hermes.',
|
||||
settings_desc_password: 'Digite nova senha para definir ou trocar. Deixe em branco para manter.',
|
||||
password_placeholder: 'Digite nova senha…',
|
||||
password_env_var_locked: 'A variável de ambiente HERMES_WEBUI_PASSWORD está definida e tem prioridade. Remova-a e reinicie o servidor para gerenciar a senha aqui.',
|
||||
password_env_var_locked_placeholder: 'Bloqueado: variável HERMES_WEBUI_PASSWORD está definida',
|
||||
disable_auth: 'Desativar Auth',
|
||||
sign_out: 'Sair',
|
||||
// Providers panel
|
||||
@@ -7278,6 +7294,8 @@ const LOCALES = {
|
||||
settings_desc_bot_name: 'UI 전체에 표시되는 Assistant 이름입니다. 기본값은 Hermes입니다.',
|
||||
settings_desc_password: '새 비밀번호를 설정하거나 변경하려면 입력하세요. 현재 설정을 유지하려면 비워 두세요.',
|
||||
password_placeholder: '새 비밀번호 입력…',
|
||||
password_env_var_locked: '현재 HERMES_WEBUI_PASSWORD 환경 변수가 설정되어 있어 우선 적용됩니다. 변수를 해제하고 서버를 재시작해야 여기에서 비밀번호를 관리할 수 있습니다.',
|
||||
password_env_var_locked_placeholder: '잠금: HERMES_WEBUI_PASSWORD 환경 변수가 설정되어 있습니다',
|
||||
disable_auth: '인증 비활성화',
|
||||
sign_out: '로그아웃',
|
||||
// Providers panel
|
||||
|
||||
@@ -929,6 +929,7 @@
|
||||
<label for="settingsPassword" data-i18n="settings_label_password">Access Password</label>
|
||||
<div style="font-size:11px;color:var(--muted);margin-bottom:6px" data-i18n="settings_desc_password">Enter a new password to set or change it. Leave blank to keep current setting.</div>
|
||||
<input type="password" id="settingsPassword" placeholder="Enter new password…" data-i18n-placeholder="password_placeholder" style="width:100%;padding:8px;background:var(--code-bg);color:var(--text);border:1px solid var(--border2);border-radius:6px;font-size:13px">
|
||||
<div id="settingsPasswordEnvLock" data-i18n="password_env_var_locked" style="display:none;margin-top:6px;padding:8px 10px;font-size:11px;color:var(--muted);background:var(--code-bg);border:1px solid var(--border2);border-radius:6px;line-height:1.45">The HERMES_WEBUI_PASSWORD environment variable is currently set and takes precedence. Unset it and restart the server to manage the password from here.</div>
|
||||
</div>
|
||||
<button class="sm-btn" id="btnDisableAuth" onclick="disableAuth()" style="margin-top:6px;width:100%;padding:8px;font-weight:600;color:#e8a030;border-color:rgba(232,160,48,.3);display:none" data-i18n="disable_auth">Disable Auth</button>
|
||||
<button class="sm-btn" id="btnSignOut" onclick="signOut()" style="margin-top:6px;width:100%;padding:8px;font-weight:600;color:var(--accent);border-color:rgba(233,69,96,.3);display:none" data-i18n="sign_out">Sign Out</button>
|
||||
|
||||
@@ -3163,11 +3163,33 @@ async function loadSettingsPanel(){
|
||||
// Password field: always blank (we don't send hash back)
|
||||
const pwField=$('settingsPassword');
|
||||
if(pwField){pwField.value='';pwField.addEventListener('input',_markSettingsDirty,{once:false});}
|
||||
// #1560: when HERMES_WEBUI_PASSWORD env var is set, the settings password
|
||||
// field silently no-ops. Disable it + reveal the lock banner so the UI
|
||||
// tells the truth before a user tries (and the backend now also returns
|
||||
// 409 as defense-in-depth).
|
||||
const pwEnvLocked=!!settings.password_env_var;
|
||||
const pwLockBanner=$('settingsPasswordEnvLock');
|
||||
if(pwField){
|
||||
pwField.disabled=pwEnvLocked;
|
||||
if(pwEnvLocked){
|
||||
pwField.value='';
|
||||
pwField.placeholder=t('password_env_var_locked_placeholder')||pwField.placeholder;
|
||||
}
|
||||
}
|
||||
if(pwLockBanner) pwLockBanner.style.display=pwEnvLocked?'block':'none';
|
||||
// Show auth buttons only when auth is active
|
||||
try{
|
||||
const authStatus=await api('/api/auth/status');
|
||||
_setSettingsAuthButtonsVisible(!!authStatus.auth_enabled);
|
||||
}catch(e){}
|
||||
// #1560: env-var-locked password also disables the Disable Auth button —
|
||||
// clearing settings.password_hash is silent no-op when the env var is set,
|
||||
// and the backend now returns 409 anyway, so don't offer the action.
|
||||
// Sign Out remains available since it only clears the session cookie.
|
||||
if(pwEnvLocked){
|
||||
const disableBtn=$('btnDisableAuth');
|
||||
if(disableBtn) disableBtn.style.display='none';
|
||||
}
|
||||
_syncHermesPanelSessionActions();
|
||||
loadProvidersPanel(); // load provider cards in background
|
||||
switchSettingsSection(_settingsSection);
|
||||
|
||||
@@ -0,0 +1,388 @@
|
||||
"""Regression tests for issue #1560 — Settings password silently no-ops when
|
||||
HERMES_WEBUI_PASSWORD env var is set.
|
||||
|
||||
Pre-fix behaviour: env-var-precedence in `api.auth.get_password_hash()` meant
|
||||
that POST /api/settings with `_set_password` would happily persist a new hash
|
||||
to settings.json AND return 200 + "Saved" — but every subsequent login still
|
||||
required the env-var password. Same for `_clear_password` ("Disable Auth").
|
||||
|
||||
Fix is two-layer:
|
||||
- Backend: GET /api/settings now exposes `password_env_var: bool`; POST
|
||||
/api/settings refuses with 409 when the env var is set and the request
|
||||
asks for `_set_password` or `_clear_password`.
|
||||
- Frontend: when `password_env_var` is true, panels.js disables the password
|
||||
input, hides the Disable Auth button, and reveals a lock-banner explaining
|
||||
that the env var must be unset and the server restarted.
|
||||
|
||||
These tests pin both layers so a future refactor can't silently re-introduce
|
||||
the silent-no-op UX bug.
|
||||
"""
|
||||
|
||||
import io
|
||||
import json
|
||||
import os
|
||||
from pathlib import Path
|
||||
from urllib.parse import urlparse
|
||||
|
||||
import pytest
|
||||
|
||||
|
||||
# ── Settings-file isolation ──────────────────────────────────────────────────
|
||||
#
|
||||
# Several tests in this module write password_hash directly to the shared
|
||||
# settings.json (test_post_set_password_settings_hash_unchanged_after_409 seeds
|
||||
# a sentinel, test_post_set_password_succeeds_when_env_var_unset goes through
|
||||
# save_settings). Without isolation, those writes leak into TEST_STATE_DIR/
|
||||
# settings.json (the path the integration server subprocess started by
|
||||
# conftest.py reads from), which flips is_auth_enabled() to True for every
|
||||
# subsequent test in the session and cascades to 401 across test_clarify_unblock,
|
||||
# test_gateway_sync, etc.
|
||||
#
|
||||
# Snapshot-and-restore is preferred over redirecting SETTINGS_FILE because
|
||||
# load_settings() / save_settings() bind to the module-level Path object
|
||||
# captured at import time and the fixture must work regardless of import order.
|
||||
@pytest.fixture(autouse=True)
|
||||
def _restore_settings_file_after_test():
|
||||
import api.config as cfg
|
||||
|
||||
original = (
|
||||
cfg.SETTINGS_FILE.read_text(encoding="utf-8")
|
||||
if cfg.SETTINGS_FILE.exists()
|
||||
else None
|
||||
)
|
||||
yield
|
||||
if original is not None:
|
||||
cfg.SETTINGS_FILE.write_text(original, encoding="utf-8")
|
||||
elif cfg.SETTINGS_FILE.exists():
|
||||
cfg.SETTINGS_FILE.unlink()
|
||||
|
||||
|
||||
# ── FakeHandler that supports GET *and* POST body reading ─────────────────────
|
||||
|
||||
class _FakeHandler:
|
||||
"""Minimal BaseHTTPRequestHandler stand-in for routes.handle_get/handle_post.
|
||||
|
||||
Exposes wfile/headers/rfile so the real handlers can read request bodies
|
||||
and write JSON responses. The only mutation we observe in tests is `status`
|
||||
+ the JSON written to `wfile`.
|
||||
"""
|
||||
|
||||
def __init__(self, body_bytes: bytes = b"", cookie: str = ""):
|
||||
self.status = None
|
||||
self.sent_headers = []
|
||||
self.body = bytearray()
|
||||
self.wfile = self
|
||||
self.rfile = io.BytesIO(body_bytes)
|
||||
self.headers = {
|
||||
"Content-Length": str(len(body_bytes)),
|
||||
}
|
||||
if cookie:
|
||||
self.headers["Cookie"] = cookie
|
||||
# set_auth_cookie() probes handler.request.getpeercert / X-Forwarded-Proto
|
||||
# to decide whether to emit the Secure flag. The default
|
||||
# BaseHTTPRequestHandler exposes a `.request` socket; FakeHandler is
|
||||
# transport-less, so expose a plain None — getattr(None, ...) is safe
|
||||
# and the resulting cookie is plain (non-Secure), which is what tests
|
||||
# care about. Without this attribute, save_settings → set_auth_cookie
|
||||
# raises AttributeError on the success path of `_set_password`.
|
||||
self.request = None
|
||||
|
||||
def send_response(self, status):
|
||||
self.status = status
|
||||
|
||||
def send_header(self, name, value):
|
||||
self.sent_headers.append((name, value))
|
||||
|
||||
def end_headers(self):
|
||||
pass
|
||||
|
||||
def write(self, data):
|
||||
self.body.extend(data)
|
||||
|
||||
def header(self, name):
|
||||
for key, value in self.sent_headers:
|
||||
if key.lower() == name.lower():
|
||||
return value
|
||||
return None
|
||||
|
||||
def json_body(self):
|
||||
return json.loads(bytes(self.body).decode("utf-8"))
|
||||
|
||||
|
||||
# ── Backend: GET /api/settings exposes password_env_var ──────────────────────
|
||||
|
||||
def test_get_settings_exposes_password_env_var_true_when_env_set(monkeypatch):
|
||||
"""Acceptance criterion: GET /api/settings includes `password_env_var: true`
|
||||
when HERMES_WEBUI_PASSWORD is set."""
|
||||
monkeypatch.setenv("HERMES_WEBUI_PASSWORD", "shadow-pw")
|
||||
|
||||
from api.routes import handle_get
|
||||
|
||||
handler = _FakeHandler()
|
||||
parsed = urlparse("http://example.com/api/settings")
|
||||
handle_get(handler, parsed)
|
||||
assert handler.status == 200
|
||||
|
||||
payload = handler.json_body()
|
||||
assert payload.get("password_env_var") is True, (
|
||||
"GET /api/settings must expose password_env_var=true when "
|
||||
"HERMES_WEBUI_PASSWORD is set so the UI can disable the password field. "
|
||||
f"Got: {payload!r}"
|
||||
)
|
||||
# Also confirm the hash is never echoed back to the client (existing
|
||||
# invariant — pinned here to catch a future change that surfaces it
|
||||
# alongside the new flag).
|
||||
assert "password_hash" not in payload
|
||||
|
||||
|
||||
def test_get_settings_password_env_var_false_when_env_unset(monkeypatch):
|
||||
"""Control case: env var unset → password_env_var:false (falsy)."""
|
||||
monkeypatch.delenv("HERMES_WEBUI_PASSWORD", raising=False)
|
||||
|
||||
from api.routes import handle_get
|
||||
|
||||
handler = _FakeHandler()
|
||||
parsed = urlparse("http://example.com/api/settings")
|
||||
handle_get(handler, parsed)
|
||||
assert handler.status == 200
|
||||
|
||||
payload = handler.json_body()
|
||||
assert payload.get("password_env_var") is False
|
||||
|
||||
|
||||
def test_get_settings_password_env_var_false_when_env_blank(monkeypatch):
|
||||
"""Whitespace-only env var must NOT shadow settings — matches the strip()
|
||||
guard in api.auth.get_password_hash."""
|
||||
monkeypatch.setenv("HERMES_WEBUI_PASSWORD", " ")
|
||||
|
||||
from api.routes import handle_get
|
||||
|
||||
handler = _FakeHandler()
|
||||
parsed = urlparse("http://example.com/api/settings")
|
||||
handle_get(handler, parsed)
|
||||
assert handler.status == 200
|
||||
|
||||
payload = handler.json_body()
|
||||
assert payload.get("password_env_var") is False
|
||||
|
||||
|
||||
# ── Backend: POST /api/settings returns 409 when env var shadows ─────────────
|
||||
|
||||
def _post_settings(body_dict, cookie=""):
|
||||
"""Helper: POST a JSON body to /api/settings via handle_post."""
|
||||
from api.routes import handle_post
|
||||
raw = json.dumps(body_dict).encode("utf-8")
|
||||
handler = _FakeHandler(body_bytes=raw, cookie=cookie)
|
||||
parsed = urlparse("http://example.com/api/settings")
|
||||
handle_post(handler, parsed)
|
||||
return handler
|
||||
|
||||
|
||||
def test_post_set_password_returns_409_when_env_var_set(monkeypatch):
|
||||
"""Acceptance criterion: POST `_set_password` returns 409 when env var is set,
|
||||
with a message naming HERMES_WEBUI_PASSWORD so the user knows what to fix."""
|
||||
monkeypatch.setenv("HERMES_WEBUI_PASSWORD", "shadow-pw")
|
||||
|
||||
handler = _post_settings({"_set_password": "new-attempt"})
|
||||
|
||||
assert handler.status == 409, (
|
||||
f"POST _set_password must return 409 when env var is set, got {handler.status}"
|
||||
)
|
||||
payload = handler.json_body()
|
||||
assert "HERMES_WEBUI_PASSWORD" in payload.get("error", ""), (
|
||||
"409 error message must name HERMES_WEBUI_PASSWORD so the user can "
|
||||
f"identify the override. Got: {payload!r}"
|
||||
)
|
||||
|
||||
|
||||
def test_post_clear_password_returns_409_when_env_var_set(monkeypatch):
|
||||
"""Acceptance criterion: POST `_clear_password=true` ("Disable Auth") returns
|
||||
409 when env var is set — disabling auth via UI is impossible while the env
|
||||
var is in force."""
|
||||
monkeypatch.setenv("HERMES_WEBUI_PASSWORD", "shadow-pw")
|
||||
|
||||
handler = _post_settings({"_clear_password": True})
|
||||
|
||||
assert handler.status == 409
|
||||
payload = handler.json_body()
|
||||
assert "HERMES_WEBUI_PASSWORD" in payload.get("error", "")
|
||||
|
||||
|
||||
def test_post_set_password_settings_hash_unchanged_after_409(monkeypatch):
|
||||
"""Acceptance criterion: env var set + POST `_set_password` → 409 +
|
||||
settings.json `password_hash` unchanged.
|
||||
|
||||
Pre-fix the write happened anyway (silently); post-fix the 409 short-circuits
|
||||
BEFORE save_settings(), so any pre-existing password_hash on disk must
|
||||
survive untouched.
|
||||
"""
|
||||
monkeypatch.setenv("HERMES_WEBUI_PASSWORD", "shadow-pw")
|
||||
|
||||
# Seed settings.json with a known sentinel hash so we can detect any write.
|
||||
from api.config import load_settings, save_settings
|
||||
# Don't go through save_settings (it would re-route _set_password) — write
|
||||
# the file directly via the same path load_settings reads from.
|
||||
import api.config as cfg
|
||||
sentinel_hash = "deadbeef" * 8 # 64 chars, matches PBKDF2 hex output shape
|
||||
settings_before = load_settings()
|
||||
settings_before["password_hash"] = sentinel_hash
|
||||
cfg.SETTINGS_FILE.parent.mkdir(parents=True, exist_ok=True)
|
||||
cfg.SETTINGS_FILE.write_text(
|
||||
json.dumps(settings_before, indent=2), encoding="utf-8"
|
||||
)
|
||||
|
||||
handler = _post_settings({"_set_password": "new-attempt"})
|
||||
assert handler.status == 409
|
||||
|
||||
settings_after = load_settings()
|
||||
assert settings_after.get("password_hash") == sentinel_hash, (
|
||||
"settings.json password_hash must be UNCHANGED after a 409-rejected "
|
||||
"POST _set_password — fix must short-circuit BEFORE save_settings(). "
|
||||
f"Got: before={sentinel_hash!r} after={settings_after.get('password_hash')!r}"
|
||||
)
|
||||
|
||||
|
||||
def test_post_set_password_succeeds_when_env_var_unset(monkeypatch):
|
||||
"""Control case: env var unset → POST _set_password is NOT a 409.
|
||||
|
||||
We don't pin the success status (200) tightly because the response path
|
||||
sets a session cookie and may use a special status flow; the important
|
||||
invariant is that the 409 guard ONLY fires when the env var is set.
|
||||
"""
|
||||
monkeypatch.delenv("HERMES_WEBUI_PASSWORD", raising=False)
|
||||
|
||||
handler = _post_settings({"_set_password": "fresh-pw"})
|
||||
|
||||
assert handler.status != 409, (
|
||||
"POST _set_password without env var must NOT trigger the #1560 409 "
|
||||
f"guard. Got status={handler.status}"
|
||||
)
|
||||
|
||||
|
||||
# ── Frontend: index.html, panels.js, i18n.js wiring ──────────────────────────
|
||||
|
||||
REPO_ROOT = Path(__file__).parent.parent
|
||||
INDEX_HTML = (REPO_ROOT / "static" / "index.html").read_text(encoding="utf-8")
|
||||
PANELS_JS = (REPO_ROOT / "static" / "panels.js").read_text(encoding="utf-8")
|
||||
I18N_JS = (REPO_ROOT / "static" / "i18n.js").read_text(encoding="utf-8")
|
||||
|
||||
|
||||
def test_index_html_has_password_lock_banner_div():
|
||||
"""index.html must include the lock-banner div with i18n key, hidden by
|
||||
default, inside the System pane near the password field."""
|
||||
# The banner must exist with the i18n key panels.js looks up
|
||||
assert 'id="settingsPasswordEnvLock"' in INDEX_HTML
|
||||
assert 'data-i18n="password_env_var_locked"' in INDEX_HTML
|
||||
# Default-hidden; panels.js reveals it when settings.password_env_var is true.
|
||||
assert 'settingsPasswordEnvLock' in INDEX_HTML
|
||||
# Sanity: banner sits inside the System pane (same context as the password
|
||||
# field) — this guards against a future refactor that moves the banner away
|
||||
# from the field it explains.
|
||||
sys_start = INDEX_HTML.index('id="settingsPaneSystem"')
|
||||
pwlock_start = INDEX_HTML.index('id="settingsPasswordEnvLock"')
|
||||
assert pwlock_start > sys_start, (
|
||||
"Lock banner must be inside the System settings pane (after "
|
||||
"settingsPaneSystem opens) so it shows next to the password field."
|
||||
)
|
||||
|
||||
|
||||
def test_panels_js_disables_password_field_when_env_locked():
|
||||
"""panels.js loadSettingsPanel must read settings.password_env_var and
|
||||
disable the password field + reveal the lock banner."""
|
||||
assert "password_env_var" in PANELS_JS, (
|
||||
"panels.js must read settings.password_env_var from GET /api/settings."
|
||||
)
|
||||
assert "settingsPasswordEnvLock" in PANELS_JS, (
|
||||
"panels.js must toggle the visibility of #settingsPasswordEnvLock."
|
||||
)
|
||||
# The password input must be disabled when locked.
|
||||
assert "pwField.disabled" in PANELS_JS or "disabled=pwEnvLocked" in PANELS_JS
|
||||
|
||||
|
||||
def test_panels_js_hides_disable_auth_when_env_locked():
|
||||
"""panels.js must hide the Disable Auth button when env-var-locked — its
|
||||
POST would 409 anyway and the UI shouldn't offer an action that can't
|
||||
succeed."""
|
||||
# Look for a section that toggles btnDisableAuth visibility based on the
|
||||
# env-lock flag.
|
||||
assert "btnDisableAuth" in PANELS_JS
|
||||
# The simplest signal: a guard that hides btnDisableAuth when pwEnvLocked
|
||||
# is true. We don't pin the exact JS expression (style.display, hidden,
|
||||
# classList — implementer's choice), but the symbol pair must co-occur.
|
||||
pw_lock_idx = PANELS_JS.find("pwEnvLocked")
|
||||
assert pw_lock_idx != -1, "panels.js must compute pwEnvLocked"
|
||||
# btnDisableAuth must be referenced in a region where pwEnvLocked is in
|
||||
# scope (same loadSettingsPanel function body — within ±3000 chars).
|
||||
btn_idx = PANELS_JS.find("btnDisableAuth")
|
||||
assert abs(btn_idx - pw_lock_idx) < 4000, (
|
||||
"btnDisableAuth handling must live near the pwEnvLocked computation "
|
||||
"in loadSettingsPanel; otherwise the env-lock state can't gate the "
|
||||
"button visibility."
|
||||
)
|
||||
|
||||
|
||||
def test_panels_js_uses_locked_placeholder_i18n_key():
|
||||
"""The locked-state input placeholder must come from the i18n key —
|
||||
matches the t('password_env_var_locked_placeholder') call site."""
|
||||
assert "password_env_var_locked_placeholder" in PANELS_JS
|
||||
|
||||
|
||||
# ── i18n keys present in all 9 locales ───────────────────────────────────────
|
||||
|
||||
# All locales currently shipped in static/i18n.js. Issue #1560 lists 9 locales
|
||||
# (en/es/de/zh/zh-Hant/ru/ja/fr/pt). The repo currently ships 9 locales but
|
||||
# substitutes 'ko' for 'fr' — we test what the repo actually has, not what the
|
||||
# issue body lists, so a future addition of fr won't fail the suite either.
|
||||
EXPECTED_LOCALES = ("en", "ja", "ru", "es", "de", "zh", "zh-Hant", "pt", "ko")
|
||||
|
||||
|
||||
def _locale_block(locale_key: str) -> str:
|
||||
"""Return the slice of i18n.js between `<key>: {` and the next top-level
|
||||
locale opener (or end-of-file). Good enough for substring assertions."""
|
||||
# Locale openers look like ` en: {` or ` 'zh-Hant': {` (two-space indent).
|
||||
if "-" in locale_key:
|
||||
opener = f" '{locale_key}':"
|
||||
else:
|
||||
opener = f" {locale_key}:"
|
||||
start = I18N_JS.index(opener)
|
||||
# Find the next locale opener, scanning all known locales.
|
||||
rest = I18N_JS[start + len(opener):]
|
||||
next_starts = []
|
||||
for other in EXPECTED_LOCALES:
|
||||
if other == locale_key:
|
||||
continue
|
||||
cand_opener = f" '{other}':" if "-" in other else f" {other}:"
|
||||
idx = rest.find(cand_opener)
|
||||
if idx >= 0:
|
||||
next_starts.append(idx)
|
||||
end = min(next_starts) if next_starts else len(rest)
|
||||
return rest[:end]
|
||||
|
||||
|
||||
def test_password_env_var_locked_key_present_in_all_locales():
|
||||
"""The lock-banner translation key must exist in every shipped locale —
|
||||
otherwise users on those locales see [object Object] / undefined / the
|
||||
raw HTML default instead of the help text."""
|
||||
missing = []
|
||||
for locale in EXPECTED_LOCALES:
|
||||
block = _locale_block(locale)
|
||||
if "password_env_var_locked:" not in block:
|
||||
missing.append(locale)
|
||||
assert not missing, (
|
||||
f"password_env_var_locked translation key missing in locales: {missing}"
|
||||
)
|
||||
|
||||
|
||||
def test_password_env_var_locked_placeholder_key_present_in_all_locales():
|
||||
"""The locked-input placeholder translation key must exist in every shipped
|
||||
locale so the disabled input field never shows English fallback to non-EN
|
||||
users."""
|
||||
missing = []
|
||||
for locale in EXPECTED_LOCALES:
|
||||
block = _locale_block(locale)
|
||||
if "password_env_var_locked_placeholder:" not in block:
|
||||
missing.append(locale)
|
||||
assert not missing, (
|
||||
"password_env_var_locked_placeholder translation key missing in "
|
||||
f"locales: {missing}"
|
||||
)
|
||||
@@ -0,0 +1,189 @@
|
||||
"""Tests for issue #1560 — Settings password silently no-ops when HERMES_WEBUI_PASSWORD env var is set.
|
||||
|
||||
Root cause: HERMES_WEBUI_PASSWORD takes precedence in api.auth.get_password_hash(),
|
||||
but the UI had no way to know — POST /api/settings happily wrote password_hash to
|
||||
settings.json, returned 200 + "Saved" toast, while every subsequent login still
|
||||
required the env-var password.
|
||||
|
||||
Fix: surface env-var precedence in GET /api/settings (`password_env_var: bool`),
|
||||
refuse the write loudly (409) when shadowed, disable the field + show help-text
|
||||
banner in the UI, with i18n keys in all 9 locales.
|
||||
"""
|
||||
|
||||
import json
|
||||
import os
|
||||
import pathlib
|
||||
import urllib.error
|
||||
import urllib.request
|
||||
|
||||
REPO = pathlib.Path(__file__).parent.parent
|
||||
|
||||
|
||||
def _read(rel_path):
|
||||
return (REPO / rel_path).read_text(encoding='utf-8')
|
||||
|
||||
|
||||
# ── Backend (api/routes.py) ───────────────────────────────────────────────
|
||||
|
||||
|
||||
def test_get_settings_surfaces_password_env_var_flag():
|
||||
"""GET /api/settings handler must include `password_env_var: bool(env)`."""
|
||||
src = _read('api/routes.py')
|
||||
# Locate the GET /api/settings block (by handler comment + path string)
|
||||
start = src.index('if parsed.path == "/api/settings":')
|
||||
# Block ends at next top-level `if parsed.path == ...` or `if parsed.path.startswith`
|
||||
end = src.index('if parsed.path', start + 50)
|
||||
block = src[start:end]
|
||||
|
||||
assert 'password_env_var' in block, \
|
||||
'GET /api/settings must expose password_env_var so UI can disable the field'
|
||||
assert 'HERMES_WEBUI_PASSWORD' in block, \
|
||||
'GET /api/settings must read HERMES_WEBUI_PASSWORD env var'
|
||||
|
||||
|
||||
def test_post_settings_refuses_set_password_when_env_var_shadowed():
|
||||
"""POST /api/settings with _set_password must return 409 when env var is set."""
|
||||
src = _read('api/routes.py')
|
||||
# The guard lives near the POST /api/settings handler; locate it via the
|
||||
# canonical error-message substring (defense-in-depth comment + bad() call).
|
||||
assert 'HERMES_WEBUI_PASSWORD env var is set' in src, \
|
||||
'POST /api/settings must refuse with a clear message naming the env var'
|
||||
assert '409' in src, 'POST /api/settings must use HTTP 409 for env-var conflict'
|
||||
|
||||
|
||||
def test_post_settings_refuses_clear_password_when_env_var_shadowed():
|
||||
"""POST /api/settings with _clear_password=true must also be refused."""
|
||||
src = _read('api/routes.py')
|
||||
# Same guard must cover both paths
|
||||
assert '_clear_password' in src
|
||||
# Find the guard and verify it tests both flags
|
||||
guard_idx = src.index('HERMES_WEBUI_PASSWORD env var is set')
|
||||
# Look back ~2KB for the conditional that triggers the guard
|
||||
window = src[max(0, guard_idx - 2000):guard_idx]
|
||||
assert 'requested_password' in window or '_set_password' in window
|
||||
assert 'requested_clear_password' in window or '_clear_password' in window, \
|
||||
'guard must cover both _set_password and _clear_password'
|
||||
|
||||
|
||||
# ── Frontend: lock UI elements (static/index.html) ────────────────────────
|
||||
|
||||
|
||||
def test_settings_html_has_password_env_lock_banner():
|
||||
"""The settings password block must include a hidden lock banner element."""
|
||||
html = _read('static/index.html')
|
||||
assert 'id="settingsPasswordEnvLock"' in html, \
|
||||
'settingsPasswordEnvLock banner element required (revealed when env var set)'
|
||||
assert 'data-i18n="password_env_var_locked"' in html, \
|
||||
'banner must use the i18n key password_env_var_locked'
|
||||
|
||||
|
||||
# ── Frontend: env-locked logic (static/panels.js) ─────────────────────────
|
||||
|
||||
|
||||
def test_panels_js_disables_password_when_env_locked():
|
||||
"""panels.js must disable the password field and show the banner when password_env_var is true."""
|
||||
src = _read('static/panels.js')
|
||||
assert 'password_env_var' in src, \
|
||||
'panels.js must read settings.password_env_var from GET /api/settings'
|
||||
assert 'settingsPasswordEnvLock' in src, \
|
||||
'panels.js must toggle the settingsPasswordEnvLock banner'
|
||||
# The disable logic should set pwField.disabled
|
||||
assert 'pwField.disabled' in src or 'disabled=pwEnvLocked' in src.replace(' ', ''), \
|
||||
'password field must be disabled when env-locked'
|
||||
|
||||
|
||||
def test_panels_js_hides_disable_auth_button_when_env_locked():
|
||||
"""The Disable Auth button must be hidden when env var shadows the settings password."""
|
||||
src = _read('static/panels.js')
|
||||
# When env-locked, btnDisableAuth should be set display:none
|
||||
# We verify by locating the env-locked block and checking it touches btnDisableAuth
|
||||
idx = src.index('pwEnvLocked')
|
||||
# Look in a window after the first env-locked reference for btnDisableAuth handling
|
||||
window = src[idx:idx + 3000]
|
||||
assert 'btnDisableAuth' in window, \
|
||||
'Disable Auth button must be hidden in the env-locked code path'
|
||||
|
||||
|
||||
# ── i18n: keys present in all 9 locales (static/i18n.js) ──────────────────
|
||||
|
||||
|
||||
LOCALES = ['en', 'ja', 'ru', 'es', 'de', 'zh', 'zh-Hant', 'pt', 'ko']
|
||||
|
||||
|
||||
def _split_locales(i18n_src):
|
||||
"""Split i18n.js into per-locale source slices.
|
||||
|
||||
Locale block headers look like ` en: {` or ` 'zh-Hant': {`. We slice each
|
||||
block from its header to the next sibling header at the same indentation.
|
||||
"""
|
||||
import re
|
||||
pattern = re.compile(r"^ ['\"]?([\w\-]+)['\"]?: \{$", re.MULTILINE)
|
||||
matches = list(pattern.finditer(i18n_src))
|
||||
blocks = {}
|
||||
for i, m in enumerate(matches):
|
||||
name = m.group(1)
|
||||
start = m.start()
|
||||
end = matches[i + 1].start() if i + 1 < len(matches) else len(i18n_src)
|
||||
blocks[name] = i18n_src[start:end]
|
||||
return blocks
|
||||
|
||||
|
||||
def test_i18n_password_env_var_locked_in_all_locales():
|
||||
"""Every locale must define the password_env_var_locked banner string."""
|
||||
src = _read('static/i18n.js')
|
||||
blocks = _split_locales(src)
|
||||
missing = [loc for loc in LOCALES if loc not in blocks
|
||||
or 'password_env_var_locked:' not in blocks[loc]]
|
||||
assert not missing, \
|
||||
f"Locales missing password_env_var_locked: {missing}"
|
||||
|
||||
|
||||
def test_i18n_password_env_var_locked_placeholder_in_all_locales():
|
||||
"""Every locale must define the password_env_var_locked_placeholder string."""
|
||||
src = _read('static/i18n.js')
|
||||
blocks = _split_locales(src)
|
||||
missing = [loc for loc in LOCALES
|
||||
if loc not in blocks
|
||||
or 'password_env_var_locked_placeholder:' not in blocks[loc]]
|
||||
assert not missing, \
|
||||
f"Locales missing password_env_var_locked_placeholder: {missing}"
|
||||
|
||||
|
||||
def test_i18n_locked_string_mentions_env_var_name_in_all_locales():
|
||||
"""Each locale's banner must literally mention HERMES_WEBUI_PASSWORD so users can find it."""
|
||||
src = _read('static/i18n.js')
|
||||
blocks = _split_locales(src)
|
||||
for loc in LOCALES:
|
||||
block = blocks.get(loc, '')
|
||||
# Find the password_env_var_locked entry
|
||||
idx = block.find('password_env_var_locked:')
|
||||
assert idx != -1, f"{loc}: missing password_env_var_locked"
|
||||
# Take the rest of that line (the message string)
|
||||
line_end = block.index('\n', idx)
|
||||
line = block[idx:line_end]
|
||||
assert 'HERMES_WEBUI_PASSWORD' in line, \
|
||||
f"{loc}: banner must literally name HERMES_WEBUI_PASSWORD"
|
||||
|
||||
|
||||
# ── Live HTTP smoke test (env var NOT set in pytest) ──────────────────────
|
||||
|
||||
|
||||
def test_get_settings_returns_password_env_var_false_when_unset():
|
||||
"""When HERMES_WEBUI_PASSWORD is not set in the test process,
|
||||
GET /api/settings must include `password_env_var: False`."""
|
||||
# The conftest server inherits this process's env; verify it's clean.
|
||||
assert not os.getenv('HERMES_WEBUI_PASSWORD', '').strip(), \
|
||||
'this test requires HERMES_WEBUI_PASSWORD to be unset'
|
||||
|
||||
from tests._pytest_port import BASE
|
||||
req = urllib.request.Request(BASE + '/api/settings')
|
||||
try:
|
||||
with urllib.request.urlopen(req, timeout=10) as r:
|
||||
payload = json.loads(r.read())
|
||||
except urllib.error.HTTPError as e:
|
||||
payload = json.loads(e.read())
|
||||
|
||||
assert 'password_env_var' in payload, \
|
||||
'GET /api/settings must always include password_env_var key'
|
||||
assert payload['password_env_var'] is False, \
|
||||
'env var unset => password_env_var must be False'
|
||||
Reference in New Issue
Block a user