From 7d37fa1a99428fa6d181137e195193017e4a183c Mon Sep 17 00:00:00 2001 From: Vinay Kaushal Date: Sat, 17 Oct 2026 21:23:38 +0530 Subject: [PATCH] repository: keep an identical existing pack when storing it is denied, fixes #10512 An aborted session can store a pack, but not the index entries for its objects. A later session then builds the same pack again (e.g. in the authenticated modes, the objects of unchanged input data are identical) and storing it overwrites the existing pack. With BORG_REPO_PERMISSIONS=no-delete or write-only, overwriting is not allowed, so every following backup failed with PermissionDenied. The pack name is the store hash of the pack bytes, so an existing pack with that name and size has the same contents. If the store is denied, check for that and keep the existing pack instead of failing. The check only runs after a denied store, so normal pack stores do not need an extra round trip. --- src/borg/repository.py | 19 ++++++++++- src/borg/testsuite/repository_test.py | 45 +++++++++++++++++++++++++++ 2 files changed, 63 insertions(+), 1 deletion(-) diff --git a/src/borg/repository.py b/src/borg/repository.py index 4ceb987c70..86e3ffa08c 100644 --- a/src/borg/repository.py +++ b/src/borg/repository.py @@ -275,7 +275,7 @@ def _store_pieces(self, pieces, outcome, trace=False): offset += obj_size self._trace("S", trace) - self.store.store("packs/" + bin_to_hex(pack_id), pack_data) + self._store_pack(pack_id, pack_data) except BaseException as exc: # incl. KeyboardInterrupt: it must not vanish with the thread outcome.error = exc else: @@ -283,6 +283,23 @@ def _store_pieces(self, pieces, outcome, trace=False): finally: self._trace(">", trace) + def _store_pack(self, pack_id, pack_data): + """Store a pack, or keep the identical pack that already is stored under its name. + + The pack name is the store hash of the pack bytes, so a stored pack with that name and size has the + same contents. That happens if an aborted session stored the pack, but not the index entries for its + objects, and a later session builds the same pack again (e.g. in the authenticated modes, the objects + of unchanged input data are identical). Overwriting is not needed then and the no-delete or + write-only repo permissions do not allow it, so the existing pack is kept (#10512). + """ + name = "packs/" + bin_to_hex(pack_id) + try: + self.store.store(name, pack_data) + except StorePermissionDenied: + info = self.store.info(name) + if not (info.exists and info.size == len(pack_data)): + raise + def _apply_outcome(self, outcome): """Apply one finished pack store to the ChunkIndex (calling thread only). diff --git a/src/borg/testsuite/repository_test.py b/src/borg/testsuite/repository_test.py index 142630a1ae..0e1c637bae 100644 --- a/src/borg/testsuite/repository_test.py +++ b/src/borg/testsuite/repository_test.py @@ -1696,6 +1696,51 @@ def test_pack_writer_async_get_waits_for_inflight_store(tmp_path): assert repository.get(H(1)) == chunk1 +def create_repo_with_index(location): + # create a repository with one stored pack and a stored index, like after a first backup. + with Repository(location, exclusive=True, create=True) as repository: + repository.put(H(0), fchunk(b"FIRST", chunk_id=H(0))) + repository.flush() + + +@pytest.mark.parametrize("permissions", ["no-delete", "write-only"]) +def test_pack_writer_keeps_identical_existing_pack(tmp_path, monkeypatch, permissions): + # an aborted session stored a pack, but not the index entries for its objects. a later session builds + # the same pack again: storing it would overwrite, which the permissions deny. the existing pack has the + # same contents (its name is their store hash), so it is kept and used instead (#10512). + location = os.fspath(tmp_path / "repo") + create_repo_with_index(location) + monkeypatch.setenv("BORG_REPO_PERMISSIONS", permissions) + chunk = fchunk(b"DATA", chunk_id=H(1)) + with Repository(location, exclusive=True) as repository: + repository.put(H(1), chunk) + repository.flush() + pack_id = repository.chunks[H(1)].pack_id + packs = pack_names(repository) + repository._chunks = None # aborted: the index entries are not persisted + with Repository(location, exclusive=True) as repository: + assert repository.chunks.get(H(1)) is None + repository.put(H(1), chunk) + repository.flush() + assert repository.chunks[H(1)].pack_id == pack_id + assert pack_names(repository) == packs + + +def test_pack_writer_denied_store_over_different_existing_pack(tmp_path, monkeypatch): + # a stored object with the pack's name, but another size, is not the same pack: the store error is raised. + location = os.fspath(tmp_path / "repo") + chunk = fchunk(b"DATA", chunk_id=H(1)) + pack_name = "packs/" + bin_to_hex(store_hash(chunk).digest()) + create_repo_with_index(location) + with Repository(location, exclusive=True) as repository: + repository.store_store(pack_name, chunk[:-1]) # e.g. a truncated pack + monkeypatch.setenv("BORG_REPO_PERMISSIONS", "no-delete") + with pytest.raises(PermissionDenied): + with Repository(location, exclusive=True) as repository: + repository.put(H(1), chunk) + repository.flush() + + def test_get_read_data_false_with_range(tmp_path): # read_data=False with ChunkIndex entries limits the load to each object's boundary. hdr_size = RepoObj.obj_header.size