Skip to content

Commit

Permalink
馃尡 attestor: e2e tests (#2529)
Browse files Browse the repository at this point in the history
* Add E2E tests

Signed-off-by: Raghav Kaul <raghavkaul@google.com>

* Change RequiredApprovers so that it checks for any reviewers on the list
instead of all

Signed-off-by: Raghav Kaul <raghavkaul@google.com>

Signed-off-by: Raghav Kaul <raghavkaul@google.com>
Co-authored-by: Azeem Shaikh <azeemshaikh38@gmail.com>
  • Loading branch information
raghavkaul and azeemshaikh38 committed Dec 12, 2022
1 parent 6107786 commit 7206a2b
Show file tree
Hide file tree
Showing 5 changed files with 264 additions and 12 deletions.
2 changes: 1 addition & 1 deletion attestor/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
5 changes: 5 additions & 0 deletions attestor/command/check.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
21 changes: 11 additions & 10 deletions attestor/policy/attestation_policy.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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)))
}
Expand All @@ -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
}
}

Expand Down
42 changes: 41 additions & 1 deletion attestor/policy/attestation_policy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand All @@ -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{
Expand Down
206 changes: 206 additions & 0 deletions e2e/attestor_policy_test.go
Original file line number Diff line number Diff line change
@@ -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))
}
})
})
})

0 comments on commit 7206a2b

Please sign in to comment.