ci(lint): add shell linter - Differential ShellCheck

[jamacku]

Differential ShellCheck is a GitHub action that performs differential ShellCheck scans on shell scripts changed via PR and reports results directly in PR.

I saw that your scripts are in great shape, but I think that you might find the differential-shellcheck action useful. It is able to produce reports in SARIF format. GitHub understands this format and is able to display it nicely as a PR comment, and on the Files Changed tab, please see below.

Documentation is available at @redhat-plumbers-in-action/differential-shellcheck. Let me know If you are missing some feature or setting. I'm always happy to extend functionality.

[kiblik]

This PR does not follow rule 0. from https://github.com/DefectDojo/django-DefectDojo/blob/master/readme-docs/CONTRIBUTING.md#submitting-pull-requests

Base your PR against the dev or bugfix branch, ...

Looks like somebody is injecting own tool to many open-source projects. It is not bad but I would like to be sure about real benefit. Can you provide an example of findings in this repo by your tool?

[jamacku]

This PR does not follow rule 0. from https://github.com/DefectDojo/django-DefectDojo/blob/master/readme-docs/CONTRIBUTING.md#submitting-pull-requests

I have changed the target branch to dev.

Looks like somebody is injecting own tool to many open-source projects. It is not bad but I would like to be sure about real benefit. Can you provide an example of findings in this repo by your tool?

"Injecting" it sounds really negative to me :-) I have no bad intentions. I want to raise awareness of static analysis of shell scripts. But feel free to reject this PR if you don't like it.

Here is the list of defects:

warning: Use single quotes, otherwise this expands now rather than when signalled.
   ┌─ ./docker/wait-for-it.sh:56:22
   │
56 │     trap "kill -INT -$PID" INT
   │                      ^^^^
   │
   = SC2064
   = For more information: https://www.shellcheck.net/wiki/SC2064

warning: Quote to prevent word splitting/globbing, or split robustly with mapfile or read -a.
   ┌─ ./docker/wait-for-it.sh:70:19
   │
70 │         hostport=(${1//:/ })
   │                   ^^^^^^^^^
   │
   = SC2206
   = For more information: https://www.shellcheck.net/wiki/SC2206

warning: Assigning an array to a string! Assign as array, or use * instead of @ to concatenate.
    ┌─ ./docker/wait-for-it.sh:116:13
    │
116 │         CLI="$@"
    │             ^^^^
    │
    = SC2124
    = For more information: https://www.shellcheck.net/wiki/SC2124

warning: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.
  ┌─ ./docker/unit-tests.sh:3:1
  │
3 │ cd /app
  │ ^^^^^^^
  │
  ┌─ ./docker/unit-tests.sh:3:8
  │
3 │ cd /app
  │        -
  │
  = SC2164
  = For more information: https://www.shellcheck.net/wiki/SC2164

warning: symlink appears unused. Verify use (or export if used externally).
   ┌─ ./docker/setEnv.sh:32:13
   │
32 │             symlink=$(readlink -f docker-compose.override.yml)
   │             ^^^^^^^
   │
   = SC2034
   = For more information: https://www.shellcheck.net/wiki/SC2034

warning: Quote this to prevent word splitting.
   ┌─ ./docker/setEnv.sh:34:28
   │
34 │         current_env=$(expr $(basename symlink) : "^docker-compose.override.\(.*\).yml$")
   │                            ^^^^^^^^^^^^^^^^^^^
   │
   = SC2046
   = For more information: https://www.shellcheck.net/wiki/SC2046

warning: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.
    ┌─ ./docker/setEnv.sh:132:1
    │
132 │ cd ${target_dir}
    │ ^^^^^^^^^^^^^^^^
    │
    ┌─ ./docker/setEnv.sh:132:17
    │
132 │ cd ${target_dir}
    │                 -
    │
    = SC2164
    = For more information: https://www.shellcheck.net/wiki/SC2164

warning: Remove quotes from right-hand side of =~ to match as a regex rather than literally.
    ┌─ ./docker/setEnv.sh:134:93
    │
134 │ if [ ${#} -eq 1 ] && [[ 'dev debug unit_tests unit_tests_cicd integration_tests release' =~ "${1}" ]]
    │                                                                                             ^^^^^^
    │
    = SC2076
    = For more information: https://www.shellcheck.net/wiki/SC2076

warning: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.
  ┌─ ./docker/entrypoint-uwsgi-dev.sh:4:1
  │
4 │ cd /app
  │ ^^^^^^^
  │
  ┌─ ./docker/entrypoint-uwsgi-dev.sh:4:8
  │
4 │ cd /app
  │        -
  │
  = SC2164
  = For more information: https://www.shellcheck.net/wiki/SC2164

warning: In POSIX sh, == in place of = is undefined.
   ┌─ ./docker/entrypoint-uwsgi-dev.sh:11:18
   │
11 │ if [ ${DD_DEBUG} == "True" ]; then
   │                  ^^
   │
   = SC3014
   = For more information: https://www.shellcheck.net/wiki/SC3014

warning: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.
  ┌─ ./docker/entrypoint-unit-tests.sh:9:1
  │
9 │ cd /app
  │ ^^^^^^^
  │
  ┌─ ./docker/entrypoint-unit-tests.sh:9:8
  │
9 │ cd /app
  │        -
  │
  = SC2164
  = For more information: https://www.shellcheck.net/wiki/SC2164

warning: In POSIX sh, echo flags are undefined.
   ┌─ ./docker/entrypoint-nginx.sh:29:8
   │
29 │   echo -n $METRICS_HTTP_AUTH_USER:$(openssl passwd -apr1 $METRICS_HTTP_AUTH_PASSWORD) >> /etc/nginx/.htpasswd
   │        ^^
   │
   = SC3037
   = For more information: https://www.shellcheck.net/wiki/SC3037

warning: Quote this to prevent word splitting.
   ┌─ ./docker/entrypoint-nginx.sh:29:35
   │
29 │   echo -n $METRICS_HTTP_AUTH_USER:$(openssl passwd -apr1 $METRICS_HTTP_AUTH_PASSWORD) >> /etc/nginx/.htpasswd
   │                                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   │
   = SC2046
   = For more information: https://www.shellcheck.net/wiki/SC2046

warning: Declare and assign separately to avoid masking return values.
   ┌─ ./docker/entrypoint-integration-tests.sh:25:8
   │
25 │ export CHROMEDRIVER=$(find /opt/chrome-driver -name chromedriver)
   │        ^^^^^^^^^^^^
   │
   = SC2155
   = For more information: https://www.shellcheck.net/wiki/SC2155

warning: In POSIX sh, echo flags are undefined.
   ┌─ ./docker/entrypoint-initializer.sh:61:6
   │
61 │ echo -n "Waiting for database to be reachable "
   │      ^^
   │
   = SC3037
   = For more information: https://www.shellcheck.net/wiki/SC3037

warning: In POSIX sh, echo flags are undefined.
   ┌─ ./docker/entrypoint-initializer.sh:64:8
   │
64 │   echo -n "."
   │        ^^
   │
   = SC3037
   = For more information: https://www.shellcheck.net/wiki/SC3037

warning: Declare and assign separately to avoid masking return values.
   ┌─ ./docker/entrypoint-initializer.sh:89:10
   │
89 │   export DD_ADMIN_PASSWORD="$(cat /dev/urandom | LC_ALL=C tr -dc a-zA-Z0-9 | \
   │          ^^^^^^^^^^^^^^^^^
   │
   = SC2155
   = For more information: https://www.shellcheck.net/wiki/SC2155

warning: Declare and assign separately to avoid masking return values.
   ┌─ ./docker/entrypoint-initializer.sh:96:10
   │
96 │   export DD_JIRA_WEBHOOK_SECRET="$(uuidgen)"
   │          ^^^^^^^^^^^^^^^^^^^^^^
   │
   = SC2155
   = For more information: https://www.shellcheck.net/wiki/SC2155

warning: In POSIX sh, echo flags are undefined.
   ┌─ ./docker/entrypoint-celery-worker.sh:19:6
   │
19 │ echo -n "Waiting for database to be reachable "
   │      ^^
   │
   = SC3037
   = For more information: https://www.shellcheck.net/wiki/SC3037

warning: In POSIX sh, echo flags are undefined.
   ┌─ ./docker/entrypoint-celery-worker.sh:22:8
   │
22 │   echo -n "."
   │        ^^
   │
   = SC3037
   = For more information: https://www.shellcheck.net/wiki/SC3037

warning: In POSIX sh, echo flags are undefined.
   ┌─ ./docker/entrypoint-celery-beat.sh:19:6
   │
19 │ echo -n "Waiting for database to be reachable "
   │      ^^
   │
   = SC3037
   = For more information: https://www.shellcheck.net/wiki/SC3037

warning: In POSIX sh, echo flags are undefined.
   ┌─ ./docker/entrypoint-celery-beat.sh:22:8
   │
22 │   echo -n "."
   │        ^^
   │
   = SC3037
   = For more information: https://www.shellcheck.net/wiki/SC3037

error: > is for string comparisons. Use -gt instead.
  ┌─ ./docker/dojo-data.bash:4:9
  │
4 │ if [ $# > 1 ]
  │         ^
  │
  = SC2071
  = For more information: https://www.shellcheck.net/wiki/SC2071

Most of them seem to be false positives or minor issues. Having a static analysis of shell scripts might help you with the review when changing the shell scripts in the future.

[kiblik]

@jamacku, sorry for this "negative narrative" :)

Based on the output of the tool, I see it makes sense. You even identified a real bug (from the processing of symlink - it should not be removed but it should be processed correctly).

I like it.

[jamacku]

@kiblik I didn't take any offense. Feel free to ask anything. I'm happy to provide more examples.

[kiblik]

@kiblik I didn't take any offense. Feel free to ask anything. I'm happy to provide more examples.

If PR will be merged. Will we see new PRs offer fixes/warnings/errors only on files which have been changed in the PR?
If so, we might need to create issue/PR based on a "hand check".
Or do you offer also regular checks (renovate or dependabot) - useful for newly released checkers - which would scan the whole repo?

[jamacku]

If PR will be merged. Will we see new PRs offer fixes/warnings/errors only on files which have been changed in the PR? If so, we might need to create issue/PR based on a "hand check". Or do you offer also regular checks (renovate or dependabot) - useful for newly released checkers - which would scan the whole repo?

When running on PR, Differential ShellCheck will report only defects caused by changes in the PR.
When running on push (once PR is merged or when someone directly pushes to target branches), the Differential ShellCheck performs differential and full scans. Then, it uploads the full scan to GitHub, but the status check (the CI status ✅/❌) will represent again only the scan of the changes in the given set of commits that have been pushed.

Once/If this PR is merged, you will see all current defects with severity warning and higher in the Security tab under Code Scanning.

Example of a single defect

[Maffooch]

This action is super cool and would provide value to DefectDojo. @mtesauro how do you feel about it?

[github-advanced-security]

This pull request sets up GitHub code scanning for this repository. Once the scans have completed and the checks have passed, the analysis results for this pull request branch will appear on this overview. Once you merge this pull request, the 'Security' tab will show more code scanning analysis results (for example, for the default branch). Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results. For more information about GitHub code scanning, check out the documentation.

[jamacku]

Sorry, I have just accidentally committed some junk. It should now be clean.

[Maffooch]

There was some hoakey issues with the unit tests for a minute. Please pull the latest dev branch with the updated changes

[mtesauro]

Approved

[mtesauro]

We're going to implement ShellCheck slightly differently than is in the PR but appreciate you bringing it to our attention.

[mtesauro]

Thanks for the contribution but I think we're going to utilize a different source to run shellcheck