Merge pull request #56310 from twangboy/fix_lgpo_admx

Only process ADMX files when loading policies

salt/modules/win_lgpo.py

@@ -4986,6 +4986,12 @@ def _load_policy_definitions(path='c:\\Windows\\PolicyDefinitions',
     for root, dirs, files in salt.utils.path.os_walk(path):
         if root == path:
             for t_admx_file in files:
+                admx_file_name, admx_file_ext = os.path.splitext(t_admx_file)
+                # Only process ADMX files, any other file will cause a
+                # stacktrace later on
+                if not admx_file_ext == '.admx':
+                    log.debug('{0} is not an ADMX file'.format(t_admx_file))
+                    continue
                 admx_file = os.path.join(root, t_admx_file)
                 # Parse xml for the ADMX file
                 try:
@@ -5001,9 +5007,6 @@ def _load_policy_definitions(path='c:\\Windows\\PolicyDefinitions',
                     namespaces['None'] = namespaces[None]
                     namespaces.pop(None)
                     namespace_string = 'None:'
-                this_prefix = xml_tree.xpath(
-                        '/{0}policyDefinitions/{0}policyNamespaces/{0}target/@prefix'.format(namespace_string),
-                        namespaces=namespaces)[0]
                 this_namespace = xml_tree.xpath(
                         '/{0}policyDefinitions/{0}policyNamespaces/{0}target/@namespace'.format(namespace_string),
                         namespaces=namespaces)[0]
@@ -5038,7 +5041,7 @@ def _load_policy_definitions(path='c:\\Windows\\PolicyDefinitions',
                 adml_file = os.path.join(
                     root,
                     language,
-                    os.path.splitext(t_admx_file)[0] + '.adml')
+                    admx_file_name + '.adml')
                 if not __salt__['file.file_exists'](adml_file):
                     log.info('An ADML file in the specified ADML language '
                              '"%s" does not exist for the ADMX "%s", the '
@@ -5048,7 +5051,7 @@ def _load_policy_definitions(path='c:\\Windows\\PolicyDefinitions',
                     adml_file = os.path.join(
                         root,
                         language.split('-')[0],
-                        os.path.splitext(t_admx_file)[0] + '.adml')
+                        admx_file_name + '.adml')
                     if not __salt__['file.file_exists'](adml_file):
                         log.info('An ADML file in the specified ADML language '
                                  'code %s does not exist for the ADMX "%s", '
@@ -5058,7 +5061,7 @@ def _load_policy_definitions(path='c:\\Windows\\PolicyDefinitions',
                         adml_file = os.path.join(
                             root,
                             display_language_fallback,
-                            os.path.splitext(t_admx_file)[0] + '.adml')
+                            admx_file_name + '.adml')
                         if not __salt__['file.file_exists'](adml_file):
                             log.info('An ADML file in the specified ADML '
                                      'fallback language "%s" '
@@ -5070,7 +5073,7 @@ def _load_policy_definitions(path='c:\\Windows\\PolicyDefinitions',
                             adml_file = os.path.join(
                                 root,
                                 display_language_fallback.split('-')[0],
-                                os.path.splitext(t_admx_file)[0] + '.adml')
+                                admx_file_name + '.adml')
                             if not __salt__['file.file_exists'](adml_file):
                                 msg = ('An ADML file in the specified ADML language '
                                        '"{0}" and the fallback language "{1}" do not '

tests/unit/modules/test_win_lgpo.py

@@ -5,6 +5,7 @@
 
 # Import Python Libs
 from __future__ import absolute_import, unicode_literals, print_function
+import glob
 import os
 
 # Import Salt Testing Libs
@@ -18,6 +19,7 @@ import salt.config
 import salt.loader
 import salt.modules.win_lgpo as win_lgpo
 import salt.states.win_lgpo
+import salt.utils.files
 import salt.utils.platform
 import salt.utils.stringutils
 
@@ -332,6 +334,39 @@ class WinLGPOGetPolicyADMXTestCase(TestCase, LoaderModuleMockMixin):
                             'Allow Telemetry': 'Not Configured'}}}}}
         self.assertDictEqual(result, expected)
 
+    @destructiveTest
+    def test__load_policy_definitions(self):
+        '''
+        Test that unexpected files in the PolicyDefinitions directory won't
+        cause the _load_policy_definitions function to explode
+        https://gitlab.com/saltstack/enterprise/lock/issues/3826
+        '''
+        # The PolicyDefinitions directory should only contain ADMX files. We
+        # want to make sure the `_load_policy_definitions` function skips non
+        # ADMX files in this directory.
+        # Create a bogus ADML file in PolicyDefinitions directory
+        bogus_fle = os.path.join(
+            'c:\\Windows\\PolicyDefinitions',
+            '_bogus.adml')
+        cache_dir = os.path.join(
+            win_lgpo.__opts__['cachedir'],
+            'lgpo',
+            'policy_defs')
+        try:
+            with salt.utils.files.fopen(bogus_fle, 'w+') as fh:
+                fh.write('<junk></junk>')
+            # This function doesn't return anything (None), it just loads
+            # the XPath structures into __context__. We're just making sure it
+            # doesn't stack trace here
+            self.assertIsNone(win_lgpo._load_policy_definitions())
+        finally:
+            # Remove source file
+            os.remove(bogus_fle)
+            # Remove cached file
+            search_string = '{0}\\_bogus*.adml'.format(cache_dir)
+            for file_name in glob.glob(search_string):
+                os.remove(file_name)
+
 
 @skipIf(not salt.utils.platform.is_windows(), 'System is not Windows')
 class WinLGPOGetPolicyFromPolicyInfoTestCase(TestCase, LoaderModuleMockMixin):