Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
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 remediation/workflow/hardenrunner/addaction.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ func AddAction(inputYaml, action string, pinActions, pinToImmutable bool, skipCo
}

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

return out, updated, nil
Expand Down
1 change: 1 addition & 0 deletions remediation/workflow/permissions/permissions.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ type SecureWorkflowReponse struct {
WorkflowFetchError bool
JobErrors []JobError
MissingActions []string
UsedActionCommitMap bool
}

type JobError struct {
Expand Down
34 changes: 24 additions & 10 deletions remediation/workflow/pin/pinactions.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import (
"gopkg.in/yaml.v3"
)

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

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

updated := false
if !strings.Contains(action, "@") || strings.HasPrefix(action, "docker://") {
Expand Down Expand Up @@ -69,15 +69,29 @@ func PinAction(action, inputYaml string, exemptedActions []string, pinToImmutabl
tc := oauth2.NewClient(ctx, ts)

client := github.NewClient(tc)
var commitSHA string
var err error

commitSHA, _, err := client.Repositories.GetCommitSHA1(ctx, owner, repo, tagOrBranch, "")
if err != nil {
return inputYaml, updated
}
if actionCommitMap != nil && actionCommitMap[action] != "" {
Copy link
Member

@ashishkurmi ashishkurmi Oct 16, 2025

Choose a reason for hiding this comment

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

Are GitHub Action references case sensitive? If not, this will not work in case the action is referenced in different case (e.g., actionorg/actionname@v2 and ActionOrg/Actionname@v2 will be treated differently in the current appraoch). If GitHub Action references are case insensitive, we should create a struct and perform case insensitive comparison on action name and/or version tag or use lowercase to create the map and look up operation.

Choose a reason for hiding this comment

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

yes they are case sensitive @ashishkurmi
mkdocs/mkdocs#3567
https://github.com/orgs/community/discussions/27086
only secret names are are case insensitive

actionWithCommit := actionCommitMap[action]
commitSHA = strings.Split(actionWithCommit, "@")[1]

if !semanticTagRegex.MatchString(tagOrBranch) {
tagOrBranch, err = getSemanticVersion(client, owner, repo, tagOrBranch, commitSHA)
if err != nil {
return inputYaml, updated
}
}
} else {
commitSHA, _, err = client.Repositories.GetCommitSHA1(ctx, owner, repo, tagOrBranch, "")
if err != nil {
return inputYaml, updated
}
tagOrBranch, err = getSemanticVersion(client, owner, repo, tagOrBranch, commitSHA)
if err != nil {
return inputYaml, updated
}

tagOrBranch, err = getSemanticVersion(client, owner, repo, tagOrBranch, commitSHA)
if err != nil {
return inputYaml, updated
}

// pinnedAction := fmt.Sprintf("%s@%s # %s", leftOfAt[0], commitSHA, tagOrBranch)
Expand Down
31 changes: 30 additions & 1 deletion remediation/workflow/pin/pinactions_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -239,6 +239,21 @@ func TestPinActions(t *testing.T) {
}
})

httpmock.RegisterResponder("GET", "https://api.github.com/repos/peter-evans-test/close-issue/commits/v1",
httpmock.NewStringResponder(200, `a700eac5bf2a1c7a8cb6da0c13f93ed96fd53dbe`))

httpmock.RegisterResponder("GET", "https://api.github.com/repos/peter-evans-test/close-issue/git/matching-refs/tags/v1.",
httpmock.NewStringResponder(200,
`[
{
"ref": "refs/tags/v1.0.4",
"object": {
"sha": "a700eac5bf2a1c7a8cb6da0c13f93ed96fd53vam",
"type": "commit"
}
}
]`))

// Mock manifest endpoints for specific versions and commit hashes
manifestResponders := []string{
// the following list will contain the list of actions with versions
Expand Down Expand Up @@ -296,15 +311,29 @@ func TestPinActions(t *testing.T) {
{fileName: "exemptaction.yml", wantUpdated: true, exemptedActions: []string{"actions/checkout", "rohith/*"}, pinToImmutable: true},
{fileName: "donotpintoimmutable.yml", wantUpdated: true, pinToImmutable: false},
{fileName: "invertedcommas.yml", wantUpdated: true, pinToImmutable: false},
{fileName: "pinusingmap.yml", wantUpdated: true, pinToImmutable: true},
}
for _, tt := range tests {

var output string
var gotUpdated bool
var err error
var actionCommitMap map[string]string

input, err := ioutil.ReadFile(path.Join(inputDirectory, tt.fileName))

if err != nil {
log.Fatal(err)
}

output, gotUpdated, err := PinActions(string(input), tt.exemptedActions, tt.pinToImmutable)
if tt.fileName == "pinusingmap.yml" {
actionCommitMap = map[string]string{
"peter-evans-test/close-issue@v1": "peter-evans-test/close-issue@a700eac5bf2a1c7a8cb6da0c13f93ed96fd53vam",
"peter-check/[email protected]": "peter-check/close-issue@a700eac5bf2a1c7a8cb6da0c13f93ed96fd53tom",
}
}

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

func SecureWorkflow(queryStringParams map[string]string, inputYaml string, svc dynamodbiface.DynamoDBAPI, params ...interface{}) (*permissions.SecureWorkflowReponse, error) {
pinActions, addHardenRunner, addPermissions, addProjectComment, replaceMaintainedActions := true, true, true, true, false
pinnedActions, addedHardenRunner, addedPermissions, replacedMaintainedActions := false, false, false, false
pinActions, addHardenRunner, addPermissions, addProjectComment, replaceMaintainedActions, useActionCommitMap := true, true, true, true, false, false
pinnedActions, addedHardenRunner, addedPermissions, replacedMaintainedActions, usedActionCommitMap := false, false, false, false, false
ignoreMissingKBs := false
enableLogging := false
addEmptyTopLevelPermissions := false
skipHardenRunnerForContainers := false
exemptedActions, pinToImmutable, maintainedActionsMap := []string{}, false, map[string]string{}
exemptedActions, pinToImmutable, maintainedActionsMap, actionCommitMap := []string{}, false, map[string]string{}, map[string]string{}

if len(params) > 0 {
if v, ok := params[0].([]string); ok {
Expand All @@ -41,6 +41,11 @@ func SecureWorkflow(queryStringParams map[string]string, inputYaml string, svc d
maintainedActionsMap = v
}
}
if len(params) > 3 {
if v, ok := params[3].(map[string]string); ok {
actionCommitMap = v
}
}

if queryStringParams["pinActions"] == "false" {
pinActions = false
Expand Down Expand Up @@ -138,12 +143,22 @@ func SecureWorkflow(queryStringParams map[string]string, inputYaml string, svc d
}
}

if useActionCommitMap {
if enableLogging {
log.Printf("Using action commit map")
}
secureWorkflowReponse.FinalOutput, usedActionCommitMap, err = pin.PinActions(secureWorkflowReponse.FinalOutput, []string{}, false, actionCommitMap)
if err != nil {
log.Printf("Error pinning actions using commit map: %v", err)
secureWorkflowReponse.HasErrors = true
}
}
if pinActions {
if enableLogging {
log.Printf("Pinning GitHub Actions")
}
pinnedAction, pinnedDocker := false, false
secureWorkflowReponse.FinalOutput, pinnedAction, _ = pin.PinActions(secureWorkflowReponse.FinalOutput, exemptedActions, pinToImmutable)
secureWorkflowReponse.FinalOutput, pinnedAction, _ = pin.PinActions(secureWorkflowReponse.FinalOutput, exemptedActions, pinToImmutable, nil)
secureWorkflowReponse.FinalOutput, pinnedDocker, _ = pin.PinDocker(secureWorkflowReponse.FinalOutput)
pinnedActions = pinnedAction || pinnedDocker
if enableLogging {
Expand Down Expand Up @@ -174,12 +189,15 @@ func SecureWorkflow(queryStringParams map[string]string, inputYaml string, svc d
secureWorkflowReponse.AddedHardenRunner = addedHardenRunner
secureWorkflowReponse.AddedPermissions = addedPermissions
secureWorkflowReponse.AddedMaintainedActions = replacedMaintainedActions
secureWorkflowReponse.UsedActionCommitMap = usedActionCommitMap

if enableLogging {
log.Printf("SecureWorkflow complete - PinnedActions: %v, AddedHardenRunner: %v, AddedPermissions: %v, HasErrors: %v",
log.Printf("SecureWorkflow complete - PinnedActions: %v, AddedHardenRunner: %v, AddedPermissions: %v, AddedMaintainedActions: %v, UsedActionCommitMap: %v, HasErrors: %v",
secureWorkflowReponse.PinnedActions,
secureWorkflowReponse.AddedHardenRunner,
secureWorkflowReponse.AddedPermissions,
secureWorkflowReponse.AddedMaintainedActions,
secureWorkflowReponse.UsedActionCommitMap,
secureWorkflowReponse.HasErrors)
}

Expand Down
28 changes: 28 additions & 0 deletions testfiles/pinactions/input/pinusingmap.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
name: "close issue"

on:
push:


jobs:
closeissue:
runs-on: ubuntu-latest

steps:
- name: Close Issue
uses: peter-evans-test/close-issue@v1
with:
issue-number: 1
comment: Auto-closing issue

- name: Close Issue
uses: peter-evans/close-issue@v1
with:
issue-number: 1
comment: Auto-closing issue

- name: Close Issue
uses: peter-check/[email protected]
with:
issue-number: 1
comment: Auto-closing issue
28 changes: 28 additions & 0 deletions testfiles/pinactions/output/pinusingmap.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
name: "close issue"

on:
push:


jobs:
closeissue:
runs-on: ubuntu-latest

steps:
- name: Close Issue
uses: peter-evans-test/close-issue@a700eac5bf2a1c7a8cb6da0c13f93ed96fd53vam # v1.0.4
with:
issue-number: 1
comment: Auto-closing issue

- name: Close Issue
uses: peter-evans/close-issue@a700eac5bf2a1c7a8cb6da0c13f93ed96fd53dbe # v1.0.3
with:
issue-number: 1
comment: Auto-closing issue

- name: Close Issue
uses: peter-check/close-issue@a700eac5bf2a1c7a8cb6da0c13f93ed96fd53tom # v1.2.3
with:
issue-number: 1
comment: Auto-closing issue