From c128990cf8395b00c5feb4eb6ea822dc0158f01a Mon Sep 17 00:00:00 2001 From: Mrityunjay Raj Date: Fri, 21 Aug 2026 10:02:33 +0530 Subject: [PATCH] compact: authenticate a gap object before dropping its bytes, #10093 superseded_gap_ranges computed the byte ranges compact drops from object headers as they are: magic, chunk id, meta_size and data_size were all taken on trust. A wrong data_size in a header whose chunk id is indexed elsewhere extended the dropped range past the object into the gap bytes behind it, and those were dropped without ever being looked at. Gaps are where the only copy of a chunk can sit (a backup that crashed before writing its index, or a stale index), so that is not free. Parse the header with PackReader._parse_header, so a header that does not parse or overruns its gap ends the walk over that gap. Read the metadata slot in the same request and require: - validate accepts the header and metadata slot, the same check the repair walk applies (PackReader._validation_problem). - the object's total size equals the index entry's obj_size. That size sets how far the reported range reaches, so the entry is a second source for it, one that does not come from the bytes being examined. Anything else keeps its bytes and the walk continues past it, so a corrupt length field can desync the walk but can no longer drop anything. validate is threaded through delete(), compact_pack() and transform_pack() from the callers that have a key (compact, repo-compress, check --repair). Without it no gap bytes are dropped at all, so "borg debug delete-obj", which opens the repository without a key, stops reclaiming superseded gap bytes. --- docs/internals/packs.rst | 25 ++++++ src/borg/archive.py | 3 +- src/borg/archiver/compact_cmd.py | 6 +- src/borg/archiver/repo_compress_cmd.py | 9 +- src/borg/repository.py | 82 ++++++++++++------ .../testsuite/archiver/compact_cmd_test.py | 70 ++++++++++------ .../archiver/repo_compress_cmd_test.py | 7 +- src/borg/testsuite/repository_test.py | 84 ++++++++++++++++++- 8 files changed, 225 insertions(+), 61 deletions(-) diff --git a/docs/internals/packs.rst b/docs/internals/packs.rst index 21b5d33932..22bfaee32b 100644 --- a/docs/internals/packs.rst +++ b/docs/internals/packs.rst @@ -241,6 +241,31 @@ single blob cannot be removed from a pack in place: all of these paths write a n pack file without it and then delete the old one, so store-level deletion always operates at pack granularity. +Gap bytes +~~~~~~~~~ + +A pack can hold bytes that no chunks index entry covers -- its *gaps*: a copy of a chunk +that was stored again elsewhere, or blobs from a backup that crashed before writing its +index. Rewriting a pack (``compact_pack``, ``transform_pack``) walks the gaps and drops +the blobs among them that are *superseded*: whose chunk id the index maps to a copy at +another location, which by the id/content invariant holds the same plaintext. + +A gap blob is dropped only when both hold: + +* its header and metadata slot authenticate, exactly as in the repair walk above + (``repoobj.object_validator``). ``OBJ_MAGIC`` plus a well-formed header is not evidence + that bytes are a blob: in the ``none-*`` and ``authenticated-*`` modes the payloads are + user content stored as it is, so a backed up file can contain one. +* its total size equals the index entry's ``obj_size``. That size decides how far the + dropped range reaches, so the index entry serves as a second source for it, one that + does not come from the bytes being examined. + +Anything else keeps its bytes, for ``borg check --repair`` to re-index. The walk still +steps by the blob's own size, so a corrupt length field can put it at a wrong offset -- +where nothing passes both checks. Authenticating needs the key, so a caller without one +drops no gap bytes: ``borg debug delete-obj`` opens the repository without a key and +therefore reclaims no superseded gap bytes from the pack it rewrites. + .. _pack-index-namespace: diff --git a/src/borg/archive.py b/src/borg/archive.py index 3bf157fb8d..8bcfb11077 100644 --- a/src/borg/archive.py +++ b/src/borg/archive.py @@ -2357,6 +2357,7 @@ def verify_data(self): if defect_chunks: if self.repair: logger.warning("Found defect chunks, removing them from the repository.") + validate = object_validator(self.repo_objs) for defect_chunk in defect_chunks: # remote repo (ssh): retry might help for strange network / NIC / RAM errors # as the chunk will be retransmitted from remote server. @@ -2377,7 +2378,7 @@ def verify_data(self): # failed twice -> remove this defect chunk. delete rewrites its pack without it, # keeping the other chunks. update_index=False: finish() rebuilds the index from # the rewritten packs anyway, so a per-chunk full index write would be wasted. - self.repository.delete(defect_chunk, update_index=False) + self.repository.delete(defect_chunk, update_index=False, validate=validate) self.chunks_modified = True # drop it from our own index too, so rebuild_archives reports the file it belongs to. del self.chunks[defect_chunk] diff --git a/src/borg/archiver/compact_cmd.py b/src/borg/archiver/compact_cmd.py index 551ba3e7a5..cc6e880088 100644 --- a/src/borg/archiver/compact_cmd.py +++ b/src/borg/archiver/compact_cmd.py @@ -15,6 +15,7 @@ from ..helpers import set_ec, EXIT_ERROR, Error, sig_int, format_file_size, bin_to_hex, hex_to_bin, IntegrityError from ..helpers import ProgressIndicatorPercent from ..manifest import Manifest +from ..repoobj import object_validator from ..repository import Repository from ..logger import create_logger @@ -407,12 +408,15 @@ def compact_packs(self): del self.chunks[id] progress += 1 pi.show(progress) # report after the work, so the final pack lands on 100% + validate = object_validator(self.manifest.repo_objs) for pid in rewrite_packs: if sig_int: break # chunks=self.chunks: the index updates (repoint kept objects, remove dropped ones) # must land in the index that save_chunk_index() persists (#9850). - _, dropped = self.repository.compact_pack(pid, keep_ids=keep[pid], drop_ids=drop[pid], chunks=self.chunks) + _, dropped = self.repository.compact_pack( + pid, keep_ids=keep[pid], drop_ids=drop[pid], chunks=self.chunks, validate=validate + ) freed += dropped # unused indexed objects plus superseded duplicates progress += 1 pi.show(progress) diff --git a/src/borg/archiver/repo_compress_cmd.py b/src/borg/archiver/repo_compress_cmd.py index 273abc787e..c97e2c81a9 100644 --- a/src/borg/archiver/repo_compress_cmd.py +++ b/src/borg/archiver/repo_compress_cmd.py @@ -11,6 +11,7 @@ from ..helpers import format_file_size, hex_to_bin from ..helpers.argparsing import ArgumentParser from ..manifest import Manifest +from ..repoobj import object_validator from ..repository import Repository from ..logger import create_logger @@ -104,6 +105,7 @@ def recompress(self): pi = ProgressIndicatorPercent( total=len(packs), msg="Recompressing %3.1f%%", step=0.1, msgid="repo_compress.recompress" ) + validate = object_validator(self.repo_objs) for i, (pack_id, pack_size) in enumerate(packs): if sig_int: break # stop cleanly at a pack boundary: save the index below, then raise @@ -111,7 +113,12 @@ def recompress(self): # a pack without indexed objects (all-gap) is left for "borg check --repair", see #9868. if ids: new_pack_id, new_size = self.repository.transform_pack( - pack_id, ids, self.transform, chunks=self.chunks, before_change=self.invalidate_stored_index + pack_id, + ids, + self.transform, + chunks=self.chunks, + before_change=self.invalidate_stored_index, + validate=validate, ) if new_pack_id != pack_id: self.packs_rewritten += 1 diff --git a/src/borg/repository.py b/src/borg/repository.py index 7b13fa11f8..257b6c2b23 100644 --- a/src/borg/repository.py +++ b/src/borg/repository.py @@ -48,6 +48,11 @@ # meta_size, which only MAX_DATA_SIZE bounds, from triggering a large read. MAX_VALIDATED_META_SIZE = 64 * 1024 +# how much superseded_gap_ranges reads past an object header, so the metadata slot behind it comes +# in the same request. A metadata slot is at most 112 bytes over all key modes; a larger one costs +# one more read. +GAP_META_READAHEAD = 256 + def repo_lister(repository, *, limit=None): marker = None @@ -545,7 +550,7 @@ def check_pack_objects(pack_hex, obj_ranges, pack_size): ) -def superseded_gap_ranges(reader, chunks, pack_id, obj_ranges, pack_size): +def superseded_gap_ranges(reader, chunks, pack_id, obj_ranges, pack_size, *, validate=None): """Find the superseded duplicates among a pack's gap bytes (bytes no index entry covers). A gap holds a chunk copy stored again elsewhere, or objects from a backup that crashed before @@ -555,10 +560,24 @@ def superseded_gap_ranges(reader, chunks, pack_id, obj_ranges, pack_size): (borg check --repair re-indexes it) or whose entry points back at this offset (its only copy) is not reported. A header that does not parse or overruns its gap ends the walk over that gap. + A duplicate is reported only when both hold: + + - validate accepts its header and metadata slot, authenticating magic, version, chunk id, + meta_size and data_size. + - its total size equals the index entry's obj_size. The total size sets how far the reported + range reaches, so the entry is a second source for it, independent of the object's own bytes. + + Anything else keeps its bytes and the walk continues past it. + obj_ranges: the offset-ordered, validated (obj_offset, obj_size) ranges of the pack's indexed objects; the gaps are the byte ranges between (and after) them. + validate: validate(chunk_id, obj) -> bool over an object's header and metadata slot, see + repoobj.object_validator. None reports nothing. Returns the offset-ordered list of (offset, size) ranges holding superseded duplicates. """ + if validate is None: + return [] + # find the gaps: byte ranges no indexed object covers. gaps = [] # (start, end) of each gap, offset-ordered cursor = 0 @@ -574,17 +593,23 @@ def superseded_gap_ranges(reader, chunks, pack_id, obj_ranges, pack_size): for gstart, gend in gaps: offset = gstart while offset < gend: - hdr_data = reader.read(offset, hdr_size) - if len(hdr_data) < hdr_size: + # the header, and the metadata slot behind it in the same request. + buf = reader.read(offset, min(gend - offset, hdr_size + GAP_META_READAHEAD)) + if len(buf) < hdr_size: break - hdr = RepoObj.ObjHeader(*RepoObj.obj_header.unpack(hdr_data)) - obj_size = hdr_size + hdr.meta_size + hdr.data_size - if hdr.magic != OBJ_MAGIC or offset + obj_size > gend: + # gend, not pack_size: an object reaching past this gap is not one of its objects. + hdr, _ = PackReader._parse_header(buf[:hdr_size], offset, gend) + if hdr is None: break - if hdr.chunk_id in chunks: - entry = chunks[hdr.chunk_id] - if entry.pack_id != pack_id or entry.obj_offset != offset: + obj_size = hdr_size + hdr.meta_size + hdr.data_size + entry = chunks.get(hdr.chunk_id) + superseded = entry is not None and (entry.pack_id != pack_id or entry.obj_offset != offset) + if superseded and obj_size == entry.obj_size: + # buf starts at offset, so the readahead above usually already holds the slot. + if reader._validation_problem(hdr, offset, buf, offset, validate) is None: drop_ranges.append((offset, obj_size)) + # the walk steps by obj_size before anything authenticates it: a wrong one lands the + # walk at a wrong offset, where the two checks above apply again. offset += obj_size return drop_ranges @@ -1525,12 +1550,15 @@ def put(self, id, data): # PackWriter shares this repository's index, so add() triggers the lazy build itself. return self._pack_writer.add(id, data) - def delete(self, id, *, update_index=True): + def delete(self, id, *, update_index=True, validate=None): """Delete a single repo object by rewriting its pack without it (via compact_pack). With update_index=True the full chunk index is written back so the next borg process sees the deletion; callers that rebuild the index themselves (check --repair) pass update_index=False to skip the per-object index rewrite. + + validate: authenticates a gap object before its bytes are dropped, see + superseded_gap_ranges. None drops no gap bytes. """ self._lock_refresh() entry = self.chunks.get(id) @@ -1540,7 +1568,7 @@ def delete(self, id, *, update_index=True): # keep every object the chunk index lists for this pack, except the one being deleted. keep_ids = {cid for cid, e in self.chunks.iteritems() if e.pack_id == pack_id} keep_ids.discard(id) - self.compact_pack(pack_id, keep_ids=keep_ids, drop_ids={id}) + self.compact_pack(pack_id, keep_ids=keep_ids, drop_ids={id}, validate=validate) if update_index: # close() only persists new entries incrementally, so write the full index here to record # the removal for the next borg process. @@ -1548,22 +1576,24 @@ def delete(self, id, *, update_index=True): write_chunkindex_to_repo(self, self.chunks, incremental=False, force_write=True, delete_other=True) - def compact_pack(self, pack_id, *, keep_ids: set, drop_ids: set, chunks=None): + def compact_pack(self, pack_id, *, keep_ids: set, drop_ids: set, chunks=None, validate=None): """Rewrite pack , keeping and dropping , then delete the old pack. keep_ids: chunk ids in this pack to copy into the new pack. drop_ids: chunk ids in this pack to discard. Must not overlap keep_ids. chunks: the ChunkIndex to look up the objects' pack locations in and to apply the index updates to. Must be the index keep_ids and drop_ids were derived from. Default: self.chunks. + validate: authenticates a gap object before its bytes are dropped, see + superseded_gap_ranges. None drops no gap bytes. Default: None. Together, keep_ids and drop_ids must cover every object the chunk index lists for this pack; an unlisted indexed object would keep its bytes in the new pack but its index entry would go stale when the old pack is deleted. Bytes that no index entry covers appear as gaps between the - listed objects: a gap object whose chunk id is in the index is a superseded duplicate (its - authoritative copy is elsewhere) and is dropped; a gap object whose id is not in the index is - copied into the new pack unchanged, to be handled by "borg check --repair". An overlap between - listed objects, or an object claiming to end past the pack file, means index corruption and - raises IntegrityError. + listed objects: a gap object that authenticates as a superseded duplicate (its authoritative + copy is elsewhere) is dropped, every other gap byte is copied into the new pack unchanged, to + be handled by "borg check --repair" - see superseded_gap_ranges. An overlap between listed + objects, or an object claiming to end past the pack file, means index corruption and raises + IntegrityError. The new pack is the old pack minus the dropped objects, built via store.defrag; kept objects are repointed in the chunk index and dropped objects' chunk index entries are removed. @@ -1605,7 +1635,7 @@ def compact_pack(self, pack_id, *, keep_ids: set, drop_ids: set, chunks=None): # toward the rewrite threshold and a wholly superseded orphan pack can be dropped outright. drop_ranges = [(offset, size) for offset, _, size, keep in located if not keep] reader = PackReader(store=self.store, pack_id=pack_id) - drop_ranges += superseded_gap_ranges(reader, chunks, pack_id, obj_ranges, pack_size) + drop_ranges += superseded_gap_ranges(reader, chunks, pack_id, obj_ranges, pack_size, validate=validate) drop_ranges.sort() dropped_bytes = sum(size for _, size in drop_ranges) # on-disk bytes this rewrite frees, for --stats @@ -1759,7 +1789,7 @@ def merge_packs(self, pack_ids, *, chunks=None, max_size=None): pi.show(increase=1) pi.finish() - def transform_pack(self, pack_id, ids, transform, *, chunks=None, before_change=None): + def transform_pack(self, pack_id, ids, transform, *, chunks=None, before_change=None, validate=None): """Rewrite pack , passing each indexed object's bytes through . ids: the chunk ids of this pack's objects. Must cover every object the chunk index lists @@ -1773,13 +1803,15 @@ def transform_pack(self, pack_id, ids, transform, *, chunks=None, before_change= updates to. Must be the index was derived from. Default: self.chunks. before_change: called once, just before the first store modification; use it to invalidate stored chunk indexes for crash safety (see #9748). Not called when the pack is kept. + validate: authenticates a gap object before its bytes are dropped, see + superseded_gap_ranges. None drops no gap bytes. Default: None. The whole pack file is loaded into memory (bounded by the pack size limit). Gap bytes - (bytes no index entry covers) are handled like in compact_pack: an object superseded by a - copy stored elsewhere is dropped, all other unindexed bytes are copied into the new pack - unchanged, to be handled by "borg check --repair". An overlap between indexed objects, or - an object claiming to end past the pack file, means index corruption and raises - IntegrityError, before anything is written. + (bytes no index entry covers) are handled like in compact_pack: an object that authenticates + as superseded by a copy stored elsewhere is dropped, all other unindexed bytes are copied + into the new pack unchanged, to be handled by "borg check --repair". An overlap between + indexed objects, or an object claiming to end past the pack file, means index corruption and + raises IntegrityError, before anything is written. If every object is kept and no gap bytes are dropped, the store and the chunk index are not touched at all. Otherwise the new pack (named sha256 of its content) is stored, the indexed @@ -1811,7 +1843,7 @@ def transform_pack(self, pack_id, ids, transform, *, chunks=None, before_change= located.sort() obj_ranges = [(offset, size) for offset, _, size in located] check_pack_objects(pack_hex, obj_ranges, pack_size) - drop_ranges = superseded_gap_ranges(reader, chunks, pack_id, obj_ranges, pack_size) + drop_ranges = superseded_gap_ranges(reader, chunks, pack_id, obj_ranges, pack_size, validate=validate) # assemble the new pack in offset order: transformed objects, dropped ranges skipped, all # other bytes copied verbatim. the two range lists never overlap (drops lie in gaps), so a diff --git a/src/borg/testsuite/archiver/compact_cmd_test.py b/src/borg/testsuite/archiver/compact_cmd_test.py index f7f6efb93a..c40cc8835f 100644 --- a/src/borg/testsuite/archiver/compact_cmd_test.py +++ b/src/borg/testsuite/archiver/compact_cmd_test.py @@ -1,11 +1,14 @@ import os from pathlib import Path +from types import SimpleNamespace import pytest from ...constants import * # NOQA from ...helpers import get_cache_dir, bin_to_hex, sig_int, Error from ...hashindex import ChunkIndex +from ...crypto.key import ChecksumKey +from ...repoobj import RepoObj from ...repository import Repository from ...cache import files_cache_name, discover_files_cache_names, list_chunkindex_hashes from ...cache import delete_chunkindex_from_repo, write_chunkindex_to_repo @@ -19,6 +22,14 @@ pytest_generate_tests = lambda metafunc: generate_archiver_tests(metafunc, kinds="local,remote,binary") # NOQA +def gc_manifest(repository): + """Manifest stand-in: repo_objs is the only attribute ArchiveGarbageCollector uses. + + ChecksumKey is the "none-sha256" mode, which formats and authenticates objects without key material. + """ + return SimpleNamespace(repo_objs=RepoObj(ChecksumKey(repository))) + + @pytest.mark.parametrize("stats", (True, False)) def test_compact_empty_repository(archivers, request, stats): archiver = request.getfixturevalue(archivers) @@ -228,7 +239,7 @@ def test_compact_packs_respects_threshold(tmp_path): flags = ChunkIndex.F_USED if H(i) in used else ChunkIndex.F_NONE repository.chunks[H(i)] = entry._replace(flags=flags) - gc = ArchiveGarbageCollector(repository, manifest=None, stats=False, threshold=40) + gc = ArchiveGarbageCollector(repository, gc_manifest(repository), stats=False, threshold=40) gc.chunks = repository.chunks gc.compact_packs() @@ -253,42 +264,47 @@ def test_compact_superseded_duplicate(tmp_path): location = os.fspath(tmp_path / "repo") with Repository(location, exclusive=True, create=True) as repository: + manifest = gc_manifest(repository) + repo_objs = manifest.repo_objs + # formatted objects: dropping the duplicate's bytes needs its metadata slot to authenticate. + w, x, y = b"WWWW", b"XXXX", b"YYYY" + w_id, x_id, y_id = (repo_objs.id_hash(data) for data in (w, x, y)) repository._pack_writer.max_count = 4 # one flush() -> one pack # pack A: three objects W, X, Y (X will be the one later superseded by a copy in pack B) - for cid, data in [(H(0), b"WWWW"), (H(1), b"XXXX"), (H(2), b"YYYY")]: - repository.put(cid, fchunk(data, chunk_id=cid)) + for cid, data in [(w_id, w), (x_id, x), (y_id, y)]: + repository.put(cid, repo_objs.format(cid, {}, data, ro_type=ROBJ_FILE_STREAM)) repository.flush() - pack_a = repository.chunks[H(0)].pack_id + pack_a = repository.chunks[w_id].pack_id pack_a_size = next(i.size for i in repository.store_list("packs") if i.name == bin_to_hex(pack_a)) - x_size = repository.chunks[H(1)].obj_size # X's copy in pack A becomes a superseded gap - y_size = repository.chunks[H(2)].obj_size + x_size = repository.chunks[x_id].obj_size # X's copy in pack A becomes a superseded gap + y_size = repository.chunks[y_id].obj_size # pack B: a second copy of X only, in its own pack (as a concurrent writer would have produced). - repository.put(H(1), fchunk(b"XXXX", chunk_id=H(1))) + repository.put(x_id, repo_objs.format(x_id, {}, x, ro_type=ROBJ_FILE_STREAM)) repository.flush() - pack_b = repository.chunks[H(1)].pack_id + pack_b = repository.chunks[x_id].pack_id assert pack_b != pack_a # after the (simulated) fragment merge, the index points X at pack B; pack A's X bytes are now # a superseded, unindexed span. put() already repointed the index to pack B, so nothing to do. # mark usage: W and X used, Y unused. pack A is now mixed (W used, X superseded gap, Y unused). - used = {H(0), H(1)} - for i in range(3): - entry = repository.chunks[H(i)] - flags = ChunkIndex.F_USED if H(i) in used else ChunkIndex.F_NONE - repository.chunks[H(i)] = entry._replace(flags=flags) + used = {w_id, x_id} + for cid in (w_id, x_id, y_id): + entry = repository.chunks[cid] + flags = ChunkIndex.F_USED if cid in used else ChunkIndex.F_NONE + repository.chunks[cid] = entry._replace(flags=flags) - gc = ArchiveGarbageCollector(repository, manifest=None, stats=False, threshold=10) + gc = ArchiveGarbageCollector(repository, manifest, stats=False, threshold=10) gc.chunks = repository.chunks gc.compact_packs() # W still readable; X still readable from pack B; Y (the unused indexed object) dropped. - assert pdchunk(repository.get(H(0))) == b"WWWW" - assert pdchunk(repository.get(H(1))) == b"XXXX" - assert repository.get(H(2), raise_missing=False) is None + assert repo_objs.parse(w_id, repository.get(w_id), ro_type=ROBJ_FILE_STREAM)[1] == w + assert repo_objs.parse(x_id, repository.get(x_id), ro_type=ROBJ_FILE_STREAM)[1] == x + assert repository.get(y_id, raise_missing=False) is None # pack A rewritten, shrunk by Y's bytes (unused indexed) plus X's superseded gap: only W remains. assert bin_to_hex(pack_a) not in [info.name for info in repository.store_list("packs")] - new_pack = repository.chunks[H(0)].pack_id + new_pack = repository.chunks[w_id].pack_id new_size = next(i.size for i in repository.store_list("packs") if i.name == bin_to_hex(new_pack)) assert new_size == pack_a_size - y_size - x_size @@ -312,7 +328,7 @@ def test_compact_keeps_orphan_pack(tmp_path): repository.store_store(orphan_key, b"orphan pack bytes") assert "ab" * 32 in [info.name for info in repository.store_list("packs")] - gc = ArchiveGarbageCollector(repository, manifest=None, stats=False, threshold=10) + gc = ArchiveGarbageCollector(repository, gc_manifest(repository), stats=False, threshold=10) gc.chunks = repository.chunks gc.compact_packs() @@ -342,7 +358,7 @@ def test_compact_keeps_unindexed_waste(tmp_path): # ... but H(1)'s big object becomes an unindexed superseded span (well over threshold if counted). del repository.chunks[H(1)] - gc = ArchiveGarbageCollector(repository, manifest=None, stats=False, threshold=10) + gc = ArchiveGarbageCollector(repository, gc_manifest(repository), stats=False, threshold=10) gc.chunks = repository.chunks gc.compact_packs() @@ -377,7 +393,7 @@ def test_compact_reclaims_indexed_waste_only(tmp_path): repository.chunks[H(2)] = repository.chunks[H(2)]._replace(flags=ChunkIndex.F_USED) del repository.chunks[H(3)] # its bytes remain in unindexed_pack as unindexed data - gc = ArchiveGarbageCollector(repository, manifest=None, stats=False, threshold=10) + gc = ArchiveGarbageCollector(repository, gc_manifest(repository), stats=False, threshold=10) gc.chunks = repository.chunks gc.compact_packs() @@ -439,7 +455,7 @@ def test_compact_keeps_stale_index_entries(tmp_path): repository.chunks[H(0)] = repository.chunks[H(0)]._replace(flags=ChunkIndex.F_USED) repository.store_delete("packs/" + bin_to_hex(gone_pack)) # delete the pack file the index still references - gc = ArchiveGarbageCollector(repository, manifest=None, stats=False, threshold=10) + gc = ArchiveGarbageCollector(repository, gc_manifest(repository), stats=False, threshold=10) gc.chunks = repository.chunks gc.compact_packs() @@ -460,7 +476,7 @@ def test_compact_skips_oversized_index_entry(tmp_path): entry = repository.chunks[H(0)] repository.chunks[H(0)] = entry._replace(flags=ChunkIndex.F_USED, obj_size=entry.obj_size + 10000) - gc = ArchiveGarbageCollector(repository, manifest=None, stats=False, threshold=10) + gc = ArchiveGarbageCollector(repository, gc_manifest(repository), stats=False, threshold=10) gc.chunks = repository.chunks gc.compact_packs() @@ -492,7 +508,7 @@ def test_compact_packs_merges_tiny_packs(tmp_path, monkeypatch): total_bytes = sum(repository.store.info("packs/" + name).size for name in packs_before) assert total_bytes >= repository.pack_max_size # combined size crosses the merge threshold - gc = ArchiveGarbageCollector(repository, manifest=None, stats=False, threshold=10) + gc = ArchiveGarbageCollector(repository, gc_manifest(repository), stats=False, threshold=10) gc.chunks = repository.chunks gc.compact_packs() assert gc.store_changed is True # the merge changed the store @@ -510,7 +526,7 @@ def test_compact_packs_merges_tiny_packs(tmp_path, monkeypatch): # a merged full-size pack is no longer tiny (the tiny limit is pack_max_size // 2 here), so a # second compact finds nothing to merge and leaves the store unchanged. - gc2 = ArchiveGarbageCollector(repository, manifest=None, stats=False, threshold=10) + gc2 = ArchiveGarbageCollector(repository, gc_manifest(repository), stats=False, threshold=10) gc2.chunks = repository.chunks gc2.compact_packs() assert gc2.store_changed is False @@ -537,7 +553,7 @@ def test_compact_packs_below_merge_size_gate_leaves_tiny_packs(tmp_path, monkeyp packs_before = {info.name for info in repository.store_list("packs")} assert len(packs_before) == 3 - gc = ArchiveGarbageCollector(repository, manifest=None, stats=False, threshold=10) + gc = ArchiveGarbageCollector(repository, gc_manifest(repository), stats=False, threshold=10) gc.chunks = repository.chunks gc.compact_packs() assert gc.store_changed is False # combined tiny bytes stay far below one full pack: leave them alone @@ -566,7 +582,7 @@ def test_compact_packs_below_all_packs_gate_changes_nothing(tmp_path): packs_before = {info.name for info in repository.store_list("packs")} assert len(packs_before) == 2 - gc = ArchiveGarbageCollector(repository, manifest=None, stats=False, threshold=10) + gc = ArchiveGarbageCollector(repository, gc_manifest(repository), stats=False, threshold=10) gc.chunks = repository.chunks gc.compact_packs() assert gc.store_changed is False # below the all-packs gate: nothing was touched diff --git a/src/borg/testsuite/archiver/repo_compress_cmd_test.py b/src/borg/testsuite/archiver/repo_compress_cmd_test.py index 739c799202..162197130a 100644 --- a/src/borg/testsuite/archiver/repo_compress_cmd_test.py +++ b/src/borg/testsuite/archiver/repo_compress_cmd_test.py @@ -12,7 +12,7 @@ from ...archiver.repo_compress_cmd import PackRecompressor from . import create_regular_file, cmd, RK_ENCRYPTION -from ..repository_test import H, fchunk, pdchunk +from ..repository_test import H, accept_all, fchunk, pdchunk def test_repo_compress(archiver): @@ -303,7 +303,10 @@ def test_transform_pack_drops_superseded_gap(tmp_path): assert pack_b != pack_a w_new = fchunk(b"W" * 100, chunk_id=H(0)) - new_pack_id, new_size = repository.transform_pack(pack_a, [H(0)], transform_via({H(0): w_new})) + # fchunk objects have no authenticating metadata slot, so accept_all stands in for validate. + new_pack_id, new_size = repository.transform_pack( + pack_a, [H(0)], transform_via({H(0): w_new}), validate=accept_all + ) assert new_pack_id != pack_a assert new_size == len(w_new) # only W remains, X's superseded bytes were dropped assert pdchunk(repository.get(H(0))) == b"W" * 100 diff --git a/src/borg/testsuite/repository_test.py b/src/borg/testsuite/repository_test.py index 7fa3c4ab9f..95b897e2a0 100644 --- a/src/borg/testsuite/repository_test.py +++ b/src/borg/testsuite/repository_test.py @@ -16,9 +16,9 @@ from ..constants import MAX_CLOCK_SKEW, ROBJ_FILE_STREAM from ..crypto.key import CHPOKey, ChecksumKey from ..helpers import IntegrityError, Location, bin_to_hex -from ..hashindex import ChunkIndex +from ..hashindex import ChunkIndex, ChunkIndexEntry from ..repository import Repository, MAX_DATA_SIZE, MAX_VALIDATED_META_SIZE, propagate_rsh, rest_serve_command -from ..repository import PackWriter, PackReader, PackTracker +from ..repository import PackWriter, PackReader, PackTracker, superseded_gap_ranges from ..repoobj import RepoObj, OBJ_MAGIC, OBJ_VERSION, object_validator from .hashindex_test import H from .repoobj_test import DATA_SIZE_OFFSET, META_SIZE_OFFSET @@ -499,7 +499,9 @@ def test_compact_pack_drops_superseded_gap(repo_fixtures, request): old_pack_id = repository.chunks[H(0)].pack_id repository.chunks[H(1)] = repository.chunks[H(1)]._replace(pack_id=H(9)) # authoritative copy elsewhere - new_pack_id, dropped = repository.compact_pack(old_pack_id, keep_ids={H(0), H(2)}, drop_ids=set()) + new_pack_id, dropped = repository.compact_pack( + old_pack_id, keep_ids={H(0), H(2)}, drop_ids=set(), validate=accept_all + ) assert new_pack_id is not None and new_pack_id != old_pack_id assert dropped == len(chunk1) # the superseded gap's bytes are counted as freed @@ -523,7 +525,9 @@ def test_compact_pack_keeps_self_referencing_gap(repo_fixtures, request): with repository: old_pack_id = repository.chunks[H(0)].pack_id - new_pack_id, dropped = repository.compact_pack(old_pack_id, keep_ids={H(0), H(2)}, drop_ids=set()) + new_pack_id, dropped = repository.compact_pack( + old_pack_id, keep_ids={H(0), H(2)}, drop_ids=set(), validate=accept_all + ) assert new_pack_id == old_pack_id # nothing dropped, defrag reproduced the same pack assert dropped == 0 # the self-referencing gap is kept, nothing freed @@ -2254,3 +2258,75 @@ def test_pack_reader_in_memory_read_returns_view(): assert bytes(view) == obj2 pack[len(obj1)] ^= 0xFF # a write to pack_contents is visible through the view assert view[0] == obj2[0] ^ 0xFF + + +THIS_PACK = H(98) # the pack whose gaps are walked +OTHER_PACK = H(99) # the pack the index points a superseded duplicate's authoritative copy at + + +def gap_pack(repo_objs, datas): + """Build a pack of repo objects, plus a chunks index that supersedes every one of them. + + datas: each object's plaintext, in pack order. + Returns (objects, chunks): chunks maps each object's id to an entry in OTHER_PACK of the same + size, so superseded_gap_ranges reports an object exactly when it passes both of its checks. + """ + objs = [repo_objs.format(repo_objs.id_hash(data), {}, data, ro_type=ROBJ_FILE_STREAM) for data in datas] + chunks = {repo_objs.id_hash(data): ChunkIndexEntry(0, 0, OTHER_PACK, 0, len(obj)) for data, obj in zip(datas, objs)} + return objs, chunks + + +def gap_ranges(pack, chunks, validate): + # the whole pack is one gap: no indexed object of THIS_PACK covers any of it. + reader = PackReader(pack_contents=pack) + return superseded_gap_ranges(reader, chunks, THIS_PACK, [], len(pack), validate=validate) + + +def test_superseded_gap_ranges_reports_an_authenticated_duplicate(tmp_path): + repo_objs = aead_repo_objs(tmp_path) + (obj,), chunks = gap_pack(repo_objs, [b"superseded"]) + + assert gap_ranges(obj, chunks, object_validator(repo_objs)) == [(0, len(obj))] + assert gap_ranges(obj, chunks, None) == [] # no validator, nothing to report + + +def test_superseded_gap_ranges_rejects_a_forged_chunk_id(tmp_path): + # The chunk id sits in cleartext in the header. Overwriting it with the id of an indexed chunk + # of the same size passes the size check, so only the metadata slot's tag rules the object out. + repo_objs = aead_repo_objs(tmp_path) + (obj,), chunks = gap_pack(repo_objs, [b"superseded"]) + victim = repo_objs.id_hash(b"a chunk stored elsewhere") + chunks[victim] = ChunkIndexEntry(0, 0, OTHER_PACK, 0, len(obj)) + forged = bytearray(obj) + forged[len(OBJ_MAGIC) + 1 : len(OBJ_MAGIC) + 1 + 32] = victim # the header's chunk_id field + + assert gap_ranges(bytes(forged), chunks, object_validator(repo_objs)) == [] + + +def test_superseded_gap_ranges_rejects_a_size_the_index_contradicts(tmp_path): + # An inflated data_size would stretch the reported range over the bytes behind the object. The + # index entry's obj_size rules it out on its own, so accept_all stands in for validate here; + # object_validator rejects it as well, over the csize the authenticated metadata records. + repo_objs = aead_repo_objs(tmp_path) + (obj, behind), chunks = gap_pack(repo_objs, [b"superseded", b"innocent bystander"]) + hdr_size = RepoObj.obj_header.size + inflated = bytearray(obj + behind) + data_size_at = hdr_size - 4 + (data_size,) = struct.unpack("