From ca00a1255a8f5ff92e97fe2f64c3495a5846c0f9 Mon Sep 17 00:00:00 2001 From: Mrityunjay Raj Date: Wed, 30 Sep 2026 00:30:27 +0530 Subject: [PATCH 1/4] compact: suggest a full check --repair for unindexed pack bytes, fixes #10429 --- src/borg/archiver/check_cmd.py | 4 ++-- src/borg/archiver/compact_cmd.py | 24 +++++++++++++------ src/borg/repository.py | 6 ++--- .../testsuite/archiver/compact_cmd_test.py | 9 ++++--- src/borg/testsuite/repository_test.py | 4 ++-- 5 files changed, 30 insertions(+), 17 deletions(-) diff --git a/src/borg/archiver/check_cmd.py b/src/borg/archiver/check_cmd.py index 50364adf45..2447f64073 100644 --- a/src/borg/archiver/check_cmd.py +++ b/src/borg/archiver/check_cmd.py @@ -90,7 +90,7 @@ def do_check(self, args, repository): raise CommandError("--repair does not allow --max-duration argument.") if args.repair and args.max_age is not None: # repair verifies every pack; reusing recorded results during repair needs repository - # repair (refs #8572). + # repair (refs #10026). raise CommandError("--repair does not allow the --max-age option.") if args.archives_only and args.max_age is not None: # --max-age only affects the repository check; --archives-only skips it. @@ -301,7 +301,7 @@ def build_parser_check(self, subparsers, common_parser, mid_common_parser): index from the packs if the index is corrupt, provided every pack matches its store hash. If any pack fails its store hash, the repository check leaves the index and the packs untouched and reports it; salvaging the intact objects of - such a pack is not implemented yet (refs #8572). The rebuild authenticates + such a pack is not implemented yet (refs #10026). The rebuild authenticates each object's header and metadata with the key, leaves an object that fails this out of the index and reports it as an error. Repair mode also removes the index entries of the chunks stored in missing packs (packs the index references, diff --git a/src/borg/archiver/compact_cmd.py b/src/borg/archiver/compact_cmd.py index f0c096bfc0..383ef8fdea 100644 --- a/src/borg/archiver/compact_cmd.py +++ b/src/borg/archiver/compact_cmd.py @@ -321,14 +321,21 @@ def compact_packs(self): logger.error(f"{stale_used} of them are still in use: repository data is missing!") set_ec(EXIT_ERROR) - # bytes no index entry covers. compact_pack reclaims the redundant duplicates among them while - # rewriting a pack; reclaiming the rest is tracked in #8572. + # unindexed bytes: pack bytes no index entry covers, e.g. the objects of an interrupted borg create. + # superseded duplicates are unindexed objects whose chunk id the index maps to another location. + # compact_pack drops those when it rewrites a pack and keeps the other unindexed objects, as + # "borg check --repair" can recover them (#9868). A full check --repair (--repository-only rebuilds + # the index only if it is corrupt) indexes one copy per chunk id, so objects whose chunk id had no + # index entry become reclaimable once unused, and the other copies stay superseded duplicates. + # TODO(#10471): count superseded duplicates as reclaimable, so compact rewrites a pack that holds only + # used objects and superseded duplicates, e.g. after a crashed borg create was re-run. unindexed = sum(total - pack_indexed[pid] for pid, total in pack_total.items() if total > pack_indexed[pid]) if unindexed: logger.info( - f"{format_file_size(unindexed)} in pack files is not covered by the index; " - "redundant copies are reclaimed on pack rewrite, reclaiming the rest is tracked in " - "https://github.com/borgbackup/borg/issues/8572." + f"{format_file_size(unindexed)} in pack files is not covered by the index. " + '"borg check --repair" (without --repository-only) indexes the objects whose chunk id has no ' + 'index entry, so "borg compact" reclaims them once unused. Copies of chunks indexed elsewhere ' + "are only reclaimed when compact rewrites their pack." ) # packs recorded corrupt in PackTracker that are still in the store @@ -502,8 +509,11 @@ def build_parser_compact(self, subparsers, common_parser, mid_common_parser): ``borg compact`` reclaims objects the chunk index knows about, plus redundant copies of indexed chunks (e.g. written by concurrent backups) that it finds while rewriting a pack. Other bytes no index entry covers, such as packs left behind by a backup that crashed - before recording its objects, are re-indexed by ``borg check --repair`` and reclaimed by - the next ``borg compact``. + before recording its objects, are kept. ``borg check --repair`` (without + ``--repository-only``) indexes one copy per chunk id, so ``borg compact`` reclaims the + objects whose chunk id had no index entry once they are unused. The remaining copies of + chunks indexed elsewhere (e.g. when the crashed backup was re-run) stay unindexed and are + only reclaimed when ``borg compact`` rewrites their pack to reclaim unused indexed objects. ``borg compact`` does not rewrite or merge packs that ``borg check`` recorded as corrupt and warns about them. ``borg check --repair --verify-data`` deletes the corrupt chunks by diff --git a/src/borg/repository.py b/src/borg/repository.py index 2745a9d9f1..c0c1908f9f 100644 --- a/src/borg/repository.py +++ b/src/borg/repository.py @@ -1489,13 +1489,13 @@ def check(self, repair=False, max_duration=0, max_age=0, repo_only=False, valida ArchiveChecker.finish. Packs are verified by the store hash, which is content-addressing rather than a MAC, so that check detects accidental corruption but not tampering; the rebuild therefore checks every object with validate, see below, refs #9901, #10026. If any pack is corrupt the index - is not rebuilt, refs #8572, #10026. Pack ids found corrupt are kept in cache/checked-packs, + is not rebuilt, refs #10026. Pack ids found corrupt are kept in cache/checked-packs, refs #9696. That object is stored in the key's envelope, too, so check() needs the key (see set_key). A pack recorded corrupt fails the check, also on a partial run that stops before re-reaching it. The record clears at the check that finds the pack intact again or gone (removed by - compact; TODO: also when repair salvages and drops it, refs #8572); prune() does this from packs/. + compact; TODO: also when repair salvages and drops it, refs #10026); prune() does this from packs/. It also reports missing packs (refs #9898): pack ids the chunk index references but that are absent from packs/. The index is read from its fragments only and its referenced pack ids are @@ -1790,7 +1790,7 @@ def note_drop(): if repo_only: logger.error( f"{done} {mode} repository check, corrupt pack(s) found{so_far}; repairing a repository " - "with corrupt packs is not implemented yet (refs #8572)." + "with corrupt packs is not implemented yet (refs #10026)." ) else: # a full check's archives phase reads archive/item metadata (and file content with diff --git a/src/borg/testsuite/archiver/compact_cmd_test.py b/src/borg/testsuite/archiver/compact_cmd_test.py index 9f177a4290..527f7e4a1f 100644 --- a/src/borg/testsuite/archiver/compact_cmd_test.py +++ b/src/borg/testsuite/archiver/compact_cmd_test.py @@ -373,10 +373,10 @@ def test_compact_keeps_unindexed_waste(tmp_path): assert pdchunk(repository.get(H(2))) == b"CCCC" -def test_compact_reclaims_indexed_waste_only(tmp_path): +def test_compact_reclaims_indexed_waste_only(tmp_path, caplog): # compact reclaims a pack's indexed-but-unused bytes, but leaves alone a pack whose only waste is # unindexed (bytes no index entry covers): those may be live data "borg check --repair" can - # recover (#9868). + # recover (#9868). It logs what a full "borg check --repair" does about them (#10429). from ...archiver.compact_cmd import ArchiveGarbageCollector location = os.fspath(tmp_path / "repo") @@ -400,8 +400,11 @@ def test_compact_reclaims_indexed_waste_only(tmp_path): gc = ArchiveGarbageCollector(repository, gc_manifest(repository), stats=False, threshold=10) gc.chunks = repository.chunks - gc.compact_packs() + with caplog.at_level(logging.INFO): + gc.compact_packs() + assert "in pack files is not covered by the index" in caplog.text + assert '"borg check --repair" (without --repository-only) indexes the objects' in caplog.text pack_names = [info.name for info in repository.store_list("packs")] # indexed waste -> compacted, kept object still readable assert bin_to_hex(waste_pack) not in pack_names diff --git a/src/borg/testsuite/repository_test.py b/src/borg/testsuite/repository_test.py index 55019ab20c..79599575e5 100644 --- a/src/borg/testsuite/repository_test.py +++ b/src/borg/testsuite/repository_test.py @@ -1874,7 +1874,7 @@ def validate(chunk_id, obj): def test_check_repair_refuses_when_pack_corrupt(tmp_path): # A repair that finds any corrupt pack leaves the index and the pack untouched (no lossy rebuild, - # nothing dropped) and fails on a repository-only run, refs #8572, #10026. + # nothing dropped) and fails on a repository-only run, refs #10026. location = os.fspath(tmp_path / "repo") with Repository(location, exclusive=True, create=True) as repository: repository.put(H(1), fchunk(b"GOOD-CHUNK", chunk_id=H(1))) @@ -1995,7 +1995,7 @@ def delete_pack(repository, pack_id): def test_check_repair_removes_missing_pack_entries(tmp_path, caplog, repo_only): # a repair removes the index entries of the chunks in a missing pack and stores the index. It fails # a repository-only run, as the chunks are lost; a full check defers them to the archives phase - # (refs #9898, #8572). + # (refs #9898). location = os.fspath(tmp_path / "repo") pack_ids = create_repo_one_pack_per_chunk(location) with Repository(location, exclusive=True) as repository: From 8d0ab1b3e14fce9049ff2174bb9b01247f169aba Mon Sep 17 00:00:00 2001 From: Mrityunjay Raj Date: Mon, 28 Sep 2026 03:33:53 +0530 Subject: [PATCH 2/4] check: a full --repair rebuilds a corrupt index once, in the archives phase, fixes #10434 --- src/borg/archive.py | 3 +- src/borg/archiver/check_cmd.py | 24 ++-- src/borg/repository.py | 74 +++++++----- src/borg/testsuite/archiver/check_cmd_test.py | 109 +++++++++++++++--- src/borg/testsuite/repository_test.py | 66 ++++++++--- 5 files changed, 209 insertions(+), 67 deletions(-) diff --git a/src/borg/archive.py b/src/borg/archive.py index 541e69e2d2..23b5245616 100644 --- a/src/borg/archive.py +++ b/src/borg/archive.py @@ -2312,7 +2312,8 @@ def check( # each index object's store hash, and the index is the authoritative record of which chunks exist, # so we do not rebuild it from the packs (reading every pack is far too slow for a routine check). # --repair does rebuild from the packs (slow_rebuild=repair), working from the real packs so it - # can detect and fix archives that reference chunks whose pack has gone missing. + # can detect and fix archives that reference chunks whose pack has gone missing. It also replaces a + # corrupt index, see Repository.check. # The rebuild validates every object header it walks, because a corrupt data_size parses fine # and points the walk into the middle of the pack. That costs one metadata slot read and one # decryption per object and it needs the key, so read the key here if we do not have it yet. diff --git a/src/borg/archiver/check_cmd.py b/src/borg/archiver/check_cmd.py index 2447f64073..f0d7bb1f47 100644 --- a/src/borg/archiver/check_cmd.py +++ b/src/borg/archiver/check_cmd.py @@ -119,7 +119,7 @@ def do_check(self, args, repository): max_duration=args.max_duration, max_age=max_age, repo_only=args.repo_only, - # the object validator for the index rebuild, which only a repair does. + # validates each object the index rebuild of a --repository-only repair walks. validate=object_validator(RepoObj(key)), ): set_ec(EXIT_WARNING) @@ -297,16 +297,18 @@ def build_parser_check(self, subparsers, common_parser, mid_common_parser): In practice, repair mode hooks into both the repository and archive checks: - 1. When checking the repository's consistency, repair mode rebuilds the repository - index from the packs if the index is corrupt, provided every pack matches its - store hash. If any pack fails its store hash, the repository check leaves the - index and the packs untouched and reports it; salvaging the intact objects of - such a pack is not implemented yet (refs #10026). The rebuild authenticates - each object's header and metadata with the key, leaves an object that fails - this out of the index and reports it as an error. Repair mode also removes the - index entries of the chunks stored in missing packs (packs the index references, - but that are absent from the repository). Only a full ``borg check --repair`` - repairs the archives that reference these chunks, ``--repository-only`` does not. + 1. When checking the repository's consistency, repair mode verifies every pack if + the index is corrupt. A full ``borg check --repair`` then rebuilds the index from + the packs in the archive check (which does so on every ``--repair`` run). With + ``--repository-only``, the repository check rebuilds it, provided every pack + matches its store hash. If any pack fails its store hash, it leaves the index and + the packs untouched and reports it; salvaging the intact objects of such a pack + is not implemented yet (refs #10026). Either rebuild authenticates each object's + header and metadata with the key, leaves an object that fails this out of the + index and reports it as an error. Repair mode also removes the index entries of + the chunks stored in missing packs (packs the index references, but that are + absent from the repository). Only a full ``borg check --repair`` repairs the + archives that reference these chunks, ``--repository-only`` does not. A missing or corrupt repository defaults object is replaced by empty defaults, so the repository can be used again; the commands then use the built-in defaults. diff --git a/src/borg/repository.py b/src/borg/repository.py index c0c1908f9f..0da3cca4cb 100644 --- a/src/borg/repository.py +++ b/src/borg/repository.py @@ -1469,7 +1469,7 @@ def info(self): info = dict(id=self.id, version=self.version) return info - def check(self, repair=False, max_duration=0, max_age=0, repo_only=False, validate=None): + def check(self, repair=False, max_duration=0, max_age=0, repo_only=None, validate=None): """Check repository consistency. packs/ and index/ objects are named by the store hash of their content, so a pack or index @@ -1484,14 +1484,14 @@ def check(self, repair=False, max_duration=0, max_age=0, repo_only=False, valida rebuild re-reads every pack anyway - so a read-only check just stops and reports it instead of continuing. A read-only check never rebuilds the index: reading every pack to do so would be far too slow and expensive for a routine (e.g. cron) check. With repair=True and a corrupt - index, and if every pack is intact, the index is rebuilt from the packs' object headers and - persisted; on a full check the archives phase rebuilds and re-persists it afterwards, see - ArchiveChecker.finish. Packs are verified by the store hash, which is content-addressing rather - than a MAC, so that check detects accidental corruption but not tampering; the rebuild therefore - checks every object with validate, see below, refs #9901, #10026. If any pack is corrupt the index - is not rebuilt, refs #10026. Pack ids found corrupt are kept in cache/checked-packs, - refs #9696. That object is stored in the key's envelope, too, so check() needs the key (see - set_key). + index, every pack is verified. With repo_only, and if every pack is intact, the index is then + rebuilt from the packs' object headers and persisted. Without repo_only, the archives phase + rebuilds and persists it (see ArchiveChecker.check and ArchiveChecker.finish), refs #10434. Packs + are verified by the store hash, which is content-addressing rather than a MAC, so that check + detects accidental corruption but not tampering; the rebuild therefore checks every object with + validate, see below, refs #9901, #10026. If any pack is corrupt the index is not rebuilt, refs + #10026. Pack ids found corrupt are kept in cache/checked-packs, refs #9696. That object is stored + in the key's envelope, too, so check() needs the key (see set_key). A pack recorded corrupt fails the check, also on a partial run that stops before re-reaching it. The record clears at the check that finds the pack intact again or gone (removed by @@ -1514,11 +1514,12 @@ def check(self, repair=False, max_duration=0, max_age=0, repo_only=False, valida max_age, accepting a future timestamp up to MAX_CLOCK_SKEW (clock skew). Results are recorded regardless of max_age. - repo_only: whether this is a repository-only run. In repair mode it sets the return value for - damage repair does not fix, i.e. a corrupt pack, a missing pack or a skipped pack byte range (see - validate): fail if repo_only, else defer (a full check's archives phase can repair a corrupt pack - holding metadata, or file content with --verify-data, and reports and repairs the archives that - reference chunks the index lacks). + repo_only: whether this is a repository-only run. Required if repair. In repair mode, if True, a + corrupt index is rebuilt here (see above), and damage repair does not fix, i.e. a corrupt pack, a + missing pack or a skipped pack byte range (see validate), fails the check. If False, both are left + to the archives phase: it rebuilds the index, can repair a corrupt pack holding metadata (or file + content with --verify-data), and reports and repairs the archives that reference chunks the index + lacks. validate: validate(chunk_id, obj) -> bool, True if obj (an object's header plus its metadata slot) is the repo object with id chunk_id, see repoobj.object_validator. Required if repair. @@ -1528,6 +1529,7 @@ def check(self, repair=False, max_duration=0, max_age=0, repo_only=False, valida none. Each skipped range counts as one error. """ assert validate is not None or not repair + assert repo_only is not None or not repair def verify(namespace, name): # name is the store hash of the object's content, so it is intact iff store.hash() matches. @@ -1597,11 +1599,15 @@ def store_list(namespace): # --repair forbids --max-duration and --max-age, so the partial and max_age handling in # the loop stays inactive during a repair. packs_scanned = True - if index_errors: + if index_errors and repo_only: logger.warning( "Repository index is corrupted; verifying all packs before deciding whether to " "rebuild it from them." ) + elif index_errors: + logger.warning( + "Repository index is corrupted; verifying all packs, the archives check rebuilds the index." + ) # packs are the bulk of the work and the part --max-duration spreads over several checks. pack_infos = store_list("packs") # drop objects whose name is not a valid pack name and count them as errors; the code @@ -1715,10 +1721,17 @@ def recorded_ts(info): logger.info("Finished checking packs.") tracker.prune(present_pack_ids) pack_pi.finish() - # rebuild only on repair, if the index was the sole problem and every pack was verified intact - # this run: sig_int breaks the loop early, so "no pack errors" must be paired with "all packs - # scanned" (pack_files == len(pack_infos)) to not rebuild from unverified packs. - if repair and index_errors and pack_errors == 0 and not sig_int and pack_files == len(pack_infos): + # rebuild only on a repository-only repair, if the index was the sole problem and every pack was + # verified intact this run: sig_int breaks the loop early, so "no pack errors" must be paired with + # "all packs scanned" (pack_files == len(pack_infos)) to not rebuild from unverified packs. + if ( + repair + and repo_only + and index_errors + and pack_errors == 0 + and not sig_int + and pack_files == len(pack_infos) + ): def note_drop(): nonlocal drops @@ -1737,14 +1750,16 @@ def note_drop(): interruptible=True, ) except ChunkIndexRebuildInterrupted: - # nothing was stored: the corrupt fragments stay, so the next use rebuilds from the packs. + # nothing was stored: the corrupt fragments stay. drops = 0 # counted by the discarded rebuild, which covered only a part of the packs - logger.warning("Index rebuild interrupted; the index stays corrupt and is rebuilt on next use.") + logger.warning('Index rebuild interrupted; the index stays corrupt, run "borg check --repair".') else: self.invalidate_chunk_index() # the rebuilt index is persisted; drop the in-memory copy index_repaired = True else: logger.error("Repository index is corrupted and must be repaired; skipping the pack check.") + # index_deferred: the archives phase rebuilds the corrupt index; it runs only if this check was not interrupted. + index_deferred = bool(index_errors) and repair and not repo_only and not sig_int objs_errors = index_errors + pack_errors + len(missing_pack_ids) + drops summary = ( f"Checked {index_files} index files ({index_errors} errors) " @@ -1797,13 +1812,15 @@ def note_drop(): # --verify-data), so it repairs a corrupt pack holding such objects; warn rather than fail. logger.warning(f"{done} {mode} repository check, corrupt pack(s) found{so_far}.") elif drops: - # a full check's archives phase reports the chunks the archives reference but the index - # lacks, so warn only. - log = logger.error if repo_only else logger.warning - log( + # drops come from a repository-only rebuild only; no archives phase follows it. + logger.error( f"{done} {mode} repository check, " f"index rebuilt without pack byte range(s) it could not authenticate{so_far}." ) + elif index_deferred: + logger.warning( + f"{done} {mode} repository check, index corrupt; the archives check rebuilds it from the packs." + ) elif index_errors and not index_repaired: # the index is corrupt but was not rebuilt, e.g. the pack verification was interrupted # before every pack was confirmed intact; the corrupt index is left in place. @@ -1817,11 +1834,10 @@ def note_drop(): else: # missing packs: the archives phase repairs the archives that reference their chunks. logger.warning(f"{done} {mode} repository check, missing pack(s) found{so_far}.") - # in repair mode a corrupt index left unrebuilt is a failure; a corrupt or missing pack, or a - # skipped pack byte range, fails only a repository-only run, while a full check defers it to the - # archives phase. + # in repair mode, a corrupt index neither rebuilt here nor deferred fails; a corrupt or missing pack, + # or a skipped pack byte range, fails a repository-only run, a full check defers it to the archives phase. if repair: - if index_errors and not index_repaired: + if index_errors and not index_repaired and not index_deferred: return False return not (repo_only and (pack_errors or corrupt_ids or missing_pack_ids or drops)) return not problems diff --git a/src/borg/testsuite/archiver/check_cmd_test.py b/src/borg/testsuite/archiver/check_cmd_test.py index 90a03f873f..0e4448fff4 100644 --- a/src/borg/testsuite/archiver/check_cmd_test.py +++ b/src/borg/testsuite/archiver/check_cmd_test.py @@ -23,7 +23,7 @@ ) from ...crypto.key import RepositoryKeyInfoMissing from ...constants import * # NOQA -from ...helpers import bin_to_hex, CommandError, CorruptPack, Error, sig_int +from ...helpers import bin_to_hex, hex_to_bin, CommandError, CorruptPack, Error, sig_int from ...helpers import BackupDamagedChunksError from ...helpers.passphrase import PassphraseWrong from ...hashindex import ChunkIndex @@ -908,10 +908,18 @@ def test_check_format_missing_archive_metadata(archivers, request): assert "Analyzing archive archive2" in output # the intact archive still uses the given format -def test_check_repair_rebuilds_corrupt_index(archivers, request): - # A corrupt index with all packs intact: the default (full) --repair rebuilds the index from the - # packs and persists it (via the archives check, see ArchiveChecker.finish), leaving the repository - # usable again without a slow rebuild on the next access. +@pytest.mark.parametrize( + "mode, message", + [ + ([], "the archives check rebuilds it from the packs"), + (["--repository-only"], "Repository index was corrupted and has been rebuilt from the packs."), + ], + ids=["full", "repository-only"], +) +def test_check_repair_rebuilds_corrupt_index(archivers, request, mode, message): + # A corrupt index with all packs intact: --repair rebuilds the index from the packs and persists it, + # leaving the repository usable again. A full check rebuilds it in the archives check (see + # ArchiveChecker.finish), a repository-only check in the repository check. archiver = request.getfixturevalue(archivers) check_cmd_setup(archiver) cmd(archiver, "check", exit_code=0) @@ -929,8 +937,8 @@ def test_check_repair_rebuilds_corrupt_index(archivers, request): else: with pytest.raises(CorruptChunkIndexFragment): cmd(archiver, "check") - output = cmd(archiver, "check", "-v", "--repair", exit_code=0) - assert "rebuilt" in output.lower() + output = cmd(archiver, "check", "-v", "--repair", *mode, exit_code=0) + assert message in output # item 6: repair persisted a fresh index instead of leaving it for a slow rebuild on the next # access. confirm the on-disk index exists and every fragment is intact. archive, repository = open_archive(archiver.repository_path, "archive1") @@ -943,6 +951,72 @@ def test_check_repair_rebuilds_corrupt_index(archivers, request): assert "archive1" in cmd(archiver, "repo-list") # and remains usable +def test_check_repair_walks_packs_once_for_corrupt_index(archiver, monkeypatch): + # A full --repair with a corrupt index walks the objects of each pack once, for the index rebuild in the + # archives check, refs #10434. + # local-only: this patches PackReader in-process. + check_cmd_setup(archiver) + with Repository(archiver.repository_path, exclusive=True) as repository: + pack_ids = {hex_to_bin(info.name) for info in repository.store_list("packs")} + for info in repository.store_list("index"): # rot every index fragment + name = f"index/{info.name}" + repository.store_store(name, corrupt(repository.store_load(name), 0)) + + orig_iter_headers = PackReader.iter_headers + walks = [] + + def counting_iter_headers(self, **kwargs): + walks.append(self.pack_id) + return orig_iter_headers(self, **kwargs) + + monkeypatch.setattr(PackReader, "iter_headers", counting_iter_headers) + output = cmd(archiver, "check", "-v", "--repair", exit_code=0) + monkeypatch.setattr(PackReader, "iter_headers", orig_iter_headers) + # finish() also walks the packs the repair wrote, so count the packs that existed before the check only. + assert sorted(pack_id for pack_id in walks if pack_id in pack_ids) == sorted(pack_ids) + assert "the archives check rebuilds it from the packs" in output + cmd(archiver, "check", exit_code=0) # the stored index is intact and matches the packs + + +def test_check_repair_interrupt_during_corrupt_index_rebuild(archiver, monkeypatch): + # A full --repair rebuilds a corrupt index in the archives check. A Ctrl-C during that rebuild stops the + # check and stores nothing: the corrupt fragments stay, a command needing the index aborts, and a second + # --repair completes, refs #10434. + # local-only: this patches PackReader in-process. + check_cmd_setup(archiver) # produces several packs + with Repository(archiver.repository_path, exclusive=True) as repository: + assert len(repository.store_list("packs")) > 1 # there is a pack boundary to stop at + for info in repository.store_list("index"): # rot every index fragment + name = f"index/{info.name}" + repository.store_store(name, corrupt(repository.store_load(name), 0)) + index_before = {info.name for info in repository.store_list("index")} + + orig_iter_headers = PackReader.iter_headers + packs_read = [] + + def iter_headers_then_interrupt(self, **kwargs): + packs_read.append(self.pack_id) + yield from orig_iter_headers(self, **kwargs) + sig_int._sig_int_triggered = True # one Ctrl-C after the first pack was walked + + monkeypatch.setattr(PackReader, "iter_headers", iter_headers_then_interrupt) + try: + with pytest.raises(Error, match="Got Ctrl-C"): + cmd(archiver, "check", "--repair") + finally: + sig_int._sig_int_triggered = False # reset the global flag for the following tests + # restore the real method; monkeypatch.undo() would also drop the autouse env (BORG_TESTONLY_WEAKEN_KDF). + monkeypatch.setattr(PackReader, "iter_headers", orig_iter_headers) + assert len(packs_read) == 1 # the repository check walked no pack, the archives check stopped after one + with Repository(archiver.repository_path, exclusive=True) as repository: + assert {info.name for info in repository.store_list("index")} == index_before # nothing stored + + with pytest.raises(CorruptChunkIndexFragment): + cmd(archiver, "repo-list") # the index is still corrupt; commands needing it abort + cmd(archiver, "check", "--repair", exit_code=0) + cmd(archiver, "check", exit_code=0) + + def tamper_object_keeping_pack_name(repository): """Flip a byte in the metadata slot of the 2nd object of a pack holding more than 2 objects, and store the pack under the store hash of its new content, so its content still matches its name. Corrupt @@ -1136,19 +1210,28 @@ def test_find_lost_archives_skips_chunk_with_corrupt_object_header(archivers, re assert "Archive consistency check complete, problems found." in output -def test_check_repair_validates_index_rebuild(archivers, request): - """--repair leaves an object that fails validation out of the index and keeps the object after it (#9901).""" +@pytest.mark.parametrize("repo_only", [False, True], ids=["full", "repository-only"]) +def test_check_repair_validates_index_rebuild(archivers, request, repo_only): + """--repair leaves an object that fails validation out of the index and keeps the object after it (#9901). + + A full check rebuilds the index in the archives check, a repository-only check in the repository check. + """ archiver = request.getfixturevalue(archivers) if archiver.get_kind() != "local": pytest.skip("inspects the store directly") check_cmd_setup(archiver) with KeyedRepository(archiver.repository_location, exclusive=True) as repository: tampered_id, neighbour_id = tamper_object_keeping_pack_name(repository) - output = cmd(archiver, "check", "-v", "--repair", exit_code=0) + if repo_only: + output = cmd(archiver, "check", "-v", "--repair", "--repository-only", exit_code=EXIT_WARNING) + assert "index rebuilt without pack byte range(s) it could not authenticate" in output + else: + output = cmd(archiver, "check", "-v", "--repair", exit_code=0) + assert "the archives check rebuilds it from the packs" in output + assert "Archive consistency check complete, problems found." in output assert "does not authenticate" in output - assert "continuing at the object at offset" in output - assert "index rebuilt without pack byte range(s) it could not authenticate" in output - assert "Archive consistency check complete, problems found." in output + # the index is rebuilt once, so the tampered object is reported once. + assert output.count("continuing at the object at offset") == 1 with KeyedRepository(archiver.repository_location) as repository: assert tampered_id not in repository.chunks assert neighbour_id in repository.chunks diff --git a/src/borg/testsuite/repository_test.py b/src/borg/testsuite/repository_test.py index 79599575e5..34958d9341 100644 --- a/src/borg/testsuite/repository_test.py +++ b/src/borg/testsuite/repository_test.py @@ -18,7 +18,7 @@ from ..compress import CNONE from ..constants import MAX_CLOCK_SKEW, ROBJ_FILE_STREAM from ..crypto.key import AESOCBKey, AuthenticatedKey, Blake3AuthenticatedKey, CHPOKey -from ..helpers import Error, IntegrityError, Location, bin_to_hex +from ..helpers import Error, IntegrityError, Location, bin_to_hex, hex_to_bin from ..hashindex import ChunkIndex, ChunkIndexEntry from ..platform import get_process_id from ..repository import Repository, MAX_DATA_SIZE, MAX_VALIDATED_META_SIZE, propagate_rsh, rest_serve_command @@ -1810,7 +1810,7 @@ def test_check_reports_invalid_pack_name(tmp_path, caplog): def test_check_repair_rebuilds_corrupt_index(tmp_path, caplog): - # check(repair=True) rebuilds a corrupt index from the packs' object headers. + # check(repair=True, repo_only=True) rebuilds a corrupt index from the packs' object headers. location = os.fspath(tmp_path / "repo") ids = [H(x) for x in range(10)] with Repository(location, exclusive=True, create=True) as repository: @@ -1828,7 +1828,8 @@ def test_check_repair_rebuilds_corrupt_index(tmp_path, caplog): with reopen(repository) as repository: caplog.clear() with caplog.at_level(logging.INFO, logger="borg.repository"): - assert repository.check(repair=True, validate=validate_any) is True # repair rebuilds the index + # repair rebuilds the index + assert repository.check(repair=True, repo_only=True, validate=validate_any) is True # each rotted fragment is counted once (the cross-check does not load the known corrupt index). assert f"Checked {len(index_names)} index files ({len(index_names)} errors)" in caplog.text with reopen(repository) as repository: @@ -1837,10 +1838,47 @@ def test_check_repair_rebuilds_corrupt_index(tmp_path, caplog): assert pdchunk(repository.get(cid)) == bytes([i]) * 20 # every chunk is indexed and resolves -@pytest.mark.parametrize("repo_only", [True, False]) -def test_check_repair_rebuild_validates_objects(tmp_path, caplog, repo_only): - # check(repair=True, validate=...) does not index an object validate rejects and reports it, refs - # #9901. That fails a repository-only run only. +def test_check_full_repair_leaves_corrupt_index_to_archives_phase(tmp_path, caplog, monkeypatch): + # check(repair=True, repo_only=False) with a corrupt index verifies every pack, walks no pack objects, + # stores no index and succeeds: the archives phase rebuilds the index, refs #10434. + location = os.fspath(tmp_path / "repo") + with Repository(location, exclusive=True, create=True) as repository: + for x in range(3): + repository.put(H(x), fchunk(b"DATA-%02d" % x, chunk_id=H(x))) + repository.flush() # seal a separate pack per chunk + with reopen(repository) as repository: + for info in repository.store_list("index"): # rot every fragment + name = f"index/{info.name}" + data = bytearray(repository.store_load(name)) + data[0] ^= 0xFF + repository.store_store(name, bytes(data)) + orig_iter_headers = PackReader.iter_headers + walked = [] + + def counting_iter_headers(self, **kwargs): + walked.append(self.pack_id) + return orig_iter_headers(self, **kwargs) + + monkeypatch.setattr(PackReader, "iter_headers", counting_iter_headers) + with reopen(repository) as repository: + pack_ids = {hex_to_bin(info.name) for info in repository.store_list("packs")} + index_before = {info.name for info in repository.store_list("index")} + with caplog.at_level(logging.INFO, logger="borg.repository"): + assert repository.check(repair=True, repo_only=False, validate=validate_any) is True + assert "and 3 packs (0 errors)." in caplog.text # every pack was verified + assert "Finished full repository check, index corrupt; the archives check rebuilds it" in caplog.text + assert "has been rebuilt" not in caplog.text + assert walked == [] # no pack object was walked + assert {info.name for info in repository.store_list("index")} == index_before # nothing stored + tracker = PackTracker.load(repository) + assert {pack_id for pack_id in pack_ids if tracker.get(pack_id).result} == pack_ids # recorded intact + with reopen(repository) as repository: + assert repository.check(repair=False) is False # the index is still corrupt + + +def test_check_repair_rebuild_validates_objects(tmp_path, caplog): + # check(repair=True, repo_only=True, validate=...) does not index an object validate rejects, reports it + # and fails, refs #9901. location = os.fspath(tmp_path / "repo") ids = [H(x) for x in range(10)] rejected_id = ids[4] @@ -1862,7 +1900,7 @@ def validate(chunk_id, obj): caplog.set_level(logging.INFO) with reopen(repository) as repository: - assert repository.check(repair=True, repo_only=repo_only, validate=validate) is not repo_only + assert repository.check(repair=True, repo_only=True, validate=validate) is False assert set(ids) <= set(validated) assert "skipped 1 pack byte range(s) it could not authenticate" in caplog.text with reopen(repository) as repository: @@ -1900,9 +1938,11 @@ def test_check_repair_refuses_when_pack_corrupt(tmp_path): assert repository.check(repair=False) is False # index was not rebuilt; still corrupt -def test_check_repair_leaves_index_when_interrupted(tmp_path, caplog, monkeypatch): +@pytest.mark.parametrize("repo_only", [True, False]) +def test_check_repair_leaves_index_when_interrupted(tmp_path, caplog, monkeypatch, repo_only): # an interrupted repair (SIGINT before every pack is verified) must not rebuild the index from - # packs it did not confirm intact: it leaves the corrupt index in place and fails. + # packs it did not confirm intact: it leaves the corrupt index in place and fails. A full check fails, too: + # an interrupted check skips the archives phase. location = os.fspath(tmp_path / "repo") ids = [H(x) for x in range(10)] with Repository(location, exclusive=True, create=True) as repository: @@ -1919,7 +1959,7 @@ def test_check_repair_leaves_index_when_interrupted(tmp_path, caplog, monkeypatc monkeypatch.setattr("borg.repository.sig_int", True) # simulate a SIGINT before the pack loop with caplog.at_level(logging.ERROR, logger="borg.repository"): # interrupted: index not rebuilt, so it fails - assert repository.check(repair=True, validate=validate_any) is False + assert repository.check(repair=True, repo_only=repo_only, validate=validate_any) is False assert "index still corrupt" in caplog.text with reopen(repository) as repository: assert repository.check(repair=False) is False # repair left the index corrupt @@ -1966,7 +2006,7 @@ def iter_headers_then_interrupt(self, **kwargs): assert len(repository.store_list("packs")) > 1 # there is a pack boundary to stop at index_before = set(info.name for info in repository.store_list("index")) with caplog.at_level(logging.WARNING, logger="borg.repository"): - assert repository.check(repair=True, validate=validate) is False + assert repository.check(repair=True, repo_only=True, validate=validate) is False assert "Index rebuild interrupted" in caplog.text assert "Interrupted full repository check, index still corrupt so far." in caplog.text assert "index rebuilt" not in caplog.text @@ -2207,7 +2247,7 @@ def test_check_repairs_index_fragment_failing_authentication(tmp_path, caplog, t assert repository.check(repair=False) is False assert f"Store object index/{name} is corrupted" in caplog.text assert name in {info.name for info in repository.store_list("index")} # a check does not write. - assert repository.check(repair=True, validate=accept_all) is True + assert repository.check(repair=True, repo_only=True, validate=accept_all) is True assert name not in {info.name for info in repository.store_list("index")} assert repository.check(repair=False) is True From e178bc3902688f2fcba9cbc9f14b2d124c047b66 Mon Sep 17 00:00:00 2001 From: Mrityunjay Raj Date: Wed, 30 Sep 2026 01:08:40 +0530 Subject: [PATCH 3/4] check: test a full --repair with a corrupt index and a corrupt pack, refs #10434 --- src/borg/repository.py | 9 +++--- src/borg/testsuite/archiver/check_cmd_test.py | 26 ++++++++++++++++ src/borg/testsuite/repository_test.py | 31 +++++++++++++++++++ 3 files changed, 62 insertions(+), 4 deletions(-) diff --git a/src/borg/repository.py b/src/borg/repository.py index 0da3cca4cb..8d24b9f949 100644 --- a/src/borg/repository.py +++ b/src/borg/repository.py @@ -1489,9 +1489,9 @@ def check(self, repair=False, max_duration=0, max_age=0, repo_only=None, validat rebuilds and persists it (see ArchiveChecker.check and ArchiveChecker.finish), refs #10434. Packs are verified by the store hash, which is content-addressing rather than a MAC, so that check detects accidental corruption but not tampering; the rebuild therefore checks every object with - validate, see below, refs #9901, #10026. If any pack is corrupt the index is not rebuilt, refs - #10026. Pack ids found corrupt are kept in cache/checked-packs, refs #9696. That object is stored - in the key's envelope, too, so check() needs the key (see set_key). + validate, see below, refs #9901, #10026. With repo_only, if any pack is corrupt, the index is not + rebuilt, refs #10026. Pack ids found corrupt are kept in cache/checked-packs, refs #9696. That + object is stored in the key's envelope, too, so check() needs the key (see set_key). A pack recorded corrupt fails the check, also on a partial run that stops before re-reaching it. The record clears at the check that finds the pack intact again or gone (removed by @@ -1810,7 +1810,8 @@ def note_drop(): else: # a full check's archives phase reads archive/item metadata (and file content with # --verify-data), so it repairs a corrupt pack holding such objects; warn rather than fail. - logger.warning(f"{done} {mode} repository check, corrupt pack(s) found{so_far}.") + deferred = "; index corrupt, the archives check rebuilds it from the packs" if index_deferred else "" + logger.warning(f"{done} {mode} repository check, corrupt pack(s) found{deferred}{so_far}.") elif drops: # drops come from a repository-only rebuild only; no archives phase follows it. logger.error( diff --git a/src/borg/testsuite/archiver/check_cmd_test.py b/src/borg/testsuite/archiver/check_cmd_test.py index 0e4448fff4..8feb5ccd28 100644 --- a/src/borg/testsuite/archiver/check_cmd_test.py +++ b/src/borg/testsuite/archiver/check_cmd_test.py @@ -951,6 +951,32 @@ def test_check_repair_rebuilds_corrupt_index(archivers, request, mode, message): assert "archive1" in cmd(archiver, "repo-list") # and remains usable +def test_check_repair_rebuilds_corrupt_index_with_corrupt_pack(archivers, request): + # A corrupt index and a corrupt pack: a full --repair rebuilds and stores the index in the archives check. + # The corrupt pack stays recorded corrupt, so a following check still fails on it, refs #10434. + archiver = request.getfixturevalue(archivers) + check_cmd_setup(archiver) + archive, repository = open_archive(archiver.repository_path, "archive1") + with repository: + bad_pack = sorted(info.name for info in repository.store_list("packs"))[0] + name = f"packs/{bad_pack}" + repository.store_store(name, corrupt(repository.store_load(name), -1)) + for info in repository.store_list("index"): # rot every index fragment + name = f"index/{info.name}" + repository.store_store(name, corrupt(repository.store_load(name), 0)) + output = cmd(archiver, "check", "-v", "--repair", exit_code=0) + assert "corrupt pack(s) found; index corrupt, the archives check rebuilds it from the packs." in output + archive, repository = open_archive(archiver.repository_path, "archive1") + with repository: + index_infos = list(repository.store_list("index")) + assert index_infos # a fresh index was stored + for info in index_infos: # each fragment's content matches its store hash name + assert repository.store.hash(f"index/{info.name}", algorithm=STORE_HASH_NAME) == info.name + output = cmd(archiver, "check", "-v", exit_code=1) + assert "Checked 1 index files (0 errors)" in output + assert f"Corrupt pack: {bad_pack}" in output + + def test_check_repair_walks_packs_once_for_corrupt_index(archiver, monkeypatch): # A full --repair with a corrupt index walks the objects of each pack once, for the index rebuild in the # archives check, refs #10434. diff --git a/src/borg/testsuite/repository_test.py b/src/borg/testsuite/repository_test.py index 34958d9341..976a9a946c 100644 --- a/src/borg/testsuite/repository_test.py +++ b/src/borg/testsuite/repository_test.py @@ -1938,6 +1938,37 @@ def test_check_repair_refuses_when_pack_corrupt(tmp_path): assert repository.check(repair=False) is False # index was not rebuilt; still corrupt +def test_check_full_repair_defers_corrupt_index_with_corrupt_pack(tmp_path, caplog): + # check(repair=True, repo_only=False) with a corrupt index and a corrupt pack succeeds and stores no + # index: the archives phase rebuilds the index and repairs what the corrupt pack held, refs #10434. + location = os.fspath(tmp_path / "repo") + with Repository(location, exclusive=True, create=True) as repository: + repository.put(H(1), fchunk(b"GOOD-CHUNK", chunk_id=H(1))) + repository.flush() # seal a pack holding H(1) + repository.put(H(2), fchunk(b"LOST-CHUNK", chunk_id=H(2))) + repository.flush() # seal a separate pack holding H(2) + with reopen(repository) as repository: + bad_pack_id = repository.chunks[H(2)].pack_id + bad_pack_name = "packs/" + bin_to_hex(bad_pack_id) + data = bytearray(repository.store_load(bad_pack_name)) + data[-1] ^= 0xFF # rot the pack holding H(2): its content no longer matches its store hash name + repository.store_store(bad_pack_name, bytes(data)) + for info in repository.store_list("index"): # rot every fragment + name = f"index/{info.name}" + idata = bytearray(repository.store_load(name)) + idata[0] ^= 0xFF + repository.store_store(name, bytes(idata)) + with reopen(repository) as repository: + index_before = {info.name for info in repository.store_list("index")} + with caplog.at_level(logging.INFO, logger="borg.repository"): + assert repository.check(repair=True, repo_only=False, validate=validate_any) is True + assert "and 2 packs (1 errors)." in caplog.text + assert "corrupt pack(s) found; index corrupt, the archives check rebuilds it from the packs." in caplog.text + assert {info.name for info in repository.store_list("index")} == index_before # nothing stored + assert bad_pack_name in [f"packs/{info.name}" for info in repository.store_list("packs")] # not dropped + assert PackTracker.load(repository).corrupt_ids() == [bad_pack_id] # recorded corrupt + + @pytest.mark.parametrize("repo_only", [True, False]) def test_check_repair_leaves_index_when_interrupted(tmp_path, caplog, monkeypatch, repo_only): # an interrupted repair (SIGINT before every pack is verified) must not rebuild the index from From e7b17fa84ad51b6ecbcfc37ad3f7e6adf8c9cf76 Mon Sep 17 00:00:00 2001 From: Mrityunjay Raj Date: Wed, 30 Sep 2026 04:16:47 +0530 Subject: [PATCH 4/4] check: do not depend on archive1 surviving the corrupt pack in the index rebuild test, refs #10434 --- src/borg/testsuite/archiver/check_cmd_test.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/borg/testsuite/archiver/check_cmd_test.py b/src/borg/testsuite/archiver/check_cmd_test.py index 8feb5ccd28..5258eb9124 100644 --- a/src/borg/testsuite/archiver/check_cmd_test.py +++ b/src/borg/testsuite/archiver/check_cmd_test.py @@ -956,8 +956,7 @@ def test_check_repair_rebuilds_corrupt_index_with_corrupt_pack(archivers, reques # The corrupt pack stays recorded corrupt, so a following check still fails on it, refs #10434. archiver = request.getfixturevalue(archivers) check_cmd_setup(archiver) - archive, repository = open_archive(archiver.repository_path, "archive1") - with repository: + with open_repository(archiver) as repository: bad_pack = sorted(info.name for info in repository.store_list("packs"))[0] name = f"packs/{bad_pack}" repository.store_store(name, corrupt(repository.store_load(name), -1)) @@ -966,8 +965,7 @@ def test_check_repair_rebuilds_corrupt_index_with_corrupt_pack(archivers, reques repository.store_store(name, corrupt(repository.store_load(name), 0)) output = cmd(archiver, "check", "-v", "--repair", exit_code=0) assert "corrupt pack(s) found; index corrupt, the archives check rebuilds it from the packs." in output - archive, repository = open_archive(archiver.repository_path, "archive1") - with repository: + with open_repository(archiver) as repository: index_infos = list(repository.store_list("index")) assert index_infos # a fresh index was stored for info in index_infos: # each fragment's content matches its store hash name