ensure predictable permissions on module artifacts (#84948)

and test it!

(cherry picked from commit 9f894b81c2)

changelogs/fragments/ensure_remote_perms.yml

@@ -0,0 +1,2 @@
+bugfixes:
+  - Ansible will now ensure predictable permissions on remote artifacts, until now it only ensured executable and relied on system masks for the rest.

lib/ansible/plugins/action/__init__.py

@@ -649,12 +649,12 @@ class ActionBase(ABC, _AnsiblePluginInfoMixin):
         # done. Make the files +x if we're asked to, and return.
         if not self._is_become_unprivileged():
             if execute:
-                # Can't depend on the file being transferred with execute permissions.
+                # Can't depend on the file being transferred with required permissions.
                 # Only need user perms because no become was used here
-                res = self._remote_chmod(remote_paths, 'u+x')
+                res = self._remote_chmod(remote_paths, 'u+rwx')
                 if res['rc'] != 0:
                     raise AnsibleError(
-                        'Failed to set execute bit on remote files '
+                        'Failed to set permissions on remote files '
                         '(rc: {0}, err: {1})'.format(
                             res['rc'],
                             to_native(res['stderr'])))
@@ -695,10 +695,10 @@ class ActionBase(ABC, _AnsiblePluginInfoMixin):
             return remote_paths
 
         # Step 3b: Set execute if we need to. We do this before anything else
-        # because some of the methods below might work but not let us set +x
-        # as part of them.
+        # because some of the methods below might work but not let us set
+        # permissions as part of them.
         if execute:
-            res = self._remote_chmod(remote_paths, 'u+x')
+            res = self._remote_chmod(remote_paths, 'u+rwx')
             if res['rc'] != 0:
                 raise AnsibleError(
                     'Failed to set file mode or acl on remote temporary files '

test/integration/targets/ansiballz_debug/tasks/main.yml

@@ -8,6 +8,18 @@
   set_fact:
     generated_wrapper: "{{ (wrapper.stdout | regex_search('PUT .*? TO (/.*?/AnsiballZ_ping.py)', '\\1'))[0] }}"
 
+- name: Check permissions
+  stat:
+    path: '{{ generated_wrapper }}'
+  register: wrapper_stats
+
+- name: Ensure permissions
+  assert:
+    that:
+      - wrapper_stats.stat.executable is true
+      - wrapper_stats.stat.readable is true
+      - wrapper_stats.stat.writeable is true
+
 - name: Explode the wrapper
   command: "{{ generated_wrapper }} explode"
   register: explode

test/units/plugins/action/test_action.py

@@ -419,7 +419,7 @@ class TestActionBase(unittest.TestCase):
             'stderr': 'and here',
         }
         assertThrowRegex(
-            'Failed to set execute bit on remote files',
+            'Failed to set permissions on remote files',
             execute=True)
 
         # Step 3: we are becoming unprivileged
@@ -434,7 +434,7 @@ class TestActionBase(unittest.TestCase):
         }
         assertSuccess()
 
-        # Step 3b: chmod +x if we need to
+        # Step 3b: chmod +rwx if we need to
         # To get here, setfacl failed, so mock it as such.
         action_base._remote_set_user_facl.return_value = {
             'rc': 1,