Skip to content

Commit bbbbe1e

Browse files
Merge pull request #2548 from vamshi-stepsecurity/cherry-pick/using-secure-repo-token
Cherry pick/using secure repo token
2 parents 06f4c25 + 1922e6e commit bbbbe1e

File tree

6 files changed

+64
-27
lines changed

6 files changed

+64
-27
lines changed

remediation/workflow/hardenrunner/addaction.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,10 @@ func AddAction(inputYaml, action string, pinActions, pinToImmutable bool, skipCo
5151
}
5252

5353
if updated && pinActions {
54-
out, _ = pin.PinAction(action, out, nil, pinToImmutable, nil)
54+
out, _, err = pin.PinAction(action, out, nil, pinToImmutable, nil)
55+
if err != nil {
56+
return out, updated, err
57+
}
5558
}
5659

5760
return out, updated, nil

remediation/workflow/permissions/permissions.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ type SecureWorkflowReponse struct {
2525
WorkflowFetchError bool
2626
JobErrors []JobError
2727
MissingActions []string
28+
UsingSecureRepoPAT bool
2829
}
2930

3031
type JobError struct {

remediation/workflow/pin/pinactions.go

Lines changed: 27 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package pin
33
import (
44
"context"
55
"fmt"
6+
"log"
67
"os"
78
"path/filepath"
89
"regexp"
@@ -29,7 +30,10 @@ func PinActions(inputYaml string, exemptedActions []string, pinToImmutable bool,
2930
for _, step := range job.Steps {
3031
if len(step.Uses) > 0 {
3132
localUpdated := false
32-
out, localUpdated = PinAction(step.Uses, out, exemptedActions, pinToImmutable, actionCommitMap)
33+
out, localUpdated, err = PinAction(step.Uses, out, exemptedActions, pinToImmutable, actionCommitMap)
34+
if err != nil {
35+
return out, updated, err
36+
}
3337
updated = updated || localUpdated
3438
}
3539
}
@@ -38,29 +42,36 @@ func PinActions(inputYaml string, exemptedActions []string, pinToImmutable bool,
3842
return out, updated, nil
3943
}
4044

41-
func PinAction(action, inputYaml string, exemptedActions []string, pinToImmutable bool, actionCommitMap map[string]string) (string, bool) {
42-
45+
func PinAction(action, inputYaml string, exemptedActions []string, pinToImmutable bool, actionCommitMap map[string]string) (string, bool, error) {
4346
updated := false
47+
4448
if !strings.Contains(action, "@") || strings.HasPrefix(action, "docker://") {
45-
return inputYaml, updated // Cannot pin local actions and docker actions
49+
return inputYaml, updated, nil // Cannot pin local actions and docker actions
4650
}
4751

4852
if isAbsolute(action) || (pinToImmutable && IsImmutableAction(action)) {
49-
return inputYaml, updated
53+
return inputYaml, updated, nil
5054
}
5155
leftOfAt := strings.Split(action, "@")
5256
tagOrBranch := leftOfAt[1]
5357

5458
// skip pinning for exempted actions
5559
if ActionExists(leftOfAt[0], exemptedActions) {
56-
return inputYaml, updated
60+
return inputYaml, updated, nil
5761
}
5862

5963
splitOnSlash := strings.Split(leftOfAt[0], "/")
6064
owner := splitOnSlash[0]
6165
repo := splitOnSlash[1]
6266

63-
PAT := os.Getenv("PAT")
67+
// use secure repo token
68+
PAT := os.Getenv("SECURE_REPO_PAT")
69+
if PAT == "" {
70+
PAT = os.Getenv("PAT")
71+
log.Println("SECURE_REPO_PAT is not set, using PAT")
72+
} else {
73+
log.Println("SECURE_REPO_PAT is set")
74+
}
6475

6576
ctx := context.Background()
6677
ts := oauth2.StaticTokenSource(
@@ -81,7 +92,7 @@ func PinAction(action, inputYaml string, exemptedActions []string, pinToImmutabl
8192
if !semanticTagRegex.MatchString(tagOrBranch) {
8293
tagOrBranch, err = getSemanticVersion(client, owner, repo, tagOrBranch, commitSHA)
8394
if err != nil {
84-
return inputYaml, updated
95+
return inputYaml, updated, err
8596
}
8697
}
8798
break
@@ -92,11 +103,11 @@ func PinAction(action, inputYaml string, exemptedActions []string, pinToImmutabl
92103
if commitSHA == "" {
93104
commitSHA, _, err = client.Repositories.GetCommitSHA1(ctx, owner, repo, tagOrBranch, "")
94105
if err != nil {
95-
return inputYaml, updated
106+
return inputYaml, updated, err
96107
}
97108
tagOrBranch, err = getSemanticVersion(client, owner, repo, tagOrBranch, commitSHA)
98109
if err != nil {
99-
return inputYaml, updated
110+
return inputYaml, updated, err
100111
}
101112

102113
}
@@ -130,7 +141,7 @@ func PinAction(action, inputYaml string, exemptedActions []string, pinToImmutabl
130141
inputYaml = actionRegex.ReplaceAllString(inputYaml, pinnedActionWithVersion+"$2")
131142

132143
inputYaml, _ = removePreviousActionComments(pinnedActionWithVersion, inputYaml)
133-
return inputYaml, !strings.EqualFold(action, pinnedActionWithVersion)
144+
return inputYaml, !strings.EqualFold(action, pinnedActionWithVersion), nil
134145
}
135146

136147
updated = !strings.EqualFold(action, fullPinned)
@@ -162,7 +173,7 @@ func PinAction(action, inputYaml string, exemptedActions []string, pinToImmutabl
162173
)
163174
inputYaml, _ = removePreviousActionComments(fullPinned, inputYaml)
164175

165-
return inputYaml, updated
176+
return inputYaml, updated, nil
166177
}
167178

168179
// It may be that there was already a comment next to the action
@@ -263,3 +274,7 @@ func ActionExists(actionName string, patterns []string) bool {
263274
}
264275
return false
265276
}
277+
278+
func UsingSecureRepoPAT() bool {
279+
return os.Getenv("SECURE_REPO_PAT") != ""
280+
}

remediation/workflow/secureworkflow.go

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -148,7 +148,13 @@ func SecureWorkflow(queryStringParams map[string]string, inputYaml string, svc d
148148
log.Printf("Pinning GitHub Actions")
149149
}
150150
pinnedAction, pinnedDocker := false, false
151-
secureWorkflowReponse.FinalOutput, pinnedAction, _ = pin.PinActions(secureWorkflowReponse.FinalOutput, exemptedActions, pinToImmutable, actionCommitMap)
151+
secureWorkflowReponse.FinalOutput, pinnedAction, err = pin.PinActions(secureWorkflowReponse.FinalOutput, exemptedActions, pinToImmutable, actionCommitMap)
152+
if err != nil {
153+
if enableLogging {
154+
log.Printf("Error pinning actions: %v", err)
155+
}
156+
return secureWorkflowReponse, err
157+
}
152158
secureWorkflowReponse.FinalOutput, pinnedDocker, _ = pin.PinDocker(secureWorkflowReponse.FinalOutput)
153159
pinnedActions = pinnedAction || pinnedDocker
154160
if enableLogging {
@@ -179,14 +185,16 @@ func SecureWorkflow(queryStringParams map[string]string, inputYaml string, svc d
179185
secureWorkflowReponse.AddedHardenRunner = addedHardenRunner
180186
secureWorkflowReponse.AddedPermissions = addedPermissions
181187
secureWorkflowReponse.AddedMaintainedActions = replacedMaintainedActions
188+
secureWorkflowReponse.UsingSecureRepoPAT = pin.UsingSecureRepoPAT()
182189

183190
if enableLogging {
184-
log.Printf("SecureWorkflow complete - PinnedActions: %v, AddedHardenRunner: %v, AddedPermissions: %v, AddedMaintainedActions: %v, HasErrors: %v",
191+
log.Printf("SecureWorkflow complete - PinnedActions: %v, AddedHardenRunner: %v, AddedPermissions: %v, AddedMaintainedActions: %v, HasErrors: %v, UsingSecureRepoPAT: %v",
185192
secureWorkflowReponse.PinnedActions,
186193
secureWorkflowReponse.AddedHardenRunner,
187194
secureWorkflowReponse.AddedPermissions,
188195
secureWorkflowReponse.AddedMaintainedActions,
189-
secureWorkflowReponse.HasErrors)
196+
secureWorkflowReponse.HasErrors,
197+
secureWorkflowReponse.UsingSecureRepoPAT)
190198
}
191199

192200
return secureWorkflowReponse, nil

remediation/workflow/secureworkflow_test.go

Lines changed: 20 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -214,16 +214,17 @@ func TestSecureWorkflow(t *testing.T) {
214214
wantAddedHardenRunner bool
215215
wantAddedPermissions bool
216216
wantAddedMaintainedActions bool
217+
wantError bool
217218
}{
218-
{fileName: "oneJob.yml", wantPinnedActions: true, wantAddedHardenRunner: true, wantAddedPermissions: false, wantAddedMaintainedActions: true},
219-
{fileName: "allscenarios.yml", wantPinnedActions: true, wantAddedHardenRunner: true, wantAddedPermissions: true},
220-
{fileName: "missingaction.yml", wantPinnedActions: true, wantAddedHardenRunner: true, wantAddedPermissions: false},
221-
{fileName: "nohardenrunner.yml", wantPinnedActions: true, wantAddedHardenRunner: false, wantAddedPermissions: true},
222-
{fileName: "noperms.yml", wantPinnedActions: true, wantAddedHardenRunner: true, wantAddedPermissions: false},
223-
{fileName: "nopin.yml", wantPinnedActions: false, wantAddedHardenRunner: true, wantAddedPermissions: true},
224-
{fileName: "allperms.yml", wantPinnedActions: false, wantAddedHardenRunner: false, wantAddedPermissions: true},
225-
{fileName: "multiplejobperms.yml", wantPinnedActions: false, wantAddedHardenRunner: false, wantAddedPermissions: true},
226-
{fileName: "error.yml", wantPinnedActions: false, wantAddedHardenRunner: false, wantAddedPermissions: false},
219+
{fileName: "oneJob.yml", wantPinnedActions: true, wantAddedHardenRunner: true, wantAddedPermissions: false, wantAddedMaintainedActions: true, wantError: false},
220+
{fileName: "allscenarios.yml", wantPinnedActions: true, wantAddedHardenRunner: true, wantAddedPermissions: true, wantError: false},
221+
{fileName: "nohardenrunner.yml", wantPinnedActions: true, wantAddedHardenRunner: false, wantAddedPermissions: true, wantError: false},
222+
{fileName: "noperms.yml", wantPinnedActions: true, wantAddedHardenRunner: true, wantAddedPermissions: false, wantError: false},
223+
{fileName: "nopin.yml", wantPinnedActions: false, wantAddedHardenRunner: true, wantAddedPermissions: true, wantError: false},
224+
{fileName: "allperms.yml", wantPinnedActions: false, wantAddedHardenRunner: false, wantAddedPermissions: true, wantError: false},
225+
{fileName: "multiplejobperms.yml", wantPinnedActions: false, wantAddedHardenRunner: false, wantAddedPermissions: true, wantError: false},
226+
{fileName: "error.yml", wantPinnedActions: false, wantAddedHardenRunner: false, wantAddedPermissions: false, wantError: false},
227+
{fileName: "missingaction.yml", wantPinnedActions: false, wantAddedHardenRunner: false, wantAddedPermissions: false, wantError: true},
227228
}
228229
for _, test := range tests {
229230
var err error
@@ -265,12 +266,21 @@ func TestSecureWorkflow(t *testing.T) {
265266
if err != nil {
266267
t.Errorf("unable to load the file %s", err)
267268
}
268-
output, err = SecureWorkflow(queryParams, string(input), &mockDynamoDBClient{}, []string{}, false, actionMap)
269+
output, err = SecureWorkflow(queryParams, string(input), &mockDynamoDBClient{}, []string{"actions/*"}, false, actionMap)
269270
} else {
270271
output, err = SecureWorkflow(queryParams, string(input), &mockDynamoDBClient{})
271272
}
272273

274+
if test.wantError {
275+
if err == nil {
276+
t.Errorf("test failed %s expected an error but got none", test.fileName)
277+
}
278+
// Skip further validation if we expected an error
279+
continue
280+
}
281+
273282
if err != nil {
283+
t.Log(err)
274284
t.Errorf("Error not expected")
275285
}
276286

testfiles/secureworkflow/output/oneJob.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ jobs:
2929
with:
3030
egress-policy: audit
3131

32-
- uses: actions/checkout@544eadc6bf3d226fd7a7a9f0dc5b5bf7ca0675b9 # v1.2.0
32+
- uses: actions/checkout@v1
3333
- uses: github/super-linter@34b2f8032d759425f6b42ea2e52231b33ae05401 # v3.17.1
3434
env:
3535
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}

0 commit comments

Comments
 (0)