diff mbox series

[6/6] coverity: detect and report when the token or project is incorrect

Message ID 458bc2ea91faf88a3e1d21945f12f314d1a7b78e.1695379323.git.gitgitgadget@gmail.com (mailing list archive)
State Superseded
Headers show
Series Add a GitHub workflow to submit builds to Coverity Scan | expand

Commit Message

Johannes Schindelin Sept. 22, 2023, 10:42 a.m. UTC
From: Johannes Schindelin <johannes.schindelin@gmx.de>

When trying to obtain the MD5 of the Coverity Scan Tool (in order to
decide whether a cached version can be used or a new version has to be
downloaded), it is possible to get a 401 (Authorization required) due to
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.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 .github/workflows/coverity.yml | 13 +++++++++++++
 1 file changed, 13 insertions(+)

Comments

Jeff King Sept. 23, 2023, 7:07 a.m. UTC | #1
On Fri, Sep 22, 2023 at 10:42:03AM +0000, Johannes Schindelin via GitGitGadget wrote:

> From: Johannes Schindelin <johannes.schindelin@gmx.de>
> 
> When trying to obtain the MD5 of the Coverity Scan Tool (in order to
> decide whether a cached version can be used or a new version has to be
> downloaded), it is possible to get a 401 (Authorization required) due to
> 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.

Ah. :) I think using "curl --fail" is probably a simpler solution here.

-Peff
Johannes Schindelin Sept. 25, 2023, 11:52 a.m. UTC | #2
Hi Jeff,

On Sat, 23 Sep 2023, Jeff King wrote:

> On Fri, Sep 22, 2023 at 10:42:03AM +0000, Johannes Schindelin via GitGitGadget wrote:
>
> > From: Johannes Schindelin <johannes.schindelin@gmx.de>
> >
> > When trying to obtain the MD5 of the Coverity Scan Tool (in order to
> > decide whether a cached version can be used or a new version has to be
> > downloaded), it is possible to get a 401 (Authorization required) due to
> > 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.
>
> Ah. :) I think using "curl --fail" is probably a simpler solution here.

Apart from the unintuitive error message. I myself was puzzled and
struggled quite a few times until I realized that it was not the token
that was incorrect, but the project name.

To help people with a similar mental capacity to my own, I would therefore
really want to keep this here patch.

As a compromise, I will switch to using `--fail` and then looking at the
exit code (with 22 indicating authentication issues).

Ciao,
Johannes
Jeff King Sept. 25, 2023, 12:17 p.m. UTC | #3
On Mon, Sep 25, 2023 at 01:52:56PM +0200, Johannes Schindelin wrote:

> > > 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.
> >
> > Ah. :) I think using "curl --fail" is probably a simpler solution here.
> 
> Apart from the unintuitive error message. I myself was puzzled and
> struggled quite a few times until I realized that it was not the token
> that was incorrect, but the project name.
> 
> To help people with a similar mental capacity to my own, I would therefore
> really want to keep this here patch.
> 
> As a compromise, I will switch to using `--fail` and then looking at the
> exit code (with 22 indicating authentication issues).

Oh, I do not at all mind de-mystifying the error message for the user. I
mostly meant that using --fail would be better than scraping the http
code from the header file. Though as you note, I guess we have to
interpret the exit code in this case anyway, so it is not that much of a
simplification.

-Peff
diff mbox series

Patch

diff --git a/.github/workflows/coverity.yml b/.github/workflows/coverity.yml
index ca51048ed9d..12cdbaf7ffd 100644
--- a/.github/workflows/coverity.yml
+++ b/.github/workflows/coverity.yml
@@ -76,7 +76,20 @@  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
+            exit 1
+            ;;
+          *) # other error
+            echo "::error::HTTP error $http_code" >&2
+            exit 1
+            ;;
+          esac
           echo "hash=$MD5" >>$GITHUB_OUTPUT
 
       # Try to cache the tool to avoid downloading 1GB+ on every run.