CI / lint (push) Successful in 3s
CI / extension-version (push) Successful in 3s
Build images / sign-extension (push) Successful in 4s
Build images / build-agent (push) Successful in 7s
CI / frontend-build (push) Successful in 23s
CI / backend-lint-and-test (push) Successful in 33s
Build images / build-web (push) Successful in 57s
Build images / smoke-web (push) Skipped
Build images / build-ml (push) Successful in 1m49s
Build images / promote (push) Skipped
CI / integration (push) Successful in 2m20s
Listing a 2026-05 tarball while investigating the 4.3T `_backups` pile showed
its second and third entries:
images/secrets/
images/secrets/credential_key.b64
That is the key that decrypts the stored Patreon/SubscribeStar session
credentials, and `cookies/` sat beside it — both unexcluded, so this was true
of every images backup taken today, not just the old ones. An images tarball
is supposed to be a media archive; one that carries the operator's account
keys is a credential leak wearing a backup's name, in a single file that is
easy to copy to another disk or restore somewhere less protected. Encryption
at rest buys nothing when the key travels in the same archive.
`secrets` and `cookies` join `_backups` and `_quarantine` in one named tuple,
each with its reason recorded — the recursion that produced 4.3T of nested
tarballs is the cautionary tale for why the list is worth explaining rather
than just listing.
A restore no longer re-establishes credentials. You sign in again, which is
the correct outcome for a media backup.
Tests cover both new names and that every exclude stays root-relative — a bare
`secrets` would also match an artist folder of that name.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LVjrnpQjRgHdvq95rASoiR
287 lines
9.8 KiB
Python
287 lines
9.8 KiB
Python
"""FC-3h: backup_service unit tests.
|
|
|
|
Subprocess calls (pg_dump, tar, 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.
|
|
"""
|
|
import json
|
|
import subprocess
|
|
from pathlib import Path
|
|
|
|
import pytest
|
|
|
|
from backend.app.services import backup_service
|
|
|
|
pytestmark = pytest.mark.integration
|
|
|
|
|
|
@pytest.fixture
|
|
def fake_subprocess(monkeypatch):
|
|
"""Fake both subprocess.run (restore path) AND subprocess.Popen (the
|
|
bounded-kill backup path) so tests run without external binaries. Each
|
|
writes the target sentinel and records the cmd in a shared list."""
|
|
calls = []
|
|
|
|
class _FakeProc:
|
|
returncode = 0
|
|
stdout = b""
|
|
stderr = b""
|
|
|
|
def _write_sentinel(cmd):
|
|
if cmd[0] == "pg_dump":
|
|
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))
|
|
_write_sentinel(cmd)
|
|
return _FakeProc()
|
|
|
|
class _FakePopen:
|
|
def __init__(self, cmd, **kwargs):
|
|
calls.append(list(cmd))
|
|
self.returncode = 0
|
|
_write_sentinel(cmd)
|
|
|
|
def communicate(self, timeout=None):
|
|
return (b"", b"")
|
|
|
|
def kill(self):
|
|
self.returncode = -9
|
|
|
|
monkeypatch.setattr("subprocess.run", _fake_run)
|
|
monkeypatch.setattr("subprocess.Popen", _FakePopen)
|
|
return calls
|
|
|
|
|
|
# --- backup_db -------------------------------------------------------
|
|
|
|
|
|
def test_backup_db_writes_sql_and_manifest(tmp_path, fake_subprocess):
|
|
result = backup_service.backup_db(
|
|
db_url="postgresql://test@x/db",
|
|
images_root=tmp_path,
|
|
tag="pre-cutover",
|
|
triggered_by="manual",
|
|
)
|
|
sql = Path(result["sql_path"])
|
|
manifest = Path(result["manifest_path"])
|
|
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())
|
|
assert parsed["kind"] == "db"
|
|
assert parsed["tag"] == "pre-cutover"
|
|
assert parsed["triggered_by"] == "manual"
|
|
assert parsed["artifact_path"] == str(sql)
|
|
|
|
|
|
def test_backup_db_strips_sqlalchemy_psycopg_driver(tmp_path, fake_subprocess):
|
|
backup_service.backup_db(
|
|
db_url="postgresql+psycopg://u:p@h/d", images_root=tmp_path,
|
|
)
|
|
cmd = fake_subprocess[0]
|
|
assert cmd[0] == "pg_dump"
|
|
assert cmd[-1].startswith("postgresql://")
|
|
assert "+psycopg" not in cmd[-1]
|
|
|
|
|
|
def test_backup_db_uses_compressed_custom_format(tmp_path, fake_subprocess):
|
|
# -Fc → pg_restore-loadable, compressed dump (#739 backup polish).
|
|
backup_service.backup_db(db_url="postgresql://u@h/d", images_root=tmp_path)
|
|
assert "-Fc" in fake_subprocess[0]
|
|
|
|
|
|
def test_backup_db_strips_asyncpg_driver(tmp_path, fake_subprocess):
|
|
backup_service.backup_db(
|
|
db_url="postgresql+asyncpg://u:p@h/d", images_root=tmp_path,
|
|
)
|
|
assert "+asyncpg" not in fake_subprocess[0][-1]
|
|
|
|
|
|
def test_backup_db_default_tag_is_none(tmp_path, fake_subprocess):
|
|
result = backup_service.backup_db(
|
|
db_url="postgresql://u@h/d", images_root=tmp_path,
|
|
)
|
|
parsed = json.loads(Path(result["manifest_path"]).read_text())
|
|
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 ------------------------------------------------------
|
|
|
|
|
|
def test_restore_db_drops_schema_then_loads(tmp_path, fake_subprocess):
|
|
dump_path = tmp_path / "fake.dump"
|
|
dump_path.write_bytes(b"\x00fake custom dump")
|
|
backup_service.restore_db(
|
|
db_url="postgresql://u@h/d", sql_path=dump_path,
|
|
)
|
|
# Two calls: psql -c (DROP SCHEMA), then pg_restore -d (load the dump).
|
|
assert len(fake_subprocess) == 2
|
|
assert fake_subprocess[0][0] == "psql"
|
|
assert "-c" in fake_subprocess[0]
|
|
assert "DROP SCHEMA IF EXISTS public CASCADE" in fake_subprocess[0][-1]
|
|
assert fake_subprocess[1][0] == "pg_restore"
|
|
assert "-d" in fake_subprocess[1]
|
|
assert str(dump_path) in fake_subprocess[1]
|
|
|
|
|
|
# --- 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),
|
|
)
|
|
assert result == {"sql": True, "tar": True, "manifest": True}
|
|
assert not sql.exists() and not tar.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,
|
|
)
|
|
assert result == {"sql": True}
|
|
|
|
|
|
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,
|
|
)
|
|
assert result == {}
|
|
|
|
|
|
# --- helpers ---------------------------------------------------------
|
|
|
|
|
|
def test_libpq_url_strips_each_known_driver():
|
|
assert backup_service._libpq_url(
|
|
"postgresql+psycopg://u@h/d"
|
|
) == "postgresql://u@h/d"
|
|
assert backup_service._libpq_url(
|
|
"postgresql+asyncpg://u@h/d"
|
|
) == "postgresql://u@h/d"
|
|
assert backup_service._libpq_url(
|
|
"postgresql+psycopg2://u@h/d"
|
|
) == "postgresql://u@h/d"
|
|
# Plain URL passes through unchanged.
|
|
assert backup_service._libpq_url(
|
|
"postgresql://u@h/d"
|
|
) == "postgresql://u@h/d"
|
|
|
|
|
|
def test_backups_dir_created_on_first_use(tmp_path):
|
|
d = backup_service._backups_dir(tmp_path)
|
|
assert d.is_dir()
|
|
assert d.name == "_backups"
|
|
|
|
|
|
# --- bounded-kill + local-temp (FC #739) -----------------------------
|
|
|
|
|
|
def test_backup_db_dumps_to_local_temp_not_nfs_backups_dir(tmp_path, fake_subprocess):
|
|
"""pg_dump must target a LOCAL temp path, not the (NFS) _backups dir —
|
|
so its long phase can't hang uninterruptibly on an NFS write."""
|
|
backup_service.backup_db(db_url="postgresql://u@h/d", images_root=tmp_path)
|
|
cmd = fake_subprocess[0]
|
|
dump_target = cmd[cmd.index("-f") + 1]
|
|
assert "_backups" not in dump_target
|
|
# The finished file still ends up in _backups (moved there).
|
|
result = backup_service.backup_db(
|
|
db_url="postgresql://u@h/d", images_root=tmp_path,
|
|
)
|
|
assert "_backups" in result["sql_path"]
|
|
|
|
|
|
def test_run_bounded_fails_fast_when_unkillable(monkeypatch):
|
|
"""A child stuck in D-state (communicate keeps timing out even after kill)
|
|
must NOT block the reaper — _run_bounded kills then re-raises promptly."""
|
|
killed = {"n": 0}
|
|
|
|
class _Hang:
|
|
def __init__(self, cmd, **kwargs):
|
|
pass
|
|
|
|
def communicate(self, timeout=None):
|
|
raise subprocess.TimeoutExpired(cmd="x", timeout=timeout or 0)
|
|
|
|
def kill(self):
|
|
killed["n"] += 1
|
|
|
|
monkeypatch.setattr("subprocess.Popen", _Hang)
|
|
with pytest.raises(subprocess.TimeoutExpired):
|
|
backup_service._run_bounded(["pg_dump"], 1)
|
|
assert killed["n"] == 1
|