Skip to content

Commit 33fec2c

Browse files
committed
gh-47798: run_pipeline: reject shell=True and executable=
The pipeline replaces the shell; per-stage shell would re-introduce the quoting and injection surface this API exists to avoid. A future Stage() wrapper is the place for the rare stage that needs it.
1 parent 5c97eac commit 33fec2c

3 files changed

Lines changed: 39 additions & 21 deletions

File tree

Doc/library/subprocess.rst

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -271,9 +271,9 @@ underlying :class:`Popen` interface can be used directly.
271271
Wait for all commands to complete, then return a :class:`CompletedPipeline`
272272
instance.
273273

274-
Each positional argument should be a command (a list of strings, or a string
275-
if ``shell=True``) to execute. The standard output of each command is
276-
connected to the standard input of the next command in the pipeline.
274+
Each positional argument should be a command (a sequence of strings) to
275+
execute. The standard output of each command is connected to the standard
276+
input of the next command in the pipeline.
277277

278278
This function requires at least two commands. For a single command, use
279279
:func:`run` instead.
@@ -341,7 +341,10 @@ underlying :class:`Popen` interface can be used directly.
341341
children (unlike a shell, which can give each command its own fd set).
342342
``close_fds=False`` is rejected because inherited copies of the
343343
inter-process pipe ends in sibling children would prevent EOF from being
344-
signaled and cause deadlocks.
344+
signaled and cause deadlocks. ``shell=True`` and ``executable=`` are also
345+
rejected: the pipeline itself replaces the shell, and per-stage shell
346+
interpretation would re-introduce the quoting and injection surface this
347+
function exists to avoid.
345348

346349
Examples::
347350

@@ -397,8 +400,7 @@ underlying :class:`Popen` interface can be used directly.
397400

398401
.. attribute:: commands
399402

400-
The list of commands used to launch the pipeline. Each command is a list
401-
of strings (or a string if ``shell=True`` was used).
403+
The list of commands used to launch the pipeline.
402404

403405
.. attribute:: returncodes
404406

Lib/subprocess.py

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -962,15 +962,14 @@ def run(*popenargs,
962962

963963
# Future extension point: an opt-in Stage(cmd, **overrides) wrapper could
964964
# be accepted in place of a bare command to allow per-stage overrides
965-
# (e.g. stderr=STDOUT, pass_fds, env, cwd) for shell-pipeline-like
966-
# topologies.
965+
# (e.g. stderr, shell, env) for shell-pipeline-like topologies.
967966
def run_pipeline(*commands, input=None, capture_output=False, timeout=None,
968967
check=False, **kwargs):
969968
"""Run a pipeline of commands connected via pipes.
970969
971-
Each positional argument should be a command (list of strings or a string
972-
if shell=True) to execute. The stdout of each command is connected to the
973-
stdin of the next command in the pipeline, similar to shell pipelines.
970+
Each positional argument should be a command (a sequence of strings) to
971+
execute. The stdout of each command is connected to the stdin of the next
972+
command in the pipeline, similar to shell pipelines.
974973
975974
Returns a CompletedPipeline instance with attributes commands, returncodes,
976975
stdout, and stderr. By default, stdout and stderr are not captured, and
@@ -1032,6 +1031,14 @@ def run_pipeline(*commands, input=None, capture_output=False, timeout=None,
10321031
'close_fds=False is not supported by run_pipeline; '
10331032
'inherited pipe ends would prevent EOF signaling between commands')
10341033

1034+
if kwargs.get('shell'):
1035+
raise ValueError(
1036+
'shell=True is not supported by run_pipeline; the pipeline itself '
1037+
'replaces the shell. Pass each stage as an argv sequence.')
1038+
if kwargs.get('executable') is not None:
1039+
raise ValueError(
1040+
'executable= is not supported by run_pipeline')
1041+
10351042
stderr_arg = kwargs.pop('stderr', None)
10361043
capture_stderr = capture_output or (stderr_arg is PIPE)
10371044

Lib/test/test_subprocess.py

Lines changed: 19 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2588,16 +2588,25 @@ def test_pipeline_error_repr(self):
25882588
self.assertIn('echo', r)
25892589
self.assertIn('false', r)
25902590

2591-
@unittest.skipIf(mswindows, "POSIX shell-specific")
2592-
def test_pipeline_shell_true(self):
2593-
"""shell=True forwards each command to the shell."""
2594-
result = subprocess.run_pipeline(
2595-
'echo hello world',
2596-
'tr a-z A-Z',
2597-
shell=True, capture_output=True, text=True,
2598-
)
2599-
self.assertEqual(result.stdout.strip(), 'HELLO WORLD')
2600-
self.assertEqual(result.returncodes, [0, 0])
2591+
def test_pipeline_shell_rejected(self):
2592+
"""shell=True is rejected; the pipeline replaces the shell."""
2593+
with self.assertRaises(ValueError) as cm:
2594+
subprocess.run_pipeline(
2595+
'echo hello world',
2596+
'tr a-z A-Z',
2597+
shell=True, capture_output=True,
2598+
)
2599+
self.assertIn('shell=True', str(cm.exception))
2600+
2601+
def test_pipeline_executable_rejected(self):
2602+
"""executable= is rejected (only meaningful with shell=)."""
2603+
with self.assertRaises(ValueError) as cm:
2604+
subprocess.run_pipeline(
2605+
[sys.executable, '-c', 'pass'],
2606+
[sys.executable, '-c', 'pass'],
2607+
executable=sys.executable,
2608+
)
2609+
self.assertIn('executable', str(cm.exception))
26012610

26022611
def test_pipeline_env(self):
26032612
"""env= is propagated to every command in the pipeline."""

0 commit comments

Comments
 (0)