Remove shell=True from subprocess.Popen to Mitigate Security Risk
Author: avimanyu786
Pull request: yes

# What does this PR do?

This PR resolves a critical security issue in the transformers package by removing the `shell=True` argument from `subprocess.Popen` calls. This update is in response to a security vulnerability flagged by the Bandit static analysis tool, which could potentially allow for execution of arbitrary code. Despite previous attempts to communicate this issue via email with no response [as suggested](https://github.com/huggingface/transformers/security/policy), this PR is being submitted to ensure the safety and integrity of the transformers package.

The vulnerability is documented in Bandit's official recommendations (https://bandit.readthedocs.io/en/1.7.6/plugins/b602_subprocess_popen_with_shell_equals_true.html). By adopting this change, we adhere to best practices for secure subprocess management in Python.

## Before submitting
- [x] This PR fixes a critical security issue and does not introduce new dependencies.
- [x] I have referred to the [contributor guideline](https://github.com/huggingface/transformers/blob/main/CONTRIBUTING.md#start-contributing-pull-requests), Pull Request section.
- [x] This issue was not previously discussed in a Github issue or forum due to the lack of response to email communications.
- [x] I have tested the stability and security of the changes.

## Who can review?

Given the security nature of this PR, I would appreciate a prompt review from @Narsil (Library pipelines).

I kindly request your review of this security update at your earliest convenience to help ensure the ongoing security and reliability of the transformers library for all users. Thank you for your time and consideration.

Comments:
I do agree this is better to remove shell access to this command line. I don't think however this is critical by any means since this is already a CLI utility which we do not leverage directly.

@michaelbenayoun @amyeroberts 


@Narsil @michaelbenayoun @amyeroberts Good to know it's not critical. Thanks a lot for your collective feedback and approval!

Thanks so much for the merge! I must admit, security isn't my primary domain 😅, but this experience piqued my curiosity about the potential security implications in different use cases of the `transformers` library.

I believe this information could be beneficial for our community, especially for those implementing `transformers` in varied environments. Therefore, I'd like to share a summary of these insights for knowledge sharing purposes only:

Through my additional research, including a discussion with GPT4, I've gained a deeper understanding of the potential security implications of using `shell=True` in `subprocess.Popen` within the transformers library. It appears that while this issue is generally not critical for the library's primary use cases, it can become more significant in specific production environments. Particularly in scenarios where the library interacts with user-generated input — such as in web application backends, desktop applications, and cloud-based ML services — the risk of arbitrary code execution increases. This insight highlights the importance of context in evaluating security risks and underscores the need for careful consideration of security best practices, especially in diverse deployment scenarios.

I'm looking forward to any further insights or thoughts from the community on this matter. It's good to have such conversations going to ensure the security and robustness of the library!

Sincere regards,
Avi