Skip to content

Commit 54605e8

Browse files
Balijepalli Vamshi KrishnaBalijepalli Vamshi Krishna
authored andcommitted
Merge branch 'int' of https://github.com/vamshi-stepsecurity/secure-repo into bug/wild-card-for-action
2 parents e3ae3be + b713701 commit 54605e8

File tree

11 files changed

+185
-38
lines changed

11 files changed

+185
-38
lines changed

knowledge-base/actions/pullreminders/slack-action/action-security.yml

Lines changed: 0 additions & 2 deletions
This file was deleted.

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)
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: 53 additions & 17 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
"regexp"
89
"strings"
@@ -13,7 +14,7 @@ import (
1314
"gopkg.in/yaml.v3"
1415
)
1516

16-
func PinActions(inputYaml string, exemptedActions []string, pinToImmutable bool) (string, bool, error) {
17+
func PinActions(inputYaml string, exemptedActions []string, pinToImmutable bool, actionCommitMap map[string]string) (string, bool, error) {
1718
workflow := metadata.Workflow{}
1819
updated := false
1920
err := yaml.Unmarshal([]byte(inputYaml), &workflow)
@@ -28,7 +29,10 @@ func PinActions(inputYaml string, exemptedActions []string, pinToImmutable bool)
2829
for _, step := range job.Steps {
2930
if len(step.Uses) > 0 {
3031
localUpdated := false
31-
out, localUpdated = PinAction(step.Uses, out, exemptedActions, pinToImmutable)
32+
out, localUpdated, err = PinAction(step.Uses, out, exemptedActions, pinToImmutable, actionCommitMap)
33+
if err != nil {
34+
return out, updated, err
35+
}
3236
updated = updated || localUpdated
3337
}
3438
}
@@ -37,29 +41,36 @@ func PinActions(inputYaml string, exemptedActions []string, pinToImmutable bool)
3741
return out, updated, nil
3842
}
3943

40-
func PinAction(action, inputYaml string, exemptedActions []string, pinToImmutable bool) (string, bool) {
41-
44+
func PinAction(action, inputYaml string, exemptedActions []string, pinToImmutable bool, actionCommitMap map[string]string) (string, bool, error) {
4245
updated := false
46+
4347
if !strings.Contains(action, "@") || strings.HasPrefix(action, "docker://") {
44-
return inputYaml, updated // Cannot pin local actions and docker actions
48+
return inputYaml, updated, nil // Cannot pin local actions and docker actions
4549
}
4650

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

5357
// skip pinning for exempted actions
5458
if ActionExists(leftOfAt[0], exemptedActions) {
55-
return inputYaml, updated
59+
return inputYaml, updated, nil
5660
}
5761

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

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

6475
ctx := context.Background()
6576
ts := oauth2.StaticTokenSource(
@@ -68,15 +79,36 @@ func PinAction(action, inputYaml string, exemptedActions []string, pinToImmutabl
6879
tc := oauth2.NewClient(ctx, ts)
6980

7081
client := github.NewClient(tc)
71-
72-
commitSHA, _, err := client.Repositories.GetCommitSHA1(ctx, owner, repo, tagOrBranch, "")
73-
if err != nil {
74-
return inputYaml, updated
82+
var commitSHA string
83+
var err error
84+
85+
if actionCommitMap != nil {
86+
// Check case-insensitively by iterating through the map
87+
for mapAction, actionWithCommit := range actionCommitMap {
88+
if strings.EqualFold(action, mapAction) && actionWithCommit != "" {
89+
commitSHA = actionWithCommit
90+
91+
if !semanticTagRegex.MatchString(tagOrBranch) {
92+
tagOrBranch, err = getSemanticVersion(client, owner, repo, tagOrBranch, commitSHA)
93+
if err != nil {
94+
return inputYaml, updated, err
95+
}
96+
}
97+
break
98+
}
99+
}
75100
}
76101

77-
tagOrBranch, err = getSemanticVersion(client, owner, repo, tagOrBranch, commitSHA)
78-
if err != nil {
79-
return inputYaml, updated
102+
if commitSHA == "" {
103+
commitSHA, _, err = client.Repositories.GetCommitSHA1(ctx, owner, repo, tagOrBranch, "")
104+
if err != nil {
105+
return inputYaml, updated, err
106+
}
107+
tagOrBranch, err = getSemanticVersion(client, owner, repo, tagOrBranch, commitSHA)
108+
if err != nil {
109+
return inputYaml, updated, err
110+
}
111+
80112
}
81113

82114
// pinnedAction := fmt.Sprintf("%s@%s # %s", leftOfAt[0], commitSHA, tagOrBranch)
@@ -108,7 +140,7 @@ func PinAction(action, inputYaml string, exemptedActions []string, pinToImmutabl
108140
inputYaml = actionRegex.ReplaceAllString(inputYaml, pinnedActionWithVersion+"$2")
109141

110142
inputYaml, _ = removePreviousActionComments(pinnedActionWithVersion, inputYaml)
111-
return inputYaml, !strings.EqualFold(action, pinnedActionWithVersion)
143+
return inputYaml, !strings.EqualFold(action, pinnedActionWithVersion), nil
112144
}
113145

114146
updated = !strings.EqualFold(action, fullPinned)
@@ -140,7 +172,7 @@ func PinAction(action, inputYaml string, exemptedActions []string, pinToImmutabl
140172
)
141173
inputYaml, _ = removePreviousActionComments(fullPinned, inputYaml)
142174

143-
return inputYaml, updated
175+
return inputYaml, updated, nil
144176
}
145177

146178
// It may be that there was already a comment next to the action
@@ -248,3 +280,7 @@ func ActionExists(actionName string, patterns []string) bool {
248280
}
249281
return false
250282
}
283+
284+
func UsingSecureRepoPAT() bool {
285+
return os.Getenv("SECURE_REPO_PAT") != ""
286+
}

remediation/workflow/pin/pinactions_test.go

Lines changed: 30 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -254,6 +254,21 @@ func TestPinActions(t *testing.T) {
254254
}
255255
})
256256

257+
httpmock.RegisterResponder("GET", "https://api.github.com/repos/peter-evans-test/close-issue/commits/v1",
258+
httpmock.NewStringResponder(200, `a700eac5bf2a1c7a8cb6da0c13f93ed96fd53dbe`))
259+
260+
httpmock.RegisterResponder("GET", "https://api.github.com/repos/peter-evans-test/close-issue/git/matching-refs/tags/v1.",
261+
httpmock.NewStringResponder(200,
262+
`[
263+
{
264+
"ref": "refs/tags/v1.0.4",
265+
"object": {
266+
"sha": "a700eac5bf2a1c7a8cb6da0c13f93ed96fd53vam",
267+
"type": "commit"
268+
}
269+
}
270+
]`))
271+
257272
// Mock manifest endpoints for specific versions and commit hashes
258273
manifestResponders := []string{
259274
// the following list will contain the list of actions with versions
@@ -311,15 +326,29 @@ func TestPinActions(t *testing.T) {
311326
{fileName: "exemptaction.yml", wantUpdated: true, exemptedActions: []string{"actions/checkout", "rohith/*", "praveen/*", "aman-*/*", "*/seperate*", "starc/*"}, pinToImmutable: true},
312327
{fileName: "donotpintoimmutable.yml", wantUpdated: true, pinToImmutable: false},
313328
{fileName: "invertedcommas.yml", wantUpdated: true, pinToImmutable: false},
329+
{fileName: "pinusingmap.yml", wantUpdated: true, pinToImmutable: true},
314330
}
315331
for _, tt := range tests {
332+
333+
var output string
334+
var gotUpdated bool
335+
var err error
336+
var actionCommitMap map[string]string
337+
316338
input, err := ioutil.ReadFile(path.Join(inputDirectory, tt.fileName))
317339

318340
if err != nil {
319341
log.Fatal(err)
320342
}
321343

322-
output, gotUpdated, err := PinActions(string(input), tt.exemptedActions, tt.pinToImmutable)
344+
if tt.fileName == "pinusingmap.yml" {
345+
actionCommitMap = map[string]string{
346+
"peter-evans-test/close-issue@v1": "a700eac5bf2a1c7a8cb6da0c13f93ed96fd53vam",
347+
"peter-check/[email protected]": "a700eac5bf2a1c7a8cb6da0c13f93ed96fd53tom",
348+
}
349+
}
350+
351+
output, gotUpdated, err = PinActions(string(input), tt.exemptedActions, tt.pinToImmutable, actionCommitMap)
323352
if tt.wantUpdated != gotUpdated {
324353
t.Errorf("test failed wantUpdated %v did not match gotUpdated %v", tt.wantUpdated, gotUpdated)
325354
}

remediation/workflow/secureworkflow.go

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ func SecureWorkflow(queryStringParams map[string]string, inputYaml string, svc d
2424
enableLogging := false
2525
addEmptyTopLevelPermissions := false
2626
skipHardenRunnerForContainers := false
27-
exemptedActions, pinToImmutable, maintainedActionsMap := []string{}, false, map[string]string{}
27+
exemptedActions, pinToImmutable, maintainedActionsMap, actionCommitMap := []string{}, false, map[string]string{}, map[string]string{}
2828

2929
if len(params) > 0 {
3030
if v, ok := params[0].([]string); ok {
@@ -41,6 +41,11 @@ func SecureWorkflow(queryStringParams map[string]string, inputYaml string, svc d
4141
maintainedActionsMap = v
4242
}
4343
}
44+
if len(params) > 3 {
45+
if v, ok := params[3].(map[string]string); ok {
46+
actionCommitMap = v
47+
}
48+
}
4449

4550
if queryStringParams["pinActions"] == "false" {
4651
pinActions = false
@@ -143,7 +148,13 @@ func SecureWorkflow(queryStringParams map[string]string, inputYaml string, svc d
143148
log.Printf("Pinning GitHub Actions")
144149
}
145150
pinnedAction, pinnedDocker := false, false
146-
secureWorkflowReponse.FinalOutput, pinnedAction, _ = pin.PinActions(secureWorkflowReponse.FinalOutput, exemptedActions, pinToImmutable)
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+
}
147158
secureWorkflowReponse.FinalOutput, pinnedDocker, _ = pin.PinDocker(secureWorkflowReponse.FinalOutput)
148159
pinnedActions = pinnedAction || pinnedDocker
149160
if enableLogging {
@@ -174,13 +185,16 @@ func SecureWorkflow(queryStringParams map[string]string, inputYaml string, svc d
174185
secureWorkflowReponse.AddedHardenRunner = addedHardenRunner
175186
secureWorkflowReponse.AddedPermissions = addedPermissions
176187
secureWorkflowReponse.AddedMaintainedActions = replacedMaintainedActions
188+
secureWorkflowReponse.UsingSecureRepoPAT = pin.UsingSecureRepoPAT()
177189

178190
if enableLogging {
179-
log.Printf("SecureWorkflow complete - PinnedActions: %v, AddedHardenRunner: %v, AddedPermissions: %v, HasErrors: %v",
191+
log.Printf("SecureWorkflow complete - PinnedActions: %v, AddedHardenRunner: %v, AddedPermissions: %v, AddedMaintainedActions: %v, HasErrors: %v, UsingSecureRepoPAT: %v",
180192
secureWorkflowReponse.PinnedActions,
181193
secureWorkflowReponse.AddedHardenRunner,
182194
secureWorkflowReponse.AddedPermissions,
183-
secureWorkflowReponse.HasErrors)
195+
secureWorkflowReponse.AddedMaintainedActions,
196+
secureWorkflowReponse.HasErrors,
197+
secureWorkflowReponse.UsingSecureRepoPAT)
184198
}
185199

186200
return secureWorkflowReponse, nil

remediation/workflow/secureworkflow_test.go

Lines changed: 22 additions & 12 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: "replaceactions.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
@@ -250,7 +251,7 @@ func TestSecureWorkflow(t *testing.T) {
250251
case "multiplejobperms.yml":
251252
queryParams["addHardenRunner"] = "false"
252253
queryParams["pinActions"] = "false"
253-
case "replaceactions.yml":
254+
case "oneJob.yml":
254255
queryParams["addMaintainedActions"] = "true"
255256
queryParams["addHardenRunner"] = "true"
256257
queryParams["pinActions"] = "true"
@@ -260,17 +261,26 @@ func TestSecureWorkflow(t *testing.T) {
260261

261262
var output *permissions.SecureWorkflowReponse
262263
var actionMap map[string]string
263-
if test.fileName == "replaceactions.yml" {
264+
if test.fileName == "oneJob.yml" {
264265
actionMap, err = maintainedactions.LoadMaintainedActions("maintainedactions/maintainedActions.json")
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

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
name: "close issue"
2+
3+
on:
4+
push:
5+
6+
7+
jobs:
8+
closeissue:
9+
runs-on: ubuntu-latest
10+
11+
steps:
12+
- name: Close Issue
13+
uses: peter-evans-test/close-issue@v1
14+
with:
15+
issue-number: 1
16+
comment: Auto-closing issue
17+
18+
- name: Close Issue
19+
uses: peter-evans/close-issue@v1
20+
with:
21+
issue-number: 1
22+
comment: Auto-closing issue
23+
24+
- name: Close Issue
25+
uses: peter-check/[email protected]
26+
with:
27+
issue-number: 1
28+
comment: Auto-closing issue
Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
name: "close issue"
2+
3+
on:
4+
push:
5+
6+
7+
jobs:
8+
closeissue:
9+
runs-on: ubuntu-latest
10+
11+
steps:
12+
- name: Close Issue
13+
uses: peter-evans-test/close-issue@a700eac5bf2a1c7a8cb6da0c13f93ed96fd53vam # v1.0.4
14+
with:
15+
issue-number: 1
16+
comment: Auto-closing issue
17+
18+
- name: Close Issue
19+
uses: peter-evans/close-issue@a700eac5bf2a1c7a8cb6da0c13f93ed96fd53dbe # v1.0.3
20+
with:
21+
issue-number: 1
22+
comment: Auto-closing issue
23+
24+
- name: Close Issue
25+
uses: peter-check/close-issue@a700eac5bf2a1c7a8cb6da0c13f93ed96fd53tom # v1.2.3
26+
with:
27+
issue-number: 1
28+
comment: Auto-closing issue

0 commit comments

Comments
 (0)