Merge branch 'main' into dependabot/go_modules/github.com/xanzy/go-gi…

…tlab-0.93.2

.golangci.yml

@@ -29,6 +29,7 @@ linters:
     - errorlint
     - exhaustive
     - exportloopref
+    - forbidigo
     - gci
     - gochecknoinits
     - gocognit
@@ -62,6 +63,8 @@ linters:
     - unused
     - whitespace
     - wrapcheck
+  presets:
+    - bugs
 linters-settings:
   errcheck:
     check-type-assertions: true
@@ -73,6 +76,10 @@ linters-settings:
   exhaustive:
     # https://golangci-lint.run/usage/linters/#exhaustive
     default-signifies-exhaustive: true
+  forbidigo:
+    forbid:
+      - p: "^fmt\\.Print.*$"
+        msg: "Do not commit print statements. Output to stdout interferes with users who redirect JSON results to files."
   govet:
     enable:
       - fieldalignment

attestor/command/cli.go

@@ -114,7 +114,7 @@ func init() {
 
 func Execute() {
 	if err := RootCmd.Execute(); err != nil {
-		fmt.Println(err)
+		fmt.Fprintln(os.Stderr, err)
 		os.Exit(1)
 	}
 }

attestor/policy/attestation_policy.go

@@ -29,7 +29,7 @@ import (
 	sclog "github.com/ossf/scorecard/v4/log"
 )
 
-//nolint:govet
+//nolint:govet,musttag // JSON usage is test only
 type AttestationPolicy struct {
 	// PreventBinaryArtifacts : set to true to require that this project's SCM repo is
 	// free of binary artifacts

attestor/policy/attestation_policy_test.go

@@ -17,7 +17,6 @@ package policy
 import (
 	"encoding/json"
 	"errors"
-	"fmt"
 	"testing"
 
 	"github.com/google/go-cmp/cmp"
@@ -533,8 +532,8 @@ func TestAttestationPolicyRead(t *testing.T) {
 			// Compare outputs only if the error is nil.
 			// TODO: compare objects.
 			if p.ToJSON() != tt.result.ToJSON() {
-				fmt.Printf("p.ToJSON(): %v\n", p.ToJSON())
-				fmt.Printf("tt.result.ToJSON(): %v\n", tt.result.ToJSON())
+				t.Logf("p.ToJSON(): %v\n", p.ToJSON())
+				t.Logf("tt.result.ToJSON(): %v\n", tt.result.ToJSON())
 				t.Fatalf("%s: invalid result", tt.name)
 			}
 		})

clients/githubrepo/roundtripper/rate_limit.go

@@ -62,6 +62,7 @@ func (gh *rateLimitTransport) RoundTrip(r *http.Request) (*http.Response, error)
 	rateLimit := resp.Header.Get("X-RateLimit-Remaining")
 	remaining, err := strconv.Atoi(rateLimit)
 	if err != nil {
+		//nolint:nilerr // just an error in metadata, response may still be useful?
 		return resp, nil
 	}
 	ctx, err := tag.New(r.Context(), tag.Upsert(githubstats.ResourceType, resp.Header.Get("X-RateLimit-Resource")))
@@ -73,6 +74,7 @@ func (gh *rateLimitTransport) RoundTrip(r *http.Request) (*http.Response, error)
 	if remaining <= 0 {
 		reset, err := strconv.Atoi(resp.Header.Get("X-RateLimit-Reset"))
 		if err != nil {
+			//nolint:nilerr // just an error in metadata, response may still be useful?
 			return resp, nil
 		}
 

clients/githubrepo/roundtripper/rate_limit_test.go

@@ -14,6 +14,7 @@
 package roundtripper
 
 import (
+	"context"
 	"net/http"
 	"net/http/httptest"
 	"testing"
@@ -60,7 +61,7 @@ func TestRoundTrip(t *testing.T) {
 	}
 
 	t.Run("Successful response", func(t *testing.T) {
-		req, err := http.NewRequest(http.MethodGet, ts.URL+"/success", nil)
+		req, err := http.NewRequestWithContext(context.Background(), http.MethodGet, ts.URL+"/success", nil)
 		if err != nil {
 			t.Fatalf("Failed to create request: %v", err)
 		}
@@ -69,13 +70,14 @@ func TestRoundTrip(t *testing.T) {
 		if err != nil {
 			t.Errorf("Unexpected error: %v", err)
 		}
+		defer resp.Body.Close()
 		if resp.StatusCode != http.StatusOK {
 			t.Errorf("Expected status code %d, got %d", http.StatusOK, resp.StatusCode)
 		}
 	})
 
 	t.Run("Retry-After header set", func(t *testing.T) {
-		req, err := http.NewRequest(http.MethodGet, ts.URL+"/retry", nil)
+		req, err := http.NewRequestWithContext(context.Background(), http.MethodGet, ts.URL+"/retry", nil)
 		if err != nil {
 			t.Fatalf("Failed to create request: %v", err)
 		}
@@ -84,6 +86,7 @@ func TestRoundTrip(t *testing.T) {
 		if err != nil {
 			t.Errorf("Unexpected error: %v", err)
 		}
+		defer resp.Body.Close()
 		if resp.StatusCode != http.StatusOK {
 			t.Errorf("Expected status code %d, got %d", http.StatusOK, resp.StatusCode)
 		}

clients/gitlabrepo/client.go

@@ -62,8 +62,7 @@ var errRepoAccess = errors.New("repo inaccessible")
 
 // Raise an error if repository access level is private or disabled.
 func checkRepoInaccessible(repo *gitlab.Project) error {
-	if (repo.RepositoryAccessLevel == gitlab.PrivateAccessControl) ||
-		(repo.RepositoryAccessLevel == gitlab.DisabledAccessControl) {
+	if repo.RepositoryAccessLevel == gitlab.DisabledAccessControl {
 		return fmt.Errorf("%w: %s access level %s",
 			errRepoAccess, repo.PathWithNamespace, string(repo.RepositoryAccessLevel),
 		)

clients/gitlabrepo/client_test.go

@@ -0,0 +1,71 @@
+// Copyright 2023 OpenSSF Scorecard Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//      http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package gitlabrepo
+
+import (
+	"errors"
+	"testing"
+
+	"github.com/xanzy/go-gitlab"
+)
+
+func TestCheckRepoInaccessible(t *testing.T) {
+	t.Parallel()
+
+	tests := []struct {
+		want error
+		repo *gitlab.Project
+		name string
+	}{
+		{
+			name: "if repo is enabled then it is accessible",
+			repo: &gitlab.Project{
+				RepositoryAccessLevel: gitlab.EnabledAccessControl,
+			},
+		},
+		{
+			name: "repo should not have public access in this case, but if it does it is accessible",
+			repo: &gitlab.Project{
+				RepositoryAccessLevel: gitlab.PublicAccessControl,
+			},
+		},
+		{
+			name: "if repo is disabled then is inaccessible",
+			repo: &gitlab.Project{
+				RepositoryAccessLevel: gitlab.DisabledAccessControl,
+			},
+			want: errRepoAccess,
+		},
+		{
+			name: "if repo is private then it is accessible",
+			repo: &gitlab.Project{
+				RepositoryAccessLevel: gitlab.PrivateAccessControl,
+			},
+		},
+	}
+
+	for _, tt := range tests {
+		tt := tt
+
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+
+			got := checkRepoInaccessible(tt.repo)
+			if !errors.Is(got, tt.want) {
+				t.Errorf("checkRepoInaccessible() got %v, want %v", got, tt.want)
+			}
+		})
+	}
+}

clients/gitlabrepo/graphql.go

@@ -45,7 +45,7 @@ func (handler *graphqlHandler) init(ctx context.Context, repourl *repoURL) {
 	src := oauth2.StaticTokenSource(
 		&oauth2.Token{AccessToken: os.Getenv("GITLAB_AUTH_TOKEN")},
 	)
-	handler.client = oauth2.NewClient(context.Background(), src)
+	handler.client = oauth2.NewClient(ctx, src)
 	handler.graphClient = graphql.NewClient(fmt.Sprintf("%s/api/graphql", repourl.Host()), handler.client)
 }
 

clients/gitlabrepo/repo_test.go

@@ -15,7 +15,6 @@
 package gitlabrepo
 
 import (
-	"fmt"
 	"os"
 	"testing"
 
@@ -107,9 +106,9 @@ func TestRepoURL_IsValid(t *testing.T) {
 				t.Errorf("repoURL.IsValid() error = %v, wantErr %v", err, tt.wantErr)
 			}
 			if !tt.wantErr && !cmp.Equal(tt.expected, r, cmpopts.IgnoreUnexported(repoURL{})) {
-				fmt.Println("expected: " + tt.expected.host + " GOT: " + r.host)
-				fmt.Println("expected: " + tt.expected.owner + " GOT: " + r.owner)
-				fmt.Println("expected: " + tt.expected.project + " GOT: " + r.project)
+				t.Logf("expected: %s GOT: %s", tt.expected.host, r.host)
+				t.Logf("expected: %s GOT: %s", tt.expected.owner, r.owner)
+				t.Logf("expected: %s GOT: %s", tt.expected.project, r.project)
 				t.Errorf("Got diff: %s", cmp.Diff(tt.expected, r))
 			}
 			if !cmp.Equal(r.Host(), tt.expected.host) {

clients/ossfuzz/client.go

@@ -39,6 +39,7 @@ var (
 )
 
 type client struct {
+	ctx       context.Context
 	err       error
 	projects  map[string]bool
 	statusURL string
@@ -54,6 +55,7 @@ type ossFuzzStatus struct {
 // CreateOSSFuzzClient returns a client which implements RepoClient interface.
 func CreateOSSFuzzClient(ossFuzzStatusURL string) clients.RepoClient {
 	return &client{
+		ctx:       context.Background(),
 		statusURL: ossFuzzStatusURL,
 		projects:  map[string]bool{},
 	}
@@ -62,6 +64,7 @@ func CreateOSSFuzzClient(ossFuzzStatusURL string) clients.RepoClient {
 // CreateOSSFuzzClientEager returns a OSS Fuzz Client which has already fetched and parsed the status file.
 func CreateOSSFuzzClientEager(ossFuzzStatusURL string) (clients.RepoClient, error) {
 	c := client{
+		ctx:       context.Background(),
 		statusURL: ossFuzzStatusURL,
 		projects:  map[string]bool{},
 	}
@@ -91,7 +94,7 @@ func (c *client) Search(request clients.SearchRequest) (clients.SearchResponse,
 }
 
 func (c *client) init() {
-	b, err := fetchStatusFile(c.statusURL)
+	b, err := fetchStatusFile(c.ctx, c.statusURL)
 	if err != nil {
 		c.err = err
 		return
@@ -118,9 +121,12 @@ func parseStatusFile(contents []byte, m map[string]bool) error {
 	return nil
 }
 
-func fetchStatusFile(uri string) ([]byte, error) {
-	//nolint:gosec // URI comes from a constant or a test HTTP server, not user input
-	resp, err := http.Get(uri)
+func fetchStatusFile(ctx context.Context, uri string) ([]byte, error) {
+	req, err := http.NewRequestWithContext(ctx, http.MethodGet, uri, nil)
+	if err != nil {
+		return nil, fmt.Errorf("making status file request: %w", err)
+	}
+	resp, err := http.DefaultClient.Do(req)
 	if err != nil {
 		return nil, fmt.Errorf("http.Get: %w", err)
 	}

cmd/internal/packagemanager/client_test.go

@@ -63,6 +63,7 @@ func Test_GetURI_calls_client_get_with_input(t *testing.T) {
 				t.Errorf("Test_GetURI_calls_client_get_with_input() error in Get= %v", err)
 				return
 			}
+			defer got.Body.Close()
 			body, err := io.ReadAll(got.Body)
 			if err != nil {
 				t.Errorf("Test_GetURI_calls_client_get_with_input() error in ReadAll= %v", err)
@@ -118,6 +119,7 @@ func Test_Get_calls_client_get_with_input(t *testing.T) {
 				t.Errorf("Test_Get_calls_client_get_with_input() error in Get = %v", err)
 				return
 			}
+			defer got.Body.Close()
 			body, err := io.ReadAll(got.Body)
 			if err != nil {
 				t.Errorf("Test_Get_calls_client_get_with_input() error in ReadAll = %v", err)

cmd/root.go

@@ -151,7 +151,7 @@ func rootCmd(o *options.Options) error {
 		for checkName := range enabledChecks {
 			fmt.Fprintf(os.Stderr, "Finished [%s]\n", checkName)
 		}
-		fmt.Println("\nRESULTS\n-------")
+		fmt.Fprintln(os.Stderr, "\nRESULTS\n-------")
 	}
 
 	resultsErr := pkg.FormatResults(

cmd/serve.go

@@ -95,7 +95,7 @@ func serveCmd(o *options.Options) *cobra.Command {
 			if port == "" {
 				port = "8080"
 			}
-			fmt.Printf("Listening on localhost:%s\n", port)
+			logger.Info("Listening on localhost:" + port + "\n")
 			//nolint: gosec // unsused.
 			err = http.ListenAndServe(fmt.Sprintf("0.0.0.0:%s", port), nil)
 			if err != nil {

cron/internal/cii/main.go

@@ -21,6 +21,7 @@ import (
 	"flag"
 	"fmt"
 	"io"
+	"log"
 	"net/http"
 	"strings"
 
@@ -46,7 +47,7 @@ func writeToCIIDataBucket(ctx context.Context, pageResp []ciiPageResp, bucketURL
 		if err != nil {
 			return fmt.Errorf("error during AsJSON: %w", err)
 		}
-		fmt.Printf("Writing result for: %s\n", projectURL)
+		log.Printf("Writing result for: %s\n", projectURL)
 		if err := data.WriteToBlobStore(ctx, bucketURL,
 			fmt.Sprintf("%s/result.json", projectURL), jsonData); err != nil {
 			return fmt.Errorf("error during data.WriteToBlobStore: %w", err)
@@ -82,7 +83,7 @@ func getPage(ctx context.Context, pageNum int) ([]ciiPageResp, error) {
 
 func main() {
 	ctx := context.Background()
-	fmt.Println("Starting...")
+	log.Println("Starting...")
 
 	flag.Parse()
 	if err := config.ReadConfig(); err != nil {
@@ -107,5 +108,5 @@ func main() {
 		panic(err)
 	}
 
-	fmt.Println("Job completed")
+	log.Println("Job completed")
 }

cron/internal/format/json.go

@@ -26,14 +26,14 @@ import (
 	"github.com/ossf/scorecard/v4/pkg"
 )
 
-//nolint
 type jsonCheckResult struct {
 	Name       string
 	Details    []string
 	Confidence int
 	Pass       bool
 }
 
+//nolint:musttag
 type jsonScorecardResult struct {
 	Repo     string
 	Date     string
@@ -47,7 +47,7 @@ type jsonCheckDocumentationV2 struct {
 	// Can be extended if needed.
 }
 
-//nolint
+//nolint:govet
 type jsonCheckResultV2 struct {
 	Details []string                 `json:"details"`
 	Score   int                      `json:"score"`

cron/internal/webhook/main.go

@@ -80,7 +80,7 @@ func scriptHandler(w http.ResponseWriter, r *http.Request) {
 
 func main() {
 	http.HandleFunc("/", scriptHandler)
-	fmt.Printf("Starting HTTP server on port 8080 ...\n")
+	log.Printf("Starting HTTP server on port 8080 ...\n")
 	// nolint:gosec // internal server.
 	if err := http.ListenAndServe(":8080", nil); err != nil {
 		log.Fatal(err)