[BUG][DISCUSSION] `file.manage_file` might cause race condition due to permission config

Description
The file.manage_file module can cause unexpected problems due to the way it handles permissions. I am happy to provide a PR, but wanted first discuss which approach would make the most sense to the maintainers.

The file.manage_file module currently works as such:

1. Create a tmp file
2. Copy tmp file to dst
3. Set permissions on the dst

The tmp file always has the 600 permissions, together with the current user + group.

Race condition case

• Salt is modifying a file in use, for example /etc/hosts, /etc/nsswitch.conf, or /etc/nscd.conf.
• Salt finishes executing step 2. - new file is copied over the old file. However, the new file uses the 600 permissions.
• The system attempts to use the file before step 3 finishes, and fails in its attempt.

Downstream issue:

• https://github.com/SUSE/spacewalk/issues/20911

Discussion

This is an understandable issue--we are modifying a file that is currently in use, and we cannot guarantee correct file permissions while the module is still executing. However, in some cases (like managing DNS in a running environment), it is impractical to stop the service that uses the file.

A few solutions that come to my mind:

Solution 1

Possibly the least intrusive way to solve the issue is to ensure that the temporary file uses the final permissions. The advantage is that when the tmp file is copied to destination, it's immediately ready to be used.

I can imagine corner cases, like changing the permissions such that salt itself cannot write into the temp file. We could mitigate this by having an additional property, e.g. atomic_permissions, that'd turn off the behavior by default.

Solution 2

We could alternatively modify the salt.utils.files.copyfile function to include permissions, and do a best-effort user/group/mode config within the function.

This would mitigate the issue by greatly reducing the window between file copying and file permission change. The issue would not be 100% solved though.

The disadvantage, in my mind, is that this would have to be a best-effort user/group/mode configuration because we probably wouldn't want to duplicate or pull in https://github.com/saltstack/salt/blob/master/salt/modules/file.py#L5069 into the utils module.

Do you have any preference for the approach to fix/mitigate the issue? Alternatively, is there another solution that I haven't considered?