python / cpython

The Python programming language
https://www.python.org
Other
63.36k stars 30.34k forks source link

subprocess.run(), subprocess.Popen() should accept file descriptor as cwd parameter #91183

Open 8e21ca5f-97fb-4994-ad23-845fc45915f8 opened 2 years ago

8e21ca5f-97fb-4994-ad23-845fc45915f8 commented 2 years ago
BPO 47027
Nosy @gpshead, @ydroneaud

Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

Show more details

GitHub fields: ```python assignee = None closed_at = None created_at = labels = ['extension-modules', 'type-feature', '3.11'] title = 'subprocess.run(), subprocess.Popen() should accept file descriptor as cwd parameter' updated_at = user = 'https://github.com/ydroneaud' ``` bugs.python.org fields: ```python activity = actor = 'gregory.p.smith' assignee = 'none' closed = False closed_date = None closer = None components = ['Extension Modules'] creation = creator = 'ydroneaud' dependencies = [] files = [] hgrepos = [] issue_num = 47027 keywords = [] message_count = 4.0 messages = ['415249', '415716', '416788', '416801'] nosy_count = 2.0 nosy_names = ['gregory.p.smith', 'ydroneaud'] pr_nums = [] priority = 'normal' resolution = None stage = 'needs patch' status = 'open' superseder = None type = 'enhancement' url = 'https://bugs.python.org/issue47027' versions = ['Python 3.11'] ```

8e21ca5f-97fb-4994-ad23-845fc45915f8 commented 2 years ago

subprocess.run() and subprocess.Popen() accepts a cwd= parameter to change directory before running the subprocess.

Unfortunately it's not possible to use a file descriptor to run the subprocess in a directory already opened.

For example:

    import os
    import subprocess
    with os.open('/usr/bin', os.O_RDONLY) as f:
        subprocess.run(["ls", "-l"], cwd=f, check=True)

fails with

    Traceback (most recent call last):
      File "<stdin>", line 1, in <module>
      File "/usr/lib/python3.9/subprocess.py", line 505, in run
        with Popen(*popenargs, **kwargs) as process:
      File "/usr/lib/python3.9/subprocess.py", line 951, in __init__
        self._execute_child(args, executable, preexec_fn, close_fds,
      File "/usr/lib/python3.9/subprocess.py", line 1754, in _execute_child
        self.pid = _posixsubprocess.fork_exec(

Using a file descriptor instead of path is useful to address TOCTOU issues.

Maybe a mean to convert a file descriptor to a Path-like object would do the trick.

gpshead commented 2 years ago

Basically you want it to call fchdir() instead of chdir() when passed a fd (integer) instead of a string/Path-like. That makes sense and should be a reasonably straight forward set of changes to _posixsubprocess.c.

(A way to convert a fd into a Path-like object would _not_ work as that'd reintroduce the TOCTOU on the directory - that'd be a pathlib feature request anyways, not a subprocess one)

8e21ca5f-97fb-4994-ad23-845fc45915f8 commented 2 years ago

I looked at posixmodule: os.chdir() accepts a file descriptor. Maybe it can be possible to invoke it from _posixsubprocess.c instead of calling chdir().

gpshead commented 2 years ago

this mostly requires plumbing to accept an int as the cwd and plumb that through to the between fork and exec code to call fchdir(cwd_fd) on the int instead of chdir(cwd) on the char*.

the Modules/_posixsubprocess.c internals are a bit of a mess today with a bazillion parameters passed to internal functions which makes this a pain... I want to refactor things to use a struct and not need that, but adding this feature is doable regardless.

ydroneaud commented 1 year ago

For python 3.12 ?