From b39558ea1f1121f91dda1ae73c980c30b5135182 Mon Sep 17 00:00:00 2001 From: Houkime <> Date: Mon, 18 Mar 2024 17:00:55 +0000 Subject: [PATCH 1/5] fix(backups): report error in the error field of the job --- selfprivacy_api/backup/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/selfprivacy_api/backup/__init__.py b/selfprivacy_api/backup/__init__.py index e4b5db7..bf111aa 100644 --- a/selfprivacy_api/backup/__init__.py +++ b/selfprivacy_api/backup/__init__.py @@ -259,7 +259,7 @@ class Backups: Backups._prune_auto_snaps(service) service.post_restore() except Exception as error: - Jobs.update(job, status=JobStatus.ERROR, status_text=str(error)) + Jobs.update(job, status=JobStatus.ERROR, error=str(error)) raise error Jobs.update(job, status=JobStatus.FINISHED) From b36701e31cd6180f0a74ea3cb16e5faec2648752 Mon Sep 17 00:00:00 2001 From: Houkime <> Date: Mon, 18 Mar 2024 17:11:27 +0000 Subject: [PATCH 2/5] style(api): enable pydantic support in mypy --- .mypy.ini | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 .mypy.ini diff --git a/.mypy.ini b/.mypy.ini new file mode 100644 index 0000000..ff37064 --- /dev/null +++ b/.mypy.ini @@ -0,0 +1,2 @@ +[mypy] +plugins = pydantic.mypy From b40df670f8f32ce7f456567c719fb99626843af4 Mon Sep 17 00:00:00 2001 From: Houkime <> Date: Mon, 18 Mar 2024 17:13:06 +0000 Subject: [PATCH 3/5] fix(backups): censor out keys from error messages We do not have any automated sending of errors to Selfprivacy but it was inconvenient for people who want to send a screenshot of their error. --- .../backup/backuppers/restic_backupper.py | 17 +++- tests/test_backup.py | 88 ++++++++++++++++++- 2 files changed, 102 insertions(+), 3 deletions(-) diff --git a/selfprivacy_api/backup/backuppers/restic_backupper.py b/selfprivacy_api/backup/backuppers/restic_backupper.py index 534b92a..9232f60 100644 --- a/selfprivacy_api/backup/backuppers/restic_backupper.py +++ b/selfprivacy_api/backup/backuppers/restic_backupper.py @@ -172,6 +172,21 @@ class ResticBackupper(AbstractBackupper): return messages + @staticmethod + def _replace_in_array(array: List[str], target, replacement) -> None: + if target == "": + return + + for i, value in enumerate(array): + if target in value: + array[i] = array[i].replace(target, replacement) + + def _censor_command(self, command: List[str]) -> List[str]: + result = command.copy() + ResticBackupper._replace_in_array(result, self.key, "CENSORED") + ResticBackupper._replace_in_array(result, LocalBackupSecret.get(), "CENSORED") + return result + @staticmethod def _get_backup_job(service_name: str) -> Optional[Job]: service = get_service_by_id(service_name) @@ -218,7 +233,7 @@ class ResticBackupper(AbstractBackupper): "Could not create a snapshot: ", str(error), "command: ", - backup_command, + self._censor_command(backup_command), ) from error @staticmethod diff --git a/tests/test_backup.py b/tests/test_backup.py index 0a2b3ed..55741cc 100644 --- a/tests/test_backup.py +++ b/tests/test_backup.py @@ -14,13 +14,14 @@ from selfprivacy_api.utils.huey import huey from selfprivacy_api.services.service import ServiceStatus -from selfprivacy_api.graphql.queries.providers import BackupProvider +from selfprivacy_api.graphql.queries.providers import BackupProvider as ProviderEnum from selfprivacy_api.graphql.common_types.backup import ( RestoreStrategy, BackupReason, ) +from selfprivacy_api.graphql.queries.providers import BackupProvider -from selfprivacy_api.jobs import Jobs, JobStatus +from selfprivacy_api.jobs import Job, Jobs, JobStatus from selfprivacy_api.models.backup.snapshot import Snapshot @@ -38,6 +39,8 @@ from selfprivacy_api.backup.tasks import ( reload_snapshot_cache, ) from selfprivacy_api.backup.storage import Storage +from selfprivacy_api.backup.local_secret import LocalBackupSecret +from selfprivacy_api.backup.jobs import get_backup_fail REPO_NAME = "test_backup" @@ -188,6 +191,87 @@ def test_backup_service(dummy_service, backups): assert_job_finished(f"services.{id}.backup", count=1) +def all_job_text(job: Job) -> str: + # Use when we update to pydantic 2.xxx + # return Job.model_dump_json() + result = "" + if job.status_text is not None: + result += job.status_text + if job.description is not None: + result += job.description + if job.error is not None: + result += job.error + + return result + + +def assert_job_errored(job: Job): + assert job is not None + assert job.status == JobStatus.ERROR + + # consider adding a useful error message to an errored-out job + assert job.error is not None + assert job.error != "" + + +def test_error_censoring_encryptionkey(dummy_service, backups): + # Discard our key to inject a failure + old_key = LocalBackupSecret.get() + LocalBackupSecret.reset() + new_key = LocalBackupSecret.get() + + with pytest.raises(ValueError): + # Should fail without correct key + Backups.back_up(dummy_service) + + job = get_backup_fail(dummy_service) + assert_job_errored(job) + + job_text = all_job_text(job) + + assert old_key not in job_text + assert new_key not in job_text + # local backups do not have login key + # assert Backups.provider().key not in job_text + + assert "CENSORED" in job_text + + +def test_error_censoring_loginkey(dummy_service, backups, fp): + # We do not want to screw up our teardown + old_provider = Backups.provider() + + secret = "aSecretNYA" + + Backups.set_provider( + BackupProvider.BACKBLAZE, login="meow", key=secret, location="moon" + ) + assert Backups.provider().key == secret + + # We could have called real backblaze but it is kind of not privacy so. + fp.allow_unregistered(True) + fp.register( + ["restic", fp.any()], + returncode=1, + stdout="only real cats are allowed", + # We do not want to suddenly call real backblaze even if code changes + occurrences=100, + ) + + with pytest.raises(ValueError): + Backups.back_up(dummy_service) + + job = get_backup_fail(dummy_service) + assert_job_errored(job) + + job_text = all_job_text(job) + assert secret not in job_text + assert job_text.count("CENSORED") == 2 + + # We do not want to screw up our teardown + Storage.store_provider(old_provider) + + def test_no_repo(memory_backup): with pytest.raises(ValueError): assert memory_backup.backupper.get_snapshots() == [] From c5b227226c9d1d9ed0b79f328c0336c563b8ee2b Mon Sep 17 00:00:00 2001 From: Houkime <> Date: Mon, 18 Mar 2024 17:32:18 +0000 Subject: [PATCH 4/5] fix(backups): do not rely on obscure behaviour --- selfprivacy_api/backup/providers/__init__.py | 2 ++ tests/test_backup.py | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/selfprivacy_api/backup/providers/__init__.py b/selfprivacy_api/backup/providers/__init__.py index 4f8bb75..8cb2a74 100644 --- a/selfprivacy_api/backup/providers/__init__.py +++ b/selfprivacy_api/backup/providers/__init__.py @@ -21,6 +21,8 @@ PROVIDER_MAPPING: dict[BackupProviderEnum, Type[AbstractBackupProvider]] = { def get_provider( provider_type: BackupProviderEnum, ) -> Type[AbstractBackupProvider]: + if provider_type not in PROVIDER_MAPPING.keys(): + raise LookupError("could not look up provider", provider_type) return PROVIDER_MAPPING[provider_type] diff --git a/tests/test_backup.py b/tests/test_backup.py index 55741cc..b96f8e7 100644 --- a/tests/test_backup.py +++ b/tests/test_backup.py @@ -244,7 +244,7 @@ def test_error_censoring_loginkey(dummy_service, backups, fp): secret = "aSecretNYA" Backups.set_provider( - BackupProvider.BACKBLAZE, login="meow", key=secret, location="moon" + ProviderEnum.BACKBLAZE, login="meow", key=secret, location="moon" ) assert Backups.provider().key == secret From 28556bd22dcb050c24a400090ca6c22261d8afdc Mon Sep 17 00:00:00 2001 From: Houkime <> Date: Mon, 18 Mar 2024 17:40:48 +0000 Subject: [PATCH 5/5] test(backups): move errored job checker into common test utils --- tests/common.py | 11 +++++++++++ tests/test_backup.py | 11 ++--------- 2 files changed, 13 insertions(+), 9 deletions(-) diff --git a/tests/common.py b/tests/common.py index 09a9cd5..5f69f3f 100644 --- a/tests/common.py +++ b/tests/common.py @@ -2,6 +2,8 @@ import json from datetime import datetime, timezone, timedelta from mnemonic import Mnemonic +from selfprivacy_api.jobs import Job, JobStatus + # for expiration tests. If headache, consider freezegun RECOVERY_KEY_VALIDATION_DATETIME = "selfprivacy_api.models.tokens.time.datetime" DEVICE_KEY_VALIDATION_DATETIME = RECOVERY_KEY_VALIDATION_DATETIME @@ -79,3 +81,12 @@ def assert_recovery_recent(time_generated: str): assert datetime.fromisoformat(time_generated) - timedelta(seconds=5) < datetime.now( timezone.utc ) + + +def assert_job_errored(job: Job): + assert job is not None + assert job.status == JobStatus.ERROR + + # consider adding a useful error message to an errored-out job + assert job.error is not None + assert job.error != "" diff --git a/tests/test_backup.py b/tests/test_backup.py index b96f8e7..4543d33 100644 --- a/tests/test_backup.py +++ b/tests/test_backup.py @@ -42,6 +42,8 @@ from selfprivacy_api.backup.storage import Storage from selfprivacy_api.backup.local_secret import LocalBackupSecret from selfprivacy_api.backup.jobs import get_backup_fail +from tests.common import assert_job_errored + REPO_NAME = "test_backup" @@ -205,15 +207,6 @@ def all_job_text(job: Job) -> str: return result -def assert_job_errored(job: Job): - assert job is not None - assert job.status == JobStatus.ERROR - - # consider adding a useful error message to an errored-out job - assert job.error is not None - assert job.error != "" - - def test_error_censoring_encryptionkey(dummy_service, backups): # Discard our key to inject a failure old_key = LocalBackupSecret.get()