[stable-2.9] allow env to override unspecified unsafe_writes (#73282) (#75397)

* [stable-2.9] allow env to override unspecified unsafe_writes (#73282)

* allow env var for fallback value for unspecified unsafe_writes.
(cherry picked from commit c7d4acc12f)

Co-authored-by: Brian Coca <bcoca@users.noreply.github.com>

* ensure unsafe writes fallback (#70722)

* Ensure we actually fallback to unsafe_writes when set to true

 add integration test
 add fix for get_url not passing the parameter from args

(cherry picked from commit 932ba36160)

* Added clog missing for issue 70722 (#73175)

(cherry picked from commit d6670da1d7)

Co-authored-by: Brian Coca <bcoca@users.noreply.github.com>

changelogs/fragments/unsafe_writes_env.yml

@@ -0,0 +1,2 @@
+minor_changes:
+  - Allow unsafe_writes to be set on target via env var, for those targets that need a blanket setting.

changelogs/fragments/unsafe_writes_fix.yml

@@ -0,0 +1,4 @@
+bugfixes:
+  - Restored unsafe_writes functionality which was being skipped.
+  - Added unsafe_writes test.
+  - Enabled unsafe_writes for get_url which was ignoring the paramter.

lib/ansible/module_utils/basic.py

@@ -228,6 +228,15 @@ _literal_eval = literal_eval
 # is an internal implementation detail
 _ANSIBLE_ARGS = None
 
+
+def env_fallback(*args, **kwargs):
+    ''' Load value from environment '''
+    for arg in args:
+        if arg in os.environ:
+            return os.environ[arg]
+    raise AnsibleFallbackNotFound
+
+
 FILE_COMMON_ARGUMENTS = dict(
     # These are things we want. About setting metadata (mode, ownership, permissions in general) on
     # created files (these are used by set_fs_attributes_if_different and included in
@@ -255,7 +264,7 @@ FILE_COMMON_ARGUMENTS = dict(
     regexp=dict(),  # used by assemble
     delimiter=dict(),  # used by assemble
     directory_mode=dict(),  # used by copy
-    unsafe_writes=dict(type='bool'),  # should be available to any module using atomic_move
+    unsafe_writes=dict(type='bool', default=False, fallback=(env_fallback, ['ANSIBLE_UNSAFE_WRITES'])),  # should be available to any module using atomic_move
 )
 
 PASSWD_ARG_RE = re.compile(r'^[-]{0,2}pass[-]?(word|wd)?')
@@ -641,14 +650,6 @@ def _load_params():
         sys.exit(1)
 
 
-def env_fallback(*args, **kwargs):
-    ''' Load value from environment '''
-    for arg in args:
-        if arg in os.environ:
-            return os.environ[arg]
-    raise AnsibleFallbackNotFound
-
-
 def missing_required_lib(library, reason=None, url=None):
     hostname = platform.node()
     msg = "Failed to import the required Python library (%s) on %s's Python %s." % (library, hostname, sys.executable)
@@ -2357,8 +2358,7 @@ class AnsibleModule(object):
             if e.errno not in [errno.EPERM, errno.EXDEV, errno.EACCES, errno.ETXTBSY, errno.EBUSY]:
                 # only try workarounds for errno 18 (cross device), 1 (not permitted),  13 (permission denied)
                 # and 26 (text file busy) which happens on vagrant synced folders and other 'exotic' non posix file systems
-                self.fail_json(msg='Could not replace file: %s to %s: %s' % (src, dest, to_native(e)),
-                               exception=traceback.format_exc())
+                self.fail_json(msg='Could not replace file: %s to %s: %s' % (src, dest, to_native(e)), exception=traceback.format_exc())
             else:
                 # Use bytes here.  In the shippable CI, this fails with
                 # a UnicodeError with surrogateescape'd strings for an unknown
@@ -2368,14 +2368,13 @@ class AnsibleModule(object):
                 error_msg = None
                 tmp_dest_name = None
                 try:
-                    tmp_dest_fd, tmp_dest_name = tempfile.mkstemp(prefix=b'.ansible_tmp',
-                                                                  dir=b_dest_dir, suffix=b_suffix)
+                    tmp_dest_fd, tmp_dest_name = tempfile.mkstemp(prefix=b'.ansible_tmp', dir=b_dest_dir, suffix=b_suffix)
                 except (OSError, IOError) as e:
                     error_msg = 'The destination directory (%s) is not writable by the current user. Error was: %s' % (os.path.dirname(dest), to_native(e))
                 except TypeError:
                     # We expect that this is happening because python3.4.x and
-                    # below can't handle byte strings in mkstemp().  Traceback
-                    # would end in something like:
+                    # below can't handle byte strings in mkstemp().
+                    # Traceback would end in something like:
                     #     file = _os.path.join(dir, pre + name + suf)
                     # TypeError: can't concat bytes to str
                     error_msg = ('Failed creating tmp file for atomic move.  This usually happens when using Python3 less than Python3.5. '
@@ -2419,11 +2418,12 @@ class AnsibleModule(object):
                                     self._unsafe_writes(b_tmp_dest_name, b_dest)
                                 else:
                                     self.fail_json(msg='Unable to make %s into to %s, failed final rename from %s: %s' %
-                                                       (src, dest, b_tmp_dest_name, to_native(e)),
-                                                   exception=traceback.format_exc())
+                                                       (src, dest, b_tmp_dest_name, to_native(e)), exception=traceback.format_exc())
                         except (shutil.Error, OSError, IOError) as e:
-                            self.fail_json(msg='Failed to replace file: %s to %s: %s' % (src, dest, to_native(e)),
-                                           exception=traceback.format_exc())
+                            if unsafe_writes:
+                                self._unsafe_writes(b_src, b_dest)
+                            else:
+                                self.fail_json(msg='Failed to replace file: %s to %s: %s' % (src, dest, to_native(e)), exception=traceback.format_exc())
                     finally:
                         self.cleanup(b_tmp_dest_name)
 

lib/ansible/modules/net_tools/basics/get_url.py

@@ -620,7 +620,7 @@ def main():
             if backup:
                 if os.path.exists(dest):
                     backup_file = module.backup_local(dest)
-            module.atomic_move(tmpsrc, dest)
+            module.atomic_move(tmpsrc, dest, unsafe_writes=module.params['unsafe_writes'])
         except Exception as e:
             if os.path.exists(tmpsrc):
                 os.remove(tmpsrc)

test/integration/targets/unsafe_writes/aliases

@@ -0,0 +1,6 @@
+needs/root
+skip/freebsd
+skip/osx
+skip/macos
+skip/aix
+shippable/posix/group3

test/integration/targets/unsafe_writes/basic.yml

@@ -0,0 +1,68 @@
+- hosts: testhost
+  gather_facts: false
+  vars:
+    testudir: '{{output_dir}}/unsafe_writes_test'
+    testufile: '{{testudir}}/unreplacablefile.txt'
+  tasks:
+    - name: test unsafe_writes on immutable dir (file cannot be atomically replaced)
+      block:
+      - name: create target dir
+        file: path={{testudir}} state=directory
+      - name: setup test file
+        copy: content=ORIGINAL dest={{testufile}}
+      - name: make target dir immutable (cannot write to file w/o unsafe_writes)
+        file: path={{testudir}} state=directory attributes="+i"
+        become: yes
+        ignore_errors: true
+        register: madeimmutable
+
+      - name: only run if immutable dir command worked, some of our test systems don't allow for it
+        when: madeimmutable is success
+        block:
+        - name: test this is actually immmutable working as we expect
+          file: path={{testufile}} state=absent
+          register: breakimmutable
+          ignore_errors: True
+
+        - name: only run if reallyh immutable dir
+          when: breakimmutable is failed
+          block:
+          - name: test overwriting file w/o unsafe
+            copy: content=NEW dest={{testufile}} unsafe_writes=False
+            ignore_errors: true
+            register: copy_without
+
+          - name: ensure we properly failed
+            assert:
+              that:
+                - copy_without is failed
+
+          - name: test overwriting file with unsafe
+            copy: content=NEWNOREALLY dest={{testufile}} unsafe_writes=True
+            register: copy_with
+
+          - name: ensure we properly changed
+            assert:
+              that:
+                - copy_with is changed
+
+          - name: test fallback env var
+            when: lookup('env', 'ANSIBLE_UNSAFE_WRITES') not in ('', None)
+            vars:
+              env_enabled: "{{lookup('env', 'ANSIBLE_UNSAFE_WRITES')|bool}}"
+            block:
+            - name: test overwriting file with unsafe depending on fallback environment setting
+              copy: content=NEWBUTNOTDIFFERENT dest={{testufile}}
+              register: copy_with_env
+              ignore_errors: True
+
+            - name: ensure we properly follow env var
+              assert:
+                msg: "Failed with envvar: {{env_enabled}}, due AUW: to {{q('env', 'ANSIBLE_UNSAFE_WRITES')}}"
+                that:
+                  - env_enabled and copy_with_env is changed or not env_enabled and copy_with_env is failed
+      always:
+      - name: remove immutable flag from dir to prevent issues with cleanup
+        file: path={{testudir}} state=directory attributes="-i"
+        ignore_errors: true
+        become: yes

test/integration/targets/unsafe_writes/runme.sh

@@ -0,0 +1,12 @@
+#!/usr/bin/env bash
+
+set -eux
+
+# test w/o fallback env var
+ansible-playbook basic.yml -i ../../inventory -e "output_dir=${OUTPUT_DIR}" "$@"
+
+# test enabled fallback env var
+ANSIBLE_UNSAFE_WRITES=1 ansible-playbook basic.yml -i ../../inventory -e "output_dir=${OUTPUT_DIR}" "$@"
+
+# test disnabled fallback env var
+ANSIBLE_UNSAFE_WRITES=0 ansible-playbook basic.yml -i ../../inventory -e "output_dir=${OUTPUT_DIR}" "$@"

test/units/module_utils/basic/test_atomic_move.py

@@ -23,6 +23,7 @@ def atomic_am(am, mocker):
     am.selinux_context = mocker.MagicMock()
     am.selinux_default_context = mocker.MagicMock()
     am.set_context_if_different = mocker.MagicMock()
+    am._unsafe_writes = mocker.MagicMock()
 
     yield am