From 2b91c57c857b50d16c03e54b01089663eb0a6d26 Mon Sep 17 00:00:00 2001
From: Sloane Hertel <19572925+s-hertel@users.noreply.github.com>
Date: Mon, 12 Aug 2024 10:34:20 -0400
Subject: [PATCH] atomic_move - fix creating file in directory with setgid bit
 (#83718)

* fix creating file in directory with setgid bit

* add a test using the copy module's content option to create a file in a directory with setgid bit

Co-authored-by: Martin Krizek <martin.krizek@gmail.com>
---
 .../46742-atomic_move-fix-setgid.yml          |  2 ++
 lib/ansible/module_utils/basic.py             |  6 ++++-
 .../targets/copy/defaults/main.yml            |  1 +
 test/integration/targets/copy/tasks/main.yml  | 13 +++++++++
 .../integration/targets/copy/tasks/setgid.yml | 27 +++++++++++++++++++
 5 files changed, 48 insertions(+), 1 deletion(-)
 create mode 100644 changelogs/fragments/46742-atomic_move-fix-setgid.yml
 create mode 100644 test/integration/targets/copy/tasks/setgid.yml

diff --git a/changelogs/fragments/46742-atomic_move-fix-setgid.yml b/changelogs/fragments/46742-atomic_move-fix-setgid.yml
new file mode 100644
index 00000000000..4c408262b47
--- /dev/null
+++ b/changelogs/fragments/46742-atomic_move-fix-setgid.yml
@@ -0,0 +1,2 @@
+bugfixes:
+  - atomic_move - fix using the setgid bit on the parent directory when creating files (https://github.com/ansible/ansible/issues/46742, https://github.com/ansible/ansible/issues/67177).
diff --git a/lib/ansible/module_utils/basic.py b/lib/ansible/module_utils/basic.py
index fac1b8d1fda..1bf44b66395 100644
--- a/lib/ansible/module_utils/basic.py
+++ b/lib/ansible/module_utils/basic.py
@@ -1686,8 +1686,12 @@ class AnsibleModule(object):
             umask = os.umask(0)
             os.umask(umask)
             os.chmod(b_dest, S_IRWU_RWG_RWO & ~umask)
+            dest_dir_stat = os.stat(os.path.dirname(b_dest))
             try:
-                os.chown(b_dest, os.geteuid(), os.getegid())
+                if dest_dir_stat.st_mode & stat.S_ISGID:
+                    os.chown(b_dest, os.geteuid(), dest_dir_stat.st_gid)
+                else:
+                    os.chown(b_dest, os.geteuid(), os.getegid())
             except OSError:
                 # We're okay with trying our best here.  If the user is not
                 # root (or old Unices) they won't be able to chown.
diff --git a/test/integration/targets/copy/defaults/main.yml b/test/integration/targets/copy/defaults/main.yml
index 8e9a5836479..ecfbcf21f63 100644
--- a/test/integration/targets/copy/defaults/main.yml
+++ b/test/integration/targets/copy/defaults/main.yml
@@ -1,2 +1,3 @@
 ---
 remote_unprivileged_user: tmp_ansible_test_user
+remote_unprivileged_user_group: test_ansible_test_group
diff --git a/test/integration/targets/copy/tasks/main.yml b/test/integration/targets/copy/tasks/main.yml
index 601312fa089..d46b783d746 100644
--- a/test/integration/targets/copy/tasks/main.yml
+++ b/test/integration/targets/copy/tasks/main.yml
@@ -29,9 +29,15 @@
       with_dict: "{{ symlinks }}"
       delegate_to: localhost
 
+    - name: Create group for remote unprivileged user
+      group:
+        name: '{{ remote_unprivileged_user_group }}'
+      register: group
+
     - name: Create remote unprivileged remote user
       user:
         name: '{{ remote_unprivileged_user }}'
+        group: '{{ remote_unprivileged_user_group }}'
       register: user
 
     - name: Check sudoers dir
@@ -78,6 +84,8 @@
     - import_tasks: selinux.yml
       when: ansible_os_family == 'RedHat' and ansible_selinux.get('mode') == 'enforcing'
 
+    - import_tasks: setgid.yml
+
     - import_tasks: no_log.yml
       delegate_to: localhost
 
@@ -122,6 +130,11 @@
         remove: yes
         force: yes
 
+    - name: Remove group for remote unprivileged user
+      group:
+        name: '{{ remote_unprivileged_user_group }}'
+        state: absent
+
     - name: Remove sudoers.d file
       file:
         path: "{{ sudoers_d_file }}"
diff --git a/test/integration/targets/copy/tasks/setgid.yml b/test/integration/targets/copy/tasks/setgid.yml
new file mode 100644
index 00000000000..66a80b19c4c
--- /dev/null
+++ b/test/integration/targets/copy/tasks/setgid.yml
@@ -0,0 +1,27 @@
+- block:
+    - name: Create test directory
+      file:
+        path: "{{ remote_tmp_dir }}/test_setgid"
+        state: directory
+        mode: '2750'
+        recurse: yes
+        owner: '{{ remote_unprivileged_user }}'
+        group: '{{ remote_unprivileged_user_group }}'
+
+    - name: Test creating a file respects setgid on parent dir
+      copy:
+        content: |
+          test file
+        dest: "{{ remote_tmp_dir }}/test_setgid/test.txt"
+
+    - stat:
+        path: "{{ remote_tmp_dir }}/test_setgid/test.txt"
+      register: result
+
+    - assert:
+        that:
+          - result.stat.gr_name == remote_unprivileged_user_group
+  always:
+    - file:
+        path: "{{ remote_tmp_dir }}/test_setgid"
+        state: absent