Skip to content

Commit

Permalink
Merge pull request #2502 from step-security/feature/exclude_pin_actio…
Browse files Browse the repository at this point in the history
…ns_main

Feature/exclude pin actions main -> main
  • Loading branch information
varunsh-coder authored Jan 31, 2025
2 parents af18f63 + 38df01f commit 84135ad
Show file tree
Hide file tree
Showing 9 changed files with 189 additions and 24 deletions.
4 changes: 2 additions & 2 deletions remediation/workflow/hardenrunner/addaction.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ const (
HardenRunnerActionName = "Harden Runner"
)

func AddAction(inputYaml, action string, pinActions bool) (string, bool, error) {
func AddAction(inputYaml, action string, pinActions, pinToImmutable bool) (string, bool, error) {
workflow := metadata.Workflow{}
updated := false
err := yaml.Unmarshal([]byte(inputYaml), &workflow)
Expand Down Expand Up @@ -47,7 +47,7 @@ func AddAction(inputYaml, action string, pinActions bool) (string, bool, error)
}

if updated && pinActions {
out, _ = pin.PinAction(action, out)
out, _ = pin.PinAction(action, out, nil, pinToImmutable)
}

return out, updated, nil
Expand Down
2 changes: 1 addition & 1 deletion remediation/workflow/hardenrunner/addaction_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ func TestAddAction(t *testing.T) {
if err != nil {
t.Fatalf("error reading test file")
}
got, gotUpdated, err := AddAction(string(input), tt.args.action, false)
got, gotUpdated, err := AddAction(string(input), tt.args.action, false, false)

if gotUpdated != tt.wantUpdated {
t.Errorf("AddAction() updated = %v, wantUpdated %v", gotUpdated, tt.wantUpdated)
Expand Down
33 changes: 28 additions & 5 deletions remediation/workflow/pin/pinactions.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"context"
"fmt"
"os"
"path/filepath"
"regexp"
"strings"

Expand All @@ -13,7 +14,7 @@ import (
"gopkg.in/yaml.v3"
)

func PinActions(inputYaml string) (string, bool, error) {
func PinActions(inputYaml string, exemptedActions []string, pinToImmutable bool) (string, bool, error) {
workflow := metadata.Workflow{}
updated := false
err := yaml.Unmarshal([]byte(inputYaml), &workflow)
Expand All @@ -28,7 +29,7 @@ func PinActions(inputYaml string) (string, bool, error) {
for _, step := range job.Steps {
if len(step.Uses) > 0 {
localUpdated := false
out, localUpdated = PinAction(step.Uses, out)
out, localUpdated = PinAction(step.Uses, out, exemptedActions, pinToImmutable)
updated = updated || localUpdated
}
}
Expand All @@ -37,19 +38,24 @@ func PinActions(inputYaml string) (string, bool, error) {
return out, updated, nil
}

func PinAction(action, inputYaml string) (string, bool) {
func PinAction(action, inputYaml string, exemptedActions []string, pinToImmutable bool) (string, bool) {

updated := false
if !strings.Contains(action, "@") || strings.HasPrefix(action, "docker://") {
return inputYaml, updated // Cannot pin local actions and docker actions
}

if isAbsolute(action) || IsImmutableAction(action) {
if isAbsolute(action) || (pinToImmutable && IsImmutableAction(action)) {
return inputYaml, updated
}
leftOfAt := strings.Split(action, "@")
tagOrBranch := leftOfAt[1]

// skip pinning for exempted actions
if actionExists(leftOfAt[0], exemptedActions) {
return inputYaml, updated
}

splitOnSlash := strings.Split(leftOfAt[0], "/")
owner := splitOnSlash[0]
repo := splitOnSlash[1]
Expand Down Expand Up @@ -78,7 +84,7 @@ func PinAction(action, inputYaml string) (string, bool) {

// if the action with version is immutable, then pin the action with version instead of sha
pinnedActionWithVersion := fmt.Sprintf("%s@%s", leftOfAt[0], tagOrBranch)
if semanticTagRegex.MatchString(tagOrBranch) && IsImmutableAction(pinnedActionWithVersion) {
if pinToImmutable && semanticTagRegex.MatchString(tagOrBranch) && IsImmutableAction(pinnedActionWithVersion) {
pinnedAction = pinnedActionWithVersion
}

Expand Down Expand Up @@ -188,3 +194,20 @@ func getSemanticVersion(client *github.Client, owner, repo, tagOrBranch, commitS
}
return tagOrBranch, nil
}

// Function to check if an action matches any pattern in the list
func actionExists(actionName string, patterns []string) bool {
for _, pattern := range patterns {
// Use filepath.Match to match the pattern
matched, err := filepath.Match(pattern, actionName)
if err != nil {
// Handle invalid patterns
fmt.Printf("Error matching pattern: %v\n", err)
continue
}
if matched {
return true
}
}
return false
}
45 changes: 32 additions & 13 deletions remediation/workflow/pin/pinactions_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,21 @@ func TestPinActions(t *testing.T) {
}
]`))

httpmock.RegisterResponder("GET", "https://api.github.com/repos/github/codeql-action/commits/v3.28.2",
httpmock.NewStringResponder(200, `d68b2d4edb4189fd2a5366ac14e72027bd4b37dd`))

httpmock.RegisterResponder("GET", "https://api.github.com/repos/github/codeql-action/git/matching-refs/tags/v3.28.2.",
httpmock.NewStringResponder(200,
`[
{
"ref": "refs/tags/v3.28.2",
"object": {
"sha": "d68b2d4edb4189fd2a5366ac14e72027bd4b37dd",
"type": "commit"
}
}
]`))

// mock ping response
httpmock.RegisterResponder("GET", "https://ghcr.io/v2/",
httpmock.NewStringResponder(200, ``))
Expand Down Expand Up @@ -263,19 +278,23 @@ func TestPinActions(t *testing.T) {
})

tests := []struct {
fileName string
wantUpdated bool
fileName string
wantUpdated bool
exemptedActions []string
pinToImmutable bool
}{
{fileName: "alreadypinned.yml", wantUpdated: false},
{fileName: "branch.yml", wantUpdated: true},
{fileName: "localaction.yml", wantUpdated: true},
{fileName: "multiplejobs.yml", wantUpdated: true},
{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},
{fileName: "immutableaction-1.yml", wantUpdated: true},
{fileName: "alreadypinned.yml", wantUpdated: false, pinToImmutable: true},
{fileName: "branch.yml", wantUpdated: true, pinToImmutable: true},
{fileName: "localaction.yml", wantUpdated: true, pinToImmutable: true},
{fileName: "multiplejobs.yml", wantUpdated: true, pinToImmutable: true},
{fileName: "basic.yml", wantUpdated: true, pinToImmutable: true},
{fileName: "dockeraction.yml", wantUpdated: true, pinToImmutable: true},
{fileName: "multipleactions.yml", wantUpdated: true, pinToImmutable: true},
{fileName: "actionwithcomment.yml", wantUpdated: true, pinToImmutable: true},
{fileName: "repeatedactionwithcomment.yml", wantUpdated: true, pinToImmutable: true},
{fileName: "immutableaction-1.yml", wantUpdated: true, pinToImmutable: true},
{fileName: "exemptaction.yml", wantUpdated: true, exemptedActions: []string{"actions/checkout", "rohith/*"}, pinToImmutable: true},
{fileName: "donotpintoimmutable.yml", wantUpdated: true, pinToImmutable: false},
}
for _, tt := range tests {
input, err := ioutil.ReadFile(path.Join(inputDirectory, tt.fileName))
Expand All @@ -284,7 +303,7 @@ func TestPinActions(t *testing.T) {
log.Fatal(err)
}

output, gotUpdated, err := PinActions(string(input))
output, gotUpdated, err := PinActions(string(input), tt.exemptedActions, tt.pinToImmutable)
if tt.wantUpdated != gotUpdated {
t.Errorf("test failed wantUpdated %v did not match gotUpdated %v", tt.wantUpdated, gotUpdated)
}
Expand Down
17 changes: 14 additions & 3 deletions remediation/workflow/secureworkflow.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,21 @@ const (
HardenRunnerActionName = "Harden Runner"
)

func SecureWorkflow(queryStringParams map[string]string, inputYaml string, svc dynamodbiface.DynamoDBAPI) (*permissions.SecureWorkflowReponse, error) {
func SecureWorkflow(queryStringParams map[string]string, inputYaml string, svc dynamodbiface.DynamoDBAPI, params ...interface{}) (*permissions.SecureWorkflowReponse, error) {
pinActions, addHardenRunner, addPermissions, addProjectComment := true, true, true, true
pinnedActions, addedHardenRunner, addedPermissions := false, false, false
ignoreMissingKBs := false
exemptedActions, pinToImmutable := []string{}, false
if len(params) > 0 {
if v, ok := params[0].([]string); ok {
exemptedActions = v
}
}
if len(params) > 1 {
if v, ok := params[1].(bool); ok {
pinToImmutable = v
}
}

if queryStringParams["pinActions"] == "false" {
pinActions = false
Expand Down Expand Up @@ -68,13 +79,13 @@ func SecureWorkflow(queryStringParams map[string]string, inputYaml string, svc d

if pinActions {
pinnedAction, pinnedDocker := false, false
secureWorkflowReponse.FinalOutput, pinnedAction, _ = pin.PinActions(secureWorkflowReponse.FinalOutput)
secureWorkflowReponse.FinalOutput, pinnedAction, _ = pin.PinActions(secureWorkflowReponse.FinalOutput, exemptedActions, pinToImmutable)
secureWorkflowReponse.FinalOutput, pinnedDocker, _ = pin.PinDocker(secureWorkflowReponse.FinalOutput)
pinnedActions = pinnedAction || pinnedDocker
}

if addHardenRunner {
secureWorkflowReponse.FinalOutput, addedHardenRunner, _ = hardenrunner.AddAction(secureWorkflowReponse.FinalOutput, HardenRunnerActionPathWithTag, pinActions)
secureWorkflowReponse.FinalOutput, addedHardenRunner, _ = hardenrunner.AddAction(secureWorkflowReponse.FinalOutput, HardenRunnerActionPathWithTag, pinActions, pinToImmutable)
}

// Setting appropriate flags
Expand Down
12 changes: 12 additions & 0 deletions testfiles/pinactions/input/donotpintoimmutable.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
name: Integration Test Github
on: [push]
jobs:
build:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v1
- uses: github/codeql-action/analyze@v3.28.2
- uses: borales/actions-yarn@v2.3.0
with:
auth-token: ${{ secrets.GITHUB_TOKEN }}
registry-url: npm.pkg.github.com
44 changes: 44 additions & 0 deletions testfiles/pinactions/input/exemptaction.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
name: publish to nuget
on:
push:
branches:
- master # Default release branch
jobs:
publish:
name: build, pack & publish
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v1

# - name: Setup dotnet
# uses: actions/setup-dotnet@v1
# with:
# dotnet-version: 3.1.200

# Publish
- name: publish on version change
id: publish_nuget
uses: brandedoutcast/publish-nuget@v2
with:
PROJECT_FILE_PATH: Core/Core.csproj
NUGET_KEY: ${{ secrets.GITHUB_TOKEN }}
NUGET_SOURCE: https://nuget.pkg.github.com/OWNER/index.json
publish1:
name: build, pack & publish
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v1

# - name: Setup dotnet
# uses: actions/setup-dotnet@v1
# with:
# dotnet-version: 3.1.200

# Publish
- name: publish on version change
id: publish_nuget
uses: rohith/publish-nuget@v2
with:
PROJECT_FILE_PATH: Core/Core.csproj
NUGET_KEY: ${{ secrets.GITHUB_TOKEN }}
NUGET_SOURCE: https://nuget.pkg.github.com/OWNER/index.json
12 changes: 12 additions & 0 deletions testfiles/pinactions/output/donotpintoimmutable.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
name: Integration Test Github
on: [push]
jobs:
build:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@544eadc6bf3d226fd7a7a9f0dc5b5bf7ca0675b9 # v1.2.0
- uses: github/codeql-action/analyze@d68b2d4edb4189fd2a5366ac14e72027bd4b37dd # v3.28.2
- uses: borales/actions-yarn@4965e1a0f0ae9c422a9a5748ebd1fb5e097d22b9 # v2.3.0
with:
auth-token: ${{ secrets.GITHUB_TOKEN }}
registry-url: npm.pkg.github.com
44 changes: 44 additions & 0 deletions testfiles/pinactions/output/exemptaction.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
name: publish to nuget
on:
push:
branches:
- master # Default release branch
jobs:
publish:
name: build, pack & publish
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v1

# - name: Setup dotnet
# uses: actions/setup-dotnet@v1
# with:
# dotnet-version: 3.1.200

# Publish
- name: publish on version change
id: publish_nuget
uses: brandedoutcast/publish-nuget@c12b8546b67672ee38ac87bea491ac94a587f7cc # v2.5.5
with:
PROJECT_FILE_PATH: Core/Core.csproj
NUGET_KEY: ${{ secrets.GITHUB_TOKEN }}
NUGET_SOURCE: https://nuget.pkg.github.com/OWNER/index.json
publish1:
name: build, pack & publish
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v1

# - name: Setup dotnet
# uses: actions/setup-dotnet@v1
# with:
# dotnet-version: 3.1.200

# Publish
- name: publish on version change
id: publish_nuget
uses: rohith/publish-nuget@v2
with:
PROJECT_FILE_PATH: Core/Core.csproj
NUGET_KEY: ${{ secrets.GITHUB_TOKEN }}
NUGET_SOURCE: https://nuget.pkg.github.com/OWNER/index.json

0 comments on commit 84135ad

Please sign in to comment.