Scope Sonar analysis to the workspace, narrow workflow permissions - #67
Merged
matt-edmondson merged 2 commits intoAug 5, 2026
Merged
Conversation
…sions Three CI fixes, no change to the tool itself. Build KtsuBuild before the SonarQube window opens. SonarScanner injects its Roslyn analyzers into every project compiled between begin and end, so cloning and building KtsuBuild inside that window made the run report roughly 25 of KtsuBuild's own code smells, S1192, S3776, S107 and friends, as warnings against this repository, and indexed KtsuBuild.CLI and KtsuBuild as modules of this analysis. Those warnings are compiler diagnostics rather than reported issues, so sonar.exclusions could never suppress them, which is why the **/KtsuBuild/** entry never had any effect. The clone now happens before the begin step, a new step compiles KtsuBuild.CLI there, and the pipeline step runs it with --no-build so nothing is recompiled inside the window. Pin sonar.projectBaseDir to the workspace. Scanner for .NET v8 otherwise derives the base directory from the projects it sees, so a project compiled under the runner temp directory widened it to the common ancestor and pulled the runner's own action checkouts into the file scan. That is what failed the quality gate on PR #66, where the reliability rating was driven entirely by a bug in merge.test.ts belonging to actions/checkout. With the base directory pinned, only this repository is ever in scope, so the _actions, _temp and KtsuBuild exclusions are all unnecessary and the Configure SonarQube exclusions step is gone. sonar.exclusions keeps only NativeExports.cs, which is a genuine in-repository exclusion, and is now passed inline like the coverage exclusions already were. Narrow update-sdks.yml permissions. The workflow declared read-all, which SonarCloud flagged as a vulnerability and which drove the security rating. The default is now contents: read. The one job that writes already declares the contents: write and pull-requests: write it needs, so behaviour is unchanged. Every step across these workflows now uses pwsh, replacing the mix of bash, Windows PowerShell and pwsh that was there before. The converted steps run no redirection into GITHUB_ENV or GITHUB_OUTPUT, so the UTF-16 encoding difference between Windows PowerShell and pwsh does not apply to any of them. The tag selection in the clone step sorts by [version] rather than as text, matching what sort -V did, and both were checked to select the same tag. Claude-Session: https://claude.ai/code/session_01Ruh9nAQiuU5VGHV8uJCXtu
…g it in the run block
Expanding ${{ secrets.SONAR_TOKEN }} inside a run block interpolates the secret
into the script text itself before the shell ever sees it, which is what the
"Avoid expanding secrets in a run block" rule objects to.
Both scanner steps already declared SONAR_TOKEN in their step env, so they now
read $env:SONAR_TOKEN instead. The value reaches the scanner identically, it is
just resolved at runtime rather than baked into the rendered script. Verified
that pwsh expands it inside the quoted native command argument and passes
/d:sonar.token=<value> as before.
The remaining ${{ secrets.* }} references are all env declarations, which is the
pattern the rule asks for, and are unchanged.
Claude-Session: https://claude.ai/code/session_01Ruh9nAQiuU5VGHV8uJCXtu
|
matt-edmondson
deleted the
fix/sonar-exclusions-and-workflow-permissions
branch
August 5, 2026 09:46
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.



Summary
Three CI fixes. Nothing in the tool itself changes.
The quality gate on #66 failed on reliability and security ratings that had nothing to do with that PR's code, and the run log was full of warnings belonging to KtsuBuild. Both turned out to have the same root cause.
Build KtsuBuild before the SonarQube window opens
SonarScanner injects its Roslyn analyzers into every project compiled between
beginandend. The workflow cloned and built KtsuBuild inside that window, so roughly 25 of KtsuBuild's own code smells (S1192,S3776,S107and friends) were reported as warnings against this repository, and the scanner indexedKtsuBuild.CLIandKtsuBuildas modules of this analysis:Those are compiler diagnostics rather than reported issues, so
sonar.exclusionscould never suppress them. The pre-existing**/KtsuBuild/**entry had no effect on them at all.The clone now happens before
begin, a new step compilesKtsuBuild.CLIthere, and the pipeline step runs it with--no-buildso nothing is recompiled inside the window.Pin sonar.projectBaseDir to the workspace
Scanner for .NET v8 derives the base directory from the projects it sees. A project compiled under the runner temp directory widened it to the common ancestor of the workspace and that directory, which pulled the runner's own action checkouts under
_actionsinto the file scan. That is what actually failed the gate: the reliability rating was driven entirely by a bug inmerge.test.ts, which belongs toactions/checkout, and four other TypeScript files from the same source contributed code smells.With the base directory pinned, only this repository is ever in scope. That makes the
_actions,_tempandKtsuBuildexclusions all unnecessary, so theConfigure SonarQube exclusionsstep is deleted.sonar.exclusionskeeps onlyNativeExports.cs, a genuine in-repository exclusion, passed inline like the coverage exclusions already were.Narrow update-sdks.yml permissions
The workflow declared
permissions: read-all, which SonarCloud flagged as a vulnerability and which drove the security rating. The default is nowcontents: read. The one job that writes already declares thecontents: writeandpull-requests: writeit needs, and job level permissions override the workflow default, so behaviour is unchanged.Shells
Every step across these workflows now uses
pwsh, replacing a mix ofbash, Windows PowerShell andpwsh. The converted steps do no redirection intoGITHUB_ENVorGITHUB_OUTPUT, so the UTF-16 versus UTF-8 encoding difference between Windows PowerShell andpwshdoes not apply to any of them. The two steps that do write there were alreadypwshand are untouched.Test plan
Verified locally where possible:
dotnet buildthendotnet run --no-buildagainst the realKtsuBuild.CLI, confirming the CLI runs without recompiling[version]rather than as text, sov1.8.15beatsv1.8.9. The bash and pwsh versions were run side by side and both selectedv1.8.15v1.8.15with the CLI presentpwshshell remainsWhat to check on this run
The base directory behaviour only manifests under a real scanner run, so this PR is its own test. In the
End SonarQubestep, look at theIndexing files of modulelines:IconHelperandIconHelper.Testmeans the diagnosis holdsKtsuBuildstill appearing means--no-buildis not taking effect_actionsfiles still being flagged meansprojectBaseDiris not being honoured🤖 Generated with Claude Code
https://claude.ai/code/session_01Ruh9nAQiuU5VGHV8uJCXtu