amundsen-io / amundsen

Amundsen is a metadata driven application for improving the productivity of data analysts, data scientists and engineers when interacting with data.
https://www.amundsen.io/amundsen/
Apache License 2.0
4.39k stars 954 forks source link

fix: remove potential shell injection vulnerability #2135

Closed Swisk closed 1 year ago

Swisk commented 1 year ago

Description

By using shell=true in the subprocess command, there is a potential shell injection vulnerability. (see https://docs.python.org/3/library/subprocess.html#security-considerations)

Since the intention is merely to run a new process, and not take advantage of shell expansion or other tricks, there is no reason to run the subprocess in a new shell. Instead, we can provide the necessary arguments in a list and achieve the same result.

Motivation and Context

How Has This Been Tested?

The new subprocess.check_call commands were tested manually in an interactive python window, and compared to the old command. We verified that the output of the command was the same in both cases.

Documentation

CheckList

boring-cyborg[bot] commented 1 year ago

Congratulations on your first Pull Request and welcome to Amundsen community! If you have any issues or are unsure about any anything please check our Contribution Guide (https://github.com/amundsen-io/amundsen/blob/main/CONTRIBUTING.md)

stale[bot] commented 1 year ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

stale[bot] commented 1 year ago

This issue has been automatically closed for inactivity. If you still wish to make these changes, please open a new pull request or reopen this one.