Repository: ansible/ansible
Pull request: add note about permission change
##### SUMMARY
<!--- Describe the change below, including rationale and design decisions -->

<!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->
Fixes #25389
##### ISSUE TYPE
<!--- Pick one below and delete the rest -->

- Docs Pull Request


##### COMPONENT NAME
<!--- Write the short name of the module, plugin, task or feature below -->
lib/ansible/modules/lineinfile.py
lib/ansible/modules/replace.py
##### ADDITIONAL INFORMATION
<!--- Include additional information to help people understand the change here -->
<!--- A step-by-step reproduction of the problem is helpful if there is no related issue -->

<!--- Paste verbatim command output below, e.g. before and after your change -->
```paste below

```


Problem statement:
file based modules change ownership after editing.
##### ISSUE TYPE
 - Bug Report

##### COMPONENT NAME
lineinfile

##### ANSIBLE VERSION
<!--- Paste verbatim output from “ansible --version” between quotes below -->
```
2.3.0.0
```

##### CONFIGURATION

##### OS / ENVIRONMENT
Centos 7

##### SUMMARY
If you use the lineinfile module to edit a file and if the file belongs to a different user than the user running Ansible, lineinfile will change the owner of the file to the user running Ansible.

##### STEPS TO REPRODUCE
Create a file owned by user A, that is rw to user B via group permissions:
-rw-rw-r-- 1 cloud_ops cloud_ops 119 Jun 1 14:10 hosts
User B in this case belongs to the cloud_ops group.

This is the lineinfile from the playbook

```yaml
      - name: Add the newly created EC2 instance(s) to the local host group
        local_action: lineinfile
                      dest="./hosts"
                      regexp={{ item.private_dns_name }}
                      insertafter="[provisioning]" line={{ item.private_dns_name }}
        with_items:
         - "{{ ec2.instances }}"
```


##### EXPECTED RESULTS
It would be expected that the ownership stay the same.

##### ACTUAL RESULTS
The file will be edited, but the new owner of the file will be user B.

Hints from issue discussion:
I was able to reproduce this bug.
I created `userA` and `userB` as part of `usertest` group

Create `ownership.txt` owned by `userA` but write permissions granted through the group permissions to `userB`.

```
[userA tmp]$ ls -al
total 4
drwxrwxrwx.  7 root  usertest 126 Jun  9 08:21 .
-rw-rw-r--.  1 userA usertest   0 Jun  9 08:20 ownership.txt

I then ran the following playbook, executing with `--become-user='userB'` on the target host:

```
- hosts: all
  tasks:
    - name: own test
      lineinfile:
        dest: /tmp/ownership.txt
        line: "foo"
        state: present
```

[userA tmp]$ ls -al
total 8
drwxrwxrwx.  7 root  usertest 126 Jun  9 08:21 .
dr-xr-xr-x. 17 root  root     224 Jan 31 19:44 ..
-rw-rw-r--.  1 userB userB      4 Jun  9 08:21 ownership.txt
[userA tmp]$ 
```

You can see userB now owns the file

I'm no ansible expert, but it looks like when lineinfile copies into place the temporary file it created to insert the change, it doesn't preserve the file's original ownership. Since the temporary file was created by `userB`, it would then have `userB:userB` ownership. It doesn't look like this is the expected behavior of lineinfile to default to the ownership to the running user.


Then as a side issue, I bet if userB has group rw permission to the file, but not the directory, then the action will fail, even though userB is allowed to write the file.

Secondly, chown is disabled by all but root on most unixes these days. So using the temporary file technique will not be an implementation option for lineinfile, if this behavior is deemed sufficiently egregious to merit correction.

Finally, I assume the component 'replace' works the same way?

Thanks,
cbeckley
@cbeckley Yes you are correct. I hit several Operation not permitted failures in my testing trying to figure out what was going on. The directory i attempted to run this in had the directory flag to prevent deletion. What component "replace" are you referring to?
Replace is a module listed with the File Modules.  Syntax is very similar to lineinfile, even regarding deprecated names for parameters.  So I assumed they had the same author and implementation.
To validate, yes my test has the same result using the replace module.
This is expected as the modules don't edit the files "in situ" as this can lead to inconsistent reads and data corruption, most modules will update the file atomically, which requires making a copy, updating the copy and them moving the copy into place. 

Permissions should also be set on the copy in most scenarios with a couple of exceptions (which you hit above), leaving this open as to either issue a warning or point out the caveat in those scenarios.
Yes, I think a statement regarding this behavior in the synopsis would be sufficient.

Currently the synopsis of both modules indicate that lines will be "changed", "replaced" or "added", which to my mind imply "editing".  I understand that a synopsis is no place for legalese definitions, but I think in this case the distinction is important ... of course that could simply be because I ran into it.

Thanks for looking into this!

Thanks,
Chris
cc @ahtik @dhozac
[click here for bot help](https://github.com/ansible/ansibullbot/blob/master/ISSUE_HELP.md)
<!--- boilerplate: notify --->
cc @samdoran
[click here for bot help](https://github.com/ansible/ansibullbot/blob/master/ISSUE_HELP.md)
<!--- boilerplate: notify --->
This should be clarified in the docs. It's a bit of an edge case so it doesn't happen often.

Commit message: add note about permission change

Patch to review:
diff --git a/lib/ansible/modules/lineinfile.py b/lib/ansible/modules/lineinfile.py
index 0e1b76f5cbad1e..3337627310baed 100644
--- a/lib/ansible/modules/lineinfile.py
+++ b/lib/ansible/modules/lineinfile.py
@@ -17,6 +17,8 @@
   - This module ensures a particular line is in a file, or replace an
     existing line using a back-referenced regular expression.
   - This is primarily useful when you want to change a single line in a file only.
+  - If you use this module to edit a file and if the file belongs to a different user than the user
+    running Ansible, this module will change the owner of the file to the user running Ansible.
   - See the M(ansible.builtin.replace) module if you want to change multiple, similar lines
     or check M(ansible.builtin.blockinfile) if you want to insert/update/remove a block of lines in a file.
     For other cases, see the M(ansible.builtin.copy) or M(ansible.builtin.template) modules.
diff --git a/lib/ansible/modules/replace.py b/lib/ansible/modules/replace.py
index 6dfd6779274128..008d31c8ebcb19 100644
--- a/lib/ansible/modules/replace.py
+++ b/lib/ansible/modules/replace.py
@@ -34,6 +34,8 @@
   - This module will replace all instances of a pattern within a file.
   - It is up to the user to maintain idempotence by ensuring that the
     same pattern would never match any replacements made.
+  - If you use this module to edit a file and if the file belongs to a different user than the user
+    running Ansible, this module will change the owner of the file to the user running Ansible.
 version_added: "1.6"
 options:
   path:


Code review comments:
diff_hunk: @@ -17,6 +17,8 @@
   - This module ensures a particular line is in a file, or replace an
     existing line using a back-referenced regular expression.
   - This is primarily useful when you want to change a single line in a file only.
+  - If you use this module to edit a file and if the file belongs to a different user than the user
+    running Ansible, this module will change the owner of the file to the user running Ansible.
path: lib/ansible/modules/lineinfile.py
text: @user1:
This is not complete, as you can specify `owner` and `group` options, probably better to explain what happens when you don't set these in each option. 'When left unspecified it will use the current user(group of the current user) ...' unless you are root, in which case in CAN preserve the previous ownership

@user1:
also ,this affects all 'file based modules' so updating the common doc fragment at the `owner` and `group` level will 'correct' the information for all of them, not just lineinfile and replace

@author:
applied the fix @user1 . Can you take another look?