Unable to commit changes due to bandit (use of subprocess module)

[sbrodehl]

Describe the bug
I am unable to commit changes in file satpy/readers/hrit_base.py due to bandit issueing B603, B404 because of subprocess usage.

To Reproduce
Change someting in satpy/readers/hrit_base.py and try to commit.

Expected behavior
I would like to commit changes.

Actual results
Commit not possible.

Here is the log:

>> Issue: [B404:blacklist] Consider possible security implications associated with the subprocess module.
   Severity: Low   Confidence: High
   CWE: CWE-78 (https://cwe.mitre.org/data/definitions/78.html)
   Location: satpy/readers/hrit_base.py:35:0
   More Info: https://bandit.readthedocs.io/en/0.0.0/blacklists/blacklist_imports.html#b404-import-subprocess
34	from io import BytesIO
35	from subprocess import PIPE, Popen
36	from tempfile import gettempdir
--------------------------------------------------

>> Issue: [B603:subprocess_without_shell_equals_true] subprocess call - check for execution of untrusted input.
   Severity: Low   Confidence: High
   CWE: CWE-78 (https://cwe.mitre.org/data/definitions/78.html)
   Location: satpy/readers/hrit_base.py:136:8
   More Info: https://bandit.readthedocs.io/en/0.0.0/plugins/b603_subprocess_without_shell_equals_true.html
135	
136	    p = Popen([cmd, infile], stdout=PIPE)
137	    stdout = BytesIO(p.communicate()[0])
--------------------------------------------------

Screenshots
If applicable, add screenshots to help explain your problem.

Environment Info:

• OS: Linux
• Satpy Version: Current Master b2a5eeb
• PyResample Version:
• Readers and writers dependencies (when relevant): [run from satpy.utils import check_satpy; check_satpy()]

Additional context
It looks like bandit is only checking the files which will be commited, and I guess that's fine.
Have you run bandit on the whole repository to check for other issues in files, which rarely get changed?

[djhoese]

Bandit is run as part of the pre-commit hooks that you've installed and that we (maintainers) use. We have not run all of the hooks on all of satpy as there are too many issues to do it all in one sitting. Some of the issues require refactoring or rewriting to do a better long term solution as well.

I think in the case of the HRIT decompression tool we don't have much choice but to use Popen (anyone have other ideas?). So in that case you could look at setting exclusion comments: https://bandit.readthedocs.io/en/latest/config.html#exclusions
Please use the specific rules that are being triggered.

from subprocess import PIPE, Popen  # nosec B404

You'll need a similar one for the use of Popen.

[djhoese]

Does anyone in @pytroll/satpy-core have a problem with this?

[mraspaud]

I'm good with this.

[sbrodehl]

Thanks for the quick reply @djhoese !
I will add exclusion comments to the required lines for HRIT decompression in an upcoming PR.

I have some more performant code for the HRIT decompression part, currently I am using a python wrapper to decompress files in-memory, and therefore I need to make changes to the HRIT reader to accept file-like objects.
I will open another PR to discuss those changes as well.