Skip to content

Harden SonarCloud workflow: scope permissions, fix injection vectors#1057

Open
efiacor wants to merge 3 commits into
kptdev:mainfrom
Nordix:sonarcloud_workflow_hardening
Open

Harden SonarCloud workflow: scope permissions, fix injection vectors#1057
efiacor wants to merge 3 commits into
kptdev:mainfrom
Nordix:sonarcloud_workflow_hardening

Conversation

@efiacor

@efiacor efiacor commented Jun 18, 2026

Copy link
Copy Markdown
Contributor

Description

  • What changed: Scoped workflow permissions to job level, removed unused checks:write, moved clone_url to env var, added missing -Dproject.settings to push scan
  • Why it's needed: Addresses linter warnings about overly broad permissions and completes the injection-hardening started in the previous commit
  • How it works: Each job now declares only the permissions it actually needs. All expression interpolation in run: blocks is replaced with env vars. Push scan now loads sonar-project.properties for correct source/coverage config.

Related Issue(s)

  • N/A

Type of Change

  • Enhancement

Checklist

  • Code follows project style guidelines
  • Self-reviewed changes
  • Tests added/updated
  • Documentation added/updated
  • All tests and gating checks pass

Testing Instructions (Optional)

  1. Trigger the SonarCloud workflow via a PR or push to verify both scan paths still work

AI Disclosure

  • I have used AI in the creation of this PR.

If so, please describe how:

  • Kiro to review the workflow for accuracy and apply fixes (permission scoping, env var hardening, missing settings file).
  • The author has fully verified all code.

@efiacor
Harden SonarCloud workflow: scope permissions, fix injection vectors
1019759 
- Move permissions from workflow level to job level (least privilege)
- Remove unused checks:write permission
- Move clone_url expression to env var (consistent with other hardening)
- Add missing -Dproject.settings to push scan step

Signed-off-by: Fiachra Corcoran <fiachra.corcoran@est.tech>
@efiacor efiacor requested review from a team June 18, 2026 13:06
@efiacor efiacor added the enhancement New feature or request label Jun 18, 2026
Copilot AI review requested due to automatic review settings June 18, 2026 13:06
@efiacor efiacor self-assigned this Jun 18, 2026
@efiacor efiacor added the enhancement New feature or request label Jun 18, 2026
@netlify

netlify Bot commented Jun 18, 2026 •
edited
Loading

Copy link
Copy Markdown

✅ Deploy Preview for kpt-porch ready!

Name Link
🔨 Latest commit 19360f9
🔍 Latest deploy log https://app.netlify.com/projects/kpt-porch/deploys/6a34f16d77148b0008303775
😎 Deploy Preview https://deploy-preview-1057--kpt-porch.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
🤖 Make changes Run an agent on this branch

To edit notification comments on pull requests, go to your Netlify project configuration.

@dosubot dosubot Bot added the size:XS This PR changes 0-9 lines, ignoring generated files. label Jun 18, 2026
Copilot AI reviewed Jun 18, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR hardens the SonarCloud/SonarQube GitHub Actions workflow by scoping GITHUB_TOKEN permissions per job and reducing injection risk in shell steps, while ensuring the scan uses the repository’s sonar-project.properties.

Changes:

  • Scoped workflow token permissions at the job level.
  • Switched checkout to use head_sha and moved dynamic values used in run: blocks into environment variables.
  • Ensured the push scan loads sonar-project.properties via -Dproject.settings.

Comment thread .github/workflows/sonarcloud.yml
Comment thread .github/workflows/sonarcloud.yml Outdated
@efiacor
Potential fix for pull request finding
46a5518 
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Signed-off-by: Fiachra Corcoran <fiachra.corcoran@est.tech>
Copilot AI review requested due to automatic review settings June 18, 2026 15:10
Copilot AI reviewed Jun 18, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 7 comments.

Comment thread .github/workflows/sonarcloud.yml Outdated
Comment thread .github/workflows/sonarcloud.yml Outdated
Comment thread .github/workflows/sonarcloud.yml Outdated
Comment thread .github/workflows/sonarcloud.yml Outdated
Comment thread .github/workflows/sonarcloud.yml Outdated
Comment thread .github/workflows/sonarcloud.yml Outdated
Comment thread .github/workflows/sonarcloud.yml Outdated
@efiacor
Pin actions to SHA, add actions:read to sonarqube job, remove debug step
19360f9 
Address Copilot review comments:
- Pin all third-party actions to commit SHAs (latest releases)
- Add actions:read permission to sonarqube job for artifact downloads
- Remove debug echo event step (leaks PR metadata to logs)

Signed-off-by: Fiachra Corcoran <fiachra.corcoran@est.tech>
@sonarqubecloud

sonarqubecloud Bot commented Jun 19, 2026

Copy link
Copy Markdown

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

@efiacor efiacor mentioned this pull request Jun 19, 2026
Merged
7 tasks
@dosubot dosubot Bot added the lgtm #ededed label Jun 22, 2026
mozesl-nokia
mozesl-nokia reviewed Jun 22, 2026
View reviewed changes
Comment thread .github/workflows/sonarcloud.yml

@mozesl-nokia mozesl-nokia Jun 22, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why use commit hashes for all the actions instead of just the versions?

@efiacor efiacor Jun 22, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know it's not ideal but it's part of the overall security compliance. Makes it difficult to read for humans but means the version is pinned for security. I added this to hopefully automate the management of them. It will start to complain about our Docker tags soon too.
See - https://github.com/kptdev/porch/security/code-scanning?query=is%3Aopen+branch%3Amain+severity%3Amedium++

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

@mozesl-nokia mozesl-nokia mozesl-nokia left review comments

@liamfallon liamfallon liamfallon approved these changes

@kispaljr kispaljr Awaiting requested review from kispaljr kispaljr is a code owner

At least 2 approving reviews are required to merge this pull request.

Assignees

@efiacor efiacor

Labels

enhancement New feature or request lgtm #ededed size:XS This PR changes 0-9 lines, ignoring generated files.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@efiacor @liamfallon @mozesl-nokia