refactor: retire the images tarball backup

Images and attachments are backed up off-site, one object per file, so the
tar+zstd of /images goes: its service and restore, both tasks, the trigger and
restore routes, the Keep last setting, the screen's Images section, and the
tests. Migration 0122 drops backup_run.tar_path and
import_settings.backup_images_keep_last_n and deletes any images runs.

The local database dump stays for an install with no off-site destination,
and the Backups card now says plainly that images and attachments are backed
up only off-site.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

tests/test_api_system_backup.py

@@ -16,21 +16,20 @@ async def _seed_runs(db):
     now = datetime.now(UTC)
     for i in range(4):
         db.add(BackupRun(
-            kind="db" if i % 2 == 0 else "images",
+            kind="db" if i % 2 == 0 else "media",
             status="ok",
             tag=None,
             triggered_by="manual",
             started_at=now - timedelta(seconds=10 - i),
             finished_at=now - timedelta(seconds=9 - i),
             sql_path="/tmp/fake.sql" if i % 2 == 0 else None,
-            tar_path=None if i % 2 == 0 else "/tmp/fake.tar.zst",
             size_bytes=100,
             manifest={},
         ))
     await db.commit()
 
 
-# --- POST /db, /images ----------------------------------------------
+# --- POST /db ---------------------------------------------------------
 
 
 @pytest.mark.asyncio
@@ -72,14 +71,10 @@ async def test_post_db_rejects_tag_too_long(client):
 
 
 @pytest.mark.asyncio
-async def test_post_images_dispatches(client, monkeypatch):
-    dispatched = []
-    monkeypatch.setattr(
-        "backend.app.tasks.backup.backup_images_task.delay",
-        lambda **kw: dispatched.append(kw),
-    )
-    resp = await client.post("/api/system/backup/images", json={})
-    assert resp.status_code == 202
+async def test_the_images_tarball_is_gone(client):
+    assert (await client.post("/api/system/backup/images", json={})).status_code in (404, 405)
+    resp = await client.get("/api/system/backup/runs?kind=images")
+    assert resp.status_code == 400
 
 
 # --- GET /runs -------------------------------------------------------
@@ -103,10 +98,10 @@ async def test_runs_filter_by_kind_db(client, _seed_runs):
 
 
 @pytest.mark.asyncio
-async def test_runs_filter_by_kind_images(client, _seed_runs):
-    resp = await client.get("/api/system/backup/runs?kind=images")
+async def test_runs_filter_by_kind_media(client, _seed_runs):
+    resp = await client.get("/api/system/backup/runs?kind=media")
     body = await resp.get_json()
-    assert all(r["kind"] == "images" for r in body["runs"])
+    assert body["runs"] and all(r["kind"] == "media" for r in body["runs"])
 
 
 @pytest.mark.asyncio
@@ -298,7 +293,7 @@ async def test_get_settings_returns_defaults(client):
     assert body["backup_db_nightly_enabled"] is False
     assert body["backup_db_nightly_hour_utc"] == 3
     assert body["backup_db_keep_last_n"] == 14
-    assert body["backup_images_keep_last_n"] == 3
+    assert "backup_images_keep_last_n" not in body
 
 
 @pytest.mark.asyncio

tests/test_backup_service.py

@@ -1,6 +1,6 @@
 """FC-3h: backup_service unit tests.
 
-Subprocess calls (pg_dump, tar, psql) are monkeypatched so tests run
+Subprocess calls (pg_dump, pg_restore, psql) are monkeypatched so tests run
 without external binaries. The real subprocess behavior is exercised
 implicitly via the Celery task tests in test_tasks_backup.py.
 """
@@ -32,9 +32,6 @@ def fake_subprocess(monkeypatch):
         if "pg_dump" in cmd:
             i = cmd.index("-f")
             Path(cmd[i + 1]).write_bytes(b"-- fake pg_dump\n")
-        elif cmd[0] == "tar" and "-cf" in cmd:
-            i = cmd.index("-cf")
-            Path(cmd[i + 1]).write_bytes(b"fake tar payload")
 
     def _fake_run(cmd, **kwargs):
         calls.append(list(cmd))
@@ -100,7 +97,6 @@ def test_backup_db_writes_sql_and_manifest(tmp_path, fake_subprocess):
     assert sql.is_file() and sql.suffix == ".dump"
     assert manifest.is_file() and manifest.suffix == ".json"
     assert result["kind"] == "db"
-    assert result["tar_path"] is None
     assert result["size_bytes"] == len(b"-- fake pg_dump\n")
 
     parsed = json.loads(manifest.read_text())
@@ -141,50 +137,6 @@ def test_backup_db_default_tag_is_none(tmp_path, fake_subprocess):
     assert parsed["tag"] is None
 
 
-# --- backup_images ---------------------------------------------------
-
-
-def test_backup_images_writes_tar_and_manifest(tmp_path, fake_subprocess):
-    result = backup_service.backup_images(
-        images_root=tmp_path, tag="monthly", triggered_by="manual",
-    )
-    tar = Path(result["tar_path"])
-    assert tar.is_file() and tar.name.endswith(".tar.zst")
-    assert result["sql_path"] is None
-    assert result["size_bytes"] == len(b"fake tar payload")
-
-
-def test_backup_images_excludes_backups_and_quarantine(tmp_path, fake_subprocess):
-    backup_service.backup_images(images_root=tmp_path)
-    cmd = fake_subprocess[0]
-    excludes = [arg for arg in cmd if arg.startswith("--exclude=")]
-    assert any("_backups" in e for e in excludes)
-    assert any("_quarantine" in e for e in excludes)
-
-
-def test_backup_images_excludes_credentials(tmp_path, fake_subprocess):
-    """#4234: the images tarball carried `secrets/credential_key.b64` — the key
-    that decrypts the stored session cookies — and `cookies/` itself. A media
-    archive must not be a credential leak; encryption at rest is worth nothing
-    if the key travels with the data."""
-    backup_service.backup_images(images_root=tmp_path)
-    excludes = [a for a in fake_subprocess[0] if a.startswith("--exclude=")]
-    assert any(e.endswith("/secrets") for e in excludes)
-    assert any(e.endswith("/cookies") for e in excludes)
-
-
-def test_backup_images_excludes_are_root_relative(tmp_path, fake_subprocess):
-    """tar matches --exclude against the archived path, which is prefixed with
-    the root's own directory name (`-C <parent> <name>`). A bare `secrets`
-    would also match an ARTIST folder called secrets; the prefix is what keeps
-    the exclusion to the top level."""
-    backup_service.backup_images(images_root=tmp_path)
-    excludes = [a for a in fake_subprocess[0] if a.startswith("--exclude=")]
-    assert excludes and all(
-        e.startswith(f"--exclude={tmp_path.name}/") for e in excludes
-    )
-
-
 # --- restore_db ------------------------------------------------------
 
 
@@ -267,41 +219,25 @@ def test_every_dump_leaves_out_account_logins(tmp_path, fake_subprocess):
     assert "--exclude-table-data=credential" in fake_subprocess[0]
 
 
-# --- restore_images --------------------------------------------------
-
-
-def test_restore_images_untar_to_parent(tmp_path, fake_subprocess):
-    tar_path = tmp_path / "fake.tar.zst"
-    tar_path.write_bytes(b"fake")
-    backup_service.restore_images(images_root=tmp_path, tar_path=tar_path)
-    cmd = fake_subprocess[0]
-    assert cmd[:3] == ["tar", "--zstd", "-xf"]
-    assert str(tar_path) in cmd
-    assert "-C" in cmd
-
-
 # --- unlink ----------------------------------------------------------
 
 
 def test_unlink_removes_present_files_and_reports(tmp_path):
     sql = tmp_path / "x.sql"
     sql.write_bytes(b"x")
-    tar = tmp_path / "x.tar.zst"
-    tar.write_bytes(b"x")
     manifest = tmp_path / "x.json"
     manifest.write_text("{}")
     result = backup_service.unlink_artifact_files(
-        sql_path=str(sql), tar_path=str(tar), manifest_path=str(manifest),
+        sql_path=str(sql), manifest_path=str(manifest),
     )
-    assert result == {"sql": True, "tar": True, "manifest": True}
-    assert not sql.exists() and not tar.exists() and not manifest.exists()
+    assert result == {"sql": True, "manifest": True}
+    assert not sql.exists() and not manifest.exists()
 
 
 def test_unlink_missing_files_returns_true(tmp_path):
     """missing_ok semantics: a non-existent file isn't an error."""
     result = backup_service.unlink_artifact_files(
-        sql_path=str(tmp_path / "nope.sql"),
-        tar_path=None, manifest_path=None,
+        sql_path=str(tmp_path / "nope.sql"), manifest_path=None,
     )
     assert result == {"sql": True}
 
@@ -309,7 +245,7 @@ def test_unlink_missing_files_returns_true(tmp_path):
 def test_unlink_skips_none_paths():
     """A None path is skipped — not added to the result dict."""
     result = backup_service.unlink_artifact_files(
-        sql_path=None, tar_path=None, manifest_path=None,
+        sql_path=None, manifest_path=None,
     )
     assert result == {}
 

tests/test_tasks_backup.py

@@ -36,9 +36,6 @@ def fake_subprocess_and_images_root(monkeypatch, tmp_path):
         if "pg_dump" in cmd:
             i = cmd.index("-f")
             Path(cmd[i + 1]).write_bytes(b"-- fake pg_dump\n")
-        elif cmd[0] == "tar" and "-cf" in cmd:
-            i = cmd.index("-cf")
-            Path(cmd[i + 1]).write_bytes(b"fake tar payload")
 
     def _fake_run(cmd, **kwargs):
         _sentinel(cmd)
@@ -55,7 +52,7 @@ def fake_subprocess_and_images_root(monkeypatch, tmp_path):
         def kill(self):
             self.returncode = -9
 
-    # backup_db/backup_images go through _run_bounded (Popen); restore via run.
+    # backup_db goes through _run_bounded (Popen); restore via run.
     monkeypatch.setattr("subprocess.run", _fake_run)
     monkeypatch.setattr("subprocess.Popen", _FakePopen)
 
@@ -67,7 +64,6 @@ def _seed_backup(db_sync, *, kind, status, started_at, tag=None,
         triggered_by="manual", started_at=started_at,
         finished_at=finished_at or (started_at + timedelta(seconds=10)),
         sql_path="/tmp/fake.sql" if kind == "db" else None,
-        tar_path="/tmp/fake.tar.zst" if kind == "images" else None,
         manifest={},
     )
     db_sync.add(row)
@@ -185,22 +181,6 @@ async def test_an_off_site_dump_that_fails_is_recorded_with_why(db_sync, monkeyp
     assert row.error == "DumpFailed: pg_dump: error: connection lost"
 
 
-# --- backup_images_task ---------------------------------------------
-
-
-@pytest.mark.asyncio
-async def test_backup_images_task_creates_backup_run(db_sync):
-    from backend.app.tasks.backup import backup_images_task
-    result = backup_images_task.delay().get()
-    row = db_sync.execute(
-        select(BackupRun.kind, BackupRun.status, BackupRun.tar_path)
-        .where(BackupRun.id == result["backup_run_id"])
-    ).one()
-    assert row.kind == "images"
-    assert row.status == "ok"
-    assert row.tar_path and row.tar_path.endswith(".tar.zst")
-
-
 # --- restore_db_task ------------------------------------------------
 
 
@@ -226,11 +206,11 @@ async def test_restore_db_task_rejects_non_db_source(db_sync):
     from backend.app.tasks.backup import restore_db_task
 
     now = datetime.now(UTC)
-    img_id = _seed_backup(db_sync, kind="images", status="ok", started_at=now)
+    media_id = _seed_backup(db_sync, kind="media", status="ok", started_at=now)
     db_sync.commit()
 
     with pytest.raises(ValueError):
-        restore_db_task.delay(source_backup_run_id=img_id).get()
+        restore_db_task.delay(source_backup_run_id=media_id).get()
 
 
 # --- prune_backups --------------------------------------------------
@@ -243,7 +223,6 @@ def test_prune_backups_keeps_last_n_untagged_per_kind(db_sync):
         select(ImportSettings).where(ImportSettings.id == 1)
     ).scalar_one()
     s.backup_db_keep_last_n = 2
-    s.backup_images_keep_last_n = 1
     db_sync.commit()
 
     now = datetime.now(UTC)
@@ -251,13 +230,13 @@ def test_prune_backups_keeps_last_n_untagged_per_kind(db_sync):
         _seed_backup(db_sync, kind="db", status="ok",
                      started_at=now - timedelta(hours=i))
     for i in range(3):
-        _seed_backup(db_sync, kind="images", status="ok",
+        _seed_backup(db_sync, kind="media", status="ok",
                      started_at=now - timedelta(hours=i))
     db_sync.commit()
 
     result = prune_backups.apply().get()
     assert result["db_deleted"] == 2
-    assert result["images_deleted"] == 2
+    assert result["media_deleted"] == 0        # 60 media runs are kept
 
 
 def test_prune_backups_protects_tagged_rows(db_sync):
@@ -321,9 +300,9 @@ def test_stall_sweep_flips_db_fast_but_spares_running_images(db_sync):
         db_sync, kind="db", status="running",
         started_at=now - timedelta(minutes=50),
     )
-    # An images backup running 50 min is still well under the 7h window → spared.
+    # A test restore running 50 min is still well under the 7h window → spared.
     img_id = _seed_backup(
-        db_sync, kind="images", status="running",
+        db_sync, kind="test", status="running",
         started_at=now - timedelta(minutes=50),
     )
     db_sync.commit()