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

Update sonar.yml permissions #166

Merged
merged 2 commits into from
Dec 10, 2023
Merged

Update sonar.yml permissions #166

merged 2 commits into from
Dec 10, 2023

Conversation

joycebrum
Copy link
Contributor

@joycebrum joycebrum commented Jun 15, 2023

I've noticed that the sonar.yml workflow was not with top level permission defined.

Looking into https://github.com/SonarSource/sonarcloud-github-c-cpp it was not clear which permissions should be granted and, considering that a SONAR_TOKEN is required I wasn't able to test. I'll wait for the CI to run here on the PR to see if it will be enough.

Related to #145 and #146

Signed-off-by: Joyce <joycebrum@google.com>
@joycebrum
Copy link
Contributor Author

Just a FYI: I believe that on pull request trigger does not have access to secrets due to security reasons (it is possible to explore a pull_request trigger or a pull_request_target with checkout to expose secrets)

See Controlling changes from forks to workflows in public repositories

Although workflows from forks do not have access to sensitive data such as secrets [...]

So the sonar.yml won't run with success on forked pull requests because the secret (SONAR_TOKEN) won't be available.

Signed-off-by: Joyce <joycebrum@google.com>
@joycebrum
Copy link
Contributor Author

Hi! Friendly ping here! Let me know if you need my help on addressing any changes on this PR 😄

@manugarg
Copy link
Owner

Read-only should be good enough here. I'll merge this and see if something breaks.

Also, I'll limit when sonar workflow runs.

@manugarg manugarg merged commit bc2afe9 into manugarg:master Dec 10, 2023
19 of 20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants