Skip to content

Commit

Permalink
Add local repo support to allstar. Also update workflow policy to test
Browse files Browse the repository at this point in the history
all branches.

Signed-off-by: Samuel Erb <[email protected]>
  • Loading branch information
serb-google committed Jan 6, 2025
1 parent 13447b7 commit bfdb8da
Show file tree
Hide file tree
Showing 7 changed files with 248 additions and 99 deletions.
2 changes: 2 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -331,6 +331,8 @@ This policy's config file is named `dangerous_workflow.yaml`, and the [config
definitions are
here](https://pkg.go.dev/github.com/ossf/allstar/pkg/policies/workflow#OrgConfig).

This policy will run against **all** branches, see rationale [here](https://github.com/ossf/allstar/issues/569).

This policy checks the GitHub Actions workflow configuration files
(`.github/workflows`), for any patterns that match known dangerous
behavior. See the [OpenSSF Scorecard
Expand Down
2 changes: 1 addition & 1 deletion pkg/policies/binary/binary.go
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ func (b Binary) 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 := scorecard.Get(ctx, fullName, false, tr)
if err != nil {
return nil, err
}
Expand Down
4 changes: 2 additions & 2 deletions pkg/policies/scorecard/scorecard.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ 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 scorecardGet func(context.Context, string, bool, http.RoundTripper) (*scorecard.ScClient, error)
var checksAllChecks checker.CheckNameToFnMap
var scRun func(context.Context, clients.Repo, ...sc.Option) (sc.Result, error)

Expand Down Expand Up @@ -141,7 +141,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 := scorecardGet(ctx, fullName, tr)
scc, err := scorecardGet(ctx, fullName, false, tr)
if err != nil {
return nil, err
}
Expand Down
163 changes: 91 additions & 72 deletions pkg/policies/workflow/workflow.go
Original file line number Diff line number Diff line change
Expand Up @@ -106,86 +106,105 @@ func (b Workflow) 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 := scorecard.Get(ctx, fullName, true, tr)
if err != nil {
return nil, err
}

allRes, err := sc.Run(ctx, scc.ScRepo,
sc.WithRepoClient(scc.ScRepoClient),
sc.WithChecks([]string{checks.CheckDangerousWorkflow}),
)
// Fetch branches and run sc.Run against every branch.
branches, err := scc.FetchBranches()
if err != nil {
msg := "Error while running checks.DangerousWorkflow"
log.Warn().
Str("org", owner).
Str("repo", repo).
Str("area", polName).
Err(err).
Msg(msg)
return &policydef.Result{
Enabled: enabled,
Pass: true,
NotifyText: fmt.Sprintf("%s: %v", msg, err),
Details: details{},
}, nil
}
if len(allRes.Checks) != 1 {
msg := "Error while running checks.DangerousWorkflow : did not get expected checks"
log.Warn().
Str("org", owner).
Str("repo", repo).
Str("area", polName).
Int("chk_len", len(allRes.Checks)).
Msg(msg)
return &policydef.Result{
Enabled: enabled,
Pass: true,
NotifyText: msg,
Details: details{},
}, nil
}
res := allRes.Checks[0]

if res.Error != nil {
msg := "Error while running checks.DangerousWorkflow"
log.Warn().
Str("org", owner).
Str("repo", repo).
Str("area", polName).
Err(res.Error).
Msg(msg)
return &policydef.Result{
Enabled: enabled,
Pass: true,
NotifyText: fmt.Sprintf("%s: %v", msg, res.Error),
Details: details{},
}, nil
return nil, err
}

logs := convertLogs(res.Details)
pass := res.Score >= checker.MaxResultScore || res.Score == checker.InconclusiveResultScore
var notify string
if !pass {
notify = fmt.Sprintf(`Project is out of compliance with Dangerous Workflow policy: %v
**Rule Description**
Dangerous workflows are GitHub Action workflows that exhibit dangerous patterns that could render them vulnerable to attack. A vulnerable workflow is susceptible to leaking repository secrets, or allowing an attacker write access using the GITHUB_TOKEN. For more information about the particular patterns that are detected, see the [OpenSSF Scorecard documentation](https://github.com/ossf/scorecard/blob/main/docs/checks.md#dangerous-workflow) on dangerous workflows.
**Remediation Steps**
Avoid the dangerous workflow patterns. See this [post](https://securitylab.github.com/research/github-actions-preventing-pwn-requests/) for information on avoiding untrusted code checkouts. See this [document](https://docs.github.com/en/actions/security-guides/security-hardening-for-github-actions#understanding-the-risk-of-script-injections) for information on avoiding and mitigating the risk of script injections.
`,
res.Reason)
if len(logs) > 10 {
notify += fmt.Sprintf(
"**First 10 Dangerous Patterns Found**\n\n%v"+
"- Run a Scorecard scan to see full list.\n\n",
listJoin(logs[:10]))
} else {
notify += fmt.Sprintf("**Dangerous Patterns Found**\n\n%v\n", listJoin(logs))
var logs []string
pass := true
for _, branch := range branches {
err = scc.SwitchLocalBranch(branch)
if err != nil {
return nil, err
}

allRes, err := sc.Run(ctx, scc.ScRepo,
sc.WithRepoClient(scc.ScRepoClient),
sc.WithChecks([]string{checks.CheckDangerousWorkflow}),
)
if err != nil {
msg := "Error while running checks.DangerousWorkflow"
log.Warn().
Str("org", owner).
Str("repo", repo).
Str("area", polName).
Err(err).
Msg(msg)
return &policydef.Result{
Enabled: enabled,
Pass: true,
NotifyText: fmt.Sprintf("%s: %v", msg, err),
Details: details{},
}, nil
}
if len(allRes.Checks) != 1 {
msg := "Error while running checks.DangerousWorkflow : did not get expected checks"
log.Warn().
Str("org", owner).
Str("repo", repo).
Str("area", polName).
Int("chk_len", len(allRes.Checks)).
Msg(msg)
return &policydef.Result{
Enabled: enabled,
Pass: true,
NotifyText: msg,
Details: details{},
}, nil
}
res := allRes.Checks[0]

if res.Error != nil {
msg := "Error while running checks.DangerousWorkflow"
log.Warn().
Str("org", owner).
Str("repo", repo).
Str("area", polName).
Err(res.Error).
Msg(msg)
return &policydef.Result{
Enabled: enabled,
Pass: true,
NotifyText: fmt.Sprintf("%s: %v", msg, res.Error),
Details: details{},
}, nil
}

logs = convertLogs(res.Details)
branchPass := res.Score >= checker.MaxResultScore || res.Score == checker.InconclusiveResultScore
if !branchPass {
pass = false
if notify == "" {
notify = fmt.Sprintf(`Project is out of compliance with Dangerous Workflow policy: %v
**Rule Description**
Dangerous workflows are GitHub Action workflows that exhibit dangerous patterns that could render them vulnerable to attack. A vulnerable workflow is susceptible to leaking repository secrets, or allowing an attacker write access using the GITHUB_TOKEN. For more information about the particular patterns that are detected, see the [OpenSSF Scorecard documentation](https://github.com/ossf/scorecard/blob/main/docs/checks.md#dangerous-workflow) on dangerous workflows. Any vulnerable branch can be exploited, so this rule will check all branches (vulnerable list below).
**Remediation Steps**
Avoid the dangerous workflow patterns. See this [post](https://securitylab.github.com/research/github-actions-preventing-pwn-requests/) for information on avoiding untrusted code checkouts. See this [document](https://docs.github.com/en/actions/security-guides/security-hardening-for-github-actions#understanding-the-risk-of-script-injections) for information on avoiding and mitigating the risk of script injections.
`,
res.Reason)
if len(logs) > 10 {
notify += fmt.Sprintf(
"**First 10 Dangerous Patterns Found**\n\n%v"+
"- Run a Scorecard scan to see full list.\n\n",
listJoin(logs[:10]))
} else {
notify += fmt.Sprintf("**Dangerous Patterns Found**\n\n%v\n", listJoin(logs))
}
notify += `**Additional Information**
This policy uses [OpenSSF Scorecard](https://github.com/ossf/scorecard/). You may wish to run a Scorecard scan directly on this repository for more details.
`
}
notify += fmt.Sprintf("\nVulnerable Branch: %s", branch)
}
notify += `**Additional Information**
This policy uses [OpenSSF Scorecard](https://github.com/ossf/scorecard/). You may wish to run a Scorecard scan directly on this repository for more details.`
}

return &policydef.Result{
Expand Down
135 changes: 118 additions & 17 deletions pkg/scorecard/scorecard.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,20 +23,28 @@ package scorecard
import (
"context"
"net/http"
"os"
"sync"

"github.com/go-git/go-git/v5"
"github.com/go-git/go-git/v5/plumbing"
"github.com/ossf/scorecard/v5/clients"
"github.com/ossf/scorecard/v5/clients/githubrepo"
"github.com/ossf/scorecard/v5/clients/localdir"
"github.com/rs/zerolog/log"
)

// Type ScClient is returned from Get. It contains the clients needed to call
// scorecard checks.
type ScClient struct {
ScRepo clients.Repo
ScRepoClient clients.RepoClient
localPath string
gitRepo *git.Repository
}

var scClients map[string]*ScClient = make(map[string]*ScClient)
var scClientsRemote map[string]*ScClient = make(map[string]*ScClient)
var scClientsLocal map[string]*ScClient = make(map[string]*ScClient)
var mMutex sync.RWMutex

const defaultGitRef = "HEAD"
Expand All @@ -52,37 +60,130 @@ func init() {
// Function Get will get the scorecard clients and create them if they don't
// exist. The github repo is initialized, which means the tarball is
// downloaded.
func Get(ctx context.Context, fullRepo string, tr http.RoundTripper) (*ScClient, error) {
// If local is set, a local copy of the repo will be used instead, this allows for branch
// changes without another scorecard repo download.
func Get(ctx context.Context, fullRepo string, local bool, tr http.RoundTripper) (*ScClient, error) {
defer mMutex.Unlock()
mMutex.Lock()
if scc, ok := scClients[fullRepo]; ok {
mMutex.Unlock()
if local {
if scc, ok := scClientsLocal[fullRepo]; ok {
return scc, nil
}
scc, err := createLocal(ctx, fullRepo)
if err != nil {
return nil, err
}
scClientsLocal[fullRepo] = scc
return scc, nil

} else {
// remote
if scc, ok := scClientsRemote[fullRepo]; ok {
return scc, nil
}
scc, err := createRemote(ctx, fullRepo, tr)
if err != nil {
return nil, err
}
scClientsRemote[fullRepo] = scc
return scc, nil
}
}

// Switch the local repo between branches
func (scc ScClient) SwitchLocalBranch(branchName string) error {
log.Info().
Str("branch", branchName).
Msg("Branch checkout")

w, err := scc.gitRepo.Worktree()
if err != nil {
return err
}
err = w.Checkout(&git.CheckoutOptions{Branch: plumbing.ReferenceName(branchName)})
if err != nil {
return err
}
scc, err := create(ctx, fullRepo, tr)
return nil
}

// Fetch branches from the local repo (used for workflow rule checking)
func (scc ScClient) FetchBranches() ([]string, error) {
refs, err := scc.gitRepo.References()
if err != nil {
mMutex.Unlock()
return nil, err
}
scClients[fullRepo] = scc
mMutex.Unlock()
return scc, nil

var ret []string
refs.ForEach(func(ref *plumbing.Reference) error {
if ref.Name().IsRemote() {
ret = append(ret, ref.Name().String())
}
return nil
})
return ret, nil
}

// Checkout a repo into a local directory
// returns the path to the local repo and a git repo reference
func checkoutRepo(fullRepo string) (string, *git.Repository, error) {
log.Info().
Str("repo", fullRepo).
Msg("Repo checkout")
// create a temp dir to store the repo
dir, err := os.MkdirTemp("", "allstar")
if err != nil {
return "", nil, err
}

// checkout to the temp dir
repo, err := git.PlainClone(dir, false, &git.CloneOptions{
URL: "https://github.com/" + fullRepo,
})
if err != nil {
return "", nil, err
}
return dir, repo, nil
}

// Function Close will close the scorecard clients. This cleans up the
// downloaded tarball.
func Close(fullRepo string) {
defer mMutex.Unlock()
mMutex.Lock()
scc, ok := scClients[fullRepo]
if !ok {
mMutex.Unlock()
return
// local
scc, ok := scClientsLocal[fullRepo]
if ok {
// delete local temp directory
os.RemoveAll(scc.localPath)
delete(scClientsLocal, fullRepo)
}
// remote
scc, ok = scClientsRemote[fullRepo]
if ok {
scc.ScRepoClient.Close()
delete(scClientsRemote, fullRepo)
}
scc.ScRepoClient.Close()
delete(scClients, fullRepo)
mMutex.Unlock()
}

func create(ctx context.Context, fullRepo string, tr http.RoundTripper) (*ScClient, error) {
func createLocal(ctx context.Context, fullRepo string) (*ScClient, error) {
localPath, gitRepo, err := checkoutRepo(fullRepo)
if err != nil {
return nil, err
}

scr, err := localdir.MakeLocalDirRepo(localPath)
if err != nil {
return nil, err
}
return &ScClient{
ScRepo: scr,
localPath: localPath,
gitRepo: gitRepo,
}, nil
}

func createRemote(ctx context.Context, fullRepo string, tr http.RoundTripper) (*ScClient, error) {
scr, err := githubrepoMakeGitHubRepo(fullRepo)
if err != nil {
return nil, err
Expand Down
Loading

0 comments on commit bfdb8da

Please sign in to comment.