Make password decryption with broken keys more reliable.

Review Request #14855 — Created Feb. 25, 2026 and submitted

Information

Review Board
release-7.x

Reviewers

I've been seeing inconsistent results for
RepositoryTests.test_password_decryption_failed, which tests that we
don't crash when the SECRET_KEY has been mistakenly changed and stored
passwords cannot be decrypted. While this works most of the time by
properly catching the decode error, occasionally the random IV used
during AES-CFB causes the decoded result to be valid(ish) unicode. These
almost always result in unprintable characters, however.

This change adds a step to decrypt_password to check if the result is
printable, and if not, raise a ValueError. This should make it so this
test (and handling of the situation in production) is much more
reliable.

Ran unit tests.

Summary ID
Make password decryption with broken keys more reliable.
I've been seeing inconsistent results for 'RepositoryTests.test_password_decryption_failed`, which tests that we don't crash when the `SECRET_KEY` has been mistakenly changed and stored passwords cannot be decrypted. While this works *most* of the time by properly catching the decode error, occasionally the random IV used during AES-CFB causes the decoded result to be valid(ish) unicode. These almost always result in unprintable characters, however. This change adds a step to `decrypt_password` to check if the result is printable, and if not, raise a `ValueError`. This should make it so this test (and handling of the situation in production) is much more reliable. Testing Done: Ran unit tests.
a109fe2e1964d1c2f065107a32b4cdcdd39755b3
Description From Last Updated

Won't this ValueError message just get swallowed when we catch it in Repository.password. Should we pass the error message to …

maubinmaubin
maubin
  1. 
      
  2. Show all issues

    Won't this ValueError message just get swallowed when we catch it in Repository.password. Should we pass the error message to the log there?

    1. It's true it will get swallowed there, but we have special handling around that that provides errors to the user. decrypt_password is used in a bunch of other places, though.

  3. 
      
maubin
  1. Ship It!
  2. 
      
david
Review request changed
Status:
Completed
Change Summary:
Pushed to release-7.x (a9285c8)