check, repo-compress: read packs from the repository, not from the store cache, fixes #10397

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:

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

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.

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.

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 <cache_dir>/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")

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

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

src/borg/testsuite/repository_test.py

@@ -819,6 +819,38 @@ 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_store_cache_default_directory(tmp_path, monkeypatch):
+    # BORG_STORE_CACHE=1 caches the packs in <cache_dir>/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)}