Skip to content

Commit

Permalink
feat: add support for different display engines
Browse files Browse the repository at this point in the history
Currently supported engines are:
* `csgrep` (default)
* `sarif-fmt`
  • Loading branch information
jamacku committed May 9, 2024
1 parent beaa0a1 commit 5665c85
Show file tree
Hide file tree
Showing 14 changed files with 257 additions and 18 deletions.
1 change: 1 addition & 0 deletions .github/workflows/differential-shellcheck.yml
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ jobs:
exclude-path: |
test/**
src/**.{zsh,osh}
display-engine: sarif-fmt
token: ${{ secrets.GITHUB_TOKEN }}

- if: always()
Expand Down
2 changes: 1 addition & 1 deletion Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ ARG rpm_shellcheck="ShellCheck-${version_shellcheck}.fc${fedora}.${arch}.rpm"
# --- Install dependencies --- #

RUN dnf -y upgrade
RUN dnf -y install git koji jq \
RUN dnf -y install git koji jq sarif-fmt \
&& dnf clean all

# Download rpms from koji
Expand Down
18 changes: 18 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -302,6 +302,24 @@ List of file paths that will be scanned by ShellCheck. Globbing is supported. Th
* requirements: `optional`
* example: `"src/**.{shell,custom}"`

### display-engine

Tool used to display the defects and fixes in the console output. Currently supported tools are [`csgrep`](https://github.com/csutils/csdiff) and [`sarif-fmt`](https://github.com/psastras/sarif-rs/tree/main/sarif-fmt#readme).

<div align="center">
<div width="400">
<img src="docs/images/csgrep-output-example.png" width="400" alt="csgrep output example" />
<p><i>`display-engine: csgrep`</i></p>
</div>
<div width="400">
<img src="docs/images/sarif-fmt-output-example.png" width="400" alt="sarif-fmt output example" />
<p><i>`display-engine: sarif-fmt`</i></p>
</div>
</div>

* requirements: `optional`
* default value: `csgrep`

### token

Token used to upload findings in SARIF format to GitHub.
Expand Down
6 changes: 6 additions & 0 deletions action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,11 @@ inputs:
default: ''
required: false

display-engine:
description: Tool used to display the defects in the output. Valid values are csgrep and sarif-fmt.
default: csgrep
required: false

token:
description: GitHub TOKEN used to upload SARIF data.
required: false
Expand All @@ -101,3 +106,4 @@ runs:
INPUT_SCAN_DIRECTORY: ${{ inputs.scan-directory }}
INPUT_EXCLUDE_PATH: ${{ inputs.exclude-path }}
INPUT_INCLUDE_PATH: ${{ inputs.include-path }}
INPUT_DISPLAY_ENGINE: ${{ inputs.display-engine }}
Binary file added docs/images/csgrep-output-example.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file added docs/images/sarif-fmt-output-example.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
30 changes: 27 additions & 3 deletions src/functions.sh
Original file line number Diff line number Diff line change
Expand Up @@ -303,13 +303,37 @@ is_debug () {
# GITHUB_ACTIONS is set when Differential ShellCheck is running in GitHub Actions
# https://docs.github.com/en/actions/learn-github-actions/variables#default-environment-variables
is_github_actions () {
if [[ -z "${GITHUB_ACTIONS}" ]]; then
return 1
fi
[[ -z "${GITHUB_ACTIONS}" ]] && return 1
return 0
}

# Function to check if the script is run in unit tests environment
is_unit_tests () {
[[ -z "${UNIT_TESTS}" ]] && return 1
return 0
}

# Function to generate SARIF report
# $1 - <string> path to a file containing defects detected by scan
# $2 - <string> name of resulting SARIF file
generate_SARIF () {
[[ $# -le 1 ]] && return 1
local defects=$1
local output=$2

shellcheck_version=$(get_shellcheck_version)

# GitHub requires an absolute path, so let's remove the './' prefix from it.
csgrep \
--strip-path-prefix './' \
--embed-context 4 \
--mode=sarif \
--set-scan-prop='tool:ShellCheck' \
--set-scan-prop="tool-version:${shellcheck_version}" \
--set-scan-prop='tool-url:https://www.shellcheck.net/wiki/' \
"${defects}" > "${output}"
}

# Function to upload the SARIF report to GitHub
# Source: https://github.com/github/codeql-action/blob/dbe6f211e66b3aa5e9a5c4731145ed310ed54e28/lib/upload-lib.js#L104-L106
# Parameters: https://github.com/github/codeql-action/blob/69e09909dc219ed3374913e41c167490fc57202a/lib/upload-lib.js#L211-L224
Expand Down
12 changes: 1 addition & 11 deletions src/index.sh
Original file line number Diff line number Diff line change
Expand Up @@ -98,17 +98,7 @@ else
cp "${WORK_DIR}defects.log" "${WORK_DIR}sarif-defects.log"
fi

shellcheck_version=$(get_shellcheck_version)

# GitHub requires an absolute path, so let's remove the './' prefix from it.
csgrep \
--strip-path-prefix './' \
--embed-context 4 \
--mode=sarif \
--set-scan-prop='tool:ShellCheck' \
--set-scan-prop="tool-version:${shellcheck_version}" \
--set-scan-prop='tool-url:https://www.shellcheck.net/wiki/' \
"${WORK_DIR}sarif-defects.log" > output.sarif
generate_SARIF "${WORK_DIR}sarif-defects.log" "output.sarif"

# Produce report in HTML format
cshtml \
Expand Down
3 changes: 3 additions & 0 deletions src/setup.sh
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@

# TODO set up required variables

export INPUT_DISPLAY_ENGINE="${INPUT_DISPLAY_ENGINE:='csgrep'}"
is_debug && echo "DISPLAY_ENGINE: ${INPUT_DISPLAY_ENGINE}"

# Set required variables based on the environment
if is_github_actions; then
is_debug && echo "Running in GitHub Actions"
Expand Down
28 changes: 26 additions & 2 deletions src/validation.sh
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,30 @@ get_fixes () {
csdiff --fixed "${1}" "${2}" > "${WORK_DIR}fixes.log"
}

# Function to print results of fixed/introduced defects using display engine
# $1 - <string> file containing results in csdiff JSON format
# $2 - <number> embedded context value for csgrep
print_result () {
[[ $# -le 0 ]] && return 1
local results="$1"

if [[ "${INPUT_DISPLAY_ENGINE:-'csgrep'}" == "sarif-fmt" ]]; then
local color="always"
is_unit_tests && color="never"

# When sarif-fmt is used, we need to generate SARIF file first
# only csgrep can utilize JSON output from csdiff
generate_SARIF "${results}" "tmp.sarif"
sarif-fmt --color="${color}" < "tmp.sarif"
rm "tmp.sarif"
else
[[ $# -le 1 ]] && return 1
local context="$2"

csgrep --embed-context "${context}" "${results}"
fi
}

# Function to evaluate results of fixed defects and to provide feedback on standard output
# It expects file '../fixes.log' to contain fixes
# $? - return value is always 0
Expand All @@ -26,7 +50,7 @@ evaluate_and_print_fixes () {
num_of_fixes=$(get_number_of fixes)
if [[ "${num_of_fixes}" -gt 0 ]]; then
echo -e "${GREEN}Fixed defects${NOCOLOR}"
csgrep --embed-context 2 "${WORK_DIR}fixes.log"
print_result "${WORK_DIR}fixes.log" 2
else
echo -e "ℹ️ ${YELLOW}No Fixes!${NOCOLOR}"
fi
Expand Down Expand Up @@ -54,7 +78,7 @@ evaluate_and_print_defects () {
print_statistics

echo -e "${YELLOW}Defects, NEEDS INSPECTION${NOCOLOR}"
csgrep --embed-context 4 "${WORK_DIR}defects.log"
print_result "${WORK_DIR}defects.log" 4
return 1
fi

Expand Down
2 changes: 1 addition & 1 deletion test/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ ARG rpm_shellcheck="ShellCheck-${version_shellcheck}.fc${fedora}.${arch}.rpm"
# --- Install dependencies --- #

RUN dnf -y upgrade
RUN dnf -y install git koji kcov bats diffutils jq \
RUN dnf -y install git koji kcov bats diffutils jq sarif-fmt \
&& dnf clean all

# Download rpms from koji
Expand Down
49 changes: 49 additions & 0 deletions test/fixtures/print_result/defects.log
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
{
"defects": [
{
"checker": "SHELLCHECK_WARNING",
"language": "shell",
"tool": "shellcheck",
"key_event_idx": 0,
"events": [
{
"file_name": "innocent-script.sh",
"line": 7,
"event": "warning[SC2034]",
"message": "UNUSED_VAR2 appears unused. Verify use (or export if used externally).",
"verbosity_level": 0
}
]
},
{
"checker": "SHELLCHECK_WARNING",
"language": "shell",
"tool": "shellcheck",
"key_event_idx": 0,
"events": [
{
"file_name": "innocent-script.sh",
"line": 11,
"event": "warning[SC2115]",
"message": "Use \"${var:?}\" to ensure this never expands to / .",
"verbosity_level": 0
}
]
},
{
"checker": "SHELLCHECK_WARNING",
"language": "shell",
"tool": "shellcheck",
"key_event_idx": 0,
"events": [
{
"file_name": "innocent-script.sh",
"line": 11,
"event": "warning[SC2115]",
"message": "Use \"${var:?}\" to ensure this never expands to / .",
"verbosity_level": 0
}
]
}
]
}
34 changes: 34 additions & 0 deletions test/fixtures/print_result/fixes.log
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
{
"defects": [
{
"checker": "SHELLCHECK_WARNING",
"language": "shell",
"tool": "shellcheck",
"key_event_idx": 0,
"events": [
{
"file_name": "innocent-script.sh",
"line": 7,
"event": "warning[SC2034]",
"message": "UNUSED_VAR2 appears unused. Verify use (or export if used externally).",
"verbosity_level": 0
}
]
},
{
"checker": "SHELLCHECK_WARNING",
"language": "shell",
"tool": "shellcheck",
"key_event_idx": 0,
"events": [
{
"file_name": "innocent-script.sh",
"line": 11,
"event": "warning[SC2115]",
"message": "Use \"${var:?}\" to ensure this never expands to / .",
"verbosity_level": 0
}
]
}
]
}
90 changes: 90 additions & 0 deletions test/print_result.bats
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
# SPDX-License-Identifier: GPL-3.0-or-later

setup_file () {
load 'test_helper/common-setup'
_common_setup
}

setup () {
load 'test_helper/bats-assert/load'
load 'test_helper/bats-support/load'
}

@test "print_result() - arguments" {
source "${PROJECT_ROOT}/src/functions.sh"
source "${PROJECT_ROOT}/src/validation.sh"

INPUT_DISPLAY_ENGINE="csgrep"
run print_result
assert_failure 1

run print_result "./test/fixtures/print_result/defects.log"
assert_failure 1

run print_result "./test/fixtures/print_result/defects.log" "4"
assert_success

INPUT_DISPLAY_ENGINE="sarif-fmt"
run print_result "./test/fixtures/print_result/defects.log"
assert_success
}

@test "print_result() - csgrep" {
source "${PROJECT_ROOT}/src/validation.sh"

INPUT_DISPLAY_ENGINE="csgrep"
run print_result "./test/fixtures/print_result/defects.log" "4"
assert_success
assert_output \
'Error: SHELLCHECK_WARNING:
innocent-script.sh:7: warning[SC2034]: UNUSED_VAR2 appears unused. Verify use (or export if used externally).
Error: SHELLCHECK_WARNING:
innocent-script.sh:11: warning[SC2115]: Use "${var:?}" to ensure this never expands to / .
Error: SHELLCHECK_WARNING:
innocent-script.sh:11: warning[SC2115]: Use "${var:?}" to ensure this never expands to / .'

run print_result "./test/fixtures/print_result/fixes.log" "2"
assert_success
assert_output \
'Error: SHELLCHECK_WARNING:
innocent-script.sh:7: warning[SC2034]: UNUSED_VAR2 appears unused. Verify use (or export if used externally).
Error: SHELLCHECK_WARNING:
innocent-script.sh:11: warning[SC2115]: Use "${var:?}" to ensure this never expands to / .'
}

@test "print_result() - sarif-fmt" {
source "${PROJECT_ROOT}/src/functions.sh"
source "${PROJECT_ROOT}/src/validation.sh"

UNIT_TESTS="true"
INPUT_DISPLAY_ENGINE="sarif-fmt"
run print_result "./test/fixtures/print_result/defects.log"
assert_success
assert_output \
'warning: UNUSED_VAR2 appears unused. Verify use (or export if used externally).
warning: Use "${var:?}" to ensure this never expands to / .
warning: Use "${var:?}" to ensure this never expands to / .
warning: 3 warnings emitted'

run print_result "./test/fixtures/print_result/fixes.log"
assert_success
assert_output \
'warning: UNUSED_VAR2 appears unused. Verify use (or export if used externally).
warning: Use "${var:?}" to ensure this never expands to / .
warning: 2 warnings emitted'
}

teardown () {
export \
INPUT_DISPLAY_ENGINE="" \
UNIT_TESTS=""
rm -f tmp.sarif
}

0 comments on commit 5665c85

Please sign in to comment.