Skip to content

Add pinToImmutable configuration #2501

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 2 commits into from
Jan 29, 2025
Merged

Add pinToImmutable configuration #2501

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
89283af
add pinToImmutable configuration
shubham-stepsecurity Jan 29, 2025
a61297d
add exemptedActions & pinToImmutable as optional params
shubham-stepsecurity Jan 29, 2025
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 main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ func (h Handler) Invoke(ctx context.Context, req []byte) ([]byte, error) {
inputYaml = httpRequest.Body
}

fixResponse, err := workflow.SecureWorkflow(httpRequest.QueryStringParameters, nil, inputYaml, dynamoDbSvc)
fixResponse, err := workflow.SecureWorkflow(httpRequest.QueryStringParameters, nil, false, inputYaml, dynamoDbSvc)

if err != nil {
response = events.APIGatewayProxyResponse{
Expand Down
4 changes: 2 additions & 2 deletions remediation/workflow/hardenrunner/addaction.go
View file
Open in desktop
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, nil)
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
View file
Open in desktop
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
10 changes: 5 additions & 5 deletions remediation/workflow/pin/pinactions.go
View file
Open in desktop
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) (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 @@ -29,7 +29,7 @@ func PinActions(inputYaml string, exemptedActions []string) (string, bool, error
for _, step := range job.Steps {
if len(step.Uses) > 0 {
localUpdated := false
out, localUpdated = PinAction(step.Uses, out, exemptedActions)
out, localUpdated = PinAction(step.Uses, out, exemptedActions, pinToImmutable)
updated = updated || localUpdated
}
}
Expand All @@ -38,14 +38,14 @@ func PinActions(inputYaml string, exemptedActions []string) (string, bool, error
return out, updated, nil
}

func PinAction(action, inputYaml string, exemptedActions []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)) {
Copy link
Collaborator

@sailikhith-stepsecurity sailikhith-stepsecurity Jan 29, 2025

Choose a reason for hiding this comment

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

Not sure if we need to add a flag check here. If the action is already immutable, Do we still need to pin it using sha ?
If yes, what we have now works.
If not, we would be pinning the actions that are already immutable.

return inputYaml, updated
}
leftOfAt := strings.Split(action, "@")
Expand Down Expand Up @@ -84,7 +84,7 @@ func PinAction(action, inputYaml string, exemptedActions []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
41 changes: 29 additions & 12 deletions remediation/workflow/pin/pinactions_test.go
View file
Open in desktop
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 @@ -266,18 +281,20 @@ func TestPinActions(t *testing.T) {
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: "exemptaction.yml", wantUpdated: true, exemptedActions: []string{"actions/checkout", "rohith/*"}},
{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 @@ -286,7 +303,7 @@ func TestPinActions(t *testing.T) {
log.Fatal(err)
}

output, gotUpdated, err := PinActions(string(input), tt.exemptedActions)
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
6 changes: 3 additions & 3 deletions remediation/workflow/secureworkflow.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ const (
HardenRunnerActionName = "Harden Runner"
)

func SecureWorkflow(queryStringParams map[string]string, exemptedActions []string, inputYaml string, svc dynamodbiface.DynamoDBAPI) (*permissions.SecureWorkflowReponse, error) {
func SecureWorkflow(queryStringParams map[string]string, exemptedActions []string, pinToImmutable bool, inputYaml string, svc dynamodbiface.DynamoDBAPI) (*permissions.SecureWorkflowReponse, error) {
pinActions, addHardenRunner, addPermissions, addProjectComment := true, true, true, true
pinnedActions, addedHardenRunner, addedPermissions := false, false, false
ignoreMissingKBs := false
Expand Down Expand Up @@ -68,13 +68,13 @@ func SecureWorkflow(queryStringParams map[string]string, exemptedActions []strin

if pinActions {
pinnedAction, pinnedDocker := false, false
secureWorkflowReponse.FinalOutput, pinnedAction, _ = pin.PinActions(secureWorkflowReponse.FinalOutput, exemptedActions)
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
2 changes: 1 addition & 1 deletion remediation/workflow/secureworkflow_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ func TestSecureWorkflow(t *testing.T) {
}
queryParams["addProjectComment"] = "false"

output, err := SecureWorkflow(queryParams, nil, string(input), &mockDynamoDBClient{})
output, err := SecureWorkflow(queryParams, nil, false, string(input), &mockDynamoDBClient{})

if err != nil {
t.Errorf("Error not expected")
Expand Down
12 changes: 12 additions & 0 deletions testfiles/pinactions/input/donotpintoimmutable.yml
View file
Open in desktop
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/[email protected]
- uses: borales/[email protected]
with:
auth-token: ${{ secrets.GITHUB_TOKEN }}
registry-url: npm.pkg.github.com
12 changes: 12 additions & 0 deletions testfiles/pinactions/output/donotpintoimmutable.yml
View file
Open in desktop
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