diff --git a/dangerzone/cli.py b/dangerzone/cli.py index d5699dd..91ac7da 100644 --- a/dangerzone/cli.py +++ b/dangerzone/cli.py @@ -117,7 +117,7 @@ def cli_main( sys.exit(1) # Ensure container is installed - should_upgrade = bool(settings.get("updater_check_all")) + should_upgrade = bool(settings.get("updater_container_needs_update")) dangerzone.isolation_provider.install(should_upgrade) # Convert the document diff --git a/dangerzone/gui/main_window.py b/dangerzone/gui/main_window.py index 1470efe..05d6aa6 100644 --- a/dangerzone/gui/main_window.py +++ b/dangerzone/gui/main_window.py @@ -29,7 +29,7 @@ else: from .. import errors from ..document import SAFE_EXTENSION, Document from ..isolation_provider.qubes import is_qubes_native_conversion -from ..updater.releases import UpdateReport +from ..updater.releases import UpdaterReport from ..util import format_exception, get_resource_path, get_version from .logic import Alert, CollapsibleBox, DangerzoneGui, UpdateDialog @@ -327,14 +327,14 @@ class MainWindow(QtWidgets.QMainWindow): ) ) - def handle_updates(self, report: UpdateReport) -> None: + def handle_updates(self, report: UpdaterReport) -> None: """Handle update reports from the update checker thread. - See Updater.check_for_updates() to find the different types of reports that it + See UpdaterReport to find the different types of reports that it may send back, depending on the outcome of an update check. """ # If there are no new updates, reset the error counter (if any) and return. - if report.empty(): + if report.is_empty: self.dangerzone.settings.set("updater_errors", 0, autosave=True) return @@ -376,34 +376,46 @@ class MainWindow(QtWidgets.QMainWindow): hamburger_menu.insertAction(sep, error_action) else: log.debug(f"Handling new version: {report.version}") - self.dangerzone.settings.set("updater_latest_version", report.version) - self.dangerzone.settings.set("updater_latest_changelog", report.changelog) self.dangerzone.settings.set("updater_errors", 0) + if report.new_github_release: + log.debug(f"New Dangerzone release: {report.version}") + self.dangerzone.settings.set("updater_latest_version", report.version) + self.dangerzone.settings.set( + "updater_latest_changelog", report.changelog + ) + self.hamburger_button.setIcon( + QtGui.QIcon( + load_svg_image( + "hamburger_menu_update_success.svg", width=64, height=64 + ) + ) + ) + + sep = hamburger_menu.insertSeparator(hamburger_menu.actions()[0]) + success_action = QAction("New version available", hamburger_menu) + success_action.setIcon( + QtGui.QIcon( + load_svg_image( + "hamburger_menu_update_dot_available.svg", + width=64, + height=64, + ) + ) + ) + success_action.triggered.connect(self.show_update_success) + hamburger_menu.insertAction(sep, success_action) + + if report.new_container_release: + log.debug(f"New container image release available") + self.dangerzone.settings.set( + "updater_container_needs_update", + report.container_needs_update, + ) # FIXME: Save the settings to the filesystem only when they have really changed, # maybe with a dirty bit. self.dangerzone.settings.save() - self.hamburger_button.setIcon( - QtGui.QIcon( - load_svg_image( - "hamburger_menu_update_success.svg", width=64, height=64 - ) - ) - ) - - sep = hamburger_menu.insertSeparator(hamburger_menu.actions()[0]) - success_action = QAction("New version available", hamburger_menu) - success_action.setIcon( - QtGui.QIcon( - load_svg_image( - "hamburger_menu_update_dot_available.svg", width=64, height=64 - ) - ) - ) - success_action.triggered.connect(self.show_update_success) - hamburger_menu.insertAction(sep, success_action) - def register_update_handler(self, signal: QtCore.SignalInstance) -> None: signal.connect(self.handle_updates) diff --git a/dangerzone/gui/updater.py b/dangerzone/gui/updater.py index ece2619..095a6b5 100644 --- a/dangerzone/gui/updater.py +++ b/dangerzone/gui/updater.py @@ -77,7 +77,7 @@ class UpdaterThread(QtCore.QThread): When finished, this thread triggers a signal with the results. """ - finished = QtCore.Signal(releases.UpdateReport) + finished = QtCore.Signal(releases.UpdaterReport) def __init__(self, dangerzone: DangerzoneGui): super().__init__() @@ -99,7 +99,7 @@ class UpdaterThread(QtCore.QThread): def should_check_for_updates(self) -> bool: try: - should_check: Optional[bool] = releases.should_check_for_releases( + should_check: Optional[bool] = releases.should_check_for_updates( self.dangerzone.settings ) except errors.NeedUserInput: diff --git a/dangerzone/isolation_provider/container.py b/dangerzone/isolation_provider/container.py index a1a3eb6..b2befde 100644 --- a/dangerzone/isolation_provider/container.py +++ b/dangerzone/isolation_provider/container.py @@ -9,6 +9,7 @@ from typing import Callable, List, Optional, Tuple from .. import container_utils, errors from ..container_utils import Runtime from ..document import Document +from ..settings import Settings from ..updater import ( DEFAULT_PUBKEY_LOCATION, UpdaterError, @@ -135,6 +136,9 @@ class Container(IsolationProvider): if update_available and image_digest: log.debug("Upgrading container image to %s", image_digest) upgrade_container_image(image_digest, callback=callback) + + settings = Settings() + settings.set("updater_container_needs_update", False, autosave=True) else: log.debug("No update available for the container.") if not installed_tags: diff --git a/dangerzone/isolation_provider/report.py b/dangerzone/isolation_provider/report.py new file mode 100644 index 0000000..b807b1f --- /dev/null +++ b/dangerzone/isolation_provider/report.py @@ -0,0 +1,28 @@ +from dataclasses import dataclass + + +@dataclass +class Report: + gh_version: str | None = None + gh_changelog: str | None = None + container_needs_upgrade: bool = False + error: str | None = None + + +def do_something_for_me() -> Report: + # I want to report the following: + # 1. There were an error + raise Exception("Something happened") + report = Report() + report.gh_version = something + report.gh_changelog = changelog + report.container_needs_upgrade = True + + return report + + +if __name__ == "__main__": + try: + report = do_something_for_me() + except ReportException: + pass diff --git a/dangerzone/settings.py b/dangerzone/settings.py index 8613ce7..36f0fa5 100644 --- a/dangerzone/settings.py +++ b/dangerzone/settings.py @@ -38,6 +38,7 @@ class Settings: # FIXME: How to invalidate those if they change upstream? "updater_latest_version": get_version(), "updater_latest_changelog": "", + "updater_container_needs_update": False, "updater_errors": 0, } diff --git a/dangerzone/updater/releases.py b/dangerzone/updater/releases.py index 2c9c773..77cb012 100644 --- a/dangerzone/updater/releases.py +++ b/dangerzone/updater/releases.py @@ -2,15 +2,22 @@ import json import platform import sys import time -from typing import Optional +from dataclasses import dataclass +from typing import Optional, Tuple import markdown import requests from packaging import version -from .. import util +from .. import container_utils, util from ..settings import Settings from . import errors, log +from .signatures import ( + DEFAULT_PUBKEY_LOCATION, +) +from .signatures import ( + is_update_available as is_container_update_available, +) # Check for updates at most every 12 hours. UPDATE_CHECK_COOLDOWN_SECS = 60 * 60 * 12 @@ -21,22 +28,31 @@ GH_RELEASE_URL = ( REQ_TIMEOUT = 15 -class UpdateReport: +@dataclass +class UpdaterReport: """A report for an update check.""" - def __init__( - self, - version: Optional[str] = None, - changelog: Optional[str] = None, - error: Optional[str] = None, - ): - self.version = version - self.changelog = changelog - self.error = error + version: Optional[str] = None + changelog: Optional[str] = None + container_needs_update: Optional[bool] = None + error: Optional[str] = None - def empty(self) -> bool: + @property + def new_github_release(self) -> bool: + return self.version is not None + + @property + def new_container_release(self) -> bool: + return self.container_needs_update is True + + @property + def is_empty(self) -> bool: return self.version is None and self.changelog is None and self.error is None + @property + def is_error(self) -> bool: + return self.error is not None + def _get_now_timestamp() -> int: return int(time.time()) @@ -61,18 +77,20 @@ def ensure_sane_update(cur_version: str, latest_version: str) -> bool: if version.parse(cur_version) == version.parse(latest_version): return False elif version.parse(cur_version) > version.parse(latest_version): - # FIXME: This is a sanity check, but we should improve its wording. - raise Exception("Received version is older than the latest version") + raise Exception( + "The version received from Github Releases is older than the latest known version" + ) else: return True -def fetch_release_info() -> UpdateReport: +def fetch_github_release_info() -> Tuple[str, str]: """Get the latest release info from GitHub. Also, render the changelog from Markdown format to HTML, so that we can show it to the users. """ + log.debug("Checking the latest GitHub release") try: res = requests.get(GH_RELEASE_URL, timeout=REQ_TIMEOUT) except Exception as e: @@ -99,10 +117,11 @@ def fetch_release_info() -> UpdateReport: f"Missing required fields in JSON response from {GH_RELEASE_URL}" ) - return UpdateReport(version=version, changelog=changelog) + log.debug(f"Latest version in GitHub is {version}") + return version, changelog -def should_check_for_releases(settings: Settings) -> bool: +def should_check_for_updates(settings: Settings) -> bool: """Determine if we can check for release updates based on settings and user prefs. Note that this method only checks if the user has expressed an interest for @@ -112,7 +131,7 @@ def should_check_for_releases(settings: Settings) -> bool: * A user may have expressed that they want to learn about new updates. * A previous update check may have found out that there's a new version out. * Thus we will always show to the user the cached info about the new version, - and won't make a new update check. + and won't make a new update check. """ check = settings.get("updater_check_all") @@ -141,7 +160,7 @@ def should_check_for_releases(settings: Settings) -> bool: return True -def check_for_updates(settings: Settings) -> UpdateReport: +def check_for_updates(settings: Settings) -> UpdaterReport: """Check for updates locally and remotely. Check for updates (locally and remotely) and return a report with the findings: @@ -156,36 +175,56 @@ def check_for_updates(settings: Settings) -> UpdateReport: message. """ try: - log.debug("Checking for Dangerzone updates") + log.debug("Checking for new DZ releases and container updates") latest_version = settings.get("updater_latest_version") - if version.parse(util.get_version()) < version.parse(latest_version): + new_gh_version = version.parse(util.get_version()) < version.parse( + latest_version + ) + new_container_update = settings.get("updater_container_needs_update") + + report = UpdaterReport() + + if new_gh_version: + report.version = latest_version + report.changelog = settings.get("updater_latest_changelog") + + if new_container_update: log.debug("Determined that there is an update due to cached results") - return UpdateReport( - version=latest_version, - changelog=settings.get("updater_latest_changelog"), - ) + report.container_needs_update = new_container_update + + if not report.is_empty: + return report # If the previous check happened before the cooldown period expires, do not # check again. Else, bump the last check timestamp, before making the actual # check. This is to ensure that even failed update checks respect the cooldown # period. if _should_postpone_update_check(settings): - return UpdateReport() + return UpdaterReport() else: settings.set("updater_last_check", _get_now_timestamp(), autosave=True) - log.debug("Checking the latest GitHub release") - report = fetch_release_info() - log.debug(f"Latest version in GitHub is {report.version}") - if report.version and ensure_sane_update(latest_version, report.version): + report = UpdaterReport() + gh_version, gh_changelog = fetch_github_release_info() + if gh_version and ensure_sane_update(latest_version, gh_version): log.debug( f"Determined that there is an update due to a new GitHub version:" - f" {latest_version} < {report.version}" + f" {latest_version} < {gh_version}" ) - return report + report.version = gh_version + report.changelog = gh_changelog + + container_name = container_utils.expected_image_name() + container_needs_update, _ = is_container_update_available( + container_name, DEFAULT_PUBKEY_LOCATION + ) + report.container_needs_update = container_needs_update + + settings.set( + "updater_container_needs_update", container_needs_update, autosave=True + ) + return report - log.debug("No need to update") - return UpdateReport() except Exception as e: log.exception("Encountered an error while checking for upgrades") - return UpdateReport(error=str(e)) + return UpdaterReport(error=str(e)) diff --git a/tests/gui/test_main_window.py b/tests/gui/test_main_window.py index 4422af1..33c1dbd 100644 --- a/tests/gui/test_main_window.py +++ b/tests/gui/test_main_window.py @@ -118,24 +118,26 @@ def test_default_menu( assert updater.dangerzone.settings.get("updater_check_all") is False -def test_no_update( +def test_no_new_release( qtbot: QtBot, updater: UpdaterThread, monkeypatch: MonkeyPatch, mocker: MockerFixture, ) -> None: - """Test that when no update has been detected, the user is not alerted.""" + """Test that when no new release has been detected, the user is not alerted.""" # Check that when no update is detected, e.g., due to update cooldown, an empty # report is received that does not affect the menu entries. curtime = int(time.time()) updater.dangerzone.settings.set("updater_check_all", True) updater.dangerzone.settings.set("updater_errors", 9) updater.dangerzone.settings.set("updater_last_check", curtime) + updater.dangerzone.settings.set("updater_container_needs_update", False) expected_settings = default_updater_settings() expected_settings["updater_check_all"] = True expected_settings["updater_errors"] = 0 # errors must be cleared expected_settings["updater_last_check"] = curtime + expected_settings["updater_container_needs_update"] = False window = MainWindow(updater.dangerzone) window.register_update_handler(updater.finished) @@ -148,7 +150,7 @@ def test_no_update( # Check that the callback function gets an empty report. handle_updates_spy.assert_called_once() - assert_report_equal(handle_updates_spy.call_args.args[0], releases.UpdateReport()) + assert_report_equal(handle_updates_spy.call_args.args[0], releases.UpdaterReport()) # Check that the menu entries remain exactly the same. menu_actions_after = window.hamburger_button.menu().actions() @@ -158,7 +160,50 @@ def test_no_update( assert updater.dangerzone.settings.get_updater_settings() == expected_settings -def test_update_detected( +def test_new_container_update( + qtbot: QtBot, + updater: UpdaterThread, + monkeypatch: MonkeyPatch, + mocker: MockerFixture, +) -> None: + """Test that when a new container image is available, the user isn't alerted""" + curtime = int(time.time()) + updater.dangerzone.settings.set("updater_check_all", True) + updater.dangerzone.settings.set("updater_errors", 9) + updater.dangerzone.settings.set("updater_last_check", curtime) + updater.dangerzone.settings.set("updater_container_needs_update", True) + + window = MainWindow(updater.dangerzone) + window.register_update_handler(updater.finished) + handle_updates_spy = mocker.spy(window, "handle_updates") + + menu_actions_before = window.hamburger_button.menu().actions() + + with qtbot.waitSignal(updater.finished): + updater.start() + + # Check that the callback function gets a report with the container update + handle_updates_spy.assert_called_once() + assert_report_equal( + handle_updates_spy.call_args.args[0], + releases.UpdaterReport(container_needs_update=True), + ) + + # Check that the menu entries remain exactly the same. + menu_actions_after = window.hamburger_button.menu().actions() + assert menu_actions_before == menu_actions_after + + # Check that any previous update errors are cleared. + expected_settings = default_updater_settings() + expected_settings["updater_check_all"] = True + expected_settings["updater_errors"] = 0 # errors must be cleared + expected_settings["updater_last_check"] = curtime + expected_settings["updater_container_needs_update"] = True + + assert updater.dangerzone.settings.get_updater_settings() == expected_settings + + +def test_new_release_is_detected( qtbot: QtBot, qt_updater: UpdaterThread, monkeypatch: MonkeyPatch, @@ -182,6 +227,11 @@ def test_update_detected( handle_updates_spy = mocker.spy(window, "handle_updates") load_svg_spy = mocker.spy(main_window_module, "load_svg_image") + # Mock the response of the container updater check + mocker.patch( + "dangerzone.updater.releases.is_container_update_available", + return_value=[False, None], + ) menu_actions_before = window.hamburger_button.menu().actions() with qtbot.waitSignal(qt_updater.finished): @@ -193,7 +243,7 @@ def test_update_detected( handle_updates_spy.assert_called_once() assert_report_equal( handle_updates_spy.call_args.args[0], - releases.UpdateReport("99.9.9", "

changelog

"), + releases.UpdaterReport("99.9.9", "

changelog

"), ) # Check that the settings have been updated properly. diff --git a/tests/gui/test_updater.py b/tests/gui/test_updater.py index 12a7cfa..39cbdc8 100644 --- a/tests/gui/test_updater.py +++ b/tests/gui/test_updater.py @@ -14,7 +14,7 @@ from dangerzone import settings from dangerzone.gui import updater as updater_module from dangerzone.gui.updater import UpdaterThread from dangerzone.updater import releases -from dangerzone.updater.releases import UpdateReport +from dangerzone.updater.releases import UpdaterReport from dangerzone.util import get_version from ..test_settings import default_settings_0_4_1, save_settings @@ -34,7 +34,7 @@ def default_updater_settings() -> dict: } -def assert_report_equal(report1: UpdateReport, report2: UpdateReport) -> None: +def assert_report_equal(report1: UpdaterReport, report2: UpdaterReport) -> None: assert report1.version == report2.version assert report1.changelog == report2.changelog assert report1.error == report2.error @@ -174,6 +174,11 @@ def test_update_checks( requests_mock().status_code = 200 # type: ignore [call-arg] requests_mock().json.return_value = mock_upstream_info # type: ignore [attr-defined, call-arg] + mocker.patch( + "dangerzone.updater.releases.is_container_update_available", + return_value=[False, None], + ) + # Always assume that we can perform multiple update checks in a row. mocker.patch( "dangerzone.updater.releases._should_postpone_update_check", return_value=False @@ -181,21 +186,21 @@ def test_update_checks( # Test 1 - Check that the current version triggers no updates. report = releases.check_for_updates(settings) - assert_report_equal(report, UpdateReport()) + assert_report_equal(report, UpdaterReport()) # Test 2 - Check that a newer version triggers updates, and that the changelog is # rendered from Markdown to HTML. mock_upstream_info["tag_name"] = "v99.9.9" report = releases.check_for_updates(settings) assert_report_equal( - report, UpdateReport(version="99.9.9", changelog="

changelog

") + report, UpdaterReport(version="99.9.9", changelog="

changelog

") ) # Test 3 - Check that HTTP errors are converted to error reports. requests_mock.side_effect = Exception("failed") # type: ignore [attr-defined] report = releases.check_for_updates(settings) error_msg = f"Encountered an exception while checking {updater_module.releases.GH_RELEASE_URL}: failed" - assert_report_equal(report, UpdateReport(error=error_msg)) + assert_report_equal(report, UpdaterReport(error=error_msg)) # Test 4 - Check that cached version/changelog info do not trigger an update check. settings.set("updater_latest_version", "99.9.9") @@ -203,7 +208,7 @@ def test_update_checks( report = releases.check_for_updates(settings) assert_report_equal( - report, UpdateReport(version="99.9.9", changelog="

changelog

") + report, UpdaterReport(version="99.9.9", changelog="

changelog

") ) @@ -220,6 +225,12 @@ def test_update_checks_cooldown(updater: UpdaterThread, mocker: MockerFixture) - mocker.patch("dangerzone.updater.releases.requests.get") requests_mock = updater_module.releases.requests.get + # Mock the response of the container updater check + mocker.patch( + "dangerzone.updater.releases.is_container_update_available", + return_value=[False, None], + ) + # # Make requests.get().json() return the version info that we want. mock_upstream_info = {"tag_name": "99.9.9", "body": "changelog"} requests_mock().status_code = 200 # type: ignore [call-arg] @@ -234,7 +245,7 @@ def test_update_checks_cooldown(updater: UpdaterThread, mocker: MockerFixture) - report = releases.check_for_updates(settings) assert cooldown_spy.spy_return is False assert settings.get("updater_last_check") == curtime - assert_report_equal(report, UpdateReport("99.9.9", "

changelog

")) + assert_report_equal(report, UpdaterReport("99.9.9", "

changelog

")) # Test 2: Advance the current time by 1 second, and ensure that no update will take # place, due to the cooldown period. The last check timestamp should remain the @@ -248,7 +259,7 @@ def test_update_checks_cooldown(updater: UpdaterThread, mocker: MockerFixture) - report = releases.check_for_updates(settings) assert cooldown_spy.spy_return is True assert settings.get("updater_last_check") == curtime - 1 # type: ignore [unreachable] - assert_report_equal(report, UpdateReport()) + assert_report_equal(report, UpdaterReport()) # Test 3: Advance the current time by seconds. Ensure that # Dangerzone checks for updates again, and the last check timestamp gets bumped. @@ -259,7 +270,7 @@ def test_update_checks_cooldown(updater: UpdaterThread, mocker: MockerFixture) - report = releases.check_for_updates(settings) assert cooldown_spy.spy_return is False assert settings.get("updater_last_check") == curtime - assert_report_equal(report, UpdateReport("99.9.9", "

changelog

")) + assert_report_equal(report, UpdaterReport("99.9.9", "

changelog

")) # Test 4: Make Dangerzone check for updates again, but this time, it should # encounter an error while doing so. In that case, the last check timestamp @@ -275,7 +286,7 @@ def test_update_checks_cooldown(updater: UpdaterThread, mocker: MockerFixture) - assert cooldown_spy.spy_return is False assert settings.get("updater_last_check") == curtime error_msg = f"Encountered an exception while checking {updater_module.releases.GH_RELEASE_URL}: failed" - assert_report_equal(report, UpdateReport(error=error_msg)) + assert_report_equal(report, UpdaterReport(error=error_msg)) def test_update_errors( @@ -285,6 +296,10 @@ def test_update_errors( settings = updater.dangerzone.settings # Always assume that we can perform multiple update checks in a row. monkeypatch.setattr(releases, "_should_postpone_update_check", lambda _: False) + mocker.patch( + "dangerzone.updater.releases.is_container_update_available", + return_value=[False, None], + ) # Mock requests.get(). mocker.patch("dangerzone.updater.releases.requests.get") @@ -363,7 +378,7 @@ def test_update_errors( requests_mock.return_value = MockResponseValid() # type: ignore [attr-defined] report = releases.check_for_updates(settings) - assert_report_equal(report, UpdateReport("99.9.9", "

changelog

")) + assert_report_equal(report, UpdaterReport("99.9.9", "

changelog

")) def test_update_check_prompt(