From 6915cbbbe1f988b31c10bd429e15ab423523b11d Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Mon, 21 Sep 2026 08:35:35 -0400 Subject: [PATCH] fix: the images backup carried the key to the accounts it backs up (4234) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) Claude-Session: https://claude.ai/code/session_01LVjrnpQjRgHdvq95rASoiR --- backend/app/services/backup_service.py | 25 +++++++++++++++++++++++-- tests/test_backup_service.py | 23 +++++++++++++++++++++++ 2 files changed, 46 insertions(+), 2 deletions(-) diff --git a/backend/app/services/backup_service.py b/backend/app/services/backup_service.py index ac59b93..3c1bced 100644 --- a/backend/app/services/backup_service.py +++ b/backend/app/services/backup_service.py @@ -24,6 +24,25 @@ from pathlib import Path _BACKUPS_DIRNAME = "_backups" +# Excluded from the images tarball, and each for its own reason (#4233, #4234): +# +# _backups — the archive would otherwise contain every previous archive. +# This is not hypothetical: the 2026-05-23/24 runs, taken before +# this exclude existed, grew 43G -> 107G -> ... -> 2123G as each +# swallowed its predecessors, and cost 4.3T of the images +# filesystem until they were reclaimed on 2026-09-21. +# _quarantine — holds files deliberately pulled OUT of the library. +# secrets — `credential_key.b64`, the key that decrypts the stored +# Patreon/SubscribeStar session credentials. +# cookies — those session cookies themselves. +# +# The last two are the ones worth stating plainly: an images tarball is a media +# archive, and a media archive that carries the key to the operator's accounts +# is a credential leak wearing a backup's name. Encryption at rest buys nothing +# when the key rides along in the same file. A restore therefore does NOT +# re-establish credentials — you sign in again, which is the correct outcome. +_IMAGES_EXCLUDED_DIRNAMES = ("_backups", "_quarantine", "secrets", "cookies") + # Subprocess-level guardrails BEYOND the Celery soft_time_limit. The Celery # soft limit signals the Python process; subprocess.Popen in a blocking syscall # ignores that signal, so these bound the worst case directly. Each sits just @@ -173,8 +192,10 @@ def backup_images( [ "tar", "--zstd", "-cf", str(tar_path), "-C", str(images_root.parent), images_root.name, - f"--exclude={images_root.name}/_backups", - f"--exclude={images_root.name}/_quarantine", + *( + f"--exclude={images_root.name}/{name}" + for name in _IMAGES_EXCLUDED_DIRNAMES + ), ], _IMAGES_SUBPROCESS_TIMEOUT_S, ) diff --git a/tests/test_backup_service.py b/tests/test_backup_service.py index cf45fb2..2c2d323 100644 --- a/tests/test_backup_service.py +++ b/tests/test_backup_service.py @@ -134,6 +134,29 @@ def test_backup_images_excludes_backups_and_quarantine(tmp_path, fake_subprocess 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 `). 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 ------------------------------------------------------