Deep-scrub secrets from public settings

/api/auth/settings is auth-exempt (the frontend + the pre-login page read it for
keybinds/TTS prefs), so non-admin and unauthenticated callers get a scrubbed
copy. The previous scrub only blanked TOP-LEVEL string values whose key matched a
short suffix list — so a secret nested under a non-secret parent key, or stored
under a key outside the list, would leak. A real exposure when the app is
reachable over a Cloudflare tunnel / reverse proxy.

- src/settings_scrub.py: NEW stdlib-only module with the scrub helpers (deep/
  recursive; broadened secret-key patterns). Kept separate from auth_routes so it
  imports + unit-tests WITHOUT pulling the FastAPI / auth / database chain
  (addresses review: the test no longer fails at collection on the DB import).
- routes/auth_routes.py: import scrub_settings from the module.
- tests/test_settings_scrub.py: import the tiny module directly.

Ran: pytest tests/test_settings_scrub.py (8 passed); verified the test pulls no
db/auth modules into sys.modules; py_compile routes/auth_routes.py.

Co-authored-by: Kanaru92 <107661007+Kanaru92@users.noreply.github.com>
This commit is contained in:
kanaru-dev
2026-06-01 07:11:50 -07:00
committed by GitHub
parent 35f11f2edc
commit a51a1fc4fc
3 changed files with 113 additions and 24 deletions

View File

@@ -8,6 +8,7 @@ import os
from core.auth import AuthManager
from src.rate_limiter import RateLimiter
from src.settings_scrub import scrub_settings
from src.settings import (
load_settings as _load_settings,
save_settings as _save_settings,
@@ -371,29 +372,6 @@ def setup_auth_routes(auth_manager: AuthManager) -> APIRouter:
# ---- App settings (admin-managed) ----
_SECRET_KEY_PATTERNS = ("_api_key", "_password", "_secret", "_token", "_key")
def _is_secret_key(name: str) -> bool:
n = (name or "").lower()
if n in ("google_pse_cx",): # public identifier, not a secret
return False
return any(n.endswith(p) or n == p.lstrip("_") for p in _SECRET_KEY_PATTERNS)
def _scrub_settings(settings: dict) -> dict:
"""Return a copy of settings with secret-shaped values masked.
Frontend reads /settings without auth for things like keybinds + TTS
prefs. Secrets (search-provider keys, IMAP/SMTP passwords) must NOT
be exposed to non-admin callers.
"""
scrubbed = {}
for k, v in (settings or {}).items():
if _is_secret_key(k) and isinstance(v, str) and v:
scrubbed[k] = "" # presence preserved, value blanked
else:
scrubbed[k] = v
return scrubbed
@router.get("/settings")
async def get_settings(request: Request):
"""Returns app settings. Admins get the full set; non-admins get
@@ -403,7 +381,7 @@ def setup_auth_routes(auth_manager: AuthManager) -> APIRouter:
settings = _load_settings()
if user and auth_manager.is_admin(user):
return settings
return _scrub_settings(settings)
return scrub_settings(settings)
@router.post("/settings")
async def set_settings(request: Request):

50
src/settings_scrub.py Normal file
View File

@@ -0,0 +1,50 @@
"""Secret-scrubbing for settings exposed to non-admin / unauthenticated callers.
Deliberately dependency-light (stdlib only) and separate from
``routes/auth_routes.py`` so it can be imported and unit-tested without dragging
in the FastAPI app / auth / database import chain.
``/api/auth/settings`` is auth-exempt — the frontend (and the pre-login page)
read it for keybinds + TTS prefs, so non-admin and unauthenticated callers get a
*scrubbed* copy. Secrets (provider API keys, IMAP/SMTP passwords, OAuth tokens)
must NOT leak to them — load-bearing when the app is reachable over a Cloudflare
tunnel / reverse proxy. Scrubbing is deep (recurses nested dicts/lists) and keyed
on secret-shaped names.
"""
_SECRET_KEY_PATTERNS = (
"_api_key", "_apikey", "_password", "_passwd", "_pass", "_pwd",
"_secret", "_client_secret", "_token", "_access_token", "_refresh_token",
"_credential", "_credentials", "_key",
)
_SECRET_KEY_ALLOW = ("google_pse_cx",) # public identifiers, not secrets
def is_secret_key(name: str) -> bool:
n = (name or "").lower()
if n in _SECRET_KEY_ALLOW:
return False
return any(n.endswith(p) or n == p.lstrip("_") for p in _SECRET_KEY_PATTERNS)
def _scrub_value(key, value):
"""Mask secret-shaped leaves, recursing into nested dicts/lists so a secret
stored under a non-secret parent key (e.g.
``{"email_account": {"smtp_password": "..."}}``) is still blanked. Only
non-empty *string* values are blanked; presence is preserved."""
if isinstance(value, dict):
return {
k: ("" if (is_secret_key(k) and isinstance(v, str) and v)
else _scrub_value(k, v))
for k, v in value.items()
}
if isinstance(value, list):
return [_scrub_value(key, item) for item in value]
if is_secret_key(key) and isinstance(value, str) and value:
return ""
return value
def scrub_settings(settings: dict) -> dict:
"""Return a copy of ``settings`` with secret-shaped values masked (deep)."""
return {k: _scrub_value(k, v) for k, v in (settings or {}).items()}

View File

@@ -0,0 +1,61 @@
"""Security tests for the /api/auth/settings secret scrubbing.
The /settings endpoint is auth-exempt (the frontend + the pre-login page read it
for keybinds / TTS prefs), so non-admin and unauthenticated callers receive a
*scrubbed* copy. Secrets must never leak to them — load-bearing when the app is
reachable over a Cloudflare tunnel / reverse proxy. These pin the scrub: deep
(nested), broad secret-key coverage, and no collateral damage to real prefs.
Imports the stdlib-only `src.settings_scrub` directly, so the test does not pull
in the FastAPI / auth / database import chain.
"""
from src.settings_scrub import is_secret_key, scrub_settings
def test_top_level_secrets_blanked():
out = scrub_settings({"search_api_key": "S", "openai_api_key": "K", "smtp_password": "P"})
assert out["search_api_key"] == "" and out["openai_api_key"] == "" and out["smtp_password"] == ""
def test_broadened_patterns_blanked():
s = {"smtp_pass": "a", "db_pwd": "b", "oauth_client_secret": "c",
"gh_access_token": "d", "refresh_token": "e", "x_credential": "f", "z_apikey": "g"}
out = scrub_settings(s)
assert all(out[k] == "" for k in s), out
def test_nested_secret_blanked():
out = scrub_settings({"email_account": {"host": "imap", "smtp_password": "NESTED"}})
assert out["email_account"]["host"] == "imap" # non-secret preserved
assert out["email_account"]["smtp_password"] == "" # nested secret blanked
def test_secret_in_list_of_dicts_blanked():
out = scrub_settings({"providers": [{"name": "a", "api_key": "P1"},
{"name": "b", "access_token": "T2"}]})
assert out["providers"][0]["name"] == "a"
assert out["providers"][0]["api_key"] == ""
assert out["providers"][1]["access_token"] == ""
def test_non_secret_keys_preserved():
s = {"keybinds": {"send": "Enter"}, "theme": "dark", "image_model": "x",
"default_endpoint_id": "ep1", "search_result_count": 5, "tts_enabled": True}
assert scrub_settings(s) == s # untouched
def test_google_pse_cx_is_public():
assert is_secret_key("google_pse_cx") is False
assert scrub_settings({"google_pse_cx": "cx123"})["google_pse_cx"] == "cx123"
def test_empty_and_nonstring_secret_values_untouched():
out = scrub_settings({"api_key": "", "feature_key": 7, "x_token": None})
assert out["api_key"] == "" # already empty
assert out["feature_key"] == 7 # int not blanked (string-only)
assert out["x_token"] is None # None not blanked
def test_exact_name_matches():
out = scrub_settings({"password": "p", "token": "t", "secret": "s", "apikey": "a", "key": "k"})
assert all(v == "" for v in out.values()), out