fix(config): degrade gracefully on a TOKEN_ENCRYPTION_KEY mismatch instead of a 500 (B-core-2)
A rotated or missing TOKEN_ENCRYPTION_KEY makes every stored secret undecryptable.
sysconfig._raw_override called security.decrypt() with no guard, and it sits on
hot paths (/api/me via google_enabled, login, every YouTube scheduler job,
account purge) — so a key mismatch raised InvalidToken and 500'd the whole app,
the SPA wouldn't even boot, with no documented recovery.
Catch the decrypt failure in _raw_override -> warn once per key and fall back to
the env/config default. The YouTube client's refresh-token decrypt now raises a
clear YouTubeError ("reconnect") that every caller already handles, and
purge_user uses decrypt_optional so a best-effort revoke can't 500 the delete.
Document that the key must never change once secrets are stored (recovery: clear
the affected system_config rows and re-enter).
This commit is contained in:
+4
-3
@@ -11,7 +11,7 @@ from app import sysconfig
|
|||||||
from app.config import settings
|
from app.config import settings
|
||||||
from app.db import get_db
|
from app.db import get_db
|
||||||
from app.models import Invite, User
|
from app.models import Invite, User
|
||||||
from app.security import decrypt
|
from app.security import decrypt_optional
|
||||||
|
|
||||||
PASSWORD_MIN_LEN = 10
|
PASSWORD_MIN_LEN = 10
|
||||||
|
|
||||||
@@ -122,9 +122,10 @@ def purge_user(db: Session, user: User, background: BackgroundTasks) -> None:
|
|||||||
user_id = user.id
|
user_id = user.id
|
||||||
tok = user.token
|
tok = user.token
|
||||||
# Both are stored encrypted — decrypt for the revoke call (the access-token fallback was passing
|
# Both are stored encrypted — decrypt for the revoke call (the access-token fallback was passing
|
||||||
# ciphertext, so a token row with no refresh token silently failed to revoke).
|
# ciphertext, so a token row with no refresh token silently failed to revoke). decrypt_optional so a
|
||||||
|
# TOKEN_ENCRYPTION_KEY mismatch can't 500 the purge — a best-effort revoke degrades to "skip". (B-core-2)
|
||||||
google_token = (
|
google_token = (
|
||||||
(decrypt(tok.refresh_token_enc) or decrypt(tok.access_token)) if tok else None
|
(decrypt_optional(tok.refresh_token_enc) or decrypt_optional(tok.access_token)) if tok else None
|
||||||
)
|
)
|
||||||
# Raw delete so Postgres applies ON DELETE CASCADE / SET NULL on every dependent table.
|
# Raw delete so Postgres applies ON DELETE CASCADE / SET NULL on every dependent table.
|
||||||
db.execute(delete(User).where(User.id == user_id))
|
db.execute(delete(User).where(User.id == user_id))
|
||||||
|
|||||||
@@ -7,6 +7,7 @@ bounds, and whether it's a secret. The resolver returns the DB override if one i
|
|||||||
stored Fernet-encrypted at rest (requires TOKEN_ENCRYPTION_KEY; without it, secrets can't be
|
stored Fernet-encrypted at rest (requires TOKEN_ENCRYPTION_KEY; without it, secrets can't be
|
||||||
stored in the DB and stay env-only).
|
stored in the DB and stay env-only).
|
||||||
"""
|
"""
|
||||||
|
import logging
|
||||||
from dataclasses import dataclass
|
from dataclasses import dataclass
|
||||||
|
|
||||||
from sqlalchemy.orm import Session
|
from sqlalchemy.orm import Session
|
||||||
@@ -15,6 +16,12 @@ from app import security
|
|||||||
from app.config import settings
|
from app.config import settings
|
||||||
from app.models import SystemConfig
|
from app.models import SystemConfig
|
||||||
|
|
||||||
|
log = logging.getLogger("siftlode.config")
|
||||||
|
|
||||||
|
# Config keys whose stored ciphertext failed to decrypt — tracked so the warning is logged once per
|
||||||
|
# key, not once per request (bounded by the number of secret config keys).
|
||||||
|
_decrypt_warned: set[str] = set()
|
||||||
|
|
||||||
|
|
||||||
@dataclass(frozen=True)
|
@dataclass(frozen=True)
|
||||||
class ConfigSpec:
|
class ConfigSpec:
|
||||||
@@ -118,11 +125,30 @@ def _coerce(s: ConfigSpec, raw: str):
|
|||||||
|
|
||||||
|
|
||||||
def _raw_override(db: Session, key: str) -> str | None:
|
def _raw_override(db: Session, key: str) -> str | None:
|
||||||
"""Stored (decrypted) override string for `key`, or None if no override is set."""
|
"""Stored (decrypted) override string for `key`, or None if no override is set.
|
||||||
|
|
||||||
|
A decrypt failure — a rotated or removed ``TOKEN_ENCRYPTION_KEY`` vs the key the value was written
|
||||||
|
with — must NOT crash the caller. `_raw_override` sits on hot paths (`/api/me` via `google_enabled`,
|
||||||
|
login, every YouTube scheduler job, account purge), so a raw ``InvalidToken`` here would 500 the
|
||||||
|
whole app and the SPA wouldn't even boot. Fall back to the env/config default instead, and warn once
|
||||||
|
per key so the operator gets a real signal without a per-request traceback. Recovery is documented in
|
||||||
|
docs/self-hosting.md: the key must never change once secrets are stored. (B-core-2)
|
||||||
|
"""
|
||||||
row = db.get(SystemConfig, key)
|
row = db.get(SystemConfig, key)
|
||||||
if row is None or row.value is None:
|
if row is None or row.value is None:
|
||||||
return None
|
return None
|
||||||
return security.decrypt(row.value) if row.encrypted else row.value
|
if not row.encrypted:
|
||||||
|
return row.value
|
||||||
|
try:
|
||||||
|
return security.decrypt(row.value)
|
||||||
|
except Exception: # noqa: BLE001 — Fernet InvalidToken / malformed ciphertext (key mismatch)
|
||||||
|
if key not in _decrypt_warned:
|
||||||
|
_decrypt_warned.add(key)
|
||||||
|
log.warning(
|
||||||
|
"config %r could not be decrypted (TOKEN_ENCRYPTION_KEY changed or missing?) — falling "
|
||||||
|
"back to the env/config default; the key must not change once secrets are stored", key,
|
||||||
|
)
|
||||||
|
return None
|
||||||
|
|
||||||
|
|
||||||
def get(db: Session, key: str):
|
def get(db: Session, key: str):
|
||||||
|
|||||||
@@ -79,7 +79,12 @@ class YouTubeClient:
|
|||||||
cached = decrypt_optional(tok.access_token)
|
cached = decrypt_optional(tok.access_token)
|
||||||
if cached and tok.expiry and tok.expiry > now + timedelta(seconds=60):
|
if cached and tok.expiry and tok.expiry > now + timedelta(seconds=60):
|
||||||
return cached
|
return cached
|
||||||
refresh = decrypt(tok.refresh_token_enc)
|
try:
|
||||||
|
refresh = decrypt(tok.refresh_token_enc)
|
||||||
|
except Exception as exc: # noqa: BLE001 — Fernet InvalidToken (TOKEN_ENCRYPTION_KEY changed)
|
||||||
|
# A key mismatch must degrade like any other token problem (caller handles YouTubeError),
|
||||||
|
# not surface as a raw 500 on every scheduler job / connect check. (B-core-2)
|
||||||
|
raise YouTubeError("Stored OAuth token is unreadable — reconnect your account") from exc
|
||||||
if not refresh:
|
if not refresh:
|
||||||
raise YouTubeError("No refresh token; user must re-authenticate")
|
raise YouTubeError("No refresh token; user must re-authenticate")
|
||||||
resp = self._send(
|
resp = self._send(
|
||||||
|
|||||||
@@ -41,7 +41,10 @@ The installer asks for the **public URL** where the instance will be reached (ju
|
|||||||
`http://localhost:8080` to try it locally). It then:
|
`http://localhost:8080` to try it locally). It then:
|
||||||
|
|
||||||
- generates the secrets it needs (`SECRET_KEY`, `TOKEN_ENCRYPTION_KEY`, a database password) into a
|
- generates the secrets it needs (`SECRET_KEY`, `TOKEN_ENCRYPTION_KEY`, a database password) into a
|
||||||
local `.env` file — keep that file private,
|
local `.env` file — keep that file private. **Do not change `TOKEN_ENCRYPTION_KEY` once the app has
|
||||||
|
stored any secret** (Google credentials, API keys, SMTP password, Plex token): those values are
|
||||||
|
encrypted with it, and a changed key makes them unreadable. If it is ever lost or rotated, clear the
|
||||||
|
affected rows from the `system_config` table and re-enter the secrets in the admin Configuration page,
|
||||||
- pulls the image and starts the app + database,
|
- pulls the image and starts the app + database,
|
||||||
- prints the **setup wizard URL**, which looks like `…/setup?token=…`.
|
- prints the **setup wizard URL**, which looks like `…/setup?token=…`.
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user