Doug/improve timeout and error handling (#40)

# Change log

* Fixed an issue where if the top level package id in sources didn't exist it could cause the CLI to fail with an unhandled exception

* Added a configurable timeout for requests

* Added the new flag to the documentation

Author: dacoburn

README.md

@@ -8,7 +8,7 @@ The Socket Security CLI was created to enable integrations with other tools like
 socketcli [-h] [--api_token API_TOKEN] [--repo REPO] [--branch BRANCH] [--committer COMMITTER] [--pr_number PR_NUMBER]
                  [--commit_message COMMIT_MESSAGE] [--default_branch] [--target_path TARGET_PATH] [--scm {api,github,gitlab}] [--sbom-file SBOM_FILE]
                  [--commit-sha COMMIT_SHA] [--generate-license GENERATE_LICENSE] [-v] [--enable-debug] [--enable-json] [--disable-overview]
-                 [--disable-security-issue] [--files FILES] [--ignore-commit-files]
+                 [--disable-security-issue] [--files FILES] [--ignore-commit-files] [--timeout]
 ````
 
 If you don't want to provide the Socket API Token every time then you can use the environment variable `SOCKET_SECURITY_API_KEY`
@@ -38,3 +38,4 @@ If you don't want to provide the Socket API Token every time then you can use th
 | --files                  |                | False    |         | If provided in the format of `["file1", "file2"]` will be used to determine if there have been supported file changes. This is used if it isn't a git repo and you would like to only run if it supported files have changed. |
 | --ignore-commit-files    |                | False    | False   | If enabled then the CLI will ignore what files are changed in the commit and look for all manifest files                                                                                                                      |
 | --disable-blocking       |                | False    | False   | Disables failing checks and will only exit with an exit code of 0                                                                                                                                                             |
+| --timeout                |                | False    | 1200    | The timeout per request for the CLI                                                                                                                                                                                           |

socketsecurity/__init__.py

@@ -1,2 +1,2 @@
 __author__ = 'socket.dev'
-__version__ = '1.0.40'
+__version__ = '1.0.41'

socketsecurity/core/__init__.py

@@ -1,12 +1,18 @@
 import logging
 from pathlib import PurePath
-
+from requests.exceptions import ReadTimeout
 import requests
 from urllib.parse import urlencode
 import base64
 import json
 from socketsecurity.core.exceptions import (
-    APIFailure, APIKeyMissing, APIAccessDenied, APIInsufficientQuota, APIResourceNotFound, APICloudflareError
+    APIFailure,
+    APIKeyMissing,
+    APIAccessDenied,
+    APIInsufficientQuota,
+    APIResourceNotFound,
+    APICloudflareError,
+    RequestTimeoutExceeded
 )
 from socketsecurity import __version__
 from socketsecurity.core.licenses import Licenses
@@ -182,15 +188,18 @@ def do_request(
     verify = True
     if allow_unverified_ssl:
         verify = False
-    response = requests.request(
-        method.upper(),
-        url,
-        headers=headers,
-        data=payload,
-        files=files,
-        timeout=timeout,
-        verify=verify
-    )
+    try:
+        response = requests.request(
+            method.upper(),
+            url,
+            headers=headers,
+            data=payload,
+            files=files,
+            timeout=timeout,
+            verify=verify
+        )
+    except ReadTimeout:
+        raise RequestTimeoutExceeded(f"Configured timeout {timeout} reached for request for path {url}")
     output_headers = headers.copy()
     output_headers['Authorization'] = "API_KEY_REDACTED"
     output = {
@@ -794,15 +803,18 @@ def get_source_data(package: Package, packages: dict) -> list:
         else:
             for top_id in package.topLevelAncestors:
                 top_package: Package
-                top_package = packages[top_id]
-                manifests = ""
-                top_purl = f"{top_package.type}/{top_package.name}@{top_package.version}"
-                for manifest_data in top_package.manifestFiles:
-                    manifest_file = manifest_data.get("file")
-                    manifests += f"{manifest_file};"
-                manifests = manifests.rstrip(";")
-                source = (top_purl, manifests)
-                introduced_by.append(source)
+                top_package = packages.get(top_id)
+                if top_package:
+                    manifests = ""
+                    top_purl = f"{top_package.type}/{top_package.name}@{top_package.version}"
+                    for manifest_data in top_package.manifestFiles:
+                        manifest_file = manifest_data.get("file")
+                        manifests += f"{manifest_file};"
+                    manifests = manifests.rstrip(";")
+                    source = (top_purl, manifests)
+                    introduced_by.append(source)
+                else:
+                    log.debug(f"Unable to get top level package info for {top_id}")
         return introduced_by
 
     @staticmethod
@@ -841,21 +853,29 @@ def create_sbom_dict(sbom: list) -> dict:
         """
         packages = {}
         top_level_count = {}
+        top_levels = {}
         for item in sbom:
             package = Package(**item)
             if package.id in packages:
-                print("Duplicate package?")
+                log.debug("Duplicate package?")
             else:
                 package = Core.get_license_details(package)
                 packages[package.id] = package
                 for top_id in package.topLevelAncestors:
                     if top_id not in top_level_count:
                         top_level_count[top_id] = 1
+                        top_levels[top_id] = [package.id]
                     else:
                         top_level_count[top_id] += 1
+                        if package.id not in top_levels[top_id]:
+                            top_levels[top_id].append(package.id)
         if len(top_level_count) > 0:
             for package_id in top_level_count:
-                packages[package_id].transitives = top_level_count[package_id]
+                if package_id not in packages:
+                    details = top_levels.get(package_id)
+                    log.debug(f"Orphaned top level package id {package_id} for packages {details}")
+                else:
+                    packages[package_id].transitives = top_level_count[package_id]
         return packages
 
     @staticmethod

socketsecurity/core/exceptions.py

@@ -36,3 +36,7 @@ class APIInsufficientQuota(Exception):
 class APIResourceNotFound(Exception):
     """Raised when access is denied to the API"""
     pass
+
+class RequestTimeoutExceeded(Exception):
+    """Raised when access is denied to the API"""
+    pass

socketsecurity/socketcli.py

@@ -161,6 +161,16 @@
     default=False
 )
 
+parser.add_argument(
+    '--timeout',
+    default=1200,
+    help='Timeout configuration for each request. Defaults to 1200 and applies to each unique HTTP request',
+    required=False,
+    type=float
+)
+
+
+
 
 def output_console_comments(diff_report: Diff, sbom_file_name: str = None) -> None:
     if diff_report.id != "NO_DIFF_RAN":
@@ -252,6 +262,8 @@ def main_code():
     ignore_commit_files = arguments.ignore_commit_files
     disable_blocking = arguments.disable_blocking
     allow_unverified = arguments.allow_unverified
+    timeout = arguments.timeout
+
     if disable_blocking:
         global blocking_disabled
         blocking_disabled = True
@@ -308,7 +320,7 @@ def main_code():
         default_branch = scm.is_default_branch
 
     base_api_url = os.getenv("BASE_API_URL") or None
-    core = Core(token=api_token, request_timeout=1200, base_api_url=base_api_url, allow_unverified=allow_unverified)
+    core = Core(token=api_token, request_timeout=timeout, base_api_url=base_api_url, allow_unverified=allow_unverified)
     no_change = True
     if ignore_commit_files:
         no_change = False