fix(ci): SonarCloud analysis scope and Postgres analysis - #3914
Open
pflynn-virtru wants to merge 2 commits into
Open
fix(ci): SonarCloud analysis scope and Postgres analysis #3914pflynn-virtru wants to merge 2 commits into
pflynn-virtru wants to merge 2 commits into
Conversation
|
Warning Review limit reachedNext included review available in 52 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Repository UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
Benchmark results, click to expandBenchmark authorization.GetDecisions Results:
Benchmark authorization.v2.GetMultiResourceDecision Results:
Benchmark Statistics
Bulk Benchmark Results
TDF3 Benchmark Results:
|
Contributor
Contributor
Benchmark results, click to expandBenchmark authorization.GetDecisions Results:
Benchmark authorization.v2.GetMultiResourceDecision Results:
Benchmark Statistics
Bulk Benchmark Results
TDF3 Benchmark Results:
|
The SonarQube Cloud quality gate fails on main, putting a red X on every
commit. Most of what it is counting is not real.
Every .sql file was being analyzed by the Oracle PL/SQL analyzer, because
sonar.plsql.file.suffixes defaults to "sql,pks,pkb" while
sonar.postgres.file.suffixes defaults to "pgsql,psql" and so matched
nothing we have. Our SQL is PostgreSQL: 26 of 48 migrations use SERIAL,
JSONB, RETURNING, ON CONFLICT or gen_random_uuid, and 8 use $$-quoted
function bodies that Oracle has no concept of. 19 of the 48 migrations
fail to parse under the Oracle grammar, which is why three of them are
reported as DELETE statements missing a WHERE clause when in fact they
rewrite a whole table from a temp table.
Of the 148 plsql findings, 131 are S1192 "define a constant instead of
duplicating this literal", and 96 of all findings sit in
service/policy/db/queries, which are sqlc query definitions where
repeating a column name is unavoidable and constants do not exist.
Point .sql at the analyzer that can parse it.
Also exclude test scaffolding from production-source analysis. test/**
and **/testdata/** hold bats harnesses and fixtures such as a JWT that
expired in 2019 and sample private keys; scanning them as production
source is what produces the recurring "leaked secret" findings we keep
hand-triaging as won't fix.
Excluding coverage.* means the gitignored coverage report is no longer
indexed as a source file, which the scan log shows happening today
("Cannot parse 'coverage.json:2:1'"). That was the only reason SCM was
disabled, so sonar.scm.disabled can be dropped and Sonar can date lines
from git blame again.
Signed-off-by: Paul Flynn <pflynn-virtru@users.noreply.github.com>
We were two majors behind. v8.2.1 runs on node24 (v6 runs node20, which GitHub is deprecating and already warns about in our scan logs) and ships SonarScanner CLI 8.1.0.6389 instead of 7.2.0.5079. v8.0.0 flipped skipSignatureVerification to false, so scanner binaries are now GPG-verified by default. v8.2.1 includes the fix for signature verification failing when the temp directory path is too long. Drop-in: the action has no required inputs and we pass none, only the SONAR_TOKEN and GITHUB_TOKEN env vars. The dependency-review allowlist entry for this action is version-agnostic. Add workflow_dispatch so scans can be run on demand instead of only on the Tue/Thu/Sat cron. Signed-off-by: Paul Flynn <pflynn-virtru@users.noreply.github.com>
pflynn-virtru
force-pushed
the
fix/sonarcloud-quality-gate-config
branch
from
August 24, 2026 15:58
888a063 to
bbc3e7a
Compare
Contributor
pflynn-virtru
added a commit
that referenced
this pull request
Aug 24, 2026
Do not merge this commit. It exists so PR #3914 scans itself and we can confirm the PL/SQL -> PostgreSQL analyzer switch behaves as expected before the config change lands. workflow_dispatch cannot be used for this: GitHub only exposes a workflow for manual dispatch once the trigger is present on the default branch, so on-demand runs are unavailable until this PR merges. Revert before taking the PR out of draft. Signed-off-by: Paul Flynn <pflynn-virtru@users.noreply.github.com>
Contributor
Contributor
Benchmark results, click to expandBenchmark authorization.GetDecisions Results:
Benchmark authorization.v2.GetMultiResourceDecision Results:
Benchmark Statistics
Bulk Benchmark Results
TDF3 Benchmark Results:
|
Contributor
Benchmark results, click to expandBenchmark authorization.GetDecisions Results:
Benchmark authorization.v2.GetMultiResourceDecision Results:
Benchmark Statistics
Bulk Benchmark Results
TDF3 Benchmark Results:
|
Contributor
pflynn-virtru
added a commit
that referenced
this pull request
Aug 24, 2026
…rCloud Do not merge this commit. It exists so PR #3914 scans itself and we can confirm the PL/SQL -> PostgreSQL analyzer switch behaves as expected before the config change lands. The push trigger is scoped to this branch and forces a full branch analysis. A pull_request scan cannot answer the question on its own: its sensors are restricted to changed files, and this PR changes no .sql, so the PostgreSQL analyzer has nothing to look at and reports no findings. workflow_dispatch cannot be used either: GitHub only exposes a workflow for manual dispatch once the trigger is present on the default branch, so on-demand runs are unavailable until this PR merges. Revert before taking the PR out of draft. Signed-off-by: Paul Flynn <pflynn-virtru@users.noreply.github.com>
pflynn-virtru
force-pushed
the
fix/sonarcloud-quality-gate-config
branch
from
August 24, 2026 16:20
0179236 to
d15e9de
Compare
Contributor
Benchmark results, click to expandBenchmark authorization.GetDecisions Results:
Benchmark authorization.v2.GetMultiResourceDecision Results:
Benchmark Statistics
Bulk Benchmark Results
TDF3 Benchmark Results:
|
|
pflynn-virtru
force-pushed
the
fix/sonarcloud-quality-gate-config
branch
from
August 24, 2026 16:30
d15e9de to
bbc3e7a
Compare
Contributor
Contributor
Benchmark results, click to expandBenchmark authorization.GetDecisions Results:
Benchmark authorization.v2.GetMultiResourceDecision Results:
Benchmark Statistics
Bulk Benchmark Results
TDF3 Benchmark Results:
|
pflynn-virtru
changed the title
pflynn-virtru
marked this pull request as ready for review
August 24, 2026 16:37
Contributor
dmihalcik-virtru
approved these changes
Aug 24, 2026
pflynn-virtru
enabled auto-merge
August 25, 2026 18:35
Contributor
|
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
2 participants
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.
Proposed Changes
SonarCloud's quality gate has been failing on
main, so every commit gets a red X. The workflow itself is green — it's the gate check that fails. Most of what it was counting wasn't real.Every
.sqlfile was being analyzed as Oracle PL/SQL.sonar.plsql.file.suffixesdefaults tosql,pks,pkb, while the PostgreSQL analyzer's default (pgsql,psql) matched nothing we have. Our SQL is Postgres, and Oracle's parser choked on 19 of our 48 migrations — which is why three were flagged asDELETEmissing aWHEREwhen they actually rewrite a table from a temp table. This points.sqlat the analyzer that can read it.Test fixtures were being scanned as production source. Excluding
test/**and**/testdata/**stops the recurring "leaked secret" findings from bats harnesses, a JWT that expired in 2019, and sample private keys — the ones we keep hand-triaging as won't fix.sonar.scm.disabledis gone. It only existed because the gitignored coverage report was being indexed as source; excludingcoverage.*removes the need. Sonar can date lines from git blame again.Also bumps
sonarqube-scan-actionv6.0.0 → v8.2.1. We were two majors behind, and v6 runs node20, which GitHub is deprecating and already warns about in our logs.Validation
Verified by temporarily enabling
pull_requestand a branch-scopedpushtrigger, then reverting. Both runs succeeded:coverage.jsonindexed as sourceWorth knowing: the PostgreSQL analyzer has 12 rules to PL/SQL's 186, so some of that 148 → 0 is lost breadth. Still the right trade — 131 of the 148 were one noise rule (duplicated string literals, mostly on sqlc query files) and 3 were BLOCKER bugs that were simply wrong. Notably
postgresdre:S2260("Postgres files should not have syntax errors") fired zero times, confirming all 62 files now parse cleanly.Checklist
n/a — CI configuration only, no Go code changed.
Testing Instructions
No Go changed, so
make lint/make testdon't apply. See the two runs linked above.Once merged,
workflow_dispatchmakes on-demand scans possible (GitHub only exposes the trigger once it's on the default branch):🤖 Generated with Claude Code
Summary by CodeRabbit