Security: stop GET /api/settings from shipping decrypted secrets to the browser

Found during the #832 audit: GET /api/settings returned dict(config_data) — and
config_data is DECRYPTED in memory — so every API key, OAuth secret, Plex/
Jellyfin token, and service password went to the browser in cleartext. Fernet
"encrypted at rest" protects a leaked DB file; it does nothing once the API
hands the plaintext to the client (devtools, HAR captures, an XSS, a screen
share, or a non-PIN'd LAN viewer).

Fix (centralized in ConfigManager):
- redacted_config() deep-copies config and replaces every _SENSITIVE_PATHS value
  that's actually set with REDACTED_SENTINEL; unset secrets stay empty so the UI
  still shows "not configured". Dict-valued secrets (tidal/qobuz OAuth sessions)
  collapse to the sentinel too. GET /api/settings now serves this copy.
- set() ignores a write of REDACTED_SENTINEL to a sensitive path, so the masked
  placeholder round-tripped by an unchanged settings form can never overwrite
  the real secret. A real value still saves; an empty value still clears.

Frontend: secret inputs are type=password, so the sentinel renders as dots
(looks like a saved secret). _wireRedactedSecrets() clears the mask on focus so
editing types fresh rather than onto the sentinel, and re-masks on blur if left
untouched — so an unchanged secret round-trips the sentinel (kept), an edited
one saves the new value, and a deliberately emptied one clears.

Tests: every sensitive path masks; unset stays empty; dict secrets mask; live
config not mutated; sentinel round-trip keeps the real secret; real value
overwrites; empty clears; sentinel on a non-secret path writes normally.
9 new tests; 518 config-touching tests pass (1 pre-existing soundcloud mock
failure, unrelated — fails identically on a clean tree).

config/settings.py

@@ -66,6 +66,12 @@ class ConfigManager:
         
         self._load_config()
 
+    # Placeholder shipped to the browser in place of a configured secret
+    # (#832 follow-up). The settings UI shows it as masked dots; if it's
+    # round-tripped back on save, ``set()`` treats it as "keep existing" so the
+    # real value is never overwritten by the mask.
+    REDACTED_SENTINEL = '__redacted_unchanged__'
+
     # Dot-notation paths to sensitive config values that must be encrypted at rest.
     # Paths pointing to dicts encrypt the entire dict as a JSON blob.
     _SENSITIVE_PATHS = frozenset({
@@ -792,7 +798,40 @@ class ConfigManager:
 
         return value
 
+    def redacted_config(self) -> Dict[str, Any]:
+        """Deep copy of the live config with every sensitive value masked.
+
+        Used for ``GET /api/settings`` so decrypted secrets never reach the
+        browser (#832 follow-up). A *set* secret becomes ``REDACTED_SENTINEL``
+        (the UI renders it as masked dots); an unset one stays empty so the UI
+        can show "not configured". Dict-valued secrets (OAuth sessions) collapse
+        to the sentinel too — the UI has no field for them anyway. The matching
+        guard in ``set()`` turns a round-tripped sentinel back into a no-op.
+        """
+        import copy
+        data = copy.deepcopy(self.config_data)
+        for path in self._SENSITIVE_PATHS:
+            keys = path.split('.')
+            parent = data
+            for k in keys[:-1]:
+                if isinstance(parent, dict) and k in parent:
+                    parent = parent[k]
+                else:
+                    parent = None
+                    break
+            if not isinstance(parent, dict):
+                continue
+            leaf = keys[-1]
+            if leaf in parent and parent[leaf] not in (None, '', {}, [], 0, False):
+                parent[leaf] = self.REDACTED_SENTINEL
+        return data
+
     def set(self, key: str, value: Any):
+        # The UI round-trips REDACTED_SENTINEL for any secret the user didn't
+        # touch — never let the mask overwrite the real value (#832 follow-up).
+        if value == self.REDACTED_SENTINEL and key in self._SENSITIVE_PATHS:
+            return
+
         keys = key.split('.')
         config = self.config_data
 

tests/test_settings_redaction.py

@@ -0,0 +1,111 @@
+"""Settings-secret redaction for GET /api/settings (#832 follow-up).
+
+GET /api/settings used to ship the DECRYPTED config to the browser — every API
+key, token, and password in cleartext. redacted_config() masks configured
+secrets with a sentinel; set() refuses to let that round-tripped sentinel
+overwrite the real value. Together: secrets never reach the client, and saving
+an unchanged form never clobbers them.
+
+ConfigManager.__init__ touches the DB, so these build instances via __new__ and
+set config_data directly — the methods under test only read/write that dict.
+"""
+
+from __future__ import annotations
+
+from config.settings import ConfigManager
+
+S = ConfigManager.REDACTED_SENTINEL
+
+
+def _cm(config_data):
+    cm = ConfigManager.__new__(ConfigManager)
+    cm.config_data = config_data
+    cm._save_config = lambda: None
+    return cm
+
+
+# ── redacted_config: secrets out, everything else intact ────────────────────
+
+def test_configured_secrets_are_masked():
+    cm = _cm({'spotify': {'client_secret': 'REAL', 'redirect_uri': 'http://x'},
+              'plex': {'token': 'PLEXTOK', 'url': 'http://plex'}})
+    r = cm.redacted_config()
+    assert r['spotify']['client_secret'] == S
+    assert r['plex']['token'] == S
+    # Non-secret siblings pass through untouched.
+    assert r['spotify']['redirect_uri'] == 'http://x'
+    assert r['plex']['url'] == 'http://plex'
+
+
+def test_unset_secret_stays_empty_not_masked():
+    # An empty secret must NOT become the sentinel — the UI shows "not set".
+    cm = _cm({'jellyfin': {'api_key': ''}, 'navidrome': {'password': None}})
+    r = cm.redacted_config()
+    assert r['jellyfin']['api_key'] == ''
+    assert r['navidrome']['password'] is None
+
+
+def test_dict_valued_secret_is_masked():
+    # OAuth session blobs (tidal/qobuz) collapse to the sentinel.
+    cm = _cm({'tidal_download': {'session': {'access': 'A', 'refresh': 'R'}}})
+    assert cm.redacted_config()['tidal_download']['session'] == S
+
+
+def test_redaction_does_not_mutate_live_config():
+    cm = _cm({'spotify': {'client_secret': 'REAL'}})
+    cm.redacted_config()
+    assert cm.config_data['spotify']['client_secret'] == 'REAL'
+
+
+def _put(cfg, path, value):
+    parent = cfg
+    keys = path.split('.')
+    for k in keys[:-1]:
+        parent = parent.setdefault(k, {})
+    parent[keys[-1]] = value
+
+
+def _at(cfg, path):
+    cur = cfg
+    for k in path.split('.'):
+        cur = cur[k]
+    return cur
+
+
+def test_every_sensitive_path_is_masked():
+    # Put a value at every sensitive path (any depth) — none may survive in clear.
+    cfg = {}
+    for path in ConfigManager._SENSITIVE_PATHS:
+        _put(cfg, path, 'VALUE')
+    r = _cm(cfg).redacted_config()
+    leaked = [p for p in ConfigManager._SENSITIVE_PATHS if _at(r, p) != S]
+    assert leaked == [], f"secrets shipped in cleartext: {leaked}"
+
+
+# ── set() guard: the sentinel can never overwrite a real secret ─────────────
+
+def test_sentinel_roundtrip_keeps_existing_secret():
+    cm = _cm({'spotify': {'client_secret': 'REAL'}})
+    cm.set('spotify.client_secret', S)            # untouched masked field saved
+    assert cm.config_data['spotify']['client_secret'] == 'REAL'
+
+
+def test_real_value_overwrites():
+    cm = _cm({'spotify': {'client_secret': 'REAL'}})
+    cm.set('spotify.client_secret', 'NEW')
+    assert cm.config_data['spotify']['client_secret'] == 'NEW'
+
+
+def test_empty_value_clears_secret():
+    # Deliberately clearing a secret must still work (empty != sentinel).
+    cm = _cm({'spotify': {'client_secret': 'REAL'}})
+    cm.set('spotify.client_secret', '')
+    assert cm.config_data['spotify']['client_secret'] == ''
+
+
+def test_sentinel_on_non_secret_path_writes_normally():
+    # The guard is scoped to sensitive paths — a literal sentinel elsewhere is
+    # a normal write (absurd in practice, but proves the guard isn't global).
+    cm = _cm({'ui_appearance': {'theme': 'dark'}})
+    cm.set('ui_appearance.theme', S)
+    assert cm.config_data['ui_appearance']['theme'] == S

web_server.py

@@ -3045,11 +3045,12 @@ def handle_settings():
             return jsonify({"success": False, "error": str(e)}), 500
     else:  # GET request
         try:
-            data = dict(config_manager.config_data)
+            # Masks every configured secret (API keys, tokens, passwords) as a
-            # Never ship the OAuth token payload to the browser — the settings
+            # sentinel so decrypted credentials never reach the browser/devtools/
-            # UI has no field for it and it doesn't belong in devtools/HAR
+            # HAR captures (#832 follow-up). Deep copy — safe to mutate below.
-            # captures. NOTE: dict() above is a SHALLOW copy of live config
+            data = config_manager.redacted_config()
-            # state, so rebuild the section instead of popping in place.
+            # Also drop the Spotify OAuth token payload — not a settings field
+            # and not in the sensitive-paths list.
             if isinstance(data.get('spotify'), dict) and 'token_info' in data['spotify']:
                 data['spotify'] = {k: v for k, v in data['spotify'].items() if k != 'token_info'}
             # Include which download sources are configured so the UI can auto-disable unconfigured ones

webui/static/settings.js

@@ -1400,12 +1400,49 @@ async function loadSettingsData() {
             console.error('Error checking dev mode:', error);
         }
 
+        // Secret fields now arrive masked as REDACTED_SECRET_SENTINEL (#832
+        // follow-up) — wire them so editing replaces the mask instead of typing
+        // on top of it, and an untouched field re-masks on blur (round-trips the
+        // sentinel, which the server treats as "keep existing").
+        _wireRedactedSecrets();
+
     } catch (error) {
         console.error('Error loading settings:', error);
         showToast('Failed to load settings', 'error');
     }
 }
 
+// Mirrors ConfigManager.REDACTED_SENTINEL — secrets are never sent to the
+// browser; configured ones come back as this placeholder (rendered as dots in
+// the password inputs).
+const REDACTED_SECRET_SENTINEL = '__redacted_unchanged__';
+
+function _wireRedactedSecrets() {
+    document.querySelectorAll('input[type="password"]').forEach(el => {
+        if (el.dataset.redactWired === '1') return;
+        el.dataset.redactWired = '1';
+        // Clear the mask on focus so the user types a fresh value, not on top
+        // of the sentinel.
+        el.addEventListener('focus', () => {
+            if (el.value === REDACTED_SECRET_SENTINEL) {
+                el.value = '';
+                el.dataset.wasRedacted = '1';
+            }
+        });
+        // Untouched (focused but not edited, left empty) → restore the mask so
+        // save round-trips the sentinel and the real secret is kept.
+        el.addEventListener('blur', () => {
+            if (el.dataset.wasRedacted === '1' && el.value === '') {
+                el.value = REDACTED_SECRET_SENTINEL;
+                el.dataset.wasRedacted = '';
+            }
+        });
+        // Real typing means a genuine change/clear — drop the redacted mark so
+        // blur won't re-mask (an emptied field then saves as a real clear).
+        el.addEventListener('input', () => { el.dataset.wasRedacted = ''; });
+    });
+}
+
 async function changeLogLevel() {
     const selector = document.getElementById('log-level-select');
     const level = selector.value;