From fc514a88741ba0c889d26e965a32ca134752a2d1 Mon Sep 17 00:00:00 2001 From: Mrityunjay Raj Date: Tue, 29 Sep 2026 21:23:09 +0530 Subject: [PATCH 1/2] check, repo-compress: read packs from the repository, not from the store cache, fixes #10397 The cached packs (BORG_STORE_CACHE) are not verified against the repository. With the cache, check verified the cached copies instead of the repository: a damaged cache file was reported as an integrity error and check --repair deleted the intact object from the repository, while damage in the repository was not found as long as an intact copy was cached. repo-compress copies objects that already use the target compression into the new pack without authenticating them, so a damaged cache file ended up in the repository. Repository(store_cache=False) ignores BORG_STORE_CACHE. with_repository and get_repository pass it through; check and repo-compress use it. --- src/borg/archiver/_common.py | 19 ++++++- src/borg/archiver/check_cmd.py | 7 ++- src/borg/archiver/help_cmd.py | 3 ++ src/borg/archiver/repo_compress_cmd.py | 6 ++- src/borg/repository.py | 11 ++-- src/borg/testsuite/archiver/check_cmd_test.py | 51 +++++++++++++++++++ .../archiver/repo_compress_cmd_test.py | 27 +++++++++- src/borg/testsuite/repository_test.py | 18 +++++++ 8 files changed, 134 insertions(+), 8 deletions(-) diff --git a/src/borg/archiver/_common.py b/src/borg/archiver/_common.py index 610f2fdeca..69c78ab2b0 100644 --- a/src/borg/archiver/_common.py +++ b/src/borg/archiver/_common.py @@ -32,11 +32,22 @@ def get_repository( - location, *, create, exclusive, lock_wait, lock, args, v1_legacy, allow_incomplete=False, other=False + location, + *, + create, + exclusive, + lock_wait, + lock, + args, + v1_legacy, + allow_incomplete=False, + other=False, + store_cache=True, ): # create_config=False: when creating, the command (repo-create) writes the repository config itself, # once the key exists, see Repository.create(). For an existing repository, the flag is irrelevant. # other=True: the "other" repository, its key is loaded with the BORG_OTHER_* settings (see key_factory). + # store_cache=False: ignore BORG_STORE_CACHE, read all packs from the repository. key_loader = functools.partial(key_factory, other=True) if other else None if location.proto == "ssh" and v1_legacy: # legacy borg 1.x repository, served by a remote "borg serve" via the legacy RPC protocol @@ -58,6 +69,7 @@ def get_repository( lock_wait=lock_wait, lock=lock, key_loader=key_loader, + store_cache=store_cache, ) else: @@ -77,6 +89,7 @@ def get_repository( lock_wait=lock_wait, lock=lock, key_loader=key_loader, + store_cache=store_cache, ) return repository @@ -121,6 +134,7 @@ def with_repository( secure=True, allow_v1=False, allow_incomplete=False, + store_cache=True, ): """ Method decorator for subcommand-handling methods: do_XYZ(self, args, repository, …) @@ -135,6 +149,8 @@ def with_repository( :param allow_v1: (bool) allow legacy Borg 1.x repositories :param allow_incomplete: (bool) also open a store without repository config (repository.incomplete is True then, nothing else is usable), see Repository.create() - for "borg repo-delete --force". + :param store_cache: (bool) use the pack cache configured by BORG_STORE_CACHE. + False: ignore BORG_STORE_CACHE, read all packs from the repository. """ # We may need to modify `lock` inside `wrapper`. Therefore we cannot use the # `nonlocal` statement to access `lock` as modifications would also @@ -164,6 +180,7 @@ def wrapper(self, args, **kwargs): args=args, v1_legacy=v1_legacy, allow_incomplete=allow_incomplete, + store_cache=store_cache, ) with repository: diff --git a/src/borg/archiver/check_cmd.py b/src/borg/archiver/check_cmd.py index 21e6f692e2..b4aa6b9173 100644 --- a/src/borg/archiver/check_cmd.py +++ b/src/borg/archiver/check_cmd.py @@ -50,7 +50,9 @@ def check_repository_defaults(repository, *, repair): class CheckMixIn: - @with_repository(exclusive=True, manifest=False) + # store_cache=False: the cached packs are not verified against the repository. check must verify the + # packs in the repository, and --repair deletes the objects that fail verification. + @with_repository(exclusive=True, manifest=False, store_cache=False) def do_check(self, args, repository): """Checks repository consistency.""" if args.repair: @@ -256,6 +258,9 @@ def build_parser_check(self, subparsers, common_parser, mid_common_parser): If the repository holds another copy of such a chunk and that copy passes the verification, borg indexes it instead, so the archives referencing the chunk stay intact. + ``borg check`` always reads the packs from the repository, also if ``BORG_STORE_CACHE`` + is set. + The ``--find-lost-archives`` option tells Borg to search for lost archive metadata. If Borg encounters any archive metadata that does not match an archive directory entry (including soft-deleted archives), it means that an diff --git a/src/borg/archiver/help_cmd.py b/src/borg/archiver/help_cmd.py index 96cca3a20e..d5357f45a2 100644 --- a/src/borg/archiver/help_cmd.py +++ b/src/borg/archiver/help_cmd.py @@ -751,6 +751,9 @@ class HelpMixIn: use that directory (it is created if it does not exist). Packs are named by content hash, so one cache directory can safely hold packs of multiple repositories. If it is not set, no such caching happens. + The cached packs are not verified against the repository: a damaged cache file makes + reads fail as if the repository was damaged, until the cache directory is deleted. + ``borg check`` and ``borg repo-compress`` always read the packs from the repository. BORG_PACK_CACHE_SIZE When set to a numeric value, limit the pack cache to that many bytes. Only has an effect if BORG_STORE_CACHE is set. diff --git a/src/borg/archiver/repo_compress_cmd.py b/src/borg/archiver/repo_compress_cmd.py index 02c0859c1c..899e066b96 100644 --- a/src/borg/archiver/repo_compress_cmd.py +++ b/src/borg/archiver/repo_compress_cmd.py @@ -222,7 +222,10 @@ def report(self, size_before, size_after): class RepoCompressMixIn: - @with_repository(manifest=True, exclusive=True) + # store_cache=False: the cached packs are not verified against the repository. transform_pack copies + # objects that already use the target compression into the new pack without authenticating them, + # then deletes the old pack. + @with_repository(manifest=True, exclusive=True, store_cache=False) def do_repo_compress(self, args, repository, manifest): """Repository (re-)compression.""" if not isinstance(repository, Repository): @@ -248,6 +251,7 @@ def build_parser_repo_compress(self, subparsers, common_parser, mid_common_parse if it holds objects that need recompression, rewritten as a whole - objects already using the desired compression are copied into the rewritten pack unchanged. A pack whose objects all already use the desired compression is not touched at all. + The packs are always read from the repository, also if ``BORG_STORE_CACHE`` is set. Please note that the outcome of recompressing a chunk might not always be the desired compression type/level - if no compression gives a shorter output, that might be chosen; such chunks are kept as they are. diff --git a/src/borg/repository.py b/src/borg/repository.py index e3c19c40dc..c3118e6c52 100644 --- a/src/borg/repository.py +++ b/src/borg/repository.py @@ -965,6 +965,7 @@ def __init__( send_log_cb=None, permissions=None, key_loader=None, + store_cache=True, ): if isinstance(path_or_location, Location): location = path_or_location @@ -997,13 +998,15 @@ def __init__( # BORG_STORE_CACHE sets the cache directory ("1" means /storecache); the # directory holds the whole store's cache, currently just the packs/ namespace. # BORG_PACK_CACHE_SIZE limits the pack cache size in bytes. + # The cached packs are not verified against the repository. + # store_cache=False: ignore BORG_STORE_CACHE, read all packs from the repository. cache_url = None - store_cache = os.environ.get("BORG_STORE_CACHE") - if store_cache: - if store_cache == "1": + store_cache_dir = os.environ.get("BORG_STORE_CACHE") if store_cache else None + if store_cache_dir: + if store_cache_dir == "1": cache_dir = Path(get_cache_dir("storecache")) else: - cache_dir = Path(store_cache) + cache_dir = Path(store_cache_dir) cache_dir.mkdir(parents=True, exist_ok=True) ns_config["packs/"]["cache"] = "writethrough" cache_size = os.environ.get("BORG_PACK_CACHE_SIZE") diff --git a/src/borg/testsuite/archiver/check_cmd_test.py b/src/borg/testsuite/archiver/check_cmd_test.py index 0bc9930851..f1ad520895 100644 --- a/src/borg/testsuite/archiver/check_cmd_test.py +++ b/src/borg/testsuite/archiver/check_cmd_test.py @@ -2521,3 +2521,54 @@ def test_items_with_unknown_keys_are_kept(archivers, request): assert items[0].as_dict()["newkey"] == "future" output = cmd(archiver, "check", "--archives-only", exit_code=0) assert "keys unknown to this borg version" in output # still just the warning + + +def fill_store_cache(archiver, monkeypatch): + """Set BORG_STORE_CACHE, extract archive1 to cache its packs and return the cached pack files.""" + cache_dir = archiver.tmpdir / "storecache" + monkeypatch.setenv("BORG_STORE_CACHE", os.fspath(cache_dir)) + with changedir(archiver.output_path): + cmd(archiver, "extract", "archive1") + cached_packs = sorted(path for path in (cache_dir / "packs").rglob("*") if path.is_file()) + assert cached_packs + return cached_packs + + +def repository_packs(archiver): + return sorted(path.name for path in Path(archiver.repository_path, "packs").rglob("*") if path.is_file()) + + +def test_check_verify_data_ignores_a_corrupt_store_cache(archivers, request, monkeypatch): + # damaged cached packs, intact repository: check reports no error and --repair deletes no pack, #10397. + archiver = request.getfixturevalue(archivers) + check_cmd_setup(archiver) + for path in fill_store_cache(archiver, monkeypatch): + path.write_bytes(corrupt(path.read_bytes(), path.stat().st_size // 2)) + packs_before = repository_packs(archiver) + output = cmd(archiver, "check", "--verify-data", exit_code=0) + assert "integrity error:" not in output + monkeypatch.setenv("BORG_CHECK_I_KNOW_WHAT_I_AM_DOING", "YES") + output = cmd(archiver, "check", "--repair", "--verify-data", exit_code=0) + assert "integrity error:" not in output + # --repair can add packs holding rewritten archive metadata, so check only that no pack was removed. + assert set(packs_before) <= set(repository_packs(archiver)) + + +def test_check_archives_only_verify_data_ignores_an_intact_store_cache(archivers, request, monkeypatch): + # damaged repository pack, intact cached copy: check reports the integrity error, #10397. + archiver = request.getfixturevalue(archivers) + check_cmd_setup(archiver) + fill_store_cache(archiver, monkeypatch) + # writethrough also writes the cache, so damage the repository pack with BORG_STORE_CACHE unset. + monkeypatch.delenv("BORG_STORE_CACHE") + archive, repository = open_archive(archiver.repository_path, "archive1") + with repository: + for item in archive.iter_items(): + if item.path.endswith(src_file): + corrupt_chunk_on_disk(repository, item.chunks[-1].id) + break + else: + pytest.fail("should not happen") + monkeypatch.setenv("BORG_STORE_CACHE", os.fspath(archiver.tmpdir / "storecache")) + output = cmd(archiver, "check", "--archives-only", "--verify-data", exit_code=1) + assert "integrity error:" in output diff --git a/src/borg/testsuite/archiver/repo_compress_cmd_test.py b/src/borg/testsuite/archiver/repo_compress_cmd_test.py index a3481b9215..842c60e7a6 100644 --- a/src/borg/testsuite/archiver/repo_compress_cmd_test.py +++ b/src/borg/testsuite/archiver/repo_compress_cmd_test.py @@ -11,7 +11,7 @@ from ...compress import ZSTD, ZLIB, LZ4, CNONE from ...archiver.repo_compress_cmd import PackRecompressor -from .. import make_test_key +from .. import changedir, make_test_key from . import create_regular_file, cmd, open_repository, RK_ENCRYPTION from ..repository_test import H, accept_all, fchunk, pdchunk, corrupt_chunk_on_disk @@ -426,3 +426,28 @@ def test_transform_pack_unchanged_pack_untouched(tmp_path): assert {info.name for info in repository.store_list("packs")} == pack_names_before assert pdchunk(repository.get(H(0))) == b"WWWW" assert pdchunk(repository.get(H(1))) == b"XXXX" + + +def test_repo_compress_ignores_a_corrupt_store_cache(archiver, monkeypatch): + # damaged cached packs, intact repository: repo-compress rewrites the packs from the repository, #10397. + create_regular_file(archiver.input_path, "compressible", contents=b"compressible " * 10000) + create_regular_file(archiver.input_path, "random", contents=os.urandom(1000000)) + cmd(archiver, "repo-create", RK_ENCRYPTION) + # "compressible" is stored lz4-compressed, "random" uncompressed: -C none recompresses "compressible" + # and copies "random", which holds the middle of the pack, unchanged into the new pack. + cmd(archiver, "create", "test", "input", "-C", "auto,lz4") + cache_dir = archiver.tmpdir / "storecache" + monkeypatch.setenv("BORG_STORE_CACHE", os.fspath(cache_dir)) + with changedir(archiver.output_path): + cmd(archiver, "extract", "test") # fills the store cache + cached_packs = [path for path in (cache_dir / "packs").rglob("*") if path.is_file()] + assert cached_packs + for path in cached_packs: + data = bytearray(path.read_bytes()) + data[len(data) // 2] ^= 0xFF + path.write_bytes(data) + + output = cmd(archiver, "repo-compress", "-C", "none", "--stats") + assert re.search(r"Packs: \d+ total, [1-9]\d* rewritten", output) + output = cmd(archiver, "check", "--verify-data", exit_code=0) + assert "integrity error:" not in output diff --git a/src/borg/testsuite/repository_test.py b/src/borg/testsuite/repository_test.py index 0c7edfd062..9771a3c54a 100644 --- a/src/borg/testsuite/repository_test.py +++ b/src/borg/testsuite/repository_test.py @@ -819,6 +819,24 @@ def test_gather_many_one_gather_for_many_packs(tmp_path, monkeypatch, variant): assert repository.store.stats["load_calls"] == loads_before +@pytest.mark.parametrize("variant", ["file", "ssh"]) +@pytest.mark.parametrize("store_cache", [True, False]) +def test_store_cache_argument(tmp_path, monkeypatch, variant, store_cache): + # store_cache=False ignores BORG_STORE_CACHE: no cache directory is made and pack reads go to the repository. + cache_dir = tmp_path / "storecache" + monkeypatch.setenv("BORG_STORE_CACHE", os.fspath(cache_dir)) + path = os.fspath(tmp_path / "repository") + location = Location(f"ssh://__testsuite__/{path}" if variant == "ssh" else path) + chunk = fchunk(b"payload", chunk_id=H(0)) + with Repository(location, exclusive=True, create=True, store_cache=store_cache) as repository: + assert repository.uses_pack_store_cache is store_cache + repository.put(H(0), chunk) + repository.flush() + assert list(repository.get_many([H(0)])) == [chunk] + assert (repository.store.stats["cache_load_calls"] > 0) is store_cache + assert cache_dir.exists() is store_cache + + def test_gather_many_batches(repo_fixtures, request, monkeypatch): # gather_many ends a batch at GATHER_MAX_COUNT objects or once a batch has GATHER_MAX_SIZE bytes. objects = {H(i): fchunk(b"payload-%02d" % i, chunk_id=H(i)) for i in range(5)} From c28db79e712fc4b282831a3b2a072ffca9da4512 Mon Sep 17 00:00:00 2001 From: Mrityunjay Raj Date: Wed, 30 Sep 2026 05:59:08 +0530 Subject: [PATCH 2/2] tests: BORG_STORE_CACHE=1 caches the packs in /storecache --- src/borg/testsuite/repository_test.py | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/src/borg/testsuite/repository_test.py b/src/borg/testsuite/repository_test.py index 9771a3c54a..f9a253e0ee 100644 --- a/src/borg/testsuite/repository_test.py +++ b/src/borg/testsuite/repository_test.py @@ -837,6 +837,20 @@ def test_store_cache_argument(tmp_path, monkeypatch, variant, store_cache): assert cache_dir.exists() is store_cache +def test_store_cache_default_directory(tmp_path, monkeypatch): + # BORG_STORE_CACHE=1 caches the packs in /storecache. + monkeypatch.setenv("BORG_CACHE_DIR", os.fspath(tmp_path / "cache")) + monkeypatch.setenv("BORG_STORE_CACHE", "1") + chunk = fchunk(b"payload", chunk_id=H(0)) + with Repository(Location(os.fspath(tmp_path / "repository")), exclusive=True, create=True) as repository: + assert repository.uses_pack_store_cache + repository.put(H(0), chunk) + repository.flush() + assert list(repository.get_many([H(0)])) == [chunk] + assert repository.store.stats["cache_load_calls"] > 0 + assert any(path.is_file() for path in (tmp_path / "cache" / "storecache").rglob("*")) + + def test_gather_many_batches(repo_fixtures, request, monkeypatch): # gather_many ends a batch at GATHER_MAX_COUNT objects or once a batch has GATHER_MAX_SIZE bytes. objects = {H(i): fchunk(b"payload-%02d" % i, chunk_id=H(i)) for i in range(5)}