From 110937c419f2c62e5a26aa4353bc5d8e392f6dfe Mon Sep 17 00:00:00 2001 From: Tobias Blachetta Date: Wed, 10 Jun 2026 13:41:02 +0200 Subject: [PATCH 1/4] fix: Fixed partial commit owner overwrite --- panos/base.py | 34 +++++++++++++++++++++++---- panos/firewall.py | 2 +- tests/test_base.py | 58 ++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 88 insertions(+), 6 deletions(-) diff --git a/panos/base.py b/panos/base.py index c1ee2def..57074fbc 100644 --- a/panos/base.py +++ b/panos/base.py @@ -556,6 +556,23 @@ def element_str(self, pretty_print=False): return parsed.toprettyxml(indent="\t", encoding="utf-8") return ET.tostring(self.element(), encoding="utf-8") + def element_str_inner(self): + """The XML of this object's children only, without the root entry/member wrapper. + + Used by create() to issue a set call against the entry's own xpath rather than + the parent container xpath. This matches how the GUI writes config: target the + entry directly with its inner content so that PAN-OS records a CREATE on the + entry instead of an EDIT on the parent container (which would change admin lock + ownership to the calling admin). + + Returns: + str: XML of the child elements as a concatenated byte string, or an empty + byte string if the root has no children. + + """ + root = self.element() + return b"".join(ET.tostring(child, encoding="utf-8") for child in root) + def _root_element(self): if self.SUFFIX == ENTRY: return ET.Element("entry", {"name": self.uid}) @@ -669,13 +686,20 @@ def create(self): device.id + ': create called on %s object "%s"' % (type(self), self.uid) ) device.set_config_changed() - element = self.element_str() + # For entry/member objects, target the entry's own xpath with inner content only. + # This matches how the GUI writes config: PAN-OS records a CREATE on the entry + # rather than an EDIT on the parent container, which would change admin lock + # ownership to the calling admin. + if self.SUFFIX in (ENTRY, MEMBER): + xpath = self.xpath() + element = self.element_str_inner() + else: + xpath = self.xpath_short() + element = self.element_str() if self.HA_SYNC: - device.active().xapi.set( - self.xpath_short(), element, retry_on_peer=self.HA_SYNC - ) + device.active().xapi.set(xpath, element, retry_on_peer=self.HA_SYNC) else: - device.xapi.set(self.xpath_short(), element, retry_on_peer=self.HA_SYNC) + device.xapi.set(xpath, element, retry_on_peer=self.HA_SYNC) for child in self.children: child._check_child_methods("create") diff --git a/panos/firewall.py b/panos/firewall.py index d131ddea..e4703271 100644 --- a/panos/firewall.py +++ b/panos/firewall.py @@ -622,7 +622,7 @@ def element(self): ET.SubElement(partial, "shared-object").text = "excluded" if self.exclude_policy_and_objects: ET.SubElement(partial, "policy-and-objects").text = "excluded" - fe.append(partial) + root.append(partial) if self.force: fe = ET.SubElement(root, "force") diff --git a/tests/test_base.py b/tests/test_base.py index 2ba1437d..7a57dc31 100644 --- a/tests/test_base.py +++ b/tests/test_base.py @@ -583,6 +583,64 @@ def test_create_without_ha_sync(self, m_uid): for c in self.obj.children: c._check_child_methods.assert_called_once_with("create") + @mock.patch("panos.base.PanObject.uid", new_callable=mock.PropertyMock) + def test_create_entry_suffix_uses_entry_xpath_and_inner_element(self, m_uid): + # ENTRY-suffix objects must use self.xpath() (not xpath_short()) and + # element_str_inner() (not element_str()) so that PAN-OS records a CREATE + # on the entry itself rather than an EDIT on the parent container. + PanDeviceId = "42" + PanDeviceXpath = "path/to/entry" + PanDeviceInnerElement = b"any" + + self.obj.SUFFIX = Base.ENTRY + + spec = {"id": PanDeviceId} + m_panos = mock.Mock(**spec) + self.obj.nearest_pandevice = mock.Mock(return_value=m_panos) + self.obj.xpath = mock.Mock(return_value=PanDeviceXpath) + self.obj.element_str_inner = mock.Mock(return_value=PanDeviceInnerElement) + m_uid.return_value = "uid" + + ret_val = self.obj.create() + + self.assertIsNone(ret_val) + m_panos.set_config_changed.assert_called_once_with() + m_panos.active().xapi.set.assert_called_once_with( + PanDeviceXpath, + PanDeviceInnerElement, + retry_on_peer=self.obj.HA_SYNC, + ) + self.obj.xpath.assert_called_once_with() + self.obj.element_str_inner.assert_called_once_with() + + @mock.patch("panos.base.PanObject.uid", new_callable=mock.PropertyMock) + def test_create_entry_suffix_without_ha_sync(self, m_uid): + PanDeviceId = "42" + PanDeviceXpath = "path/to/entry" + PanDeviceInnerElement = b"any" + + self.obj.SUFFIX = Base.ENTRY + self.obj.HA_SYNC = False + + spec = {"id": PanDeviceId} + m_panos = mock.Mock(**spec) + self.obj.nearest_pandevice = mock.Mock(return_value=m_panos) + self.obj.xpath = mock.Mock(return_value=PanDeviceXpath) + self.obj.element_str_inner = mock.Mock(return_value=PanDeviceInnerElement) + m_uid.return_value = "uid" + + ret_val = self.obj.create() + + self.assertIsNone(ret_val) + m_panos.set_config_changed.assert_called_once_with() + m_panos.xapi.set.assert_called_once_with( + PanDeviceXpath, + PanDeviceInnerElement, + retry_on_peer=self.obj.HA_SYNC, + ) + self.obj.xpath.assert_called_once_with() + self.obj.element_str_inner.assert_called_once_with() + @mock.patch("panos.base.PanObject.uid", new_callable=mock.PropertyMock) def test_delete_with_ha_sync_no_parent(self, m_uid): PanDeviceId = "42" From 3127ce831587ee21817708990a76c86c8b2169a9 Mon Sep 17 00:00:00 2001 From: Tobias Blachetta Date: Wed, 10 Jun 2026 15:38:34 +0200 Subject: [PATCH 2/4] fix: refactored fix --- panos/base.py | 50 +++++++++++++++++++--------------------------- tests/test_base.py | 28 +++++++++++++------------- 2 files changed, 35 insertions(+), 43 deletions(-) diff --git a/panos/base.py b/panos/base.py index 57074fbc..1c0c2c33 100644 --- a/panos/base.py +++ b/panos/base.py @@ -556,23 +556,6 @@ def element_str(self, pretty_print=False): return parsed.toprettyxml(indent="\t", encoding="utf-8") return ET.tostring(self.element(), encoding="utf-8") - def element_str_inner(self): - """The XML of this object's children only, without the root entry/member wrapper. - - Used by create() to issue a set call against the entry's own xpath rather than - the parent container xpath. This matches how the GUI writes config: target the - entry directly with its inner content so that PAN-OS records a CREATE on the - entry instead of an EDIT on the parent container (which would change admin lock - ownership to the calling admin). - - Returns: - str: XML of the child elements as a concatenated byte string, or an empty - byte string if the root has no children. - - """ - root = self.element() - return b"".join(ET.tostring(child, encoding="utf-8") for child in root) - def _root_element(self): if self.SUFFIX == ENTRY: return ET.Element("entry", {"name": self.uid}) @@ -686,20 +669,29 @@ def create(self): device.id + ': create called on %s object "%s"' % (type(self), self.uid) ) device.set_config_changed() - # For entry/member objects, target the entry's own xpath with inner content only. - # This matches how the GUI writes config: PAN-OS records a CREATE on the entry - # rather than an EDIT on the parent container, which would change admin lock - # ownership to the calling admin. + # For entry/member objects, use edit() against the entry's own xpath rather than + # set() against the parent container xpath. PAN-OS records edit() on an entry xpath + # as a CREATE owned by the calling admin; set() against the parent container is + # recorded as an EDIT on the container and changes admin lock ownership to the + # calling admin, which breaks partial commits for other admins. if self.SUFFIX in (ENTRY, MEMBER): - xpath = self.xpath() - element = self.element_str_inner() - else: - xpath = self.xpath_short() - element = self.element_str() - if self.HA_SYNC: - device.active().xapi.set(xpath, element, retry_on_peer=self.HA_SYNC) + if self.HA_SYNC: + device.active().xapi.edit( + self.xpath(), self.element_str(), retry_on_peer=self.HA_SYNC + ) + else: + device.xapi.edit( + self.xpath(), self.element_str(), retry_on_peer=self.HA_SYNC + ) else: - device.xapi.set(xpath, element, retry_on_peer=self.HA_SYNC) + if self.HA_SYNC: + device.active().xapi.set( + self.xpath_short(), self.element_str(), retry_on_peer=self.HA_SYNC + ) + else: + device.xapi.set( + self.xpath_short(), self.element_str(), retry_on_peer=self.HA_SYNC + ) for child in self.children: child._check_child_methods("create") diff --git a/tests/test_base.py b/tests/test_base.py index 7a57dc31..a0d26aee 100644 --- a/tests/test_base.py +++ b/tests/test_base.py @@ -584,13 +584,13 @@ def test_create_without_ha_sync(self, m_uid): c._check_child_methods.assert_called_once_with("create") @mock.patch("panos.base.PanObject.uid", new_callable=mock.PropertyMock) - def test_create_entry_suffix_uses_entry_xpath_and_inner_element(self, m_uid): - # ENTRY-suffix objects must use self.xpath() (not xpath_short()) and - # element_str_inner() (not element_str()) so that PAN-OS records a CREATE - # on the entry itself rather than an EDIT on the parent container. + def test_create_entry_suffix_uses_edit_on_entry_xpath(self, m_uid): + # ENTRY-suffix objects must use xapi.edit() against the entry's own xpath so + # that PAN-OS records a CREATE on the entry rather than an EDIT on the parent + # container (which would change admin lock ownership to the calling admin). PanDeviceId = "42" PanDeviceXpath = "path/to/entry" - PanDeviceInnerElement = b"any" + PanDeviceElementStr = "element string" self.obj.SUFFIX = Base.ENTRY @@ -598,26 +598,26 @@ def test_create_entry_suffix_uses_entry_xpath_and_inner_element(self, m_uid): m_panos = mock.Mock(**spec) self.obj.nearest_pandevice = mock.Mock(return_value=m_panos) self.obj.xpath = mock.Mock(return_value=PanDeviceXpath) - self.obj.element_str_inner = mock.Mock(return_value=PanDeviceInnerElement) + self.obj.element_str = mock.Mock(return_value=PanDeviceElementStr) m_uid.return_value = "uid" ret_val = self.obj.create() self.assertIsNone(ret_val) m_panos.set_config_changed.assert_called_once_with() - m_panos.active().xapi.set.assert_called_once_with( + m_panos.active().xapi.edit.assert_called_once_with( PanDeviceXpath, - PanDeviceInnerElement, + PanDeviceElementStr, retry_on_peer=self.obj.HA_SYNC, ) self.obj.xpath.assert_called_once_with() - self.obj.element_str_inner.assert_called_once_with() + self.obj.element_str.assert_called_once_with() @mock.patch("panos.base.PanObject.uid", new_callable=mock.PropertyMock) def test_create_entry_suffix_without_ha_sync(self, m_uid): PanDeviceId = "42" PanDeviceXpath = "path/to/entry" - PanDeviceInnerElement = b"any" + PanDeviceElementStr = "element string" self.obj.SUFFIX = Base.ENTRY self.obj.HA_SYNC = False @@ -626,20 +626,20 @@ def test_create_entry_suffix_without_ha_sync(self, m_uid): m_panos = mock.Mock(**spec) self.obj.nearest_pandevice = mock.Mock(return_value=m_panos) self.obj.xpath = mock.Mock(return_value=PanDeviceXpath) - self.obj.element_str_inner = mock.Mock(return_value=PanDeviceInnerElement) + self.obj.element_str = mock.Mock(return_value=PanDeviceElementStr) m_uid.return_value = "uid" ret_val = self.obj.create() self.assertIsNone(ret_val) m_panos.set_config_changed.assert_called_once_with() - m_panos.xapi.set.assert_called_once_with( + m_panos.xapi.edit.assert_called_once_with( PanDeviceXpath, - PanDeviceInnerElement, + PanDeviceElementStr, retry_on_peer=self.obj.HA_SYNC, ) self.obj.xpath.assert_called_once_with() - self.obj.element_str_inner.assert_called_once_with() + self.obj.element_str.assert_called_once_with() @mock.patch("panos.base.PanObject.uid", new_callable=mock.PropertyMock) def test_delete_with_ha_sync_no_parent(self, m_uid): From a0e13a137015a8d9090f6748ddd307a87ace4803 Mon Sep 17 00:00:00 2001 From: Tobias Blachetta Date: Wed, 10 Jun 2026 16:24:25 +0200 Subject: [PATCH 3/4] fix: Changed xapi from edit to set --- panos/base.py | 14 ++++++++------ tests/test_base.py | 13 +++++++------ 2 files changed, 15 insertions(+), 12 deletions(-) diff --git a/panos/base.py b/panos/base.py index 1c0c2c33..4d68878b 100644 --- a/panos/base.py +++ b/panos/base.py @@ -670,17 +670,19 @@ def create(self): ) device.set_config_changed() # For entry/member objects, use edit() against the entry's own xpath rather than - # set() against the parent container xpath. PAN-OS records edit() on an entry xpath - # as a CREATE owned by the calling admin; set() against the parent container is - # recorded as an EDIT on the container and changes admin lock ownership to the - # calling admin, which breaks partial commits for other admins. + # For entry/member objects, use set() against the entry's own xpath rather than + # the parent container xpath. PAN-OS records set() on an entry xpath as a CREATE + # owned by the calling admin. Using the parent container xpath is recorded as an + # EDIT on the container and changes admin lock ownership, breaking partial commits + # for other admins. xapi.edit() cannot be used here because PAN-OS rejects edit() + # on a non-existent entry — only set() creates new entries reliably. if self.SUFFIX in (ENTRY, MEMBER): if self.HA_SYNC: - device.active().xapi.edit( + device.active().xapi.set( self.xpath(), self.element_str(), retry_on_peer=self.HA_SYNC ) else: - device.xapi.edit( + device.xapi.set( self.xpath(), self.element_str(), retry_on_peer=self.HA_SYNC ) else: diff --git a/tests/test_base.py b/tests/test_base.py index a0d26aee..b1fdd6bf 100644 --- a/tests/test_base.py +++ b/tests/test_base.py @@ -584,10 +584,11 @@ def test_create_without_ha_sync(self, m_uid): c._check_child_methods.assert_called_once_with("create") @mock.patch("panos.base.PanObject.uid", new_callable=mock.PropertyMock) - def test_create_entry_suffix_uses_edit_on_entry_xpath(self, m_uid): - # ENTRY-suffix objects must use xapi.edit() against the entry's own xpath so - # that PAN-OS records a CREATE on the entry rather than an EDIT on the parent - # container (which would change admin lock ownership to the calling admin). + def test_create_entry_suffix_uses_set_on_entry_xpath(self, m_uid): + # ENTRY-suffix objects must use xapi.set() against the entry's own xpath (not + # the parent container xpath) so that PAN-OS records a CREATE on the entry + # rather than an EDIT on the parent container, which would change admin lock + # ownership to the calling admin and break partial commits for others. PanDeviceId = "42" PanDeviceXpath = "path/to/entry" PanDeviceElementStr = "element string" @@ -605,7 +606,7 @@ def test_create_entry_suffix_uses_edit_on_entry_xpath(self, m_uid): self.assertIsNone(ret_val) m_panos.set_config_changed.assert_called_once_with() - m_panos.active().xapi.edit.assert_called_once_with( + m_panos.active().xapi.set.assert_called_once_with( PanDeviceXpath, PanDeviceElementStr, retry_on_peer=self.obj.HA_SYNC, @@ -633,7 +634,7 @@ def test_create_entry_suffix_without_ha_sync(self, m_uid): self.assertIsNone(ret_val) m_panos.set_config_changed.assert_called_once_with() - m_panos.xapi.edit.assert_called_once_with( + m_panos.xapi.set.assert_called_once_with( PanDeviceXpath, PanDeviceElementStr, retry_on_peer=self.obj.HA_SYNC, From 12756aa2e77e9d10f7417eb877196bce47fe6d39 Mon Sep 17 00:00:00 2001 From: Tobias Blachetta Date: Wed, 10 Jun 2026 22:00:07 +0200 Subject: [PATCH 4/4] revert changes back to original --- panos/base.py | 27 +++++++++++++++++++++------ tests/test_base.py | 26 +++++++++++++------------- 2 files changed, 34 insertions(+), 19 deletions(-) diff --git a/panos/base.py b/panos/base.py index 4d68878b..7ac361bc 100644 --- a/panos/base.py +++ b/panos/base.py @@ -556,6 +556,19 @@ def element_str(self, pretty_print=False): return parsed.toprettyxml(indent="\t", encoding="utf-8") return ET.tostring(self.element(), encoding="utf-8") + def element_str_inner(self): + """XML of this object's child elements only, without the root entry/member wrapper. + + Used by create() when issuing set() against the entry's own xpath. PAN-OS + expects inner content only at that xpath — passing the full wrapper + causes a schema error ("may need to override template object first"). + + Returns: + bytes: concatenated XML of child elements, empty bytes if none. + """ + root = self.element() + return b"".join(ET.tostring(child) for child in root) + def _root_element(self): if self.SUFFIX == ENTRY: return ET.Element("entry", {"name": self.uid}) @@ -672,18 +685,20 @@ def create(self): # For entry/member objects, use edit() against the entry's own xpath rather than # For entry/member objects, use set() against the entry's own xpath rather than # the parent container xpath. PAN-OS records set() on an entry xpath as a CREATE - # owned by the calling admin. Using the parent container xpath is recorded as an - # EDIT on the container and changes admin lock ownership, breaking partial commits - # for other admins. xapi.edit() cannot be used here because PAN-OS rejects edit() - # on a non-existent entry — only set() creates new entries reliably. + # For entry/member objects, use set() against the entry's own xpath with inner + # content only (no wrapper). PAN-OS records this as a CREATE owned by + # the calling admin. Using the parent container xpath is recorded as an EDIT on + # the container and changes admin lock ownership, breaking partial commits for + # other admins. Passing the full wrapper at the entry xpath causes a + # PAN-OS schema error ("may need to override template object first"). if self.SUFFIX in (ENTRY, MEMBER): if self.HA_SYNC: device.active().xapi.set( - self.xpath(), self.element_str(), retry_on_peer=self.HA_SYNC + self.xpath(), self.element_str_inner(), retry_on_peer=self.HA_SYNC ) else: device.xapi.set( - self.xpath(), self.element_str(), retry_on_peer=self.HA_SYNC + self.xpath(), self.element_str_inner(), retry_on_peer=self.HA_SYNC ) else: if self.HA_SYNC: diff --git a/tests/test_base.py b/tests/test_base.py index b1fdd6bf..c072ef55 100644 --- a/tests/test_base.py +++ b/tests/test_base.py @@ -584,14 +584,14 @@ def test_create_without_ha_sync(self, m_uid): c._check_child_methods.assert_called_once_with("create") @mock.patch("panos.base.PanObject.uid", new_callable=mock.PropertyMock) - def test_create_entry_suffix_uses_set_on_entry_xpath(self, m_uid): - # ENTRY-suffix objects must use xapi.set() against the entry's own xpath (not - # the parent container xpath) so that PAN-OS records a CREATE on the entry - # rather than an EDIT on the parent container, which would change admin lock - # ownership to the calling admin and break partial commits for others. + def test_create_entry_suffix_uses_set_on_entry_xpath_with_inner_element(self, m_uid): + # ENTRY-suffix objects must use xapi.set() against the entry's own xpath with + # inner content only (no wrapper). Passing the full element_str() at + # the entry xpath causes PAN-OS schema error; using parent xpath causes an EDIT + # on the container changing admin lock ownership and breaking partial commits. PanDeviceId = "42" PanDeviceXpath = "path/to/entry" - PanDeviceElementStr = "element string" + PanDeviceInnerElement = b"any" self.obj.SUFFIX = Base.ENTRY @@ -599,7 +599,7 @@ def test_create_entry_suffix_uses_set_on_entry_xpath(self, m_uid): m_panos = mock.Mock(**spec) self.obj.nearest_pandevice = mock.Mock(return_value=m_panos) self.obj.xpath = mock.Mock(return_value=PanDeviceXpath) - self.obj.element_str = mock.Mock(return_value=PanDeviceElementStr) + self.obj.element_str_inner = mock.Mock(return_value=PanDeviceInnerElement) m_uid.return_value = "uid" ret_val = self.obj.create() @@ -608,17 +608,17 @@ def test_create_entry_suffix_uses_set_on_entry_xpath(self, m_uid): m_panos.set_config_changed.assert_called_once_with() m_panos.active().xapi.set.assert_called_once_with( PanDeviceXpath, - PanDeviceElementStr, + PanDeviceInnerElement, retry_on_peer=self.obj.HA_SYNC, ) self.obj.xpath.assert_called_once_with() - self.obj.element_str.assert_called_once_with() + self.obj.element_str_inner.assert_called_once_with() @mock.patch("panos.base.PanObject.uid", new_callable=mock.PropertyMock) def test_create_entry_suffix_without_ha_sync(self, m_uid): PanDeviceId = "42" PanDeviceXpath = "path/to/entry" - PanDeviceElementStr = "element string" + PanDeviceInnerElement = b"any" self.obj.SUFFIX = Base.ENTRY self.obj.HA_SYNC = False @@ -627,7 +627,7 @@ def test_create_entry_suffix_without_ha_sync(self, m_uid): m_panos = mock.Mock(**spec) self.obj.nearest_pandevice = mock.Mock(return_value=m_panos) self.obj.xpath = mock.Mock(return_value=PanDeviceXpath) - self.obj.element_str = mock.Mock(return_value=PanDeviceElementStr) + self.obj.element_str_inner = mock.Mock(return_value=PanDeviceInnerElement) m_uid.return_value = "uid" ret_val = self.obj.create() @@ -636,11 +636,11 @@ def test_create_entry_suffix_without_ha_sync(self, m_uid): m_panos.set_config_changed.assert_called_once_with() m_panos.xapi.set.assert_called_once_with( PanDeviceXpath, - PanDeviceElementStr, + PanDeviceInnerElement, retry_on_peer=self.obj.HA_SYNC, ) self.obj.xpath.assert_called_once_with() - self.obj.element_str.assert_called_once_with() + self.obj.element_str_inner.assert_called_once_with() @mock.patch("panos.base.PanObject.uid", new_callable=mock.PropertyMock) def test_delete_with_ha_sync_no_parent(self, m_uid):