Unexpected execute_shellcheck behavior

[aidanSoles]

Type of issue

Bug (unexpected behavior).

Description

Hi! First, this project is great. TYSM for making/maintaining it.

Second, I'm hitting some unexpected behavior with this GitHub action. After doing some digging into the container's internals (after the GitHub action wasn't behaving as I expected), it looks like calls to execute_shellcheck and raw shellcheck calls behave differently (see below, which is output from inside the container):

[root@8d587919ec54 workspace]# ls # A sample script with warnings.
foo.sh
[root@8d587919ec54 workspace]# cat foo.sh 
#!/bin/bash

echo 'hi there'
if [ $? -ne 0 ]; then
    echo 'success'
fi

printf "%s hey\n"
printf "hi there %s\n"
[root@8d587919ec54 workspace]# cd /action
[root@8d587919ec54 action]# source functions.sh # To source the execute_shellcheck function.
[root@8d587919ec54 action]# cd -
/github/workspace
[root@8d587919ec54 workspace]# execute_shellcheck foo.sh # Emulating the call in /action/index.sh.

[root@8d587919ec54 workspace]# echo $?
0
[root@8d587919ec54 workspace]# shellcheck foo.sh # Raw shellcheck call.

In foo.sh line 4:
if [ $? -ne 0 ]; then
     ^-- SC2320 (warning): This $? refers to echo/printf, not a previous command. Assign to variable to avoid it being overwritten.
     ^-- SC2181 (style): Check exit code directly with e.g. 'if ! mycmd;', not indirectly with $?.


In foo.sh line 8:
printf "%s hey\n"
       ^--------^ SC2183 (warning): This format string has 1 variable, but is passed 0 argument.


In foo.sh line 9:
printf "hi there %s\n"
       ^-------------^ SC2183 (warning): This format string has 1 variable, but is passed 0 argument.

For more information:
  https://www.shellcheck.net/wiki/SC2183 -- This format string has 1 variable...
  https://www.shellcheck.net/wiki/SC2320 -- This $? refers to echo/printf, no...
  https://www.shellcheck.net/wiki/SC2181 -- Check exit code directly with e.g...
[root@8d587919ec54 workspace]# echo $?
1

I'd expect execute_shellcheck and raw shellcheck to act the same (same output and return code), but it doesn't seem like they are.

The questions I have are: Is this the expected behavior? Or is there a bug here? Or, do I have something misconfigured on my end (a very real possibility)?

Last, the job in my GitHub actions file is as follows:

  shellcheck:
    runs-on: ubuntu-latest
    # The below perms are needed by Differential ShellCheck.
    permissions:
      security-events: write
      actions: read
      contents: read
    name: Run shellcheck on modified shell scripts
    steps:
      - name: Checkout Code Repository
        uses: actions/checkout@v4
        with:
          # Differential ShellCheck requires full git history
          fetch-depth: 0
          # lfs: true
      - id: differential-shellcheck
        name: Differential ShellCheck
        uses: redhat-plumbers-in-action/differential-shellcheck@v5
        with:
          severity: style

TYSM. 😄

Describe the solution you'd like

A bug fix if this is a bug, and working config options if this is a PEBKAC. ❤️

[jamacku]

Hello, thank you for reaching out. I have tried to reproduce your issue.

$ bash

$ cat script.sh 
#!/bin/bash

echo 'hi there'
if [ $? -ne 0 ]; then
    echo 'success'
fi

printf "%s hey\n"
printf "hi there %s\n"

$ execute_shellcheck () {
  true && local external_sources=--external-sources

  local shellcheck_args=(
    --format=json1
    "${external_sources:-}"
    --severity="style" # <-- "${INPUT_SEVERITY}"
    "${@}"
  )

  local output
  output=$(shellcheck "${shellcheck_args[@]}" 2> /dev/null)

  echo "${output}"
}

$ execute_shellcheck script.sh 
{"comments":[{"file":"script.sh","line":4,"endLine":4,"column":6,"endColumn":8,"level":"warning","code":2320,"message":"This $? refers to echo/printf, not a previous command. Assign to variable to avoid it being overwritten.","fix":null},{"file":"script.sh","line":4,"endLine":4,"column":6,"endColumn":8,"level":"style","code":2181,"message":"Check exit code directly with e.g. 'if ! mycmd;', not indirectly with $?.","fix":null},{"file":"script.sh","line":8,"endLine":8,"column":8,"endColumn":18,"level":"warning","code":2183,"message":"This format string has 1 variable, but is passed 0 argument.","fix":null},{"file":"script.sh","line":9,"endLine":9,"column":8,"endColumn":23,"level":"warning","code":2183,"message":"This format string has 1 variable, but is passed 0 argument.","fix":null}]}

The problem in your reproducer was that you didn't set style input, which is passed to the script as ENV. If I wouldn't redirect stderr to /dev/null you would see the following error:

Unknown value for --severity. Valid options are: error, warning, info, style

It seems that execute_shellcheck works as expected. Please note that Differential ShellCheck reports only newly added defects, so it might be the case that you don't see any reports on commit/PR. All reports are accessible in the Security Dashboard under the Security tab in the GitHub UI.

But you can configure it in a way that it will always report all the reports if it is what you want - please see https://github.com/redhat-plumbers-in-action/differential-shellcheck?tab=readme-ov-file#diff-scan

[aidanSoles]

Hi @jamacku! Thanks for the quick reply, and for the correction to my repro. I'll test out what you said above and reply back here soon! ❤️

[aidanSoles]

Hey @jamacku, I did indeed try again with no success. But, after doing some more reading, I'm pretty sure it wasn't working because of the case outlined here, which was exactly what I was doing (running git rebase and force pushing to my branch--which is how I often dev). I'll close this for now and reopen if I'm actually seeing an error. Thanks so much for all your help! ❤️