diff --git a/remediation/workflow/hardenrunner/addaction.go b/remediation/workflow/hardenrunner/addaction.go index c47d2e35..bb0eb7e6 100644 --- a/remediation/workflow/hardenrunner/addaction.go +++ b/remediation/workflow/hardenrunner/addaction.go @@ -51,7 +51,10 @@ func AddAction(inputYaml, action string, pinActions, pinToImmutable bool, skipCo } if updated && pinActions { - out, _ = pin.PinAction(action, out, nil, pinToImmutable, nil) + out, _, err = pin.PinAction(action, out, nil, pinToImmutable, nil) + if err != nil { + return out, updated, err + } } return out, updated, nil diff --git a/remediation/workflow/permissions/permissions.go b/remediation/workflow/permissions/permissions.go index 018eea47..2cbee465 100644 --- a/remediation/workflow/permissions/permissions.go +++ b/remediation/workflow/permissions/permissions.go @@ -25,6 +25,7 @@ type SecureWorkflowReponse struct { WorkflowFetchError bool JobErrors []JobError MissingActions []string + UsingSecureRepoPAT bool } type JobError struct { diff --git a/remediation/workflow/pin/pinactions.go b/remediation/workflow/pin/pinactions.go index 8d16b211..a7285493 100644 --- a/remediation/workflow/pin/pinactions.go +++ b/remediation/workflow/pin/pinactions.go @@ -3,6 +3,7 @@ package pin import ( "context" "fmt" + "log" "os" "path/filepath" "regexp" @@ -29,7 +30,10 @@ func PinActions(inputYaml string, exemptedActions []string, pinToImmutable bool, for _, step := range job.Steps { if len(step.Uses) > 0 { localUpdated := false - out, localUpdated = PinAction(step.Uses, out, exemptedActions, pinToImmutable, actionCommitMap) + out, localUpdated, err = PinAction(step.Uses, out, exemptedActions, pinToImmutable, actionCommitMap) + if err != nil { + return out, updated, err + } updated = updated || localUpdated } } @@ -38,29 +42,36 @@ func PinActions(inputYaml string, exemptedActions []string, pinToImmutable bool, return out, updated, nil } -func PinAction(action, inputYaml string, exemptedActions []string, pinToImmutable bool, actionCommitMap map[string]string) (string, bool) { - +func PinAction(action, inputYaml string, exemptedActions []string, pinToImmutable bool, actionCommitMap map[string]string) (string, bool, error) { updated := false + if !strings.Contains(action, "@") || strings.HasPrefix(action, "docker://") { - return inputYaml, updated // Cannot pin local actions and docker actions + return inputYaml, updated, nil // Cannot pin local actions and docker actions } if isAbsolute(action) || (pinToImmutable && IsImmutableAction(action)) { - return inputYaml, updated + return inputYaml, updated, nil } leftOfAt := strings.Split(action, "@") tagOrBranch := leftOfAt[1] // skip pinning for exempted actions if ActionExists(leftOfAt[0], exemptedActions) { - return inputYaml, updated + return inputYaml, updated, nil } splitOnSlash := strings.Split(leftOfAt[0], "/") owner := splitOnSlash[0] repo := splitOnSlash[1] - PAT := os.Getenv("PAT") + // use secure repo token + PAT := os.Getenv("SECURE_REPO_PAT") + if PAT == "" { + PAT = os.Getenv("PAT") + log.Println("SECURE_REPO_PAT is not set, using PAT") + } else { + log.Println("SECURE_REPO_PAT is set") + } ctx := context.Background() ts := oauth2.StaticTokenSource( @@ -81,7 +92,7 @@ func PinAction(action, inputYaml string, exemptedActions []string, pinToImmutabl if !semanticTagRegex.MatchString(tagOrBranch) { tagOrBranch, err = getSemanticVersion(client, owner, repo, tagOrBranch, commitSHA) if err != nil { - return inputYaml, updated + return inputYaml, updated, err } } break @@ -92,11 +103,11 @@ func PinAction(action, inputYaml string, exemptedActions []string, pinToImmutabl if commitSHA == "" { commitSHA, _, err = client.Repositories.GetCommitSHA1(ctx, owner, repo, tagOrBranch, "") if err != nil { - return inputYaml, updated + return inputYaml, updated, err } tagOrBranch, err = getSemanticVersion(client, owner, repo, tagOrBranch, commitSHA) if err != nil { - return inputYaml, updated + return inputYaml, updated, err } } @@ -130,7 +141,7 @@ func PinAction(action, inputYaml string, exemptedActions []string, pinToImmutabl inputYaml = actionRegex.ReplaceAllString(inputYaml, pinnedActionWithVersion+"$2") inputYaml, _ = removePreviousActionComments(pinnedActionWithVersion, inputYaml) - return inputYaml, !strings.EqualFold(action, pinnedActionWithVersion) + return inputYaml, !strings.EqualFold(action, pinnedActionWithVersion), nil } updated = !strings.EqualFold(action, fullPinned) @@ -162,7 +173,7 @@ func PinAction(action, inputYaml string, exemptedActions []string, pinToImmutabl ) inputYaml, _ = removePreviousActionComments(fullPinned, inputYaml) - return inputYaml, updated + return inputYaml, updated, nil } // It may be that there was already a comment next to the action @@ -263,3 +274,7 @@ func ActionExists(actionName string, patterns []string) bool { } return false } + +func UsingSecureRepoPAT() bool { + return os.Getenv("SECURE_REPO_PAT") != "" +} diff --git a/remediation/workflow/secureworkflow.go b/remediation/workflow/secureworkflow.go index 71025561..2d2779e3 100644 --- a/remediation/workflow/secureworkflow.go +++ b/remediation/workflow/secureworkflow.go @@ -148,7 +148,13 @@ func SecureWorkflow(queryStringParams map[string]string, inputYaml string, svc d log.Printf("Pinning GitHub Actions") } pinnedAction, pinnedDocker := false, false - secureWorkflowReponse.FinalOutput, pinnedAction, _ = pin.PinActions(secureWorkflowReponse.FinalOutput, exemptedActions, pinToImmutable, actionCommitMap) + secureWorkflowReponse.FinalOutput, pinnedAction, err = pin.PinActions(secureWorkflowReponse.FinalOutput, exemptedActions, pinToImmutable, actionCommitMap) + if err != nil { + if enableLogging { + log.Printf("Error pinning actions: %v", err) + } + return secureWorkflowReponse, err + } secureWorkflowReponse.FinalOutput, pinnedDocker, _ = pin.PinDocker(secureWorkflowReponse.FinalOutput) pinnedActions = pinnedAction || pinnedDocker if enableLogging { @@ -179,14 +185,16 @@ func SecureWorkflow(queryStringParams map[string]string, inputYaml string, svc d secureWorkflowReponse.AddedHardenRunner = addedHardenRunner secureWorkflowReponse.AddedPermissions = addedPermissions secureWorkflowReponse.AddedMaintainedActions = replacedMaintainedActions + secureWorkflowReponse.UsingSecureRepoPAT = pin.UsingSecureRepoPAT() if enableLogging { - log.Printf("SecureWorkflow complete - PinnedActions: %v, AddedHardenRunner: %v, AddedPermissions: %v, AddedMaintainedActions: %v, HasErrors: %v", + log.Printf("SecureWorkflow complete - PinnedActions: %v, AddedHardenRunner: %v, AddedPermissions: %v, AddedMaintainedActions: %v, HasErrors: %v, UsingSecureRepoPAT: %v", secureWorkflowReponse.PinnedActions, secureWorkflowReponse.AddedHardenRunner, secureWorkflowReponse.AddedPermissions, secureWorkflowReponse.AddedMaintainedActions, - secureWorkflowReponse.HasErrors) + secureWorkflowReponse.HasErrors, + secureWorkflowReponse.UsingSecureRepoPAT) } return secureWorkflowReponse, nil diff --git a/remediation/workflow/secureworkflow_test.go b/remediation/workflow/secureworkflow_test.go index da0d8454..80b9945a 100644 --- a/remediation/workflow/secureworkflow_test.go +++ b/remediation/workflow/secureworkflow_test.go @@ -214,16 +214,17 @@ func TestSecureWorkflow(t *testing.T) { wantAddedHardenRunner bool wantAddedPermissions bool wantAddedMaintainedActions bool + wantError bool }{ - {fileName: "oneJob.yml", wantPinnedActions: true, wantAddedHardenRunner: true, wantAddedPermissions: false, wantAddedMaintainedActions: true}, - {fileName: "allscenarios.yml", wantPinnedActions: true, wantAddedHardenRunner: true, wantAddedPermissions: true}, - {fileName: "missingaction.yml", wantPinnedActions: true, wantAddedHardenRunner: true, wantAddedPermissions: false}, - {fileName: "nohardenrunner.yml", wantPinnedActions: true, wantAddedHardenRunner: false, wantAddedPermissions: true}, - {fileName: "noperms.yml", wantPinnedActions: true, wantAddedHardenRunner: true, wantAddedPermissions: false}, - {fileName: "nopin.yml", wantPinnedActions: false, wantAddedHardenRunner: true, wantAddedPermissions: true}, - {fileName: "allperms.yml", wantPinnedActions: false, wantAddedHardenRunner: false, wantAddedPermissions: true}, - {fileName: "multiplejobperms.yml", wantPinnedActions: false, wantAddedHardenRunner: false, wantAddedPermissions: true}, - {fileName: "error.yml", wantPinnedActions: false, wantAddedHardenRunner: false, wantAddedPermissions: false}, + {fileName: "oneJob.yml", wantPinnedActions: true, wantAddedHardenRunner: true, wantAddedPermissions: false, wantAddedMaintainedActions: true, wantError: false}, + {fileName: "allscenarios.yml", wantPinnedActions: true, wantAddedHardenRunner: true, wantAddedPermissions: true, wantError: false}, + {fileName: "nohardenrunner.yml", wantPinnedActions: true, wantAddedHardenRunner: false, wantAddedPermissions: true, wantError: false}, + {fileName: "noperms.yml", wantPinnedActions: true, wantAddedHardenRunner: true, wantAddedPermissions: false, wantError: false}, + {fileName: "nopin.yml", wantPinnedActions: false, wantAddedHardenRunner: true, wantAddedPermissions: true, wantError: false}, + {fileName: "allperms.yml", wantPinnedActions: false, wantAddedHardenRunner: false, wantAddedPermissions: true, wantError: false}, + {fileName: "multiplejobperms.yml", wantPinnedActions: false, wantAddedHardenRunner: false, wantAddedPermissions: true, wantError: false}, + {fileName: "error.yml", wantPinnedActions: false, wantAddedHardenRunner: false, wantAddedPermissions: false, wantError: false}, + {fileName: "missingaction.yml", wantPinnedActions: false, wantAddedHardenRunner: false, wantAddedPermissions: false, wantError: true}, } for _, test := range tests { var err error @@ -265,12 +266,21 @@ func TestSecureWorkflow(t *testing.T) { if err != nil { t.Errorf("unable to load the file %s", err) } - output, err = SecureWorkflow(queryParams, string(input), &mockDynamoDBClient{}, []string{}, false, actionMap) + output, err = SecureWorkflow(queryParams, string(input), &mockDynamoDBClient{}, []string{"actions/*"}, false, actionMap) } else { output, err = SecureWorkflow(queryParams, string(input), &mockDynamoDBClient{}) } + if test.wantError { + if err == nil { + t.Errorf("test failed %s expected an error but got none", test.fileName) + } + // Skip further validation if we expected an error + continue + } + if err != nil { + t.Log(err) t.Errorf("Error not expected") } diff --git a/testfiles/secureworkflow/output/oneJob.yml b/testfiles/secureworkflow/output/oneJob.yml index e8d874fd..ada285de 100644 --- a/testfiles/secureworkflow/output/oneJob.yml +++ b/testfiles/secureworkflow/output/oneJob.yml @@ -29,7 +29,7 @@ jobs: with: egress-policy: audit - - uses: actions/checkout@544eadc6bf3d226fd7a7a9f0dc5b5bf7ca0675b9 # v1.2.0 + - uses: actions/checkout@v1 - uses: github/super-linter@34b2f8032d759425f6b42ea2e52231b33ae05401 # v3.17.1 env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}