From d2a167ff5fc02b03d38422ea1a0f821e0a4a2304 Mon Sep 17 00:00:00 2001
From: BoulderBadgeDad <batmanesignal@gmail.com>
Date: Wed, 10 Jun 2026 00:48:03 -0700
Subject: [PATCH] =?UTF-8?q?Profiles:=20review=20fixes=20=E2=80=94=20close?=
 =?UTF-8?q?=20two=20gating=20gaps=20+=20reject=20whitespace=20secrets?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Adversarial line-by-line review of the feature diff turned up:
- /api/database/update/stop was NOT @admin_only while its sibling start_update
  was — a non-admin could abort a library scan. Gated.
- /api/metadata-cache/evict was NOT gated while its clear siblings were. Gated.
- validate_credential_payload now treats whitespace-only values as missing, so
  a blank-but-spacey secret can't be saved to fail confusingly later.

Tests updated: both endpoints added to the admin-gating matrix; a whitespace-only
validation case added. 42 credential/gating tests pass.

Review also confirmed (no change needed): migration is idempotent + additive +
O(1); encryption round-trips with a non-dict guard; no SQL injection; stale
selections fall back to None safely; no secret ever returned to the browser;
the hybrid-drag index math is correct in both directions; the new resolver is
fully DORMANT (zero runtime callers) so existing client behaviour is untouched;
and @admin_only is a no-op for single-profile installs (default profile = admin).
---
 core/credentials/store.py         | 8 +++++++-
 tests/test_admin_gating.py        | 2 ++
 tests/test_service_credentials.py | 7 +++++++
 web_server.py                     | 2 ++
 4 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/core/credentials/store.py b/core/credentials/store.py
index 2d39fdf1..8cceb253 100644
--- a/core/credentials/store.py
+++ b/core/credentials/store.py
@@ -49,7 +49,13 @@ def validate_credential_payload(service: str, payload):
         return False, []
     if not isinstance(payload, dict):
         return False, list(required)
-    missing = [k for k in required if not payload.get(k)]
+
+    def _present(v):
+        # Whitespace-only strings count as missing — they'd otherwise save a
+        # blank secret that fails confusingly at the real service later.
+        return bool(v.strip()) if isinstance(v, str) else bool(v)
+
+    missing = [k for k in required if not _present(payload.get(k))]
     return (not missing), missing
 
 
diff --git a/tests/test_admin_gating.py b/tests/test_admin_gating.py
index 6ab66818..06dc1adf 100644
--- a/tests/test_admin_gating.py
+++ b/tests/test_admin_gating.py
@@ -34,12 +34,14 @@ GATED = [
     ('DELETE', '/api/library/album/123'),
     ('POST', '/api/library/tracks/delete-batch'),
     ('POST', '/api/database/update'),
+    ('POST', '/api/database/update/stop'),
     ('POST', '/api/database/backup'),
     ('DELETE', '/api/database/backups/x.db'),
     ('POST', '/api/database/backups/x.db/restore'),
     ('POST', '/api/database/maintenance/vacuum'),
     ('DELETE', '/api/metadata-cache/clear'),
     ('DELETE', '/api/metadata-cache/clear-musicbrainz'),
+    ('POST', '/api/metadata-cache/evict'),
 ]
 
 
diff --git a/tests/test_service_credentials.py b/tests/test_service_credentials.py
index aec802ec..64deae38 100644
--- a/tests/test_service_credentials.py
+++ b/tests/test_service_credentials.py
@@ -51,6 +51,13 @@ def test_validate_treats_empty_string_as_missing():
     assert not ok and missing == ['username']
 
 
+def test_validate_treats_whitespace_only_as_missing():
+    # A blank-but-spacey secret must be rejected, not saved to fail later.
+    ok, missing = validate_credential_payload('navidrome',
+                                              {'base_url': 'http://x', 'username': '   ', 'password': 'p'})
+    assert not ok and missing == ['username']
+
+
 # ── pure: active-set selection (stale-safe) ──────────────────────────────────
 
 def test_pick_active_credential_match_and_misses():
diff --git a/web_server.py b/web_server.py
index 805ed158..8376c9b5 100644
--- a/web_server.py
+++ b/web_server.py
@@ -16105,6 +16105,7 @@ def get_database_update_status():
         return jsonify(db_update_state)
 
 @app.route('/api/database/update/stop', methods=['POST'])
+@admin_only
 def stop_database_update():
     """Endpoint to stop the current database update."""
     global db_update_worker
@@ -16586,6 +16587,7 @@ def metadata_cache_clear():
         return jsonify({"success": False, "error": str(e)}), 500
 
 @app.route('/api/metadata-cache/evict', methods=['POST'])
+@admin_only
 def metadata_cache_evict():
     """Evict expired entries from the metadata cache."""
     try: