Skip to content

Commit

Permalink
Fix the script and patch plugins tempfile ownership
Browse files Browse the repository at this point in the history
Unified tmp accidentally removed the containing tmpdir from the list of
files to fix the permissions on when we're becoming a different
unprivileged user.  This resulted in a visible bug for script but not
for patch.  This is because patch also uploads the module to the same
temporary directory and the uploaded module also ends up calling
fixup_perms2() which includes the temporary directory.  So by the time
patch needs to access the temporary patch file, the directory is
appropriately set.

script's breakage was visible because script does not upload a module
(it's akin to raw in this way).  Therefore, we only call fixup_perms2()
once in script and so leaving out the tmpdir in script means that the
containing directory never has its permissions set appropriately.

Fixing both because it does not cause an extra round trip for patch so
any speedup would be minimal and it's better to fix the perms as close
as possible to where we know we need it.  Otherwise, changes to
seemingly unrelated code later could end up breaking it.

Fixes ansible#36398
  • Loading branch information
abadger committed Feb 19, 2018
1 parent 4a5809f commit d1f9bea
Show file tree
Hide file tree
Showing 2 changed files with 2 additions and 2 deletions.
2 changes: 1 addition & 1 deletion lib/ansible/plugins/action/patch.py
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ def run(self, tmp=None, task_vars=None):

tmp_src = self._connection._shell.join_path(self._connection._shell.tmpdir, os.path.basename(src))
self._transfer_file(src, tmp_src)
self._fixup_perms2((tmp_src,))
self._fixup_perms2((self._connection._shell.tmpdir, tmp_src))

new_module_args = self._task.args.copy()
new_module_args.update(
Expand Down
2 changes: 1 addition & 1 deletion lib/ansible/plugins/action/script.py
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ def run(self, tmp=None, task_vars=None):
self._transfer_file(source, tmp_src)

# set file permissions, more permissive when the copy is done as a different user
self._fixup_perms2((tmp_src,), execute=True)
self._fixup_perms2((self._connection._shell.tmpdir, tmp_src), execute=True)

# add preparation steps to one ssh roundtrip executing the script
env_dict = dict()
Expand Down

0 comments on commit d1f9bea

Please sign in to comment.