🌱 attestor: e2e tests

attestor/README.md

@@ -25,7 +25,7 @@ Policies for scorecard attestor can be passed through the CLI using the `--polic
   * `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.
+  * `CodeReviewRequirements.RequiredApprovers`: A set of approvers, any of whom must be found to have approved all changes. If a change is found without any approvals from this list, the check fails.
 
 ### Policy schema
 

attestor/command/check.go

@@ -35,6 +35,11 @@ func (ep EmptyParameterError) Error() string {
 }
 
 func runCheck() (policy.PolicyResult, error) {
+	return RunCheckWithParams(repoURL, commitSHA, policyPath)
+}
+
+// RunCheckWithParams: Run scorecard check on repo. Export for testability.
+func RunCheckWithParams(repoURL, commitSHA, policyPath string) (policy.PolicyResult, error) {
 	ctx := context.Background()
 	logger := sclog.NewLogger(sclog.DefaultLevel)
 

attestor/policy/attestation_policy.go

@@ -206,8 +206,6 @@ func CheckCodeReviewed(
 		changeset := &results.CodeReviewResults.DefaultBranchChangesets[i]
 		numApprovers := 0
 		approvals := make(map[string]bool)
-		// CodeReview check is limited to github.com pull request reviews
-		// Log if a change isn't a github pr since it's a bit unintuitive
 		foundLinkedReviews := false
 
 		for i := range changeset.Commits {
@@ -221,6 +219,8 @@ func CheckCodeReviewed(
 			}
 		}
 
+		// CodeReview check is limited to github.com pull request reviews
+		// Log if a change isn't a github pr since it's a bit unintuitive
 		if !foundLinkedReviews {
 			logger.Info(fmt.Sprintf("no code reviews linked to %s", toString(changeset)))
 		}
@@ -237,17 +237,18 @@ func CheckCodeReviewed(
 			return Fail, nil
 		}
 
-		missingApprovers := false
+		if len(reqs.RequiredApprovers) == 0 {
+			continue
+		}
+
+		missingApprovers := true
 		for _, appr := range reqs.RequiredApprovers {
-			if approved, ok := approvals[appr]; !(ok && approved) {
+			if approved, ok := approvals[appr]; ok && approved {
 				logger.Info(
-					fmt.Sprintf(
-						"approver %s required but didn't approve %s",
-						appr,
-						toString(changeset),
-					),
+					fmt.Sprintf("found approver %s for %s", appr, toString(changeset)),
 				)
-				missingApprovers = true
+				missingApprovers = false
+				break
 			}
 		}
 

attestor/policy/attestation_policy_test.go

@@ -222,7 +222,7 @@ func TestCheckCodeReviewed(t *testing.T) {
 			name: "no approvals from the right users",
 			reqs: CodeReviewRequirements{
 				MinReviewers:      2,
-				RequiredApprovers: []string{"bob"},
+				RequiredApprovers: []string{"bob", "bob-2"},
 			},
 			raw: &checker.RawResults{
 				CodeReviewResults: checker.CodeReviewData{
@@ -245,6 +245,46 @@ func TestCheckCodeReviewed(t *testing.T) {
 			},
 			expected: Fail,
 		},
+		{
+			name: "approvals from one of the required approvers",
+			reqs: CodeReviewRequirements{
+				MinReviewers:      2,
+				RequiredApprovers: []string{"bob", "alice"},
+			},
+			raw: &checker.RawResults{
+				CodeReviewResults: checker.CodeReviewData{
+					DefaultBranchChangesets: []checker.Changeset{
+						{
+							RevisionID: "1",
+							Commits: []clients.Commit{{
+								SHA: "a",
+								AssociatedMergeRequest: clients.PullRequest{
+									Reviews: []clients.Review{
+										{Author: &clients.User{Login: "alice"}, State: "APPROVED"},
+										{Author: &clients.User{Login: "alice-2"}, State: "APPROVED"},
+										{Author: &clients.User{Login: "bob"}, State: "NEEDS_CHANGES"},
+									},
+								},
+							}},
+						},
+						{
+							RevisionID: "1",
+							Commits: []clients.Commit{{
+								SHA: "a",
+								AssociatedMergeRequest: clients.PullRequest{
+									Reviews: []clients.Review{
+										{Author: &clients.User{Login: "alice"}, State: "NEEDS_CHANGES"},
+										{Author: &clients.User{Login: "alice-2"}, State: "APPROVED"},
+										{Author: &clients.User{Login: "bob"}, State: "APPROVED"},
+									},
+								},
+							}},
+						},
+					},
+				},
+			},
+			expected: Pass,
+		},
 		{
 			name: "some changesets not reviewed",
 			reqs: CodeReviewRequirements{

e2e/attestor_policy_test.go

@@ -0,0 +1,206 @@
+// Copyright 2021 Security 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 e2e
+
+import (
+	"fmt"
+	"os"
+	"strings"
+
+	. "github.com/onsi/ginkgo/v2"
+	. "github.com/onsi/gomega"
+	"gopkg.in/yaml.v2"
+
+	"github.com/ossf/scorecard/v4/attestor/command"
+	"github.com/ossf/scorecard/v4/attestor/policy"
+)
+
+var _ = Describe("E2E TEST PAT: scorecard-attestor policy", func() {
+	Context("E2E TEST:Validating scorecard attestation policy", func() {
+		It("Should attest to repos based on policy", func() {
+			tt := []struct {
+				name     string
+				repoURL  string
+				commit   string
+				policy   policy.AttestationPolicy
+				expected policy.PolicyResult
+			}{
+				{
+					name:    "test good repo",
+					repoURL: "https://github.com/ossf-tests/scorecard-binauthz-test-good",
+					policy: policy.AttestationPolicy{
+						PreventBinaryArtifacts:      true,
+						PreventKnownVulnerabilities: true,
+						PreventUnpinnedDependencies: true,
+					},
+					expected: policy.Pass,
+				},
+				{
+					name:    "test bad repo with vulnerabilities prevented but no known vulnerabilities",
+					repoURL: "https://github.com/ossf-tests/scorecard-binauthz-test-bad",
+					policy: policy.AttestationPolicy{
+						PreventKnownVulnerabilities: true,
+					},
+					expected: policy.Pass,
+				},
+				{
+					name:    "test bad repo with ignored binary artifact",
+					repoURL: "https://github.com/ossf-tests/scorecard-binauthz-test-bad",
+					policy: policy.AttestationPolicy{
+						PreventBinaryArtifacts:      true,
+						AllowedBinaryArtifacts:      []string{"test-binary-artifact-*"},
+						PreventKnownVulnerabilities: true,
+					},
+					expected: policy.Pass,
+				},
+				{
+					name:    "test bad repo with ignored binary artifact",
+					repoURL: "https://github.com/ossf-tests/scorecard-binauthz-test-bad",
+					policy: policy.AttestationPolicy{
+						PreventBinaryArtifacts:      true,
+						PreventKnownVulnerabilities: true,
+					},
+					expected: policy.Fail,
+				},
+				{
+					name:    "test bad repo with ignored dep by path",
+					repoURL: "https://github.com/ossf-tests/scorecard-binauthz-test-bad",
+					policy: policy.AttestationPolicy{
+						PreventUnpinnedDependencies: true,
+						AllowedUnpinnedDependencies: []policy.Dependency{{Filepath: "Dockerfile"}},
+					},
+					expected: policy.Pass,
+				},
+				{
+					name:    "test bad repo without ignored dep",
+					repoURL: "https://github.com/ossf-tests/scorecard-binauthz-test-bad",
+					policy: policy.AttestationPolicy{
+						PreventUnpinnedDependencies: true,
+					},
+					expected: policy.Fail,
+				},
+				{
+					name:    "test bad repo with ignored dep by name",
+					repoURL: "https://github.com/ossf-tests/scorecard-binauthz-test-bad",
+					policy: policy.AttestationPolicy{
+						PreventUnpinnedDependencies: true,
+						AllowedUnpinnedDependencies: []policy.Dependency{{PackageName: "static-debian11"}, {PackageName: "golang"}},
+					},
+					expected: policy.Pass,
+				},
+				{
+					name:    "test bad repo with everything ignored",
+					repoURL: "https://github.com/ossf-tests/scorecard-binauthz-test-bad",
+					policy: policy.AttestationPolicy{
+						PreventBinaryArtifacts:      true,
+						AllowedBinaryArtifacts:      []string{"test-binary-artifact-*"},
+						PreventKnownVulnerabilities: true,
+						PreventUnpinnedDependencies: true,
+						AllowedUnpinnedDependencies: []policy.Dependency{{Filepath: "Dockerfile"}},
+					},
+					expected: policy.Pass,
+				},
+				{
+					name:    "test repo with simple code review requirements",
+					repoURL: "https://github.com/ossf/scorecard",
+					commit:  "fa0592fab28aa92560f04e1ae8649dfff566ae2b",
+					policy: policy.AttestationPolicy{
+						EnsureCodeReviewed: true,
+						CodeReviewRequirements: policy.CodeReviewRequirements{
+							MinReviewers: 1,
+						},
+					},
+					expected: policy.Pass,
+				},
+				{
+					name:    "test code reviews required but repo doesn't have code reviews",
+					repoURL: "https://github.com/ossf-tests/scorecard-binauthz-test-bad",
+					policy: policy.AttestationPolicy{
+						PreventBinaryArtifacts:      true,
+						PreventKnownVulnerabilities: true,
+						PreventUnpinnedDependencies: true,
+						EnsureCodeReviewed:          true,
+					},
+					expected: policy.Fail,
+				},
+				{
+					name:    "test code reviews required with min reviewers",
+					repoURL: "https://github.com/ossf/scorecard",
+					commit:  "fa0592fab28aa92560f04e1ae8649dfff566ae2b",
+					policy: policy.AttestationPolicy{
+						PreventBinaryArtifacts:      true,
+						PreventKnownVulnerabilities: true,
+						PreventUnpinnedDependencies: true,
+						EnsureCodeReviewed:          true,
+						CodeReviewRequirements: policy.CodeReviewRequirements{
+							MinReviewers: 1,
+						},
+					},
+					expected: policy.Pass,
+				},
+				{
+					name:    "test code reviews required with min reviewers and required reviewers",
+					repoURL: "https://github.com/ossf/scorecard",
+					commit:  "fa0592fab28aa92560f04e1ae8649dfff566ae2b",
+					policy: policy.AttestationPolicy{
+						PreventBinaryArtifacts:      true,
+						PreventKnownVulnerabilities: true,
+						PreventUnpinnedDependencies: true,
+						EnsureCodeReviewed:          true,
+						CodeReviewRequirements: policy.CodeReviewRequirements{
+							MinReviewers:      1,
+							RequiredApprovers: []string{"spencerschrock", "laurentsimon", "naveensrinivasan", "azeemshaikh38"},
+						},
+					},
+					expected: policy.Pass,
+				},
+				{
+					name:    "test code reviews required with too many min reviewers but matching required reviewers",
+					repoURL: "https://github.com/ossf/scorecard",
+					commit:  "fa0592fab28aa92560f04e1ae8649dfff566ae2b",
+					policy: policy.AttestationPolicy{
+						PreventBinaryArtifacts:      true,
+						PreventKnownVulnerabilities: true,
+						PreventUnpinnedDependencies: true,
+						EnsureCodeReviewed:          true,
+						CodeReviewRequirements: policy.CodeReviewRequirements{
+							MinReviewers:      2,
+							RequiredApprovers: []string{"spencerschrock", "laurentsimon", "naveensrinivasan", "azeemshaikh38"},
+						},
+					},
+					expected: policy.Fail,
+				},
+			}
+
+			for _, tc := range tt {
+				fmt.Printf("attestor_policy_test.go: %s\n", tc.name)
+				f, err := os.CreateTemp("/tmp", strings.ReplaceAll(tc.name, " ", "-"))
+				Expect(err).Should(BeNil())
+				defer os.Remove(f.Name())
+
+				buf, err := yaml.Marshal(tc.policy)
+				Expect(err).Should(BeNil())
+
+				nbytes, err := f.Write(buf)
+				Expect(err).Should(BeNil())
+				Expect(nbytes).Should(BeNumerically(">", 0))
+
+				result, err := command.RunCheckWithParams(tc.repoURL, tc.commit, f.Name())
+				Expect(err).Should(BeNil())
+				Expect(result).Should(BeEquivalentTo(tc.expected))
+			}
+		})
+	})
+})