From a9f2ba076c08107f07c51ad81bf9b3731a88a00d Mon Sep 17 00:00:00 2001 From: Varun Sharma Date: Sat, 22 Apr 2023 21:56:08 -0700 Subject: [PATCH] Release v1.3.2 (#2095) --- .github/workflows/codeql.yml | 2 +- .github/workflows/kbanalysis.yml | 2 +- .../workflow/hardenrunner/addaction.go | 2 +- .../workflow/permissions/permissions.go | 3 ++ remediation/workflow/pin/pinactions.go | 28 ++++++++++++++++ remediation/workflow/pin/pinactions_test.go | 2 ++ remediation/workflow/secureworkflow_test.go | 1 + .../addaction/input/alreadypresent_2.yml | 2 +- testfiles/addaction/output/2jobs.yml | 4 +-- testfiles/addaction/output/action-issues.yml | 2 +- testfiles/addaction/output/alreadypresent.yml | 2 +- .../addaction/output/alreadypresent_2.yml | 2 +- .../pinactions/input/actionwithcomment.yml | 28 ++++++++++++++++ .../input/repeatedactionwithcomment.yml | 33 +++++++++++++++++++ .../pinactions/output/actionwithcomment.yml | 28 ++++++++++++++++ .../output/repeatedactionwithcomment.yml | 33 +++++++++++++++++++ testfiles/secureworkflow/input/error.yml | 0 .../secureworkflow/output/allscenarios.yml | 2 +- testfiles/secureworkflow/output/error.yml | 0 .../secureworkflow/output/missingaction.yml | 2 +- testfiles/secureworkflow/output/noperms.yml | 2 +- testfiles/secureworkflow/output/nopin.yml | 2 +- 22 files changed, 169 insertions(+), 13 deletions(-) create mode 100644 testfiles/pinactions/input/actionwithcomment.yml create mode 100644 testfiles/pinactions/input/repeatedactionwithcomment.yml create mode 100644 testfiles/pinactions/output/actionwithcomment.yml create mode 100644 testfiles/pinactions/output/repeatedactionwithcomment.yml create mode 100644 testfiles/secureworkflow/input/error.yml create mode 100644 testfiles/secureworkflow/output/error.yml diff --git a/.github/workflows/codeql.yml b/.github/workflows/codeql.yml index 25a92ea4..62d466be 100644 --- a/.github/workflows/codeql.yml +++ b/.github/workflows/codeql.yml @@ -43,7 +43,7 @@ jobs: - name: Harden Runner uses: step-security/harden-runner@ebacdc22ef6c2cfb85ee5ded8f2e640f4c776dd5 with: - egress-policy: audit # TODO: change to 'egress-policy: block' after couple of runs + egress-policy: audit - name: Checkout repository uses: actions/checkout@2541b1294d2704b0964813337f33b291d3f8596b diff --git a/.github/workflows/kbanalysis.yml b/.github/workflows/kbanalysis.yml index d13f88de..6d846e15 100644 --- a/.github/workflows/kbanalysis.yml +++ b/.github/workflows/kbanalysis.yml @@ -24,7 +24,7 @@ jobs: - name: Harden Runner uses: step-security/harden-runner@ebacdc22ef6c2cfb85ee5ded8f2e640f4c776dd5 with: - egress-policy: audit # TODO: change to 'egress-policy: block' after couple of runs + egress-policy: audit - uses: actions/checkout@d0651293c4a5a52e711f25b41b05b2212f385d28 with: diff --git a/remediation/workflow/hardenrunner/addaction.go b/remediation/workflow/hardenrunner/addaction.go index f3564f08..15703d02 100644 --- a/remediation/workflow/hardenrunner/addaction.go +++ b/remediation/workflow/hardenrunner/addaction.go @@ -83,7 +83,7 @@ func addAction(inputYaml, jobName, action string) (string, error) { output = append(output, spaces+fmt.Sprintf("- name: %s", HardenRunnerActionName)) output = append(output, spaces+fmt.Sprintf(" uses: %s", action)) output = append(output, spaces+" with:") - output = append(output, spaces+" egress-policy: audit # TODO: change to 'egress-policy: block' after couple of runs") + output = append(output, spaces+" egress-policy: audit") output = append(output, "") for i := jobNode.Line - 1; i < len(inputLines); i++ { diff --git a/remediation/workflow/permissions/permissions.go b/remediation/workflow/permissions/permissions.go index f51d4e83..b26d63a3 100644 --- a/remediation/workflow/permissions/permissions.go +++ b/remediation/workflow/permissions/permissions.go @@ -101,6 +101,9 @@ func AddWorkflowLevelPermissions(inputYaml string, addProjectComment bool) (stri line := 0 column := 0 topNode := t.Content + if len(topNode) == 0 { + return inputYaml, fmt.Errorf("Workflow file provided is Empty") + } for _, n := range topNode[0].Content { if n.Value == "jobs" && n.Tag == "!!str" { line = n.Line diff --git a/remediation/workflow/pin/pinactions.go b/remediation/workflow/pin/pinactions.go index cdefef8a..b8aef31c 100644 --- a/remediation/workflow/pin/pinactions.go +++ b/remediation/workflow/pin/pinactions.go @@ -76,6 +76,34 @@ func PinAction(action, inputYaml string) (string, bool) { pinnedAction := fmt.Sprintf("%s@%s # %s", leftOfAt[0], commitSHA, tagOrBranch) updated = !strings.EqualFold(action, pinnedAction) inputYaml = strings.ReplaceAll(inputYaml, action, pinnedAction) + yamlWithPreviousActionCommentsRemoved, wasModified := removePreviousActionComments(pinnedAction, inputYaml) + if wasModified { + return yamlWithPreviousActionCommentsRemoved, updated + } + return inputYaml, updated +} + +// It may be that there was already a comment next to the action +// In this case we want to remove the earlier comment +// we add a comment with the Action version so dependabot/ renovatebot can update it +// if there was no comment next to any action, updated will be false +func removePreviousActionComments(pinnedAction, inputYaml string) (string, bool) { + updated := false + stringParts := strings.Split(inputYaml, pinnedAction) + if len(stringParts) > 1 { + inputYaml = "" + inputYaml = stringParts[0] + for idx := 1; idx < len(stringParts); idx++ { + trimmedString := strings.SplitN(stringParts[idx], "\n", 2) + if len(trimmedString) > 1 { + if strings.Contains(trimmedString[0], "#") { + updated = true + } + inputYaml = inputYaml + pinnedAction + "\n" + trimmedString[1] + } + } + } + return inputYaml, updated } diff --git a/remediation/workflow/pin/pinactions_test.go b/remediation/workflow/pin/pinactions_test.go index 1eb15b17..a4872be7 100644 --- a/remediation/workflow/pin/pinactions_test.go +++ b/remediation/workflow/pin/pinactions_test.go @@ -182,6 +182,8 @@ func TestPinActions(t *testing.T) { {fileName: "basic.yml", wantUpdated: true}, {fileName: "dockeraction.yml", wantUpdated: true}, {fileName: "multipleactions.yml", wantUpdated: true}, + {fileName: "actionwithcomment.yml", wantUpdated: true}, + {fileName: "repeatedactionwithcomment.yml", wantUpdated: true}, } for _, tt := range tests { input, err := ioutil.ReadFile(path.Join(inputDirectory, tt.fileName)) diff --git a/remediation/workflow/secureworkflow_test.go b/remediation/workflow/secureworkflow_test.go index c311b675..9b5baa8b 100644 --- a/remediation/workflow/secureworkflow_test.go +++ b/remediation/workflow/secureworkflow_test.go @@ -120,6 +120,7 @@ func TestSecureWorkflow(t *testing.T) { {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}, } for _, test := range tests { input, err := ioutil.ReadFile(path.Join(inputDirectory, test.fileName)) diff --git a/testfiles/addaction/input/alreadypresent_2.yml b/testfiles/addaction/input/alreadypresent_2.yml index 9ef1e918..d82449ae 100644 --- a/testfiles/addaction/input/alreadypresent_2.yml +++ b/testfiles/addaction/input/alreadypresent_2.yml @@ -13,6 +13,6 @@ jobs: - name: Harden Runner uses: step-security/harden-runner@v2 with: - egress-policy: audit # TODO: change to 'egress-policy: block' after couple of runs + egress-policy: audit - run: ls -R \ No newline at end of file diff --git a/testfiles/addaction/output/2jobs.yml b/testfiles/addaction/output/2jobs.yml index 1f708e4b..ce942a75 100644 --- a/testfiles/addaction/output/2jobs.yml +++ b/testfiles/addaction/output/2jobs.yml @@ -8,7 +8,7 @@ jobs: - name: Harden Runner uses: step-security/harden-runner@v2 with: - egress-policy: audit # TODO: change to 'egress-policy: block' after couple of runs + egress-policy: audit - run: ls -R list-directory1: @@ -17,6 +17,6 @@ jobs: - name: Harden Runner uses: step-security/harden-runner@v2 with: - egress-policy: audit # TODO: change to 'egress-policy: block' after couple of runs + egress-policy: audit - run: ls -R \ No newline at end of file diff --git a/testfiles/addaction/output/action-issues.yml b/testfiles/addaction/output/action-issues.yml index d4cb09f2..0596710d 100644 --- a/testfiles/addaction/output/action-issues.yml +++ b/testfiles/addaction/output/action-issues.yml @@ -12,7 +12,7 @@ jobs: - name: Harden Runner uses: step-security/harden-runner@v2 with: - egress-policy: audit # TODO: change to 'egress-policy: block' after couple of runs + egress-policy: audit - name: Close Issue uses: peter-evans/close-issue@v1 diff --git a/testfiles/addaction/output/alreadypresent.yml b/testfiles/addaction/output/alreadypresent.yml index 9ef1e918..d82449ae 100644 --- a/testfiles/addaction/output/alreadypresent.yml +++ b/testfiles/addaction/output/alreadypresent.yml @@ -13,6 +13,6 @@ jobs: - name: Harden Runner uses: step-security/harden-runner@v2 with: - egress-policy: audit # TODO: change to 'egress-policy: block' after couple of runs + egress-policy: audit - run: ls -R \ No newline at end of file diff --git a/testfiles/addaction/output/alreadypresent_2.yml b/testfiles/addaction/output/alreadypresent_2.yml index 9ef1e918..d82449ae 100644 --- a/testfiles/addaction/output/alreadypresent_2.yml +++ b/testfiles/addaction/output/alreadypresent_2.yml @@ -13,6 +13,6 @@ jobs: - name: Harden Runner uses: step-security/harden-runner@v2 with: - egress-policy: audit # TODO: change to 'egress-policy: block' after couple of runs + egress-policy: audit - run: ls -R \ No newline at end of file diff --git a/testfiles/pinactions/input/actionwithcomment.yml b/testfiles/pinactions/input/actionwithcomment.yml new file mode 100644 index 00000000..91b31023 --- /dev/null +++ b/testfiles/pinactions/input/actionwithcomment.yml @@ -0,0 +1,28 @@ +name: "close issue" + +on: + push: + +jobs: + closeissue: + runs-on: ubuntu-latest + + steps: + - name: Close Issue + uses: peter-evans/close-issue@v1 #Mock comment to remove + with: + issue-number: 1 + comment: Auto-closing issue + publish: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v1 #Mock Comment + - uses: actions/setup-node@v1 #Mock Comment + with: + node-version: 10 + - run: npm install + - run: npm test + - uses: JS-DevTools/npm-publish@v1 #Mock Comment + with: + token: ${{ secrets.GITHUB_TOKEN }} + registry: https://npm.pkg.github.com diff --git a/testfiles/pinactions/input/repeatedactionwithcomment.yml b/testfiles/pinactions/input/repeatedactionwithcomment.yml new file mode 100644 index 00000000..59c8ae95 --- /dev/null +++ b/testfiles/pinactions/input/repeatedactionwithcomment.yml @@ -0,0 +1,33 @@ +name: "close issue" + +on: + push: + +jobs: + closeissue: + runs-on: ubuntu-latest + + steps: + - name: Close Issue + uses: peter-evans/close-issue@v1 #Mock comment to remove + with: + issue-number: 1 + comment: Auto-closing issue + publish: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v1 #Mock Comment + - uses: actions/setup-node@v1 #Mock Comment + with: + node-version: 10 + - run: npm install + - run: npm test + - uses: JS-DevTools/npm-publish@v1 #Mock Comment + with: + token: ${{ secrets.GITHUB_TOKEN }} + registry: https://npm.pkg.github.com + - name: Close Issue + uses: peter-evans/close-issue@v1 #Mock comment to remove + with: + issue-number: 1 + comment: Auto-closing issue diff --git a/testfiles/pinactions/output/actionwithcomment.yml b/testfiles/pinactions/output/actionwithcomment.yml new file mode 100644 index 00000000..a1131c83 --- /dev/null +++ b/testfiles/pinactions/output/actionwithcomment.yml @@ -0,0 +1,28 @@ +name: "close issue" + +on: + push: + +jobs: + closeissue: + runs-on: ubuntu-latest + + steps: + - name: Close Issue + uses: peter-evans/close-issue@a700eac5bf2a1c7a8cb6da0c13f93ed96fd53dbe # v1.0.3 + with: + issue-number: 1 + comment: Auto-closing issue + publish: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@544eadc6bf3d226fd7a7a9f0dc5b5bf7ca0675b9 # v1.2.0 + - uses: actions/setup-node@f1f314fca9dfce2769ece7d933488f076716723e # v1.4.6 + with: + node-version: 10 + - run: npm install + - run: npm test + - uses: JS-DevTools/npm-publish@0f451a94170d1699fd50710966d48fb26194d939 # v1.4.3 + with: + token: ${{ secrets.GITHUB_TOKEN }} + registry: https://npm.pkg.github.com diff --git a/testfiles/pinactions/output/repeatedactionwithcomment.yml b/testfiles/pinactions/output/repeatedactionwithcomment.yml new file mode 100644 index 00000000..dbd50839 --- /dev/null +++ b/testfiles/pinactions/output/repeatedactionwithcomment.yml @@ -0,0 +1,33 @@ +name: "close issue" + +on: + push: + +jobs: + closeissue: + runs-on: ubuntu-latest + + steps: + - name: Close Issue + uses: peter-evans/close-issue@a700eac5bf2a1c7a8cb6da0c13f93ed96fd53dbe # v1.0.3 + with: + issue-number: 1 + comment: Auto-closing issue + publish: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@544eadc6bf3d226fd7a7a9f0dc5b5bf7ca0675b9 # v1.2.0 + - uses: actions/setup-node@f1f314fca9dfce2769ece7d933488f076716723e # v1.4.6 + with: + node-version: 10 + - run: npm install + - run: npm test + - uses: JS-DevTools/npm-publish@0f451a94170d1699fd50710966d48fb26194d939 # v1.4.3 + with: + token: ${{ secrets.GITHUB_TOKEN }} + registry: https://npm.pkg.github.com + - name: Close Issue + uses: peter-evans/close-issue@a700eac5bf2a1c7a8cb6da0c13f93ed96fd53dbe # v1.0.3 + with: + issue-number: 1 + comment: Auto-closing issue diff --git a/testfiles/secureworkflow/input/error.yml b/testfiles/secureworkflow/input/error.yml new file mode 100644 index 00000000..e69de29b diff --git a/testfiles/secureworkflow/output/allscenarios.yml b/testfiles/secureworkflow/output/allscenarios.yml index e2326923..99e0bb51 100644 --- a/testfiles/secureworkflow/output/allscenarios.yml +++ b/testfiles/secureworkflow/output/allscenarios.yml @@ -17,7 +17,7 @@ jobs: - name: Harden Runner uses: step-security/harden-runner@ebacdc22ef6c2cfb85ee5ded8f2e640f4c776dd5 # v2.0.0 with: - egress-policy: audit # TODO: change to 'egress-policy: block' after couple of runs + egress-policy: audit - uses: actions/checkout@544eadc6bf3d226fd7a7a9f0dc5b5bf7ca0675b9 # v1.2.0 - uses: github/super-linter@34b2f8032d759425f6b42ea2e52231b33ae05401 # v3.17.1 diff --git a/testfiles/secureworkflow/output/error.yml b/testfiles/secureworkflow/output/error.yml new file mode 100644 index 00000000..e69de29b diff --git a/testfiles/secureworkflow/output/missingaction.yml b/testfiles/secureworkflow/output/missingaction.yml index 2d7bffa0..20305eff 100644 --- a/testfiles/secureworkflow/output/missingaction.yml +++ b/testfiles/secureworkflow/output/missingaction.yml @@ -11,7 +11,7 @@ jobs: - name: Harden Runner uses: step-security/harden-runner@ebacdc22ef6c2cfb85ee5ded8f2e640f4c776dd5 # v2.0.0 with: - egress-policy: audit # TODO: change to 'egress-policy: block' after couple of runs + egress-policy: audit - uses: actions/missingaction@v2 - uses: github/super-linter@34b2f8032d759425f6b42ea2e52231b33ae05401 # v3.17.1 diff --git a/testfiles/secureworkflow/output/noperms.yml b/testfiles/secureworkflow/output/noperms.yml index b6bf8824..4112b73a 100644 --- a/testfiles/secureworkflow/output/noperms.yml +++ b/testfiles/secureworkflow/output/noperms.yml @@ -11,7 +11,7 @@ jobs: - name: Harden Runner uses: step-security/harden-runner@ebacdc22ef6c2cfb85ee5ded8f2e640f4c776dd5 # v2.0.0 with: - egress-policy: audit # TODO: change to 'egress-policy: block' after couple of runs + egress-policy: audit - uses: actions/checkout@544eadc6bf3d226fd7a7a9f0dc5b5bf7ca0675b9 # v1.2.0 - uses: github/super-linter@34b2f8032d759425f6b42ea2e52231b33ae05401 # v3.17.1 diff --git a/testfiles/secureworkflow/output/nopin.yml b/testfiles/secureworkflow/output/nopin.yml index ecd0d298..41b19b7d 100644 --- a/testfiles/secureworkflow/output/nopin.yml +++ b/testfiles/secureworkflow/output/nopin.yml @@ -17,7 +17,7 @@ jobs: - name: Harden Runner uses: step-security/harden-runner@v2 with: - egress-policy: audit # TODO: change to 'egress-policy: block' after couple of runs + egress-policy: audit - uses: actions/checkout@v1 - uses: github/super-linter@v3