From 0ee1c64959e339387b25dfb83dc3fce6bff7e548 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20L=C3=B3pez?= Date: Tue, 8 Nov 2022 17:14:01 -0300 Subject: [PATCH 1/2] fail-on: rework feature This adds a new `fail_on` config option that can be changed with a mutually exclusive group of argument flags. It also decouples the exclude_* and fail_on flags, so you can do things like `fail_on: pedantic` while disabling optimization findings. Additionally, this adds some new code to detect the old-style config options, migrate their settings, and alert the user. Fixes #1458 --- slither/__main__.py | 78 +++++++++++++++++++---------------- slither/utils/command_line.py | 50 ++++++++++++++++++++-- 2 files changed, 88 insertions(+), 40 deletions(-) diff --git a/slither/__main__.py b/slither/__main__.py index 70357586e9..a2402fb606 100644 --- a/slither/__main__.py +++ b/slither/__main__.py @@ -28,6 +28,8 @@ from slither.utils.output_capture import StandardOutputCapture from slither.utils.colors import red, set_colorization_enabled from slither.utils.command_line import ( + FailOnLevel, + migrate_config_options, output_detectors, output_results_to_markdown, output_detectors_json, @@ -220,22 +222,22 @@ def choose_detectors( detectors_to_run = sorted(detectors_to_run, key=lambda x: x.IMPACT) return detectors_to_run - if args.exclude_optimization and not args.fail_pedantic: + if args.exclude_optimization: detectors_to_run = [ d for d in detectors_to_run if d.IMPACT != DetectorClassification.OPTIMIZATION ] - if args.exclude_informational and not args.fail_pedantic: + if args.exclude_informational: detectors_to_run = [ d for d in detectors_to_run if d.IMPACT != DetectorClassification.INFORMATIONAL ] - if args.exclude_low and not args.fail_low: + if args.exclude_low: detectors_to_run = [d for d in detectors_to_run if d.IMPACT != DetectorClassification.LOW] - if args.exclude_medium and not args.fail_medium: + if args.exclude_medium: detectors_to_run = [ d for d in detectors_to_run if d.IMPACT != DetectorClassification.MEDIUM ] - if args.exclude_high and not args.fail_high: + if args.exclude_high: detectors_to_run = [d for d in detectors_to_run if d.IMPACT != DetectorClassification.HIGH] if args.detectors_to_exclude: detectors_to_run = [ @@ -401,41 +403,44 @@ def parse_args( default=defaults_flag_in_config["exclude_high"], ) - group_detector.add_argument( + fail_on_group = group_detector.add_mutually_exclusive_group() + fail_on_group.add_argument( "--fail-pedantic", - help="Return the number of findings in the exit code", - action="store_true", - default=defaults_flag_in_config["fail_pedantic"], + help="Fail if any findings are detected", + action="store_const", + dest="fail_on", + const=FailOnLevel.PEDANTIC, ) - - group_detector.add_argument( - "--no-fail-pedantic", - help="Do not return the number of findings in the exit code. Opposite of --fail-pedantic", - dest="fail_pedantic", - action="store_false", - required=False, - ) - - group_detector.add_argument( + fail_on_group.add_argument( "--fail-low", - help="Fail if low or greater impact finding is detected", - action="store_true", - default=defaults_flag_in_config["fail_low"], + help="Fail if any low or greater impact findings are detected", + action="store_const", + dest="fail_on", + const=FailOnLevel.LOW, ) - - group_detector.add_argument( + fail_on_group.add_argument( "--fail-medium", - help="Fail if medium or greater impact finding is detected", - action="store_true", - default=defaults_flag_in_config["fail_medium"], + help="Fail if any medium or greater impact findings are detected", + action="store_const", + dest="fail_on", + const=FailOnLevel.MEDIUM, ) - - group_detector.add_argument( + fail_on_group.add_argument( "--fail-high", - help="Fail if high impact finding is detected", - action="store_true", - default=defaults_flag_in_config["fail_high"], + help="Fail if any high impact findings are detected", + action="store_const", + dest="fail_on", + const=FailOnLevel.HIGH, + ) + fail_on_group.add_argument( + "--fail-none", + "--no-fail-pedantic", + help="Do not return the number of findings in the exit code", + action="store_const", + dest="fail_on", + const=FailOnLevel.NONE, ) + fail_on_group.set_defaults(fail_on=FailOnLevel.PEDANTIC) group_detector.add_argument( "--show-ignored-findings", @@ -910,17 +915,18 @@ def main_impl( stats = pstats.Stats(cp).sort_stats("cumtime") stats.print_stats() - if args.fail_high: + fail_on = FailOnLevel(args.fail_on) + if fail_on == FailOnLevel.HIGH: fail_on_detection = any(result["impact"] == "High" for result in results_detectors) - elif args.fail_medium: + elif fail_on == FailOnLevel.MEDIUM: fail_on_detection = any( result["impact"] in ["Medium", "High"] for result in results_detectors ) - elif args.fail_low: + elif fail_on == FailOnLevel.LOW: fail_on_detection = any( result["impact"] in ["Low", "Medium", "High"] for result in results_detectors ) - elif args.fail_pedantic: + elif fail_on == FailOnLevel.PEDANTIC: fail_on_detection = bool(results_detectors) else: fail_on_detection = False diff --git a/slither/utils/command_line.py b/slither/utils/command_line.py index c2fef5eca0..a650960f56 100644 --- a/slither/utils/command_line.py +++ b/slither/utils/command_line.py @@ -1,4 +1,5 @@ import argparse +import enum import json import os import re @@ -27,6 +28,15 @@ "list-printers", ] + +class FailOnLevel(enum.Enum): + PEDANTIC = "pedantic" + LOW = "low" + MEDIUM = "medium" + HIGH = "high" + NONE = "none" + + # Those are the flags shared by the command line and the config file defaults_flag_in_config = { "detectors_to_run": "all", @@ -38,10 +48,7 @@ "exclude_low": False, "exclude_medium": False, "exclude_high": False, - "fail_pedantic": True, - "fail_low": False, - "fail_medium": False, - "fail_high": False, + "fail_on": FailOnLevel.PEDANTIC.value, "json": None, "sarif": None, "json-types": ",".join(DEFAULT_JSON_OUTPUT_TYPES), @@ -57,6 +64,13 @@ **DEFAULTS_FLAG_IN_CONFIG_CRYTIC_COMPILE, } +deprecated_flags = { + "fail_pedantic": True, + "fail_low": False, + "fail_medium": False, + "fail_high": False, +} + def read_config_file(args: argparse.Namespace) -> None: # No config file was provided as an argument @@ -73,6 +87,12 @@ def read_config_file(args: argparse.Namespace) -> None: with open(args.config_file, encoding="utf8") as f: config = json.load(f) for key, elem in config.items(): + if key in deprecated_flags: + logger.info( + yellow(f"{args.config_file} has a deprecated key: {key} : {elem}") + ) + migrate_config_options(args, key, elem) + continue if key not in defaults_flag_in_config: logger.info( yellow(f"{args.config_file} has an unknown key: {key} : {elem}") @@ -87,6 +107,28 @@ def read_config_file(args: argparse.Namespace) -> None: logger.error(yellow("Falling back to the default settings...")) +def migrate_config_options(args: argparse.Namespace, key: str, elem): + if key.startswith("fail_") and getattr(args, "fail_on") == defaults_flag_in_config["fail_on"]: + if key == "fail_pedantic": + pedantic_setting = elem + fail_on = pedantic_setting and FailOnLevel.PEDANTIC or FailOnLevel.NONE + setattr(args, "fail_on", fail_on) + logger.info( + "Migrating fail_pedantic: {} as fail_on: {}".format(pedantic_setting, fail_on.value) + ) + elif key == "fail_low" and elem == True: + logger.info("Migrating fail_low: true -> fail_on: low") + setattr(args, "fail_on", FailOnLevel.LOW) + elif key == "fail_medium" and elem == True: + logger.info("Migrating fail_medium: true -> fail_on: medium") + setattr(args, "fail_on", FailOnLevel.MEDIUM) + elif key == "fail_high" and elem == True: + logger.info("Migrating fail_high: true -> fail_on: high") + setattr(args, "fail_on", FailOnLevel.HIGH) + else: + logger.warn(yellow("Key {} was deprecated but no migration was provided".format(key))) + + def output_to_markdown( detector_classes: List[Type[AbstractDetector]], printer_classes: List[Type[AbstractPrinter]], From 8b0fd32cbe32b6e873bccc07de5231f8793e1686 Mon Sep 17 00:00:00 2001 From: alpharush <0xalpharush@protonmail.com> Date: Fri, 23 Jun 2023 09:33:05 -0500 Subject: [PATCH 2/2] use enum instead of value in config, lint --- slither/__main__.py | 1 - slither/utils/command_line.py | 18 +++++++++--------- 2 files changed, 9 insertions(+), 10 deletions(-) diff --git a/slither/__main__.py b/slither/__main__.py index 4fcfd896d6..d9201a90d9 100644 --- a/slither/__main__.py +++ b/slither/__main__.py @@ -36,7 +36,6 @@ from slither.utils.colors import red, set_colorization_enabled from slither.utils.command_line import ( FailOnLevel, - migrate_config_options, output_detectors, output_results_to_markdown, output_detectors_json, diff --git a/slither/utils/command_line.py b/slither/utils/command_line.py index 8518ada4a6..0824725821 100644 --- a/slither/utils/command_line.py +++ b/slither/utils/command_line.py @@ -54,7 +54,7 @@ class FailOnLevel(enum.Enum): "exclude_low": False, "exclude_medium": False, "exclude_high": False, - "fail_on": FailOnLevel.PEDANTIC.value, + "fail_on": FailOnLevel.PEDANTIC, "json": None, "sarif": None, "json-types": ",".join(DEFAULT_JSON_OUTPUT_TYPES), @@ -118,22 +118,22 @@ def migrate_config_options(args: argparse.Namespace, key: str, elem): if key.startswith("fail_") and getattr(args, "fail_on") == defaults_flag_in_config["fail_on"]: if key == "fail_pedantic": pedantic_setting = elem - fail_on = pedantic_setting and FailOnLevel.PEDANTIC or FailOnLevel.NONE + fail_on = FailOnLevel.PEDANTIC if pedantic_setting else FailOnLevel.NONE setattr(args, "fail_on", fail_on) - logger.info( - "Migrating fail_pedantic: {} as fail_on: {}".format(pedantic_setting, fail_on.value) - ) - elif key == "fail_low" and elem == True: + logger.info(f"Migrating fail_pedantic: {pedantic_setting} as fail_on: {fail_on.value}") + elif key == "fail_low" and elem is True: logger.info("Migrating fail_low: true -> fail_on: low") setattr(args, "fail_on", FailOnLevel.LOW) - elif key == "fail_medium" and elem == True: + + elif key == "fail_medium" and elem is True: logger.info("Migrating fail_medium: true -> fail_on: medium") setattr(args, "fail_on", FailOnLevel.MEDIUM) - elif key == "fail_high" and elem == True: + + elif key == "fail_high" and elem is True: logger.info("Migrating fail_high: true -> fail_on: high") setattr(args, "fail_on", FailOnLevel.HIGH) else: - logger.warn(yellow("Key {} was deprecated but no migration was provided".format(key))) + logger.warning(yellow(f"Key {key} was deprecated but no migration was provided")) def output_to_markdown(