`no-fail` CI option seems to conflict with config file `exclude_informational`

0xCLARITY commented 1 year ago

I have a slither config file that looks like this:

{
  "detectors_to_exclude": "timestamp,uninitialized-local,variable-scope",
  "exclude_dependencies": true,
  "exclude_informational": true,
  "solc": "0.8.17",

  "filter_paths": "lib",
  "solc_remaps": ["ds-test/=lib/ds-test/src/", "forge-std/=lib/forge-std/src/"]
}

With Slither 0.9.0, even running slither locally, I seem to need to pass the --no-fail-pedantic flag to get it to properly parse the exclude_informational flag in the config file as described here: https://github.com/crytic/slither/issues/1408

In a fork of slither-action, I tried echoing the --no-fail-pedantic flag if the fail-on was set to config:

if [ "$FAIL_ON_LEVEL" = "config" ]; then
       echo "--no-fail-pedantic"
       return
fi

This does get it to parse the exclude_informational flag in the config file but it incorrectly marks CI as successful even when I have a high severity failure (in my test case, I have some assembly that triggers https://github.com/crytic/slither/wiki/Detector-Documentation#incorrect-shift-in-assembly).

Any ideas on what the fix is here? Or do I need to rearrange my config file / CI setup to adhere to the newest version of Slither?

elopez commented 1 year ago

HI @0xCLARITY, thanks for the report! With the current implementation in Slither, fail-pedantic is mutually exclusive with exclude-informational and exclude-optimization, on the rationale that asking slither to fail on anything >= informational and then excluding informational findings does not make much sense (although it's unfortunate that optimizations and informationals are combined on that level). If you have any feedback or ideas to share on how to improve the situation, https://github.com/crytic/slither/issues/1408 would be the best place for it.

As for a way forward for your use case at this time, have you considered using fail-on: config plus something like this? All of these new command-line flags can also be provided in the config file.

{
  /* this is the default level, so disable it */
  "fail_pedantic": false,
  /* and enable other higher level, eg. low */
  "fail_low": true,

  /* and exclude informational */
  "exclude_informational": true,

  /* other options */
  /* ... */
}

0xCLARITY commented 1 year ago

That worked! Thank you for the guidance for how to work with the current implementation of fail-on!

I'll close this issue out.

crytic / slither-action

`no-fail` CI option seems to conflict with config file `exclude_informational` #38