Skip to content

presidio-cli: --no-warnings crashes, exit code is always 0, -f github creates no annotations, config threshold is not validated #2265

Description

@v0ropaev

Four bugs in presidio-cli on main (f251c51). Setup, following AGENTS.md:

git clone https://github.com/data-privacy-stack/presidio
cd presidio/presidio-cli
uv sync --locked --all-extras --group dev
uv run python -m spacy download en_core_web_lg
CLI_PROJECT=$PWD

Run everything below from an empty directory. Inside presidio-cli/ the CLI loads presidio-cli/.presidiocli, which has no URL entity, so the two URL findings shown below would be missing.

mkdir /tmp/presidio-cli-repro && cd /tmp/presidio-cli-repro
presidio() { uv run --project "$CLI_PROJECT" presidio "$@"; }
printf 'Hello Paulo Santos.\nThe latest statement for your credit card account 4111 1111 1111 1111\nwas mailed to 123 Any Street, Seattle, WA 98109.\nContact: john.smith@example.com\n' > pii.txt
printf 'nothing to see here\n' > clean.txt

1. --no-warnings crashes when a file has a finding

$ presidio --no-warnings -f standard pii.txt
Traceback (most recent call last):
  ...
  File ".../presidio_cli/cli.py", line 135, in show_problems
    if no_warn and (problem.level != "error"):
                    ^^^^^^^^^^^^^
AttributeError: 'PIIProblem' object has no attribute 'level'

show_problems() reads problem.level, but PIIProblem never sets it. The unit tests miss this because the problems fixture in tests/test_cli.py is a mocker.Mock, which creates .level on access.

2. The exit code is always 0

$ presidio -f standard pii.txt; echo "exit=$?"
pii.txt
  1:7       0.85     PERSON
  2:51      1.0      CREDIT_CARD
  3:31      0.85     LOCATION
  3:40      0.85     LOCATION
  4:10      1.0      EMAIL_ADDRESS
  4:10      0.5      URL
  4:21      0.5      URL

exit=0
$ presidio -f standard pii.txt clean.txt > /dev/null; echo "exit=$?"
exit=0
$ presidio -f standard - < pii.txt > /dev/null; echo "exit=$?"
exit=0

run() ends with if prob_num > 0: return_code = 1, so exiting 1 on findings is intended. But show_problems() returns max_level = 0, which is never updated, and run() overwrites prob_num for every file instead of adding to it. The tests lock this in: test_show_problems expects rc == 0 for two problems, and test_run_current_dir expects exit 0 for a workspace that contains a credit card number.

3. -f github does not create annotations

$ presidio -f github pii.txt
::group::pii.txt
::0.85 file=pii.txt,line=1,col=7::1:7 [PERSON]
::1.0 file=pii.txt,line=2,col=51::2:51 [CREDIT_CARD]
::0.85 file=pii.txt,line=3,col=31::3:31 [LOCATION]
::0.85 file=pii.txt,line=3,col=40::3:40 [LOCATION]
::1.0 file=pii.txt,line=4,col=10::4:10 [EMAIL_ADDRESS]
::0.5 file=pii.txt,line=4,col=10::4:10 [URL]
::0.5 file=pii.txt,line=4,col=21::4:21 [URL]
::endgroup::

::0.85 is not a workflow command. The Actions runner only handles registered commands such as warning, error and notice (ActionCommand.TryParseV2 returns false for any other name), so these lines are printed as plain log text. The documented syntax is ::warning file={name},line={line},col={col}::{message} (https://docs.github.com/en/actions/reference/workflows-and-actions/workflow-commands). Property values and the message are not percent-escaped, so a file name containing a newline is printed as a separate log line, which the runner can read as a workflow command. With a directory argument (presidio -f github .) the file property is also ./pii.txt, and the runner passes relative paths through unchanged.

4. threshold in the config file is not validated

$ presidio -d "threshold: 5" -f standard pii.txt; echo "exit=$?"
exit=0
$ presidio -d "threshold: abc" -f standard pii.txt
Traceback (most recent call last):
  ...
  File ".../presidio_cli/config.py", line 108, in parse
    self.threshold = float(conf["threshold"])
ValueError: could not convert string to float: 'abc'

PresidioCLIConfig.parse() range-checks self.threshold, which still holds the default 0, and only afterwards assigns the configured value. So threshold: 5 is accepted and hides every finding, and a non-number raises an uncaught ValueError. The --threshold flag already rejects 5 (threshold must be between 0.0 and 1.0, exit 2).

Root cause

The CLI came from insightsengineering/presidio-cli, a yamllint port, and was imported in #918. The upstream initial commit already has line += str(problem.score) # TODO score to level and # max_level+=1 commented out, so level and exit-code handling was never finished. The config check has tested the wrong variable since the upstream "Threshold filtering" commit.

Expected behavior

  1. Each finding has a level: error for score 1.0 and warning below it, the same split the colored output already uses (red/yellow). --no-warnings prints only errors.
  2. The exit code is 1 when at least one finding is printed (across all files, or stdin), and 0 otherwise.
  3. -f github prints ::warning / ::error commands with escaped values and a plain relative file path, so GitHub Actions creates annotations.
  4. An invalid threshold in the config fails with a clear PresidioCLIConfigError message and exit code 1.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions