Skip to content
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

Safe usage of popen #6490

Merged
merged 1 commit into from
Sep 4, 2024
Merged

Safe usage of popen #6490

merged 1 commit into from
Sep 4, 2024

Conversation

tjruwase
Copy link
Contributor

@tjruwase tjruwase commented Sep 4, 2024

Avoid shell=True security issues with Popen

@tjruwase tjruwase removed the request for review from awan-10 September 4, 2024 15:45
Copy link
Contributor

@tohtana tohtana left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great fixes @tjruwase,
I think subprocess.run and subprocess.check_output have the same security risk. Shall we fix all? We can grep all with shell=True.

@tjruwase
Copy link
Contributor Author

tjruwase commented Sep 4, 2024

I think subprocess.run and subprocess.check_output have the same security risk. Shall we fix all? We can grep all with shell=True.

@tohtana. good point. I will address others in separate PR since this one needs to be released immediately.

@loadams, can we please create a release once this is merged? Thanks!

@loadams
Copy link
Contributor

loadams commented Sep 4, 2024

I think subprocess.run and subprocess.check_output have the same security risk. Shall we fix all? We can grep all with shell=True.

@tohtana. good point. I will address others in separate PR since this one needs to be released immediately.

@loadams, can we please create a release once this is merged? Thanks!

@tjruwase - yes, I'll ensure this PR is merged and we will do a release shortly after.

@loadams loadams added this pull request to the merge queue Sep 4, 2024
Merged via the queue into master with commit 662a421 Sep 4, 2024
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants