From 9f894b81c2137716299ea2071143b72e2a66b3f7 Mon Sep 17 00:00:00 2001
From: Brian Coca <bcoca@users.noreply.github.com>
Date: Wed, 30 Apr 2025 10:09:06 -0400
Subject: [PATCH] ensure predictable permissions on module artifacts (#84948)

and test it!
---
 changelogs/fragments/ensure_remote_perms.yml         |  2 ++
 lib/ansible/plugins/action/__init__.py               | 12 ++++++------
 .../targets/ansiballz_debug/tasks/main.yml           | 12 ++++++++++++
 test/units/plugins/action/test_action.py             |  4 ++--
 4 files changed, 22 insertions(+), 8 deletions(-)
 create mode 100644 changelogs/fragments/ensure_remote_perms.yml

diff --git a/changelogs/fragments/ensure_remote_perms.yml b/changelogs/fragments/ensure_remote_perms.yml
new file mode 100644
index 00000000000..fe6a30588f5
--- /dev/null
+++ b/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.
diff --git a/lib/ansible/plugins/action/__init__.py b/lib/ansible/plugins/action/__init__.py
index 64a16775e54..a69ec36b597 100644
--- a/lib/ansible/plugins/action/__init__.py
+++ b/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 '
diff --git a/test/integration/targets/ansiballz_debug/tasks/main.yml b/test/integration/targets/ansiballz_debug/tasks/main.yml
index 3729adb161b..ff72a21be5c 100644
--- a/test/integration/targets/ansiballz_debug/tasks/main.yml
+++ b/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
diff --git a/test/units/plugins/action/test_action.py b/test/units/plugins/action/test_action.py
index d9158d10564..1eeb2893582 100644
--- a/test/units/plugins/action/test_action.py
+++ b/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,