diff --git a/changelogs/fragments/44412-copy-fix-unwanted-acls.yaml b/changelogs/fragments/44412-copy-fix-unwanted-acls.yaml
new file mode 100644
index 00000000000..1f6a88a0246
--- /dev/null
+++ b/changelogs/fragments/44412-copy-fix-unwanted-acls.yaml
@@ -0,0 +1,2 @@
+bugfixes:
+ - Fix unwanted ACLs when using copy module (https://github.com/ansible/ansible/issues/44412)
diff --git a/lib/ansible/modules/files/copy.py b/lib/ansible/modules/files/copy.py
index 8a102a820a0..0bbbf5a6ba6 100644
--- a/lib/ansible/modules/files/copy.py
+++ b/lib/ansible/modules/files/copy.py
@@ -257,19 +257,26 @@ state:
sample: file
'''
+import errno
+import filecmp
+import grp
import os
import os.path
-import shutil
-import filecmp
+import platform
import pwd
-import grp
+import shutil
import stat
-import errno
import tempfile
import traceback
from ansible.module_utils.basic import AnsibleModule
+from ansible.module_utils.common.process import get_bin_path
from ansible.module_utils._text import to_bytes, to_native
+from ansible.module_utils.six import PY3
+
+
+# The AnsibleModule object
+module = None
class AnsibleModuleError(Exception):
@@ -277,6 +284,20 @@ class AnsibleModuleError(Exception):
self.results = results
+# Once we get run_command moved into common, we can move this into a common/files module. We can't
+# until then because of the module.run_command() method. We may need to move it into
+# basic::AnsibleModule() until then but if so, make it a private function so that we don't have to
+# keep it for backwards compatibility later.
+def clear_facls(path):
+ setfacl = get_bin_path('setfacl', True)
+ # FIXME "setfacl -b" is available on Linux and FreeBSD. There is "setfacl -D e" on z/OS. Others?
+ acl_command = [setfacl, '-b', path]
+ b_acl_command = [to_bytes(x) for x in acl_command]
+ rc, out, err = module.run_command(b_acl_command, environ_update=dict(LANG='C', LC_ALL='C', LC_MESSAGES='C'))
+ if rc != 0:
+ raise RuntimeError('Error running "{0}": stdout: "{1}"; stderr: "{2}"'.format(' '.join(b_acl_command), out, err))
+
+
def split_pre_existing_dir(dirname):
'''
Return the first pre-existing directory and a list of the new directories that will be created.
@@ -467,6 +488,8 @@ def copy_common_dirs(src, dest, module):
def main():
+ global module
+
module = AnsibleModule(
# not checking because of daisy chain to file module
argument_spec=dict(
@@ -631,7 +654,55 @@ def main():
module.warn("Unable to copy stats {0}".format(to_native(b_src)))
else:
raise
+
+ # might be needed below
+ if PY3 and hasattr(os, 'listxattr'):
+ try:
+ src_has_acls = 'system.posix_acl_access' in os.listxattr(src)
+ except Exception as e:
+ # assume unwanted ACLs by default
+ src_has_acls = True
+
module.atomic_move(b_mysrc, dest, unsafe_writes=module.params['unsafe_writes'])
+
+ if PY3 and hasattr(os, 'listxattr') and platform.system() == 'Linux' and not remote_src:
+ # atomic_move used above to copy src into dest might, in some cases,
+ # use shutil.copy2 which in turn uses shutil.copystat.
+ # Since Python 3.3, shutil.copystat copies file extended attributes:
+ # https://docs.python.org/3/library/shutil.html#shutil.copystat
+ # os.listxattr (along with others) was added to handle the operation.
+
+ # This means that on Python 3 we are copying the extended attributes which includes
+ # the ACLs on some systems - further limited to Linux as the documentation above claims
+ # that the extended attributes are copied only on Linux. Also, os.listxattr is only
+ # available on Linux.
+
+ # If not remote_src, then the file was copied from the controller. In that
+ # case, any filesystem ACLs are artifacts of the copy rather than preservation
+ # of existing attributes. Get rid of them:
+
+ if src_has_acls:
+ # FIXME If dest has any default ACLs, there are not applied to src now because
+ # they were overridden by copystat. Should/can we do anything about this?
+ # 'system.posix_acl_default' in os.listxattr(os.path.dirname(b_dest))
+
+ try:
+ clear_facls(dest)
+ except ValueError as e:
+ if 'setfacl' in to_native(e):
+ # No setfacl so we're okay. The controller couldn't have set a facl
+ # without the setfacl command
+ pass
+ else:
+ raise
+ except RuntimeError as e:
+ # setfacl failed.
+ if 'Operation not supported' in to_native(e):
+ # The file system does not support ACLs.
+ pass
+ else:
+ raise
+
except (IOError, OSError):
module.fail_json(msg="failed to copy: %s to %s" % (src, dest), traceback=traceback.format_exc())
changed = True
diff --git a/lib/ansible/plugins/action/__init__.py b/lib/ansible/plugins/action/__init__.py
index 3682145ec7e..ba70552bf1a 100644
--- a/lib/ansible/plugins/action/__init__.py
+++ b/lib/ansible/plugins/action/__init__.py
@@ -407,6 +407,20 @@ class ActionBase(with_metaclass(ABCMeta, object)):
self._connection._shell.tmpdir = None
def _transfer_file(self, local_path, remote_path):
+ """
+ Copy a file from the controller to a remote path
+
+ :arg local_path: Path on controller to transfer
+ :arg remote_path: Path on the remote system to transfer into
+
+ .. warning::
+ * When you use this function you likely want to use use fixup_perms2() on the
+ remote_path to make sure that the remote file is readable when the user becomes
+ a non-privileged user.
+ * If you use fixup_perms2() on the file and copy or move the file into place, you will
+ need to then remove filesystem acls on the file once it has been copied into place by
+ the module. See how the copy module implements this for help.
+ """
self._connection.put_file(local_path, remote_path)
return remote_path
diff --git a/lib/ansible/plugins/action/copy.py b/lib/ansible/plugins/action/copy.py
index 198c55c2a5a..ef14dd37981 100644
--- a/lib/ansible/plugins/action/copy.py
+++ b/lib/ansible/plugins/action/copy.py
@@ -305,6 +305,10 @@ class ActionModule(ActionBase):
self._remove_tempfile_if_content_defined(content, content_tempfile)
self._loader.cleanup_tmp_file(source_full)
+ # FIXME: I don't think this is needed when PIPELINING=0 because the source is created
+ # world readable. Access to the directory itself is controlled via fixup_perms2() as
+ # part of executing the module. Check that umask with scp/sftp/piped doesn't cause
+ # a problem before acting on this idea. (This idea would save a round-trip)
# fix file permissions when the copy is done as a different user
if remote_path:
self._fixup_perms2((self._connection._shell.tmpdir, remote_path))
diff --git a/test/integration/targets/copy/tasks/acls.yml b/test/integration/targets/copy/tasks/acls.yml
new file mode 100644
index 00000000000..9a3be9b0640
--- /dev/null
+++ b/test/integration/targets/copy/tasks/acls.yml
@@ -0,0 +1,33 @@
+- block:
+ - block:
+ - name: Testing ACLs
+ copy:
+ content: "TEST"
+ mode: 0644
+ dest: "~/test.txt"
+
+ - shell: getfacl ~/test.txt
+ register: acls
+
+ become: yes
+ become_user: "{{ remote_unprivileged_user }}"
+
+ - name: Check that there are no ACLs leftovers
+ assert:
+ that:
+ - "'user:{{ remote_unprivileged_user }}:r-x\t#effective:r--' not in acls.stdout_lines"
+
+ - name: Check that permissions match with what was set in the mode param
+ assert:
+ that:
+ - "'user::rw-' in acls.stdout_lines"
+ - "'group::r--' in acls.stdout_lines"
+ - "'other::r--' in acls.stdout_lines"
+
+ always:
+ - name: Clean up
+ file:
+ path: "~/test.txt"
+ state: absent
+ become: yes
+ become_user: "{{ remote_unprivileged_user }}"
diff --git a/test/integration/targets/copy/tasks/main.yml b/test/integration/targets/copy/tasks/main.yml
index e1ae5a46b6d..f1861ea7f8b 100644
--- a/test/integration/targets/copy/tasks/main.yml
+++ b/test/integration/targets/copy/tasks/main.yml
@@ -58,6 +58,9 @@
- import_tasks: tests.yml
remote_user: '{{ remote_unprivileged_user }}'
+ - import_tasks: acls.yml
+ when: ansible_system == 'Linux'
+
always:
- name: Cleaning
file: