BlackArch / blackarch

An ArchLinux based distribution for penetration testers and security researchers.
https://www.blackarch.org
Other
2.83k stars 571 forks source link

[BUG] CI GH Actions not working properly #4301

Open D3vil0p3r opened 1 month ago

D3vil0p3r commented 1 month ago

Bug description

CI GitHub Actions workflow seems to not work properly.

When you inject a syntax error/warning or a build error in a PR, it is not able to propagate the exit status != 0 to the output and the workflow will end erroneously as success.

CI build logs CI pkgcheck logs

Analysis

Let's suppose our commit has changed two files:

pkgcheck job:

      - name: Check Styling
        run: |
           COMMIT_RANGE=($(git diff --name-only --diff-filter=d $(git log -2 --format=%H | tr '\n' ' ')))
           CHANGED_FILES=($(git diff --name-only $COMMIT_RANGE))
           for FILE in $CHANGED_FILES; do
              if [[ $(basename $FILE) == "PKGBUILD" ]]; then
                pkgcheck $FILE
              fi
           done

In particular:

COMMIT_RANGE=($(git diff --name-only --diff-filter=d $(git log -2 --format=%H | tr '\n' ' ')))

returns filenames (as an array) and not the commit range as stated by its variable name. The commit range is returned only by git log -2 --format=%H | tr '\n' ' ' command. Probably CHANGED_FILES variable will be always empty, so, useless. For loop, looping on an empty variable, never gets in and the workflow will exit always with SUCCESS status.

$COMMIT_RANGE is an array, not a scalar variable, so for FILE in $CHANGED_FILES; do only loops on the first element.

pkgcheck-arch is installed by --user flag by pip. It could install pkgcheck executable on /root/.local/bin (not inside $PATH) instead of /usr/bin, so, by running pkgcheck $FILE will result in a command not found.

build job:

       - name: Build Package
         run: |
           CHANGED_FILES=($(git diff --name-only --diff-filter=d $THE_LAST_COMMIT HEAD))
           for FILE in $CHANGED_FILES; do
              if [[ $(basename $FILE) == "PKGBUILD" ]]; then
                docker build -t builder -f travis/Dockerfile $(dirname $FILE)
                docker run --rm builder
              fi
           done

Here, $THE_LAST_COMMIT does not refer to anything and it does not belong to GH Env variables. Probably it should be replaced by ${{ github.event.pull_request.head.sha }} in order to get the list of files committed in the last commit.

docker build needs --load argument otherwise docker run won't work.

Overall consideration

Btw, the approach of two jobs to get the list of files changed in the last commit could not be efficient because, if I open a PR, and I change two files A and B, the CI will run correctly on these two files that are listed on a last commit. If I need to edit the PR by adding/editing a different file C (i.e., I need to add a new dependency) a new commit is created and CI will refer only to the files related to this new latest commit of PR (file C), so, A and B will be ignored by the CI check.

D3vil0p3r commented 1 month ago

Fix https://github.com/BlackArch/blackarch/pull/4303