SonarCloud failing for dependabot PRs

StuAA78 commented 3 years ago

SonarCloud is frequently failing on dependabot PRs with the error Set the SONAR_TOKEN env variable. We can see in the log that SONAR_TOKEN is not being set:

Run sonarsource/sonarcloud-github-action@master
  with:
    projectBaseDir: .
  env:
    ...
    SONAR_TOKEN:

However, if we simply re-run the job then the token is set and SonarCloud passes:

Run sonarsource/sonarcloud-github-action@master
  with:
    projectBaseDir: .
  env:
    ...
    SONAR_TOKEN: ***

Cruikshanks commented 3 years ago

This should be fixed by https://github.com/DEFRA/sroc-charging-module-api/pull/521

Cruikshanks commented 3 years ago

We attempted to Fix dependabot builds but it didn't work.

Here is what we wrote about the fix

We use Dependabot to

[..] keep the packages you use updated to the latest versions.

Back in 2020 we also switched this project plus a number of other Defra ones to use GitHub Actions instead of Travis CI. Travis brought in a new pricing plan that limited the build minutes to 1000 minutes per month which caused builds to start failing across all our projects.

This worked fine until Feb 2021 when GitHub made a change to the permissions Dependabot PRs have. The TL;DR is that workflows triggered by Dependabot would no longer have access to repo secrets.

Our CI workflow needs access to these because they contain the SONAR_TOKEN used to authenticate and send SonarQube's analysis results to SonarCloud. Because this step in the workflow started failing, all Dependabot PRs failed.

We have been able to work around the problem by manually restarting jobs. This has the effect of changing the context such that GitHub sees a recognised owner of the repo and thus grants access to secrets. But it's not ideal.

Both in the docs and a blog post that accompanied the announcement GitHub suggest the ideal solution is to break up your workflow into 2. The first does any actions that are at risk in the locked-down context and then the second does whatever actions that need access to secrets. Our issue is SonarCloud needs access to both the code and the output of running the tests. Artifacts can be transferred between workflows but it does vastly increase the complexity of the CI pipeline. Plus we saw reports of it still not working.

As an alternate, GitHub also suggests watching for the pull_request_target and triggering your CI from that. This runs the workflow with the same permissions Dependabot used to have but also exposes you to malicious pull requests that can access your repo secrets. The way to protect yourself is to include checks that the PR has come from a recognised source. You can do this by checking the github.actor in the workflow.

jobs:
  dependabot:
    runs-on: ubuntu-latest
    if: ${{ github.actor == 'dependabot[bot]' }}
    steps:
      - uses: actions/checkout@v2
        with:
          # Check out the pull request HEAD
          ref: ${{ github.event.pull_request.head.sha }}
          github-token: ${{ secrets.GITHUB_TOKEN }}

Another way is to only trigger the workflow for the pull_request_target event if the activity type is 'labeled'. Only those with write privileges to the repo have the ability to label a PR. So, anonymous PR's will not trigger the workflow to run.

The first solution is more explicit but has issues. If your workflow is also used for other events like pull requests and pushes to main you have to decide to either

duplicate the workflow; have one triggered by safe events without the check and the other with the check
add in a long if condition; for each scenario (unsafe PR from Dependabot, safe PR from a team member, push to main, etc) you have to include it as a condition to ensure the workflow gets run

The second solution is less explicit but avoids these issues, which is why it's the one we have gone for. So, this change updates our current CI workflow to allow Dependabot PR's to pass CI in the future by updating what events trigger it to run.

Unfortunately, it didn't work. We ended up Resetting the Dependabot build fix.

In Fix dependabot builds we attempted to solve a problem we have been facing with running sonar cloud analysis for Dependabot builds. The original PR goes into detail but TL;DR Dependabot is not allowed access to the sonar token needed to authenticate with SonarCloud.

We thought based on our research of GitHub actions and other's solutions that we had cracked it. However, something still isn't right. We can get the CI build to pass and even the SonarCloud analysis to run but we get no update back from SonarCloud. We always have SonarCloud as a required check so this means we are still left with a stuck PR.

After discussing it as a team, until we can properly crack this nut we feel the best solution is to reset our changes and add a new one that skips the SonarCloud analysis if the actor is Dependabot. The PR will still remain stuck but the checks will more accurately reflect what is going on.

So, at this point we have CI passing for Dependabot PR's but skipping sonarcloud analysis. What do you think @StuAA78 ? Can we close this issue now?

tacascer commented 8 months ago

I managed to pass in the SONAR_TOKEN to Dependabot using Dependabot secrets

DEFRA / sroc-service-team

SonarCloud failing for dependabot PRs #90