Is your feature request related to a problem?
nnUNetV2Runner.train_parallel currently joins each device's argument-list commands with ; and starts the resulting string with subprocess.Popen(..., shell=True).
The commands appear to be constructed internally, so I am not reporting this as a confirmed command-injection vulnerability. However, invoking a shell is unnecessary here and increases the attack surface if command construction changes in the future.
Describe the solution you'd like
Run each device's commands sequentially with shell=False, while retaining parallel execution across devices. One possible implementation is:
- one worker per active device;
- commands within a device are executed in their original order;
- different devices run concurrently;
- each command keeps its associated environment;
- empty stages are skipped.
I prepared and locally checked a proof-of-concept patch. The checks cover command order, list arguments, shell=False, empty-stage handling, Python compilation, and patch applicability. I have not run MONAI's complete nnUNet integration test suite, so I would appreciate maintainer guidance on the preferred test location and whether this change fits the current design.
Alternatives considered
Splitting the semicolon-joined string with shlex.split was rejected because it changes command semantics. Launching every command at once was also rejected because commands assigned to the same GPU must remain sequential.
Additional context
This suggestion came from a static-analysis validation exercise against MONAI. The initial rule finding was manually reviewed before filing this request, and this issue is intentionally framed as defense-in-depth rather than a confirmed vulnerability.
Is your feature request related to a problem?
nnUNetV2Runner.train_parallelcurrently joins each device's argument-list commands with;and starts the resulting string withsubprocess.Popen(..., shell=True).The commands appear to be constructed internally, so I am not reporting this as a confirmed command-injection vulnerability. However, invoking a shell is unnecessary here and increases the attack surface if command construction changes in the future.
Describe the solution you'd like
Run each device's commands sequentially with
shell=False, while retaining parallel execution across devices. One possible implementation is:I prepared and locally checked a proof-of-concept patch. The checks cover command order, list arguments,
shell=False, empty-stage handling, Python compilation, and patch applicability. I have not run MONAI's complete nnUNet integration test suite, so I would appreciate maintainer guidance on the preferred test location and whether this change fits the current design.Alternatives considered
Splitting the semicolon-joined string with
shlex.splitwas rejected because it changes command semantics. Launching every command at once was also rejected because commands assigned to the same GPU must remain sequential.Additional context
This suggestion came from a static-analysis validation exercise against MONAI. The initial rule finding was manually reviewed before filing this request, and this issue is intentionally framed as defense-in-depth rather than a confirmed vulnerability.