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 7ebfd13e4c7..3fd2f729bbd 100644 --- a/lib/ansible/plugins/action/__init__.py +++ b/lib/ansible/plugins/action/__init__.py @@ -634,12 +634,12 @@ class ActionBase(ABC): # 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']))) @@ -680,10 +680,10 @@ class ActionBase(ABC): 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/aliases b/test/integration/targets/ansiballz_debug/aliases new file mode 100644 index 00000000000..ea8d1627352 --- /dev/null +++ b/test/integration/targets/ansiballz_debug/aliases @@ -0,0 +1,3 @@ +shippable/posix/group5 +context/controller +gather_facts/no diff --git a/test/integration/targets/ansiballz_debug/tasks/main.yml b/test/integration/targets/ansiballz_debug/tasks/main.yml new file mode 100644 index 00000000000..ff72a21be5c --- /dev/null +++ b/test/integration/targets/ansiballz_debug/tasks/main.yml @@ -0,0 +1,59 @@ +- name: Run a module while preserving the generated AnsiballZ wrapper + command: ansible -m ping localhost -vvv + environment: + ANSIBLE_KEEP_REMOTE_FILES: 1 + register: wrapper + +- name: Locate the generated AnsiballZ wrapper + 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 + +- name: Locate the exploded results + set_fact: + exploded_dir: "{{ (explode.stdout | regex_search('^Module expanded into:\n(.*)$', '\\1', multiline=True))[0] }}" + +- name: Spot check the exploded results contents + assert: + that: + - (exploded_dir + '/args') is file + - (exploded_dir + '/ansible/modules/ping.py') is file + +- name: Execute the wrapper + command: "{{ generated_wrapper }} execute" + register: execute + +- name: Deserialize the result + set_fact: + result: "{{ execute.stdout | from_json }}" + +- name: Spot check the result + assert: + that: + - result.invocation.module_args.data == "pong" + - result.ping == "pong" + +- name: Remove wrapper + file: + path: "{{ generated_wrapper }}" + state: absent + +- name: Remove exploded files + file: + path: "{{ exploded_dir }}" + state: absent diff --git a/test/units/plugins/action/test_action.py b/test/units/plugins/action/test_action.py index 7d9c2915dd3..98b5c578dad 100644 --- a/test/units/plugins/action/test_action.py +++ b/test/units/plugins/action/test_action.py @@ -401,7 +401,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 @@ -416,7 +416,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,