Communicate path character limits in GUI tools

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/*
 

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)

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:

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

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()

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()

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):

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):

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/*