Merge branch 'main' into scorecards-cleanup

.github/workflows/integration.yml

@@ -78,24 +78,14 @@ jobs:
           timeout_minutes: 30
           command: make e2e-pat
 
-      - name: Run attestor e2e  #using retry because the GitHub token is being throttled.
-        uses: nick-invision/retry@3e91a01664abd3c5cd539100d10d33b9c5b68482
-        env:
-          GITHUB_AUTH_TOKEN: ${{ secrets.GH_AUTH_TOKEN }}
-        with:
-          max_attempts: 3
-          retry_on: error
-          timeout_minutes: 10
-          command: make e2e-attestor
-
       - name: codecov
         uses: codecov/codecov-action@81cd2dc8148241f03f5839d295e000b8f761e378 # 2.1.0
         with:
-         files: ./e2e-coverage.out,./attestor/e2e/e2e-coverage.out
+         files: ./e2e-coverage.out
          verbose: true
 
       - name: find comment
-        uses: peter-evans/find-comment@b657a70ff16d17651703a84bee1cb9ad9d2be2ea # v2.0.1
+        uses: peter-evans/find-comment@f4499a714d59013c74a08789b48abe4b704364a0 # v2.1.0
         id: fc
         with:
           issue-number: ${{ github.event.pull_request.number || github.event.client_payload.pull_request.number }}

.github/workflows/main.yml

@@ -64,11 +64,11 @@ jobs:
          check-latest: true
          cache: true
      - name: Run unit-tests
-       run: make unit-test unit-test-attestor
+       run: make unit-test
      - name: Upload codecoverage
        uses: codecov/codecov-action@81cd2dc8148241f03f5839d295e000b8f761e378 # 2.1.0
        with:
-         files: ./unit-coverage.out,./attestor/unit-coverage.out
+         files: ./unit-coverage.out
          verbose: true
   generate-mocks:
     name: generate-mocks

Makefile

@@ -84,7 +84,7 @@ install: $(GOLANGCI_LINT) \
 ##@ Build
 ################################## make all ###################################
 all:  ## Runs build, test and verify
-all-targets = build unit-test unit-test-attestor check-linter validate-docs add-projects validate-projects
+all-targets = build unit-test check-linter validate-docs add-projects validate-projects
 .PHONY: all all-targets-update-dependencies $(all-targets) update-dependencies tree-status
 all-targets-update-dependencies: $(all-targets) | update-dependencies
 all: update-dependencies all-targets-update-dependencies tree-status
@@ -93,7 +93,6 @@ update-dependencies: ## Update go dependencies for all modules
 	# Update root go modules
 	go mod tidy && go mod verify
 	cd tools; go mod tidy && go mod verify; cd ../
-	cd attestor; go mod tidy && go mod verify; cd ../
 
 check-linter: ## Install and run golang linter
 check-linter: | $(GOLANGCI_LINT)
@@ -167,7 +166,7 @@ validate-docs: docs/checks/internal/generate/main.go
 	# Validating checks.yaml
 	go run ./docs/checks/internal/validate/main.go
 
-SCORECARD_DEPS = $(shell find . -iname "*.go" | grep -v tools/ | grep -v attestor/)
+SCORECARD_DEPS = $(shell find . -iname "*.go" | grep -v tools/)
 build-scorecard: ## Build Scorecard CLI
 build-scorecard: scorecard
 scorecard: $(SCORECARD_DEPS)
@@ -308,7 +307,7 @@ cron-worker-docker:
 
 ##@ Tests
 ################################# make test ###################################
-test-targets = unit-test unit-test-attestor e2e-pat e2e-gh-token ci-e2e
+test-targets = unit-test e2e-pat e2e-gh-token ci-e2e
 .PHONY: test $(test-targets)
 test: $(test-targets)
 

attestor/README.md

@@ -16,6 +16,17 @@ Unless there's an internal error, scorecard-attestor will always return a succes
 
 Policies for scorecard attestor can be passed through the CLI using the `--policy` flag. Examples of policies can be seen in [attestor/policy/testdata](/attestor/policy/testdata).
 
+### Policies
+
+* `PreventBinaryArtifacts`: Ensure that a repository is free from binary artifacts, which can link against the final repo artifact but isn't reviewable.
+  * `AllowedBinaryArtifacts`: A list of binary artifacts, by repo path, to ignore. If not specified, no binary artifacts will be allowed
+* `PreventKnownVulnerabilities`: Ensure that the project is free from security vulnerabilities/advisories, as registered in osv.dev.
+* `PreventUnpinnedDependencies`: Ensure that a project's dependencies are pinned by hash. Dependency pinning makes builds more predictable, and prevents the consumption of malicious package versions from a compromised upstream.
+  * `AllowedUnpinnedDependencies`: Ignore some dependencies, either by the filepath of the dependency management file (`filepath`, e.g. requirements.txt or package.json) or the dependency name (`packagename`, the specific package being ignored). If multiple filepaths/names, or a combination of filepaths and names are specified, all of them will be used. If not specified, no unpinned dependencies will be allowed.
+* `RequireCodeReviewed`: Require that If `CodeReviewRequirements` is not specified, at least one reviewer will be required on all changesets. Scorecard-attestor inherits scorecard's deafult commit window (i.e. will only look at the last 30 commits to determine if they are reviewed or not).
+  * `CodeReviewRequirements.MinReviewers`: The minimum number of distinct approvals required.
+  * `CodeReviewRequirements.RequiredApprovers`: A set of approvers, all of whom must be found to have approved all changes. If any have not approved any changes, the check fails.
+
 ### Policy schema
 
 Policies follow the following schema:
@@ -48,14 +59,6 @@ optional:
             minReviewers: "//int"
 ```
 
-### Missing parameters
-
-Policies that are left blank will be ignored. Policies that allow users additional configuration options will be given default parameters as listed below.
-
-* `PreventBinaryArtifacts`: If not specified, `AllowedBinaryArtifacts` will be empty, i.e. no binary artifacts will be allowed
-* `PreventUnpinnedDependencies`: If not specified, `AllowedUnpinnedDependencies` will be empty, i.e. no unpinned dependencies will be allowed
-* `RequireCodeReviewed`: If not specified, `CodeReviewRequirements` will require at least one reviewer on all changesets.
-
 ## Sample
 
 Examples of how to use scorecard-attestor with binary authorization in your project can be found in these two repos:

attestor/command/check.go

@@ -19,7 +19,7 @@ import (
 	"fmt"
 	"os"
 
-	"github.com/ossf/scorecard-attestor/policy"
+	"github.com/ossf/scorecard/v4/attestor/policy"
 	"github.com/ossf/scorecard/v4/checker"
 	"github.com/ossf/scorecard/v4/checks"
 	sclog "github.com/ossf/scorecard/v4/log"
@@ -81,9 +81,18 @@ func runCheck() (policy.PolicyResult, error) {
 	requiredChecks := attestationPolicy.GetRequiredChecksForPolicy()
 
 	enabledChecks := map[string]checker.Check{
-		"BinaryArtifacts": {
+		checks.CheckBinaryArtifacts: {
 			Fn: checks.BinaryArtifacts,
 		},
+		checks.CheckVulnerabilities: {
+			Fn: checks.Vulnerabilities,
+		},
+		checks.CheckCodeReview: {
+			Fn: checks.CodeReview,
+		},
+		checks.CheckPinnedDependencies: {
+			Fn: checks.PinningDependencies,
+		},
 	}
 
 	// Filter out checks that won't be needed for policy-evaluation time

attestor/command/cli.go

@@ -24,36 +24,40 @@ import (
 )
 
 var (
-	// input flags
+	// input flags.
 	repoURL            string
 	commitSHA          string
-	mode               string
 	image              string
 	policyPath         string
 	attestationProject string
 	overwrite          bool
-	// input flags: pgp key flags
+	// input flags: pgp key flags.
 	pgpPriKeyPath string
 	pgpPassphrase string
-	// pkix key flags
+	// pkix key flags.
 	pkixPriKeyPath string
 	pkixAlg        string
 
-	// input flags: kms flags
+	// input flags: kms flags.
 	kmsKeyName   string
 	kmsDigestAlg string
 )
 
+//nolint:lll
 func addCheckFlags(cmd *cobra.Command) {
 	cmd.PersistentFlags().StringVar(&policyPath, "policy", "", "(required for check) scorecard attestation policy file path, e.g., /tmp/policy-binauthz.yml")
+	//nolint:errcheck
 	cmd.MarkPersistentFlagRequired("policy")
 	cmd.PersistentFlags().StringVar(&repoURL, "repo-url", "", "Repo URL from which source was built")
+	//nolint:errcheck
 	cmd.MarkPersistentFlagRequired("repo-url")
 	cmd.PersistentFlags().StringVar(&commitSHA, "commit", "", "Git SHA at which image was built")
 }
 
+//nolint:lll
 func addSignFlags(cmd *cobra.Command) {
 	cmd.PersistentFlags().StringVar(&image, "image", "", "Image url, e.g., gcr.io/foo/bar@sha256:abcd")
+	//nolint:errcheck
 	cmd.MarkPersistentFlagRequired("image")
 	cmd.PersistentFlags().StringVar(&attestationProject, "attestation-project", "", "project id for GCP project that stores attestation, use image project if set to empty")
 	cmd.PersistentFlags().BoolVar(&overwrite, "overwrite", false, "overwrite attestation if already existed (default false)")
@@ -63,10 +67,8 @@ func addSignFlags(cmd *cobra.Command) {
 	cmd.PersistentFlags().StringVar(&pgpPassphrase, "pgp-passphrase", "", "passphrase for pgp private key, if any")
 	cmd.PersistentFlags().StringVar(&pkixPriKeyPath, "pkix-private-key", "", "pkix private signing key path, e.g., /dev/shm/key.pem")
 	cmd.PersistentFlags().StringVar(&pkixAlg, "pkix-alg", "", "pkix signature algorithm, e.g., ecdsa-p256-sha256")
-
 }
 
-// Export for testability
 var RootCmd = &cobra.Command{
 	Use:   "scorecard-attestor",
 	Short: "scorecard-attestor generates attestations based on scorecard results",
@@ -77,7 +79,6 @@ var checkAndSignCmd = &cobra.Command{
 	Short: "Run scorecard and sign a container image if attestation policy check passes",
 	RunE: func(cmd *cobra.Command, args []string) error {
 		passed, err := runCheck()
-
 		if err != nil {
 			return err
 		}
@@ -101,6 +102,7 @@ var checkCmd = &cobra.Command{
 	SilenceUsage: true,
 }
 
+//nolint:gochecknoinits
 func init() {
 	RootCmd.AddCommand(checkCmd, checkAndSignCmd)
 

attestor/command/sign.go

@@ -16,67 +16,78 @@ package command
 
 import (
 	"fmt"
-	"io/ioutil"
+	"os"
 	"strings"
 
 	"github.com/grafeas/kritis/pkg/attestlib"
 	"github.com/grafeas/kritis/pkg/kritis/metadata/containeranalysis"
 	"github.com/grafeas/kritis/pkg/kritis/signer"
 	"github.com/grafeas/kritis/pkg/kritis/util"
+
 	sclog "github.com/ossf/scorecard/v4/log"
 )
 
 const scorecardNoteID = "ossf-scorecard-attestation"
 
+type EncryptionParamError struct {
+	Message string
+}
+
+func (ep EncryptionParamError) Error() string {
+	return ep.Message
+}
+
 func runSign() error {
 	logger := sclog.NewLogger(sclog.DefaultLevel)
 
 	// Create a client
 	client, err := containeranalysis.New()
 	if err != nil {
-		return fmt.Errorf("could not initialize the client %v", err)
+		return fmt.Errorf("could not initialize the client %w", err)
 	}
 
 	// Read the signing credentials
 	// Either kmsKeyName or pgpPriKeyPath needs to be set
 	if kmsKeyName == "" && pgpPriKeyPath == "" && pkixPriKeyPath == "" {
-		return fmt.Errorf("neither kms_key_name, pgp_private_key, or pkix_private_key is specified")
+		return EncryptionParamError{"neither kms_key_name, pgp_private_key, or pkix_private_key is specified"}
 	}
 	var cSigner attestlib.Signer
-	if kmsKeyName != "" {
+	switch {
+	case kmsKeyName != "":
 		logger.Info(fmt.Sprintf("Using kms key %s for signing.", kmsKeyName))
 		if kmsDigestAlg == "" {
-			return fmt.Errorf("kms_digest_alg is unspecified, must be one of SHA256|SHA384|SHA512, and the same as specified by the key version's algorithm")
+			//nolint:lll
+			return EncryptionParamError{"kms_digest_alg is unspecified, must be one of SHA256|SHA384|SHA512, and the same as specified by the key version's algorithm"}
 		}
 		kmsDigestAlg = strings.ToUpper(kmsDigestAlg)
 		cSigner, err = signer.NewCloudKmsSigner(kmsKeyName, signer.DigestAlgorithm(kmsDigestAlg))
 		if err != nil {
-			return fmt.Errorf("creating kms signer failed: %v\n", err)
+			return fmt.Errorf("creating kms signer failed: %w", err)
 		}
-	} else if pgpPriKeyPath != "" {
+	case pgpPriKeyPath != "":
 		logger.Info("Using pgp key for signing.")
-		signerKey, err := ioutil.ReadFile(pgpPriKeyPath)
+		signerKey, err := os.ReadFile(pgpPriKeyPath)
 		if err != nil {
-			return fmt.Errorf("fail to read signer key: %v\n", err)
+			return fmt.Errorf("fail to read signer key: %w", err)
 		}
 		// Create a cryptolib signer
 		cSigner, err = attestlib.NewPgpSigner(signerKey, pgpPassphrase)
 		if err != nil {
-			return fmt.Errorf("creating pgp signer failed: %v\n", err)
+			return fmt.Errorf("creating pgp signer failed: %w", err)
 		}
-	} else {
+	default:
 		logger.Info("Using pkix key for signing.")
-		signerKey, err := ioutil.ReadFile(pkixPriKeyPath)
+		signerKey, err := os.ReadFile(pkixPriKeyPath)
 		if err != nil {
-			return fmt.Errorf("fail to read signer key: %v\n", err)
+			return fmt.Errorf("fail to read signer key: %w", err)
 		}
 		sAlg := attestlib.ParseSignatureAlgorithm(pkixAlg)
 		if sAlg == attestlib.UnknownSigningAlgorithm {
-			return fmt.Errorf("empty or unknown PKIX signature algorithm: %s\n", pkixAlg)
+			return EncryptionParamError{fmt.Sprintf("empty or unknown PKIX signature algorithm: %s", pkixAlg)}
 		}
 		cSigner, err = attestlib.NewPkixSigner(signerKey, sAlg, "")
 		if err != nil {
-			return fmt.Errorf("creating pkix signer failed: %v\n", err)
+			return fmt.Errorf("creating pkix signer failed: %w", err)
 		}
 	}
 
@@ -93,15 +104,15 @@ func runSign() error {
 
 	err = util.CheckNoteName(scorecardNoteName)
 	if err != nil {
-		return fmt.Errorf("note name is invalid %v", err)
+		return fmt.Errorf("note name is invalid %w", err)
 	}
 
 	// Create signer
 	r := signer.New(client, cSigner, scorecardNoteName, attestationProject, overwrite)
 	// Sign image
 	err = r.SignImage(image)
 	if err != nil {
-		return fmt.Errorf("signing image failed: %v", err)
+		return fmt.Errorf("signing image failed: %w", err)
 	}
 	return nil
 }