From 24ceea4dc98c53cb35951210ba3a3cc415f86f97 Mon Sep 17 00:00:00 2001 From: William Murphy Date: Mon, 23 Oct 2023 11:26:42 -0400 Subject: [PATCH] Remove bare "except:" clauses except those that re-raise (#351) And some small logging improvements. Signed-off-by: Will Murphy --- src/vunnel/providers/__init__.py | 2 +- src/vunnel/providers/alpine/parser.py | 10 +++++----- src/vunnel/providers/rhel/parser.py | 9 +++++---- src/vunnel/providers/sles/parser.py | 8 +++----- src/vunnel/providers/ubuntu/git.py | 25 +++++++++++++------------ src/vunnel/providers/ubuntu/parser.py | 3 ++- src/vunnel/providers/wolfi/parser.py | 2 +- src/vunnel/utils/__init__.py | 6 ++++-- src/vunnel/utils/oval_parser.py | 2 +- src/vunnel/utils/oval_v2.py | 4 +++- 10 files changed, 38 insertions(+), 33 deletions(-) diff --git a/src/vunnel/providers/__init__.py b/src/vunnel/providers/__init__.py index afcbb1fc..73498e01 100644 --- a/src/vunnel/providers/__init__.py +++ b/src/vunnel/providers/__init__.py @@ -76,6 +76,6 @@ def load_plugins() -> None: try: logging.debug(f"loading provider plugin {tool.name!r}") tool.load() - except: # noqa: E722 + except Exception: # note: this should not be fatal. Log and move on. logging.exception(f"failed loading provider plugin {tool.name!r}") diff --git a/src/vunnel/providers/alpine/parser.py b/src/vunnel/providers/alpine/parser.py index 1bda36ec..b9d918e3 100644 --- a/src/vunnel/providers/alpine/parser.py +++ b/src/vunnel/providers/alpine/parser.py @@ -86,15 +86,15 @@ def _download(self): if not os.path.exists(self.secdb_dir_path): os.makedirs(self.secdb_dir_path, exist_ok=True) - self.logger.info("downloading alpine secdb metadata from: {}".format(self.metadata_url)) + self.logger.info(f"downloading alpine secdb metadata from: {self.metadata_url}") r = self._download_metadata_url() try: self.logger.debug("HTML parsing secdb landing page content for links") parser = SecdbLandingParser() parser.feed(r.text) links = parser.links - except: - self.logger.warning("unable to html parse secdb landing page content for links") + except Exception: + self.logger.warning("unable to html parse secdb landing page content for links", exc_info=True) if not links: self.logger.debug("string parsing secdb landing page content for links") @@ -135,8 +135,8 @@ def _download(self): except KeyboardInterrupt: raise - except: - self.logger.exception("ignoring error processing secdb for {}".format(link)) + except Exception: + self.logger.exception(f"ignoring error processing secdb for {link}") @utils.retry_with_backoff() def _download_metadata_url(self) -> requests.Response: diff --git a/src/vunnel/providers/rhel/parser.py b/src/vunnel/providers/rhel/parser.py index 03e0cd42..cb142a70 100644 --- a/src/vunnel/providers/rhel/parser.py +++ b/src/vunnel/providers/rhel/parser.py @@ -163,7 +163,7 @@ def _sync_cves(self, skip_if_exists=False, do_full_sync=True): # noqa last_full_sync = dt_parser.parse(fp.read()) if (now - last_full_sync).days < self.full_sync_interval: do_full_sync = False - except: + except Exception: self.logger.debug("ignoring error loading last_full_sync timestamp from disk", exc_info=True) do_full_sync = True @@ -282,7 +282,7 @@ def _fetch_rhsa_fix_version(self, rhsa_id, platform, package): ) else: self.logger.debug(f"{rhsa_id} not found for platform {platform}") - except: + except Exception: self.logger.exception(f"error looking up {package} in {rhsa_id} for {platform}") return fixed_ver, module_name @@ -633,7 +633,7 @@ def _parse_package_state(self, cve_id: str, fixed: list[FixedIn], content) -> li else: self.logger.debug(f"{state!r} is an unknown state") continue - except: + except Exception: self.logger.exception(f"error parsing {cve_id} package state entity: {item}") return affected + out_of_support @@ -679,7 +679,8 @@ def _parse_cve(self, cve_id, content): cvssv3.get("cvss3_base_score", None), cvssv3.get("status", None), ) - except: + except Exception: + self.logger.info("unable to make cvss3, defaulting to None", exc_info=True) cvssv3_obj = None for item in nfins: # process not fixed in packages first as that trumps fixes diff --git a/src/vunnel/providers/sles/parser.py b/src/vunnel/providers/sles/parser.py index ff43d334..5e8316ed 100644 --- a/src/vunnel/providers/sles/parser.py +++ b/src/vunnel/providers/sles/parser.py @@ -379,17 +379,15 @@ def parse(cls, xml_element, config: OVALParserConfig) -> SLESOVALVulnerability | identity = xml_element.attrib["id"] oval_ns_match = re.search(config.namespace_regex, xml_element.tag) - if oval_ns_match and len(oval_ns_match.groups()) > 0: - oval_ns = oval_ns_match.group(1) - else: - oval_ns = "" + oval_ns = oval_ns_match.group(1) if oval_ns_match and len(oval_ns_match.groups()) > 0 else "" # def_version = xml_element.attrib["version"] name = xml_element.find(config.title_xpath_query.format(oval_ns)).text severity_element = xml_element.find(config.severity_xpath_query.format(oval_ns)) try: severity = config.severity_map.get(severity_element.text.lower()) - except: + except Exception: + cls.logger.info("unknown severity due to exception", exc_info=True) severity = "Unknown" # TODO temporary hack! sles 15 data was tripping this, figure out a better way description = xml_element.find(config.description_xpath_query.format(oval_ns)).text.strip() diff --git a/src/vunnel/providers/ubuntu/git.py b/src/vunnel/providers/ubuntu/git.py index cc183203..ae71de89 100644 --- a/src/vunnel/providers/ubuntu/git.py +++ b/src/vunnel/providers/ubuntu/git.py @@ -82,9 +82,9 @@ def _check(self, destination): cmd = self._is_git_repo_cmd_ out = self._exec_cmd(cmd, cwd=destination) - self.logger.debug("check for git repository, cmd: {}, output: {}".format(cmd, out.decode())) - except: - self.logger.debug(f"git working tree not found at {destination}") + self.logger.debug(f"check for git repository, cmd: {cmd}, output: {out.decode()}") + except Exception: + self.logger.debug(f"git working tree not found at {destination}", exc_info=True) return False return True @@ -146,21 +146,22 @@ def sync_with_upstream(self): try: self._exec_cmd(self._clean_cmd_, cwd=self.dest) self._exec_cmd(self._reset_cmd_, cwd=self.dest) - except: - pass + except Exception: + self.logger.info("failed to clean and reset", exc_info=True) self._exec_cmd(self._check_out_cmd_.format(branch=self.branch), cwd=self.dest) - except: # nosec - pass + except Exception: + self.logger.info(f"failed to run git checkout of {self.branch}", exc_info=True) try: out = self._exec_cmd(self._pull_ff_only_cmd_, cwd=self.dest) - self.logger.debug("synced with upstream git repo, output: {}".format(out.decode())) + self.logger.debug(f"synced with upstream git repo, output: {out.decode()}") self._write_graph() except UbuntuGitServer503Error: raise - except: # nosec + except Exception: # if something other than 503 occurred at this point just remove the repo and re-clone + self.logger.exception("unexpected exception syncing with upstream; will delete and re-clone") self._delete_repo() self._clone_repo() @@ -207,14 +208,14 @@ def get_content(self, git_rev: GitRevision) -> list[str]: cmd = self._get_rev_content_cmd_.format(sha=git_rev.sha, file=git_rev.file) out = self._exec_cmd(cmd, cwd=self.dest) return out.decode().splitlines() - except: - self.logger.exception("failed to get content for {} from git commit {}".format(git_rev.file, git_rev.sha)) + except Exception: + self.logger.exception(f"failed to get content for {git_rev.file} from git commit {git_rev.sha}") def get_current_rev(self) -> str: try: rev = self._exec_cmd(self._head_rev_cmd_, cwd=self.dest) return rev.decode().strip() if isinstance(rev, bytes) else rev - except: + except Exception: self.logger.exception("unable to get current git revision") @staticmethod diff --git a/src/vunnel/providers/ubuntu/parser.py b/src/vunnel/providers/ubuntu/parser.py index 566fa709..584d0e21 100644 --- a/src/vunnel/providers/ubuntu/parser.py +++ b/src/vunnel/providers/ubuntu/parser.py @@ -496,7 +496,8 @@ def map_parsed(parsed_cve: CVEFile, logger: logging.Logger | None = None): r = Vulnerability() try: r.Severity = getattr(Severity, parsed_cve.priority.capitalize()) - except: + except Exception: + logger.exception("setting unknown severity due to exception getting severity") r.Severity = Severity.Unknown r.Name = parsed_cve.name diff --git a/src/vunnel/providers/wolfi/parser.py b/src/vunnel/providers/wolfi/parser.py index 84d25e81..9a18c383 100644 --- a/src/vunnel/providers/wolfi/parser.py +++ b/src/vunnel/providers/wolfi/parser.py @@ -59,7 +59,7 @@ def _download(self): fp.write(chunk) else: r.raise_for_status() - except: # noqa + except Exception: self.logger.exception(f"ignoring error processing secdb for {self.url}") def _load(self): diff --git a/src/vunnel/utils/__init__.py b/src/vunnel/utils/__init__.py index 382c8b80..435c5e56 100644 --- a/src/vunnel/utils/__init__.py +++ b/src/vunnel/utils/__init__.py @@ -22,19 +22,21 @@ def wrapper(*args: Any, **kwargs: Any) -> Any: logger = logging.getLogger("utils:retry-with-backoff") attempt = 0 while attempt < retries: + err = None try: return f(*args, **kwargs) except KeyboardInterrupt: logger.warning("keyboard interrupt, cancelling request...") raise - except: # noqa: E722 + except Exception as e: + err = e if attempt >= retries: logger.exception(f"failed after {retries} retries") raise # explanation of S311 disable: random number is not used for cryptography sleep = backoff_in_seconds * 2**attempt + random.uniform(0, 1) # noqa: S311 - logger.warning(f"{f} failed. Retrying in {int(sleep)} seconds (attempt {attempt+1} of {retries})") + logger.warning(f"{f} failed with {err}. Retrying in {int(sleep)} seconds (attempt {attempt+1} of {retries})") time.sleep(sleep) attempt += 1 diff --git a/src/vunnel/utils/oval_parser.py b/src/vunnel/utils/oval_parser.py index 47477966..e2e524c0 100644 --- a/src/vunnel/utils/oval_parser.py +++ b/src/vunnel/utils/oval_parser.py @@ -81,7 +81,7 @@ def parse(dest_file: str, config: Config, vuln_dict: dict | None = None): elif event == "end" and re.search(config.tag_pattern, element.tag).group(1) == "definition": try: _process_definition(element, vuln_dict, config) - except: + except Exception: logger.exception("Error parsing oval record. Logging error and continuing") finally: processing = False diff --git a/src/vunnel/utils/oval_v2.py b/src/vunnel/utils/oval_v2.py index 74da4212..b5689773 100644 --- a/src/vunnel/utils/oval_v2.py +++ b/src/vunnel/utils/oval_v2.py @@ -177,6 +177,7 @@ def _parse_group(criteria_element: ET.Element, config: OVALParserConfig) -> list """ results = [] + logger = logging.getLogger("oval-v2-parser") # further parsing makes the assumption that this element has 2 children, bail out of here if that's not true if len(criteria_element) != 2: @@ -190,7 +191,8 @@ def _parse_group(criteria_element: ET.Element, config: OVALParserConfig) -> list try: test_ids = VulnerabilityParser._parse_sub_group(criteria_element[1], config, config.artifact_regex) - except: + except Exception: + logger.exception("returning results early due to exception in _parse_sub_group") return results if not test_ids: