Skip to content

check --repair: resync past corrupt object headers when rebuilding the chunks index - #10094

Open
mr-raj12 wants to merge 1 commit into
borgbackup:masterfrom
mr-raj12:pack-resync-followup-8476
Open

check --repair: resync past corrupt object headers when rebuilding the chunks index#10094
mr-raj12 wants to merge 1 commit into
borgbackup:masterfrom
mr-raj12:pack-resync-followup-8476

Conversation

@mr-raj12

@mr-raj12 mr-raj12 commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

Follow-up to #10083 (header validation, #8476).

When borg check --repair rebuilds the chunks index from the packs, a corrupt object header no longer aborts the whole pack. iter_headers(validate=...) scans forward for the next object, confirms it by authenticating it with the key, and resumes there, so the objects after the damaged region are still indexed. The skipped byte range is logged.

Without a validate function (repository-only check, routine rebuild) the behavior is unchanged: a corrupt header raises IntegrityError.

Details:

  • _find_header searches for OBJ_MAGIC a window at a time. OBJ_MAGIC also occurs inside payloads, so a candidate is accepted only when its header parses and validate confirms it.
  • resync_validator builds validate from the key: AEAD keys authenticate by decrypting the metadata (header + meta, a few hundred bytes); keys with id_check_is_authentication need the blob data and check chunk_id == id_hash(content).
  • docs/internals/packs.rst updated to describe the validation and the repair scan.

Tested in repository_test, cache_test and check_cmd_test.

@codecov

codecov Bot commented Aug 12, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 95.38462% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 85.61%. Comparing base (0ccf95b) to head (cc0d2c0).
⚠️ Report is 1 commits behind head on master.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
src/borg/repository.py 93.33% 1 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master   #10094      +/-   ##
==========================================
- Coverage   86.82%   85.61%   -1.21%     
==========================================
  Files          99       99              
  Lines       17342    17398      +56     
  Branches     2631     2642      +11     
==========================================
- Hits        15057    14896     -161     
- Misses       1589     1792     +203     
- Partials      696      710      +14     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

@mr-raj12

Copy link
Copy Markdown
Contributor Author

Does #10092 (comment) apply here too?

validate.needs_data is what it costs: AEAD keys authenticate a candidate from header+meta, the other keys read the whole object and get the id check from parse(). Removing it drops that attribute, makes _find_header always slice hdr_size + meta_size, and inverts one test. Four lines.

The consequence is in authenticated mode, where an object stored verbatim inside a backed up file looks exactly like a real one, so the scan indexes it. If its chunk id matches an object living elsewhere in the repo, the rebuilt index points into the file and the real object becomes unreachable. check --repair is an unfortunate place for that, though it is only modes you want gone anyway.

Either is fine by me, just say which.

@mr-raj12
mr-raj12 force-pushed the pack-resync-followup-8476 branch 2 times, most recently from a6533ae to 1877f65 Compare August 13, 2026 08:48
@mr-raj12 mr-raj12 changed the title check --repair: resync to the next authenticated object in a corrupt pack check --repair: resync past corrupt object headers when rebuilding the chunks index Aug 13, 2026
…e chunks index, borgbackup#8476

When check --repair rebuilds the chunks index from the packs, a corrupt object
header now makes iter_headers resync rather than raise: it takes a validate
function and scans forward for the next object, in 1 MiB windows that overlap by
one header so a header on a window boundary is still found. Repository-only
checks pass no validate and keep raising IntegrityError on a corrupt header.

OBJ_MAGIC also occurs inside payloads, so a candidate is accepted only when it
authenticates. For AEAD keys, decrypting the metadata authenticates it against
the header's magic, version and chunk_id, so the walk confirms a chunk id from a
few hundred bytes. Keys that authenticate by chunk_id == id_hash(content)
(id_check_is_authentication) read the whole object and parse() at the "repair"
id place; validate.needs_data selects between the two.

Authentication needs the key, so check --repair makes it before the rebuild with
manifest_only=True. A repair that cannot read the manifest has no key and walks
without resyncing.
@mr-raj12
mr-raj12 force-pushed the pack-resync-followup-8476 branch from 1877f65 to cc0d2c0 Compare August 13, 2026 08:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant