Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SonarCloud for external PRs #161

Open
LorenzoBettini opened this issue Jul 27, 2021 · 5 comments
Open

SonarCloud for external PRs #161

LorenzoBettini opened this issue Jul 27, 2021 · 5 comments
Labels
for: CI 🚀 Related to Continuous Integration impacts: code quality 💯 Related to the quality of the code priority: high ⚡ Must be done quickly

Comments

@LorenzoBettini
Copy link
Collaborator

@echebbi a well-known limitation of using SonarCloud from GitHub Actions is that secrets are NOT available for external PRs.
This means that we are not able to scan PRs with SonarCloud (unless we merge them, which might be too late, or recreate a clone PR on this very repository with our push admin rights, which is definitely a huge effort).

I saw that other projects simply put the SonarCloud token in clear text in their GitHub Actions workflow... after all, what an attacker would do is only messing up with the SonarCloud results...

shall we try?

I'd feel more comfortable if we analyzed also external PRs :)

@LorenzoBettini LorenzoBettini added priority: high ⚡ Must be done quickly for: CI 🚀 Related to Continuous Integration impacts: code quality 💯 Related to the quality of the code labels Jul 27, 2021
@echebbi
Copy link
Collaborator

echebbi commented Jul 27, 2021

I saw that other projects simply put the SonarCloud token in clear text in their GitHub Actions workflow... after all, what an attacker would do is only messing up with the SonarCloud results...

Well, since SonarCloud reports issues on code attackers could get access to the full codebase, couldn't they? That's not an issue for public repos, but it could be a threat for private ones. And since, AFAIK, Sonar tokens provide full access to the account (not repository-based restriction) hence exposing all repositories of the owner.

shall we try?

That being said my SonarCloud only has public repo and I don't plan to add private ones in the near future so, yeah, sure, I think we can give it a try.

@LorenzoBettini
Copy link
Collaborator Author

OK, let's see first whether I can find an alternative solution (I'm already working on that) and we really can't find another way we'll try with the clear text token

@echebbi
Copy link
Collaborator

echebbi commented Aug 7, 2021

Hey @LorenzoBettini did you find any solution? I believe that having two different PR for a single contribution is highly confusing during reviews. If we don't have any alternative I'm willing to share SonarCloud's token: as I said previously I don't think that's a big deal for me right now since all my repos are public.

@LorenzoBettini
Copy link
Collaborator Author

I succeeded in analyzing an external PR with SonarCloud (I created a very small project so it was easy to experiment); I've used a mix of pull_request_target (note that's different from pull_request) and a kind of hack (documented) to get the code of the pull request. Unfortunately, such an analysis does not help at all:

  • it does not annotate the PR anyway
  • it does not link the issues (and code coverage) to the code (since it comes from an external GitHub repository)

So currently there's no way to do that... creating an additional internal PR is not too much work in the end (and the two PRs seem to stay connected: if I build our PR it also shows the builds on the original external PR; if I merge the original external PR also our PR is considered as merged).

What I did, in general, was to add as a remote Git repository the repository of the contributor; this also allowed me to have the code on my computer (that's how I got to know about the problems on KDE)

Maybe we could go on like that some more time?

@echebbi
Copy link
Collaborator

echebbi commented Aug 10, 2021

and the two PRs seem to stay connected: if I build our PR it also shows the builds on the original external PR; if I merge the original external PR also our PR is considered as merged

That's good to know.

Maybe we could go on like that some more time?

Understood.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
for: CI 🚀 Related to Continuous Integration impacts: code quality 💯 Related to the quality of the code priority: high ⚡ Must be done quickly
Projects
None yet
Development

No branches or pull requests

2 participants