From 8c32faec25a3ffa4355567440e2b2406ceca8d28 Mon Sep 17 00:00:00 2001 From: Mrityunjay Raj Date: Mon, 10 Aug 2026 01:43:55 +0530 Subject: [PATCH 1/5] check --repair: rebuild a corrupt repository index from the packs, #10026 A read-only check that finds the repository index corrupt stops and reports it, as before. With --repair, and only if every pack is intact, the index is now rebuilt from the packs' object headers and persisted; if any pack is corrupt the index and the packs are left unchanged and the corruption is reported (salvaging a corrupt pack's still-intact objects is not implemented yet, refs #8572). Packs are named and verified by the sha256 of their content, which is content-addressing rather than a MAC, so this rebuild detects accidental corruption but not tampering, refs #9901. On a full check the archives phase runs after the repository phase, so ArchiveChecker.finish() now persists the chunks index instead of deleting it: it rebuilds from the packs when repair changed them, else writes out the index it already holds, then drops the in-memory copy so close() does not overwrite it. Previously finish() deleted the on-disk index, forcing a slow rebuild on the next repository access. check() gains a repo_only argument: a corrupt pack fails a repository-only repair, but a full check defers the verdict to the archives phase, which can repair a corrupt pack holding archive/item metadata (or file content with --verify-data). The slow rebuild path now shows a progress indicator. --- src/borg/archive.py | 28 +++++-- src/borg/archiver/check_cmd.py | 10 ++- src/borg/cache.py | 12 ++- src/borg/repository.py | 73 ++++++++++++++----- src/borg/testsuite/archiver/check_cmd_test.py | 30 ++++++++ src/borg/testsuite/repository_test.py | 52 +++++++++++++ 6 files changed, 175 insertions(+), 30 deletions(-) diff --git a/src/borg/archive.py b/src/borg/archive.py index db7de5cd03..5a95c875a9 100644 --- a/src/borg/archive.py +++ b/src/borg/archive.py @@ -23,7 +23,7 @@ from . import xattr from .chunkers import get_chunker, Chunk, release_chunk_data -from .cache import ChunkListEntry, build_chunkindex_from_repo, delete_chunkindex_from_repo +from .cache import ChunkListEntry, build_chunkindex_from_repo, write_chunkindex_to_repo from .crypto.key import key_factory, UnsupportedPayloadError from .constants import * # NOQA from .crypto.low_level import IntegrityError as IntegrityErrorBase @@ -1884,6 +1884,9 @@ class ArchiveChecker: def __init__(self): self.error_found = False self.key = None + # True once repair drops a defect chunk or writes a new one, i.e. once the chunks index no + # longer matches the packs. + self.chunks_modified = False def check( self, @@ -2081,6 +2084,7 @@ def verify_data(self): # 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.chunks_modified = True # drop it from our own index too, so rebuild_archives reports the file it belongs to. del self.chunks[defect_chunk] else: @@ -2237,6 +2241,7 @@ def add_reference(id_, size, cdata): if self.repair: pack_results = self.repository.put(id_, cdata) self.chunks.update_pack_info(pack_results) + self.chunks_modified = True def verify_file_chunks(archive_name, item): """Verify that all of a file's chunks are present, collecting any missing ones for the report.""" @@ -2450,13 +2455,20 @@ def valid_item(obj): def finish(self): if self.repair: - # store packs still buffered from chunks re-added during repair, so their index entries - # are set before the index is dropped below and no chunk is left buffered at close(). - self.repository.flush() - # we may have deleted chunks. delete_chunkindex_from_repo() removes the on-disk index and - # drops the stale in-memory index, so the next repository access rebuilds it from the repo. - logger.info("Deleting chunk indexes in repository - next repository access will cause a rebuild.") - delete_chunkindex_from_repo(self.repository) + if self.chunks_modified: + # the packs changed, so the index no longer matches them: rebuild it from the packs + # and persist it. flush first so the rewritten and newly written packs are on the store. + self.repository.flush() + logger.info("Rebuilding and writing the repository chunks index.") + build_chunkindex_from_repo(self.repository, slow_rebuild=True, write_immediately=True) + else: + # the packs are unchanged, so the index still matches them: persist it as is. + logger.info("Writing the rebuilt repository chunks index.") + write_chunkindex_to_repo( + self.repository, self.chunks, incremental=False, clear=False, force_write=True, delete_other=True + ) + # drop the in-memory index so close() does not persist it over the index just written. + self.repository.invalidate_chunk_index() logger.info("Writing Manifest.") self.manifest.write() diff --git a/src/borg/archiver/check_cmd.py b/src/borg/archiver/check_cmd.py index fa31c6a98b..717051fcb1 100644 --- a/src/borg/archiver/check_cmd.py +++ b/src/borg/archiver/check_cmd.py @@ -78,7 +78,9 @@ def do_check(self, args, repository): # the repository check has finished, which can take hours. ArchiveFormatter.validate_format(format) if not args.archives_only: - if not repository.check(repair=args.repair, max_duration=args.max_duration, max_age=max_age): + if not repository.check( + repair=args.repair, max_duration=args.max_duration, max_age=max_age, repo_only=args.repo_only + ): set_ec(EXIT_WARNING) if sig_int: # repository check interrupted; skip the archive check raise Error("Got Ctrl-C / SIGINT.") @@ -232,8 +234,10 @@ 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 removes corrupted - objects from the repository after it did a 2nd try to read them correctly. + 1. When checking the repository's consistency, repair mode rebuilds the repository + index from the packs if the index is corrupt, provided every pack is intact. If + any pack is corrupt, the index is left as-is and the corruption is reported; + salvaging a corrupt pack's still-intact objects is not implemented yet. 2. When checking the consistency and correctness of archives, repair mode might remove whole archives from the manifest if their archive metadata chunk is diff --git a/src/borg/cache.py b/src/borg/cache.py index e49e3533be..b018879b38 100644 --- a/src/borg/cache.py +++ b/src/borg/cache.py @@ -30,7 +30,7 @@ from .helpers import hex_to_bin, bin_to_hex, parse_stringified_list from .helpers import format_file_size, safe_encode from .helpers import safe_ns -from .helpers import ProgressIndicatorMessage +from .helpers import ProgressIndicatorMessage, ProgressIndicatorPercent from .helpers import msgpack from .helpers.msgpack import int_to_timestamp, timestamp_to_int from .item import ChunkListEntry @@ -938,15 +938,23 @@ def build_chunkindex_from_repo( # headers and skipping the (much larger) encrypted payloads. Don't call Repository.list() here: # it iterates this same index we are building, so it would recurse. The headers also give each # object's real (chunk_id, offset, size), so every object in a pack is indexed individually. - for info in repository.store_list("packs"): + pack_infos = repository.store_list("packs") + pi = ProgressIndicatorPercent( + total=len(pack_infos), msg="Rebuilding chunk index %3.0f%%", msgid="cache.build_chunkindex_from_repo" + ) + for info in pack_infos: # PackReader uses the store directly, so refresh the lock here; a full rebuild can be slow. repository._lock_refresh() + pi.show(increase=1) pack_id = hex_to_bin(info.name) for chunk_id, obj_offset, obj_size in PackReader(repository.store, pack_id).iter_headers(): num_chunks += 1 chunks[chunk_id] = ChunkIndexEntry( flags=init_flags, size=0, pack_id=pack_id, obj_offset=obj_offset, obj_size=obj_size ) + if pack_infos: + pi.show(current=len(pack_infos)) # finish at 100% + pi.finish() duration = perf_counter() - t0 or 0.001 # Chunk IDs in a list are encoded in 34 bytes: 1 byte msgpack header, 1 byte length, 32 ID bytes. # Protocol overhead is neglected in this calculation. diff --git a/src/borg/repository.py b/src/borg/repository.py index d6a851237f..67349b8531 100644 --- a/src/borg/repository.py +++ b/src/borg/repository.py @@ -992,7 +992,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): + def check(self, repair=False, max_duration=0, max_age=0, repo_only=False): """Check repository consistency. packs/ and index/ objects are named by the sha256 of their content, so a pack or index file @@ -1002,14 +1002,18 @@ def check(self, repair=False, max_duration=0, max_age=0): The index is hashed first and the packs only if it is intact. The packs could be hashed even with a corrupt index, but a corrupt index already means the user has to repair it, and that rebuild re-reads every pack anyway - so a read-only check just stops and reports it instead of - continuing. The index is never rebuilt here in any case: reading every pack to do so would be - far too slow and expensive for a routine (e.g. cron) check. Salvaging good objects out of - corrupt packs and dropping those packs is left to repair, refs #8572. The ids of the packs - found corrupt are kept in cache/checked-packs for repair, refs #9696. + 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 sha256, which is content-addressing rather than a + MAC, so this rebuild detects accidental corruption but not tampering, refs #9901, #10026. If any + pack is corrupt the index is left unchanged, refs #8572, #10026. Pack ids found corrupt are kept + in cache/checked-packs, refs #9696. 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, or salvaged and dropped by repair; refs #8572); prune() does this from packs/. + compact; TODO: also when repair salvages and drops it, refs #8572); 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 @@ -1022,6 +1026,10 @@ def check(self, repair=False, max_duration=0, max_age=0): max_age (seconds, 0 = verify every pack): skip packs whose intact record is younger than 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 a + corrupt pack, which repair does not fix: 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). """ def verify(namespace, name): @@ -1061,6 +1069,8 @@ def store_list(namespace): index_files = index_errors = 0 pack_files = pack_errors = pack_skipped = 0 missing_pack_ids = [] # packs referenced by the index but absent from packs/ (refs #9898) + index_repaired = False + packs_scanned = False # index and packs get separate progress indicators, each running from 0% to 100%. # the index is checked first and in full, on partial checks too: it is small, and index errors # stop the pack check below. @@ -1082,7 +1092,13 @@ def store_list(namespace): if index_infos: index_pi.show(current=len(index_infos)) # finish at 100% index_pi.finish() - if index_errors == 0: + if index_errors == 0 or repair: + # verify the packs; during repair, rebuild the corrupt index from them afterwards. + # --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: + logger.warning("Repository index is corrupted; rebuilding it from the packs.") # 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 @@ -1172,8 +1188,16 @@ def recorded_ts(info): logger.info("Finished checking packs.") tracker.prune(present_pack_ids) pack_pi.finish() + if index_errors and pack_errors == 0: + from .cache import build_chunkindex_from_repo + + # rebuild the index from the packs. the exclusive check lock keeps the pack set + # fixed, so re-listing packs/ inside build_chunkindex_from_repo matches this + # verification. write_immediately persists the index and drops the corrupt fragments. + build_chunkindex_from_repo(self, slow_rebuild=True, write_immediately=True) + self.invalidate_chunk_index() # the rebuilt index is persisted; drop the in-memory copy + index_repaired = True else: - # TODO: --repair will rebuild the index from the packs here instead of stopping (refs #8572). logger.error("Repository index is corrupted and must be repaired; skipping the pack check.") objs_errors = index_errors + pack_errors + len(missing_pack_ids) summary = ( @@ -1192,11 +1216,13 @@ def recorded_ts(info): "The chunks stored in these packs are lost. Repairing the index (dropping the " "stale references) is tracked in https://github.com/borgbackup/borg/issues/8572." ) - # corrupt_ids() is every pack recorded corrupt, including from earlier runs. with a corrupt - # index the packs were not scanned, so report nothing. - corrupt_ids = tracker.corrupt_ids() if index_errors == 0 else [] + if index_repaired: + logger.info("Repository index was corrupted and has been rebuilt from the packs.") + # corrupt_ids() includes packs recorded corrupt in earlier runs; report them only when this + # run scanned the packs. + corrupt_ids = tracker.corrupt_ids() if packs_scanned else [] if corrupt_ids: - # one id per line (the list can be long). + # one id per line, the list can be long. logger.error(f"Found {len(corrupt_ids)} corrupt pack(s):") for pack_id in corrupt_ids: logger.error(f"Corrupt pack: {bin_to_hex(pack_id)}") @@ -1206,12 +1232,25 @@ def recorded_ts(info): done, so_far = ("Interrupted", " so far") if sig_int else ("Finished", "") if not problems: logger.info(f"{done} {mode} repository check, no problems found{so_far}.") - elif repair: - logger.error(f"{done} {mode} repository check, errors found{so_far} (repository repair not implemented).") - else: + elif not repair: logger.error(f"{done} {mode} repository check, errors found{so_far}.") - # True means the checked objects were clean; --repair returns True so the caller proceeds to fix them. - return not problems or repair + elif not (pack_errors or corrupt_ids): + # only the index was corrupt, and it was rebuilt. + logger.info(f"{done} {mode} repository check, repaired{so_far}.") + elif 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)." + ) + 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}.") + # in repair mode a corrupt pack fails only a repository-only run; a full check defers to the + # archives phase. + if repair: + return not (repo_only and (pack_errors or corrupt_ids)) + return not problems def list(self, limit=None, marker=None): """ diff --git a/src/borg/testsuite/archiver/check_cmd_test.py b/src/borg/testsuite/archiver/check_cmd_test.py index f9b881cc38..f738db6b7b 100644 --- a/src/borg/testsuite/archiver/check_cmd_test.py +++ b/src/borg/testsuite/archiver/check_cmd_test.py @@ -594,6 +594,36 @@ def test_spoofed_manifest(archivers, request): cmd(archiver, "check", exit_code=0) +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. + archiver = request.getfixturevalue(archivers) + check_cmd_setup(archiver) + cmd(archiver, "check", exit_code=0) + archive, repository = open_archive(archiver.repository_path, "archive1") + with repository: + assert isinstance(repository, Repository) + for info in repository.store_list("index"): # rot every index fragment + name = f"index/{info.name}" + data = bytearray(repository.store_load(name)) + data[0] ^= 0xFF + repository.store_store(name, bytes(data)) + cmd(archiver, "check", exit_code=1) # read-only check reports the corrupt index + output = cmd(archiver, "check", "-v", "--repair", exit_code=0) + assert "rebuilt" in output.lower() + # 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") + with repository: + index_infos = list(repository.store_list("index")) + assert index_infos # a fresh index was persisted + for info in index_infos: # each fragment's content still matches its sha256 name + assert repository.store.hash(f"index/{info.name}") == info.name + cmd(archiver, "check", exit_code=0) # the repository is consistent again + assert "archive1" in cmd(archiver, "repo-list") # and remains usable + + @pytest.mark.skip(reason="TODO: repair does not yet rewrite store-corrupted packs, refs #8572") def test_manifest_rebuild_corrupted_chunk(archivers, request): archiver = request.getfixturevalue(archivers) diff --git a/src/borg/testsuite/repository_test.py b/src/borg/testsuite/repository_test.py index de21668bef..c9020dd851 100644 --- a/src/borg/testsuite/repository_test.py +++ b/src/borg/testsuite/repository_test.py @@ -1080,6 +1080,58 @@ def test_check_reports_invalid_pack_name(tmp_path, caplog): assert after.table[intact_id].result == 1 # the valid pack was checked +def test_check_repair_rebuilds_corrupt_index(tmp_path): + # check(repair=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: + for i, cid in enumerate(ids): + repository.put(cid, fchunk(bytes([i]) * 20, chunk_id=cid)) + repository.flush() # seal the pack(s) and let close() persist the index + with reopen(repository) as repository: + index_names = [f"index/{info.name}" for info in repository.store_list("index")] + assert index_names # close() persisted at least one index fragment + for name in index_names: # rot every fragment so its content no longer matches its sha256 name + data = bytearray(repository.store_load(name)) + data[0] ^= 0xFF + repository.store_store(name, bytes(data)) + assert repository.check(repair=False) is False # read-only check reports the corrupt index + with reopen(repository) as repository: + assert repository.check(repair=True) is True # repair rebuilds the index from the packs + with reopen(repository) as repository: + assert repository.check(repair=False) is True # the rebuilt index passes a read-only check + for i, cid in enumerate(ids): + assert pdchunk(repository.get(cid)) == bytes([i]) * 20 # every chunk is indexed and resolves + + +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. + 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_name = "packs/" + bin_to_hex(repository.chunks[H(2)].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 sha256 name + repository.store_store(bad_pack_name, bytes(data)) + for info in repository.store_list("index"): # rot the index so repair takes the rebuild path + 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: + # a repository-only repair cannot fix a corrupt pack, so it fails. + assert repository.check(repair=True, repo_only=True) is False + # the corrupt pack is left in place, not dropped. + assert bad_pack_name in [f"packs/{info.name}" for info in repository.store_list("packs")] + with reopen(repository) as repository: + assert repository.check(repair=False) is False # index was not rebuilt; still corrupt + + def test_check_warns_on_invalid_chunk_index(tmp_path, caplog): # check warns about an invalid chunk index but does not fail, since the index is not part of # the repository's object integrity. From e1fdba6bcf582fbc39e8927b7a223619901d770c Mon Sep 17 00:00:00 2001 From: Mrityunjay Raj Date: Wed, 12 Aug 2026 14:17:30 +0530 Subject: [PATCH 2/5] check --repair: only rebuild the index when every pack was verified intact this run Gate the rebuild on an uninterrupted, full pack scan, flush re-added chunks before rebuilding in finish(), and report/return honestly when the index stays corrupt. --- src/borg/archive.py | 6 ++- src/borg/repository.py | 44 ++++++++++++------- src/borg/testsuite/archiver/check_cmd_test.py | 4 +- 3 files changed, 34 insertions(+), 20 deletions(-) diff --git a/src/borg/archive.py b/src/borg/archive.py index 5a95c875a9..6af3bb0612 100644 --- a/src/borg/archive.py +++ b/src/borg/archive.py @@ -2455,10 +2455,12 @@ def valid_item(obj): def finish(self): if self.repair: + # flush chunks re-added during repair so their packs are on the store and out of the pack + # writer buffer (close() requires an empty buffer, #10055) before we (re)build the index. + self.repository.flush() if self.chunks_modified: # the packs changed, so the index no longer matches them: rebuild it from the packs - # and persist it. flush first so the rewritten and newly written packs are on the store. - self.repository.flush() + # and persist it. logger.info("Rebuilding and writing the repository chunks index.") build_chunkindex_from_repo(self.repository, slow_rebuild=True, write_immediately=True) else: diff --git a/src/borg/repository.py b/src/borg/repository.py index 67349b8531..56e597fe0e 100644 --- a/src/borg/repository.py +++ b/src/borg/repository.py @@ -1098,7 +1098,7 @@ def store_list(namespace): # the loop stays inactive during a repair. packs_scanned = True if index_errors: - logger.warning("Repository index is corrupted; rebuilding it from the packs.") + logger.warning("Repository index is corrupted; verifying all packs before rebuilding it from them.") # 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 @@ -1188,12 +1188,15 @@ def recorded_ts(info): logger.info("Finished checking packs.") tracker.prune(present_pack_ids) pack_pi.finish() - if index_errors and pack_errors == 0: + # rebuild only 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 index_errors and pack_errors == 0 and not sig_int and pack_files == len(pack_infos): from .cache import build_chunkindex_from_repo - # rebuild the index from the packs. the exclusive check lock keeps the pack set - # fixed, so re-listing packs/ inside build_chunkindex_from_repo matches this - # verification. write_immediately persists the index and drops the corrupt fragments. + # the exclusive check lock keeps the pack set fixed, so re-listing packs/ inside + # build_chunkindex_from_repo matches this verification. write_immediately persists the + # index and drops the corrupt fragments. build_chunkindex_from_repo(self, slow_rebuild=True, write_immediately=True) self.invalidate_chunk_index() # the rebuilt index is persisted; drop the in-memory copy index_repaired = True @@ -1234,21 +1237,28 @@ def recorded_ts(info): logger.info(f"{done} {mode} repository check, no problems found{so_far}.") elif not repair: logger.error(f"{done} {mode} repository check, errors found{so_far}.") - elif not (pack_errors or corrupt_ids): - # only the index was corrupt, and it was rebuilt. + elif index_repaired and not (pack_errors or corrupt_ids): + # the index was the only problem and it has been rebuilt from the packs. logger.info(f"{done} {mode} repository check, repaired{so_far}.") - elif 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)." - ) + elif pack_errors or corrupt_ids: + 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)." + ) + 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}.") 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}.") - # in repair mode a corrupt pack fails only a repository-only run; a full check defers to the - # archives phase. + # 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. + logger.error(f"{done} {mode} repository check, index still corrupt{so_far}.") + # in repair mode a corrupt index left unrebuilt is a failure; a corrupt pack fails only a + # repository-only run, while a full check defers it to the archives phase. if repair: + if index_errors and not index_repaired: + return False return not (repo_only and (pack_errors or corrupt_ids)) return not problems diff --git a/src/borg/testsuite/archiver/check_cmd_test.py b/src/borg/testsuite/archiver/check_cmd_test.py index f738db6b7b..85123a8d0e 100644 --- a/src/borg/testsuite/archiver/check_cmd_test.py +++ b/src/borg/testsuite/archiver/check_cmd_test.py @@ -719,7 +719,7 @@ def test_extra_chunks(archivers, request): def test_repair_finish_flushes_pack_writer(archivers, request): - """finish() stores chunks re-added during --repair before it drops the index (#10055). + """finish() stores chunks re-added during --repair before it (re)builds the index (#10055). close() asserts an empty pack writer buffer, so a chunk left buffered by finish() would trip it. @@ -736,6 +736,8 @@ def test_repair_finish_flushes_pack_writer(archivers, request): checker.repository = repository checker.key = checker.make_key(repository) checker.manifest = Manifest.load(repository, (Manifest.Operation.CHECK,), key=checker.key) + # re-adding a chunk makes the chunks index no longer match the packs, so finish() rebuilds it. + checker.chunks_modified = True # a chunk re-added during repair, buffered in the pack writer: key = b"01234567890123456789012345678901" From 8b80d148170e3899bb5d25fcf2e5556f7361b3b5 Mon Sep 17 00:00:00 2001 From: Mrityunjay Raj Date: Thu, 13 Aug 2026 13:23:53 +0530 Subject: [PATCH 3/5] check --repair: scope the corrupt-pack index note to the repository check, soften the pre-rebuild warning --- src/borg/archiver/check_cmd.py | 5 +++-- src/borg/repository.py | 5 ++++- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/src/borg/archiver/check_cmd.py b/src/borg/archiver/check_cmd.py index 717051fcb1..d6aedb8517 100644 --- a/src/borg/archiver/check_cmd.py +++ b/src/borg/archiver/check_cmd.py @@ -236,8 +236,9 @@ def build_parser_check(self, subparsers, common_parser, mid_common_parser): 1. When checking the repository's consistency, repair mode rebuilds the repository index from the packs if the index is corrupt, provided every pack is intact. If - any pack is corrupt, the index is left as-is and the corruption is reported; - salvaging a corrupt pack's still-intact objects is not implemented yet. + any pack is corrupt, the repository check leaves the index and the packs untouched + and reports the corruption; salvaging a corrupt pack's still-intact objects is not + implemented yet (refs #8572). 2. When checking the consistency and correctness of archives, repair mode might remove whole archives from the manifest if their archive metadata chunk is diff --git a/src/borg/repository.py b/src/borg/repository.py index 56e597fe0e..354f6943aa 100644 --- a/src/borg/repository.py +++ b/src/borg/repository.py @@ -1098,7 +1098,10 @@ def store_list(namespace): # the loop stays inactive during a repair. packs_scanned = True if index_errors: - logger.warning("Repository index is corrupted; verifying all packs before rebuilding it from them.") + logger.warning( + "Repository index is corrupted; verifying all packs before deciding whether to " + "rebuild it from them." + ) # 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 From 64c79e113fbdbc96f8ae1fa0fa66235399d43076 Mon Sep 17 00:00:00 2001 From: Mrityunjay Raj Date: Sat, 15 Aug 2026 18:44:24 +0530 Subject: [PATCH 4/5] check --repair: report index-referenced missing packs as errors, not a corrupt index --- src/borg/repository.py | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/src/borg/repository.py b/src/borg/repository.py index 354f6943aa..a007f7561f 100644 --- a/src/borg/repository.py +++ b/src/borg/repository.py @@ -1195,8 +1195,6 @@ def recorded_ts(info): # 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 index_errors and pack_errors == 0 and not sig_int and pack_files == len(pack_infos): - from .cache import build_chunkindex_from_repo - # the exclusive check lock keeps the pack set fixed, so re-listing packs/ inside # build_chunkindex_from_repo matches this verification. write_immediately persists the # index and drops the corrupt fragments. @@ -1240,7 +1238,7 @@ def recorded_ts(info): logger.info(f"{done} {mode} repository check, no problems found{so_far}.") elif not repair: logger.error(f"{done} {mode} repository check, errors found{so_far}.") - elif index_repaired and not (pack_errors or corrupt_ids): + elif index_repaired and not (pack_errors or corrupt_ids or missing_pack_ids): # the index was the only problem and it has been rebuilt from the packs. logger.info(f"{done} {mode} repository check, repaired{so_far}.") elif pack_errors or corrupt_ids: @@ -1253,16 +1251,19 @@ def recorded_ts(info): # 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}.") - else: + 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. logger.error(f"{done} {mode} repository check, index still corrupt{so_far}.") - # in repair mode a corrupt index left unrebuilt is a failure; a corrupt pack fails only a - # repository-only run, while a full check defers it to the archives phase. + else: + # index-referenced packs are missing, so their chunks are lost. + logger.error(f"{done} {mode} repository check, errors found{so_far}.") + # in repair mode a corrupt index left unrebuilt is a failure; a corrupt or missing pack fails + # only a repository-only run, while a full check defers it to the archives phase. if repair: if index_errors and not index_repaired: return False - return not (repo_only and (pack_errors or corrupt_ids)) + return not (repo_only and (pack_errors or corrupt_ids or missing_pack_ids)) return not problems def list(self, limit=None, marker=None): From 9bf6d5ce9acf93af5217ccf8351734d869d3b8ea Mon Sep 17 00:00:00 2001 From: Mrityunjay Raj Date: Sat, 15 Aug 2026 18:54:18 +0530 Subject: [PATCH 5/5] check --repair: test the interrupted-rebuild guard and missing-pack error path --- src/borg/testsuite/repository_test.py | 47 +++++++++++++++++++++++++++ 1 file changed, 47 insertions(+) diff --git a/src/borg/testsuite/repository_test.py b/src/borg/testsuite/repository_test.py index c9020dd851..2352e5ad79 100644 --- a/src/borg/testsuite/repository_test.py +++ b/src/borg/testsuite/repository_test.py @@ -1132,6 +1132,53 @@ 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): + # 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. + location = os.fspath(tmp_path / "repo") + ids = [H(x) for x in range(10)] + with Repository(location, exclusive=True, create=True) as repository: + for i, cid in enumerate(ids): + repository.put(cid, fchunk(bytes([i]) * 20, chunk_id=cid)) + repository.flush() # seal the pack(s) and let close() persist the index + with reopen(repository) as repository: + for info in repository.store_list("index"): # rot every fragment so repair takes the rebuild path + name = f"index/{info.name}" + data = bytearray(repository.store_load(name)) + data[0] ^= 0xFF + repository.store_store(name, bytes(data)) + with reopen(repository) as repository: + monkeypatch.setattr("borg.repository.sig_int", True) # simulate a SIGINT before the pack loop + with caplog.at_level(logging.ERROR, logger="borg.repository"): + assert repository.check(repair=True) is False # interrupted: index not rebuilt, so it fails + assert "index still corrupt" in caplog.text + with reopen(repository) as repository: + assert repository.check(repair=False) is False # repair left the index corrupt + + +def test_check_repair_reports_missing_pack_as_error(tmp_path, caplog): + # a repair with an intact index but a pack the index references missing from packs/ reports the + # loss and fails a repository-only run; a full check defers it to the archives phase (refs #9898, + # #8572). + 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() # flush before close persists the index + with reopen(repository) as repository: + pack_id = repository.chunks[H(0)].pack_id + repository.store_delete("packs/" + bin_to_hex(pack_id)) # pack gone, index entry kept + with reopen(repository) as repository: + # a repository-only repair cannot recover the lost chunks, so it fails and reports the error. + with caplog.at_level(logging.ERROR, logger="borg.repository"): + assert repository.check(repair=True, repo_only=True) is False + assert f"Missing pack: {bin_to_hex(pack_id)}" in caplog.text + assert "errors found" in caplog.text + with reopen(repository) as repository: + # a full check defers the missing pack to the archives phase, so the repository phase passes. + assert repository.check(repair=True, repo_only=False) is True + + def test_check_warns_on_invalid_chunk_index(tmp_path, caplog): # check warns about an invalid chunk index but does not fail, since the index is not part of # the repository's object integrity.