Add context and fix bandit and semgrep findings

Signed-off-by: Francesco Giordano <[email protected]>

Author: francesco-giordano

awsbatch-cli/src/awsbatch/awsbhosts.py

@@ -284,13 +284,13 @@ def __get_ecs_clusters(self, compute_environments):
         try:
             # connect to batch and ask for compute environments
             batch_client = self.boto3_factory.get_client("batch")
-            next_token = ""  # nosec
-            while next_token is not None:
+            next_page = ""
+            while next_page is not None:
                 response = batch_client.describe_compute_environments(
-                    computeEnvironments=compute_environments, nextToken=next_token
+                    computeEnvironments=compute_environments, nextToken=next_page
                 )
                 ecs_clusters.extend(self.__get_clusters(response["computeEnvironments"]))
-                next_token = response.get("nextToken")
+                next_page = response.get("nextToken")
         except Exception as e:
             fail("Error listing compute environments from AWS Batch. Failed with exception: %s" % e)
 

awsbatch-cli/src/awsbatch/awsbstat.py

@@ -461,16 +461,16 @@ def __populate_output_by_queue(self, job_queue, job_status, expand_children, det
             single_jobs = []
             jobs_with_children = []
             for status in job_status:
-                next_token = ""  # nosec
-                while next_token is not None:
-                    response = self.batch_client.list_jobs(jobStatus=status, jobQueue=job_queue, nextToken=next_token)
+                next_page = ""
+                while next_page is not None:
+                    response = self.batch_client.list_jobs(jobStatus=status, jobQueue=job_queue, nextToken=next_page)
 
                     for job in response["jobSummaryList"]:
                         if get_job_type(job) != "SIMPLE" and expand_children is True:
                             jobs_with_children.append(job["jobId"])
                         else:
                             single_jobs.append(job)
-                    next_token = response.get("nextToken")
+                    next_page = response.get("nextToken")
 
             # create output items for job array children
             self.__populate_output_by_job_ids(jobs_with_children, details)

cli/src/pcluster/api/controllers/cluster_compute_fleet_controller.py

@@ -80,8 +80,11 @@ def update_compute_fleet(update_compute_fleet_request_content, cluster_name, reg
         elif status == RequestedComputeFleetStatus.STOP_REQUESTED:
             cluster.stop()
         else:
-            raise BadRequestException(  # nosec
-                "the update compute fleet status can only be set to"
+            # A nosec comment is appended to the following line in order to disable the false positive B608 check.
+            # False positive since it is not a SQL query.
+            # [B608:hardcoded_sql_expressions] Possible SQL injection vector through string-based query construction.
+            raise BadRequestException(
+                "the update compute fleet status can only be set to"  # nosec B608
                 " `START_REQUESTED` or `STOP_REQUESTED` for %s scheduler clusters."
                 % cluster.stack.scheduler.capitalize()
             )

cli/src/pcluster/api/util.py

@@ -180,23 +180,25 @@ def _assert_node_executable():
 def _assert_node_version():
     try:
         # A nosec comment is appended to the following line in order to disable the B607 and B603 checks.
+        # It is a false positive since the PATH search is wanted and the input of the check_output is static.
         # [B607:start_process_with_partial_path] Is suppressed because location of executable is retrieved from env
         #   PATH
         # [B603:subprocess_without_shell_equals_true] Is suppressed because input of check_output is not coming from
         #   untrusted source
-        node_version_string = subprocess.check_output(  # nosec
+        node_version_string = subprocess.check_output(  # nosec B607 B603
             ["node", "--version"], stderr=subprocess.STDOUT, shell=False, encoding="utf-8"
         )
         LOGGER.debug("Found Node.js version (%s)", node_version_string)
     except Exception:
         LOGGER.debug("Unable to determine current Node.js version from node")
         try:
             # A nosec comment is appended to the following line in order to disable the B607 and B603 checks.
+            # It is a false positive since the PATH search is wanted and the input of the check_output is static.
             # [B607:start_process_with_partial_path] Is suppressed because location of executable is retrieved from env
             #   PATH
             # [B603:subprocess_without_shell_equals_true] Is suppressed because input of check_output is not coming from
             #   untrusted source
-            node_version_string = subprocess.check_output(  # nosec
+            node_version_string = subprocess.check_output(  # nosec B607 B603
                 ["nvm", "current"], stderr=subprocess.STDOUT, shell=False, encoding="utf-8"
             )
             LOGGER.debug("Found Node.js version '%s' in use", node_version_string)

cli/src/pcluster/cli/commands/dcv_connect.py

@@ -38,8 +38,8 @@ class DCVConnectionError(Exception):
 def _check_command_output(cmd):
     # A nosec comment is appended to the following line in order to disable the B602 check.
     # This is done because it's needed to enable the desired functionality. The only caller
-    # of this function is _retrieve_dcv_session_url, which passes a command that should be safe.
-    return sub.check_output(cmd, shell=True, universal_newlines=True, stderr=sub.STDOUT).strip()  # nosec nosemgrep
+    # of this function is _retrieve_dcv_session_url, which passes a command that is safe.
+    return sub.check_output(cmd, shell=True, universal_newlines=True, stderr=sub.STDOUT).strip()  # nosec B602 nosemgrep
 
 
 def _dcv_connect(args):

cli/src/pcluster/cli/commands/ssh.py

@@ -64,7 +64,7 @@ def _ssh(args, extra_args):
             # - The args passed to the remote command are sanitized.
             # - The default command to which these args is known.
             # - Users have full control over any customization of the command to which args are passed.
-            os.system(cmd)  # nosec nosemgrep
+            os.system(cmd)  # nosec B605 nosemgrep
         else:
             print(json.dumps({"command": cmd}, indent=2))
 

cli/src/pcluster/models/cluster.py

@@ -570,9 +570,10 @@ def _render_and_upload_scheduler_plugin_template(self, dry_run=False):
                 )
                 file_content = result["Body"].read().decode("utf-8")
             else:
-                with urlopen(  # nosec nosemgrep - scheduler_plugin_template url is properly validated
-                    scheduler_plugin_template
-                ) as f:
+                # A nosec comment is appended to the following line in order to disable the B310 check.
+                # The urlopen argument is properly validated
+                # [B310:blacklist] Audit url open for permitted schemes.
+                with urlopen(scheduler_plugin_template) as f:  # nosec B310 nosemgrep
                     file_content = f.read().decode("utf-8")
         except Exception as e:
             raise BadRequestClusterActionError(
@@ -587,7 +588,12 @@ def _render_and_upload_scheduler_plugin_template(self, dry_run=False):
             LOGGER.info("Rendering the following scheduler plugin CloudFormation template:\n%s", file_content)
             environment = SandboxedEnvironment(loader=BaseLoader)
             environment.filters["hash"] = (
-                lambda value: hashlib.sha1(value.encode()).hexdigest()[0:16].capitalize()  # nosec nosemgrep
+                # A nosec comment is appended to the following line in order to disable the B324 checks.
+                # The sha1 is used just as a hashing function.
+                # [B324:hashlib] Use of weak MD4, MD5, or SHA1 hash for security. Consider usedforsecurity=False
+                lambda value: hashlib.sha1(value.encode())  # nosec B324 nosemgrep
+                .hexdigest()[0:16]
+                .capitalize()
             )
             template = environment.from_string(file_content)
             rendered_template = template.render(

cli/src/pcluster/resources/custom_resources/custom_resources_code/crhelper/resource_helper.py

@@ -261,7 +261,10 @@ def _cleanup_response(self):
 
     @staticmethod
     def _rand_string(length):
-        return ''.join(random.choice(string.ascii_uppercase + string.digits) for _ in range(length))  # nosec
+        # A nosec comment is appended to the following line in order to disable the B311 check.
+        # The random.choice is used to generate random string for names.
+        # [B311:blacklist] Standard pseudo-random generators are not suitable for security/cryptographic purposes.
+        return ''.join(random.choice(string.ascii_uppercase + string.digits) for _ in range(length))  # nosec B311
 
     def _add_permission(self, rule_arn):
         sid = self._event['LogicalResourceId'] + self._rand_string(8)

cli/src/pcluster/resources/custom_resources/custom_resources_code/crhelper/utils.py

@@ -27,7 +27,10 @@ def _send_response(response_url, response_body):
     url = urlunsplit(("", "", *split_url[2:]))
     while True:
         try:
-            connection = HTTPSConnection(host)  # nosec nosemgrep
+            # A nosec comment is appended to the following line in order to disable the B309 check.
+            # ParallelCluster only supports python >= 3.4.3
+            # [B309:blacklist] Use of HTTPSConnection on older versions of Python prior to 2.7.9 and 3.4.3 do not provide security, see https://wiki.openstack.org/wiki/OSSN/OSSN-0033
+            connection = HTTPSConnection(host)  # nosec B309 nosemgrep
             connection.request(method="PUT", url=url, body=json_response_body, headers=headers)
             response = connection.getresponse()
             logger.info("CloudFormation returned status code: %s", response.reason)

cli/src/pcluster/resources/custom_resources/custom_resources_code/send_build_notification.py

@@ -38,7 +38,11 @@ def handler(event, context):  # pylint: disable=unused-argument
     url = urlunsplit(("", "", *split_url[2:]))
     while True:
         try:
-            connection = HTTPSConnection(host)  # nosec nosemgrep
+            # A nosec comment is appended to the following line in order to disable the B309 check.
+            # ParallelCluster only supports python >= 3.4.3
+            # [B309:blacklist] Use of HTTPSConnection on older versions of Python prior to 2.7.9 and 3.4.3
+            # do not provide security, see https://wiki.openstack.org/wiki/OSSN/OSSN-0033
+            connection = HTTPSConnection(host)  # nosec B309 nosemgrep
             connection.request(
                 method="PUT", url=url, body=data, headers={"Content-Type": "", "content-length": str(len(data))}
             )

cli/src/pcluster/schemas/cluster_schema.py

@@ -1764,7 +1764,10 @@ def _fetch_scheduler_definition_from_s3(self, original_scheduler_definition, s3_
 
     def _fetch_scheduler_definition_from_https(self, original_scheduler_definition):
         try:
-            with urlopen(original_scheduler_definition) as f:  # nosec nosemgrep
+            # A nosec comment is appended to the following line in order to disable the B310 check.
+            # The urlopen argument is properly validated
+            # [B310:blacklist] Audit url open for permitted schemes.
+            with urlopen(original_scheduler_definition) as f:  # nosec B310 nosemgrep
                 scheduler_definition = f.read().decode("utf-8")
                 return scheduler_definition
         except Exception:

cli/src/pcluster/templates/cdk_builder_utils.py

@@ -58,7 +58,10 @@ def create_hash_suffix(string_to_hash: str):
     return (
         string_to_hash
         if string_to_hash == "HeadNode"
-        else sha1(string_to_hash.encode("utf-8")).hexdigest()[:16].capitalize()  # nosec nosemgrep
+        # A nosec comment is appended to the following line in order to disable the B324 check.
+        # The sha1 is used just as a hashing function.
+        # [B324:hashlib] Use of weak MD4, MD5, or SHA1 hash for security. Consider usedforsecurity=False
+        else sha1(string_to_hash.encode("utf-8")).hexdigest()[:16].capitalize()  # nosec B324 nosemgrep
     )
 
 
@@ -315,7 +318,10 @@ def generate_launch_template_version_cfn_parameter_hash(queue, compute_resource)
     :param compute_resource
     :return: 16 chars string e.g. 2238a84ac8a74529
     """
-    return hashlib.sha1((queue + compute_resource).encode()).hexdigest()[0:16].capitalize()  # nosec nosemgrep
+    # A nosec comment is appended to the following line in order to disable the B324 check.
+    # The sha1 is used just as a hashing function.
+    # [B324:hashlib] Use of weak MD4, MD5, or SHA1 hash for security. Consider usedforsecurity=False
+    return hashlib.sha1((queue + compute_resource).encode()).hexdigest()[0:16].capitalize()  # nosec B324 nosemgrep
 
 
 class NodeIamResourcesBase(Construct):

cli/src/pcluster/templates/cluster_stack.py

@@ -1074,7 +1074,10 @@ def _add_head_node(self):
             },
             "deployConfigFiles": {
                 "files": {
-                    "/tmp/dna.json": {  # nosec
+                    # A nosec comment is appended to the following line in order to disable the B108 check.
+                    # The file is needed by the product
+                    # [B108:hardcoded_tmp_directory] Probable insecure usage of temp file/directory.
+                    "/tmp/dna.json": {  # nosec B108
                         "content": dna_json,
                         "mode": "000644",
                         "owner": "root",
@@ -1087,13 +1090,19 @@ def _add_head_node(self):
                         "group": "root",
                         "content": "cookbook_path ['/etc/chef/cookbooks']",
                     },
-                    "/tmp/extra.json": {  # nosec
+                    # A nosec comment is appended to the following line in order to disable the B108 check.
+                    # The file is needed by the product
+                    # [B108:hardcoded_tmp_directory] Probable insecure usage of temp file/directory.
+                    "/tmp/extra.json": {  # nosec B108
                         "mode": "000644",
                         "owner": "root",
                         "group": "root",
                         "content": self.config.extra_chef_attributes,
                     },
-                    "/tmp/wait_condition_handle.txt": {  # nosec
+                    # A nosec comment is appended to the following line in order to disable the B108 check.
+                    # The file is needed by the product
+                    # [B108:hardcoded_tmp_directory] Probable insecure usage of temp file/directory.
+                    "/tmp/wait_condition_handle.txt": {  # nosec B108
                         "mode": "000644",
                         "owner": "root",
                         "group": "root",

cli/src/pcluster/utils.py

@@ -18,7 +18,6 @@
 import string
 import sys
 import time
-import urllib.request
 import zipfile
 from io import BytesIO
 from shlex import quote
@@ -28,7 +27,6 @@
 import dateutil.parser
 import pkg_resources
 import yaml
-from pkg_resources import packaging
 from yaml import SafeLoader
 from yaml.constructor import ConstructorError
 from yaml.resolver import BaseResolver
@@ -64,7 +62,7 @@ def generate_random_name_with_prefix(name_prefix):
     Example: <name_prefix>-4htvo26lchkqeho1
     """
     random_string = generate_random_prefix()
-    output_name = "-".join([name_prefix.lower()[: 63 - len(random_string) - 1], random_string])  # nosec
+    output_name = "-".join([name_prefix.lower()[: 63 - len(random_string) - 1], random_string])
     return output_name
 
 
@@ -74,7 +72,10 @@ def generate_random_prefix():
 
     Example: 4htvo26lchkqeho1
     """
-    return "".join(random.choice(string.ascii_lowercase + string.digits) for _ in range(16))  # nosec
+    # A nosec comment is appended to the following line in order to disable the B311 check.
+    # The random.choice is used to generate random string for names.
+    # [B311:blacklist] Standard pseudo-random generators are not suitable for security/cryptographic purposes.
+    return "".join(random.choice(string.ascii_lowercase + string.digits) for _ in range(16))  # nosec B311
 
 
 def _add_file_to_zip(zip_file, path, arcname):
@@ -251,18 +252,6 @@ def get_installed_version(base_version_only: bool = False):
     return pkg_distribution.version if not base_version_only else pkg_distribution.parsed_version.base_version
 
 
-def check_if_latest_version():
-    """Check if the current package version is the latest one."""
-    try:
-        pypi_url = "https://pypi.python.org/pypi/aws-parallelcluster/json"
-        with urllib.request.urlopen(pypi_url) as url:  # nosec nosemgrep
-            latest = json.loads(url.read())["info"]["version"]
-        if packaging.version.parse(get_installed_version()) < packaging.version.parse(latest):
-            print(f"Info: There is a newer version {latest} of AWS ParallelCluster available.")
-    except Exception:  # nosec
-        pass
-
-
 def warn(message):
     """Print a warning message."""
     print(f"WARNING: {message}")

cli/src/pcluster/validators/cluster_validators.py

@@ -878,7 +878,10 @@ def _validate(self, mount_dir: str):
             "/sbin",
             "/srv",
             "/sys",
-            "/tmp",  # nosec nosemgrep
+            # A nosec comment is appended to the following line in order to disable the B108 check.
+            # It is a false positive since is a list to check folder name
+            # [B108:hardcoded_tmp_directory] Probable insecure usage of temp file/directory.
+            "/tmp",  # nosec B108
             "/usr",
             "/var",
         ]

cli/src/pcluster/validators/s3_validators.py

@@ -68,7 +68,10 @@ def _validate_s3_uri(self, url: str, fail_on_error: bool, expected_bucket_owner:
 
     def _validate_https_uri(self, url: str, fail_on_error: bool):
         try:
-            with urlopen(url):  # nosec nosemgrep
+            # A nosec comment is appended to the following line in order to disable the B310 check.
+            # The urlopen argument is properly validated
+            # [B310:blacklist] Audit url open for permitted schemes.
+            with urlopen(url):  # nosec B310 nosemgrep
                 pass
         except HTTPError as e:
             self._add_failure(

scheduler_plugins/slurm/artifacts/slurm_plugin_cookbook/files/default/head_node_slurm/slurm/pcluster_slurm_config_generator.py

@@ -145,10 +145,7 @@ def _generate_slurm_parallelcluster_configs(
 def _get_jinja_env(template_directory):
     """Return jinja environment with trim_blocks/lstrip_blocks set to True."""
     file_loader = FileSystemLoader(template_directory)
-    # A nosec comment is appended to the following line in order to disable the B701 check.
-    # The contents of the default templates are known and the input configuration data is
-    # validated by the CLI.
-    env = SandboxedEnvironment(loader=file_loader, trim_blocks=True, lstrip_blocks=True)  # nosec nosemgrep
+    env = SandboxedEnvironment(loader=file_loader, trim_blocks=True, lstrip_blocks=True)
     env.filters["sanify_name"] = lambda value: re.sub(r"[^A-Za-z0-9]", "", value)
     env.filters["gpus"] = _gpu_count
     env.filters["gpu_type"] = _gpu_type

scheduler_plugins/slurm/artifacts/slurm_plugin_cookbook/recipes/install_slurm.rb

@@ -48,7 +48,7 @@
 ruby_block 'Validate Slurm Tarball Checksum' do
   block do
     require 'digest'
-    checksum = Digest::SHA1.file(slurm_tarball).hexdigest # nosemgrep
+    checksum = Digest::SHA1.file(slurm_tarball).hexdigest
     raise "Downloaded Tarball Checksum #{checksum} does not match expected checksum #{node['slurm']['sha1']}" if checksum != node['slurm']['sha1']
   end
 end

util/sync_buckets.py

@@ -125,7 +125,7 @@ def _aws_credentials_type(value):
 
 def _get_s3_object_metadata(s3_url):
     req = urllib.request.Request(url=s3_url, method="HEAD")
-    response = urllib.request.urlopen(req)  # nosec nosemgrep
+    response = urllib.request.urlopen(req)  # nosec
     assert response.status == 200  # nosec
     metadata = {}
     metadata["version_id"] = response.headers.get("x-amz-version-id")

util/update_pcluster_configs.py

@@ -294,7 +294,7 @@ def _get_batch_instance_whitelist(region, credentials):
 
 
 def _read_json_from_url(url):
-    response = urlopen(url)  # nosec nosemgrep
+    response = urlopen(url)  # nosec
     return json.loads(response.read().decode("utf-8"))
 
 

util/upload-cookbook.py

@@ -134,7 +134,7 @@ def _get_bucket_name(args, region):
 
 def _md5sum(cookbook_archive_file, md5sum_file):
     blocksize = 65536
-    hasher = hashlib.md5()  # nosec nosemgrep
+    hasher = hashlib.md5()  # nosec
     with open(cookbook_archive_file, "rb") as arch:
         buf = arch.read(blocksize)
         while len(buf) > 0: