stop validating untrusted/persisted data with assert

[ThomasWaldmann]

From a global code review of the borg2 codebase (2026-08). Original scope corrected in the comments below: the -O startup guard already exists (#8649), so this issue is now only about not using assert to validate untrusted/persisted data.

Near-duplicate of #8649 ("review assert usage") — same policy, this issue just names the concrete untrusted-data places. See also the pending draft PR #8682. Close as duplicate if you prefer to track it all in #8649.

Already done (not part of this issue)

src/borg/archiver/__init__.py refuses to start when assertions are disabled (13b7e1f, #8649): it probes assert False at import time and exits with rc 2. That covers -O, -OO and PYTHONOPTIMIZE, for the borg/borgfs scripts, python -m borg and the PyInstaller binary — and, since -OO implies -O, it also covers the docstring-derived error messages (helpers/errors.py: get_message() formats type(self).__doc__) and argparse descriptions built from do_*.__doc__.

Problem

Non-test code contains ~175 assert statements. Some of them are not internal invariants but validation of persisted/untrusted data:

• the fix_* converters in src/borg/item.pyx (checking types/shapes of unpacked msgpack data),
• the key-length check in item.pyx (assert len(k) in (32 + 32, 32 + 128)),
• the size check in DownloadPipeline.fetch_many.

With the startup guard in place these no longer vanish silently, so this is not a safety-net hole anymore — but corrupt or malicious repo data still surfaces as a bare AssertionError traceback instead of a proper borg error with a sensible exit code.

Proposed direction

1. Convert asserts that check untrusted/persisted data (item decoding, envelope parsing, sizes from the repo) into real exceptions (IntegrityError/ValueError); keep assert for genuine internal invariants.
2. Add a test for the startup guard so it cannot be removed unnoticed.
3. Optionally: the guard lives in borg.archiver, so importing e.g. borg.item alone under -O is unguarded — only relevant for library-style use.
4. Optionally: move error message templates from docstrings to a normal class attribute so -OO has one less failure mode in principle.

[ThomasWaldmann]

Correction: point 1 is already implemented — my claim that "nothing enforces it" was wrong.

src/borg/archiver/__init__.py has had this guard since 13b7e1f (2025-04-21, #8649, changelog entry present):

# quickfix to disallow running borg with assertions switched off
try:
    assert False
except AssertionError:
    pass  # OK
else:
    print("Borg requires working assertions. ...", file=sys.stderr)
    sys.exit(2)  # == EXIT_ERROR

Verified it fires: python -O -m borg --version, python -OO -c "import borg.archiver" and PYTHONOPTIMIZE=1 python -m borg --version all print the message and exit 2. It is module-level, so it covers the borg/borgfs console scripts, python -m borg and the PyInstaller binary. Because -OO implies -O, the docstring-based error messages (helpers/errors.py get_message()) and the argparse do_*.__doc__ descriptions are covered by the same guard, so point 3 is not a live failure mode either.

What remains of this issue:

• Untrusted-data asserts (point 2) — still valid, but as hardening/error-taxonomy rather than a vanishing safety net: the fix_* converters in item.pyx (types/shapes of unpacked msgpack), the key-length check at item.pyx:487, and the DownloadPipeline.fetch_many size check validate persisted data with assert. Corrupt input therefore surfaces as an AssertionError traceback instead of a proper IntegrityError/ValueError with a borg exit code.
• Two minor residuals: no test covers the guard, and the guard lives in borg.archiver, so python -O -c "import borg.item" is unguarded (only relevant for library-style use).

Retitling/rescoping accordingly.

[ThomasWaldmann]

This is a near-duplicate of the older #8649 ("review assert usage", 2025-01-21, still open), which already states the policy and the remaining task:

In the code, assert should be only used for: our test suite; at places in production code where it doesn't really matter if the assert is executed or not [...]; if it matters, the assert should be replaced by if <condition>: raise SomeException

So, the task for borg master branch is to do a systematic review and fix all problematic asserts.

#8649 is also where the -O startup guard came from — that was the explicitly-labelled quickfix there (PR #8685 for master, earlier for 1.4-maint), with the systematic review left open. There is a pending draft PR #8682 ("bugfix : review assert and replace by if condition", hashindex.pyx / platform/posix.pyx).

So what is left in this issue is a subset of #8649: concrete places where the asserted condition validates persisted/untrusted repo data rather than an internal invariant (item.pyx fix_* converters, the key-length check, DownloadPipeline.fetch_many). Feel free to close this as a duplicate and keep the list here as a starting point for #8649 — or keep it as the narrower, actionable slice, whichever you prefer.