Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
ydroneaud mannequin opened this issue Mar 15, 2022 · 6 comments
Open

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

ydroneaud mannequin opened this issue Mar 15, 2022 · 6 comments
Labels
extension-modules C modules in the Modules dir topic-subprocess Subprocess issues. type-feature A feature request or enhancement

Comments

@ydroneaud
Copy link
Mannequin

ydroneaud mannequin commented Mar 15, 2022

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:

assignee = None
closed_at = None
created_at = <Date 2022-03-15.14:55:54.695>
labels = ['extension-modules', 'type-feature', '3.11']
title = 'subprocess.run(), subprocess.Popen() should accept file descriptor as cwd parameter'
updated_at = <Date 2022-04-05.18:51:32.079>
user = 'https://github.com/ydroneaud'

bugs.python.org fields:

activity = <Date 2022-04-05.18:51:32.079>
actor = 'gregory.p.smith'
assignee = 'none'
closed = False
closed_date = None
closer = None
components = ['Extension Modules']
creation = <Date 2022-03-15.14:55:54.695>
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']

@ydroneaud
Copy link
Mannequin Author

ydroneaud mannequin commented Mar 15, 2022

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.

@ydroneaud ydroneaud mannequin added 3.9 only security fixes 3.10 only security fixes stdlib Python modules in the Lib dir type-feature A feature request or enhancement labels Mar 15, 2022
@gpshead
Copy link
Member

gpshead commented Mar 21, 2022

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)

@gpshead gpshead added 3.11 only security fixes and removed 3.9 only security fixes 3.10 only security fixes labels Mar 21, 2022
@ydroneaud
Copy link
Mannequin Author

ydroneaud mannequin commented Apr 5, 2022

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
Copy link
Member

gpshead commented Apr 5, 2022

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.

@gpshead gpshead added extension-modules C modules in the Modules dir and removed stdlib Python modules in the Lib dir labels Apr 5, 2022
@ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
@ydroneaud
Copy link

For python 3.12 ?

@AlexWaygood AlexWaygood removed the 3.11 only security fixes label Feb 22, 2023
@arhadthedev arhadthedev added stdlib Python modules in the Lib dir topic-multiprocessing and removed stdlib Python modules in the Lib dir labels Feb 22, 2023
@vstinner vstinner added the topic-subprocess Subprocess issues. label Jul 8, 2024
@RomainBrault
Copy link

For python 3.15 ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
extension-modules C modules in the Modules dir topic-subprocess Subprocess issues. type-feature A feature request or enhancement
Projects
None yet
Development

No branches or pull requests

6 participants