Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix markup injection issues #236

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

DemiMarie
Copy link

This fixes some (theoretical) markup injection problems. I believe none of the strings that I escape here will ever contain "<" or "&", but it is always safer to escape.

@marmarta
Copy link
Member

merge conflict, otherwise looks fine (but let's tests check it too)

@DemiMarie DemiMarie force-pushed the fix-markup-injection branch 2 times, most recently from 81f3f70 to be5c363 Compare December 13, 2024 02:50
@DemiMarie DemiMarie marked this pull request as draft December 13, 2024 02:53
@DemiMarie DemiMarie force-pushed the fix-markup-injection branch 3 times, most recently from 4efac67 to a224c12 Compare December 13, 2024 06:07
This fixes some (theoretical) markup injection problems.  I believe none
of the strings that I escape here will ever contain "<" or "&", but it
is always safer to escape.
@DemiMarie DemiMarie force-pushed the fix-markup-injection branch from a224c12 to fd9ff2f Compare December 14, 2024 03:09
@@ -96,6 +98,20 @@ def check_update(vm) -> bool:
return True
return False

def _escape_str(s: Union[str, float, int]) -> Union[str, float, int]:
# pylint: disable=unidiomatic-typecheck
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pylint was right here, use isinstance()

@@ -87,19 +88,21 @@ def setup_menu(self):
self.tray_menu.set_reserve_toggle_size(False)

if self.vms_needing_update:
self.tray_menu.append(TextItem(_("<b>Qube updates available!</b>")))
self.tray_menu.append(TextItem("Qube updates available!"))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Better apply _() to the string constant (tools extracting strings for translation will be happier).

Copy link

codecov bot commented Dec 14, 2024

Codecov Report

Attention: Patch coverage is 60.00000% with 12 lines in your changes missing coverage. Please review.

Project coverage is 93.08%. Comparing base (a3e4d17) to head (fd9ff2f).

Files with missing lines Patch % Lines
qui/utils.py 30.76% 9 Missing ⚠️
qubes_config/widgets/gtk_utils.py 72.72% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #236      +/-   ##
==========================================
- Coverage   93.43%   93.08%   -0.35%     
==========================================
  Files          57       58       +1     
  Lines       10999    11068      +69     
==========================================
+ Hits        10277    10303      +26     
- Misses        722      765      +43     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@qubesos-bot
Copy link

qubesos-bot commented Dec 15, 2024

OpenQA test summary

Complete test suite and dependencies: https://openqa.qubes-os.org/tests/overview?distri=qubesos&version=4.3&build=2024121519-4.3&flavor=pull-requests

Test run included the following:

New failures, excluding unstable

Compared to: https://openqa.qubes-os.org/tests/overview?distri=qubesos&version=4.3&build=2024111705-4.3&flavor=update

  • system_tests_gui_tools

    • qui_widgets_disk_space: unnamed test (unknown)
    • qui_widgets_disk_space: Failed (test died)
      # Test died: no candidate needle with tag(s) 'qui-disk-space-widget...
  • system_tests_kde_gui_interactive

    • gui_keyboard_layout: wait_serial (wait serial expected)
      # wait_serial expected: "echo -e '[Layout]\nLayoutList=us,de' | sud...
  • system_tests_gui_tools@hw7

    • qui_widgets_disk_space: unnamed test (unknown)
    • qui_widgets_disk_space: Failed (test died)
      # Test died: no candidate needle with tag(s) 'qui-disk-space-widget...
  • system_tests_guivm_vnc_gui_interactive

    • guivm_startup: Failed (test died)
      # Test died: command 'qvm-run --nogui -pu root sys-gui-vnc env XAUT...
  • system_tests_guivm_gui_interactive

    • guivm_manager: unnamed test (unknown)
    • guivm_manager: Failed (test died)
      # Test died: no candidate needle with tag(s) 'manager-vm-settings' ...

Failed tests

9 failures
  • system_tests_gui_tools

    • qui_widgets_disk_space: unnamed test (unknown)
    • qui_widgets_disk_space: Failed (test died)
      # Test died: no candidate needle with tag(s) 'qui-disk-space-widget...
  • system_tests_kde_gui_interactive

    • gui_keyboard_layout: wait_serial (wait serial expected)
      # wait_serial expected: "echo -e '[Layout]\nLayoutList=us,de' | sud...

    • gui_keyboard_layout: Failed (test died)
      # Test died: command 'test "$(cd ~user;ls e1*)" = "$(qvm-run -p wor...

  • system_tests_gui_tools@hw7

    • qui_widgets_disk_space: unnamed test (unknown)
    • qui_widgets_disk_space: Failed (test died)
      # Test died: no candidate needle with tag(s) 'qui-disk-space-widget...
  • system_tests_guivm_vnc_gui_interactive

    • guivm_startup: Failed (test died)
      # Test died: command 'qvm-run --nogui -pu root sys-gui-vnc env XAUT...
  • system_tests_guivm_gui_interactive

    • guivm_manager: unnamed test (unknown)
    • guivm_manager: Failed (test died)
      # Test died: no candidate needle with tag(s) 'manager-vm-settings' ...

Fixed failures

Compared to: https://openqa.qubes-os.org/tests/119126#dependencies

3 fixed
  • system_tests_audio@hw1

  • system_tests_extra

    • TC_00_QVCTest_whonix-gateway-17: test_010_screenshare (failure)
      ~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^... AssertionError: 0 == 0
  • system_tests_basic_vm_qrexec_gui_zfs

    • switch_pool: Failed (test died)
      # Test died: command 'dnf install -y ./zfs-release.rpm' failed at /...

Unstable tests

  • system_tests_audio@hw1

    TC_20_AudioVM_PipeWire_fedora-40-xfce/test_260_audio_mic_enabled_switch_audiovm (1/5 times with errors)
    • job 117586 AssertionError: too short audio, expected 10s, got 0.00013605442176...
  • system_tests_audio

    TC_20_AudioVM_PipeWire_fedora-40-xfce/test_260_audio_mic_enabled_switch_audiovm (1/5 times with errors)
    • job 117586 AssertionError: too short audio, expected 10s, got 0.00013605442176...

@marmarek
Copy link
Member

FWIW openQA says:

Dec 15 19:01:36.069179 dom0 widget-wrapper[3495]: Traceback (most recent call last):
Dec 15 19:01:36.070033 dom0 widget-wrapper[3495]:   File "/usr/lib/python3.13/site-packages/qui/tray/disk_space.py", line 435, in make_menu
Dec 15 19:01:36.070033 dom0 widget-wrapper[3495]:     menu.append(self.make_title_item('Volumes'))
Dec 15 19:01:36.070033 dom0 widget-wrapper[3495]:                 ~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^
Dec 15 19:01:36.070033 dom0 widget-wrapper[3495]:   File "/usr/lib/python3.13/site-packages/qui/tray/disk_space.py", line 474, in make_title_item
Dec 15 19:01:36.070033 dom0 widget-wrapper[3495]:     label.set_markup("<b>{}</b>").format(GLib.markup_escape_text(text))
Dec 15 19:01:36.070033 dom0 widget-wrapper[3495]:     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Dec 15 19:01:36.070033 dom0 widget-wrapper[3495]: AttributeError: 'NoneType' object has no attribute 'format'

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants