Repository navigation
replace asserts on external data by real checks, refs #8649 - #10522
Open
ThomasWaldmann wants to merge 6 commits into
Open
ThomasWaldmann wants to merge 6 commits into
ThomasWaldmann wants to merge 6 commits into
Conversation
…orgbackup#8649 The borg 1.x index file is data from disk, so do not validate it with assert. LegacyRepository.prepare_txn catches ValueError and rebuilds the index, an AssertionError skipped that recovery. From PR borgbackup#8682. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
borgbackup#8649 The item.pyx fix_*/want_* helpers and update_internal() methods validate msgpack data from the repository (items, archive metadata, legacy manifests), key envelopes and decrypted keys: raise IntegrityError instead of asserting. borg check's robust item iterator now reports such an item and continues. RepoObj.parse: an inconsistent csize or an invalid psize in the object metadata raises IntegrityError. compress: a decompressed size that does not match the size in the metadata and the borg 1.x-only formats (legacy zlib, obfuscation header) in a borg 2 object raise DecompressionError. repo-compress and debug format-obj check the object type they pass to RepoObj.format (stored object metadata / user supplied JSON). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…, refs borgbackup#8649 The chunk sizes in an archive's chunk lists are repository data: - DownloadPipeline.fetch_many: a size that does not match the chunk data or that is invalid for a replacement chunk raises IntegrityError. - seen_chunk / reuse_chunk: invalid or inconsistent sizes raise IntegrityError. - files cache: compress_entry raises ValueError for invalid or inconsistent sizes, building the files cache from the previous archive skips such entries (the files cache is only an optimization). Archive metadata must have exactly one of item_ptrs / items, check that where it is loaded. Archives._parse_archive_meta treats invalid metadata like corrupted metadata, so one bad archive does not break listing it. import-tar: a BORG tar member with an unsupported BORG.item.version gets a warning and status "E" and is skipped. CacheConfig.create: do not fail if a concurrently running borg already created the cache config. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…up#8649 storelocking: _parse_lock checks the types of the lock owner fields and that the timestamp has a timezone. A malformed (but authenticated) lock is treated as unreadable instead of crashing in platform.process_alive(), so borg break-lock can remove it. release() / migrate_lock() select the lock object we hold (my_lock_key) instead of asserting that only one lock with our process id exists: a not yet stale leftover of a dead process with the same id (pid reuse, e.g. in containers) made them fail. migrate_lock raises LockTimeout if our lock was killed, like refresh does. fslocking: a lock roster with invalid entries is treated as corrupt (empty), like a roster that is not valid JSON. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…gbackup#8649 key: - an empty borg 1.x manifest raises IntegrityError. - repo-create --copy-crypt-key: a crypt key of the other repository that the new encryption mode can not use (e.g. the 32 + 128 bytes key of a borg 1.x blake2 mode for authenticated-sha256) raises Error. - legacy AES-CTR keys: the key class is selected by the unauthenticated type byte of the borg 1.x manifest, so check that the key material fits the crypto suite (IntegrityError). - loading a key with invalid crypt_key / id_key lengths raises KeyfileInvalidError. legacy: - borg serve --from-borg1: if the client closes the connection without calling close() (borg 1.x clients do that), close the repository like borg 1.4 did instead of failing with an AssertionError. - the legacy remote checks that "get" results from a borg 1.x server are bytes. - transfer: borg 1.x items with None values or without the required keys raise IntegrityError. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…8649 - ChunkerParams: validate list/tuple input (e.g. a YAML list from a config file) like the string form. buzhash: require window_size >= 1 and window_size + 2^chunk_min + 1 <= 2^chunk_max, buzhash64: require window_size >= 1. Such params made the chunker assert, crash (SIGBUS) or hang. - FilesCacheMode: reject a size-only mode, the files cache needs ctime or mtime (or disabled). - create: the slashdot hack pointing to the root directory (//./, ///./) works like /./ instead of failing with an AssertionError. - recreate without --target: check the archive names from the repository before changing anything. - benchmark crud: a failing inner borg command raises CommandError. - broken version metadata raises RuntimeError. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #10522 +/- ##
==========================================
- Coverage 89.32% 88.46% -0.86%
==========================================
Files 104 104
Lines 20079 20178 +99
Branches 3152 3199 +47
==========================================
- Hits 17936 17851 -85
- Misses 1484 1668 +184
Partials 659 659 ☔ View full report in Codecov by Harness. |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Applies the "Using assert" rules from
docs/development.rst(#10521) to the code base.All 342 non-test
assertstatements were reviewed. Most check internal invariants and stay. The ones that check data that can be bad without a borg bug now raise proper exceptions, at the boundary where the data enters where that is practical.Repository data (
IntegrityError/DecompressionError)item.pyx: thefix_*/want_*helpers andupdate_internal()(items, archive metadata, legacy manifests, key envelopes, keys).borg checknow reports a malformed item and continues.RepoObj.parse: csize / psize.compress: a decompressed size that does not match the metadata; the borg 1.x-only formats (legacy zlib, obfuscation header) in a borg 2 object.fetch_many,seen_chunk,reuse_chunk, and the files cache, which skips such entries.item_ptrs/items. One bad archive does not breakrepo-list.repo-compress: invalid object type.Locks
process_alive()asserted on it andbreak-lockcould not remove it.release()/migrate_lock()use the lock we hold (my_lock_key). Before, they asserted, so a not yet stale leftover lock with our process id (pid reuse, e.g. in containers) made them fail.Keys and borg 1.x data
repo-create --copy-crypt-keywith a crypt key the new mode can not use, e.g. borg 1.x blake2 →authenticated-sha256: properErrorinstead of an AssertionError.serve --from-borg1: closes the repository when a borg 1.x client disconnects without calling close(), like borg 1.4 did. This removes the corresponding workaround in the test.User input
ChunkerParams: validate list input (YAML from a config file), and the missing buzhash / buzhash64 window checks. Such params made the chunker assert, crash with SIGBUS or hang.--files-cache=sizeis rejected by the parser. It used to pass the parser and then fail at an assert in the cache. borg 1.x supports size-only mode; supporting it in borg 2 would need a cache change, so that is left for a separate decision.create //./andcreate ///./work like/./.--targetchecks the archive names first.import-tar: an unsupportedBORG.item.versiongives a warning and skips the member.benchmark crud: a failing inner command raisesCommandError.RuntimeError.Tests
Each change has tests. Full suite locally on macOS: 3834 passed, 1 failed. The failure is
test_migrate_lock_alive, a FUSE mount segfault that fails the same way on unmodified master (local macFUSE on macOS 27).Supersedes #8682.
🤖 Generated with Claude Code