Bandit report

[netoarmando]

I decided to execute bandit on the codebase after seeing on its README that assert must be avoided and it's also considered as general bad practice in OpenStack codebases (outside tests).

Run started:2020-07-27 19:42:42.741117

Test results:
>> Issue: [B404:blacklist] Consider possible security implications associated with subprocess module.
   Severity: Low   Confidence: High
   Location: src/aiohabit/actions/ssh.py:18
   More Info: https://bandit.readthedocs.io/en/latest/blacklists/blacklist_imports.html#b404-import-subprocess
17  import os
18  import subprocess
19  
20  from aiohabit.errors import ApplicationError

--------------------------------------------------
>> Issue: [B322:blacklist] The input method in Python 2 will read from standard input, evaluate and run the resulting string as python source code. This is similar, though in many ways worse, then using eval. On Python 2, use raw_input instead, input is safe in Python 3.
   Severity: High   Confidence: High
   Location: src/aiohabit/actions/ssh.py:53
   More Info: https://bandit.readthedocs.io/en/latest/blacklists/blacklist_calls.html#b322-input
52              try:
53                  host_idx = int(input("Enter a number of host to ssh into: "))
54              except ValueError:

--------------------------------------------------
>> Issue: [B603:subprocess_without_shell_equals_true] subprocess call - check for execution of untrusted input.
   Severity: Low   Confidence: High
   Location: src/aiohabit/actions/ssh.py:119
   More Info: https://bandit.readthedocs.io/en/latest/plugins/b603_subprocess_without_shell_equals_true.html
118         print(cmd)
119         process = subprocess.Popen(cmd, **run_args)
120         process.communicate(input=psw_input)

--------------------------------------------------
>> Issue: [B101:assert_used] Use of assert detected. The enclosed code will be removed when compiling to optimised byte code.
   Severity: Low   Confidence: High
   Location: src/aiohabit/providers/aws.py:106
   More Info: https://bandit.readthedocs.io/en/latest/plugins/b101_assert_used.html
105         ids = [srv.id for srv in aws_res]
106         assert len(ids) == 1  # ids must be len of 1 as we provision one vm at the time
107 
108         # creating name for instance (visible in aws ec2 WebUI)
109         self.ec2.create_tags(

--------------------------------------------------
>> Issue: [B101:assert_used] Use of assert detected. The enclosed code will be removed when compiling to optimised byte code.
   Severity: Low   Confidence: High
   Location: src/aiohabit/providers/aws.py:148
   More Info: https://bandit.readthedocs.io/en/latest/plugins/b101_assert_used.html
147         response = self.client.describe_instances(InstanceIds=[aws_id])
148         assert len(response["Reservations"]) == 1
149         assert len(response["Reservations"][0]["Instances"]) == 1

--------------------------------------------------
>> Issue: [B101:assert_used] Use of assert detected. The enclosed code will be removed when compiling to optimised byte code.
   Severity: Low   Confidence: High
   Location: src/aiohabit/providers/aws.py:149
   More Info: https://bandit.readthedocs.io/en/latest/plugins/b101_assert_used.html
148         assert len(response["Reservations"]) == 1
149         assert len(response["Reservations"][0]["Instances"]) == 1
150         result = response["Reservations"][0]["Instances"][0]

--------------------------------------------------

Code scanned:
    Total lines of code: 1950
    Total lines skipped (#nosec): 0

Run metrics:
    Total issues (by severity):
        Undefined: 0.0
        Low: 5.0
        Medium: 0.0
        High: 1.0
    Total issues (by confidence):
        Undefined: 0.0
        Low: 0.0
        Medium: 0.0
        High: 6.0
Files skipped (0):

[pvoborni]

Shouldn't we run it only on src dir and not tests. A lot of the asserts problem are from tests dir.

[netoarmando]

I agree, we should fix the asserts on src only. I've updated the description to reflect that.

[pvoborni]

+1 for removing the asserts. From the other issues. I'd ignore the B322:blacklist (the only "high" one). As it says it applies only for Python2 and is safe for Python3. Q is if we can somehow configure it to appear in other runs. With the 2 subprocess things. I'm not sure what to do about them. IMHO we want to call "ssh". But we probably doesn't need "shell==True".

[Tiboris]

Some isssues might be fixed byt this PR https://github.com/pvoborni/mrack/pull/24

[netoarmando]

Closing this, most of the issues were fixed, the remaining ones are related to subprocess and how we parse xml from Beaker. Full report: bandit-report.txt