| Message ID | pull.1588.v2.git.1695642662.gitgitgadget@gmail.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Add a GitHub workflow to submit builds to Coverity Scan | expand |
On Mon, Sep 25, 2023 at 11:50:56AM +0000, Johannes Schindelin via GitGitGadget wrote: > Changes since v1: > > * After verifying that cURL's --fail option does what we need in Coverity's > context, I switched to using that in every curl invocation. > * Adjusted quoting (the ${{ ... }} constructs do not require double quotes > because they are interpolated before the script is run). > * Touched up a few commit messages, based on the feedback I received. > * Addressed an actionlint [https://rhysd.github.io/actionlint/] warning. Thanks, this looks fine to me. The only other comment is the same one I made for Taylor's version: that COVERITY_SCAN_EMAIL could probably be a "var" and not a "secret". Though the main advantage there (besides it being a little easier for the user to edit in the web UI) is that it could be used in the jobs.*.if context (to skip the job in unconfigured repos). But since your "if" uses a default-disallow when ENABLE_COVERITY_SCAN_FOR_BRANCHES is not set, it is not that important to check COVERITY_SCAN_EMAIL. -Peff
"Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com> writes: > Coverity [https://scan.coverity.com/] is a powerful static analysis tool > that helps prevent vulnerabilities. It is free to use by open source > projects, and Git benefits from this, as well as Git for Windows. As is the > case with many powerful tools, using Coverity comes with its own set of > challenges, one of which being that submitting a build is quite laborious. > ... One thing that caught my eye was the asterisks around "22" that look as if they were designed to confuse readers and cause them wonder if there are other codes like 122 and 224 that we would also want to catch there. Unless they know what the case statement replaced, that is---the old code to match http_code that was scraped from a text file may not have the code alone and may contain other cruft, so it is entirely understandable, but the new one checks $? and there is no reason other than to catch 122 and 224 to use *22*. > -+ http_code="$(sed -n 1p <"$RUNNER_TEMP"/headers.txt)" > -+ case "$http_code" in > -+ *200*) ;; # okay > -+ *401*) # access denied > -+ echo "::error::incorrect token or project? ($http_code)" >&2 > + --fail \ > + --form token='${{ secrets.COVERITY_SCAN_TOKEN }}' \ > + --form project="$COVERITY_PROJECT" \ > +- --form md5=1) && > ++ --form md5=1) > ++ case $? in > ++ 0) ;; # okay > ++ *22*) # 40x, i.e. access denied > ++ echo "::error::incorrect token or project?" >&2 > + exit 1 > + ;; > + *) # other error > -+ echo "::error::HTTP error $http_code" >&2 > ++ echo "::error::Failed to retrieve MD5" >&2 > + exit 1 > + ;; > + esac Other than that, while I was watching from the sideline, I am very happy to see that you, with Peff's constructive input, came up with a new iteration that looks simpler and more consistent in its use of curl. Will replace but I may be tempted to edit those asterisks out myself while queueing. Thanks.
Hi Junio, On Mon, 25 Sep 2023, Junio C Hamano wrote: > One thing that caught my eye was the asterisks around "22" that look > as if they were designed to confuse readers [...] > > "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com> > writes: > > > [...] > > ++ case $? in > > ++ 0) ;; # okay > > ++ *22*) # 40x, i.e. access denied > > ++ echo "::error::incorrect token or project?" >&2 > > + exit 1 > > + ;; > > [...] > > Will replace but I may be tempted to edit those asterisks out myself > while queueing. D'oh, of course. Thank you, Johannes
Coverity [https://scan.coverity.com/] is a powerful static analysis tool that helps prevent vulnerabilities. It is free to use by open source projects, and Git benefits from this, as well as Git for Windows. As is the case with many powerful tools, using Coverity comes with its own set of challenges, one of which being that submitting a build is quite laborious. The help with this, the Git for Windows project created an Azure Pipeline to automate submitting builds to Coverity Scan [https://dev.azure.com/git-for-windows/git/_build/index?definitionId=35] which is ported to a GitHub workflow in this here patch series, keeping an eye specifically on allowing both the Git and the Git for Windows project to use this workflow. Since Coverity build submissions require authentication, this workflow is skipped by default. To enable it, the repository variable [https://docs.github.com/en/actions/learn-github-actions/variables] ENABLE_COVERITY_SCAN_FOR_BRANCHES needs to be added. Its value needs to be a JSON string array containing the branch names, e.g. ["master", "next"]. Further, two repository secrets [https://docs.github.com/en/actions/security-guides/using-secrets-in-github-actions] need to be set: COVERITY_SCAN_EMAIL and COVERITY_SCAN_TOKEN. An example run in the Git for Windows project can be admired here [https://github.com/git-for-windows/git/actions/runs/6298399881]. Note: While inspired by vapier/coverity-scan-action [https://github.com/vapier/coverity-scan-action], I found too many limitations in that Action to be used here. However, I would be willing to fork that Action into the git organization on GitHub, improve the code to accommodate Git's needs, and maintain that Action, if there is enough support for taking that route instead of simply hard-coding the steps in Git's .github/workflows/coverity.yml. This patch series is based on v2.42.0, but would apply literally everywhere because it adds a new file and modifies no existing one. Changes since v1: * After verifying that cURL's --fail option does what we need in Coverity's context, I switched to using that in every curl invocation. * Adjusted quoting (the ${{ ... }} constructs do not require double quotes because they are interpolated before the script is run). * Touched up a few commit messages, based on the feedback I received. * Addressed an actionlint [https://rhysd.github.io/actionlint/] warning. Johannes Schindelin (6): ci: add a GitHub workflow to submit Coverity scans coverity: cache the Coverity Build Tool coverity: allow overriding the Coverity project coverity: support building on Windows coverity: allow running on macOS coverity: detect and report when the token or project is incorrect .github/workflows/coverity.yml | 163 +++++++++++++++++++++++++++++++++ 1 file changed, 163 insertions(+) create mode 100644 .github/workflows/coverity.yml base-commit: 43c8a30d150ecede9709c1f2527c8fba92c65f40 Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1588%2Fdscho%2Fcoverity-workflow-v2 Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1588/dscho/coverity-workflow-v2 Pull-Request: https://github.com/gitgitgadget/git/pull/1588 Range-diff vs v1: 1: 8cb92968c5e ! 1: 46fb6b583d3 ci: add a GitHub workflow to submit Coverity scans @@ .github/workflows/coverity.yml (new) + - name: download the Coverity Build Tool (${{ env.COVERITY_LANGUAGE }} / ${{ env.COVERITY_PLATFORM}}) + run: | + curl https://scan.coverity.com/download/$COVERITY_LANGUAGE/$COVERITY_PLATFORM \ -+ --no-progress-meter \ ++ --fail --no-progress-meter \ + --output $RUNNER_TEMP/cov-analysis.tgz \ -+ --data "token=${{ secrets.COVERITY_SCAN_TOKEN }}&project=$COVERITY_PROJECT" ++ --form token='${{ secrets.COVERITY_SCAN_TOKEN }}' \ ++ --form project="$COVERITY_PROJECT" + - name: extract the Coverity Build Tool + run: | + mkdir $RUNNER_TEMP/cov-analysis && @@ .github/workflows/coverity.yml (new) + - name: submit the build to Coverity Scan + run: | + curl \ -+ --form token="${{ secrets.COVERITY_SCAN_TOKEN }}" \ -+ --form email="${{ secrets.COVERITY_SCAN_EMAIL }}" \ ++ --fail \ ++ --form token='${{ secrets.COVERITY_SCAN_TOKEN }}' \ ++ --form email='${{ secrets.COVERITY_SCAN_EMAIL }}' \ + --form file=@cov-int.tgz \ -+ --form version="${{ github.sha }}" \ ++ --form version='${{ github.sha }}' \ + "https://scan.coverity.com/builds?project=$COVERITY_PROJECT" 2: 8420a76eba3 ! 2: e26545b1ed9 coverity: cache the Coverity Build Tool @@ .github/workflows/coverity.yml: jobs: + id: lookup + run: | + MD5=$(curl https://scan.coverity.com/download/$COVERITY_LANGUAGE/$COVERITY_PLATFORM \ -+ --data "token=${{ secrets.COVERITY_SCAN_TOKEN }}&project=$COVERITY_PROJECT&md5=1") ++ --fail \ ++ --form token='${{ secrets.COVERITY_SCAN_TOKEN }}' \ ++ --form project="$COVERITY_PROJECT" \ ++ --form md5=1) && + echo "hash=$MD5" >>$GITHUB_OUTPUT + + # Try to cache the tool to avoid downloading 1GB+ on every run. @@ .github/workflows/coverity.yml: jobs: + if: steps.cache.outputs.cache-hit != 'true' run: | curl https://scan.coverity.com/download/$COVERITY_LANGUAGE/$COVERITY_PLATFORM \ - --no-progress-meter \ - --output $RUNNER_TEMP/cov-analysis.tgz \ - --data "token=${{ secrets.COVERITY_SCAN_TOKEN }}&project=$COVERITY_PROJECT" + --fail --no-progress-meter \ +@@ .github/workflows/coverity.yml: jobs: + --form token='${{ secrets.COVERITY_SCAN_TOKEN }}' \ + --form project="$COVERITY_PROJECT" - name: extract the Coverity Build Tool + if: steps.cache.outputs.cache-hit != 'true' run: | 3: 6c1c8286281 = 3: ea85e351233 coverity: allow overriding the Coverity project 4: 14cdefff082 ! 4: 84e1c3eede8 coverity: support building on Windows @@ Commit message value, say, `["windows-latest"]`, this GitHub workflow now runs on Windows, allowing to analyze Windows-specific issues. + This allows, say, the Git for Windows fork to submit Windows builds to + Coverity Scan instead of Linux builds. + Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> ## .github/workflows/coverity.yml ## @@ .github/workflows/coverity.yml: name: Coverity COVERITY_PROJECT: ${{ vars.COVERITY_PROJECT || 'git' }} COVERITY_LANGUAGE: cxx - COVERITY_PLATFORM: linux64 ++ COVERITY_PLATFORM: overridden-below steps: - uses: actions/checkout@v3 + - name: install minimal Git for Windows SDK @@ .github/workflows/coverity.yml: name: Coverity + echo "COVERITY_PLATFORM=$COVERITY_PLATFORM" >>$GITHUB_ENV + echo "COVERITY_TOOL_FILENAME=$COVERITY_TOOL_FILENAME" >>$GITHUB_ENV MD5=$(curl https://scan.coverity.com/download/$COVERITY_LANGUAGE/$COVERITY_PLATFORM \ - --data "token=${{ secrets.COVERITY_SCAN_TOKEN }}&project=$COVERITY_PROJECT&md5=1") - echo "hash=$MD5" >>$GITHUB_OUTPUT + --fail \ + --form token='${{ secrets.COVERITY_SCAN_TOKEN }}' \ @@ .github/workflows/coverity.yml: jobs: run: | curl https://scan.coverity.com/download/$COVERITY_LANGUAGE/$COVERITY_PLATFORM \ - --no-progress-meter \ + --fail --no-progress-meter \ - --output $RUNNER_TEMP/cov-analysis.tgz \ + --output $RUNNER_TEMP/$COVERITY_TOOL_FILENAME \ - --data "token=${{ secrets.COVERITY_SCAN_TOKEN }}&project=$COVERITY_PROJECT" + --form token='${{ secrets.COVERITY_SCAN_TOKEN }}' \ + --form project="$COVERITY_PROJECT" - name: extract the Coverity Build Tool if: steps.cache.outputs.cache-hit != 'true' run: | 5: 782cf2b4403 ! 5: 3d24b6f3b22 coverity: allow running on macOS @@ .github/workflows/coverity.yml: jobs: echo "COVERITY_TOOL_FILENAME=$COVERITY_TOOL_FILENAME" >>$GITHUB_ENV + echo "MAKEFLAGS=$MAKEFLAGS" >>$GITHUB_ENV MD5=$(curl https://scan.coverity.com/download/$COVERITY_LANGUAGE/$COVERITY_PLATFORM \ - --data "token=${{ secrets.COVERITY_SCAN_TOKEN }}&project=$COVERITY_PROJECT&md5=1") - echo "hash=$MD5" >>$GITHUB_OUTPUT + --fail \ + --form token='${{ secrets.COVERITY_SCAN_TOKEN }}' \ @@ .github/workflows/coverity.yml: jobs: mkdir $RUNNER_TEMP/cov-analysis && tar -xzf $RUNNER_TEMP/$COVERITY_TOOL_FILENAME --strip 1 -C $RUNNER_TEMP/cov-analysis 6: 458bc2ea91f ! 6: b45cc4b8a25 coverity: detect and report when the token or project is incorrect @@ Commit message either an incorrect token, or even more likely due to an incorrect Coverity project name. - Let's detect that scenario and provide a helpful error message instead - of trying to go forward with an empty string instead of the correct MD5. + Seeing an authorization failure that is caused by an incorrect project + name was somewhat surprising to me when developing the Coverity + workflow, as I found such a failure suggestive of an incorrect token + instead. + + So let's provide a helpful error message about that specifically when + encountering authentication issues. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> ## .github/workflows/coverity.yml ## @@ .github/workflows/coverity.yml: jobs: - echo "COVERITY_TOOL_FILENAME=$COVERITY_TOOL_FILENAME" >>$GITHUB_ENV - echo "MAKEFLAGS=$MAKEFLAGS" >>$GITHUB_ENV - MD5=$(curl https://scan.coverity.com/download/$COVERITY_LANGUAGE/$COVERITY_PLATFORM \ -+ -D "$RUNNER_TEMP"/headers.txt \ - --data "token=${{ secrets.COVERITY_SCAN_TOKEN }}&project=$COVERITY_PROJECT&md5=1") -+ http_code="$(sed -n 1p <"$RUNNER_TEMP"/headers.txt)" -+ case "$http_code" in -+ *200*) ;; # okay -+ *401*) # access denied -+ echo "::error::incorrect token or project? ($http_code)" >&2 + --fail \ + --form token='${{ secrets.COVERITY_SCAN_TOKEN }}' \ + --form project="$COVERITY_PROJECT" \ +- --form md5=1) && ++ --form md5=1) ++ case $? in ++ 0) ;; # okay ++ *22*) # 40x, i.e. access denied ++ echo "::error::incorrect token or project?" >&2 + exit 1 + ;; + *) # other error -+ echo "::error::HTTP error $http_code" >&2 ++ echo "::error::Failed to retrieve MD5" >&2 + exit 1 + ;; + esac