Check for unsafe action references in GitHub Actions

[roryabraham]

Details

Coming from https://expensify.slack.com/archives/CC7NECV4L/p1743022578963949

This PR has evolved to essentially do a few things:

1. Split the existing .github/scripts/validateActionsAndWorkflows.sh into two scripts for the distinct things it checks:
   • validating schemas in .github/scripts/validateWorkflowSchemas.sh
   • running actionlint in .github/scripts/actionlint.sh
2. Added a new script .github/scripts/validateImmutableActionRefs.sh that performs the check for unsafe action references
3. Calls those three scripts in separate jobs in the existing validateActions.yml workflow.
4. Updates bash style across the repo to conform to our style guide
5. Remove unused code from shellUtils

Related Issues

https://github.com/Expensify/Expensify/issues/484931

Manual Tests

1. Make an action ref mutable.
2. Run the script in this repo with .github/scripts/validateImmutableActionRefs.sh, and verify that it catches the mutable action references.
3. Make all action refs immutable.
4. Run the script with .github/scripts/validateImmutableActionRefs.sh and verify it passes.
5. Run .github/scripts/actionlint.sh and verify it passes.
6. Add some code to an workflow run step that will cause shellcheck to fail, such as:

MY_VAR="$(echo "Hello world")"

8. Run .github/scripts/actionlint.sh and verify that it fails due to the shellcheck error
9. Run .github/scripts/shellCheck.sh and verify that it passes.
10. Add that same problematic bash code to another .sh file
11. Run .github/scripts/shellCheck.sh and verify that it fails as expected.
12. Run .github/scripts/validateWorkflowSchemas.sh and verify that it passes.
13. Make some erroneous change to a workflow yaml file
14. Run .github/scripts/validateWorkflowSchemas.sh and verify that it fails as expected.

I also tested the TRUSTED_ORGS whitelist by temporarily adding ^actions/ to it, and after doing so it correctly did not flag mutable references to actions in the actions/ org. So that's extensible to other non-Expensify orgs if we want to use it.

[roryabraham]

History of this action failing as expected before passing: https://github.com/Expensify/GitHub-Actions/actions/runs/14094365311/job/39478494190?pr=33

[roryabraham]

Whoops, this is working on macos but has a few issues on linux. Will test in the VM and get it fixed.

[rafecolton]

Almost there!

[rafecolton]

Seems my comment disappeared, so replying to your comment:

ok, I see the security concern you've identified here. Can I address it in a follow-up?

No, I think we should address this now. It reintroduces the core problem this change is meant to solve.

Here's a patch for how I think we should do this:

diff --git a/.github/scripts/.gitignore b/.github/scripts/.gitignore
new file mode 100644
index 0000000..3f1633d
--- /dev/null
+++ b/.github/scripts/.gitignore
@@ -0,0 +1,3 @@
+# Ignore actionlint binary and any leftover directories related to installing it
+actionlint*
+!actionlint_checksums.txt
diff --git a/.github/scripts/actionlint.sh b/.github/scripts/actionlint.sh
index c0f6805..b6f5ac9 100755
--- a/.github/scripts/actionlint.sh
+++ b/.github/scripts/actionlint.sh
@@ -3,9 +3,6 @@
 #    Lint workflows with https://github.com/rhysd/actionlint    #
 #################################################################
 
-# v1.7.7
-readonly ACTIONLINT_VERSION=03d0035246f3e81f36aed592ffb4bebf33a03106
-
 # Verify that shellcheck is installed (preinstalled on GitHub Actions runners)
 if ! command -v shellcheck &>/dev/null; then
     error 'This script requires shellcheck. Please install it and try again'
@@ -15,21 +12,97 @@ fi
 SCRIPT_DIR="$(cd -- "$(dirname -- "${BASH_SOURCE[0]}")" &> /dev/null && pwd)"
 readonly SCRIPT_DIR
 
+# To update the actionlint version, replace this file with the updated checksums file from the GitHub release
+readonly CHECKSUMS_FILE="$SCRIPT_DIR/actionlint_checksums.txt"
+readonly ACTIONLINT_PATH="$SCRIPT_DIR/actionlint"
+
 source "$SCRIPT_DIR/shellUtils.sh"
 
 title 'Lint Github Actions via actionlint (https://github.com/rhysd/actionlint)'
 
-info 'Downloading actionlint...'
-if ! bash <(curl --silent https://raw.githubusercontent.com/rhysd/actionlint/"$ACTIONLINT_VERSION"/scripts/download-actionlint.bash); then
-    error 'Error downloading actionlint'
-    exit 1
-fi
+# Get the actionlint tarball name - used both for downloading and verifying checksums. When updating versions
+actionlint_tarball_name() {
+    local OS="$(uname)"
+    local ARCH="$(uname -m)"
+    local OS_ARCH
 
-success 'Successfully downloaded actionlint!'
-echo
+    if [[ "$OS" == "Darwin" ]] && [[ "$ARCH" == "arm64" ]] ; then
+        OS_ARCH="darwin_arm64"
+    elif [[ "$OS" == "Linux" ]] && [[ "$ARCH" == "x86_64" ]] ; then
+        OS_ARCH="linux_amd64"
+    elif [[ "$OS" == "Linux" ]] && [[ "$ARCH" == "aarch64" ]] ; then
+        OS_ARCH="linux_arm64"
+    else
+        error "Unknown architecture, unable to get actionlint tarball name" >&2
+        return
+    fi
+
+    grep "$OS_ARCH" "$CHECKSUMS_FILE" | awk '{print $2}'
+}
+
+actionlint_checksum() {
+    local TARBALL_NAME="$(actionlint_tarball_name)"
+    if [[ -z "$TARBALL_NAME" ]] ; then
+        error "Unable to get actionlint checksum" >&2
+        return
+    fi
+
+    grep "$(actionlint_tarball_name)" "$CHECKSUMS_FILE" | awk '{print $1}'
+}
+
+intended_actionlint_version() {
+    echo "$(actionlint_tarball_name)" | grep -oE 'actionlint_[0-9\.]+_' | awk -F_ '{print $2}'
+}
+
+correct_actionlint_version_installed() {
+    if [[ -x "$ACTIONLINT_PATH" ]] && [[ "$(intended_actionlint_version)" == "$("$ACTIONLINT_PATH" -version | head -n 1)" ]] ; then
+        return 0
+    fi
+
+    return 1
+}
 
-# Cleanup actionlint when we're done
-trap 'rm -rf ./actionlint' EXIT
+download_and_verify_actionlint() {
+    local TARBALL="$SCRIPT_DIR/actionlint.tar.gz"
+    local VERSION="$(intended_actionlint_version)"
+
+    if correct_actionlint_version_installed ; then
+        info "Found actionlint version $VERSION already installed" >&2
+        return 0
+    fi
+
+    info 'Downloading actionlint...' >&2
+    curl -sL "https://github.com/rhysd/actionlint/releases/download/v${VERSION}/$(actionlint_tarball_name)" \
+        -o "$TARBALL"
+
+    # Ensure tarball is cleaned up
+    trap 'rm -f $TARBALL' RETURN
+
+    local EXPECTED_CHECKSUM="$(actionlint_checksum)"
+    local ACTUAL_CHECKSUM="$(sha256sum "$TARBALL" | awk '{print $1}')"
+
+    if [[ "$ACTUAL_CHECKSUM" != "$EXPECTED_CHECKSUM" ]] ; then
+        error "Checksums did not match, expected $EXPECTED_CHECKSUM, got $ACTUAL_CHECKSUM" >&2
+        return 1
+    fi
+
+    local TMPDIR="$SCRIPT_DIR/actionlint-$VERSION"
+    mkdir -p "$TMPDIR"
+
+    # It's only possible to have one return trap, so we have to update to include both items we wish to remove
+    trap 'rm -rf $TMPDIR $TARBALL' RETURN
+    tar -C "$TMPDIR" -xzf "$TARBALL"
+    mv "$TMPDIR/actionlint" "$SCRIPT_DIR/actionlint"
+
+    local INSTALLED_VERSION="$("$SCRIPT_DIR/actionlint" -version | head -n 1)"
+    info "Successfully installed actionlint version $INSTALLED_VERSION" >&2
+    return 0
+}
+
+if ! download_and_verify_actionlint ; then
+    error 'Unable to install actionlint binary'
+    exit 1
+fi
 
 info 'Linting workflows...'
 echo
diff --git a/.github/scripts/actionlint_checksums.txt b/.github/scripts/actionlint_checksums.txt
new file mode 100644
index 0000000..b845ab3
--- /dev/null
+++ b/.github/scripts/actionlint_checksums.txt
@@ -0,0 +1,11 @@
+28e5de5a05fc558474f638323d736d822fff183d2d492f0aecb2b73cc44584f5  actionlint_1.7.7_darwin_amd64.tar.gz
+2693315b9093aeacb4ebd91a993fea54fc215057bf0da2659056b4bc033873db  actionlint_1.7.7_darwin_arm64.tar.gz
+a6ee742126aa632b32009bdd6f820e0274c5b24536dc302a9574118378ee92f4  actionlint_1.7.7_freebsd_386.tar.gz
+f6eec0e5efd17183a954f0d88280885b7a58f39808050cdfcd7f068fc1734bc8  actionlint_1.7.7_freebsd_amd64.tar.gz
+01d4c173f411aeecf670d5219c008e07bf11539cf1181fdeee0cdb0eb8244aac  actionlint_1.7.7_linux_386.tar.gz
+023070a287cd8cccd71515fedc843f1985bf96c436b7effaecce67290e7e0757  actionlint_1.7.7_linux_amd64.tar.gz
+401942f9c24ed71e4fe71b76c7d638f66d8633575c4016efd2977ce7c28317d0  actionlint_1.7.7_linux_arm64.tar.gz
+82e98d7252341b83fa557764824f225fa50431801b8e3d8e99f70dc1efc317b2  actionlint_1.7.7_linux_armv6.tar.gz
+66de2b65bcee17de1866287a513cadf47d812229e976e99024e9757645258adb  actionlint_1.7.7_windows_386.zip
+7f12f1801bca3d480d67aaf7774f4c2a6359a3ca8eebe382c95c10c9704aa731  actionlint_1.7.7_windows_amd64.zip
+76e9514cfac18e5677aa04f3a89873c981f16a2f2353bb97372a86cd09b1f5a8  actionlint_1.7.7_windows_arm64.zip

Evidence of testing:

[roryabraham]

updated

[rafecolton]

Just one NAB, looks good otherwise! Did you re-test after all the changes? If not, please do before merging.

[roryabraham]

Retested everything, merging