check --repair: a corrupt pack in which no object authenticates is never removed, check and compact keep failing #10479

If no object in a corrupt pack authenticates, salvage_pack returns SALVAGE_NOTHING_AUTHENTICATES and the pack is left in place. After that, nothing short of borg check --repair --verify-data gets the repository clean again:

• borg check --repair exits with rc 0 and says "corrupt pack(s) left". Running it again changes nothing.
• Every later borg check exits with rc 1.
• Every later borg compact exits with rc 1 and says Run "borg check --repair" to salvage them. Following that hint doesn't help. The pack is too small to pass compact's worth_reclaiming gate, so compact never drops it, even though its objects are unused after the repair.

This is easy to hit. Each archive's metadata object (ROBJ_ARCHIVE_META) is stored in its own pack of about 500 bytes, so a single flipped bit in that pack is enough. The same happens with a pack holding only an archive's item-metadata and item-pointer objects when all of them are damaged.

Reproduction

master 25c2211, aes256-ocb, local repository:

borg repo-create -e aes256-ocb
borg create a1 src          # src: one 3 MB file
borg create a2 src
# the smallest pack holds only the archive metadata object of one archive (500 bytes)
P=$(ls -S "$BORG_REPO"/packs/*/* | tail -1)
python3 -c "import sys;f=open(sys.argv[1],'r+b');f.seek(-10,2);b=f.read(1);f.seek(-10,2);f.write(bytes([b[0]^1]))" "$P"

borg check --repair         # rc 0
borg check --repair         # rc 0, same output
borg check                  # rc 1
borg compact                # rc 1
borg check --repair --verify-data   # rc 0
borg check                  # rc 0
borg compact                # rc 0

The first borg check --repair prints:

Store object packs/60cbc3f8... is corrupted: content does not match its name (store hash).
Corrupt pack 60cbc3f8... was not salvaged: nothing authenticates.
Found 1 corrupt pack(s):
Corrupt pack: 60cbc3f8...
Finished full repository check, corrupt pack(s) left.
Archive metadata block f999b0a0... is corrupted: Data integrity error: ... Could not decrypt [Authentication / EVP_DecryptFinal_ex failed]
Deleting broken archive archive-metadata-has-integrity-error f999b0a0....
Archive consistency check complete, problems found.

The second one reports the same corrupt pack again and also exits with rc 0. The archive was already deleted, but its corrupt metadata object stays in the index and in the pack.

borg compact:

1 pack(s) recorded corrupt by "borg check" are not rewritten or merged. Run "borg check --repair" to salvage them.

Why it happens

• Repository.check() only warns for a full check (not --repository-only) with corrupt packs left. The comment there says "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". Without --verify-data that is not true. The archives phase deletes the broken archive but does not remove the corrupt object. Only the defect-chunk delete of --verify-data does, and that deletes the pack too.
• Before check: --repair salvages the packs recorded corrupt, refs #10026 #10455, compact's hint said borg check --repair --verify-data, which works for this case.

Why the pack is kept

I looked through the PR and issue comments, review comments and commit messages of the last 3 months. The only reason given is in the salvage plan on #10026 (comment):

If no object in the pack authenticates, leave the pack and report it. This matches the rebuild's refusal when not one object validates.

That rule wasn't one of the plan's "(decide)" points and was not discussed. #10455 describes the behaviour but gives no reason.

The rebuild guard it refers to (#10094, now in build_chunkindex_from_repo) protects against a buggy validator or a key that does not belong to the packs. Either would reject every object in every pack, and the repair would then empty the chunk list of every archive. That risk is repository-wide, not per pack:

• salvage_pack uses the same authenticate callable for every pack in the run, and borg check loads the key before it checks anything. If other objects authenticated in the same run, a broken authenticator is not a plausible explanation for one pack failing completely.
• A salvage only runs on packs that already failed their store hash, so the pack is known to be damaged.
• The "a second read must also fail" rule from borg2 check: implement repository repair (index and pack rebuild) #10026 is not applied in this case: return unchanged(SALVAGE_NOTHING_AUTHENTICATES) comes before the second store.load(). Keeping the pack does not guard against a flaky read here. It just means nothing is ever done about the pack.

Suggested fix

Treat a corrupt pack in which no object authenticates like a salvage that keeps zero objects: delete the pack and remove its index entries, reported like any other lossy salvage. Do that only if:

• a second load returns the same bytes, as for the other salvage outcomes, and
• other objects authenticated with the same authenticator in this run, so that a broken authenticator or wrong key still leaves everything untouched, as the rebuild guard does.

If the pack should be kept anyway, compact's hint and the check() comment should at least say that borg check --repair --verify-data is needed.

Refs #10026.