diff --git a/debian/install b/debian/install index cbd682c3..8618d00a 100644 --- a/debian/install +++ b/debian/install @@ -62,6 +62,7 @@ /usr/lib/*/dist-packages/qubesmanager/tests/test_vm_settings.py /usr/lib/*/dist-packages/qubesmanager/tests/test_clone_vm.py /usr/lib/*/dist-packages/qubesmanager/tests/test_utils.py +/usr/lib/*/dist-packages/qubesmanager/tests/test_bootfromdevice.py /usr/lib/*/dist-packages/qubesmanager-*.egg-info/* diff --git a/qubesmanager/backup.py b/qubesmanager/backup.py index ab30735c..93a74d87 100644 --- a/qubesmanager/backup.py +++ b/qubesmanager/backup.py @@ -92,6 +92,9 @@ def __init__(self, qt_app, qubes_app, dispatcher, parent=None): self.progress_status.text = self.tr("Backup in progress...") self.dir_line_edit.setReadOnly(False) + path_allowed_chars_message = utils.get_path_chars_message() + self.dir_line_edit.setToolTip(path_allowed_chars_message) + self.select_path_button.setToolTip(path_allowed_chars_message) self.select_vms_widget = multiselectwidget.MultiSelectWidget(self) self.verticalLayout.insertWidget(1, self.select_vms_widget) diff --git a/qubesmanager/backup_utils.py b/qubesmanager/backup_utils.py index aa813af4..cde8188d 100644 --- a/qubesmanager/backup_utils.py +++ b/qubesmanager/backup_utils.py @@ -98,12 +98,11 @@ def select_path_button_clicked(dialog, select_file=False, read_only=False): vm, "qubes.SelectFile" if select_file else "qubes.SelectDirectory") - except ValueError: + except ValueError as ex: QtWidgets.QMessageBox.warning( dialog, dialog.tr("Unexpected characters in path!"), - dialog.tr("Backup path can only contain the following " - "special characters: /:.,_+=() -")) + str(ex)) except subprocess.CalledProcessError as ex: # dialog cancelled if read_only and ex.returncode == 1: diff --git a/qubesmanager/bootfromdevice.py b/qubesmanager/bootfromdevice.py index 8f4c926b..636c4386 100644 --- a/qubesmanager/bootfromdevice.py +++ b/qubesmanager/bootfromdevice.py @@ -100,6 +100,9 @@ def __init_buttons__(self): self.fileVM.setEnabled(False) self.selectFileButton.setEnabled(False) self.blockDeviceComboBox.setEnabled(False) + path_allowed_chars_message = utils.get_path_chars_message() + self.pathText.setToolTip(path_allowed_chars_message) + self.selectFileButton.setToolTip(path_allowed_chars_message) self.blockDeviceRadioButton.clicked.connect(self.radio_button_clicked) self.fileRadioButton.clicked.connect(self.radio_button_clicked) @@ -152,6 +155,13 @@ def select_file_dialog(self): # Error other than 'user did not select a file' error_occurred = True new_path = None + except ValueError as ex: + QtWidgets.QMessageBox.warning( + self, + self.tr("Unexpected characters in path!"), + str(ex) + ) + new_path = None except Exception: # pylint: disable=broad-except error_occurred = True new_path = None diff --git a/qubesmanager/tests/test_backup_utils.py b/qubesmanager/tests/test_backup_utils.py index afd4a888..b36d8199 100644 --- a/qubesmanager/tests/test_backup_utils.py +++ b/qubesmanager/tests/test_backup_utils.py @@ -17,9 +17,11 @@ # with this program; if not, write to the Free Software Foundation, Inc., # 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. # +from unittest import mock + from PyQt6 import QtWidgets -from qubesmanager import backup_utils +from qubesmanager import backup_utils, utils def test_01_fill_apvms(qapp, test_qubes_app): @@ -46,3 +48,25 @@ def test_01_fill_apvms(qapp, test_qubes_app): assert sorted(expected_vm_list) == sorted(received_vm_list), \ "VM list not filled correctly" + + +@mock.patch("PyQt6.QtWidgets.QMessageBox.warning") +@mock.patch("qubesmanager.backup_utils.utils.get_path_from_vm") +def test_02_select_path_invalid_characters(mock_get_path, mock_warning): + dialog = mock.Mock() + dialog.dir_line_edit.text.return_value = "" + dialog.appvm_combobox.currentText.return_value = "test-vm" + dialog.qubes_app.domains = {"test-vm": mock.Mock(name="test-vm")} + dialog.tr.side_effect = lambda text: text + mock_get_path.side_effect = ValueError( + utils.get_path_chars_message() + ) + + backup_utils.select_path_button_clicked(dialog) + + mock_warning.assert_called_once_with( + dialog, + "Unexpected characters in path!", + utils.get_path_chars_message(), + ) + dialog.dir_line_edit.setText.assert_not_called() diff --git a/qubesmanager/tests/test_bootfromdevice.py b/qubesmanager/tests/test_bootfromdevice.py new file mode 100644 index 00000000..3538eb88 --- /dev/null +++ b/qubesmanager/tests/test_bootfromdevice.py @@ -0,0 +1,72 @@ +# The Qubes OS Project, https://www.qubes-os.org/ +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 2 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License along +# with this program; if not, write to the Free Software Foundation, Inc., +# 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. +# +from unittest import mock + +from qubesmanager import bootfromdevice, utils + + +def _boot_dialog(): + dialog = bootfromdevice.VMBootFromDeviceWindow.__new__( + bootfromdevice.VMBootFromDeviceWindow + ) + dialog.tr = lambda text: text + return dialog + + +@mock.patch("qubesmanager.bootfromdevice.utils.initialize_widget") +@mock.patch("qubesmanager.bootfromdevice.utils.initialize_widget_with_vms") +def test_00_init_buttons_sets_path_tooltips( + mock_initialize_widget_with_vms, mock_initialize_widget +): + dialog = _boot_dialog() + dialog.fileVM = mock.Mock() + dialog.selectFileButton = mock.Mock() + dialog.blockDeviceComboBox = mock.Mock() + dialog.blockDeviceRadioButton = mock.Mock() + dialog.fileRadioButton = mock.Mock() + dialog.pathText = mock.Mock() + dialog.vm = "test-vm" + dialog.qubesapp = mock.Mock() + dialog.qubesapp.domains = [] + + bootfromdevice.VMBootFromDeviceWindow.__init_buttons__(dialog) + + message = utils.get_path_chars_message() + dialog.pathText.setToolTip.assert_called_once_with(message) + dialog.selectFileButton.setToolTip.assert_called_once_with(message) + mock_initialize_widget_with_vms.assert_called_once() + mock_initialize_widget.assert_not_called() + + +@mock.patch("PyQt6.QtWidgets.QMessageBox.warning") +@mock.patch("qubesmanager.bootfromdevice.utils.get_path_from_vm") +def test_01_select_file_dialog_invalid_characters(mock_get_path, mock_warning): + dialog = _boot_dialog() + dialog.fileVM = mock.Mock() + dialog.pathText = mock.Mock() + backend_vm = mock.Mock(name="backend-vm") + dialog.fileVM.currentData.return_value = backend_vm + mock_get_path.side_effect = ValueError(utils.get_path_chars_message()) + + bootfromdevice.VMBootFromDeviceWindow.select_file_dialog(dialog) + + mock_warning.assert_called_once_with( + dialog, + "Unexpected characters in path!", + utils.get_path_chars_message(), + ) + dialog.pathText.setText.assert_not_called() diff --git a/qubesmanager/tests/test_utils.py b/qubesmanager/tests/test_utils.py index 64541889..3822294c 100644 --- a/qubesmanager/tests/test_utils.py +++ b/qubesmanager/tests/test_utils.py @@ -18,9 +18,50 @@ # 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. # +import unittest +from unittest import mock + from PyQt6 import QtGui # pylint: disable=import-error from qubesmanager import utils -import unittest + + +class TestPathFromVM(unittest.TestCase): + def test_00_accepts_ascii_path(self): + vm = mock.Mock() + vm.run_service_for_stdio.return_value = ( + b"/home/user/backup-file_01.iso\n", + None, + ) + + path = utils.get_path_from_vm(vm, "qubes.SelectFile") + + self.assertEqual(path, "/home/user/backup-file_01.iso") + + def test_01_rejects_non_ascii_path(self): + vm = mock.Mock() + vm.run_service_for_stdio.return_value = ( + "zażółć.pdf\n".encode("utf-8"), + None, + ) + + with self.assertRaises(ValueError) as exc: + utils.get_path_from_vm(vm, "qubes.SelectFile") + + self.assertIn("ASCII", str(exc.exception)) + + def test_02_rejects_control_character(self): + vm = mock.Mock() + vm.run_service_for_stdio.return_value = ( + b"/home/user/bad\tname.pdf\n", + None, + ) + + with self.assertRaises(ValueError) as exc: + utils.get_path_from_vm(vm, "qubes.SelectFile") + + self.assertEqual( + str(exc.exception), utils.get_path_chars_message() + ) class TestCaseQImage(unittest.TestCase): diff --git a/qubesmanager/utils.py b/qubesmanager/utils.py index e04965d5..caf3c260 100644 --- a/qubesmanager/utils.py +++ b/qubesmanager/utils.py @@ -42,6 +42,21 @@ import qasync +PATH_ALLOWED_SPECIAL_CHARACTERS = "/:.,_+=() ?-" +PATH_ALLOWED_CHARACTERS_RE = re.compile( + r"[a-zA-Z0-9" + re.escape(PATH_ALLOWED_SPECIAL_CHARACTERS) + r"]*" +) +PATH_MAX_LEN = 512 + + +def get_path_chars_message(): + return QtCore.QCoreApplication.translate( + "ManagerUtils", + "Paths can contain only ASCII letters, digits, spaces, and the " + "following special characters: {characters}", + ).format(characters=PATH_ALLOWED_SPECIAL_CHARACTERS) + + # important usage note: which initialize_widget should I use? # - if you want a list of VMs, use initialize_widget_with_vms, optionally # adding a property if you want to handle qubesadmin.DEFAULT and the @@ -464,25 +479,24 @@ def get_path_from_vm(vm, service_name): :return: path to file, checked for validity """ - path_re = re.compile(r"[a-zA-Z0-9/:.,_+=() ?-]*") - path_max_len = 512 - if not vm: return None stdout, _stderr = vm.run_service_for_stdio(service_name) stdout = stdout.strip() - untrusted_path = stdout.decode(encoding='ascii')[:path_max_len] + try: + untrusted_path = stdout.decode(encoding='ascii')[:PATH_MAX_LEN] + except UnicodeDecodeError as ex: + raise ValueError(get_path_chars_message()) from ex if not untrusted_path: return None - if path_re.fullmatch(untrusted_path): + if PATH_ALLOWED_CHARACTERS_RE.fullmatch(untrusted_path): assert '../' not in untrusted_path assert '\0' not in untrusted_path return untrusted_path.strip() - raise ValueError(QtCore.QCoreApplication.translate( - "ManagerUtils", 'Unexpected characters in path.')) + raise ValueError(get_path_chars_message()) def format_dependencies_list(dependencies): diff --git a/rpm_spec/qmgr.spec.in b/rpm_spec/qmgr.spec.in index dfae655c..1f473b0a 100644 --- a/rpm_spec/qmgr.spec.in +++ b/rpm_spec/qmgr.spec.in @@ -123,6 +123,7 @@ rm -rf $RPM_BUILD_ROOT %{python3_sitelib}/qubesmanager/tests/test_vm_settings.py %{python3_sitelib}/qubesmanager/tests/test_clone_vm.py %{python3_sitelib}/qubesmanager/tests/test_utils.py +%{python3_sitelib}/qubesmanager/tests/test_bootfromdevice.py %dir %{python3_sitelib}/qubesmanager-*.egg-info %{python3_sitelib}/qubesmanager-*.egg-info/*