From 833c0e6db7601e2b3390a9492f05585e59f0dc1d Mon Sep 17 00:00:00 2001
From: Nihal <121309701+nihalxkumar@users.noreply.github.com>
Date: Fri, 27 Mar 2026 12:55:10 +0530
Subject: [PATCH 1/2] firewall: Add comment field to firewall rules
Allows users to add optional plain-text comments/notes to individual
firewall rules for documentation purposes. The comment is metadata only
and does not affect the actual firewall behavior.
---
qubesmanager/firewall.py | 20 +++++-
qubesmanager/i18n/qubesmanager_en.ts | 15 ++++
qubesmanager/i18n/qubesmanager_es.ts | 15 ++++
qubesmanager/tests/test_vm_settings.py | 96 ++++++++++++++++++++++++++
ui/newfwruledlg.ui | 27 +++++++-
5 files changed, 169 insertions(+), 4 deletions(-)
diff --git a/qubesmanager/firewall.py b/qubesmanager/firewall.py
index 8802e69d5..bd14fde77 100644
--- a/qubesmanager/firewall.py
+++ b/qubesmanager/firewall.py
@@ -107,6 +107,10 @@ def try_to_create_rule(self):
"invalid.".format(parsed_service)))
return False
+ comment_text = self.commentLineEdit.text().strip()
+ if comment_text:
+ rule.comment = comment_text
+
if self.model.current_row is not None:
self.model.set_child(self.model.current_row, rule)
else:
@@ -173,7 +177,12 @@ def __init__(self, parent=None):
self.current_row = None
self.current_dialog = None
- self.__column_names = {0: "Address", 1: "Port/Service", 2: "Protocol", }
+ self.__column_names = {
+ 0: "Address",
+ 1: "Port/Service",
+ 2: "Protocol",
+ 3: "Comment",
+ }
self.__services = []
self.port_range_pattern = re.compile(r'\d+-\d+')
@@ -243,6 +252,13 @@ def get_column_string(self, col, rule):
if rule.proto is None:
return "any"
return str(rule.proto)
+
+ # Comment
+ if col == 3:
+ if rule.comment is None:
+ return ""
+ return str(rule.comment)
+
return "unknown"
def get_firewall_conf(self, vm):
@@ -392,6 +408,8 @@ def populate_edit_dialog(self, dialog, row):
dialog.udp_radio.setChecked(True)
else:
dialog.any_radio.setChecked(True)
+ comment = self.get_column_string(3, self.children[row])
+ dialog.commentLineEdit.setText(comment)
def run_rule_dialog(self, dialog, row=None):
self.current_row = row
diff --git a/qubesmanager/i18n/qubesmanager_en.ts b/qubesmanager/i18n/qubesmanager_en.ts
index 13d59cbf2..656419090 100644
--- a/qubesmanager/i18n/qubesmanager_en.ts
+++ b/qubesmanager/i18n/qubesmanager_en.ts
@@ -793,6 +793,21 @@ p, li { white-space: pre-wrap; }
<html><head/><body><p>Port/service can be provided as either port number (e.g. 122), port range (1024-1234) or service name (e.g. smtp) . For full list of services known, see /etc/services in dom0.</p></body></html>
+
+
+ Comment
+
+
+
+
+ Optional note for your own reference.
+
+
+
+
+ Optional note (e.g. reason for this rule)
+
+
QubeManager
diff --git a/qubesmanager/i18n/qubesmanager_es.ts b/qubesmanager/i18n/qubesmanager_es.ts
index 3f538ee23..db8cbb818 100644
--- a/qubesmanager/i18n/qubesmanager_es.ts
+++ b/qubesmanager/i18n/qubesmanager_es.ts
@@ -815,6 +815,21 @@ p, li { white-space: pre-wrap; }
<html><head/><body><p>Port/service can be provided as either port number (e.g. 122), port range (1024-1234) or service name (e.g. smtp) . For full list of services known, see /etc/services in dom0.</p></body></html>
+
+
+ Comment
+
+
+
+
+ Optional note for your own reference.
+
+
+
+
+ Optional note (e.g. reason for this rule)
+
+
QubeManager
diff --git a/qubesmanager/tests/test_vm_settings.py b/qubesmanager/tests/test_vm_settings.py
index 3174445ac..64ae8f0b9 100644
--- a/qubesmanager/tests/test_vm_settings.py
+++ b/qubesmanager/tests/test_vm_settings.py
@@ -1459,6 +1459,42 @@ def test_305_firewall_edit_rule(settings_fixture):
assert expected_call in settings_window.qubesapp.actual_calls
+@check_errors
+@pytest.mark.parametrize("settings_fixture", ["test-vm-set"], indirect=True)
+def test_305b_firewall_add_rule_with_comment(settings_fixture):
+ settings_window, page, vm_name = settings_fixture
+
+ assert settings_window.policy_deny_radio_button.isChecked()
+
+ settings_window.new_rule_button.click()
+ settings_window.fw_model.current_dialog.addressComboBox.setCurrentText(
+ "example.com"
+ )
+ settings_window.fw_model.current_dialog.commentLineEdit.setText(
+ "temporary access for testing"
+ )
+ settings_window.fw_model.current_dialog.buttonBox.button(
+ QtWidgets.QDialogButtonBox.StandardButton.Ok
+ ).click()
+
+ expected_call = (
+ "test-vm-set",
+ "admin.vm.firewall.Set",
+ None,
+ b"action=accept dsthost=qubes-os.org\n"
+ b"action=accept dsthost=example.com"
+ b" comment=temporary access for testing\n"
+ b"action=accept specialtarget=dns\n"
+ b"action=accept proto=icmp\n"
+ b"action=drop\n",
+ )
+ assert expected_call not in settings_window.qubesapp.actual_calls
+ settings_window.qubesapp.expected_calls[expected_call] = b"0\x00"
+
+ settings_window.accept()
+
+ assert expected_call in settings_window.qubesapp.actual_calls
+
@check_errors
@pytest.mark.parametrize("settings_fixture", ["test-vm-set"], indirect=True)
def test_306_firewall_unlimit(settings_fixture):
@@ -1494,6 +1530,66 @@ def test_306_firewall_unlimit(settings_fixture):
assert expected_call in settings_window.qubesapp.actual_calls
+@check_errors
+@mock.patch("subprocess.check_output")
+def test_306b_firewall_preserve_cli_comment(mock_subprocess, qapp, test_qubes_app):
+ """Rules created via CLI with comments should survive GUI round-trip."""
+ mock_subprocess.return_value = b""
+
+ fw_rules = [
+ {"action": "accept", "dsthost": "gitlab.com",
+ "comment": "IP for gitlab at the time"},
+ {"action": "accept", "specialtarget": "dns"},
+ {"action": "accept", "proto": "icmp"},
+ {"action": "drop"},
+ ]
+
+ test_qubes_app._qubes["test-vm-comment"] = MockQube(
+ name="test-vm-comment",
+ qapp=test_qubes_app,
+ label="green",
+ firewall_rules=fw_rules,
+ )
+ test_qubes_app.expected_calls[
+ ("test-vm-comment", "admin.vm.notes.Get", None, None)
+ ] = b"0\x00"
+ test_qubes_app.update_vm_calls()
+
+ settings_window = vm_settings.VMSettingsWindow(
+ "test-vm-comment", "firewall", qapp, test_qubes_app
+ )
+
+ # Comment should be visible in the model
+ assert settings_window.fw_model.get_column_string(
+ 3, settings_window.fw_model.children[0]
+ ) == "IP for gitlab at the time"
+
+ # Edit the rule (opens dialog, accept it unchanged) to trigger fw_changed
+ settings_window.rulesTreeView.setCurrentIndex(
+ settings_window.fw_model.index(0, 0))
+ settings_window.edit_rule_button.click()
+ settings_window.fw_model.current_dialog.buttonBox.button(
+ QtWidgets.QDialogButtonBox.StandardButton.Ok
+ ).click()
+
+ # The comment must survive the edit round-trip
+ expected_call = (
+ "test-vm-comment",
+ "admin.vm.firewall.Set",
+ None,
+ b"action=accept dsthost=gitlab.com"
+ b" comment=IP for gitlab at the time\n"
+ b"action=accept specialtarget=dns\n"
+ b"action=accept proto=icmp\n"
+ b"action=drop\n",
+ )
+ settings_window.qubesapp.expected_calls[expected_call] = b"0\x00"
+
+ settings_window.accept()
+
+ assert expected_call in settings_window.qubesapp.actual_calls
+
+
@check_errors
@mock.patch("subprocess.check_output")
def test_307_open_with_limit(mock_subprocess, qapp, test_qubes_app):
diff --git a/ui/newfwruledlg.ui b/ui/newfwruledlg.ui
index 958515856..65bd0d344 100644
--- a/ui/newfwruledlg.ui
+++ b/ui/newfwruledlg.ui
@@ -10,7 +10,7 @@
0
0
381
- 193
+ 225
@@ -105,8 +105,28 @@
true
-
- -
+
+ -
+
+
+ Comment
+
+
+
+ -
+
+
+ Optional note for your own reference.
+
+
+ Optional note (e.g. reason for this rule)
+
+
+ 512
+
+
+
+ -
@@ -152,6 +172,7 @@
udp_radio
any_radio
serviceComboBox
+ commentLineEdit
buttonBox
From d4db7b55d818e0f69f143aa3669cb1dd8e56a27a Mon Sep 17 00:00:00 2001
From: Nihal Kumar
Date: Tue, 7 Apr 2026 18:45:42 +0530
Subject: [PATCH 2/2] straight forward placeholder
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Co-authored-by: Marta Marczykowska-Górecka
---
ui/newfwruledlg.ui | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/ui/newfwruledlg.ui b/ui/newfwruledlg.ui
index 65bd0d344..0b37336f6 100644
--- a/ui/newfwruledlg.ui
+++ b/ui/newfwruledlg.ui
@@ -116,10 +116,10 @@
-
- Optional note for your own reference.
+ Comments are for user reference only
- Optional note (e.g. reason for this rule)
+ Comment (optional)
512