From 2641a1b87478f397a5ef10dc71f6e490f484db9e Mon Sep 17 00:00:00 2001 From: andrew Date: Fri, 10 Jul 2026 15:44:31 -0600 Subject: [PATCH] P2 cleanups: pin deps, prune job-run rows, unmask non-secret creds, enforce allowlist at login - R11: pin requirements.txt to the versions running in the image, so a rebuild can't pull a breaking upstream release. Documented how to upgrade. - R12: log rotation now also prunes job_runs rows older than 90 days (nulling the manual_imports FK reference first), so the table doesn't grow without bound. dedup/genre candidates are review state and left alone. - U5: credential form shows non-secret fields (URLs, usernames, prefs) as plain text so they can be verified while typing; only real secrets stay masked. Dashboard IP card reworded from gluetun-specific to generic "Outbound IP". - S9: enforce ALLOWED_EMAIL at the OIDC callback, before any user row or session is created, instead of only on later requests. Verified: pinned build resolves; prune deletes only old rows and keeps the manual_imports record; credentials page renders text+password inputs; a disallowed email gets 403 with no user row, an allowed one succeeds. Co-Authored-By: Claude Opus 4.8 --- app/db.py | 22 +++++++++++++++++ app/routers/auth.py | 9 ++++++- app/routers/credentials.py | 1 + app/services/credential_service.py | 14 +++++++++++ app/services/scheduler_service.py | 8 ++++-- app/templates/credentials/index.html | 4 ++- app/templates/dashboard.html | 4 +-- requirements.txt | 37 ++++++++++++++++------------ 8 files changed, 77 insertions(+), 22 deletions(-) diff --git a/app/db.py b/app/db.py index b630a5d..81e50f5 100644 --- a/app/db.py +++ b/app/db.py @@ -71,6 +71,28 @@ def init_db() -> None: conn.close() +def prune_old_job_runs(max_age_days: int = 90) -> int: + """Delete job_runs rows older than max_age_days so the table doesn't grow + without bound (roughly 30 scheduled runs/day). Rows are tiny and useful for + history, so the retention is longer than the 30-day log-file rotation. + + manual_imports.job_run_id references job_runs, so null those references + first to respect the foreign key (the import record itself is kept). dedup + and genre candidates are review state, not run history, so they are left + alone here. Returns the number of job_runs rows deleted.""" + cutoff = time.time() - max_age_days * 86400 + with engine.begin() as conn: + conn.execute( + text( + "UPDATE manual_imports SET job_run_id = NULL " + "WHERE job_run_id IN (SELECT id FROM job_runs WHERE started_at < :c)" + ), + {"c": cutoff}, + ) + result = conn.execute(text("DELETE FROM job_runs WHERE started_at < :c"), {"c": cutoff}) + return result.rowcount or 0 + + def mark_interrupted_runs() -> None: """A job_run still marked 'running' at startup was killed mid-flight: the subprocess does not survive a container restart. Mark those so the UI and diff --git a/app/routers/auth.py b/app/routers/auth.py index 856753c..c94e96e 100644 --- a/app/routers/auth.py +++ b/app/routers/auth.py @@ -1,6 +1,6 @@ import time -from fastapi import APIRouter, Request +from fastapi import APIRouter, HTTPException, Request from fastapi.responses import RedirectResponse from sqlalchemy import select @@ -8,6 +8,7 @@ from app.db import SessionLocal from app.models import User from app.security.oidc import oauth from app.services import diagnostics +from app.settings import settings router = APIRouter(prefix="/auth", tags=["auth"]) @@ -33,6 +34,12 @@ async def auth_callback(request: Request): name = userinfo.get("name") now = time.time() + # Enforce the allowlist here, before creating any user row or session, so a + # disallowed identity never gets persisted or a cookie (require_auth also + # checks it on every request, but that's after the fact). + if settings.allowed_email and email != settings.allowed_email: + raise HTTPException(status_code=403, detail="This account is not authorized to use alembic.") + with SessionLocal() as db: user = db.execute(select(User).where(User.oidc_sub == sub)).scalar_one_or_none() if user is None: diff --git a/app/routers/credentials.py b/app/routers/credentials.py index e9e6849..d3a602e 100644 --- a/app/routers/credentials.py +++ b/app/routers/credentials.py @@ -30,6 +30,7 @@ async def credentials_index(request: Request, user: dict = Depends(require_auth) "scope_fields": credential_service.SCOPE_FIELDS, "scope_descriptions": credential_service.SCOPE_DESCRIPTIONS, "core_scopes": credential_service.CORE_SCOPES, + "secret_keys": credential_service.SECRET_KEYS, "configured": configured, "enabled": enabled, "key_present": crypto.key_present(), diff --git a/app/services/credential_service.py b/app/services/credential_service.py index e1d37fa..623b29d 100644 --- a/app/services/credential_service.py +++ b/app/services/credential_service.py @@ -44,6 +44,20 @@ SCOPE_DESCRIPTIONS = { CORE_SCOPES = {"spotify", "soulseek", "navidrome"} OPTIONAL_SCOPES = {"bandcamp", "azuracast", "qobuz", "telegram"} +# The genuinely-secret fields, masked in the UI. Everything else is an +# identifier, URL, or preference the user should be able to see while typing +# so they can verify it (e.g. the Navidrome base URL or their Soulseek +# username). Values are write-only either way -- never sent back to the browser. +SECRET_KEYS = { + "client_secret", + "password", + "admin_pass", + "cookies_txt", + "api_key", + "token", + "bot_token", +} + def _upsert(db: Session, scope: str, key: str, value: str) -> None: row = db.execute( diff --git a/app/services/scheduler_service.py b/app/services/scheduler_service.py index c91ed08..51c3ba3 100644 --- a/app/services/scheduler_service.py +++ b/app/services/scheduler_service.py @@ -85,7 +85,10 @@ async def _genre_run(triggered_by: str = "schedule"): async def _log_rotation(triggered_by: str = "schedule"): - """Replaces `find /var/log/sldl -name '*.log' -mtime +30 -delete`.""" + """Replaces `find /var/log/sldl -name '*.log' -mtime +30 -delete`, and also + prunes old job_runs rows so the database doesn't grow without bound.""" + from app.db import prune_old_job_runs + cutoff = time.time() - 30 * 86400 removed = 0 if settings.logs_dir.exists(): @@ -93,7 +96,8 @@ async def _log_rotation(triggered_by: str = "schedule"): if path.is_file() and path.stat().st_mtime < cutoff: path.unlink(missing_ok=True) removed += 1 - return removed + pruned_rows = prune_old_job_runs() + return {"logs_removed": removed, "job_run_rows_pruned": pruned_rows} MAINTENANCE_JOBS: dict[str, tuple[dict, callable]] = { diff --git a/app/templates/credentials/index.html b/app/templates/credentials/index.html index 39ed214..db09ae2 100644 --- a/app/templates/credentials/index.html +++ b/app/templates/credentials/index.html @@ -54,8 +54,10 @@ {% if field in ("cookies_txt",) %} - {% else %} + {% elif field in secret_keys %} + {% else %} + {% endif %} {% endfor %} diff --git a/app/templates/dashboard.html b/app/templates/dashboard.html index bd18df9..154ed85 100644 --- a/app/templates/dashboard.html +++ b/app/templates/dashboard.html @@ -39,12 +39,12 @@
+ title="{% if public_ip.error and not public_ip.ip %}Couldn't check: {{ public_ip.error }}{% elif public_ip.error %}Showing last known good check -- most recent check failed: {{ public_ip.error }}{% else %}alembic's outbound IP. If you route Soulseek through a VPN, confirm this is the VPN's IP and not your home connection's.{% endif %}"> {% if public_ip.ip %}{{ public_ip.ip }}{% else %}unknown{% endif %}
- Public IP (via gluetun) + Outbound IP
diff --git a/requirements.txt b/requirements.txt index 86ea3df..b524512 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,19 +1,24 @@ +# Pinned to the versions currently running in the image. Rebuilds are then +# reproducible: a breaking upstream release can't be pulled in on the next +# `docker build`. To upgrade, bump a pin here deliberately, rebuild, and test. + # Control-plane app -fastapi -uvicorn[standard] -jinja2 -python-multipart -pydantic-settings -sqlalchemy -apscheduler -authlib -itsdangerous -cryptography -httpx +fastapi==0.139.0 +uvicorn[standard]==0.51.0 +jinja2==3.1.6 +python-multipart==0.0.32 +pydantic==2.13.4 +pydantic-settings==2.14.2 +sqlalchemy==2.0.51 +apscheduler==3.11.3 +authlib==1.7.2 +itsdangerous==2.2.0 +cryptography==49.0.0 +httpx==0.28.1 # Pipeline scripts (migrated from /opt/sldl) -beets -mutagen -numpy -pyacoustid -requests +beets==2.12.0 +mutagen==1.48.1 +numpy==2.4.6 +pyacoustid==1.3.1 +requests==2.34.2