From 7d5fad10f09523337ed03ef6190b0b640db184dd Mon Sep 17 00:00:00 2001 From: Marko Bevc Date: Tue, 15 Sep 2026 22:22:23 +0100 Subject: [PATCH 1/4] feat(sonar): fail when a pull-request analysis is of another revision SonarQube holds one analysis per pull request, the latest one. Until the scan for the current push is processed, that is the previous push's analysis, and it was being attested against the new commit. When the caller names a revision alongside the pull request, the pull request's analysed commit must now match it. Nothing is checked when no revision is given, so existing callers are unaffected. Refs #1192 --- internal/sonar/sonar.go | 6 ++++ internal/sonar/sonar_test.go | 64 ++++++++++++++++++++++++++++++++++++ 2 files changed, 70 insertions(+) diff --git a/internal/sonar/sonar.go b/internal/sonar/sonar.go index bf1e9ad16..77347102b 100644 --- a/internal/sonar/sonar.go +++ b/internal/sonar/sonar.go @@ -257,6 +257,12 @@ func (sc *SonarConfig) GetSonarResults(logger *log.Logger) (*SonarResults, error if err != nil { return nil, err } + // SonarQube holds one analysis per pull request, the latest. Until the scan + // for the current push is processed that is the previous push's, so a + // revision named alongside the pull request must be the analysed one (#1192). + if sc.pullRequest != "" && sc.revision != "" && sonarResults.Revision != sc.revision { + return nil, fmt.Errorf("analysis for pull request %s of project %s is of revision %s, not %s. \nThe scan for revision %s may still be being processed by SonarQube, try again later.\n Otherwise check the revision is correct", sc.pullRequest, project.Key, sonarResults.Revision, sc.revision, sc.revision) + } } //Get the quality gate status from the qualitygates/project_status API diff --git a/internal/sonar/sonar_test.go b/internal/sonar/sonar_test.go index 785537351..759f7e0e7 100644 --- a/internal/sonar/sonar_test.go +++ b/internal/sonar/sonar_test.go @@ -167,6 +167,13 @@ func (f *fakeSonarProject) handler() http.HandlerFunc { Commit: sonar.PRCommit{SHA: revRevision}, }}, }) + case "/api/ce/task": + // The CE task of the pull request scan, as report-task.txt or + // --sonar-ce-task-url would point at it. + _ = json.NewEncoder(w).Encode(sonar.TaskResponse{Task: sonar.Task{ + TaskID: revTaskID, AnalysisID: revAnalysisKey, Status: "SUCCESS", ComponentKey: revProjectKey, + ComponentName: "customer project", PullRequest: revPullRequest, + }}) case "/api/qualitygates/project_status": _ = json.NewEncoder(w).Encode(sonar.QualityGateResponse{ ProjectStatus: sonar.ProjectStatus{Status: "OK", Conditions: []sonar.Conditions{}}, @@ -379,6 +386,63 @@ func TestGetSonarResults_BranchIgnoredForPullRequest(t *testing.T) { } } +// TestGetSonarResults_PullRequestRevision is the issue #1192 check: a pull request's +// latest analysis is whatever SonarQube holds for the PR, which is the previous +// push's scan until the current one is processed. When the caller names the +// revision alongside the pull request, an analysis of another commit must fail +// rather than be attested against a commit nobody scanned. Without a revision +// nothing is checked, so existing callers are unaffected. Both ways of naming the +// scan funnel through the same lookup, so both are pinned. +func TestGetSonarResults_PullRequestRevision(t *testing.T) { + const otherRevision = "0000000000000000000000000000000000000000" + + cases := []struct { + name string + revision string + wantErr bool + }{ + {name: "matching revision is attested", revision: revRevision}, + {name: "other revision fails", revision: otherRevision, wantErr: true}, + {name: "no revision is not checked", revision: ""}, + } + + for _, c := range cases { + for _, path := range []string{"project key", "CE task"} { + t.Run(path+": "+c.name, func(t *testing.T) { + fake := &fakeSonarProject{} + srv := httptest.NewServer(fake.handler()) + defer srv.Close() + + var sc *sonar.SonarConfig + if path == "CE task" { + sc = sonar.NewSonarConfig("tok", t.TempDir(), srv.URL+"/api/ce/task?id="+revTaskID, "", "", c.revision, revPullRequest, "", 5) + } else { + sc = sonar.NewSonarConfig("tok", t.TempDir(), "", revProjectKey, srv.URL, c.revision, revPullRequest, "", 5) + } + results, err := sc.GetSonarResults(discardLogger()) + + if !c.wantErr { + if err != nil { + t.Fatalf("expected the pull-request scan to be attested, got error: %v", err) + } + if results.Revision != revRevision { + t.Errorf("expected the analysed revision %q in the results, got %q", revRevision, results.Revision) + } + return + } + if err == nil { + t.Fatalf("expected an error: pull request %s was analysed at %s, not %s", revPullRequest, revRevision, otherRevision) + } + for _, want := range []string{revPullRequest, revRevision, otherRevision} { + if !strings.Contains(err.Error(), want) { + t.Errorf("expected %q in the error, got: %v", want, err) + } + } + }) + } + } +} + // TestGetSonarResults_NoMatchingTask_KeepsSuppliedBranch pins the payload when // api/ce/activity holds no task for the analysis. That is reachable in production: // the endpoint needs administrative permission, so an ordinary CI token gets an From d146a19f7cf6a94a752002cc3a562fef6fa413f7 Mon Sep 17 00:00:00 2001 From: Marko Bevc Date: Tue, 15 Sep 2026 22:22:26 +0100 Subject: [PATCH 2/4] feat(attest sonar): accept --sonar-revision with --pull-request The two flags were mutually exclusive, so nothing could ask for the pull request's analysed commit to be checked. They are now accepted together, and the pull request's analysis must then be of the given revision. Only a revision the user set (flag, env var or config) is passed on for a pull-request scan. The flag defaults to the CI commit, which nobody asked to have checked, so existing pull-request attestations are unchanged. Refs #1192 --- cmd/kosli/attestSonar.go | 18 +++++++---- cmd/kosli/attestSonar_test.go | 61 ++++++++++++++++++++++++++++++++--- 2 files changed, 69 insertions(+), 10 deletions(-) diff --git a/cmd/kosli/attestSonar.go b/cmd/kosli/attestSonar.go index 37e308e77..7a0f97f70 100644 --- a/cmd/kosli/attestSonar.go +++ b/cmd/kosli/attestSonar.go @@ -28,6 +28,9 @@ type attestSonarOptions struct { branch string maxWait int payload SonarAttestationPayload + // revisionExplicit is true when --sonar-revision was set by the user rather + // than defaulted from the CI commit. + revisionExplicit bool } const attestSonarShortDesc = `Report a SonarQube attestation to an artifact or a trail in a Kosli flow. ` @@ -188,11 +191,6 @@ func newAttestSonarCmd(out io.Writer) *cobra.Command { return err } - err = MuXRequiredFlags(cmd, []string{"sonar-revision", "pull-request"}, false) - if err != nil { - return err - } - err = MuXRequiredFlags(cmd, []string{"sonar-branch", "pull-request"}, false) if err != nil { return err @@ -209,6 +207,7 @@ func newAttestSonarCmd(out io.Writer) *cobra.Command { RunE: func(cmd *cobra.Command, args []string) error { o.repoURLExplicit = cmd.Flags().Changed("repo-url") o.repoNameExplicit = cmd.Flags().Changed("repository") + o.revisionExplicit = cmd.Flags().Changed("sonar-revision") return o.run(args) }, } @@ -244,7 +243,14 @@ func (o *attestSonarOptions) run(args []string) error { return err } - sc := sonar.NewSonarConfig(o.apiToken, o.workingDir, o.ceTaskURL, o.projectKey, o.serverURL, o.revision, o.pullRequest, o.branch, o.maxWait) + // The flag defaults to the CI commit, so only a revision the user actually + // gave is checked against the pull request's analysed commit (#1192). + revision := o.revision + if o.pullRequest != "" && !o.revisionExplicit { + revision = "" + } + + sc := sonar.NewSonarConfig(o.apiToken, o.workingDir, o.ceTaskURL, o.projectKey, o.serverURL, revision, o.pullRequest, o.branch, o.maxWait) o.payload.SonarResults, err = sc.GetSonarResults(logger) if err != nil { diff --git a/cmd/kosli/attestSonar_test.go b/cmd/kosli/attestSonar_test.go index 1a05440bb..bbad227f0 100644 --- a/cmd/kosli/attestSonar_test.go +++ b/cmd/kosli/attestSonar_test.go @@ -45,6 +45,7 @@ type AttestSonarCommandTestSuite struct { prScannerWorkDir string prKey string prCETaskURL string + prRevision string suite.Suite defaultKosliArguments string } @@ -76,6 +77,7 @@ func (suite *AttestSonarCommandTestSuite) SetupTest() { suite.mainScannerWorkDir, suite.mainCETaskURL, suite.mainRevision = downloadMainScanData(suite.T()) suite.prScannerWorkDir, suite.prKey, suite.prCETaskURL = downloadPRScanData(suite.T()) + suite.prRevision = getPRAnalysisRevision(suite.T(), suite.prKey) } func (suite *AttestSonarQubeCommandTestSuite) SetupTest() { @@ -229,10 +231,9 @@ func (suite *AttestSonarCommandTestSuite) TestAttestSonarCmd() { golden: "Error: open .scannerwork/report-task.txt: no such file or directory. Check your working directory is set correctly. Alternatively provide the project key and either revision or pull-request ID for the scan to attest\n", }, { - wantError: true, - name: "25 can't provide both revision and pull-request", - cmd: fmt.Sprintf("attest sonar --name cli.foo --commit HEAD --origin-url http://www.example.com --sonar-project-key cyber-dojo_differ --sonar-revision xxx --pull-request 5 %s", suite.defaultKosliArguments), - golden: "Error: only one of --sonar-revision, --pull-request is allowed\n", + name: "25 can attest a pull request scan when --sonar-revision is the commit it analysed", + cmd: fmt.Sprintf("attest sonar --name cli.foo --commit HEAD --origin-url http://www.example.com --sonar-project-key cyber-dojo_differ --pull-request %s --sonar-revision %s %s", suite.prKey, suite.prRevision, suite.defaultKosliArguments), + golden: "sonar attestation 'foo' is reported to trail: test-123\n", }, { name: "26 can attest sonar for a pull request scan using --sonar-ce-task-url", @@ -263,6 +264,12 @@ func (suite *AttestSonarCommandTestSuite) TestAttestSonarCmd() { cmd: fmt.Sprintf("attest sonar --name cli.foo --commit HEAD --origin-url http://www.example.com --sonar-project-key cyber-dojo_differ --sonar-branch release/uat --pull-request 5 %s", suite.defaultKosliArguments), golden: "Error: only one of --sonar-branch, --pull-request is allowed\n", }, + { + wantError: true, + name: "31 attesting a pull request scan with --sonar-revision of a commit it did not analyse fails", + cmd: fmt.Sprintf("attest sonar --name cli.foo --commit HEAD --origin-url http://www.example.com --sonar-project-key cyber-dojo_differ --pull-request %s --sonar-revision 0000000000000000000000000000000000000000 %s", suite.prKey, suite.defaultKosliArguments), + golden: fmt.Sprintf("Error: analysis for pull request %s of project cyber-dojo_differ is of revision %s, not 0000000000000000000000000000000000000000. \nThe scan for revision 0000000000000000000000000000000000000000 may still be being processed by SonarQube, try again later.\n Otherwise check the revision is correct\n", suite.prKey, suite.prRevision), + }, } runTestCmd(suite.T(), tests) @@ -435,6 +442,52 @@ func getLatestAnalysisRevision(t *testing.T) string { return result.Analyses[0].Revision } +// getPRAnalysisRevision fetches the commit SonarCloud analysed for the given +// pull request of cyber-dojo_differ. PR analyses are not listed by +// project_analyses/search on SonarCloud, so this uses project_pull_requests/list. +func getPRAnalysisRevision(t *testing.T, prKey string) string { + t.Helper() + + sonarToken := os.Getenv("KOSLI_SONAR_API_TOKEN") + httpClient := &http.Client{} + + req, err := http.NewRequest("GET", + "https://sonarcloud.io/api/project_pull_requests/list?project=cyber-dojo_differ", nil) + if err != nil { + t.Fatalf("failed to create pull requests list request: %v", err) + } + req.Header.Add("Authorization", fmt.Sprintf("Bearer %s", sonarToken)) + + resp, err := httpClient.Do(req) + if err != nil { + t.Fatalf("failed to fetch pull requests from SonarCloud: %v", err) + } + defer func() { _ = resp.Body.Close() }() + + if resp.StatusCode != http.StatusOK { + t.Fatalf("SonarCloud pull requests API returned status %d", resp.StatusCode) + } + + var result struct { + PullRequests []struct { + Key string `json:"key"` + Commit struct { + SHA string `json:"sha"` + } `json:"commit"` + } `json:"pullRequests"` + } + if err := json.NewDecoder(resp.Body).Decode(&result); err != nil { + t.Fatalf("failed to decode pull requests response: %v", err) + } + for _, pr := range result.PullRequests { + if pr.Key == prKey { + return pr.Commit.SHA + } + } + t.Fatalf("pull request %s not found for cyber-dojo_differ on SonarCloud", prKey) + return "" +} + // downloadPRScanData downloads the report-task.txt from the latest // SonarCloud PR scan of cyber-dojo_differ. // Returns the directory containing report-task.txt, the PR key, and the CE task URL. From bba9718cce1d7d5c05e30deaf0324a6739b3d3b7 Mon Sep 17 00:00:00 2001 From: Marko Bevc Date: Tue, 15 Sep 2026 22:22:28 +0100 Subject: [PATCH 3/4] docs(attest sonar): describe --sonar-revision with --pull-request Refs #1192 --- cmd/kosli/attestSonar.go | 40 +++++++++++++++++++++++------------ cmd/kosli/attestSonar_test.go | 5 ++--- cmd/kosli/root.go | 4 ++-- internal/sonar/sonar.go | 5 ++--- internal/sonar/sonar_test.go | 13 ++++-------- 5 files changed, 36 insertions(+), 31 deletions(-) diff --git a/cmd/kosli/attestSonar.go b/cmd/kosli/attestSonar.go index 7a0f97f70..ed38d7b85 100644 --- a/cmd/kosli/attestSonar.go +++ b/cmd/kosli/attestSonar.go @@ -18,18 +18,16 @@ type SonarAttestationPayload struct { type attestSonarOptions struct { *CommonAttestationOptions - apiToken string - workingDir string - ceTaskURL string - projectKey string - serverURL string - revision string - pullRequest string - branch string - maxWait int - payload SonarAttestationPayload - // revisionExplicit is true when --sonar-revision was set by the user rather - // than defaulted from the CI commit. + apiToken string + workingDir string + ceTaskURL string + projectKey string + serverURL string + revision string + pullRequest string + branch string + maxWait int + payload SonarAttestationPayload revisionExplicit bool } @@ -55,6 +53,8 @@ or have overridden the revision in SonarQube via parameters to the Sonar scanner If the scan ran on a branch other than the project's main branch in SonarQube, also provide the branch name using the ^--sonar-branch^ flag. SonarQube only searches the project's main branch unless told otherwise, so without this flag the scan cannot be found. For pull request scans: provide the pull-request ID using the ^--pull-request^ flag instead of the revision. +SonarQube keeps only the latest analysis of a pull request, so if the scan for the current push has not finished, that is the previous push's analysis. +To make sure the attested analysis is of the commit you expect, also pass ^--sonar-revision^: the command then fails if the pull request's analysis is of another commit. Kosli then finds the scan results for the specified project key and revision or pull-request ID. 3. Providing the CE task URL directly via ^--sonar-ce-task-url^. The CE task URL can be found in the ^report-task.txt^ file @@ -135,6 +135,18 @@ kosli attest sonar \ --api-token yourAPIToken \ --org yourOrgName \ +# report a SonarQube Cloud attestation about a trail for a pull request scan, failing unless the analysis is of the given commit: +kosli attest sonar \ + --name yourAttestationName \ + --flow yourFlowName \ + --trail yourTrailName \ + --sonar-api-token yourSonarAPIToken \ + --sonar-project-key yourSonarProjectKey \ + --pull-request yourPullRequestID \ + --sonar-revision yourSonarRevision \ + --api-token yourAPIToken \ + --org yourOrgName \ + # report a SonarQube Cloud attestation about a trail with an attachment using SonarQube's metadata, waiting for up to 300 seconds for the results to be available: kosli attest sonar \ --name yourAttestationName \ @@ -243,8 +255,8 @@ func (o *attestSonarOptions) run(args []string) error { return err } - // The flag defaults to the CI commit, so only a revision the user actually - // gave is checked against the pull request's analysed commit (#1192). + // --sonar-revision defaults to the CI commit; only one the user gave is + // checked against the pull request's analysed commit (#1192). revision := o.revision if o.pullRequest != "" && !o.revisionExplicit { revision = "" diff --git a/cmd/kosli/attestSonar_test.go b/cmd/kosli/attestSonar_test.go index bbad227f0..9ed59d624 100644 --- a/cmd/kosli/attestSonar_test.go +++ b/cmd/kosli/attestSonar_test.go @@ -442,9 +442,8 @@ func getLatestAnalysisRevision(t *testing.T) string { return result.Analyses[0].Revision } -// getPRAnalysisRevision fetches the commit SonarCloud analysed for the given -// pull request of cyber-dojo_differ. PR analyses are not listed by -// project_analyses/search on SonarCloud, so this uses project_pull_requests/list. +// getPRAnalysisRevision fetches the commit SonarCloud analysed for the given pull +// request of cyber-dojo_differ. project_analyses/search does not list PR analyses. func getPRAnalysisRevision(t *testing.T, prKey string) string { t.Helper() diff --git a/cmd/kosli/root.go b/cmd/kosli/root.go index 18b17bd3a..c87ff03c2 100644 --- a/cmd/kosli/root.go +++ b/cmd/kosli/root.go @@ -315,8 +315,8 @@ Paths the list already matches stay excluded whatever is later added there, so k sonarWorkingDirFlag = "[conditional] The base directory of the repo scanned by SonarQube. Only required if you have overridden the default in the Sonar scanner or you are running the CLI locally in a separate folder from the repo." sonarProjectKeyFlag = "[conditional] The project key of the SonarQube project. Only required if you want to use the project key/revision/pull-request to get the scan results rather than using Sonar's metadata file." sonarServerURLFlag = "[conditional] The URL of your SonarQube server. Only required if you are using SonarQube Server and not using SonarQube's metadata file to get scan results." - sonarRevisionFlag = "[conditional] The revision of the SonarQube project. Only required if you want to use the project key/revision to get the scan results rather than using Sonar's metadata file and you have overridden the default revision, or you aren't using a CI. Defaults to the value of the git commit flag. Cannot be used with --pull-request." - sonarPRFlag = "[conditional] The ID of the pull-request. Only required if you want to use the project key/pull-request to get the scan results rather than using Sonar's metadata file. Cannot be used with --sonar-revision or --sonar-branch." + sonarRevisionFlag = "[conditional] The revision of the SonarQube project. Only required if you want to use the project key/revision to get the scan results rather than using Sonar's metadata file and you have overridden the default revision, or you aren't using a CI. Defaults to the value of the git commit flag. With --pull-request, optional: when given, the pull request's analysis must be of this revision or the command fails." + sonarPRFlag = "[conditional] The ID of the pull-request. Only required if you want to use the project key/pull-request to get the scan results rather than using Sonar's metadata file. Cannot be used with --sonar-branch." sonarBranchFlag = "[conditional] The name of the branch the SonarQube scan ran on. Only required if you are using the project key/revision to get the scan results and the scan ran on a branch other than the project's main branch in SonarQube. Cannot be used with --pull-request." sonarMaxWaitFlag = "[optional] Allow the command to wait and retry fetching the scan results from SonarQube, up to the maximum number of seconds provided, with exponential backoff. Useful when using SonarQube's metadata file to retrieve and attest scans that take a long time to process . Defaults to 30 seconds." sonarCETaskURLFlag = "[conditional] The URL of the SonarQube CE task. Can be used instead of --sonar-working-dir when the report-task.txt file is not accessible, e.g. due to container isolation in CI/CD pipelines." diff --git a/internal/sonar/sonar.go b/internal/sonar/sonar.go index 77347102b..a1bb726f5 100644 --- a/internal/sonar/sonar.go +++ b/internal/sonar/sonar.go @@ -257,9 +257,8 @@ func (sc *SonarConfig) GetSonarResults(logger *log.Logger) (*SonarResults, error if err != nil { return nil, err } - // SonarQube holds one analysis per pull request, the latest. Until the scan - // for the current push is processed that is the previous push's, so a - // revision named alongside the pull request must be the analysed one (#1192). + // SonarQube keeps only a pull request's latest analysis, which is the + // previous push's until the current scan is processed (#1192). if sc.pullRequest != "" && sc.revision != "" && sonarResults.Revision != sc.revision { return nil, fmt.Errorf("analysis for pull request %s of project %s is of revision %s, not %s. \nThe scan for revision %s may still be being processed by SonarQube, try again later.\n Otherwise check the revision is correct", sc.pullRequest, project.Key, sonarResults.Revision, sc.revision, sc.revision) } diff --git a/internal/sonar/sonar_test.go b/internal/sonar/sonar_test.go index 759f7e0e7..43478b797 100644 --- a/internal/sonar/sonar_test.go +++ b/internal/sonar/sonar_test.go @@ -168,8 +168,6 @@ func (f *fakeSonarProject) handler() http.HandlerFunc { }}, }) case "/api/ce/task": - // The CE task of the pull request scan, as report-task.txt or - // --sonar-ce-task-url would point at it. _ = json.NewEncoder(w).Encode(sonar.TaskResponse{Task: sonar.Task{ TaskID: revTaskID, AnalysisID: revAnalysisKey, Status: "SUCCESS", ComponentKey: revProjectKey, ComponentName: "customer project", PullRequest: revPullRequest, @@ -386,13 +384,10 @@ func TestGetSonarResults_BranchIgnoredForPullRequest(t *testing.T) { } } -// TestGetSonarResults_PullRequestRevision is the issue #1192 check: a pull request's -// latest analysis is whatever SonarQube holds for the PR, which is the previous -// push's scan until the current one is processed. When the caller names the -// revision alongside the pull request, an analysis of another commit must fail -// rather than be attested against a commit nobody scanned. Without a revision -// nothing is checked, so existing callers are unaffected. Both ways of naming the -// scan funnel through the same lookup, so both are pinned. +// TestGetSonarResults_PullRequestRevision is the #1192 check: SonarQube keeps only a +// pull request's latest analysis, so a revision named alongside the pull request +// must be the analysed commit, and no revision means no check. Both ways of +// naming the scan reach the same lookup, so both paths are pinned. func TestGetSonarResults_PullRequestRevision(t *testing.T) { const otherRevision = "0000000000000000000000000000000000000000" From 1fc3bc48fb5569f9e24cc9fe9d5f8130c5a8e93b Mon Sep 17 00:00:00 2001 From: Marko Bevc Date: Tue, 15 Sep 2026 22:22:29 +0100 Subject: [PATCH 4/4] fix(sonar): address review feedback on the pull-request revision check - Note in the comment that only a revision passed alongside --pull-request is checked; on the report-task.txt/--sonar-ce-task-url paths without that flag, sc.revision is the defaulted CI commit, not an expectation to verify. - Pin that gap with a dedicated test: when the pull request is discovered from the CE task response rather than --pull-request, the check does not fire. - Fix the error message's stray trailing space and one-space indent, and drop the redundant third interpolation of the expected revision. - Rename test case 25 to not overstate what it checks: getPRAnalysisRevision reads the expected SHA from the same endpoint the production code reads the actual SHA from, so it is a smoke test for the flag combination, not an independent assertion on the revision. --- cmd/kosli/attestSonar.go | 6 ++++-- cmd/kosli/attestSonar_test.go | 4 ++-- internal/sonar/sonar.go | 6 ++++-- internal/sonar/sonar_test.go | 28 ++++++++++++++++++++++++++-- 4 files changed, 36 insertions(+), 8 deletions(-) diff --git a/cmd/kosli/attestSonar.go b/cmd/kosli/attestSonar.go index ed38d7b85..4fd2572cc 100644 --- a/cmd/kosli/attestSonar.go +++ b/cmd/kosli/attestSonar.go @@ -255,8 +255,10 @@ func (o *attestSonarOptions) run(args []string) error { return err } - // --sonar-revision defaults to the CI commit; only one the user gave is - // checked against the pull request's analysed commit (#1192). + // --sonar-revision defaults to the CI commit, so only a revision the user + // gave explicitly is checked against the pull request's analysed commit + // (#1192). The non-PR path still needs the default: it looks the analysis + // up by revision. revision := o.revision if o.pullRequest != "" && !o.revisionExplicit { revision = "" diff --git a/cmd/kosli/attestSonar_test.go b/cmd/kosli/attestSonar_test.go index 9ed59d624..ed8f3aef3 100644 --- a/cmd/kosli/attestSonar_test.go +++ b/cmd/kosli/attestSonar_test.go @@ -231,7 +231,7 @@ func (suite *AttestSonarCommandTestSuite) TestAttestSonarCmd() { golden: "Error: open .scannerwork/report-task.txt: no such file or directory. Check your working directory is set correctly. Alternatively provide the project key and either revision or pull-request ID for the scan to attest\n", }, { - name: "25 can attest a pull request scan when --sonar-revision is the commit it analysed", + name: "25 can attest a pull request scan alongside --sonar-revision", cmd: fmt.Sprintf("attest sonar --name cli.foo --commit HEAD --origin-url http://www.example.com --sonar-project-key cyber-dojo_differ --pull-request %s --sonar-revision %s %s", suite.prKey, suite.prRevision, suite.defaultKosliArguments), golden: "sonar attestation 'foo' is reported to trail: test-123\n", }, @@ -268,7 +268,7 @@ func (suite *AttestSonarCommandTestSuite) TestAttestSonarCmd() { wantError: true, name: "31 attesting a pull request scan with --sonar-revision of a commit it did not analyse fails", cmd: fmt.Sprintf("attest sonar --name cli.foo --commit HEAD --origin-url http://www.example.com --sonar-project-key cyber-dojo_differ --pull-request %s --sonar-revision 0000000000000000000000000000000000000000 %s", suite.prKey, suite.defaultKosliArguments), - golden: fmt.Sprintf("Error: analysis for pull request %s of project cyber-dojo_differ is of revision %s, not 0000000000000000000000000000000000000000. \nThe scan for revision 0000000000000000000000000000000000000000 may still be being processed by SonarQube, try again later.\n Otherwise check the revision is correct\n", suite.prKey, suite.prRevision), + golden: fmt.Sprintf("Error: analysis for pull request %s of project cyber-dojo_differ is of revision %s, not 0000000000000000000000000000000000000000.\nThe scan for that revision may still be being processed by SonarQube, try again later.\nOtherwise check the revision is correct\n", suite.prKey, suite.prRevision), }, } diff --git a/internal/sonar/sonar.go b/internal/sonar/sonar.go index a1bb726f5..a300e831f 100644 --- a/internal/sonar/sonar.go +++ b/internal/sonar/sonar.go @@ -258,9 +258,11 @@ func (sc *SonarConfig) GetSonarResults(logger *log.Logger) (*SonarResults, error return nil, err } // SonarQube keeps only a pull request's latest analysis, which is the - // previous push's until the current scan is processed (#1192). + // previous push's until the current scan is processed (#1192). Only a + // revision the user passed alongside --pull-request is checked; on the + // other paths sc.revision is the defaulted commit, not an expectation. if sc.pullRequest != "" && sc.revision != "" && sonarResults.Revision != sc.revision { - return nil, fmt.Errorf("analysis for pull request %s of project %s is of revision %s, not %s. \nThe scan for revision %s may still be being processed by SonarQube, try again later.\n Otherwise check the revision is correct", sc.pullRequest, project.Key, sonarResults.Revision, sc.revision, sc.revision) + return nil, fmt.Errorf("analysis for pull request %s of project %s is of revision %s, not %s.\nThe scan for that revision may still be being processed by SonarQube, try again later.\nOtherwise check the revision is correct", sc.pullRequest, project.Key, sonarResults.Revision, sc.revision) } } diff --git a/internal/sonar/sonar_test.go b/internal/sonar/sonar_test.go index 43478b797..a3c85c0ad 100644 --- a/internal/sonar/sonar_test.go +++ b/internal/sonar/sonar_test.go @@ -386,8 +386,10 @@ func TestGetSonarResults_BranchIgnoredForPullRequest(t *testing.T) { // TestGetSonarResults_PullRequestRevision is the #1192 check: SonarQube keeps only a // pull request's latest analysis, so a revision named alongside the pull request -// must be the analysed commit, and no revision means no check. Both ways of -// naming the scan reach the same lookup, so both paths are pinned. +// must be the analysed commit, and no revision means no check. Both variants pass +// --pull-request explicitly, discovering the scan via --sonar-project-key or via +// --sonar-ce-task-url; see TestGetSonarResults_PullRequestRevision_DiscoveredWithoutFlag +// for the case where the pull request is discovered without that flag. func TestGetSonarResults_PullRequestRevision(t *testing.T) { const otherRevision = "0000000000000000000000000000000000000000" @@ -438,6 +440,28 @@ func TestGetSonarResults_PullRequestRevision(t *testing.T) { } } +// TestGetSonarResults_PullRequestRevision_DiscoveredWithoutFlag pins the gap the +// comment above notes: when a CE task's own response names the pull request +// (--sonar-ce-task-url or report-task.txt used without --pull-request), sc.pullRequest +// is empty and the #1192 check does not fire, even for a revision that does not +// match — the same staleness the check exists to catch elsewhere. +func TestGetSonarResults_PullRequestRevision_DiscoveredWithoutFlag(t *testing.T) { + const otherRevision = "0000000000000000000000000000000000000000" + + fake := &fakeSonarProject{} + srv := httptest.NewServer(fake.handler()) + defer srv.Close() + + sc := sonar.NewSonarConfig("tok", t.TempDir(), srv.URL+"/api/ce/task?id="+revTaskID, "", "", otherRevision, "", "", 5) + results, err := sc.GetSonarResults(discardLogger()) + if err != nil { + t.Fatalf("expected no check without --pull-request, got error: %v", err) + } + if results.Revision != revRevision { + t.Errorf("expected the analysed revision %q in the results, got %q", revRevision, results.Revision) + } +} + // TestGetSonarResults_NoMatchingTask_KeepsSuppliedBranch pins the payload when // api/ce/activity holds no task for the analysis. That is reachable in production: // the endpoint needs administrative permission, so an ordinary CI token gets an