Skip to content

Fix sentence and remove existing action comments #2067

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Apr 13, 2023
Merged

Fix sentence and remove existing action comments #2067

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9cfad7d
Fix typo
arjundashrath Apr 9, 2023
8967abf
Remove action comments when pinning
arjundashrath Apr 10, 2023
8375800
Add more complex testcase
arjundashrath Apr 13, 2023
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: 1 addition & 1 deletion .github/workflows/codeql.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 # TODO: change to 'egress-policy: block' after a couple of runs

- name: Checkout repository
uses: actions/checkout@2541b1294d2704b0964813337f33b291d3f8596b
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/kbanalysis.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 # TODO: change to 'egress-policy: block' after a couple of runs

- uses: actions/checkout@d0651293c4a5a52e711f25b41b05b2212f385d28
with:
Expand Down
2 changes: 1 addition & 1 deletion remediation/workflow/hardenrunner/addaction.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 # TODO: change to 'egress-policy: block' after a couple of runs")
output = append(output, "")

for i := jobNode.Line - 1; i < len(inputLines); i++ {
Expand Down
7 changes: 7 additions & 0 deletions remediation/workflow/pin/pinactions.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,13 @@ 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)
stringParts := strings.SplitN(inputYaml, pinnedAction, 2)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Devils-Knight can you please review this code? I think you had worked on this code recently, so just review this logic once. Also confirm if this change is on the latest from main.

if len(stringParts) > 1 {
trimmedString := strings.SplitN(stringParts[1], "\n", 2)
if len(trimmedString) > 1 {
inputYaml = stringParts[0] + pinnedAction + "\n" + trimmedString[1]
}
}
return inputYaml, updated
}

Expand Down
1 change: 1 addition & 0 deletions remediation/workflow/pin/pinactions_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,7 @@ 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},
}
for _, tt := range tests {
input, err := ioutil.ReadFile(path.Join(inputDirectory, tt.fileName))
Expand Down
2 changes: 1 addition & 1 deletion testfiles/addaction/input/alreadypresent_2.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 # TODO: change to 'egress-policy: block' after a couple of runs

- run: ls -R
4 changes: 2 additions & 2 deletions testfiles/addaction/output/2jobs.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 # TODO: change to 'egress-policy: block' after a couple of runs

- run: ls -R
list-directory1:
Expand All @@ -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 # TODO: change to 'egress-policy: block' after a couple of runs

- run: ls -R
2 changes: 1 addition & 1 deletion testfiles/addaction/output/action-issues.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 # TODO: change to 'egress-policy: block' after a couple of runs

- name: Close Issue
uses: peter-evans/close-issue@v1
Expand Down
2 changes: 1 addition & 1 deletion testfiles/addaction/output/alreadypresent.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 # TODO: change to 'egress-policy: block' after a couple of runs

- run: ls -R
2 changes: 1 addition & 1 deletion testfiles/addaction/output/alreadypresent_2.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 # TODO: change to 'egress-policy: block' after a couple of runs

- run: ls -R
29 changes: 29 additions & 0 deletions testfiles/pinactions/input/actionwithcomment.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@arjundashrath can you please update this test workflow to have two jobs and use the same Action with comment in both jobs? Also, have another Action in one of the jobs with a comment. I want to check if it will fix this for multiple instances and more complex workflows.

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
29 changes: 29 additions & 0 deletions testfiles/pinactions/output/actionwithcomment.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
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
2 changes: 1 addition & 1 deletion testfiles/secureworkflow/output/allscenarios.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 # TODO: change to 'egress-policy: block' after a couple of runs

- uses: actions/checkout@544eadc6bf3d226fd7a7a9f0dc5b5bf7ca0675b9 # v1.2.0
- uses: github/super-linter@34b2f8032d759425f6b42ea2e52231b33ae05401 # v3.17.1
Expand Down
2 changes: 1 addition & 1 deletion testfiles/secureworkflow/output/missingaction.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 # TODO: change to 'egress-policy: block' after a couple of runs

- uses: actions/missingaction@v2
- uses: github/super-linter@34b2f8032d759425f6b42ea2e52231b33ae05401 # v3.17.1
Expand Down
2 changes: 1 addition & 1 deletion testfiles/secureworkflow/output/noperms.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 # TODO: change to 'egress-policy: block' after a couple of runs

- uses: actions/checkout@544eadc6bf3d226fd7a7a9f0dc5b5bf7ca0675b9 # v1.2.0
- uses: github/super-linter@34b2f8032d759425f6b42ea2e52231b33ae05401 # v3.17.1
Expand Down
2 changes: 1 addition & 1 deletion testfiles/secureworkflow/output/nopin.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 # TODO: change to 'egress-policy: block' after a couple of runs

- uses: actions/checkout@v1
- uses: github/super-linter@v3
Expand Down