Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update workflow policy to scan all branches for dangerous workflows #569 #622

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
2 changes: 1 addition & 1 deletion pkg/policies/scorecard/scorecard_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,7 @@ func TestCheck(t *testing.T) {
error) {
return true, nil
}
scorecardGet = func(ctx context.Context, fullRepo string,
scorecardGet = func(ctx context.Context, fullRepo string, local bool,
tr http.RoundTripper) (*scorecard.ScClient, error) {
return &scorecard.ScClient{}, nil
}
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
Loading
Loading