Profiles: review fixes — close two gating gaps + reject whitespace secrets

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).
This commit is contained in:
BoulderBadgeDad 2026-06-10 00:48:03 -07:00
parent bb0f68a8bf
commit d2a167ff5f
4 changed files with 18 additions and 1 deletions

View file

@ -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

View file

@ -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'),
]

View file

@ -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():

View file

@ -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: