Skip to content

Commit

Permalink
Address comments, add test.
Browse files Browse the repository at this point in the history
  • Loading branch information
jeffmendoza committed Aug 8, 2022
1 parent 5285f50 commit 73c95e7
Show file tree
Hide file tree
Showing 4 changed files with 103 additions and 8 deletions.
1 change: 0 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -341,7 +341,6 @@ for more information on each check.

- Ensure dependabot is enabled.
- Check that dependencies are pinned/frozen.
- More [checks from scorecard](https://github.com/ossf/scorecard/#scorecard-checks).

## **Example Config Repository**

Expand Down
2 changes: 2 additions & 0 deletions pkg/policies/policies.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"github.com/ossf/allstar/pkg/policies/binary"
"github.com/ossf/allstar/pkg/policies/branch"
"github.com/ossf/allstar/pkg/policies/outside"
"github.com/ossf/allstar/pkg/policies/scorecard"
"github.com/ossf/allstar/pkg/policies/security"
"github.com/ossf/allstar/pkg/policies/workflow"
"github.com/ossf/allstar/pkg/policydef"
Expand All @@ -31,6 +32,7 @@ func GetPolicies() []policydef.Policy {
binary.NewBinary(),
branch.NewBranch(),
outside.NewOutside(),
scorecard.NewScorecard(),
security.NewSecurity(),
workflow.NewWorkflow(),
}
Expand Down
25 changes: 18 additions & 7 deletions pkg/policies/scorecard/scorecard.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package scorecard
import (
"context"
"fmt"
"net/http"

"github.com/ossf/allstar/pkg/config"
"github.com/ossf/allstar/pkg/config/operator"
Expand Down Expand Up @@ -83,10 +84,18 @@ type details struct {
}

var configFetchConfig func(context.Context, *github.Client, string, string, string, config.ConfigLevel, interface{}) error
var configIsEnabled func(context.Context, config.OrgOptConfig, config.RepoOptConfig, config.RepoOptConfig, *github.Client, string, string) (bool, error)
var scorecardGet func(context.Context, string, http.RoundTripper) (*scorecard.ScClient, error)

var doNothingOnOptOut = operator.DoNothingOnOptOut

var checksAllChecks checker.CheckNameToFnMap

func init() {
configFetchConfig = config.FetchConfig
configIsEnabled = config.IsEnabled
checksAllChecks = checks.AllChecks
scorecardGet = scorecard.Get
}

// Scorecard is the Security Scorecard policy object, implements
Expand All @@ -109,7 +118,8 @@ func (b Scorecard) Name() string {
func (b Scorecard) Check(ctx context.Context, c *github.Client, owner,
repo string) (*policydef.Result, error) {
oc, orc, rc := getConfig(ctx, c, owner, repo)
enabled, err := config.IsEnabled(ctx, oc.OptConfig, orc.OptConfig, rc.OptConfig, c, owner, repo)
enabled, err := configIsEnabled(ctx, oc.OptConfig, orc.OptConfig,
rc.OptConfig, c, owner, repo)
if err != nil {
return nil, err
}
Expand All @@ -134,7 +144,7 @@ func (b Scorecard) Check(ctx context.Context, c *github.Client, owner,

fullName := fmt.Sprintf("%s/%s", owner, repo)
tr := c.Client().Transport
scc, err := scorecard.Get(ctx, fullName, tr)
scc, err := scorecardGet(ctx, fullName, tr)
if err != nil {
return nil, err
}
Expand All @@ -153,7 +163,7 @@ func (b Scorecard) Check(ctx context.Context, c *github.Client, owner,
Dlogger: l,
}

res := checks.AllChecks[n].Fn(cr)
res := checksAllChecks[n].Fn(cr)
if res.Error != nil {
// We are not sure that all checks are safe to run inside Allstar, some
// might error, and we don't want to abort a whole org enforcement loop
Expand All @@ -174,23 +184,24 @@ func (b Scorecard) Check(ctx context.Context, c *github.Client, owner,
if len(logs) > 0 {
f[n] = logs
}
if res.Score < mc.Threshold {
if res.Score < mc.Threshold && res.Score != checker.InconclusiveResultScore {
pass = false
if notify == "" {
notify = fmt.Sprintf(`Project is out of compliance with Security Scorecards policy
notify = `Project is out of compliance with Security Scorecards policy
**Rule Description**
This is a generic passthrough policy that runs the configured checks from Security Scorecards. Please see the [Security Scorecards Documentation](https://github.com/ossf/scorecard/blob/main/docs/checks.md#dangerous-workflow) for more infomation on each check.
`)
`
}
if len(logs) > 10 {
notify += fmt.Sprintf(
"**First 10 Results from policy: %v : %v **\n\n%v"+
"- Run a Scorecards scan to see full list.\n\n",
res.Name, res.Reason, listJoin(logs[:10]))
} else {
notify += fmt.Sprintf("**Results from policy: %v : %v **\n\n%v\n", res.Name, res.Reason, listJoin(logs))
notify += fmt.Sprintf("**Results from policy: %v : %v **\n\n%v\n",
res.Name, res.Reason, listJoin(logs))
}
}
}
Expand Down
83 changes: 83 additions & 0 deletions pkg/policies/scorecard/scorecard_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,14 @@ package scorecard

import (
"context"
"net/http"
"testing"

"github.com/google/go-cmp/cmp"
"github.com/google/go-github/v43/github"
"github.com/ossf/allstar/pkg/config"
"github.com/ossf/allstar/pkg/scorecard"
"github.com/ossf/scorecard/v4/checker"
)

func TestConfigPrecedence(t *testing.T) {
Expand Down Expand Up @@ -129,3 +132,83 @@ func TestConfigPrecedence(t *testing.T) {
})
}
}

func TestCheck(t *testing.T) {
tests := []struct {
Name string
Org OrgConfig
OrgRepo RepoConfig
Repo RepoConfig
Result checker.CheckResult
ExpPass bool
}{
{
Name: "Pass",
Org: OrgConfig{
Checks: []string{"test"},
Threshold: 10,
},
OrgRepo: RepoConfig{},
Repo: RepoConfig{},
Result: checker.CheckResult{
Score: 10,
},
ExpPass: true,
},
{
Name: "Fail",
Org: OrgConfig{
Checks: []string{"test"},
Threshold: 8,
},
OrgRepo: RepoConfig{},
Repo: RepoConfig{},
Result: checker.CheckResult{
Score: 7,
},
ExpPass: false,
},
}
for _, test := range tests {
t.Run(test.Name, func(t *testing.T) {
configFetchConfig = func(ctx context.Context, c *github.Client, owner,
repo, path string, ol config.ConfigLevel, out interface{}) error {
switch ol {
case config.RepoLevel:
rc := out.(*RepoConfig)
*rc = test.Repo
case config.OrgRepoLevel:
orc := out.(*RepoConfig)
*orc = test.OrgRepo
case config.OrgLevel:
oc := out.(*OrgConfig)
*oc = test.Org
}
return nil
}
configIsEnabled = func(ctx context.Context, o config.OrgOptConfig, orc,
r config.RepoOptConfig, c *github.Client, owner, repo string) (bool,
error) {
return true, nil
}
scorecardGet = func(ctx context.Context, fullRepo string,
tr http.RoundTripper) (*scorecard.ScClient, error) {
return &scorecard.ScClient{}, nil
}
checksAllChecks = checker.CheckNameToFnMap{}
checksAllChecks["test"] = checker.Check{
Fn: func(cr *checker.CheckRequest) checker.CheckResult {
return test.Result
},
}
s := NewScorecard()
res, err := s.Check(context.Background(), github.NewClient(nil), "", "")
if err != nil {
t.Fatalf("Unexpected error: %v", err)
}
if res.Pass != test.ExpPass {
t.Errorf("Expected pass: %v, got: %v", test.ExpPass, res.Pass)
}
})
}
}

0 comments on commit 73c95e7

Please sign in to comment.