diff --git a/README.md b/README.md index bc54929b..1869a810 100644 --- a/README.md +++ b/README.md @@ -96,7 +96,9 @@ docker compose -f docker-compose.selfhost.yml up -d That follows `latest`, which moves with every release. Set `IMAGE_TAG=0.80.0` in `.env` to pin a version and upgrade on your own schedule; released versions are listed in [`CHANGELOG.md`](CHANGELOG.md) -and in the app's Release Notes. +and in the app's Release Notes. The registry keeps only the newest few of those, so a pin decides +*when* you take the next hop rather than letting you stay put forever — see +[docs/self-hosting.md](docs/self-hosting.md#upgrading). **Back up before upgrading.** Migrations run automatically on startup and are not undone by pulling the old image back. A backup is the database dump **plus your `.env`** — `TOKEN_ENCRYPTION_KEY` is diff --git a/backend/app/downloads/errors.py b/backend/app/downloads/errors.py index 0c883bfa..335af9cd 100644 --- a/backend/app/downloads/errors.py +++ b/backend/app/downloads/errors.py @@ -8,9 +8,19 @@ explanation (and decides whether a retry makes sense). The raw text stays availa `_RETRIABLE_MARKERS` — kept here as the single source of truth for "what kind of failure is this". """ +# Failure texts the WORKER itself raises, as opposed to ones yt-dlp/ffmpeg produce. They live here, +# beside the rules that classify them, because a message and its category drifting apart is exactly +# how a category silently becomes unreachable — the rule below matches a fragment of each, and +# test_download_errors pins that they still classify as intended. +QUOTA_FULL_MESSAGE = "Your download storage is full — free up space, then try again." +NETWORK_REDACTED_MESSAGE = "The server could not reach that address." + # (marker, category) — first match wins, so order most-specific → most-general. Matching is # case-insensitive on a substring, against the raw yt-dlp/ffmpeg message. _RULES: tuple[tuple[str, str], ...] = ( + # Our own refusal: the worker found the user already at their storage cap (SEC-13). Placed + # first — it is our wording, so it can never be a false match on an upstream message. + ("download storage is full", "quota_exceeded"), # Login / age / members / bot-check — the user must be signed in (we deliberately are not). ("sign in to confirm your age", "login_required"), ("confirm your age", "login_required"), @@ -91,3 +101,20 @@ def classify(msg: str | None) -> str: if marker in low: return category return "unknown" + + +def user_error_text(source_kind: str, msg: str, code: str) -> str: + """The failure text to STORE on the job, given the raw message and its category. + + SEC-12: for a user-supplied URL, a transport-level failure text distinguishes "connection + refused" from "HTTP Error 401" from "timed out" per host:port — which is a port scanner with + extra steps, since the raw text is shown to the user behind the details disclosure. `urlguard` + stops the fetch aiming at our own network; this stops the remaining readout. + + Only the network class, and only for ``url`` jobs: a YouTube job has no user-chosen host, and + every other class (login_required, geo_blocked, drm, …) is real, useful diagnosis. A separate + function rather than an inline branch in the worker so it can be pinned by a test — the raw + text leaking back is a security regression that would otherwise pass a green suite.""" + if source_kind == "url" and code == "network": + return NETWORK_REDACTED_MESSAGE + return msg diff --git a/backend/app/downloads/pipeline.py b/backend/app/downloads/pipeline.py index cb5cd7b4..a83512b0 100644 --- a/backend/app/downloads/pipeline.py +++ b/backend/app/downloads/pipeline.py @@ -440,13 +440,19 @@ def _download( cookiefile = credentials.resolve_cookiefile( cdb, user_id, url, asset_sig, spec, staging ) - # SEC-13: what this user has left. `check_enqueue` only compares the CURRENT footprint - # against the cap — a download's size is unknown until it finishes, so a single job could - # previously blow through the cap by any amount. Handing the remainder to yt-dlp as - # `max_filesize` makes the format selector refuse an oversized file up front instead. - lim = quota.resolve(cdb, user_id) - if not lim.unlimited: - remaining_bytes = max(0, lim.max_bytes - quota.footprint(cdb, user_id)) + # SEC-13: what this user has left. `check_enqueue` only compared the footprint as it + # stood when the job was QUEUED — a download's size is unknown until it finishes, so a + # single job could previously blow through the cap by any amount. Handing the remainder + # to yt-dlp as `max_filesize` makes the format selector refuse an oversized file up + # front instead. + remaining_bytes = quota.remaining_bytes(cdb, user_id) + # …and zero is a REFUSAL, not "no limit". Earlier jobs of this user may have filled the cap + # while this one sat in the queue, and 0 is not an expressible yt-dlp `max_filesize`, so + # passing it down would leave the ceiling unset — the one user already at their cap would be + # the only one downloading unbounded. Fail the job here instead. Not retryable until they + # free space, which is what the `quota_exceeded` category tells them. + if remaining_bytes == 0: + raise RuntimeError(dlerrors.QUOTA_FULL_MESSAGE) info = _media_info( _extract_with_fallback( job_id, @@ -551,14 +557,8 @@ def _download( except Exception as exc: # noqa: BLE001 — surface any failure to the user, keep worker alive msg = str(exc)[:500] code = dlerrors.classify(msg) - # SEC-12: for a user-supplied URL, a transport-level failure text distinguishes "connection - # refused" from "HTTP Error 401" from "timed out" per host:port — which is a port scanner - # with extra steps, since the raw text is shown to the user behind the details disclosure. - # The urlguard above stops it aiming at our own network; this stops the remaining readout. - # Only the network class, and only for `url` jobs: a YouTube job has no user-chosen host, and - # every other class (login_required, geo_blocked, drm, …) is real, useful diagnosis. - if source_kind == "url" and code == "network": - msg = "The server could not reach that address." + # SEC-12: redact a transport-level readout for user-supplied URLs (see user_error_text). + msg = dlerrors.user_error_text(source_kind, msg, code) with SessionLocal() as db: asset = db.get(MediaAsset, asset_id) if asset: diff --git a/backend/app/downloads/quota.py b/backend/app/downloads/quota.py index f91c1c80..caf26e00 100644 --- a/backend/app/downloads/quota.py +++ b/backend/app/downloads/quota.py @@ -6,7 +6,9 @@ Limits come from a per-user DownloadQuota row if the admin set one, else the sys `unlimited` (e.g. the admin's own account) bypasses the byte cap. Enforcement split: max_jobs + max_bytes are checked at enqueue (they bound holdings); per-user -max_concurrent is enforced by the worker at claim time (it bounds simultaneous downloads). +max_concurrent is enforced by the worker at claim time (it bounds simultaneous downloads). max_bytes +is checked a SECOND time in the worker, via `remaining_bytes` — enqueue can only compare the +footprint as it stood when the job was created, and earlier jobs may have filled the cap since. """ from dataclasses import dataclass @@ -122,6 +124,21 @@ def check_enqueue(db: Session, user_id: int) -> None: raise QuotaExceeded("max_bytes", lim.max_bytes, used) +def remaining_bytes(db: Session, user_id: int) -> int | None: + """Storage this user may still spend on ONE download: ``None`` for an unlimited user, else the + cap minus the current footprint, floored at 0. + + Zero is a real answer, not "no limit" — it means the cap is already met and the caller must + REFUSE rather than proceed unbounded. `check_enqueue` cannot make that refusal on the caller's + behalf: it ran when the job was queued, while the user was still under the cap, and the jobs + that filled it have landed since. Keeping the arithmetic here (rather than inline in the worker) + is what keeps every max_bytes decision in this module.""" + lim = resolve(db, user_id) + if lim.unlimited: + return None + return max(0, lim.max_bytes - footprint(db, user_id)) + + def check_upload(db: Session, user_id: int, add_bytes: int) -> None: """Like check_enqueue, but for an upload whose size is KNOWN at ingest: also reject when adding this file would push the footprint over max_bytes. A download can't pre-check its size (unknown diff --git a/backend/app/downloads/urlguard.py b/backend/app/downloads/urlguard.py index 2d9d633f..63d11d30 100644 --- a/backend/app/downloads/urlguard.py +++ b/backend/app/downloads/urlguard.py @@ -10,11 +10,19 @@ What this closes: a URL whose host resolves to a loopback / private / link-local is refused, at enqueue AND again in the worker before extraction. Both, because DNS can answer differently between the two moments, and the worker is where the socket is actually opened. -What it does NOT close, stated plainly rather than implied: a host that passes the worker's check -and then answers yt-dlp's own resolution with an internal address (DNS rebinding). Closing that -needs the connection itself to be gated — a custom socket factory inside yt-dlp — which is a much -larger change than this one. The two checks here stop the straightforward case (typing -``http://127.0.0.1:6379`` or an internal hostname into the box), which is the reachable one. +What it does NOT close, stated plainly rather than implied, because both need the same fix — gating +the connection itself with a custom socket factory inside yt-dlp, a much larger change than this: + +* DNS rebinding: a host that passes the worker's check and then answers yt-dlp's own resolution + with an internal address. +* HTTP redirects: a public host that answers 3xx with an internal ``Location``. yt-dlp follows it, + and only the URL the user TYPED was ever judged here. An attacker-controlled host or any public + open redirect reaches it, so this is the cheaper of the two — but it still buys only what a + media-shaped internal response yields, since a non-media body fails the mp4 remux and stores + nothing. Tracked as SEC-17. + +The two checks here stop the straightforward case (typing ``http://127.0.0.1:6379`` or an internal +hostname into the box), which is the reachable one. Not used for YouTube jobs: those resolve to a fixed public host from an 11-char id, so there is no user-controlled destination to guard. @@ -28,6 +36,7 @@ from urllib.parse import urlparse # vocabulary (frontend `localizeDetail`) — keep them stable. REASON_SCHEME = "scheme" # not http(s) REASON_NO_HOST = "no_host" # URL carries no hostname +REASON_BAD_PORT = "bad_port" # ":99999" / ":abc" — urlparse only raises on ACCESS, so guard it REASON_UNRESOLVABLE = "unresolvable" # DNS said nothing (honest: not the same as "blocked") REASON_PRIVATE = "private" # resolves into a network the server must not fetch from @@ -79,7 +88,15 @@ def check_url(raw: str) -> None: host = u.hostname if not host: raise BlockedUrl(REASON_NO_HOST) - for addr in _resolve(host, u.port): + # `.port` is a lazily-parsed property: it raises ValueError for ":99999" or ":abc" on ACCESS, + # not at urlparse() time, and `.hostname` above happily returns first. Unguarded, one typo + # escapes the caller's `except BlockedUrl` as a 500 instead of the structured 422 this whole + # module exists to return. + try: + port = u.port + except ValueError as exc: + raise BlockedUrl(REASON_BAD_PORT) from exc + for addr in _resolve(host, port): try: ip = ipaddress.ip_address(addr) except ValueError: # getaddrinfo returned something unparseable; treat as unusable diff --git a/backend/app/routes/playlists.py b/backend/app/routes/playlists.py index b9a4a008..dfae5af9 100644 --- a/backend/app/routes/playlists.py +++ b/backend/app/routes/playlists.py @@ -29,9 +29,22 @@ router = APIRouter(prefix="/api/playlists", tags=["playlists"]) # caller owns and then every item in each, against the instance-wide YouTube budget. Its own # limiter rather than a shared one — spending the manual subscription-import allowance must not # also close the playlist button, and vice versa. +# +# It guards EVERY route that spends under QuotaAction.PLAYLISTS_PULL, not just the bulk one: a +# scripted loop over the single-playlist re-pull drains the same shared budget, and limiting one of +# two routes with the same quota action is the asymmetric fix this codebase keeps re-learning. +# Deliberately NOT on the push side (`push-plan` / `push`): those run on every push attempt, +# including the no-op "already up to date" path, so a 3-per-10-minute allowance would refuse +# legitimate work. Their exposure is tracked as SEC-18 instead. _playlist_pull_limiter = RateLimiter(max_events=3, window_seconds=600) +def _check_pull_cooldown(user: User) -> None: + """Shared gate so the two PLAYLISTS_PULL routes cannot drift apart again.""" + if not _playlist_pull_limiter.allow(str(user.id)): + raise HTTPException(status_code=429, detail={"code": "sync_cooldown"}) + + def _own_playlist(db: Session, user: User, playlist_id: int) -> Playlist: pl = db.get(Playlist, playlist_id) if pl is None or pl.user_id != user.id: @@ -291,8 +304,7 @@ def sync_youtube( status_code=403, detail="Connect YouTube (read access) to sync your playlists.", ) - if not _playlist_pull_limiter.allow(str(user.id)): - raise HTTPException(status_code=429, detail={"code": "sync_cooldown"}) + _check_pull_cooldown(user) with quota.attribute(user.id, quota.QuotaAction.PLAYLISTS_PULL): return sync_user_playlists(db, user) @@ -313,6 +325,7 @@ def revert_youtube( raise HTTPException( status_code=403, detail="Connect YouTube (read access) to sync your playlists." ) + _check_pull_cooldown(user) try: with quota.attribute(user.id, quota.QuotaAction.PLAYLISTS_PULL): return repull_playlist(db, user, pl) diff --git a/backend/tests/test_abuse_limits.py b/backend/tests/test_abuse_limits.py index 85193b5d..3ebd011b 100644 --- a/backend/tests/test_abuse_limits.py +++ b/backend/tests/test_abuse_limits.py @@ -85,3 +85,28 @@ def test_the_cooldown_is_per_user_not_global(db, as_user, make_user, monkeypatch ca.post("/api/sync/subscriptions") assert ca.post("/api/sync/subscriptions").status_code == 429 assert as_user(b).post("/api/sync/subscriptions").status_code == 200 + + +def test_the_single_playlist_repull_shares_the_pull_cooldown(db, as_user, make_user, monkeypatch): + # SEC-14, the sweep half: `revert-youtube` spends under the SAME QuotaAction.PLAYLISTS_PULL key + # as the bulk `sync-youtube`, so limiting only one of them left the identical drain open one + # route over. Both must draw on the one allowance. + from app.models import Playlist + from app.routes import playlists as plroutes + + user = make_user() + pl = Playlist(user_id=user.id, name="Linked", kind="youtube", yt_playlist_id="PL123") + db.add(pl) + db.commit() + monkeypatch.setattr(plroutes, "has_read_scope", lambda u: True) + monkeypatch.setattr(plroutes, "repull_playlist", lambda d, u, p: {"ok": True}) + monkeypatch.setattr(plroutes, "sync_user_playlists", lambda d, u: {"ok": True}) + + c = as_user(user) + codes = [c.post(f"/api/playlists/{pl.id}/revert-youtube").status_code for _ in range(4)] + assert codes[:3] == [200, 200, 200] + assert codes[3] == 429 + # One allowance, not one each: the bulk route is now on cooldown too. + r = c.post("/api/playlists/sync-youtube") + assert r.status_code == 429 + assert r.json()["detail"] == {"code": "sync_cooldown"} diff --git a/backend/tests/test_download_errors.py b/backend/tests/test_download_errors.py index 3614ec55..680d2d13 100644 --- a/backend/tests/test_download_errors.py +++ b/backend/tests/test_download_errors.py @@ -1,5 +1,6 @@ """Category classification of raw yt-dlp / ffmpeg download-failure strings (S2).""" +from app.downloads import errors as dlerrors from app.downloads.errors import classify @@ -54,3 +55,22 @@ def test_login_beats_generic_unavailable_when_both_present(): "This video is unavailable" ) assert classify(msg) == "login_required" + + +def test_our_own_worker_refusals_classify_as_themselves(): + # The worker raises these itself (not yt-dlp), and the rule that catches each one matches only a + # fragment of the text. Pinning message and rule together is what stops an edit to the wording + # from silently orphaning the category — the UI would then fall back to "Download failed". + assert classify(dlerrors.QUOTA_FULL_MESSAGE) == "quota_exceeded" + + +def test_a_url_jobs_network_failure_is_redacted_but_youtube_keeps_its_text(): + # SEC-12: the raw transport text for a user-chosen host:port is a port-scanner readout, so it is + # replaced for `url` jobs. Everything else must survive verbatim, or a real diagnosis is lost. + raw = "ERROR: Unable to download webpage: " + assert dlerrors.user_error_text("url", raw, "network") == dlerrors.NETWORK_REDACTED_MESSAGE + # Same failure, but the destination was never user-chosen — nothing to leak, keep the detail. + assert dlerrors.user_error_text("youtube", raw, "network") == raw + # A user-chosen host, but a category that says something real about the CONTENT, not the network. + login = "ERROR: [facebook] 123: You must be logged in to see this post" + assert dlerrors.user_error_text("url", login, "login_required") == login diff --git a/backend/tests/test_download_limits.py b/backend/tests/test_download_limits.py index 29322697..918b72fc 100644 --- a/backend/tests/test_download_limits.py +++ b/backend/tests/test_download_limits.py @@ -10,6 +10,7 @@ Uses the `db` fixture (real Postgres) — skipped when no database is reachable. its OWN session, so every test COMMITS its setup before calling it. """ +from app.downloads import quota from app.models import DownloadJob, DownloadQuota, MediaAsset, User from app.worker import _claim_job @@ -169,3 +170,61 @@ def test_list_downloads_is_capped(db, monkeypatch): monkeypatch.setattr(dl, "_MAX_LIST", 2) rows = dl.list_downloads(user=u, db=db) assert len(rows) == 2 # bounded by the cap, not the 3 rows on disk + + +# --- E12 · S3 · the worker-side max_bytes remainder ------------------------------------------ + + +def _sized_asset(db, size_bytes, ref): + a = MediaAsset( + source_kind="url", + source_ref=ref, + format_sig="s", + status="ready", + ref_count=1, + size_bytes=size_bytes, + ) + db.add(a) + db.flush() + return a + + +def test_remaining_bytes_is_none_for_an_unlimited_user(db): + u = User(email="unlimited@example.com") + db.add(u) + db.flush() + _quota(db, u.id, max_concurrent=2, unlimited=True) + db.flush() + # None, not a number: "no cap" and "no budget left" must never be the same value downstream — + # build_ydl_opts leaves max_filesize unset for both, so only the caller can tell them apart. + assert quota.remaining_bytes(db, u.id) is None + + +def test_remaining_bytes_is_the_unspent_part_of_the_cap(db): + u = User(email="partial@example.com") + db.add(u) + db.flush() + db.add( + DownloadQuota(user_id=u.id, max_bytes=1000, max_concurrent=2, max_jobs=100, unlimited=False) + ) + a = _sized_asset(db, 400, "partial-1") + _job(db, u.id, queue_pos=0, status="done", asset_id=a.id) + db.flush() + assert quota.remaining_bytes(db, u.id) == 600 + + +def test_remaining_bytes_is_zero_once_the_cap_is_met(db): + # THE case this exists for: `check_enqueue` let these jobs in while the user was under the cap, + # and an earlier one has filled it since. Zero is what makes the worker refuse; if this ever + # returned the raw negative or None instead, the user already over quota would be the only one + # downloading with no size ceiling at all. + u = User(email="atcap@example.com") + db.add(u) + db.flush() + db.add( + DownloadQuota(user_id=u.id, max_bytes=1000, max_concurrent=2, max_jobs=100, unlimited=False) + ) + a = _sized_asset(db, 1500, "atcap-1") # overshot: one job finished larger than the remainder + _job(db, u.id, queue_pos=0, status="done", asset_id=a.id) + db.flush() + assert quota.remaining_bytes(db, u.id) == 0 diff --git a/backend/tests/test_urlguard.py b/backend/tests/test_urlguard.py index 9cbd5abf..85e63909 100644 --- a/backend/tests/test_urlguard.py +++ b/backend/tests/test_urlguard.py @@ -85,3 +85,12 @@ def test_non_http_schemes_are_refused_before_any_lookup(monkeypatch, url): def test_a_url_without_a_host_is_refused(): assert _blocked_reason("http:///just/a/path") == urlguard.REASON_NO_HOST + + +def test_a_malformed_port_is_a_refusal_not_a_crash(monkeypatch): + # `urlparse(...).port` raises ValueError on ACCESS, not at parse time, and `.hostname` returns + # first — so an unguarded read turns a user typo into a 500 that escapes the caller's + # `except BlockedUrl` and loses the localized detail this module exists to produce. + _stub_dns(monkeypatch, ["93.184.216.34"]) + assert _blocked_reason("http://example.com:99999/v.mp4") == urlguard.REASON_BAD_PORT + assert _blocked_reason("http://example.com:abc/v.mp4") == urlguard.REASON_BAD_PORT diff --git a/docs/self-hosting.md b/docs/self-hosting.md index 0e12567d..085fd17b 100644 --- a/docs/self-hosting.md +++ b/docs/self-hosting.md @@ -172,22 +172,41 @@ Dump and restore the database with these two commands. They also exist as script (`scripts/backup.sh` + `scripts/restore.sh`, or `backup.ps1` / `restore.ps1` on Windows) if you have the source checkout: -```bash -# Back up (writes backups/siftlode-.dump) -mkdir -p backups && docker compose -f docker-compose.selfhost.yml exec -T db \ - pg_dump -U siftlode -Fc siftlode > "backups/siftlode-$(date +%Y%m%d-%H%M%S).dump" +If you customised `POSTGRES_USER` / `POSTGRES_DB` in `.env`, substitute them for `siftlode` below — +the scripts read them from the environment, these one-liners cannot. -# Restore into the running instance +```bash +# Back up. Write to a .part name and rename only on success: redirecting straight into the final +# name creates the file BEFORE pg_dump runs, so a failed dump leaves a plausible-looking 0-byte +# file in backups/ that you will not discover until the day you need it. +mkdir -p backups && out="backups/siftlode-$(date +%Y%m%d-%H%M%S).dump" && \ + docker compose -f docker-compose.selfhost.yml exec -T db \ + pg_dump -U siftlode -Fc siftlode > "$out.part" \ + && [ -s "$out.part" ] && mv "$out.part" "$out" && echo "wrote $out" \ + || { rm -f "$out.part"; echo "BACKUP FAILED"; } + +# Restore. Stop the app first: pg_restore --clean DROPs each object in its own transaction, and a +# running api holds locks that make those DROPs queue — abort the wait and you are left with a +# half-dropped database. Restoring with the app down also means nothing writes into a schema that +# is only partly back. +docker compose -f docker-compose.selfhost.yml stop api worker docker compose -f docker-compose.selfhost.yml exec -T db \ pg_restore -U siftlode -d siftlode --clean --if-exists < backups/siftlode-.dump +docker compose -f docker-compose.selfhost.yml up -d ``` On Windows, do **not** pipe the dump through PowerShell — it corrupts binary data. Write it inside the -container and copy it out instead: +container and copy it out instead. Check the exit code between the two steps and use a timestamped +name: the dump inside the container keeps its name, so a `cp` that runs after a *failed* dump would +copy out the previous run's file and quietly overwrite your only backup with a stale one. ```powershell +New-Item -ItemType Directory -Force -Path backups | Out-Null +$out = "backups\siftlode-$(Get-Date -Format yyyyMMdd-HHmmss).dump" docker compose -f docker-compose.selfhost.yml exec -T db pg_dump -U siftlode -Fc siftlode -f /tmp/s.dump -docker compose -f docker-compose.selfhost.yml cp db:/tmp/s.dump backups\siftlode.dump +if ($LASTEXITCODE -ne 0) { throw "BACKUP FAILED - pg_dump returned $LASTEXITCODE" } +docker compose -f docker-compose.selfhost.yml cp db:/tmp/s.dump $out +docker compose -f docker-compose.selfhost.yml exec -T db rm -f /tmp/s.dump ``` ## Upgrading @@ -211,23 +230,53 @@ Then `pull` + `up -d` stays on that version until you change the line. Released changed in them are listed in the app itself, under **Release Notes** in the account menu, and in [`CHANGELOG.md`](../CHANGELOG.md). +**The registry keeps only the newest few releases, not every version the changelog lists.** A pin is +"decide when you take the next hop", not "stay here forever": once you are several releases behind, +your pinned tag can no longer be pulled — which matters the day you rebuild the host, prune your +Docker images, or move to a new machine. A container that is already running is unaffected (Docker +keeps the image locally), and you can always go back to `latest` or build the image yourself from the +repository. If you want to sit on one version long term, keep your own copy: +`docker save git.b1fr0st.eu/peter/siftlode:0.80.0 -o siftlode-0.80.0.tar`. + ### Rollback If a release misbehaves, set `IMAGE_TAG` back to the previous version and `up -d`. That is enough **as long as the release did not change the database schema**. If it did, the older image will refuse -to start against the newer schema, and you need to step the schema back first: +to start against the newer schema. + +**Restoring your pre-upgrade dump is the recommended way out of that** — it puts the database back +exactly where the old image expects it, in one step, with no revision arithmetic: ```bash # 1. stop the app (leave the database up) docker compose -f docker-compose.selfhost.yml stop api worker -# 2. step the schema back one revision -docker compose -f docker-compose.selfhost.yml run --rm api alembic downgrade -1 +# 2. restore the dump you took before upgrading (see Backups above) +docker compose -f docker-compose.selfhost.yml exec -T db \ + pg_restore -U siftlode -d siftlode --clean --if-exists < backups/siftlode-.dump # 3. pin the previous version in .env, then start again docker compose -f docker-compose.selfhost.yml up -d ``` -If you are not sure whether the schema moved, restoring your pre-upgrade dump is the safer path — it -puts the database back exactly where the old image expects it. +If you have no usable dump, you can step the schema back instead — but **not with `alembic downgrade +-1`**. One release routinely ships several migrations (0.80.0 shipped four), so `-1` undoes only the +newest one and leaves the schema at a revision the older image *also* cannot resolve: you lose that +migration's data and still get a crash loop. Downgrade to the revision the target version expects, +which that version can tell you itself: + +```bash +# 1. stop the app (leave the database up) +docker compose -f docker-compose.selfhost.yml stop api worker +# 2. ask the version you are rolling back TO which schema revision it expects +IMAGE_TAG=0.79.0 docker compose -f docker-compose.selfhost.yml run --rm api alembic heads +# 3. step the schema back to exactly that revision — run this with the CURRENT (newer) image, +# because it is the one that ships the migrations being undone +docker compose -f docker-compose.selfhost.yml run --rm api alembic downgrade +# 4. pin the previous version in .env, then start again +docker compose -f docker-compose.selfhost.yml up -d +``` + +A downgrade drops the columns the newer version added, so anything stored only in those columns is +gone. That is the price of not having the dump. ### A migration failed on start diff --git a/frontend/src/components/AccountPanel.tsx b/frontend/src/components/AccountPanel.tsx index 4060681d..a7e25382 100644 --- a/frontend/src/components/AccountPanel.tsx +++ b/frontend/src/components/AccountPanel.tsx @@ -7,6 +7,8 @@ import { LoadingState, StateMessage } from "./QueryState"; import { api } from "../lib/api"; import { qk } from "../lib/queryKeys"; import { notify } from "../lib/notifications"; +import { copyText } from "../lib/clipboard"; +import { invalidateUserAccess, isEmailOff } from "../lib/adminUsers"; import type { AdminUserAccess, AdminUserRow } from "../lib/api/admin"; // The admin's answer to "why can this person not get in?", and the place every recovery action for @@ -57,35 +59,47 @@ export default function AccountPanel({ queryFn: () => api.adminUserAccess(user.id), }); + // Every action here goes through a `quiet: true` endpoint, which suppresses the GLOBAL error + // dialog on the promise that the caller reports the failure inline. Without an `onError` that + // promise is broken and a refusal renders as nothing at all — on the broken-SMTP instance this + // panel exists for, the admin would read silence as "the mail went out". const issue = useMutation({ mutationFn: () => api.createSetPasswordLink(user.id), onSuccess: (r) => setLink({ url: r.url, expires_at: r.expires_at }), + onError: () => notify({ level: "error", message: t("users.account.linkFailed") }), }); const resend = useMutation({ mutationFn: () => api.resendUserVerification(user.id), onSuccess: () => notify({ level: "success", message: t("users.account.resendSent") }), + onError: (err) => + notify({ + level: "error", + message: isEmailOff(err) + ? t("users.pendingAccess.resendEmailOff") + : t("users.pendingAccess.resendFailed"), + }), }); const activate = useMutation({ mutationFn: () => api.activateUser(user.id), - onSuccess: () => { - qc.invalidateQueries({ queryKey: qk.adminUsers() }); - qc.invalidateQueries({ queryKey: qk.adminUserAccess(user.id) }); - }, + onSuccess: () => invalidateUserAccess(qc, user.id), + onError: () => notify({ level: "error", message: t("users.pendingAccess.activateFailed") }), }); const copy = async () => { if (!link) return; - try { - await navigator.clipboard.writeText(link.url); + // Via the shared helper, whose synchronous `execCommand` path is what makes copying work on a + // plain-http LAN origin — where `navigator.clipboard` is undefined, and which is exactly the + // minimal self-host this no-email link was built for. + if (await copyText(link.url)) { setCopied(true); window.setTimeout(() => setCopied(false), 2000); - } catch { - // Clipboard access can be refused (permissions, an insecure origin). The link is on screen and - // selectable, so this is a convenience that failed — not the action failing. - notify({ level: "warning", message: t("users.account.copyFailed") }); + return; } + // The link is on screen and selectable, so a failed copy is a convenience that failed — not the + // action failing. Say so rather than leaving the button looking inert. + notify({ level: "warning", message: t("users.account.copyFailed") }); }; const a: AdminUserAccess | undefined = q.data; diff --git a/frontend/src/components/AdminUsers.tsx b/frontend/src/components/AdminUsers.tsx index 2de09101..6b9fc0bc 100644 --- a/frontend/src/components/AdminUsers.tsx +++ b/frontend/src/components/AdminUsers.tsx @@ -24,6 +24,7 @@ import { type PendingAccessRow, } from "../lib/api"; import { notify } from "../lib/notifications"; +import { invalidateUserAccess, isEmailOff } from "../lib/adminUsers"; import { ADMIN_USERS_TAB_KEY } from "../lib/adminUsersTab"; import Tooltip from "./Tooltip"; import InlineNotice from "./InlineNotice"; @@ -215,7 +216,7 @@ function UsersRoles({ me }: { me: Me }) {