• 
      

    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)