fix(settings): a plugin's password is sealed at rest, and a blank box keeps it (#5671)

The UniFi password sat in plugin.unifi in plain text: SECRET_KEYS covers
whole settings, and a plugin's settings are one dict. Saving the plugin's
settings page also wiped it, since the password box is never filled back
in and its blank value was stored.

- A plugin field whose name holds password, secret or token is a
  credential (is_plugin_secret). set_setting seals it inside the dict,
  every read opens it, startup seals any plaintext left from before, and
  the undecryptable-secrets banner names it as plugin.<name>.<field>.
- The settings page uses the same test for its password boxes, and a blank
  box keeps the saved value.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

steward/app.py

@@ -155,6 +155,9 @@ def create_app(
     for _name in ("metric_label", "metric_about", "metric_unit", "metric_value", "source_label",
                   "cert_left", "field_label"):
         app.jinja_env.globals[_name] = getattr(_fmt, _name)
+    # Which plugin settings fields are credentials (a password box, never echoed).
+    from .core.settings import is_plugin_secret as _is_plugin_secret
+    app.jinja_env.globals["is_plugin_secret"] = _is_plugin_secret
     from .alerts.service import rule_condition as _rule_condition
     app.jinja_env.globals["rule_condition"] = _rule_condition
     from .models.monitors import MONITOR_TYPE_LABELS as _type_labels

steward/core/settings.py

@@ -204,6 +204,11 @@ def _decode(value: Any, key: str = "") -> Any:
     exact setting that needs re-entering.
     """
     from steward.core.crypto import decrypt_secret, is_encrypted
+    if key.startswith(PLUGIN_PREFIX) and isinstance(value, dict):
+        name = key[len(PLUGIN_PREFIX):]
+        return {f: decrypt_secret(v, context=plugin_secret_key(name, f))
+                if is_plugin_secret(f) and is_encrypted(v) else v
+                for f, v in value.items()}
     return decrypt_secret(value, context=key) if is_encrypted(value) else value
 
 
@@ -260,6 +265,38 @@ def source_token_key(name: str) -> str:
     return f"{SOURCES_KEY}[{name}].http_token"
 
 
+# A plugin's settings are one dict under plugin.<name>; a field whose name
+# says it holds a credential is sealed inside it, field by field. The same
+# test decides that the settings page shows it as a password box.
+PLUGIN_PREFIX = "plugin."
+_PLUGIN_SECRET_WORDS = ("password", "secret", "token")
+
+
+def is_plugin_secret(field: str) -> bool:
+    """Whether a plugin config field holds a credential: unifi's password,
+    any *_secret or *_token."""
+    return any(w in field for w in _PLUGIN_SECRET_WORDS)
+
+
+def seal_plugin_secrets(cfg: Any) -> tuple[Any, int]:
+    """Encrypt each plaintext secret field of one plugin's settings dict;
+    returns (cfg, how many)."""
+    from steward.core.crypto import encrypt_secret, is_encrypted
+    if not isinstance(cfg, dict):
+        return cfg, 0
+    sealed, count = dict(cfg), 0
+    for field, value in cfg.items():
+        if is_plugin_secret(field) and isinstance(value, str) and value and not is_encrypted(value):
+            sealed[field] = encrypt_secret(value)
+            count += 1
+    return sealed, count
+
+
+def plugin_secret_key(name: str, field: str) -> str:
+    """How a plugin's secret is named in logs and the undecryptable-secrets banner."""
+    return f"{PLUGIN_PREFIX}{name}.{field}"
+
+
 # ─────────────────────────────────────────────────────────────────────────────
 # Async helpers (use inside request handlers / scheduled tasks)
 # ─────────────────────────────────────────────────────────────────────────────
@@ -281,6 +318,8 @@ async def set_setting(session: AsyncSession, key: str, value: Any) -> None:
     if key in SECRET_KEYS and isinstance(value, str) and value:
         from steward.core.crypto import encrypt_secret
         to_store = encrypt_secret(value)
+    elif key.startswith(PLUGIN_PREFIX):
+        to_store, _ = seal_plugin_secrets(value)
     result = await session.execute(
         select(AppSetting).where(AppSetting.key == key)
     )
@@ -569,6 +608,12 @@ def migrate_plaintext_secrets(db_url: str) -> int:
                         if sealed:
                             row.value_json = json.dumps(sources)
                             converted += sealed
+                    for row in (await session.execute(select(AppSetting).where(
+                            AppSetting.key.startswith(PLUGIN_PREFIX)))).scalars():
+                        cfg, sealed = seal_plugin_secrets(json.loads(row.value_json))
+                        if sealed:
+                            row.value_json = json.dumps(cfg)
+                            converted += sealed
         finally:
             await engine.dispose()
         return converted
@@ -605,6 +650,13 @@ def scan_undecryptable_secrets(db_url: str) -> list[str]:
                         key = source_token_key(src.get("name", "?"))
                         if _is_undecryptable(src.get("http_token"), key):
                             bad.append(key)
+                for row in (await session.execute(select(AppSetting).where(
+                        AppSetting.key.startswith(PLUGIN_PREFIX)))).scalars():
+                    cfg = json.loads(row.value_json)
+                    for field, value in (cfg.items() if isinstance(cfg, dict) else ()):
+                        key = plugin_secret_key(row.key[len(PLUGIN_PREFIX):], field)
+                        if is_plugin_secret(field) and _is_undecryptable(value, key):
+                            bad.append(key)
         finally:
             await engine.dispose()
         return bad

steward/settings/routes.py

@@ -15,6 +15,7 @@ from steward.core.settings import (
     get_stored_plugin_names, find_orphaned_plugins,
     to_smtp_cfg, to_webhook_cfg, to_ansible_cfg, to_plugins_cfg,
     to_oidc_cfg, to_ldap_cfg, to_thresholds_cfg, to_access_cfg, DEFAULTS, seal_source_tokens,
+    is_plugin_secret,
 )
 
 settings_bp = Blueprint("settings", __name__, url_prefix="/settings")
@@ -830,6 +831,12 @@ async def save_plugin_detail(name: str):
     old_enabled = to_plugins_cfg(old_settings).get(name, {}).get("enabled", False)
 
     plugin_cfg = _build_plugin_cfg_from_form(plugin, form)
+    # A secret is never sent back to the browser, so its box arrives blank
+    # unless someone typed a new one: blank keeps the saved value.
+    old_cfg = to_plugins_cfg(old_settings).get(name, {})
+    for field, value in list(plugin_cfg.items()):
+        if is_plugin_secret(field) and value == "" and old_cfg.get(field):
+            plugin_cfg[field] = old_cfg[field]
 
     async with current_app.db_sessionmaker() as db:
         async with db.begin():

steward/templates/settings/plugin_detail.html

@@ -108,10 +108,11 @@
         {% else %}
           <div class="form-group" style="margin:0;">
             <label style="font-size:0.82rem;">{{ field_label(cfg_key) }}</label>
-            <input type="{% if 'password' in cfg_key or 'secret' in cfg_key %}password{% else %}text{% endif %}"
+            {% set secret = is_plugin_secret(cfg_key) %}
+            <input type="{% if secret %}password{% else %}text{% endif %}"
                    name="plugin.{{ plugin._dir }}.{{ cfg_key }}"
-                   value="{% if 'password' in cfg_key or 'secret' in cfg_key %}{% else %}{{ plugin._config.get(cfg_key, cfg_default) }}{% endif %}"
-                   {% if 'password' in cfg_key or 'secret' in cfg_key %}placeholder="••••••••"{% endif %}>
+                   value="{% if secret %}{% else %}{{ plugin._config.get(cfg_key, cfg_default) }}{% endif %}"
+                   {% if secret %}autocomplete="new-password" placeholder="{{ 'kept; type to change' if plugin._config.get(cfg_key) else 'none saved' }}"{% endif %}>
           </div>
         {% endif %}
 

tests/core/test_plugin_secrets.py

@@ -0,0 +1,34 @@
+"""A plugin's credentials are sealed inside its settings dict (#5671): the
+UniFi password was stored in plain text, so a backup would have carried it."""
+from steward.core.crypto import init_crypto, is_encrypted
+from steward.core.settings import _decode, is_plugin_secret, seal_plugin_secrets
+
+
+def setup_function():
+    init_crypto("unit-test-secret-key")
+
+
+def test_which_fields_are_credentials():
+    assert is_plugin_secret("password") and is_plugin_secret("client_secret")
+    assert is_plugin_secret("api_token")
+    assert not is_plugin_secret("username") and not is_plugin_secret("host")
+
+
+def test_sealing_touches_only_plaintext_secrets_and_reading_opens_them():
+    cfg = {"enabled": True, "host": "https://10.0.0.1", "username": "admin",
+           "password": "hunter2", "poll_interval_seconds": 60}
+    sealed, n = seal_plugin_secrets(cfg)
+    assert n == 1 and is_encrypted(sealed["password"])
+    assert {k: v for k, v in sealed.items() if k != "password"} == {
+        k: v for k, v in cfg.items() if k != "password"}
+    again, n2 = seal_plugin_secrets(sealed)          # idempotent
+    assert n2 == 0 and again == sealed
+    assert _decode(sealed, "plugin.unifi") == cfg
+    assert seal_plugin_secrets({"password": ""}) == ({"password": ""}, 0)
+    assert seal_plugin_secrets("not a dict") == ("not a dict", 0)
+
+
+def test_only_plugin_settings_are_opened():
+    # A dict under a non-plugin key is left alone: only plugin.* is opened.
+    sealed, _ = seal_plugin_secrets({"password": "x"})
+    assert _decode(sealed, "thresholds.cpu") == sealed

tests/integration/test_plugin_secrets.py

@@ -0,0 +1,91 @@
+"""Integration: a plugin's password is ciphertext in app_settings, reads back
+as itself, is sealed at startup if it was stored in plain text, and is kept
+when the settings form is saved with its box blank (#5671). Requires
+STEWARD_DATABASE_URL."""
+from __future__ import annotations
+
+import asyncio
+import json
+import os
+import time
+import uuid
+
+import pytest
+
+pytestmark = pytest.mark.integration
+
+
+@pytest.fixture
+def app():
+    if not os.environ.get("STEWARD_DATABASE_URL"):
+        pytest.skip("needs Postgres")
+    from steward.app import create_app
+    return create_app(testing=False)
+
+
+def test_a_plugin_password_is_sealed_read_back_and_kept_on_a_blank_save(app):
+    from sqlalchemy import delete, select
+
+    from steward.core.crypto import is_encrypted
+    from steward.core.settings import get_setting, migrate_plaintext_secrets, set_setting
+    from steward.models.settings import AppSetting
+    from steward.models.users import User, UserRole
+
+    async def raw(key):
+        async with app.db_sessionmaker() as s:
+            row = (await s.execute(select(AppSetting).where(AppSetting.key == key))).scalar_one()
+            return json.loads(row.value_json)
+
+    async def go():
+        tag = uuid.uuid4().hex[:8]
+        cfg = {"enabled": False, "host": "https://10.9.9.1", "username": "admin",
+               "password": f"pw-{tag}", "site": "default", "verify_ssl": False,
+               "poll_interval_seconds": 60, "top_clients": 10}
+        async with app.db_sessionmaker() as s:
+            async with s.begin():
+                await set_setting(s, "plugin.unifi", cfg)
+        stored = await raw("plugin.unifi")
+        assert is_encrypted(stored["password"]) and stored["username"] == "admin"
+        async with app.db_sessionmaker() as s:
+            assert (await get_setting(s, "plugin.unifi"))["password"] == f"pw-{tag}"
+
+        # A row written before this fix, in plain text, is sealed at startup.
+        async with app.db_sessionmaker() as s:
+            async with s.begin():
+                row = (await s.execute(select(AppSetting).where(AppSetting.key == "plugin.unifi"))).scalar_one()
+                row.value_json = json.dumps(cfg)
+        assert not is_encrypted((await raw("plugin.unifi"))["password"])
+        await asyncio.to_thread(migrate_plaintext_secrets, app.config["DATABASE_URL"])
+        assert is_encrypted((await raw("plugin.unifi"))["password"])
+
+        # Saving the settings page with the password box blank keeps it.
+        async with app.db_sessionmaker() as s:
+            async with s.begin():
+                user = User(username=f"ps-{tag}", email=f"ps-{tag}@test", password_hash="x",
+                            role=UserRole.admin, is_active=True)
+                s.add(user)
+            uid = user.id
+        client = app.test_client()
+        async with client.session_transaction() as sess:
+            sess.update(user_id=uid, user_role="admin", username=f"ps-{tag}",
+                        _csrf="test-token", login_at=time.time())
+        resp = await client.post("/settings/plugins/unifi/", form={
+            "csrf_token": "test-token", "plugin.unifi.host": "https://10.9.9.1",
+            "plugin.unifi.username": "admin", "plugin.unifi.password": "",
+            "plugin.unifi.site": "default", "plugin.unifi.poll_interval_seconds": "120",
+            "plugin.unifi.top_clients": "10"})
+        assert resp.status_code in (200, 302, 303), await resp.get_data(as_text=True)
+        async with app.db_sessionmaker() as s:
+            after = await get_setting(s, "plugin.unifi")
+        assert after["password"] == f"pw-{tag}" and after["poll_interval_seconds"] == 120
+        assert is_encrypted((await raw("plugin.unifi"))["password"])
+
+        page = await (await client.get("/settings/plugins/unifi/")).get_data(as_text=True)
+        assert f"pw-{tag}" not in page and "kept; type to change" in page
+
+        async with app.db_sessionmaker() as s:
+            async with s.begin():
+                await s.execute(delete(AppSetting).where(AppSetting.key == "plugin.unifi"))
+                await s.execute(delete(User).where(User.id == uid))
+
+    asyncio.run(go())