borgbackup/borg

review "assert" usage

Open

#8,649 opened on Jan 21, 2025

View on GitHub
 (17 comments) (0 reactions) (1 assignee)Python (732 forks)batch import
bughelp wantedsecurity

Repository metrics

Stars
 (10,086 stars)
PR merge metrics
 (Avg merge 5d 12h) (91 merged PRs in 30d)

Description

See there for the problem description:

https://community.sonarsource.com/t/feature-python-assert-should-be-consider-harmful/38501

TL;DR: for now, do not run borg via python3 -O or with PYTHONOPTIMIZE set.

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. it must never be used if not executing the assert would influence correctness or security.
  • 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.

In case we find a lot of places to fix, a quick workaround for 1.4-maint branch could be to disallow running borg with assertions switched off, something like:

+try:
+    assert False
+except AssertionError:
+    pass  # OK
+else:
+    print("Borg requires working assertions. Please run Python without -O and/or unset PYTHONOPTIMIZE")
+    sys.exit(2)

Note: 2 is the classic error code for a fatal error, but borg 1.4.x also supports modern exit codes, so an appropriate one (2 or more specific) needs to be returned for that.

Contributor guide