[migration to score] 3: branch protection, frozen-deps, token permissions (#719)

* details-1

* nits

* typo

* commments

* dependabot and binary artifacts checks

* typo

* linter

* missing errors.go

* linter

* merge fix

* branch protection, frozen-deps, token permissions

* linter

* linter
This commit is contained in:
laurentsimon 2021-07-21 09:21:43 -07:00 committed by GitHub
parent 5e634c8945
commit c741335683
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
11 changed files with 1051 additions and 1036 deletions

View File

@ -16,18 +16,18 @@ package checks
import (
"context"
"errors"
"fmt"
"regexp"
"github.com/google/go-github/v32/github"
"github.com/ossf/scorecard/checker"
sce "github.com/ossf/scorecard/errors"
)
const (
// CheckBranchProtection is the registered name for BranchProtection.
CheckBranchProtection = "Branch-Protection"
minReviews = 1
minReviews = 2
)
//nolint:gochecknoinits
@ -46,31 +46,23 @@ type repositories interface {
*github.Protection, *github.Response, error)
}
type logger func(s string, f ...interface{})
// ErrCommitishNil TargetCommitish nil for release.
var ErrCommitishNil = errors.New("target_commitish is nil for release")
// ErrBranchNotFound branch from TargetCommitish not found.
var ErrBranchNotFound = errors.New("branch not found")
func BranchProtection(c *checker.CheckRequest) checker.CheckResult {
// Checks branch protection on both release and development branch
return checkReleaseAndDevBranchProtection(c.Ctx, c.Client.Repositories, c.Logf, c.Owner, c.Repo)
// Checks branch protection on both release and development branch.
return checkReleaseAndDevBranchProtection(c.Ctx, c.Client.Repositories, c.Dlogger, c.Owner, c.Repo)
}
func checkReleaseAndDevBranchProtection(ctx context.Context, r repositories, l logger, ownerStr,
func checkReleaseAndDevBranchProtection(ctx context.Context, r repositories, dl checker.DetailLogger, ownerStr,
repoStr string) checker.CheckResult {
// Get all branches. This will include information on whether they are protected.
branches, _, err := r.ListBranches(ctx, ownerStr, repoStr, &github.BranchListOptions{})
if err != nil {
return checker.MakeRetryResult(CheckBranchProtection, err)
return checker.CreateRuntimeErrorResult(CheckBranchProtection, err)
}
// Get release branches
releases, _, err := r.ListReleases(ctx, ownerStr, repoStr, &github.ListOptions{})
if err != nil {
return checker.MakeRetryResult(CheckBranchProtection, err)
return checker.CreateRuntimeErrorResult(CheckBranchProtection, err)
}
var checks []checker.CheckResult
@ -79,7 +71,9 @@ func checkReleaseAndDevBranchProtection(ctx context.Context, r repositories, l l
for _, release := range releases {
if release.TargetCommitish == nil {
// Log with a named error if target_commitish is nil.
checks = append(checks, checker.MakeFailResult(CheckBranchProtection, ErrCommitishNil))
e := sce.Create(sce.ErrScorecardInternal, errInternalCommitishNil.Error())
r := checker.CreateRuntimeErrorResult(CheckBranchProtection, e)
checks = append(checks, r)
continue
}
@ -92,7 +86,9 @@ func checkReleaseAndDevBranchProtection(ctx context.Context, r repositories, l l
name, err := resolveBranchName(branches, *release.TargetCommitish)
if err != nil {
// If the commitish branch is still not found, fail.
checks = append(checks, checker.MakeFailResult(CheckBranchProtection, ErrBranchNotFound))
e := sce.Create(sce.ErrScorecardInternal, errInternalBranchNotFound.Error())
r := checker.CreateRuntimeErrorResult(CheckBranchProtection, e)
checks = append(checks, r)
continue
}
@ -100,10 +96,10 @@ func checkReleaseAndDevBranchProtection(ctx context.Context, r repositories, l l
checkBranches[*name] = true
}
// Add default branch
// Add default branch.
repo, _, err := r.Get(ctx, ownerStr, repoStr)
if err != nil {
return checker.MakeRetryResult(CheckBranchProtection, err)
return checker.CreateRuntimeErrorResult(CheckBranchProtection, err)
}
checkBranches[*repo.DefaultBranch] = true
@ -111,23 +107,22 @@ func checkReleaseAndDevBranchProtection(ctx context.Context, r repositories, l l
for b := range checkBranches {
protected, err := isBranchProtected(branches, b)
if err != nil {
checks = append(checks, checker.MakeFailResult(CheckBranchProtection, ErrBranchNotFound))
e := sce.Create(sce.ErrScorecardInternal, errInternalBranchNotFound.Error())
r := checker.CreateRuntimeErrorResult(CheckBranchProtection, e)
checks = append(checks, r)
}
if !protected {
l("!! branch protection not enabled for branch %s", b)
checks = append(checks, checker.CheckResult{
Name: CheckBranchProtection,
Pass: false,
Confidence: checker.MaxResultConfidence,
})
r := checker.CreateMinScoreResult(CheckBranchProtection,
fmt.Sprintf("branch protection not enabled for branch '%s'", b))
checks = append(checks, r)
} else {
// The branch is protected. Check the protection.
res := getProtectionAndCheck(ctx, r, l, ownerStr, repoStr, b)
res := getProtectionAndCheck(ctx, r, dl, ownerStr, repoStr, b)
checks = append(checks, res)
}
}
return checker.MakeAndResult(checks...)
return checker.MakeAndResult2(checks...)
}
func resolveBranchName(branches []*github.Branch, name string) (*string, error) {
@ -144,7 +139,8 @@ func resolveBranchName(branches []*github.Branch, name string) (*string, error)
return resolveBranchName(branches, "main")
}
return nil, ErrBranchNotFound
//nolint
return nil, sce.Create(sce.ErrScorecardInternal, errInternalBranchNotFound.Error())
}
func isBranchProtected(branches []*github.Branch, name string) (bool, error) {
@ -154,71 +150,79 @@ func isBranchProtected(branches []*github.Branch, name string) (bool, error) {
return b.GetProtected(), nil
}
}
return false, ErrBranchNotFound
//nolint
return false, sce.Create(sce.ErrScorecardInternal, errInternalBranchNotFound.Error())
}
func getProtectionAndCheck(ctx context.Context, r repositories, l logger, ownerStr, repoStr,
func getProtectionAndCheck(ctx context.Context, r repositories, dl checker.DetailLogger, ownerStr, repoStr,
branch string) checker.CheckResult {
// We only call this if the branch is protected. An error indicates not found.
protection, resp, err := r.GetBranchProtection(ctx, ownerStr, repoStr, branch)
const fileNotFound = 404
if resp.StatusCode == fileNotFound {
return checker.MakeRetryResult(CheckBranchProtection, err)
e := sce.Create(sce.ErrScorecardInternal, err.Error())
return checker.CreateRuntimeErrorResult(CheckBranchProtection, e)
}
return IsBranchProtected(protection, branch, l)
return IsBranchProtected(protection, branch, dl)
}
func IsBranchProtected(protection *github.Protection, branch string, l logger) checker.CheckResult {
totalChecks := 6
func IsBranchProtected(protection *github.Protection, branch string, dl checker.DetailLogger) checker.CheckResult {
totalChecks := 10
totalSuccess := 0
// This is disabled by default (good).
if protection.GetAllowForcePushes() != nil &&
protection.AllowForcePushes.Enabled {
l("!! branch protection - AllowForcePushes should be disabled on %s", branch)
dl.Warn("AllowForcePushes enabled on branch '%s'", branch)
} else {
dl.Info("AllowForcePushes disabled on branch '%s'", branch)
totalSuccess++
}
// This is disabled by default (good).
if protection.GetAllowDeletions() != nil &&
protection.AllowDeletions.Enabled {
l("!! branch protection - AllowDeletions should be disabled on %s", branch)
dl.Warn("AllowDeletions enabled on branch '%s'", branch)
} else {
dl.Info("AllowDeletions disabled on branch '%s'", branch)
totalSuccess++
}
// This is disabled by default (bad).
if protection.GetEnforceAdmins() != nil &&
protection.EnforceAdmins.Enabled {
dl.Info("EnforceAdmins disabled on branch '%s'", branch)
totalSuccess++
} else {
l("!! branch protection - EnforceAdmins should be enabled on %s", branch)
dl.Warn("EnforceAdmins disabled on branch '%s'", branch)
}
// This is disabled by default (bad).
if protection.GetRequireLinearHistory() != nil &&
protection.RequireLinearHistory.Enabled {
dl.Info("Linear history enabled on branch '%s'", branch)
totalSuccess++
} else {
l("!! branch protection - Linear history should be enabled on %s", branch)
dl.Warn("Linear history disabled on branch '%s'", branch)
}
if requiresStatusChecks(protection, branch, l) {
if requiresStatusChecks(protection, branch, dl) {
dl.Info("Strict status check enabled on branch '%s'", branch)
totalSuccess++
}
if requiresThoroughReviews(protection, branch, l) {
if requiresThoroughReviews(protection, branch, dl) {
totalSuccess++
}
return checker.MakeProportionalResult(CheckBranchProtection, totalSuccess, totalChecks, 1.0)
return checker.CreateProportionalScoreResult(CheckBranchProtection,
"%d out of %d branch protection settings are enabled", totalSuccess, totalChecks)
}
// Returns true if several PR status checks requirements are enabled. Otherwise returns false and logs why it failed.
func requiresStatusChecks(protection *github.Protection, branch string, l logger) bool {
func requiresStatusChecks(protection *github.Protection, branch string, dl checker.DetailLogger) bool {
// This is disabled by default (bad).
if protection.GetRequiredStatusChecks() != nil &&
protection.RequiredStatusChecks.Strict &&
@ -228,17 +232,17 @@ func requiresStatusChecks(protection *github.Protection, branch string, l logger
switch {
case protection.RequiredStatusChecks == nil ||
!protection.RequiredStatusChecks.Strict:
l("!! branch protection - Status checks for merging should be enabled on %s", branch)
dl.Warn("Status checks for merging disabled on branch '%s'", branch)
case len(protection.RequiredStatusChecks.Contexts) == 0:
l("!! branch protection - Status checks for merging should have specific status to check for on %s", branch)
dl.Warn("Status checks for merging have no specific status to check on branch '%s'", branch)
default:
panic("!! branch protection - Unhandled status checks error")
panic("Unhandled status checks error")
}
return false
}
// Returns true if several PR review requirements are enabled. Otherwise returns false and logs why it failed.
func requiresThoroughReviews(protection *github.Protection, branch string, l logger) bool {
func requiresThoroughReviews(protection *github.Protection, branch string, dl checker.DetailLogger) bool {
// This is disabled by default (bad).
if protection.GetRequiredPullRequestReviews() != nil &&
protection.RequiredPullRequestReviews.RequiredApprovingReviewCount >= minReviews &&
@ -248,18 +252,19 @@ func requiresThoroughReviews(protection *github.Protection, branch string, l log
}
switch {
case protection.RequiredPullRequestReviews == nil:
l("!! branch protection - Pullrequest reviews should be enabled on %s", branch)
dl.Warn("Pullrequest reviews disabled on branch '%s'", branch)
fallthrough
case protection.RequiredPullRequestReviews.RequiredApprovingReviewCount < minReviews:
l("!! branch protection - %v pullrequest reviews should be enabled on %s", minReviews, branch)
dl.Warn("Number of required reviewers is only %d on branch '%s'",
protection.RequiredPullRequestReviews.RequiredApprovingReviewCount, branch)
fallthrough
case !protection.RequiredPullRequestReviews.DismissStaleReviews:
l("!! branch protection - Stale review dismissal should be enabled on %s", branch)
dl.Warn("Stale review dismissal disabled on branch '%s'", branch)
fallthrough
case !protection.RequiredPullRequestReviews.RequireCodeOwnerReviews:
l("!! branch protection - Owner review should be enabled on %s", branch)
dl.Warn("Owner review not required on branch '%s'", branch)
default:
panic("!! branch protection - Unhandled pull request error")
panic("Unhandled pull request error")
}
return false
}

View File

@ -16,24 +16,16 @@ package checks
import (
"context"
"fmt"
"net/http"
"testing"
"github.com/google/go-github/v32/github"
"github.com/ossf/scorecard/checker"
sce "github.com/ossf/scorecard/errors"
scut "github.com/ossf/scorecard/utests"
)
// TODO: these logging functions are repeated from lib/check_fn.go. Reuse code.
type log struct {
messages []string
}
func (l *log) Logf(s string, f ...interface{}) {
l.messages = append(l.messages, fmt.Sprintf(s, f...))
}
type mockRepos struct {
branches []*string
protections map[string]*github.Protection
@ -68,7 +60,8 @@ func (m mockRepos) GetBranchProtection(ctx context.Context, o string, r string,
return nil, &github.Response{
Response: &http.Response{StatusCode: http.StatusNotFound},
},
ErrBranchNotFound
//nolint
sce.Create(sce.ErrScorecardInternal, errInternalBranchNotFound.Error())
}
func (m mockRepos) ListBranches(ctx context.Context, owner string, repo string,
@ -81,9 +74,8 @@ func (m mockRepos) ListBranches(ctx context.Context, owner string, repo string,
return res, nil, nil
}
func TestReleaseAndDevBranchProtected(t *testing.T) { //nolint:tparallel // mocks return different results per test case
func TestReleaseAndDevBranchProtected(t *testing.T) {
t.Parallel()
l := log{}
rel1 := "release/v.1"
sha := "8fb3cb86082b17144a80402f5367ae65f06083bd"
@ -91,14 +83,21 @@ func TestReleaseAndDevBranchProtected(t *testing.T) { //nolint:tparallel // mock
//nolint
tests := []struct {
name string
expected scut.TestReturn
branches []*string
defaultBranch *string
releases []*string
protections map[string]*github.Protection
want checker.CheckResult
}{
{
name: "Only development branch",
name: "Only development branch",
expected: scut.TestReturn{
Errors: nil,
Score: 2,
NumberOfWarn: 6,
NumberOfInfo: 2,
NumberOfDebug: 0,
},
defaultBranch: &main,
branches: []*string{&rel1, &main},
releases: nil,
@ -137,17 +136,16 @@ func TestReleaseAndDevBranchProtected(t *testing.T) { //nolint:tparallel // mock
},
},
},
want: checker.CheckResult{
Name: CheckBranchProtection,
Pass: false,
Details: nil,
Confidence: 7,
ShouldRetry: false,
Error: nil,
},
},
{
name: "Take worst of release and development",
name: "Take worst of release and development",
expected: scut.TestReturn{
Errors: nil,
Score: 2,
NumberOfWarn: 9,
NumberOfInfo: 7,
NumberOfDebug: 0,
},
defaultBranch: &main,
branches: []*string{&rel1, &main},
releases: []*string{&rel1},
@ -219,17 +217,16 @@ func TestReleaseAndDevBranchProtected(t *testing.T) { //nolint:tparallel // mock
},
},
},
want: checker.CheckResult{
Name: CheckBranchProtection,
Pass: false,
Details: nil,
Confidence: 7,
ShouldRetry: false,
Error: nil,
},
},
{
name: "Both release and development are OK",
name: "Both release and development are OK",
expected: scut.TestReturn{
Errors: nil,
Score: 5,
NumberOfWarn: 6,
NumberOfInfo: 10,
NumberOfDebug: 0,
},
defaultBranch: &main,
branches: []*string{&rel1, &main},
releases: []*string{&rel1},
@ -301,17 +298,16 @@ func TestReleaseAndDevBranchProtected(t *testing.T) { //nolint:tparallel // mock
},
},
},
want: checker.CheckResult{
Name: CheckBranchProtection,
Pass: true,
Details: nil,
Confidence: 10,
ShouldRetry: false,
Error: nil,
},
},
{
name: "Ignore a non-branch targetcommitish",
name: "Ignore a non-branch targetcommitish",
expected: scut.TestReturn{
Errors: nil,
Score: 2,
NumberOfWarn: 6,
NumberOfInfo: 2,
NumberOfDebug: 0,
},
defaultBranch: &main,
branches: []*string{&rel1, &main},
releases: []*string{&sha},
@ -350,17 +346,16 @@ func TestReleaseAndDevBranchProtected(t *testing.T) { //nolint:tparallel // mock
},
},
},
want: checker.CheckResult{
Name: CheckBranchProtection,
Pass: false,
Details: nil,
Confidence: 7,
ShouldRetry: false,
Error: nil,
},
},
{
name: "TargetCommittish nil",
name: "TargetCommittish nil",
expected: scut.TestReturn{
Errors: []error{sce.ErrScorecardInternal},
Score: checker.InconclusiveResultScore,
NumberOfWarn: 6,
NumberOfInfo: 2,
NumberOfDebug: 0,
},
defaultBranch: &main,
branches: []*string{&main},
releases: []*string{nil},
@ -399,476 +394,430 @@ func TestReleaseAndDevBranchProtected(t *testing.T) { //nolint:tparallel // mock
},
},
},
want: checker.CheckResult{
Name: CheckBranchProtection,
Pass: false,
Details: nil,
Confidence: 10,
ShouldRetry: false,
Error: ErrCommitishNil,
},
},
}
for _, tt := range tests { //nolint:paralleltest // mocks return different results per test case
for _, tt := range tests {
tt := tt // Re-initializing variable so it is not changed while executing the closure below
l.messages = []string{}
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
m := mockRepos{
defaultBranch: tt.defaultBranch,
branches: tt.branches,
releases: tt.releases,
protections: tt.protections,
}
got := checkReleaseAndDevBranchProtection(context.Background(), m,
l.Logf, "testowner", "testrepo")
got.Details = l.messages
if got.Confidence != tt.want.Confidence || got.Pass != tt.want.Pass {
t.Errorf("IsBranchProtected() = %s, %v, want %v", tt.name, got, tt.want)
}
dl := scut.TestDetailLogger{}
r := checkReleaseAndDevBranchProtection(context.Background(), m,
&dl, "testowner", "testrepo")
scut.ValidateTestReturn(t, tt.name, &tt.expected, &r, &dl)
})
}
}
func TestIsBranchProtected(t *testing.T) {
t.Parallel()
type args struct {
protection *github.Protection
}
l := log{}
tests := []struct {
name string
args args
want checker.CheckResult
name string
protection *github.Protection
expected scut.TestReturn
}{
{
name: "Nothing is enabled",
args: args{
protection: &github.Protection{
RequiredStatusChecks: &github.RequiredStatusChecks{
Strict: false,
Contexts: nil,
},
RequiredPullRequestReviews: &github.PullRequestReviewsEnforcement{
DismissalRestrictions: &github.DismissalRestrictions{
Users: nil,
Teams: nil,
},
DismissStaleReviews: false,
RequireCodeOwnerReviews: false,
RequiredApprovingReviewCount: 0,
},
EnforceAdmins: &github.AdminEnforcement{
URL: nil,
Enabled: false,
},
Restrictions: &github.BranchRestrictions{
expected: scut.TestReturn{
Errors: nil,
Score: 2,
NumberOfWarn: 6,
NumberOfInfo: 2,
NumberOfDebug: 0,
},
protection: &github.Protection{
RequiredStatusChecks: &github.RequiredStatusChecks{
Strict: false,
Contexts: nil,
},
RequiredPullRequestReviews: &github.PullRequestReviewsEnforcement{
DismissalRestrictions: &github.DismissalRestrictions{
Users: nil,
Teams: nil,
Apps: nil,
},
RequireLinearHistory: &github.RequireLinearHistory{
Enabled: false,
},
AllowForcePushes: &github.AllowForcePushes{
Enabled: false,
},
AllowDeletions: &github.AllowDeletions{
Enabled: false,
},
DismissStaleReviews: false,
RequireCodeOwnerReviews: false,
RequiredApprovingReviewCount: 0,
},
EnforceAdmins: &github.AdminEnforcement{
URL: nil,
Enabled: false,
},
Restrictions: &github.BranchRestrictions{
Users: nil,
Teams: nil,
Apps: nil,
},
RequireLinearHistory: &github.RequireLinearHistory{
Enabled: false,
},
AllowForcePushes: &github.AllowForcePushes{
Enabled: false,
},
AllowDeletions: &github.AllowDeletions{
Enabled: false,
},
},
want: checker.CheckResult{
Name: CheckBranchProtection,
Pass: false,
Details: nil,
Confidence: 7,
ShouldRetry: false,
Error: nil,
},
},
{
name: "Required status check enabled",
args: args{
protection: &github.Protection{
RequiredStatusChecks: &github.RequiredStatusChecks{
Strict: true,
Contexts: []string{"foo"},
},
RequiredPullRequestReviews: &github.PullRequestReviewsEnforcement{
DismissalRestrictions: &github.DismissalRestrictions{
Users: nil,
Teams: nil,
},
DismissStaleReviews: false,
RequireCodeOwnerReviews: false,
RequiredApprovingReviewCount: 0,
},
EnforceAdmins: &github.AdminEnforcement{
URL: nil,
Enabled: false,
},
Restrictions: &github.BranchRestrictions{
expected: scut.TestReturn{
Errors: nil,
Score: 3,
NumberOfWarn: 5,
NumberOfInfo: 3,
NumberOfDebug: 0,
},
protection: &github.Protection{
RequiredStatusChecks: &github.RequiredStatusChecks{
Strict: true,
Contexts: []string{"foo"},
},
RequiredPullRequestReviews: &github.PullRequestReviewsEnforcement{
DismissalRestrictions: &github.DismissalRestrictions{
Users: nil,
Teams: nil,
Apps: nil,
},
RequireLinearHistory: &github.RequireLinearHistory{
Enabled: false,
},
AllowForcePushes: &github.AllowForcePushes{
Enabled: false,
},
AllowDeletions: &github.AllowDeletions{
Enabled: false,
},
DismissStaleReviews: false,
RequireCodeOwnerReviews: false,
RequiredApprovingReviewCount: 0,
},
EnforceAdmins: &github.AdminEnforcement{
URL: nil,
Enabled: false,
},
Restrictions: &github.BranchRestrictions{
Users: nil,
Teams: nil,
Apps: nil,
},
RequireLinearHistory: &github.RequireLinearHistory{
Enabled: false,
},
AllowForcePushes: &github.AllowForcePushes{
Enabled: false,
},
AllowDeletions: &github.AllowDeletions{
Enabled: false,
},
},
want: checker.CheckResult{
Name: CheckBranchProtection,
Pass: false,
Details: nil,
Confidence: 5,
ShouldRetry: false,
Error: nil,
},
},
{
name: "Required status check enabled without checking for status string",
args: args{
protection: &github.Protection{
RequiredStatusChecks: &github.RequiredStatusChecks{
Strict: true,
Contexts: nil,
},
RequiredPullRequestReviews: &github.PullRequestReviewsEnforcement{
DismissalRestrictions: &github.DismissalRestrictions{
Users: nil,
Teams: nil,
},
DismissStaleReviews: false,
RequireCodeOwnerReviews: false,
RequiredApprovingReviewCount: 0,
},
EnforceAdmins: &github.AdminEnforcement{
URL: nil,
Enabled: false,
},
Restrictions: &github.BranchRestrictions{
expected: scut.TestReturn{
Errors: nil,
Score: 2,
NumberOfWarn: 6,
NumberOfInfo: 2,
NumberOfDebug: 0,
},
protection: &github.Protection{
RequiredStatusChecks: &github.RequiredStatusChecks{
Strict: true,
Contexts: nil,
},
RequiredPullRequestReviews: &github.PullRequestReviewsEnforcement{
DismissalRestrictions: &github.DismissalRestrictions{
Users: nil,
Teams: nil,
Apps: nil,
},
RequireLinearHistory: &github.RequireLinearHistory{
Enabled: false,
},
AllowForcePushes: &github.AllowForcePushes{
Enabled: false,
},
AllowDeletions: &github.AllowDeletions{
Enabled: false,
},
DismissStaleReviews: false,
RequireCodeOwnerReviews: false,
RequiredApprovingReviewCount: 0,
},
EnforceAdmins: &github.AdminEnforcement{
URL: nil,
Enabled: false,
},
Restrictions: &github.BranchRestrictions{
Users: nil,
Teams: nil,
Apps: nil,
},
RequireLinearHistory: &github.RequireLinearHistory{
Enabled: false,
},
AllowForcePushes: &github.AllowForcePushes{
Enabled: false,
},
AllowDeletions: &github.AllowDeletions{
Enabled: false,
},
},
want: checker.CheckResult{
Name: CheckBranchProtection,
Pass: false,
Details: nil,
Confidence: 7,
ShouldRetry: false,
Error: nil,
},
},
{
name: "Required pull request enabled",
args: args{
protection: &github.Protection{
RequiredStatusChecks: &github.RequiredStatusChecks{
Strict: false,
Contexts: []string{"foo"},
},
RequiredPullRequestReviews: &github.PullRequestReviewsEnforcement{
DismissalRestrictions: &github.DismissalRestrictions{
Users: nil,
Teams: nil,
},
DismissStaleReviews: false,
RequireCodeOwnerReviews: false,
RequiredApprovingReviewCount: 1,
},
EnforceAdmins: &github.AdminEnforcement{
URL: nil,
Enabled: false,
},
Restrictions: &github.BranchRestrictions{
expected: scut.TestReturn{
Errors: nil,
Score: 3,
NumberOfWarn: 5,
NumberOfInfo: 3,
NumberOfDebug: 0,
},
protection: &github.Protection{
RequiredStatusChecks: &github.RequiredStatusChecks{
Strict: false,
Contexts: []string{"foo"},
},
RequiredPullRequestReviews: &github.PullRequestReviewsEnforcement{
DismissalRestrictions: &github.DismissalRestrictions{
Users: nil,
Teams: nil,
Apps: nil,
},
RequireLinearHistory: &github.RequireLinearHistory{
Enabled: true,
},
AllowForcePushes: &github.AllowForcePushes{
Enabled: false,
},
AllowDeletions: &github.AllowDeletions{
Enabled: false,
},
DismissStaleReviews: false,
RequireCodeOwnerReviews: false,
RequiredApprovingReviewCount: 1,
},
EnforceAdmins: &github.AdminEnforcement{
URL: nil,
Enabled: false,
},
Restrictions: &github.BranchRestrictions{
Users: nil,
Teams: nil,
Apps: nil,
},
RequireLinearHistory: &github.RequireLinearHistory{
Enabled: true,
},
AllowForcePushes: &github.AllowForcePushes{
Enabled: false,
},
AllowDeletions: &github.AllowDeletions{
Enabled: false,
},
},
want: checker.CheckResult{
Name: CheckBranchProtection,
Pass: false,
Details: nil,
Confidence: 5,
ShouldRetry: false,
Error: nil,
},
},
{
name: "Required admin enforcement enabled",
args: args{
protection: &github.Protection{
RequiredStatusChecks: &github.RequiredStatusChecks{
Strict: false,
Contexts: []string{"foo"},
},
RequiredPullRequestReviews: &github.PullRequestReviewsEnforcement{
DismissalRestrictions: &github.DismissalRestrictions{
Users: nil,
Teams: nil,
},
DismissStaleReviews: false,
RequireCodeOwnerReviews: false,
RequiredApprovingReviewCount: 0,
},
EnforceAdmins: &github.AdminEnforcement{
URL: nil,
Enabled: true,
},
Restrictions: &github.BranchRestrictions{
expected: scut.TestReturn{
Errors: nil,
Score: 3,
NumberOfWarn: 5,
NumberOfInfo: 3,
NumberOfDebug: 0,
},
protection: &github.Protection{
RequiredStatusChecks: &github.RequiredStatusChecks{
Strict: false,
Contexts: []string{"foo"},
},
RequiredPullRequestReviews: &github.PullRequestReviewsEnforcement{
DismissalRestrictions: &github.DismissalRestrictions{
Users: nil,
Teams: nil,
Apps: nil,
},
RequireLinearHistory: &github.RequireLinearHistory{
Enabled: false,
},
AllowForcePushes: &github.AllowForcePushes{
Enabled: false,
},
AllowDeletions: &github.AllowDeletions{
Enabled: false,
},
DismissStaleReviews: false,
RequireCodeOwnerReviews: false,
RequiredApprovingReviewCount: 0,
},
EnforceAdmins: &github.AdminEnforcement{
URL: nil,
Enabled: true,
},
Restrictions: &github.BranchRestrictions{
Users: nil,
Teams: nil,
Apps: nil,
},
RequireLinearHistory: &github.RequireLinearHistory{
Enabled: false,
},
AllowForcePushes: &github.AllowForcePushes{
Enabled: false,
},
AllowDeletions: &github.AllowDeletions{
Enabled: false,
},
},
want: checker.CheckResult{
Name: CheckBranchProtection,
Pass: false,
Details: nil,
Confidence: 5,
ShouldRetry: false,
Error: nil,
},
},
{
name: "Required linear history enabled",
args: args{
protection: &github.Protection{
RequiredStatusChecks: &github.RequiredStatusChecks{
Strict: false,
Contexts: []string{"foo"},
},
RequiredPullRequestReviews: &github.PullRequestReviewsEnforcement{
DismissalRestrictions: &github.DismissalRestrictions{
Users: nil,
Teams: nil,
},
DismissStaleReviews: false,
RequireCodeOwnerReviews: false,
RequiredApprovingReviewCount: 0,
},
EnforceAdmins: &github.AdminEnforcement{
URL: nil,
Enabled: false,
},
Restrictions: &github.BranchRestrictions{
expected: scut.TestReturn{
Errors: nil,
Score: 3,
NumberOfWarn: 5,
NumberOfInfo: 3,
NumberOfDebug: 0,
},
protection: &github.Protection{
RequiredStatusChecks: &github.RequiredStatusChecks{
Strict: false,
Contexts: []string{"foo"},
},
RequiredPullRequestReviews: &github.PullRequestReviewsEnforcement{
DismissalRestrictions: &github.DismissalRestrictions{
Users: nil,
Teams: nil,
Apps: nil,
},
RequireLinearHistory: &github.RequireLinearHistory{
Enabled: true,
},
AllowForcePushes: &github.AllowForcePushes{
Enabled: false,
},
AllowDeletions: &github.AllowDeletions{
Enabled: false,
},
DismissStaleReviews: false,
RequireCodeOwnerReviews: false,
RequiredApprovingReviewCount: 0,
},
EnforceAdmins: &github.AdminEnforcement{
URL: nil,
Enabled: false,
},
Restrictions: &github.BranchRestrictions{
Users: nil,
Teams: nil,
Apps: nil,
},
RequireLinearHistory: &github.RequireLinearHistory{
Enabled: true,
},
AllowForcePushes: &github.AllowForcePushes{
Enabled: false,
},
AllowDeletions: &github.AllowDeletions{
Enabled: false,
},
},
want: checker.CheckResult{
Name: CheckBranchProtection,
Pass: false,
Details: nil,
Confidence: 5,
ShouldRetry: false,
Error: nil,
},
},
{
name: "Allow force push enabled",
args: args{
protection: &github.Protection{
RequiredStatusChecks: &github.RequiredStatusChecks{
Strict: false,
Contexts: []string{"foo"},
},
RequiredPullRequestReviews: &github.PullRequestReviewsEnforcement{
DismissalRestrictions: &github.DismissalRestrictions{
Users: nil,
Teams: nil,
},
DismissStaleReviews: false,
RequireCodeOwnerReviews: false,
RequiredApprovingReviewCount: 0,
},
EnforceAdmins: &github.AdminEnforcement{
URL: nil,
Enabled: false,
},
Restrictions: &github.BranchRestrictions{
expected: scut.TestReturn{
Errors: nil,
Score: 1,
NumberOfWarn: 7,
NumberOfInfo: 1,
NumberOfDebug: 0,
},
protection: &github.Protection{
RequiredStatusChecks: &github.RequiredStatusChecks{
Strict: false,
Contexts: []string{"foo"},
},
RequiredPullRequestReviews: &github.PullRequestReviewsEnforcement{
DismissalRestrictions: &github.DismissalRestrictions{
Users: nil,
Teams: nil,
Apps: nil,
},
RequireLinearHistory: &github.RequireLinearHistory{
Enabled: false,
},
AllowForcePushes: &github.AllowForcePushes{
Enabled: true,
},
AllowDeletions: &github.AllowDeletions{
Enabled: false,
},
DismissStaleReviews: false,
RequireCodeOwnerReviews: false,
RequiredApprovingReviewCount: 0,
},
EnforceAdmins: &github.AdminEnforcement{
URL: nil,
Enabled: false,
},
Restrictions: &github.BranchRestrictions{
Users: nil,
Teams: nil,
Apps: nil,
},
RequireLinearHistory: &github.RequireLinearHistory{
Enabled: false,
},
AllowForcePushes: &github.AllowForcePushes{
Enabled: true,
},
AllowDeletions: &github.AllowDeletions{
Enabled: false,
},
},
want: checker.CheckResult{
Name: CheckBranchProtection,
Pass: false,
Details: nil,
Confidence: 9,
ShouldRetry: false,
Error: nil,
},
},
{
name: "Allow deletions enabled",
args: args{
protection: &github.Protection{
RequiredStatusChecks: &github.RequiredStatusChecks{
Strict: false,
Contexts: []string{"foo"},
},
RequiredPullRequestReviews: &github.PullRequestReviewsEnforcement{
DismissalRestrictions: &github.DismissalRestrictions{
Users: nil,
Teams: nil,
},
DismissStaleReviews: false,
RequireCodeOwnerReviews: false,
RequiredApprovingReviewCount: 0,
},
EnforceAdmins: &github.AdminEnforcement{
URL: nil,
Enabled: false,
},
Restrictions: &github.BranchRestrictions{
expected: scut.TestReturn{
Errors: nil,
Score: 1,
NumberOfWarn: 7,
NumberOfInfo: 1,
NumberOfDebug: 0,
},
protection: &github.Protection{
RequiredStatusChecks: &github.RequiredStatusChecks{
Strict: false,
Contexts: []string{"foo"},
},
RequiredPullRequestReviews: &github.PullRequestReviewsEnforcement{
DismissalRestrictions: &github.DismissalRestrictions{
Users: nil,
Teams: nil,
Apps: nil,
},
RequireLinearHistory: &github.RequireLinearHistory{
Enabled: false,
},
AllowForcePushes: &github.AllowForcePushes{
Enabled: false,
},
AllowDeletions: &github.AllowDeletions{
Enabled: true,
},
DismissStaleReviews: false,
RequireCodeOwnerReviews: false,
RequiredApprovingReviewCount: 0,
},
EnforceAdmins: &github.AdminEnforcement{
URL: nil,
Enabled: false,
},
Restrictions: &github.BranchRestrictions{
Users: nil,
Teams: nil,
Apps: nil,
},
RequireLinearHistory: &github.RequireLinearHistory{
Enabled: false,
},
AllowForcePushes: &github.AllowForcePushes{
Enabled: false,
},
AllowDeletions: &github.AllowDeletions{
Enabled: true,
},
},
want: checker.CheckResult{
Name: CheckBranchProtection,
Pass: false,
Details: nil,
Confidence: 9,
ShouldRetry: false,
Error: nil,
},
},
{
name: "Branches are protected",
args: args{
protection: &github.Protection{
RequiredStatusChecks: &github.RequiredStatusChecks{
Strict: true,
Contexts: []string{"foo"},
},
RequiredPullRequestReviews: &github.PullRequestReviewsEnforcement{
DismissalRestrictions: &github.DismissalRestrictions{
Users: nil,
Teams: nil,
},
DismissStaleReviews: true,
RequireCodeOwnerReviews: true,
RequiredApprovingReviewCount: 1,
},
EnforceAdmins: &github.AdminEnforcement{
URL: nil,
Enabled: true,
},
Restrictions: &github.BranchRestrictions{
expected: scut.TestReturn{
Errors: nil,
Score: 5,
NumberOfWarn: 3,
NumberOfInfo: 5,
NumberOfDebug: 0,
},
protection: &github.Protection{
RequiredStatusChecks: &github.RequiredStatusChecks{
Strict: true,
Contexts: []string{"foo"},
},
RequiredPullRequestReviews: &github.PullRequestReviewsEnforcement{
DismissalRestrictions: &github.DismissalRestrictions{
Users: nil,
Teams: nil,
Apps: nil,
},
RequireLinearHistory: &github.RequireLinearHistory{
Enabled: true,
},
AllowForcePushes: &github.AllowForcePushes{
Enabled: false,
},
AllowDeletions: &github.AllowDeletions{
Enabled: false,
},
DismissStaleReviews: true,
RequireCodeOwnerReviews: true,
RequiredApprovingReviewCount: 1,
},
EnforceAdmins: &github.AdminEnforcement{
URL: nil,
Enabled: true,
},
Restrictions: &github.BranchRestrictions{
Users: nil,
Teams: nil,
Apps: nil,
},
RequireLinearHistory: &github.RequireLinearHistory{
Enabled: true,
},
AllowForcePushes: &github.AllowForcePushes{
Enabled: false,
},
AllowDeletions: &github.AllowDeletions{
Enabled: false,
},
},
want: checker.CheckResult{
Name: CheckBranchProtection,
Pass: true,
Details: nil,
Confidence: 10,
ShouldRetry: false,
Error: nil,
},
},
}
for _, tt := range tests {
tt := tt // Re-initializing variable so it is not changed while executing the closure below
l.messages = []string{}
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
got := IsBranchProtected(tt.args.protection, "test", l.Logf)
got.Details = l.messages
if got.Confidence != tt.want.Confidence || got.Pass != tt.want.Pass {
t.Errorf("IsBranchProtected() = %s, %v, want %v", tt.name, got, tt.want)
}
dl := scut.TestDetailLogger{}
r := IsBranchProtected(tt.protection, "test", &dl)
scut.ValidateTestReturn(t, tt.name, &tt.expected, &r, &dl)
})
}
}

View File

@ -15,7 +15,6 @@
package checks
import (
"errors"
"fmt"
"regexp"
"strings"
@ -24,17 +23,12 @@ import (
"gopkg.in/yaml.v2"
"github.com/ossf/scorecard/checker"
sce "github.com/ossf/scorecard/errors"
)
// CheckFrozenDeps is the registered name for FrozenDeps.
const CheckFrozenDeps = "Frozen-Deps"
// ErrInvalidDockerfile : Invalid docker file.
var ErrInvalidDockerfile = errors.New("invalid docker file")
// ErrEmptyFile : Invalid docker file.
var ErrEmptyFile = errors.New("file has no content")
// Structure for workflow config.
// We only declare the fields we need.
// Github workflows format: https://docs.github.com/en/actions/reference/workflow-syntax-for-github-actions
@ -65,7 +59,7 @@ func init() {
// FrozenDeps will check the repository if it contains frozen dependecies.
func FrozenDeps(c *checker.CheckRequest) checker.CheckResult {
return checker.MultiCheckAnd(
return checker.MultiCheckAnd2(
isPackageManagerLockFilePresent,
isGitHubActionsWorkflowPinned,
isDockerfilePinned,
@ -78,28 +72,70 @@ func FrozenDeps(c *checker.CheckRequest) checker.CheckResult {
// TODO(laurent): need to support GCB pinning.
func isShellScriptFreeOfInsecureDownloads(c *checker.CheckRequest) checker.CheckResult {
return CheckFilesContent(CheckFrozenDeps, "*", false, c, validateShellScriptDownloads)
r, err := CheckFilesContent2("*", false, c, validateShellScriptIsFreeOfInsecureDownloads)
return createResultForIsShellScriptFreeOfInsecureDownloads(r, err)
}
func validateShellScriptDownloads(pathfn string, content []byte,
logf func(s string, f ...interface{})) (bool, error) {
func createResultForIsShellScriptFreeOfInsecureDownloads(r bool, err error) checker.CheckResult {
if err != nil {
return checker.CreateRuntimeErrorResult(CheckFrozenDeps, err)
}
if !r {
return checker.CreateMinScoreResult(CheckFrozenDeps,
"insecure (unpinned) dependency downloads found in shell scripts")
}
return checker.CreateMaxScoreResult(CheckFrozenDeps,
"no insecure (unpinned) dependency downloads found in shell scripts")
}
func testValidateShellScriptIsFreeOfInsecureDownloads(pathfn string,
content []byte, dl checker.DetailLogger) checker.CheckResult {
r, err := validateShellScriptIsFreeOfInsecureDownloads(pathfn, content, dl)
return createResultForIsShellScriptFreeOfInsecureDownloads(r, err)
}
func validateShellScriptIsFreeOfInsecureDownloads(pathfn string, content []byte,
dl checker.DetailLogger) (bool, error) {
// Validate the file type.
if !isShellScriptFile(pathfn, content) {
return true, nil
}
return validateShellFile(pathfn, content, logf)
return validateShellFile(pathfn, content, dl)
}
func isDockerfileFreeOfInsecureDownloads(c *checker.CheckRequest) checker.CheckResult {
return CheckFilesContent(CheckFrozenDeps, "*Dockerfile*", false, c, validateDockerfileDownloads)
r, err := CheckFilesContent2("*Dockerfile*", false, c, validateDockerfileIsFreeOfInsecureDownloads)
return createResultForIsDockerfileFreeOfInsecureDownloads(r, err)
}
func validateDockerfileDownloads(pathfn string, content []byte,
logf func(s string, f ...interface{})) (bool, error) {
// Create the result.
func createResultForIsDockerfileFreeOfInsecureDownloads(r bool, err error) checker.CheckResult {
if err != nil {
return checker.CreateRuntimeErrorResult(CheckFrozenDeps, err)
}
if !r {
return checker.CreateMinScoreResult(CheckFrozenDeps,
"insecure (unpinned) dependency downloads found in Dockerfiles")
}
return checker.CreateMaxScoreResult(CheckFrozenDeps,
"no insecure (unpinned) dependency downloads found in Dockerfiles")
}
func testValidateDockerfileIsFreeOfInsecureDownloads(pathfn string,
content []byte, dl checker.DetailLogger) checker.CheckResult {
r, err := validateDockerfileIsFreeOfInsecureDownloads(pathfn, content, dl)
return createResultForIsDockerfileFreeOfInsecureDownloads(r, err)
}
func validateDockerfileIsFreeOfInsecureDownloads(pathfn string, content []byte,
dl checker.DetailLogger) (bool, error) {
contentReader := strings.NewReader(string(content))
res, err := parser.Parse(contentReader)
if err != nil {
return false, fmt.Errorf("cannot read dockerfile content: %w", err)
//nolint
return false, sce.Create(sce.ErrScorecardInternal, fmt.Sprintf("%v: %v", errInternalInvalidDockerFile, err))
}
// nolint: prealloc
@ -119,7 +155,8 @@ func validateDockerfileDownloads(pathfn string, content []byte,
}
if len(valueList) == 0 {
return false, ErrParsingDockerfile
//nolint
return false, sce.Create(sce.ErrScorecardInternal, errInternalInvalidDockerFile.Error())
}
// Build a file content.
@ -127,15 +164,33 @@ func validateDockerfileDownloads(pathfn string, content []byte,
bytes = append(bytes, cmd...)
bytes = append(bytes, '\n')
}
return validateShellFile(pathfn, bytes, logf)
return validateShellFile(pathfn, bytes, dl)
}
func isDockerfilePinned(c *checker.CheckRequest) checker.CheckResult {
return CheckFilesContent(CheckFrozenDeps, "*Dockerfile*", false, c, validateDockerfile)
r, err := CheckFilesContent2("*Dockerfile*", false, c, validateDockerfileIsPinned)
return createResultForIsDockerfilePinned(r, err)
}
func validateDockerfile(pathfn string, content []byte,
logf func(s string, f ...interface{})) (bool, error) {
// Create the result.
func createResultForIsDockerfilePinned(r bool, err error) checker.CheckResult {
if err != nil {
return checker.CreateRuntimeErrorResult(CheckFrozenDeps, err)
}
if r {
return checker.CreateMaxScoreResult(CheckFrozenDeps, "Dockerfile dependencies are pinned")
}
return checker.CreateMinScoreResult(CheckFrozenDeps, "unpinned dependencies found Dockerfiles")
}
func testValidateDockerfileIsPinned(pathfn string, content []byte, dl checker.DetailLogger) checker.CheckResult {
r, err := validateDockerfileIsPinned(pathfn, content, dl)
return createResultForIsDockerfilePinned(r, err)
}
func validateDockerfileIsPinned(pathfn string, content []byte,
dl checker.DetailLogger) (bool, error) {
// Users may use various names, e.g.,
// Dockerfile.aarch64, Dockerfile.template, Dockerfile_template, dockerfile, Dockerfile-name.template
// Templates may trigger false positives, e.g. FROM { NAME }.
@ -150,7 +205,8 @@ func validateDockerfile(pathfn string, content []byte,
pinnedAsNames := make(map[string]bool)
res, err := parser.Parse(contentReader)
if err != nil {
return false, fmt.Errorf("cannot read dockerfile content: %w", err)
//nolint
return false, sce.Create(sce.ErrScorecardInternal, fmt.Sprintf("%v: %v", errInternalInvalidDockerFile, err))
}
for _, child := range res.AST.Children {
@ -188,44 +244,70 @@ func validateDockerfile(pathfn string, content []byte,
// Not pinned.
ret = false
logf("!! frozen-deps/docker - %v has non-pinned dependency '%v'", pathfn, name)
dl.Warn("unpinned dependency detected in %v: '%v'", pathfn, name)
// FROM name.
case len(valueList) == 1:
name := valueList[0]
if !regex.Match([]byte(name)) {
ret = false
logf("!! frozen-deps/docker - %v has non-pinned dependency '%v'", pathfn, name)
dl.Warn("unpinned dependency detected in %v: '%v'", pathfn, name)
}
default:
// That should not happen.
return false, ErrInvalidDockerfile
//nolint
return false, sce.Create(sce.ErrScorecardInternal, errInternalInvalidDockerFile.Error())
}
}
// The file should have at least one FROM statement.
if !fromFound {
return false, ErrInvalidDockerfile
//nolint
return false, sce.Create(sce.ErrScorecardInternal, errInternalInvalidDockerFile.Error())
}
return ret, nil
}
func isGitHubWorkflowScriptFreeOfInsecureDownloads(c *checker.CheckRequest) checker.CheckResult {
return CheckFilesContent(CheckFrozenDeps, ".github/workflows/*", false, c, validateGitHubWorkflowShellScriptDownloads)
r, err := CheckFilesContent2(".github/workflows/*", false, c, validateGitHubWorkflowIsFreeOfInsecureDownloads)
return createResultForIsGitHubWorkflowScriptFreeOfInsecureDownloads(r, err)
}
func validateGitHubWorkflowShellScriptDownloads(pathfn string, content []byte,
logf func(s string, f ...interface{})) (bool, error) {
// Create the result.
func createResultForIsGitHubWorkflowScriptFreeOfInsecureDownloads(r bool, err error) checker.CheckResult {
if err != nil {
return checker.CreateRuntimeErrorResult(CheckFrozenDeps, err)
}
if !r {
return checker.CreateMinScoreResult(CheckFrozenDeps,
"insecure (unpinned) dependency downloads found in GitHub workflows")
}
return checker.CreateMaxScoreResult(CheckFrozenDeps,
"no insecure (unpinned) dependency downloads found in GitHub workflows")
}
func testValidateGitHubWorkflowScriptFreeOfInsecureDownloads(pathfn string,
content []byte, dl checker.DetailLogger) checker.CheckResult {
r, err := validateGitHubWorkflowIsFreeOfInsecureDownloads(pathfn, content, dl)
return createResultForIsGitHubWorkflowScriptFreeOfInsecureDownloads(r, err)
}
func validateGitHubWorkflowIsFreeOfInsecureDownloads(pathfn string, content []byte,
dl checker.DetailLogger) (bool, error) {
if len(content) == 0 {
return false, ErrEmptyFile
//nolint
return false, sce.Create(sce.ErrScorecardInternal, errInternalEmptyFile.Error())
}
var workflow gitHubActionWorkflowConfig
err := yaml.Unmarshal(content, &workflow)
if err != nil {
return false, fmt.Errorf("!! frozen-deps - cannot unmarshal file %v\n%v: %w", pathfn, string(content), err)
//nolint
return false, sce.Create(sce.ErrScorecardInternal,
fmt.Sprintf("%v:%s:%s:%v", errInternalInvalidYamlFile, pathfn, string(content), err))
}
githubVarRegex := regexp.MustCompile(`{{[^{}]*}}`)
@ -262,7 +344,7 @@ func validateGitHubWorkflowShellScriptDownloads(pathfn string, content []byte,
}
if scriptContent != "" {
validated, err = validateShellFile(pathfn, []byte(scriptContent), logf)
validated, err = validateShellFile(pathfn, []byte(scriptContent), dl)
if err != nil {
return false, err
}
@ -273,19 +355,40 @@ func validateGitHubWorkflowShellScriptDownloads(pathfn string, content []byte,
// Check pinning of github actions in workflows.
func isGitHubActionsWorkflowPinned(c *checker.CheckRequest) checker.CheckResult {
return CheckFilesContent(CheckFrozenDeps, ".github/workflows/*", true, c, validateGitHubActionWorkflow)
r, err := CheckFilesContent2(".github/workflows/*", true, c, validateGitHubActionWorkflow)
return createResultForIsGitHubActionsWorkflowPinned(r, err)
}
// Create the result.
func createResultForIsGitHubActionsWorkflowPinned(r bool, err error) checker.CheckResult {
if err != nil {
return checker.CreateRuntimeErrorResult(CheckFrozenDeps, err)
}
if r {
return checker.CreateMaxScoreResult(CheckFrozenDeps, "GitHub actions are pinned")
}
return checker.CreateMinScoreResult(CheckFrozenDeps, "GitHub actions are not pinned")
}
func testIsGitHubActionsWorkflowPinned(pathfn string, content []byte, dl checker.DetailLogger) checker.CheckResult {
r, err := validateGitHubActionWorkflow(pathfn, content, dl)
return createResultForIsGitHubActionsWorkflowPinned(r, err)
}
// Check file content.
func validateGitHubActionWorkflow(pathfn string, content []byte, logf func(s string, f ...interface{})) (bool, error) {
func validateGitHubActionWorkflow(pathfn string, content []byte, dl checker.DetailLogger) (bool, error) {
if len(content) == 0 {
return false, ErrEmptyFile
//nolint
return false, sce.Create(sce.ErrScorecardInternal, errInternalEmptyFile.Error())
}
var workflow gitHubActionWorkflowConfig
err := yaml.Unmarshal(content, &workflow)
if err != nil {
return false, fmt.Errorf("!! frozen-deps - cannot unmarshal file %v\n%v: %w", pathfn, string(content), err)
//nolint
return false, sce.Create(sce.ErrScorecardInternal,
fmt.Sprintf("%v:%s:%s:%v", errInternalInvalidYamlFile, pathfn, string(content), err))
}
hashRegex := regexp.MustCompile(`^.*@[a-f\d]{40,}`)
@ -301,7 +404,7 @@ func validateGitHubActionWorkflow(pathfn string, content []byte, logf func(s str
match := hashRegex.Match([]byte(step.Uses))
if !match {
ret = false
logf("!! frozen-deps/github-actions - %v has non-pinned dependency '%v' (job '%v')", pathfn, step.Uses, jobName)
dl.Warn("unpinned dependency detected in %v: '%v' (job '%v')", pathfn, step.Uses, jobName)
}
}
}
@ -312,38 +415,49 @@ func validateGitHubActionWorkflow(pathfn string, content []byte, logf func(s str
// Check presence of lock files thru validatePackageManagerFile().
func isPackageManagerLockFilePresent(c *checker.CheckRequest) checker.CheckResult {
return CheckIfFileExists(CheckFrozenDeps, c, validatePackageManagerFile)
r, err := CheckIfFileExists2(CheckFrozenDeps, c, validatePackageManagerFile)
if err != nil {
return checker.CreateRuntimeErrorResult(CheckFrozenDeps, err)
}
if !r {
return checker.CreateInconclusiveResult(CheckFrozenDeps, "no lock files detected for a package manager")
}
return checker.CreateMaxScoreResult(CheckFrozenDeps, "lock file detected for a package manager")
}
// validatePackageManagerFile will validate the if frozen dependecies file name exists.
// TODO(laurent): need to differentiate between libraries and programs.
// TODO(laurent): handle multi-language repos.
func validatePackageManagerFile(name string, logf func(s string, f ...interface{})) (bool, error) {
func validatePackageManagerFile(name string, dl checker.DetailLogger) (bool, error) {
switch strings.ToLower(name) {
case "go.mod", "go.sum":
logf("go modules found: %s", name)
// TODO(laurent): "go.mod" is for libraries
case "go.sum":
dl.Info("go lock file detected: %s", name)
return true, nil
case "vendor/", "third_party/", "third-party/":
logf("vendor dir found: %s", name)
dl.Info("vendoring detected in: %s", name)
return true, nil
case "package-lock.json", "npm-shrinkwrap.json":
logf("nodejs packages found: %s", name)
dl.Info("javascript lock file detected: %s", name)
return true, nil
// TODO(laurent): add check for hashbased pinning in requirements.txt - https://davidwalsh.name/hashin
case "requirements.txt", "pipfile.lock":
logf("python requirements found: %s", name)
// Note: because requirements.txt does not handle transitive dependencies, we consider it
// not a lock file, until we have remediation steps for pip-build.
case "pipfile.lock":
dl.Info("python lock file detected: %s", name)
return true, nil
case "gemfile.lock":
logf("ruby gems found: %s", name)
dl.Info("ruby lock file detected: %s", name)
return true, nil
case "cargo.lock":
logf("rust crates found: %s", name)
dl.Info("rust lock file detected: %s", name)
return true, nil
case "yarn.lock":
logf("yarn packages found: %s", name)
dl.Info("yarn lock file detected: %s", name)
return true, nil
case "composer.lock":
logf("composer packages found: %s", name)
dl.Info("composer lock file detected: %s", name)
return true, nil
default:
return false, nil

View File

@ -15,525 +15,421 @@
package checks
import (
"errors"
"fmt"
"io/ioutil"
"strings"
"testing"
"github.com/ossf/scorecard/checker"
sce "github.com/ossf/scorecard/errors"
scut "github.com/ossf/scorecard/utests"
)
func TestGithubWorkflowPinning(t *testing.T) {
t.Parallel()
//nolint
type args struct {
Log log
Filename string
}
type returnValue struct {
Error error
Result bool
NumberOfErrors int
}
//nolint
tests := []struct {
args args
want returnValue
name string
name string
filename string
expected scut.TestReturn
}{
{
name: "Zero size content",
args: args{
Filename: "",
Log: log{},
},
want: returnValue{
Error: ErrEmptyFile,
Result: false,
NumberOfErrors: 0,
name: "Zero size content",
filename: "",
expected: scut.TestReturn{
Errors: []error{sce.ErrScorecardInternal},
Score: checker.InconclusiveResultScore,
NumberOfWarn: 0,
NumberOfInfo: 0,
NumberOfDebug: 0,
},
},
{
name: "Pinned workflow",
args: args{
Filename: "./testdata/workflow-pinned.yaml",
Log: log{},
},
want: returnValue{
Error: nil,
Result: true,
NumberOfErrors: 0,
name: "Pinned workflow",
filename: "./testdata/workflow-pinned.yaml",
expected: scut.TestReturn{
Errors: nil,
Score: checker.MaxResultScore,
NumberOfWarn: 0,
NumberOfInfo: 0,
NumberOfDebug: 0,
},
},
{
name: "Non-pinned workflow",
args: args{
Filename: "./testdata/workflow-not-pinned.yaml",
Log: log{},
},
want: returnValue{
Error: nil,
Result: false,
NumberOfErrors: 1,
name: "Non-pinned workflow",
filename: "./testdata/workflow-not-pinned.yaml",
expected: scut.TestReturn{
Errors: nil,
Score: checker.MinResultScore,
NumberOfWarn: 1,
NumberOfInfo: 0,
NumberOfDebug: 0,
},
},
}
//nolint
for _, tt := range tests {
tt := tt // Re-initializing variable so it is not changed while executing the closure below
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
var content []byte
var err error
if tt.args.Filename == "" {
if tt.filename == "" {
content = make([]byte, 0)
} else {
content, err = ioutil.ReadFile(tt.args.Filename)
content, err = ioutil.ReadFile(tt.filename)
if err != nil {
panic(fmt.Errorf("cannot read file: %w", err))
}
}
r, err := validateGitHubActionWorkflow(tt.args.Filename, content, tt.args.Log.Logf)
if !errors.Is(err, tt.want.Error) ||
r != tt.want.Result ||
len(tt.args.Log.messages) != tt.want.NumberOfErrors {
t.Errorf("TestDockerfileScriptDownload:\"%v\": %v (%v,%v,%v) want (%v, %v, %v)\n%v",
tt.name, tt.args.Filename, r, err, len(tt.args.Log.messages), tt.want.Result, tt.want.Error, tt.want.NumberOfErrors,
strings.Join(tt.args.Log.messages, "\n"))
}
dl := scut.TestDetailLogger{}
r := testIsGitHubActionsWorkflowPinned(tt.filename, content, &dl)
scut.ValidateTestReturn(t, tt.name, &tt.expected, &r, &dl)
})
}
}
func TestDockerfilePinning(t *testing.T) {
t.Parallel()
type args struct {
Logf func(s string, f ...interface{})
Filename string
}
type returnValue struct {
Error error
Result bool
}
l := log{}
tests := []struct {
args args
want returnValue
name string
name string
filename string
expected scut.TestReturn
}{
{
name: "Invalid dockerfile",
args: args{
Filename: "./testdata/Dockerfile-invalid",
Logf: l.Logf,
},
want: returnValue{
Error: ErrInvalidDockerfile,
Result: false,
name: "Invalid dockerfile",
filename: "./testdata/Dockerfile-invalid",
expected: scut.TestReturn{
Errors: []error{sce.ErrScorecardInternal},
Score: checker.InconclusiveResultScore,
NumberOfWarn: 0,
NumberOfInfo: 0,
NumberOfDebug: 0,
},
},
{
name: "Pinned dockerfile",
args: args{
Filename: "./testdata/Dockerfile-pinned",
Logf: l.Logf,
},
want: returnValue{
Error: nil,
Result: true,
name: "Pinned dockerfile",
filename: "./testdata/Dockerfile-pinned",
expected: scut.TestReturn{
Errors: nil,
Score: checker.MaxResultScore,
NumberOfWarn: 0,
NumberOfInfo: 0,
NumberOfDebug: 0,
},
},
{
name: "Pinned dockerfile as",
args: args{
Filename: "./testdata/Dockerfile-pinned-as",
Logf: l.Logf,
},
want: returnValue{
Error: nil,
Result: true,
name: "Pinned dockerfile as",
filename: "./testdata/Dockerfile-pinned-as",
expected: scut.TestReturn{
Errors: nil,
Score: checker.MaxResultScore,
NumberOfWarn: 0,
NumberOfInfo: 0,
NumberOfDebug: 0,
},
},
{
name: "Non-pinned dockerfile as",
args: args{
Filename: "./testdata/Dockerfile-not-pinned-as",
Logf: l.Logf,
},
want: returnValue{
Error: nil,
Result: false,
name: "Non-pinned dockerfile as",
filename: "./testdata/Dockerfile-not-pinned-as",
expected: scut.TestReturn{
Errors: nil,
Score: checker.MinResultScore,
NumberOfWarn: 3, // TODO: should be 2, https://github.com/ossf/scorecard/issues/701.
NumberOfInfo: 0,
NumberOfDebug: 0,
},
},
{
name: "Non-pinned dockerfile",
args: args{
Filename: "./testdata/Dockerfile-not-pinned",
Logf: l.Logf,
},
want: returnValue{
Error: nil,
Result: false,
name: "Non-pinned dockerfile",
filename: "./testdata/Dockerfile-not-pinned",
expected: scut.TestReturn{
Errors: nil,
Score: checker.MinResultScore,
NumberOfWarn: 1,
NumberOfInfo: 0,
NumberOfDebug: 0,
},
},
}
for _, tt := range tests {
tt := tt // Re-initializing variable so it is not changed while executing the closure below
l.messages = []string{}
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
var content []byte
var err error
content, err = ioutil.ReadFile(tt.args.Filename)
if err != nil {
panic(fmt.Errorf("cannot read file: %w", err))
}
r, err := validateDockerfile(tt.args.Filename, content, tt.args.Logf)
if !errors.Is(err, tt.want.Error) ||
r != tt.want.Result {
t.Errorf("TestGithubWorkflowPinning:\"%v\": %v (%v,%v) want (%v, %v)",
tt.name, tt.args.Filename, r, err, tt.want.Result, tt.want.Error)
if tt.filename == "" {
content = make([]byte, 0)
} else {
content, err = ioutil.ReadFile(tt.filename)
if err != nil {
panic(fmt.Errorf("cannot read file: %w", err))
}
}
dl := scut.TestDetailLogger{}
r := testValidateDockerfileIsPinned(tt.filename, content, &dl)
scut.ValidateTestReturn(t, tt.name, &tt.expected, &r, &dl)
})
}
}
func TestDockerfileScriptDownload(t *testing.T) {
t.Parallel()
//nolint
type args struct {
// Note: this seems to be defined in e2e/e2e_suite_test.go
Log log
Filename string
}
type returnValue struct {
Error error
Result bool
NumberOfErrors int
}
//nolint
tests := []struct {
args args
want returnValue
name string
name string
filename string
expected scut.TestReturn
}{
{
name: "curl | sh",
args: args{
Filename: "testdata/Dockerfile-curl-sh",
Log: log{},
},
want: returnValue{
Error: nil,
Result: false,
NumberOfErrors: 4,
name: "curl | sh",
filename: "testdata/Dockerfile-curl-sh",
expected: scut.TestReturn{
Errors: nil,
Score: checker.MinResultScore,
NumberOfWarn: 4,
NumberOfInfo: 0,
NumberOfDebug: 0,
},
},
{
name: "wget | /bin/sh",
args: args{
Filename: "testdata/Dockerfile-wget-bin-sh",
Log: log{},
},
want: returnValue{
Error: nil,
Result: false,
NumberOfErrors: 3,
name: "wget | /bin/sh",
filename: "testdata/Dockerfile-wget-bin-sh",
expected: scut.TestReturn{
Errors: nil,
Score: checker.MinResultScore,
NumberOfWarn: 3,
NumberOfInfo: 0,
NumberOfDebug: 0,
},
},
{
name: "wget no exec",
args: args{
Filename: "testdata/Dockerfile-script-ok",
Log: log{},
},
want: returnValue{
Error: nil,
Result: true,
NumberOfErrors: 0,
name: "wget no exec",
filename: "testdata/Dockerfile-script-ok",
expected: scut.TestReturn{
Errors: nil,
Score: checker.MaxResultScore,
NumberOfWarn: 0,
NumberOfInfo: 0,
NumberOfDebug: 0,
},
},
{
name: "curl file sh",
args: args{
Filename: "testdata/Dockerfile-curl-file-sh",
Log: log{},
},
want: returnValue{
Error: nil,
Result: false,
NumberOfErrors: 12,
name: "curl file sh",
filename: "testdata/Dockerfile-curl-file-sh",
expected: scut.TestReturn{
Errors: nil,
Score: checker.MinResultScore,
NumberOfWarn: 12,
NumberOfInfo: 0,
NumberOfDebug: 0,
},
},
{
name: "proc substitution",
args: args{
Filename: "testdata/Dockerfile-proc-subs",
Log: log{},
},
want: returnValue{
Error: nil,
Result: false,
NumberOfErrors: 6,
name: "proc substitution",
filename: "testdata/Dockerfile-proc-subs",
expected: scut.TestReturn{
Errors: nil,
Score: checker.MinResultScore,
NumberOfWarn: 6,
NumberOfInfo: 0,
NumberOfDebug: 0,
},
},
{
name: "wget file",
args: args{
Filename: "testdata/Dockerfile-wget-file",
Log: log{},
},
want: returnValue{
Error: nil,
Result: false,
NumberOfErrors: 10,
name: "wget file",
filename: "testdata/Dockerfile-wget-file",
expected: scut.TestReturn{
Errors: nil,
Score: checker.MinResultScore,
NumberOfWarn: 10,
NumberOfInfo: 0,
NumberOfDebug: 0,
},
},
{
name: "gsutil file",
args: args{
Filename: "testdata/Dockerfile-gsutil-file",
Log: log{},
},
want: returnValue{
Error: nil,
Result: false,
NumberOfErrors: 17,
name: "gsutil file",
filename: "testdata/Dockerfile-gsutil-file",
expected: scut.TestReturn{
Errors: nil,
Score: checker.MinResultScore,
NumberOfWarn: 17,
NumberOfInfo: 0,
NumberOfDebug: 0,
},
},
{
name: "aws file",
args: args{
Filename: "testdata/Dockerfile-aws-file",
Log: log{},
},
want: returnValue{
Error: nil,
Result: false,
NumberOfErrors: 15,
name: "aws file",
filename: "testdata/Dockerfile-aws-file",
expected: scut.TestReturn{
Errors: nil,
Score: checker.MinResultScore,
NumberOfWarn: 15,
NumberOfInfo: 0,
NumberOfDebug: 0,
},
},
{
name: "pkg managers",
args: args{
Filename: "testdata/Dockerfile-pkg-managers",
Log: log{},
},
want: returnValue{
Error: nil,
Result: false,
NumberOfErrors: 27,
name: "pkg managers",
filename: "testdata/Dockerfile-pkg-managers",
expected: scut.TestReturn{
Errors: nil,
Score: checker.MinResultScore,
NumberOfWarn: 27,
NumberOfInfo: 0,
NumberOfDebug: 0,
},
},
}
//nolint
for _, tt := range tests {
tt := tt // Re-initializing variable so it is not changed while executing the closure below
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
var content []byte
var err error
content, err = ioutil.ReadFile(tt.args.Filename)
if err != nil {
panic(fmt.Errorf("cannot read file: %w", err))
}
r, err := validateDockerfileDownloads(tt.args.Filename, content, tt.args.Log.Logf)
if !errors.Is(err, tt.want.Error) ||
r != tt.want.Result ||
len(tt.args.Log.messages) != tt.want.NumberOfErrors {
t.Errorf("TestDockerfileScriptDownload:\"%v\": %v (%v,%v,%v) want (%v, %v, %v)\n%v",
tt.name, tt.args.Filename, r, err, len(tt.args.Log.messages), tt.want.Result, tt.want.Error, tt.want.NumberOfErrors,
strings.Join(tt.args.Log.messages, "\n"))
if tt.filename == "" {
content = make([]byte, 0)
} else {
content, err = ioutil.ReadFile(tt.filename)
if err != nil {
panic(fmt.Errorf("cannot read file: %w", err))
}
}
dl := scut.TestDetailLogger{}
r := testValidateDockerfileIsFreeOfInsecureDownloads(tt.filename, content, &dl)
scut.ValidateTestReturn(t, tt.name, &tt.expected, &r, &dl)
})
}
}
func TestShellScriptDownload(t *testing.T) {
t.Parallel()
//nolint
type args struct {
// Note: this seems to be defined in e2e/e2e_suite_test.go
Log log
Filename string
}
type returnValue struct {
Error error
Result bool
NumberOfErrors int
}
//nolint
tests := []struct {
args args
want returnValue
name string
name string
filename string
expected scut.TestReturn
}{
{
name: "sh script",
args: args{
Filename: "testdata/script-sh",
Log: log{},
},
want: returnValue{
Error: nil,
Result: false,
NumberOfErrors: 7,
name: "sh script",
filename: "testdata/script-sh",
expected: scut.TestReturn{
Errors: nil,
Score: checker.MinResultScore,
NumberOfWarn: 7,
NumberOfInfo: 0,
NumberOfDebug: 0,
},
},
{
name: "bash script",
args: args{
Filename: "testdata/script-bash",
Log: log{},
},
want: returnValue{
Error: nil,
Result: false,
NumberOfErrors: 7,
name: "bash script",
filename: "testdata/script-bash",
expected: scut.TestReturn{
Errors: nil,
Score: checker.MinResultScore,
NumberOfWarn: 7,
NumberOfInfo: 0,
NumberOfDebug: 0,
},
},
{
name: "sh script 2",
args: args{
Filename: "testdata/script.sh",
Log: log{},
},
want: returnValue{
Error: nil,
Result: false,
NumberOfErrors: 7,
name: "sh script 2",
filename: "testdata/script.sh",
expected: scut.TestReturn{
Errors: nil,
Score: checker.MinResultScore,
NumberOfWarn: 7,
NumberOfInfo: 0,
NumberOfDebug: 0,
},
},
{
name: "pkg managers",
args: args{
Filename: "testdata/script-pkg-managers",
Log: log{},
},
want: returnValue{
Error: nil,
Result: false,
NumberOfErrors: 24,
name: "pkg managers",
filename: "testdata/script-pkg-managers",
expected: scut.TestReturn{
Errors: nil,
Score: checker.MinResultScore,
NumberOfWarn: 24,
NumberOfInfo: 0,
NumberOfDebug: 0,
},
},
}
//nolint
for _, tt := range tests {
tt := tt // Re-initializing variable so it is not changed while executing the closure below
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
var content []byte
var err error
content, err = ioutil.ReadFile(tt.args.Filename)
if err != nil {
panic(fmt.Errorf("cannot read file: %w", err))
}
r, err := validateShellScriptDownloads(tt.args.Filename, content, tt.args.Log.Logf)
if !errors.Is(err, tt.want.Error) ||
r != tt.want.Result ||
len(tt.args.Log.messages) != tt.want.NumberOfErrors {
t.Errorf("TestDockerfileScriptDownload:\"%v\": %v (%v,%v,%v) want (%v, %v, %v)\n%v",
tt.name, tt.args.Filename, r, err, len(tt.args.Log.messages), tt.want.Result, tt.want.Error, tt.want.NumberOfErrors,
strings.Join(tt.args.Log.messages, "\n"))
if tt.filename == "" {
content = make([]byte, 0)
} else {
content, err = ioutil.ReadFile(tt.filename)
if err != nil {
panic(fmt.Errorf("cannot read file: %w", err))
}
}
dl := scut.TestDetailLogger{}
r := testValidateShellScriptIsFreeOfInsecureDownloads(tt.filename, content, &dl)
scut.ValidateTestReturn(t, tt.name, &tt.expected, &r, &dl)
})
}
}
func TestGitHubWorflowRunDownload(t *testing.T) {
t.Parallel()
//nolint
type args struct {
// Note: this seems to be defined in e2e/e2e_suite_test.go
Log log
Filename string
}
type returnValue struct {
Error error
Result bool
NumberOfErrors int
}
//nolint
tests := []struct {
args args
want returnValue
name string
name string
filename string
expected scut.TestReturn
}{
{
name: "workflow curl default",
args: args{
Filename: "testdata/github-workflow-curl-default",
Log: log{},
},
want: returnValue{
Error: nil,
Result: false,
NumberOfErrors: 1,
name: "workflow curl default",
filename: "testdata/github-workflow-curl-default",
expected: scut.TestReturn{
Errors: nil,
Score: checker.MinResultScore,
NumberOfWarn: 1,
NumberOfInfo: 0,
NumberOfDebug: 0,
},
},
{
name: "workflow curl no default",
args: args{
Filename: "testdata/github-workflow-curl-no-default",
Log: log{},
},
want: returnValue{
Error: nil,
Result: false,
NumberOfErrors: 1,
name: "workflow curl no default",
filename: "testdata/github-workflow-curl-no-default",
expected: scut.TestReturn{
Errors: nil,
Score: checker.MinResultScore,
NumberOfWarn: 1,
NumberOfInfo: 0,
NumberOfDebug: 0,
},
},
{
name: "wget across steps",
args: args{
Filename: "testdata/github-workflow-wget-across-steps",
Log: log{},
},
want: returnValue{
Error: nil,
Result: false,
NumberOfErrors: 2,
name: "wget across steps",
filename: "testdata/github-workflow-wget-across-steps",
expected: scut.TestReturn{
Errors: nil,
Score: checker.MinResultScore,
NumberOfWarn: 2,
NumberOfInfo: 0,
NumberOfDebug: 0,
},
},
}
//nolint
for _, tt := range tests {
tt := tt // Re-initializing variable so it is not changed while executing the closure below
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
var content []byte
var err error
content, err = ioutil.ReadFile(tt.args.Filename)
if err != nil {
panic(fmt.Errorf("cannot read file: %w", err))
}
r, err := validateGitHubWorkflowShellScriptDownloads(tt.args.Filename, content, tt.args.Log.Logf)
if !errors.Is(err, tt.want.Error) ||
r != tt.want.Result ||
len(tt.args.Log.messages) != tt.want.NumberOfErrors {
t.Errorf("TestDockerfileScriptDownload:\"%v\": %v (%v,%v,%v) want (%v, %v, %v)\n%v",
tt.name, tt.args.Filename, r, err, len(tt.args.Log.messages), tt.want.Result, tt.want.Error, tt.want.NumberOfErrors,
strings.Join(tt.args.Log.messages, "\n"))
if tt.filename == "" {
content = make([]byte, 0)
} else {
content, err = ioutil.ReadFile(tt.filename)
if err != nil {
panic(fmt.Errorf("cannot read file: %w", err))
}
}
dl := scut.TestDetailLogger{}
r := testValidateGitHubWorkflowScriptFreeOfInsecureDownloads(tt.filename, content, &dl)
scut.ValidateTestReturn(t, tt.name, &tt.expected, &r, &dl)
})
}
}

View File

@ -15,46 +15,46 @@
package checks
import (
"errors"
"fmt"
"strings"
"gopkg.in/yaml.v2"
"github.com/ossf/scorecard/checker"
sce "github.com/ossf/scorecard/errors"
)
const CheckPermissions = "Token-Permissions"
// ErrInvalidGitHubWorkflowFile : Invalid GitHub workflow file.
var ErrInvalidGitHubWorkflowFile = errors.New("invalid GitHub workflow file")
//nolint:gochecknoinits
func init() {
registerCheck(CheckPermissions, leastPrivilegedTokens)
}
func leastPrivilegedTokens(c *checker.CheckRequest) checker.CheckResult {
return CheckFilesContent(CheckPermissions, ".github/workflows/*", true, c, validateGitHubActionTokenPermissions)
r, err := CheckFilesContent2(".github/workflows/*", false, c, validateGitHubActionTokenPermissions)
return createResultForLeastPrivilegeTokens(r, err)
}
func validatePermission(key string, value interface{}, path string,
logf func(s string, f ...interface{})) (bool, error) {
dl checker.DetailLogger) (bool, error) {
val, ok := value.(string)
if !ok {
return false, ErrInvalidGitHubWorkflowFile
//nolint
return false, sce.Create(sce.ErrScorecardInternal, errInvalidGitHubWorkflowFile.Error())
}
if strings.EqualFold(val, "write") {
logf("!! token-permissions/github-token - %v permission set to '%v' in %v", key, val, path)
dl.Warn("'%v' permission set to '%v' in %v", key, val, path)
return false, nil
}
dl.Info("'%v' permission set to '%v' in %v", key, val, path)
return true, nil
}
func validateMapPermissions(values map[interface{}]interface{}, path string,
logf func(s string, f ...interface{})) (bool, error) {
dl checker.DetailLogger) (bool, error) {
permissionRead := true
var r bool
var err error
@ -63,10 +63,11 @@ func validateMapPermissions(values map[interface{}]interface{}, path string,
for k, v := range values {
key, ok := k.(string)
if !ok {
return false, ErrInvalidGitHubWorkflowFile
//nolint
return false, sce.Create(sce.ErrScorecardInternal, errInvalidGitHubWorkflowFile.Error())
}
if r, err = validatePermission(key, v, path, logf); err != nil {
if r, err = validatePermission(key, v, path, dl); err != nil {
return false, err
}
@ -78,13 +79,13 @@ func validateMapPermissions(values map[interface{}]interface{}, path string,
}
func validateReadPermissions(config map[interface{}]interface{}, path string,
logf func(s string, f ...interface{})) (bool, error) {
dl checker.DetailLogger) (bool, error) {
var permissions interface{}
// Check if permissions are set explicitly.
permissions, ok := config["permissions"]
if !ok {
logf("!! token-permissions/github-token - no permission defined in %v", path)
dl.Warn("no permission defined in %v", path)
return false, nil
}
@ -93,17 +94,18 @@ func validateReadPermissions(config map[interface{}]interface{}, path string,
// Empty string is nil type.
// It defaults to 'none'
case nil:
dl.Info("permission set to 'none' in %v", path)
// String type.
case string:
if !strings.EqualFold(val, "read-all") && val != "" {
logf("!! token-permissions/github-token - permission set to '%v' in %v", val, path)
dl.Warn("permission set to '%v' in %v", val, path)
return false, nil
}
dl.Info("permission set to '%v' in %v", val, path)
// Map type.
case map[interface{}]interface{}:
if res, err := validateMapPermissions(val, path, logf); err != nil {
if res, err := validateMapPermissions(val, path, dl); err != nil {
return false, err
} else if !res {
return false, nil
@ -111,17 +113,39 @@ func validateReadPermissions(config map[interface{}]interface{}, path string,
// Invalid type.
default:
return false, ErrInvalidGitHubWorkflowFile
//nolint
return false, sce.Create(sce.ErrScorecardInternal, errInvalidGitHubWorkflowFile.Error())
}
return true, nil
}
// Create the result.
func createResultForLeastPrivilegeTokens(r bool, err error) checker.CheckResult {
if err != nil {
return checker.CreateRuntimeErrorResult(CheckPermissions, err)
}
if !r {
return checker.CreateMinScoreResult(CheckPermissions,
"non read-only tokens detected in GitHub workflows")
}
return checker.CreateMaxScoreResult(CheckPermissions,
"tokens are read-only in GitHub workflows")
}
func testValidateGitHubActionTokenPermissions(pathfn string,
content []byte, dl checker.DetailLogger) checker.CheckResult {
r, err := validateGitHubActionTokenPermissions(pathfn, content, dl)
return createResultForLeastPrivilegeTokens(r, err)
}
// Check file content.
func validateGitHubActionTokenPermissions(path string, content []byte,
logf func(s string, f ...interface{})) (bool, error) {
dl checker.DetailLogger) (bool, error) {
if len(content) == 0 {
return false, ErrEmptyFile
//nolint
return false, sce.Create(sce.ErrScorecardInternal, errInternalEmptyFile.Error())
}
var workflow map[interface{}]interface{}
@ -129,8 +153,8 @@ func validateGitHubActionTokenPermissions(path string, content []byte,
var err error
err = yaml.Unmarshal(content, &workflow)
if err != nil {
return false, fmt.Errorf("!! token-permissions - cannot unmarshal file %v\n%v\n%v: %w",
path, content, string(content), err)
//nolint
return false, sce.Create(sce.ErrScorecardInternal, fmt.Sprintf("yaml.Unmarshal: %v", err))
}
// 1. Check that each file uses 'content: read' only or 'none'.
@ -138,7 +162,7 @@ func validateGitHubActionTokenPermissions(path string, content []byte,
// https://docs.github.com/en/actions/reference/authentication-in-a-workflow#example-1-passing-the-github_token-as-an-input,
// https://github.blog/changelog/2021-04-20-github-actions-control-permissions-for-github_token/,
// https://docs.github.com/en/actions/reference/authentication-in-a-workflow#modifying-the-permissions-for-the-github_token.
if r, err = validateReadPermissions(workflow, path, logf); err != nil {
if r, err = validateReadPermissions(workflow, path, dl); err != nil {
return false, nil
}
if !r {

View File

@ -15,130 +15,118 @@
package checks
import (
"errors"
"fmt"
"io/ioutil"
"testing"
"github.com/ossf/scorecard/checker"
scut "github.com/ossf/scorecard/utests"
)
//nolint
func TestGithubTokenPermissions(t *testing.T) {
t.Parallel()
type args struct {
Logf func(s string, f ...interface{})
Filename string
}
type returnValue struct {
Error error
Result bool
}
l := log{}
tests := []struct {
args args
want returnValue
name string
name string
filename string
expected scut.TestReturn
}{
{
name: "Write all test",
args: args{
Filename: "./testdata/github-workflow-permissions-writeall.yaml",
Logf: l.Logf,
},
want: returnValue{
Error: nil,
Result: false,
name: "Write all test",
filename: "./testdata/github-workflow-permissions-writeall.yaml",
expected: scut.TestReturn{
Errors: nil,
Score: checker.MinResultScore,
NumberOfWarn: 1,
NumberOfInfo: 0,
NumberOfDebug: 0,
},
},
{
name: "Read all test",
args: args{
Filename: "./testdata/github-workflow-permissions-readall.yaml",
Logf: l.Logf,
},
want: returnValue{
Error: nil,
Result: true,
name: "Read all test",
filename: "./testdata/github-workflow-permissions-readall.yaml",
expected: scut.TestReturn{
Errors: nil,
Score: checker.MaxResultScore,
NumberOfWarn: 0,
NumberOfInfo: 1,
NumberOfDebug: 0,
},
},
{
name: "No permission test",
args: args{
Filename: "./testdata/github-workflow-permissions-absent.yaml",
Logf: l.Logf,
},
want: returnValue{
Error: nil,
Result: false,
name: "No permission test",
filename: "./testdata/github-workflow-permissions-absent.yaml",
expected: scut.TestReturn{
Errors: nil,
Score: checker.MinResultScore,
NumberOfWarn: 1,
NumberOfInfo: 0,
NumberOfDebug: 0,
},
},
{
name: "Writes test",
args: args{
Filename: "./testdata/github-workflow-permissions-writes.yaml",
Logf: l.Logf,
},
want: returnValue{
Error: nil,
Result: false,
name: "Writes test",
filename: "./testdata/github-workflow-permissions-writes.yaml",
expected: scut.TestReturn{
Errors: nil,
Score: checker.MinResultScore,
NumberOfWarn: 1,
NumberOfInfo: 1,
NumberOfDebug: 0,
},
},
{
name: "Reads test",
args: args{
Filename: "./testdata/github-workflow-permissions-reads.yaml",
Logf: l.Logf,
},
want: returnValue{
Error: nil,
Result: true,
name: "Reads test",
filename: "./testdata/github-workflow-permissions-reads.yaml",
expected: scut.TestReturn{
Errors: nil,
Score: checker.MaxResultScore,
NumberOfWarn: 0,
NumberOfInfo: 10,
NumberOfDebug: 0,
},
},
{
name: "Nones test",
args: args{
Filename: "./testdata/github-workflow-permissions-nones.yaml",
Logf: l.Logf,
},
want: returnValue{
Error: nil,
Result: true,
name: "Nones test",
filename: "./testdata/github-workflow-permissions-nones.yaml",
expected: scut.TestReturn{
Errors: nil,
Score: checker.MaxResultScore,
NumberOfWarn: 0,
NumberOfInfo: 10,
NumberOfDebug: 0,
},
},
{
name: "None test",
args: args{
Filename: "./testdata/github-workflow-permissions-none.yaml",
Logf: l.Logf,
},
want: returnValue{
Error: nil,
Result: true,
name: "None test",
filename: "./testdata/github-workflow-permissions-none.yaml",
expected: scut.TestReturn{
Errors: nil,
Score: checker.MaxResultScore,
NumberOfWarn: 0,
NumberOfInfo: 1,
NumberOfDebug: 0,
},
},
}
for _, tt := range tests {
tt := tt // Re-initializing variable so it is not changed while executing the closure below
l.messages = []string{}
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
var content []byte
var err error
if tt.args.Filename == "" {
if tt.filename == "" {
content = make([]byte, 0)
} else {
content, err = ioutil.ReadFile(tt.args.Filename)
content, err = ioutil.ReadFile(tt.filename)
if err != nil {
panic(fmt.Errorf("cannot read file: %w", err))
}
}
r, err := validateGitHubActionTokenPermissions(tt.args.Filename, content, tt.args.Logf)
if !errors.Is(err, tt.want.Error) ||
r != tt.want.Result {
t.Errorf("TestGithubTokenPermissions:\"%v\": %v (%v,%v) want (%v, %v)",
tt.name, tt.args.Filename, r, err, tt.want.Result, tt.want.Error)
}
dl := scut.TestDetailLogger{}
r := testValidateGitHubActionTokenPermissions(tt.filename, content, &dl)
scut.ValidateTestReturn(t, tt.name, &tt.expected, &r, &dl)
})
}
}

View File

@ -17,7 +17,6 @@ package checks
import (
"bufio"
"bytes"
"errors"
"fmt"
"net/url"
"path"
@ -26,14 +25,11 @@ import (
"strings"
"mvdan.cc/sh/v3/syntax"
"github.com/ossf/scorecard/checker"
sce "github.com/ossf/scorecard/errors"
)
// ErrParsingDockerfile indicates a problem parsing the dockerfile.
var ErrParsingDockerfile = errors.New("file cannot be parsed")
// ErrParsingShellCommand indicates a problem parsing a shell command.
var ErrParsingShellCommand = errors.New("shell command cannot be parsed")
// List of interpreters.
var pythonInterpreters = []string{"python", "python3", "python2.7"}
@ -106,7 +102,8 @@ func getWgetOutputFile(cmd []string) (pathfn string, ok bool, err error) {
u, err := url.Parse(cmd[i])
if err != nil {
return "", false, fmt.Errorf("url.Parse: %w", err)
//nolint
return "", false, sce.Create(sce.ErrScorecardInternal, fmt.Sprintf("url.Parse: %v", err))
}
return path.Base(u.Path), true, nil
}
@ -125,7 +122,8 @@ func getGsutilOutputFile(cmd []string) (pathfn string, ok bool, err error) {
// Directory.
u, err := url.Parse(cmd[i])
if err != nil {
return "", false, fmt.Errorf("url.Parse: %w", err)
//nolint
return "", false, sce.Create(sce.ErrScorecardInternal, fmt.Sprintf("url.Parse: %v", err))
}
return filepath.Join(filepath.Dir(pathfn), path.Base(u.Path)), true, nil
}
@ -150,7 +148,8 @@ func getAWSOutputFile(cmd []string) (pathfn string, ok bool, err error) {
if filepath.Clean(filepath.Dir(ofile)) == filepath.Clean(ofile) {
u, err := url.Parse(ifile)
if err != nil {
return "", false, fmt.Errorf("url.Parse: %w", err)
//nolint
return "", false, sce.Create(sce.ErrScorecardInternal, fmt.Sprintf("url.Parse: %v", err))
}
return filepath.Join(filepath.Dir(ofile), path.Base(u.Path)), true, nil
}
@ -266,7 +265,7 @@ func extractCommand(cmd interface{}) ([]string, bool) {
}
func isFetchPipeExecute(node syntax.Node, cmd, pathfn string,
logf func(s string, f ...interface{})) bool {
dl checker.DetailLogger) bool {
// BinaryCmd {Op=|, X=CallExpr{Args={curl, -s, url}}, Y=CallExpr{Args={bash,}}}.
bc, ok := node.(*syntax.BinaryCmd)
if !ok {
@ -295,8 +294,7 @@ func isFetchPipeExecute(node syntax.Node, cmd, pathfn string,
return false
}
logf("!! frozen-deps/fetch-execute - %v is fetching and executing non-pinned program '%v'",
pathfn, cmd)
dl.Warn("insecure (unpinned) download detected in %v: '%v'", pathfn, cmd)
return true
}
@ -322,7 +320,7 @@ func getRedirectFile(red []*syntax.Redirect) (string, bool) {
}
func isExecuteFiles(node syntax.Node, cmd, pathfn string, files map[string]bool,
logf func(s string, f ...interface{})) bool {
dl checker.DetailLogger) bool {
ce, ok := node.(*syntax.CallExpr)
if !ok {
return false
@ -336,8 +334,7 @@ func isExecuteFiles(node syntax.Node, cmd, pathfn string, files map[string]bool,
ok = false
for fn := range files {
if isInterpreterWithFile(c, fn) || isExecuteFile(c, fn) {
logf("!! frozen-deps/fetch-execute - %v is fetching and executing non-pinned program '%v'",
pathfn, cmd)
dl.Warn("insecure (unpinned) download detected in %v: '%v'", pathfn, cmd)
ok = true
}
}
@ -479,7 +476,7 @@ func isPipUnpinnedDownload(cmd []string) bool {
}
func isUnpinnedPakageManagerDownload(node syntax.Node, cmd, pathfn string,
logf func(s string, f ...interface{})) bool {
dl checker.DetailLogger) bool {
ce, ok := node.(*syntax.CallExpr)
if !ok {
return false
@ -492,15 +489,13 @@ func isUnpinnedPakageManagerDownload(node syntax.Node, cmd, pathfn string,
// Go get/install.
if isGoUnpinnedDownload(c) {
logf("!! frozen-deps/fetch-execute - %v is fetching an non-pinned dependency '%v'",
pathfn, cmd)
dl.Warn("insecure (unpinned) download detected in %v: '%v'", pathfn, cmd)
return true
}
// Pip install.
if isPipUnpinnedDownload(c) {
logf("!! frozen-deps/fetch-execute - %v is fetching an non-pinned dependency '%v'",
pathfn, cmd)
dl.Warn("insecure (unpinned) download detected in %v: '%v'", pathfn, cmd)
return true
}
@ -533,7 +528,7 @@ func recordFetchFileFromNode(node syntax.Node) (pathfn string, ok bool, err erro
}
func isFetchProcSubsExecute(node syntax.Node, cmd, pathfn string,
logf func(s string, f ...interface{})) bool {
dl checker.DetailLogger) bool {
ce, ok := node.(*syntax.CallExpr)
if !ok {
return false
@ -583,8 +578,7 @@ func isFetchProcSubsExecute(node syntax.Node, cmd, pathfn string,
return false
}
logf("!! frozen-deps/fetch-execute - %v is fetching and executing non-pinned program '%v'",
pathfn, cmd)
dl.Warn("insecure (unpinned) download detected in %v: '%v'", pathfn, cmd)
return true
}
@ -655,17 +649,20 @@ func nodeToString(p *syntax.Printer, node syntax.Node) (string, error) {
err := p.Print(&buf, node)
// This is ugly, but the parser does not have a defined error type :/.
if err != nil && !strings.Contains(err.Error(), "unsupported node type") {
return "", fmt.Errorf("syntax.Printer.Print: %w", err)
//nolint
return "", sce.Create(sce.ErrScorecardInternal, fmt.Sprintf("syntax.Printer.Print: %v", err))
}
return buf.String(), nil
}
func validateShellFileAndRecord(pathfn string, content []byte, files map[string]bool,
logf func(s string, f ...interface{})) (bool, error) {
dl checker.DetailLogger) (bool, error) {
in := strings.NewReader(string(content))
f, err := syntax.NewParser().Parse(in, "")
if err != nil {
return false, ErrParsingShellCommand
//nolint
return false, sce.Create(sce.ErrScorecardInternal,
fmt.Sprintf("%v: %v", errInternalInvalidShellCode, err))
}
printer := syntax.NewPrinter()
@ -682,7 +679,7 @@ func validateShellFileAndRecord(pathfn string, content []byte, files map[string]
c, ok := extractInterpreterCommandFromNode(node)
// nolinter
if ok {
ok, e := validateShellFileAndRecord(pathfn, []byte(c), files, logf)
ok, e := validateShellFileAndRecord(pathfn, []byte(c), files, dl)
validated = ok
if e != nil {
err = e
@ -691,23 +688,23 @@ func validateShellFileAndRecord(pathfn string, content []byte, files map[string]
}
// `curl | bash` (supports `sudo`).
if isFetchPipeExecute(node, cmdStr, pathfn, logf) {
if isFetchPipeExecute(node, cmdStr, pathfn, dl) {
validated = false
}
// Check if we're calling a file we previously downloaded.
// Includes `curl > /tmp/file [&&|;] [bash] /tmp/file`
if isExecuteFiles(node, cmdStr, pathfn, files, logf) {
if isExecuteFiles(node, cmdStr, pathfn, files, dl) {
validated = false
}
// `bash <(wget -qO- http://website.com/my-script.sh)`. (supports `sudo`).
if isFetchProcSubsExecute(node, cmdStr, pathfn, logf) {
if isFetchProcSubsExecute(node, cmdStr, pathfn, dl) {
validated = false
}
// Package manager's unpinned installs.
if isUnpinnedPakageManagerDownload(node, cmdStr, pathfn, logf) {
if isUnpinnedPakageManagerDownload(node, cmdStr, pathfn, dl) {
validated = false
}
// TODO(laurent): add check for cat file | bash.
@ -783,7 +780,7 @@ func isShellScriptFile(pathfn string, content []byte) bool {
return false
}
func validateShellFile(pathfn string, content []byte, logf func(s string, f ...interface{})) (bool, error) {
func validateShellFile(pathfn string, content []byte, dl checker.DetailLogger) (bool, error) {
files := make(map[string]bool)
return validateShellFileAndRecord(pathfn, content, files, logf)
return validateShellFileAndRecord(pathfn, content, files, dl)
}

View File

@ -23,4 +23,10 @@ RUN CGO_ENABLED=0 make build-scorecard
FROM build
RUN /hello-world
FROM base as base2
RUN ls
FROM base2
RUN ls
FROM python@sha256:45b23dee08af5e43a7fea6c4cf9c25ccf269ee113168c19722f87876677c5cb2

View File

@ -22,13 +22,15 @@ import (
"github.com/ossf/scorecard/checker"
"github.com/ossf/scorecard/checks"
sce "github.com/ossf/scorecard/errors"
scut "github.com/ossf/scorecard/utests"
)
var _ = Describe("E2E TEST:Branch Protection", func() {
Context("E2E TEST:Validating branch protection", func() {
It("Should fail to return branch protection on other repositories", func() {
l := log{}
checkRequest := checker.CheckRequest{
dl := scut.TestDetailLogger{}
req := checker.CheckRequest{
Ctx: context.Background(),
Client: ghClient,
HTTPClient: httpClient,
@ -36,11 +38,22 @@ var _ = Describe("E2E TEST:Branch Protection", func() {
Owner: "apache",
Repo: "airflow",
GraphClient: graphClient,
Logf: l.Logf,
Dlogger: &dl,
}
result := checks.BranchProtection(&checkRequest)
expected := scut.TestReturn{
Errors: []error{sce.ErrScorecardInternal},
Score: checker.InconclusiveResultScore,
NumberOfWarn: 0,
NumberOfInfo: 0,
NumberOfDebug: 0,
}
result := checks.BranchProtection(&req)
// UPGRADEv2: to remove.
// Old version.
Expect(result.Error).ShouldNot(BeNil())
Expect(result.Pass).Should(BeFalse())
// New version.
Expect(scut.ValidateTestReturn(nil, "branch protection not accessible", &expected, &result, &dl)).Should(BeTrue())
})
})
})

View File

@ -12,7 +12,6 @@
// See the License for the specific language governing permissions and
// limitations under the License.
//nolint: dupl // repeating test cases that are slightly different is acceptable
package e2e
import (
@ -24,17 +23,20 @@ import (
"github.com/ossf/scorecard/checker"
"github.com/ossf/scorecard/checks"
"github.com/ossf/scorecard/clients/githubrepo"
scut "github.com/ossf/scorecard/utests"
)
// TODO: use dedicated repo that don't change.
// TODO: need negative results.
var _ = Describe("E2E TEST:FrozenDeps", func() {
Context("E2E TEST:Validating deps are frozen", func() {
It("Should return deps are not frozen", func() {
l := log{}
dl := scut.TestDetailLogger{}
repoClient := githubrepo.CreateGithubRepoClient(context.Background(), ghClient)
err := repoClient.InitRepo("tensorflow", "tensorflow")
Expect(err).Should(BeNil())
checkRequest := checker.CheckRequest{
req := checker.CheckRequest{
Ctx: context.Background(),
Client: ghClient,
HTTPClient: httpClient,
@ -42,19 +44,30 @@ var _ = Describe("E2E TEST:FrozenDeps", func() {
Owner: "tensorflow",
Repo: "tensorflow",
GraphClient: graphClient,
Logf: l.Logf,
Dlogger: &dl,
}
result := checks.FrozenDeps(&checkRequest)
expected := scut.TestReturn{
Errors: nil,
Score: checker.InconclusiveResultScore,
NumberOfWarn: 219,
NumberOfInfo: 0,
NumberOfDebug: 0,
}
result := checks.FrozenDeps(&req)
// UPGRADEv2: to remove.
// Old version.
Expect(result.Error).Should(BeNil())
Expect(result.Pass).Should(BeFalse())
// New version.
Expect(scut.ValidateTestReturn(nil, "deps not frozen", &expected, &result, &dl)).Should(BeTrue())
})
It("Should return deps are frozen", func() {
l := log{}
dl := scut.TestDetailLogger{}
repoClient := githubrepo.CreateGithubRepoClient(context.Background(), ghClient)
err := repoClient.InitRepo("ossf", "scorecard")
Expect(err).Should(BeNil())
checkRequest := checker.CheckRequest{
req := checker.CheckRequest{
Ctx: context.Background(),
Client: ghClient,
HTTPClient: httpClient,
@ -62,11 +75,22 @@ var _ = Describe("E2E TEST:FrozenDeps", func() {
Owner: "ossf",
Repo: "scorecard",
GraphClient: graphClient,
Logf: l.Logf,
Dlogger: &dl,
}
result := checks.FrozenDeps(&checkRequest)
expected := scut.TestReturn{
Errors: nil,
Score: checker.MaxResultScore,
NumberOfWarn: 0,
NumberOfInfo: 1,
NumberOfDebug: 0,
}
result := checks.FrozenDeps(&req)
// UPGRADEv2: to remove.
// Old version.
Expect(result.Error).Should(BeNil())
Expect(result.Pass).Should(BeTrue())
// New version.
Expect(scut.ValidateTestReturn(nil, "deps frozen", &expected, &result, &dl)).Should(BeTrue())
})
})
})

View File

@ -12,7 +12,6 @@
// See the License for the specific language governing permissions and
// limitations under the License.
//nolint:dupl // repeating test cases that are slightly different is acceptable
package e2e
import (