pyinfra icon indicating copy to clipboard operation
pyinfra copied to clipboard

file.put with _sudo=True and _chdir is not None can fail if the sudo is required for the chdir to succeed

Open morrison12 opened this issue 4 months ago • 5 comments

Describe the bug

Removal of the temporary file fails in the case where the sudo is required for the chdir to succeed/

To Reproduce

Any file.put with:

  1. _chdir is not None`
  2. _sudo=True and
  3. the sudo is required for the `chdir to succeed

Expected behavior

The full operation should succeed.

Meta

System: Darwin Platform: macOS-14.7.7-arm64-arm-64bit Release: 23.6.0 Machine: arm64 pyinfra: v3.4.1 click: v8.2.1 click: v8.2.1 click: v8.2.1 distro: v1.9.0 gevent: v25.5.1 jinja2: v3.1.6 packaging: v25.0 paramiko: v3.5.1 python-dateutil: v2.9.0.post0 pywinrm: v0.5.0 typeguard: v4.4.4 typing-extensions: v4.14.1 Executable: /Users/someone/.local/bin/pyinfra Python: 3.12.11 (CPython, Clang 20.1.4 )

morrison12 avatar Aug 12 '25 00:08 morrison12

Hm. The _sudo argument should be applied before any _chdir resulting in a command like this:

sudo -H -n sh -c 'cd /tmp && echo hi'

Which should then allow the cd and command to run in the same sudo (or not) context. Will try to setup a repro for this. In the meantime @morrison12 could you paste logs here showing the command being run, might help.

Fizzadar avatar Aug 19 '25 13:08 Fizzadar

I have a repro but it assumes the change for #1425 in place to make it work as without it things won't work at all on the restrictive system (that doesn't allow execution of files in /tmp) so a bit messy..

Basically one needs to upload a file where the _chdir requires _sudo , imagine something like:


who = "something"
dir_path = f"/var/services/home/{who}"
file_path = f"{dir_path}/foo"
ugc = {"user": who, "group": "somegroup", "_chdir": dir_path}
content = io.StringIO()
content.write("very important info")

files.directory(
    dir_path, present=True, mode=700, name=f"Ensure some directory directory exists", **ugc)
files.put(content, file_path, mode=600, name="Ensure file has right contents", **ugc)

plus whatever whatever is required to get the ssh connection and sudo to work.

Then if the file does need to be uploaded, the

and removal of the askpass fails like this:

    [pyinfra.connectors.ssh] Running command on hlf.example.com: (pty=False) sh -c 'cd /var/services/homes/something && rm -f /volume1/tfr/tmp/pyinfra-6b6c971ce9dd9aaea30991dbaa63ef13f3cc0e4c'
[hlf.bevjam.com] >>> sh -c 'cd /var/services/homes/something && rm -f /volume1/tfr/tmp/pyinfra-6b6c971ce9dd9aaea30991dbaa63ef13f3cc0e4c'
[hlf.bevjam.com] sh: line 0: cd: /var/services/homes/something: Permission denied
    [pyinfra.connectors.ssh] Waiting for exit status...
    [pyinfra.connectors.ssh] Command exit status: 1
    Unable to remove temporary file: sh: line 0: cd: /var/services/homes/something: Permission denied
    [hlf.bevjam.com] Error: executed 0 commands
    [pyinfra.api.state] Failing hosts: hlf.example.com

--> Disconnecting from hosts...
    [pyinfra.connectors.ssh] Running command on hlf.example.com: (pty=False) sh -c 'rm -f /volume1/tfr/tmp/pyinfra-sudo-askpass-z88il0GNoEif'
    [pyinfra.connectors.ssh] Waiting for exit status...
    [pyinfra.connectors.ssh] Command exit status: 0

--> pyinfra error: No hosts remaining!

morrison12 avatar Aug 26 '25 01:08 morrison12

To elaborate on the logs above, the removal of askpass doesn't seem to be inside the sudo but the sudo is required for access to the _chdir

morrison12 avatar Sep 01 '25 20:09 morrison12

Hm this appears to be failing at the prior command:

[hlf.bevjam.com] >>> sh -c 'cd /var/services/homes/something && rm -f /volume1/tfr/tmp/pyinfra-6b6c971ce9dd9aaea30991dbaa63ef13f3cc0e4c'

The askpass removal succeeds here:

--> Disconnecting from hosts...
    [pyinfra.connectors.ssh] Running command on hlf.example.com: (pty=False) sh -c 'rm -f /volume1/tfr/tmp/pyinfra-sudo-askpass-z88il0GNoEif'
    [pyinfra.connectors.ssh] Waiting for exit status...
    [pyinfra.connectors.ssh] Command exit status: 0

Re - the failing command - it looks like the files.put call is uploading the file successfully (without sudo) and the same user then can't remove that file? The "Unable to remove temporary file" line is from the upload path in the connector: https://github.com/pyinfra-dev/pyinfra/blob/3.x/src/pyinfra/connectors/ssh.py#L563

Fizzadar avatar Sep 16 '25 19:09 Fizzadar

Apologies for the delay..

The simplest example I can come up with is basically someone trying to add a file (using sudo) to a directory they normally don't have access to (e.g. imagine someone using pyinfra to setup a directory for a new user):

  1 import io
  2
  3 from pyinfra.operations import files
  4
  5 USER_DIR = "/home/grace" # any directory the account running pyinfra doesn't have access to
  6
  7 content = io.StringIO("FOO")
  8 file_name = "foo"
  9
 10 files.put(content, file_name, name='create a file in the new directory', _chdir=USER_DIR, _sudo=True)

The standard output is:

--> Loading config...
--> Loading inventory...
--> Connecting to hosts...
    [domo] Connected

--> Preparing operation files...
    Loading: test.py
    [domo] Ready: test.py

--> Detected changes:
    Operation                            Change     Conditional Change
    create a file in the new directory   1 (domo)   -

    Detected changes may not include every change pyinfra will execute.
    Hidden side effects of operations may alter behaviour of future operations,
    this will be shown in the results. The remote state will always be updated
    to reflect the state defined by the input operations.

    Detected changes displayed above, skip this step with -y

--> Beginning operation run...
--> Starting operation: create a file in the new directory
    Unable to remove temporary file: sh: line 1: cd: /home/grace: Permission denied
    [domo] Error: executed 0 commands

--> Disconnecting from hosts...
--> pyinfra error: No hosts remaining!

and the output with --debug and -vvvvv is:

--> Loading config...
--> Loading inventory...
    [pyinfra_cli.inventory] Creating fake inventory...
    [pyinfra_cli.inventory] Checking possible group_data at: /.../group_data

--> Connecting to hosts...
    [pyinfra.connectors.ssh] Connecting to: domo ({'allow_agent': True, 'look_for_keys': True, '_pyinfra_ssh_forward_agent': False, '_pyinfra_ssh_config_file': None, '_pyinfra_ssh_known_hosts_file': None, '_pyinfra_ssh_strict_host_key_checking': 'accept-new', '_pyinfra_ssh_paramiko_connect_kwargs': None, 'timeout': 10})
    [pyinfra.connectors.sshuserclient.client] Loading SSH config: None
    [domo] Connected
    [pyinfra.api.state] Activating host: domo

--> Preparing operation files...
    Loading: test.py
    [pyinfra.api.operation] Adding operation, {'create a file in the new directory'}, opOrder=(0, 10), opHash=3b2f90227d43a40a2288506c2a99bdc5e71f32b2
    [pyinfra.api.facts] Getting fact: files.File (path=foo) (ensure_hosts: None)
    [pyinfra.connectors.ssh] Running command on domo: (pty=False) sudo -H -n sh -c 'cd /home/grace && ! (test -e foo || test -L foo ) || ( stat -c '"'"'user=%U group=%G mode=%A atime=%X mtime=%Y ctime=%Z size=%s %N'"'"' foo 2> /dev/null || stat -f '"'"'user=%Su group=%Sg mode=%Sp atime=%a mtime=%m ctime=%c size=%z %N%SY'"'"' foo || ls -ld foo )'
[domo] >>> sudo -H -n sh -c 'cd /home/grace && ! (test -e foo || test -L foo ) || ( stat -c '"'"'user=%U group=%G mode=%A atime=%X mtime=%Y ctime=%Z size=%s %N'"'"' foo 2> /dev/null || stat -f '"'"'user=%Su group=%Sg mode=%Sp atime=%a mtime=%m ctime=%c size=%z %N%SY'"'"' foo || ls -ld foo )'
    [pyinfra.connectors.ssh] Waiting for exit status...
    [pyinfra.connectors.ssh] Command exit status: 0
    [domo] Loaded fact files.File (path=foo)
    [pyinfra.api.facts] Getting fact: files.Directory (path=foo) (ensure_hosts: None)
    [pyinfra.connectors.ssh] Running command on domo: (pty=False) sudo -H -n sh -c 'cd /home/grace && ! (test -e foo || test -L foo ) || ( stat -c '"'"'user=%U group=%G mode=%A atime=%X mtime=%Y ctime=%Z size=%s %N'"'"' foo 2> /dev/null || stat -f '"'"'user=%Su group=%Sg mode=%Sp atime=%a mtime=%m ctime=%c size=%z %N%SY'"'"' foo || ls -ld foo )'
[domo] >>> sudo -H -n sh -c 'cd /home/grace && ! (test -e foo || test -L foo ) || ( stat -c '"'"'user=%U group=%G mode=%A atime=%X mtime=%Y ctime=%Z size=%s %N'"'"' foo 2> /dev/null || stat -f '"'"'user=%Su group=%Sg mode=%Sp atime=%a mtime=%m ctime=%c size=%z %N%SY'"'"' foo || ls -ld foo )'
    [pyinfra.connectors.ssh] Waiting for exit status...
    [pyinfra.connectors.ssh] Command exit status: 0
    [domo] Loaded fact files.Directory (path=foo)
    [pyinfra.api.facts] Getting fact: server.TmpDir () (ensure_hosts: None)
    [pyinfra.connectors.ssh] Running command on domo: (pty=False) sudo -H -n sh -c 'cd /home/grace && echo $TMPDIR'
[domo] >>> sudo -H -n sh -c 'cd /home/grace && echo $TMPDIR'
[domo]
    [pyinfra.connectors.ssh] Waiting for exit status...
    [pyinfra.connectors.ssh] Command exit status: 0
    [domo] Loaded fact server.TmpDir
    [domo] Ready: test.py

--> Detected changes:
    Operation                            Change     Conditional Change
    create a file in the new directory   1 (domo)   -

    Detected changes may not include every change pyinfra will execute.
    Hidden side effects of operations may alter behaviour of future operations,
    this will be shown in the results. The remote state will always be updated
    to reflect the state defined by the input operations.

    Detected changes displayed above, skip this step with -y

--> Beginning operation run...
--> Starting operation: create a file in the new directory
    [pyinfra.api.operations] Starting operation {'create a file in the new directory'} on domo
    [pyinfra.api.facts] Getting fact: files.File (path=foo) (ensure_hosts: None)
    [pyinfra.connectors.ssh] Running command on domo: (pty=False) sudo -H -n sh -c 'cd /home/grace && ! (test -e foo || test -L foo ) || ( stat -c '"'"'user=%U group=%G mode=%A atime=%X mtime=%Y ctime=%Z size=%s %N'"'"' foo 2> /dev/null || stat -f '"'"'user=%Su group=%Sg mode=%Sp atime=%a mtime=%m ctime=%c size=%z %N%SY'"'"' foo || ls -ld foo )'
[domo] >>> sudo -H -n sh -c 'cd /home/grace && ! (test -e foo || test -L foo ) || ( stat -c '"'"'user=%U group=%G mode=%A atime=%X mtime=%Y ctime=%Z size=%s %N'"'"' foo 2> /dev/null || stat -f '"'"'user=%Su group=%Sg mode=%Sp atime=%a mtime=%m ctime=%c size=%z %N%SY'"'"' foo || ls -ld foo )'
    [pyinfra.connectors.ssh] Waiting for exit status...
    [pyinfra.connectors.ssh] Command exit status: 0
    [domo] Loaded fact files.File (path=foo)
    [pyinfra.api.facts] Getting fact: files.Directory (path=foo) (ensure_hosts: None)
    [pyinfra.connectors.ssh] Running command on domo: (pty=False) sudo -H -n sh -c 'cd /home/grace && ! (test -e foo || test -L foo ) || ( stat -c '"'"'user=%U group=%G mode=%A atime=%X mtime=%Y ctime=%Z size=%s %N'"'"' foo 2> /dev/null || stat -f '"'"'user=%Su group=%Sg mode=%Sp atime=%a mtime=%m ctime=%c size=%z %N%SY'"'"' foo || ls -ld foo )'
[domo] >>> sudo -H -n sh -c 'cd /home/grace && ! (test -e foo || test -L foo ) || ( stat -c '"'"'user=%U group=%G mode=%A atime=%X mtime=%Y ctime=%Z size=%s %N'"'"' foo 2> /dev/null || stat -f '"'"'user=%Su group=%Sg mode=%Sp atime=%a mtime=%m ctime=%c size=%z %N%SY'"'"' foo || ls -ld foo )'
    [pyinfra.connectors.ssh] Waiting for exit status...
    [pyinfra.connectors.ssh] Command exit status: 0
    [domo] Loaded fact files.Directory (path=foo)
    [pyinfra.connectors.ssh] Attempting upload of <_io.StringIO object at 0x104d7bc70> to /tmp/pyinfra-0beec7b5ea3f0fdbc95d0dd47f3c5bc275da8a33
    [pyinfra.connectors.ssh] Using SFTP for file transfer
    [pyinfra.connectors.ssh] Running command on domo: (pty=False) sudo -H -n sh -c 'cd /home/grace && cp /tmp/pyinfra-0beec7b5ea3f0fdbc95d0dd47f3c5bc275da8a33 foo'
[domo] >>> sudo -H -n sh -c 'cd /home/grace && cp /tmp/pyinfra-0beec7b5ea3f0fdbc95d0dd47f3c5bc275da8a33 foo'
    [pyinfra.connectors.ssh] Waiting for exit status...
    [pyinfra.connectors.ssh] Command exit status: 0
    [pyinfra.connectors.ssh] Running command on domo: (pty=False) sh -c 'cd /home/grace && rm -f /tmp/pyinfra-0beec7b5ea3f0fdbc95d0dd47f3c5bc275da8a33'
[domo] >>> sh -c 'cd /home/grace && rm -f /tmp/pyinfra-0beec7b5ea3f0fdbc95d0dd47f3c5bc275da8a33'
[domo] sh: line 1: cd: /home/grace: Permission denied
    [pyinfra.connectors.ssh] Waiting for exit status...
    [pyinfra.connectors.ssh] Command exit status: 1
    Unable to remove temporary file: sh: line 1: cd: /home/grace: Permission denied
    [domo] Error: executed 0 commands
    [pyinfra.api.state] Failing hosts: domo

--> Disconnecting from hosts...
--> pyinfra error: No hosts remaining!

Indeed it is the code that removes the uploaded file that has the _chdir but not the _sudo: https://github.com/pyinfra-dev/pyinfra/blob/a43146afc6a3c2b34078b74651a81f89e5c7c02b/src/pyinfra/connectors/ssh.py#L607-L618 however the _sudo was removed from arguments earlier: https://github.com/pyinfra-dev/pyinfra/blob/a43146afc6a3c2b34078b74651a81f89e5c7c02b/src/pyinfra/connectors/ssh.py#L565

morrison12 avatar Nov 02 '25 21:11 morrison12