From 31c2fcf37733c965f663958f7331b090ba5bf7ea Mon Sep 17 00:00:00 2001 From: Stefan Agner Date: Tue, 10 Mar 2026 09:53:43 +0100 Subject: [PATCH] Treat empty string password as None in backup restore (#6618) * Treat empty string password as None in backup restore Work around a securetar 2026.2.0 bug where an empty string password sets encrypted=True but fails to derive a key, leading to an AttributeError on restore. This also restores consistency with backup creation which uses a truthiness check to skip encryption for empty passwords. Co-Authored-By: Claude Opus 4.6 * Explicitly mention that "" is treated as no password * Add tests for empty string password handling in backups Verify that empty string password is treated as no password on both backup creation (not marked as protected) and restore (normalized to None in set_password). Co-Authored-By: Claude Opus 4.6 * Improve comment --------- Co-authored-by: Claude Opus 4.6 --- supervisor/backups/backup.py | 12 ++++++--- tests/backups/test_backup.py | 48 ++++++++++++++++++++++++++++++++++++ 2 files changed, 57 insertions(+), 3 deletions(-) diff --git a/supervisor/backups/backup.py b/supervisor/backups/backup.py index a68c49ba1..e5b065816 100644 --- a/supervisor/backups/backup.py +++ b/supervisor/backups/backup.py @@ -320,7 +320,8 @@ class Backup(JobGroup): # Add defaults self._data = SCHEMA_BACKUP(self._data) - # Set password + # Set password - intentionally using truthiness check so that empty + # string is treated as no password, consistent with set_password(). if password: self._password = password self._data[ATTR_PROTECTED] = True @@ -331,8 +332,13 @@ class Backup(JobGroup): self._data[ATTR_COMPRESSED] = False def set_password(self, password: str | None) -> None: - """Set the password for an existing backup.""" - self._password = password + """Set the password for an existing backup. + + Treat empty string as None to stay consistent with backup creation + and Supervisor behavior before #6402, independent of SecureTar + behavior in this regard. + """ + self._password = password or None async def validate_backup(self, location: str | None) -> None: """Validate backup. diff --git a/tests/backups/test_backup.py b/tests/backups/test_backup.py index ef358e53c..8ec71cde9 100644 --- a/tests/backups/test_backup.py +++ b/tests/backups/test_backup.py @@ -310,6 +310,12 @@ async def test_validate_backup( BackupInvalidError, match="Invalid password for backup f92f0339" ), ), + ( + "", + pytest.raises( + BackupInvalidError, match="Invalid password for backup f92f0339" + ), + ), ], ) async def test_validate_backup_v3( @@ -333,3 +339,45 @@ async def test_validate_backup_v3( with expected_exception: await v3_backup.validate_backup(None) + + +@pytest.mark.parametrize( + ("password", "expect_protected"), + [ + ("my_password", True), + (None, False), + ("", False), + ], +) +async def test_new_backup_empty_password_not_protected( + coresys: CoreSys, + tmp_path: Path, + password: str | None, + expect_protected: bool, +): + """Test that empty string password is treated as no password on backup creation.""" + backup = Backup(coresys, tmp_path / "my_backup.tar", "test", None) + backup.new( + "test", "2023-07-21T21:05:00.000000+00:00", BackupType.FULL, password=password + ) + assert backup.protected is expect_protected + + +@pytest.mark.parametrize( + ("password", "expected_password"), + [ + ("my_password", "my_password"), + (None, None), + ("", None), + ], +) +def test_set_password_empty_string_is_none( + coresys: CoreSys, + tmp_path: Path, + password: str | None, + expected_password: str | None, +): + """Test that set_password treats empty string as None.""" + backup = Backup(coresys, tmp_path / "my_backup.tar", "test", None) + backup.set_password(password) + assert backup._password == expected_password # pylint: disable=protected-access