diff --git a/.golangci.yml b/.golangci.yml index 3f7d7108bd1..f046e3f76b6 100644 --- a/.golangci.yml +++ b/.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 diff --git a/attestor/command/cli.go b/attestor/command/cli.go index 0f8e89239c8..27e8b829689 100644 --- a/attestor/command/cli.go +++ b/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) } } diff --git a/attestor/policy/attestation_policy.go b/attestor/policy/attestation_policy.go index 1052bbad948..cfc991db1fd 100644 --- a/attestor/policy/attestation_policy.go +++ b/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 diff --git a/attestor/policy/attestation_policy_test.go b/attestor/policy/attestation_policy_test.go index 61e222b906d..0c1f991e875 100644 --- a/attestor/policy/attestation_policy_test.go +++ b/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) } }) diff --git a/clients/githubrepo/roundtripper/rate_limit.go b/clients/githubrepo/roundtripper/rate_limit.go index d2ae8378ab1..6d7d76b482c 100644 --- a/clients/githubrepo/roundtripper/rate_limit.go +++ b/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 } diff --git a/clients/githubrepo/roundtripper/rate_limit_test.go b/clients/githubrepo/roundtripper/rate_limit_test.go index b74b7d997b8..83b50291581 100644 --- a/clients/githubrepo/roundtripper/rate_limit_test.go +++ b/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) } diff --git a/clients/gitlabrepo/client.go b/clients/gitlabrepo/client.go index 601d9dbff75..a0196d245b1 100644 --- a/clients/gitlabrepo/client.go +++ b/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), ) diff --git a/clients/gitlabrepo/client_test.go b/clients/gitlabrepo/client_test.go new file mode 100644 index 00000000000..c305533550e --- /dev/null +++ b/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) + } + }) + } +} diff --git a/clients/gitlabrepo/graphql.go b/clients/gitlabrepo/graphql.go index 34a7694d4fb..4318e289c88 100644 --- a/clients/gitlabrepo/graphql.go +++ b/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) } diff --git a/clients/gitlabrepo/repo_test.go b/clients/gitlabrepo/repo_test.go index 87b36894355..a46250de82a 100644 --- a/clients/gitlabrepo/repo_test.go +++ b/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) { diff --git a/clients/ossfuzz/client.go b/clients/ossfuzz/client.go index 054ea50a122..8c19d8038e6 100644 --- a/clients/ossfuzz/client.go +++ b/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) } diff --git a/cmd/internal/packagemanager/client_test.go b/cmd/internal/packagemanager/client_test.go index 97d9c915130..04ffd40e783 100644 --- a/cmd/internal/packagemanager/client_test.go +++ b/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) diff --git a/cmd/root.go b/cmd/root.go index 4dccf166d64..506cdb819ed 100644 --- a/cmd/root.go +++ b/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( diff --git a/cmd/serve.go b/cmd/serve.go index 65f4ea50bca..31825104527 100644 --- a/cmd/serve.go +++ b/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 { diff --git a/cron/internal/cii/main.go b/cron/internal/cii/main.go index fa8616606da..dce89c8a571 100644 --- a/cron/internal/cii/main.go +++ b/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") } diff --git a/cron/internal/format/json.go b/cron/internal/format/json.go index b07f24390e8..88c78cf44cf 100644 --- a/cron/internal/format/json.go +++ b/cron/internal/format/json.go @@ -26,7 +26,6 @@ import ( "github.com/ossf/scorecard/v4/pkg" ) -//nolint type jsonCheckResult struct { Name string Details []string @@ -34,6 +33,7 @@ type jsonCheckResult struct { 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"` diff --git a/cron/internal/webhook/main.go b/cron/internal/webhook/main.go index 2324cd908d4..9724f79a0fa 100644 --- a/cron/internal/webhook/main.go +++ b/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) diff --git a/options/options.go b/options/options.go index f4d528b44f8..30acfe0efe3 100644 --- a/options/options.go +++ b/options/options.go @@ -18,13 +18,14 @@ package options import ( "errors" "fmt" + "log" "os" "strings" "github.com/caarlos0/env/v6" "github.com/ossf/scorecard/v4/clients" - "github.com/ossf/scorecard/v4/log" + sclog "github.com/ossf/scorecard/v4/log" ) // Options define common options for configuring scorecard. @@ -54,7 +55,7 @@ type Options struct { func New() *Options { opts := &Options{} if err := env.Parse(opts); err != nil { - fmt.Printf("could not parse env vars, using default options: %v", err) + log.Printf("could not parse env vars, using default options: %v", err) } // Defaulting. // TODO(options): Consider moving this to a separate function/method. @@ -105,7 +106,7 @@ const ( var ( // DefaultLogLevel retrieves the default log level. - DefaultLogLevel = log.DefaultLevel.String() + DefaultLogLevel = sclog.DefaultLevel.String() errCommitIsEmpty = errors.New("commit should be non-empty") errFormatNotSupported = errors.New("unsupported format") diff --git a/pkg/dependencydiff_result.go b/pkg/dependencydiff_result.go index d0a7bd2ecfc..88038983612 100644 --- a/pkg/dependencydiff_result.go +++ b/pkg/dependencydiff_result.go @@ -55,6 +55,8 @@ type ScorecardResultWithError struct { } // DependencyCheckResult is the dependency structure used in the returned results. +// +//nolint:musttag // functionality is deprecated anyway type DependencyCheckResult struct { // ChangeType indicates whether the dependency is added, updated, or removed. ChangeType *ChangeType diff --git a/pkg/json.go b/pkg/json.go index a1fab73f4a6..fd81223cffa 100644 --- a/pkg/json.go +++ b/pkg/json.go @@ -27,7 +27,7 @@ import ( "github.com/ossf/scorecard/v4/log" ) -// nolint: govet +//nolint:govet type jsonCheckResult struct { Name string Details []string @@ -35,6 +35,7 @@ type jsonCheckResult struct { Pass bool } +//nolint:musttag type jsonScorecardResult struct { Repo string Date string