From 732c995d91b6559d50c03c8cfe3aea571e830699 Mon Sep 17 00:00:00 2001 From: Dutch AI Agency Date: Sun, 3 May 2026 22:27:52 +0100 Subject: [PATCH 1/3] fix(#1560): refuse password change when HERMES_WEBUI_PASSWORD env var is set Settings password silently no-opped when HERMES_WEBUI_PASSWORD was set: the env var takes precedence in api.auth.get_password_hash(), but the UI happily POSTed _set_password and returned a green "Saved" toast while every subsequent login still required the env-var password. Same for Disable Auth (_clear_password=true). Backend (api/routes.py): - GET /api/settings now exposes password_env_var: bool so the UI knows the field is shadowed. - POST /api/settings refuses _set_password and _clear_password with HTTP 409 + a clear message naming HERMES_WEBUI_PASSWORD when the env var is set. Short-circuits BEFORE save_settings() so settings.json is not touched. Frontend (static/index.html, static/panels.js, static/i18n.js): - Added settingsPasswordEnvLock banner div in the System pane. - panels.js reads settings.password_env_var, disables the password field, swaps in a localized "locked" placeholder, reveals the banner, and hides the Disable Auth button (its POST would 409 anyway). - New i18n keys password_env_var_locked and password_env_var_locked_placeholder added to all 9 locales (en, ja, ru, es, de, zh, zh-Hant, pt, ko). Tests: - tests/test_issue1560_password_env_var_lock.py: requirement-pinning (handler exposes flag, 409 on set/clear, banner div, panels.js wiring, i18n in all 9 locales, env var name in messages, live HTTP smoke when env unset). - tests/test_1560_password_env_var_no_op.py: behavioral via FakeHandler (real status codes for env-set/unset/blank, settings.json hash unchanged after 409, panels.js disable+banner+placeholder+disable-auth-hidden). Both files run clean: 23 passed in 2.04s. test_issue1139_password_remote.py unaffected (4/4 still pass). --- api/routes.py | 22 ++ static/i18n.js | 18 + static/index.html | 1 + static/panels.js | 22 ++ tests/test_1560_password_env_var_no_op.py | 354 ++++++++++++++++++ tests/test_issue1560_password_env_var_lock.py | 189 ++++++++++ 6 files changed, 606 insertions(+) create mode 100644 tests/test_1560_password_env_var_no_op.py create mode 100644 tests/test_issue1560_password_env_var_lock.py diff --git a/api/routes.py b/api/routes.py index 0d1ab915..1c927620 100644 --- a/api/routes.py +++ b/api/routes.py @@ -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 diff --git a/static/i18n.js b/static/i18n.js index a5d435a1..f9f440b7 100644 --- a/static/i18n.js +++ b/static/i18n.js @@ -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 diff --git a/static/index.html b/static/index.html index 8e5f30c3..2d22a9f1 100644 --- a/static/index.html +++ b/static/index.html @@ -929,6 +929,7 @@
Enter a new password to set or change it. Leave blank to keep current setting.
+ diff --git a/static/panels.js b/static/panels.js index eaf4ab2e..32e4b365 100644 --- a/static/panels.js +++ b/static/panels.js @@ -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); diff --git a/tests/test_1560_password_env_var_no_op.py b/tests/test_1560_password_env_var_no_op.py new file mode 100644 index 00000000..2aac2a62 --- /dev/null +++ b/tests/test_1560_password_env_var_no_op.py @@ -0,0 +1,354 @@ +"""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 +import tempfile +from pathlib import Path +from urllib.parse import urlparse + +# Force a clean state dir before importing api.config / api.auth — both modules +# resolve STATE_DIR at import time. Mirrors the pattern in test_auth_sessions.py. +_TEST_STATE = Path(tempfile.mkdtemp(prefix="hermes-test-1560-")) +os.environ["HERMES_WEBUI_STATE_DIR"] = str(_TEST_STATE) + + +# ── 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 + + 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 `: {` 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}" + ) diff --git a/tests/test_issue1560_password_env_var_lock.py b/tests/test_issue1560_password_env_var_lock.py new file mode 100644 index 00000000..587a9327 --- /dev/null +++ b/tests/test_issue1560_password_env_var_lock.py @@ -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' From b6f6640b17ce96f5e3046779d473c51963684db9 Mon Sep 17 00:00:00 2001 From: Dutch AI Agency Date: Sun, 3 May 2026 22:51:12 +0100 Subject: [PATCH 2/3] fix(tests): isolate settings.json writes in #1560 tests to prevent CI bleed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CI failed across test_clarify_unblock + test_gateway_sync (~25 tests, all 401 Unauthorized) because two tests in this module write `password_hash` directly to the shared TEST_STATE_DIR/settings.json (the path the integration server reads): - `test_post_set_password_settings_hash_unchanged_after_409` seeds a sentinel hash to verify the 409 short-circuit doesn't overwrite it. - `test_post_set_password_succeeds_when_env_var_unset` goes through save_settings() with `_set_password`, persisting a real hash. After this module ran, the integration server saw `is_auth_enabled() == True` and rejected every subsequent request from test_clarify_unblock / test_gateway_sync with 401. Fix: - Add `_restore_settings_file_after_test` autouse fixture that snapshots cfg.SETTINGS_FILE before each test and restores it after, so password_hash writes don't leak to later tests. - Remove the misleading module-level `os.environ['HERMES_WEBUI_STATE_DIR']` override — api.config.STATE_DIR resolves at import time (already done by conftest.py before this module loads), so the override never reached the in-process state path it claimed to redirect. - Add `self.request = None` to FakeHandler so set_auth_cookie's `getattr(handler.request, 'getpeercert', None)` probe doesn't AttributeError on the success path of `_set_password` once settings are properly cleaned between tests (the prior CI pass relied on stale state bouncing the request with 401 before set_auth_cookie ran). Verified locally: 85 tests pass across test_1560_*, test_issue1560_*, test_clarify_unblock, test_gateway_sync (the previously-affected suites). --- tests/test_1560_password_env_var_no_op.py | 44 ++++++++++++++++++++--- 1 file changed, 39 insertions(+), 5 deletions(-) diff --git a/tests/test_1560_password_env_var_no_op.py b/tests/test_1560_password_env_var_no_op.py index 2aac2a62..c085108e 100644 --- a/tests/test_1560_password_env_var_no_op.py +++ b/tests/test_1560_password_env_var_no_op.py @@ -21,14 +21,40 @@ the silent-no-op UX bug. import io import json import os -import tempfile from pathlib import Path from urllib.parse import urlparse -# Force a clean state dir before importing api.config / api.auth — both modules -# resolve STATE_DIR at import time. Mirrors the pattern in test_auth_sessions.py. -_TEST_STATE = Path(tempfile.mkdtemp(prefix="hermes-test-1560-")) -os.environ["HERMES_WEBUI_STATE_DIR"] = str(_TEST_STATE) +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 ───────────────────── @@ -52,6 +78,14 @@ class _FakeHandler: } 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 From b852096dad58f43044ed47e88a50c0aef3fe3be7 Mon Sep 17 00:00:00 2001 From: Hermes Bot Date: Sun, 3 May 2026 21:09:08 +0000 Subject: [PATCH 3/3] =?UTF-8?q?release:=20stamp=20v0.50.286=20=E2=80=94=20?= =?UTF-8?q?PR=20#1561=20password=20env-var=20lock=20UI=20(4028=20=E2=86=92?= =?UTF-8?q?=204051=20tests)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- CHANGELOG.md | 19 +++++++++++++++++++ ROADMAP.md | 2 +- TESTING.md | 4 ++-- 3 files changed, 22 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d957545b..8ef32abd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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) diff --git a/ROADMAP.md b/ROADMAP.md index bb74013b..7351ff5b 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -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) diff --git a/TESTING.md b/TESTING.md index e16cf17e..465a0740 100644 --- a/TESTING.md +++ b/TESTING.md @@ -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: /*