fix(cli): fix --no-warnings, exit code, github output and threshold config

presidio-cli/README.md

@@ -70,7 +70,7 @@ Configuration file supports the following parameters in a yaml file:
 
 - allow - list of tokens that should not be marked as PII.
 
-- threshold - only show problems/findings whose scores are at or above this threshold.
+- threshold - only show problems/findings whose scores are at or above this threshold. Must be a number between 0 and 1.
 
 Note: a file requires at least one parameter to be set.
 
@@ -163,19 +163,19 @@ tests/conftest.py
   37:33     0.85     PERSON
 ```
 
-- github - similar to diff function in github
+- github - [GitHub Actions workflow commands](https://docs.github.com/en/actions/reference/workflows-and-actions/workflow-commands) that create a `warning` or `error` annotation for each finding
 
 ```shell
 presidio -d "entities:
   - PERSON" -f github tests/conftest.py
 # result
 ::group::tests/conftest.py
-::0.85 file=tests/conftest.py,line=34,col=58::34:58 [PERSON]
-::0.85 file=tests/conftest.py,line=37,col=33::37:33 [PERSON]
+::warning file=tests/conftest.py,line=34,col=58::34:58 [PERSON] score=0.85
+::warning file=tests/conftest.py,line=37,col=33::37:33 [PERSON] score=0.85
 ::endgroup::
 ```
 
-- colored - standard output format but with colors
+- colored - standard output format but with colors: error scores are red, warning scores are yellow
 
 - parsable - easy to parse automaticaly
 
@@ -191,6 +191,31 @@ presidio -d "entities:
   - github, if run on github - environment variables `GITHUB_ACTIONS` and `GITHUB_WORKFLOW` are set
   - colored, otherwise
 
+### Warnings and errors
+
+Each finding has a level based on its score:
+
+- error - the score is 1.0
+- warning - the score is below 1.0
+
+Use `--no-warnings` to output only error-level findings:
+
+```shell
+presidio --no-warnings tests/
+```
+
+### Exit codes
+
+- `0` - no findings were output
+- `1` - at least one finding was output, or the configuration is invalid
+- `2` - invalid command-line arguments
+
+Findings filtered out by `threshold` or `--no-warnings` do not affect the exit code. To report findings without failing a CI step, ignore the exit code:
+
+```shell
+presidio . || true
+```
+
 ### List of all parameters
 
 Simply run the following to get a list of all available options for the CLI:

presidio-cli/presidio_cli/analyzer.py

@@ -61,6 +61,8 @@ def __init__(self, line: int, recognizer_result: RecognizerResult) -> None:
         self.type = self.recognizer_result["entity_type"]
         # Score as a probability determined by the model
         self.score = self.recognizer_result["score"]
+        #: Severity: "error" for a full-confidence finding, "warning" otherwise
+        self.level = "error" if self.score >= 1.0 else "warning"
 
 
 def _analyze(

presidio-cli/presidio_cli/cli.py

@@ -53,7 +53,7 @@ def standard_color(problem: PIIProblem) -> str:
         """
         line = "  \033[2m%d:%d\033[0m" % (problem.line, problem.column)
         line += max(20 - len(line), 0) * " "
-        if problem.score < 1:  # warning
+        if problem.level == "warning":
             line += "\033[33m%s\033[0m" % problem.score
         else:
             line += "\033[31m%s\033[0m" % problem.score
@@ -66,19 +66,34 @@ def standard_color(problem: PIIProblem) -> str:
     @staticmethod
     def github(problem: PIIProblem, filename: str) -> str:
         """
-        Output the problem in git-diff-like format.
+        Output the problem as a GitHub Actions warning or error workflow command.
 
         :param problem: PIIProblem to be formatted.
         :param filename: Filename where the problem occurs.
+        :return: Workflow command that creates an annotation for the problem.
         """
-        line = (
-            f"::{str(problem.score)} file={filename},line={format(problem.line)},"
-            + f"col={format(problem.column)}::{format(problem.line)}"
-            + f":{format(problem.column)} [{problem.type}]"
-        )
+        message = f"{problem.line}:{problem.column} [{problem.type}]"
+        message += f" score={problem.score}"
         if problem.explanation:
-            line += problem.explanation
-        return line
+            message += f" ({problem.explanation})"
+        # Drop the ./ that `presidio .` adds, as run() does before analyze()
+        if filename.startswith(("./", ".\\")):
+            filename = filename[2:]
+        file = _escape_github_property(filename)
+        return (
+            f"::{problem.level} file={file},line={problem.line},col={problem.column}"
+            f"::{_escape_github_data(message)}"
+        )
+
+
+def _escape_github_data(value: str) -> str:
+    """Escape the message of a GitHub Actions workflow command."""
+    return value.replace("%", "%25").replace("\r", "%0D").replace("\n", "%0A")
+
+
+def _escape_github_property(value: str) -> str:
+    """Escape a property value of a GitHub Actions workflow command."""
+    return _escape_github_data(value).replace(":", "%3A").replace(",", "%2C")
 
 
 def threshold_value(value: str) -> float:
@@ -113,16 +128,17 @@ def show_problems(
     file: str,
     args_format: str,
     no_warn: bool,
-):
+) -> int:
     """
     Show formatted output of discovered problems.
 
     :param problems: generator of PIIProblem objects
     :param file: processed filename for 'stdin'
     :param args_format: format in which to output discovered problems
     :param no_warn: whether to output only error level problems
+    :return: number of problems that were output
     """
-    max_level = 0
+    prob_num = 0
     first = True
 
     if args_format == "auto":
@@ -134,11 +150,12 @@ def show_problems(
     for problem in problems:
         if no_warn and (problem.level != "error"):
             continue
+        prob_num += 1
         if args_format == "parsable":
             print(Format.parsable(problem))
         elif args_format == "github":
             if first:
-                print("::group::%s" % file)
+                print("::group::%s" % _escape_github_data(file))
                 first = False
             print(Format.github(problem, file))
         elif args_format == "colored":
@@ -158,7 +175,7 @@ def show_problems(
     if not first and args_format != "parsable":
         print("")
 
-    return max_level
+    return prob_num
 
 
 def find_files_recursively(
@@ -269,7 +286,7 @@ def run() -> None:
         except Exception:
             traceback.print_exc()
             continue
-        prob_num = show_problems(
+        prob_num += show_problems(
             problems, file, args_format=args.format, no_warn=args.no_warnings
         )
 
@@ -279,7 +296,7 @@ def run() -> None:
         except EnvironmentError as e:
             print(e, file=sys.stderr)
             sys.exit(1)
-        prob_num = show_problems(
+        prob_num += show_problems(
             problems,
             "stdin",
             args_format=args.format,

presidio-cli/presidio_cli/config.py

@@ -100,12 +100,22 @@ def parse(self, raw_content: str) -> None:
             self.entities = self.analyzer.get_supported_entities()
 
         if "threshold" in conf:
-            if not 0 <= float(self.threshold) <= 1:
+            try:
+                # YAML loads true/yes/on as booleans, which float() would accept
+                if isinstance(conf["threshold"], bool):
+                    raise TypeError("threshold is a boolean")
+                threshold = float(conf["threshold"])
+            except (TypeError, ValueError, OverflowError) as e:
+                raise PresidioCLIConfigError(
+                    f"Invalid threshold value: {conf['threshold']}. "
+                    "Threshold must be a number"
+                ) from e
+            if not 0 <= threshold <= 1:
                 raise PresidioCLIConfigError(
-                    f"Invalid threshold value: {self.threshold}. "
+                    f"Invalid threshold value: {conf['threshold']}. "
                     f"Threshold must be between 0 and 1"
                 )
-            self.threshold = float(conf["threshold"])
+            self.threshold = threshold
         if "allow" in conf:
             self.allow_list = conf["allow"]
         if "language" in conf:

presidio-cli/tests/test_analyzer.py

@@ -1,5 +1,6 @@
 import pytest
-from presidio_cli.analyzer import analyze, line_generator
+from presidio_analyzer import RecognizerResult
+from presidio_cli.analyzer import PIIProblem, analyze, line_generator
 
 
 def test_line_generator():
@@ -59,3 +60,13 @@ def test_analyze_with_allow_list(en_core_web_lg, config, config_with_allow_list)
 def test_analyze_type_error(en_core_web_lg, config):
     with pytest.raises(TypeError):
         analyze({}, config)
+
+
+@pytest.mark.parametrize(
+    ("score", "level"),
+    [(1.0, "error"), (0.99, "warning"), (0.85, "warning"), (0.0, "warning")],
+)
+def test_pii_problem_level_is_error_only_for_full_confidence(score, level):
+    problem = PIIProblem(1, RecognizerResult("PERSON", 0, 5, score))
+
+    assert problem.level == level