From 4a0f11a91a0fa0d430c9c731d93e10a34921bc37 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20L=C3=B3pez?= Date: Mon, 15 Aug 2022 23:52:18 -0300 Subject: [PATCH 1/3] Add support for new `--fail-*` behavior Fixes: #28 --- README.md | 32 +++++++++++++++++++++------- action.yml | 4 ++++ entrypoint.sh | 58 +++++++++++++++++++++++++++++++++++++++++++++++++-- 3 files changed, 84 insertions(+), 10 deletions(-) diff --git a/README.md b/README.md index fb337e9..a917ffb 100644 --- a/README.md +++ b/README.md @@ -32,6 +32,7 @@ jobs: | Key | Description |------------------|------------ | `ignore-compile` | If set to true, the Slither action will not attempt to compile the project. False by default. See [Advanced compilation](#advanced-compilation). +| `ignore-compile` | Cause the action to fail if Slither finds any issue of this severity or higher. See [action fail behavior](#action-fail-behavior). | `node-version` | The version of `node` to use. If this field is not set, the latest version will be used. | `sarif` | If provided, the path of the SARIF file to produce, relative to the repo root (see [Github Code Scanning integration](#github-code-scanning-integration)). | `slither-args` | Extra arguments to pass to Slither. @@ -47,6 +48,21 @@ If the project requires advanced compilation settings or steps, set Slither. You can find an example workflow that uses this option in the [examples](#examples) section. +### Action fail behavior + +The Slither action supports a `fail-on` option, based on the `--fail-*` flags added in Slither 0.8.4. To maintain the current action behavior, this option defaults to `all`. The following table summarizes the action behavior across different Slither versions. You +may adjust this option as needed for your workflows. If you are setting these options on your config file, set `fail-on: config` to prevent the action from overriding your settings. + + +| `fail-on` | Slither <= 0.8.3 | Slither > 0.8.3 +|--------------------]---------------------------|-------- +| `all` / `pedantic` | Fail on any finding | Fail on any finding +| `low` | Fail on any finding | Fail on any finding >= low +| `medium` | Fail on any finding | Fail on any finding >= medium +| `high` | Fail on any finding | Fail on any finding >- high +| `none` | Do not fail on findings | Do not fail on findings +| `config` | Determined by config file | Determined by config file + ### Triaging results Add `// slither-disable-next-line DETECTOR_NAME` before the finding, or use the @@ -84,9 +100,9 @@ jobs: - name: Run Slither uses: crytic/slither-action@v0.1.1 id: slither - continue-on-error: true with: sarif: results.sarif + fail-on: none - name: Upload SARIF file uses: github/codeql-action/upload-sarif@v2 @@ -96,7 +112,7 @@ jobs: Here: -- `continue-on-error: true` is required to let the SARIF upload step run if Slither finds issues +- `fail-on: none` is required to let the SARIF upload step run if Slither finds issues - `id: slither` is the name used to reference the step later on (e.g., in `steps.slither.outputs.sarif`) ## Examples @@ -131,8 +147,8 @@ NodeJS 16.x and install project dependencies before running Slither on the project. Slither will output findings in SARIF format, and those will get uploaded to GitHub. -We include `continue-on-error: true` on the Slither action to avoid failing the -run if findings are found. +We include `fail-on: none` on the Slither action to avoid failing the run if +findings are found. ```yaml name: Slither Analysis @@ -155,11 +171,11 @@ jobs: - name: Run Slither uses: crytic/slither-action@v0.1.1 - continue-on-error: true id: slither with: node-version: 16 sarif: results.sarif + fail-on: none - name: Upload SARIF file uses: github/codeql-action/upload-sarif@v2 @@ -176,8 +192,8 @@ virtual environment and install project dependencies before running Slither on the project. Slither will output findings in SARIF format, and those will get uploaded to GitHub. -We also include `continue-on-error: true` on the Slither action to avoid -failing the run if findings are found. +We also include `fail-on: none` on the Slither action to avoid failing the run +if findings are found. ```yaml name: Slither Analysis @@ -200,10 +216,10 @@ jobs: - name: Run Slither uses: crytic/slither-action@v0.1.1 - continue-on-error: true id: slither with: sarif: results.sarif + fail-on: none - name: Upload SARIF file uses: github/codeql-action/upload-sarif@v2 diff --git a/action.yml b/action.yml index 074ade5..fdf79df 100644 --- a/action.yml +++ b/action.yml @@ -21,6 +21,10 @@ inputs: description: 'Whether to ignore the compilation step when running crytic-compile and Slither.' default: false type: boolean + fail-on: + description: 'Cause the action to fail if Slither finds any findings of this severity or higher. By default it will fail if any finding is found' + default: all + type: string internal-github-workspace: # Do not set manually. This is a hacky way to pass the host workspace path to inside the action # This is used to improve compatibility when using `ignore-compile`. diff --git a/entrypoint.sh b/entrypoint.sh index 2523922..8703f6f 100755 --- a/entrypoint.sh +++ b/entrypoint.sh @@ -6,6 +6,10 @@ get() { env | sed -n "s/^$1=\(.*\)/\1/;T;p" } +version_lte() { + printf '%s\n%s\n' "$1" "$2" | sort -C -V +} + TARGET="$1" SOLCVER="$2" NODEVER="$3" @@ -30,6 +34,54 @@ compatibility_link() fi } +fail_on_flags() +{ + INSTALLED_VERSION="$(slither --version)" + FAIL_ON_LEVEL="$(get INPUT_FAIL-ON)" + + if [ "$FAIL_ON_LEVEL" = "config" ]; then + return + fi + + if version_lte "$INSTALLED_VERSION" "0.8.3"; then + # older behavior - fail on findings by default + case "$FAIL_ON_LEVEL" in + low|medium|high|pedantic|all) + echo "[!] Requested fail-on $FAIL_ON_LEVEL but it is unsupported on Slither $INSTALLED_VERSION, ignoring" >&2 + ;; + none) + echo "--ignore-return-value" + ;; + *) + echo "[!] Unknown fail-on value $FAIL_ON_LEVEL, ignoring" >&2 + ;; + esac + else + # newer behavior - does not fail on findings by default + case "$FAIL_ON_LEVEL" in + all|pedantic) + echo "--fail-pedantic" + ;; + low) + echo "--fail-low" + ;; + medium) + echo "--fail-medium" + ;; + high) + echo "--fail-high" + ;; + none) + # default behavior + ;; + *) + echo "[!] Unknown fail-on value $FAIL_ON_LEVEL, ignoring" >&2 + ;; + esac + + fi +} + install_solc() { if [[ -z "$SOLCVER" ]]; then @@ -190,9 +242,11 @@ if [[ -n "$SLITHERCONF" ]]; then CONFIGFLAG="--config-file=$SLITHERCONF" fi +FAILONFLAG="$(fail_on_flags)" + if [[ -z "$SLITHERARGS" ]]; then - slither "$TARGET" $SARIFFLAG $IGNORECOMPILEFLAG $CONFIGFLAG + slither "$TARGET" $SARIFFLAG $IGNORECOMPILEFLAG $FAILONFLAG $CONFIGFLAG else echo "[-] SLITHERARGS provided. Running slither with extra arguments" - printf "%s\n" "$SLITHERARGS" | xargs slither "$TARGET" $SARIFFLAG $IGNORECOMPILEFLAG $CONFIGFLAG + printf "%s\n" "$SLITHERARGS" | xargs slither "$TARGET" $SARIFFLAG $IGNORECOMPILEFLAG $FAILONFLAG $CONFIGFLAG fi From 81e9adf798dbf28b47c3006b1229a7cd125d7d6d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20L=C3=B3pez?= Date: Tue, 16 Aug 2022 00:13:14 -0300 Subject: [PATCH 2/3] Fix README typos and style --- README.md | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/README.md b/README.md index a917ffb..3e94fbb 100644 --- a/README.md +++ b/README.md @@ -32,7 +32,7 @@ jobs: | Key | Description |------------------|------------ | `ignore-compile` | If set to true, the Slither action will not attempt to compile the project. False by default. See [Advanced compilation](#advanced-compilation). -| `ignore-compile` | Cause the action to fail if Slither finds any issue of this severity or higher. See [action fail behavior](#action-fail-behavior). +| `fail-on` | Cause the action to fail if Slither finds any issue of this severity or higher. See [action fail behavior](#action-fail-behavior). | `node-version` | The version of `node` to use. If this field is not set, the latest version will be used. | `sarif` | If provided, the path of the SARIF file to produce, relative to the repo root (see [Github Code Scanning integration](#github-code-scanning-integration)). | `slither-args` | Extra arguments to pass to Slither. @@ -50,16 +50,19 @@ Slither. You can find an example workflow that uses this option in the ### Action fail behavior -The Slither action supports a `fail-on` option, based on the `--fail-*` flags added in Slither 0.8.4. To maintain the current action behavior, this option defaults to `all`. The following table summarizes the action behavior across different Slither versions. You -may adjust this option as needed for your workflows. If you are setting these options on your config file, set `fail-on: config` to prevent the action from overriding your settings. - +The Slither action supports a `fail-on` option, based on the `--fail-*` flags +added in Slither 0.8.4. To maintain the current action behavior, this option +defaults to `all`. The following table summarizes the action behavior across +different Slither versions. You may adjust this option as needed for your +workflows. If you are setting these options on your config file, set `fail-on: +config` to prevent the action from overriding your settings. | `fail-on` | Slither <= 0.8.3 | Slither > 0.8.3 -|--------------------]---------------------------|-------- -| `all` / `pedantic` | Fail on any finding | Fail on any finding +|--------------------|---------------------------|---------------- +| `all` / `pedantic` | Fail on any finding | Fail on any finding | `low` | Fail on any finding | Fail on any finding >= low | `medium` | Fail on any finding | Fail on any finding >= medium -| `high` | Fail on any finding | Fail on any finding >- high +| `high` | Fail on any finding | Fail on any finding >= high | `none` | Do not fail on findings | Do not fail on findings | `config` | Determined by config file | Determined by config file From 02a3526f7a94438918a7e557169cd2ca934fb95d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20L=C3=B3pez?= Date: Wed, 24 Aug 2022 13:08:05 -0300 Subject: [PATCH 3/3] Consider new --fail-pedantic default behavior The default behavior has been changed to --fail-pedantic, to maintain compatibility with old versions of this action and other tools. --- entrypoint.sh | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/entrypoint.sh b/entrypoint.sh index 8703f6f..a809d60 100755 --- a/entrypoint.sh +++ b/entrypoint.sh @@ -60,6 +60,7 @@ fail_on_flags() # newer behavior - does not fail on findings by default case "$FAIL_ON_LEVEL" in all|pedantic) + # default behavior on slither >= 0.8.4 echo "--fail-pedantic" ;; low) @@ -72,7 +73,7 @@ fail_on_flags() echo "--fail-high" ;; none) - # default behavior + echo "--no-fail-pedantic" ;; *) echo "[!] Unknown fail-on value $FAIL_ON_LEVEL, ignoring" >&2