diff --git a/remediation/docker/securedockerfile.go b/remediation/docker/securedockerfile.go index 53aaf2a16..60e7617ab 100644 --- a/remediation/docker/securedockerfile.go +++ b/remediation/docker/securedockerfile.go @@ -9,6 +9,7 @@ import ( "github.com/google/go-containerregistry/pkg/authn" "github.com/google/go-containerregistry/pkg/name" "github.com/google/go-containerregistry/pkg/v1/remote" + "github.com/step-security/secure-repo/remediation/workflow/pin" ) var Tr http.RoundTripper = remote.DefaultTransport @@ -20,7 +21,11 @@ type SecureDockerfileResponse struct { DockerfileFetchError bool } -func SecureDockerFile(inputDockerFile string) (*SecureDockerfileResponse, error) { +type dockerfileConfig struct { + exemptedImages []string +} + +func SecureDockerFile(inputDockerFile string, opts ...dockerfileConfig) (*SecureDockerfileResponse, error) { reader := strings.NewReader(inputDockerFile) cmds, err := dockerfile.ParseReader(reader) if err != nil { @@ -32,6 +37,12 @@ func SecureDockerFile(inputDockerFile string) (*SecureDockerfileResponse, error) response.OriginalInput = inputDockerFile response.IsChanged = false + // Get exempted images list, default to empty if no config provided + var exemptedImages []string + if len(opts) > 0 { + exemptedImages = opts[0].exemptedImages + } + for _, c := range cmds { if strings.Contains(c.Cmd, "FROM") && strings.Contains(c.Value[0], ":") { // For being fixable @@ -64,6 +75,12 @@ func SecureDockerFile(inputDockerFile string) (*SecureDockerfileResponse, error) // is already pinned isPinned = true } + + // Check if image is exempted (skip pinning) + if len(exemptedImages) > 0 && pin.ActionExists(image, exemptedImages) { + continue + } + if !isPinned { sha, err := getSHA(image, tag) if err != nil { diff --git a/remediation/docker/securedockerfile_test.go b/remediation/docker/securedockerfile_test.go index d36ce9c79..1afa6470f 100644 --- a/remediation/docker/securedockerfile_test.go +++ b/remediation/docker/securedockerfile_test.go @@ -40,12 +40,16 @@ func TestSecureDockerFile(t *testing.T) { httpmock.RegisterResponder("GET", "https://index.docker.io/v2/library/python/manifests/3.7", httpmock.NewStringResponder(200, resp)) tests := []struct { - fileName string - isChanged bool + fileName string + isChanged bool + exemptedImages []string + useExemptConfig bool }{ - {fileName: "Dockerfile-not-pinned", isChanged: true}, - {fileName: "Dockerfile-not-pinned-as", isChanged: true}, - {fileName: "Dockerfile-multiple-images", isChanged: true}, + {fileName: "Dockerfile-not-pinned", isChanged: true, useExemptConfig: false}, + {fileName: "Dockerfile-not-pinned-as", isChanged: true, useExemptConfig: false}, + {fileName: "Dockerfile-multiple-images", isChanged: true, useExemptConfig: false}, + {fileName: "Dockerfile-exempted", isChanged: false, exemptedImages: []string{"python"}, useExemptConfig: true}, + {fileName: "Dockerfile-exempted-wildcard", isChanged: true, exemptedImages: []string{"amazon*", "alpine"}, useExemptConfig: true}, } for _, test := range tests { @@ -55,7 +59,15 @@ func TestSecureDockerFile(t *testing.T) { log.Fatal(err) } - output, err := SecureDockerFile(string(input)) + var output *SecureDockerfileResponse + if test.useExemptConfig { + config := dockerfileConfig{ + exemptedImages: test.exemptedImages, + } + output, err = SecureDockerFile(string(input), config) + } else { + output, err = SecureDockerFile(string(input)) + } if err != nil { t.Fatalf("Error not expected: %s", err) } diff --git a/remediation/workflow/maintainedactions/maintainedActions.go b/remediation/workflow/maintainedactions/maintainedActions.go index e0d6e3feb..d0be4884d 100644 --- a/remediation/workflow/maintainedactions/maintainedActions.go +++ b/remediation/workflow/maintainedactions/maintainedActions.go @@ -93,6 +93,29 @@ func ReplaceActions(inputYaml string, customerMaintainedActions map[string]strin } } } + + // For composite actions + if workflow.Runs.Using == "composite" { + for stepIdx, step := range workflow.Runs.Steps { + if len(step.Uses) > 0 { + actionName := strings.Split(step.Uses, "@")[0] + if newAction, ok := actionMap[actionName]; ok { + latestVersion, err := GetLatestRelease(newAction) + if err != nil { + return inputYaml, updated, fmt.Errorf("unable to get latest release: %v", err) + } + replacements = append(replacements, replacement{ + jobName: "composite", // special marker for composite actions + stepIdx: stepIdx, + newAction: newAction, + originalAction: step.Uses, + latestVersion: latestVersion, + }) + } + } + } + } + if len(replacements) == 0 { // No changes needed return inputYaml, false, nil @@ -115,9 +138,19 @@ func ReplaceActions(inputYaml string, customerMaintainedActions map[string]strin func replaceAction(t *yaml.Node, inputLines []string, replacements []replacement, updated bool) ([]string, bool) { for _, r := range replacements { - jobsNode := permissions.IterateNode(t, "jobs", "!!map", 0) - jobNode := permissions.IterateNode(jobsNode, r.jobName, "!!map", 0) - stepsNode := permissions.IterateNode(jobNode, "steps", "!!seq", 0) + var stepsNode *yaml.Node + + if r.jobName == "composite" { + // Handle composite actions + runsNode := permissions.IterateNode(t, "runs", "!!map", 0) + stepsNode = permissions.IterateNode(runsNode, "steps", "!!seq", 0) + } else { + // Handle regular workflow jobs + jobsNode := permissions.IterateNode(t, "jobs", "!!map", 0) + jobNode := permissions.IterateNode(jobsNode, r.jobName, "!!map", 0) + stepsNode = permissions.IterateNode(jobNode, "steps", "!!seq", 0) + } + if stepsNode == nil { continue } diff --git a/remediation/workflow/maintainedactions/maintainedactions_test.go b/remediation/workflow/maintainedactions/maintainedactions_test.go index ffbb7f46b..0e47e07ba 100644 --- a/remediation/workflow/maintainedactions/maintainedactions_test.go +++ b/remediation/workflow/maintainedactions/maintainedactions_test.go @@ -77,6 +77,13 @@ func TestReplaceActions(t *testing.T) { wantUpdated: true, wantErr: false, }, + { + name: "composite action with actions to replace", + inputFile: "compositeAction.yml", + outputFile: "compositeAction.yml", + wantUpdated: true, + wantErr: false, + }, } for _, tt := range tests { diff --git a/remediation/workflow/secureworkflow_test.go b/remediation/workflow/secureworkflow_test.go index 80b9945aa..61273ecfd 100644 --- a/remediation/workflow/secureworkflow_test.go +++ b/remediation/workflow/secureworkflow_test.go @@ -225,6 +225,7 @@ func TestSecureWorkflow(t *testing.T) { {fileName: "multiplejobperms.yml", wantPinnedActions: false, wantAddedHardenRunner: false, wantAddedPermissions: true, wantError: false}, {fileName: "error.yml", wantPinnedActions: false, wantAddedHardenRunner: false, wantAddedPermissions: false, wantError: false}, {fileName: "missingaction.yml", wantPinnedActions: false, wantAddedHardenRunner: false, wantAddedPermissions: false, wantError: true}, + {fileName: "compositeAction.yml", wantPinnedActions: true, wantAddedHardenRunner: false, wantAddedPermissions: false, wantAddedMaintainedActions: true, wantError: false}, } for _, test := range tests { var err error @@ -256,12 +257,17 @@ func TestSecureWorkflow(t *testing.T) { queryParams["addHardenRunner"] = "true" queryParams["pinActions"] = "true" queryParams["addPermissions"] = "false" + case "compositeAction.yml": + queryParams["addMaintainedActions"] = "true" + queryParams["addHardenRunner"] = "false" + queryParams["pinActions"] = "true" + queryParams["addPermissions"] = "false" } queryParams["addProjectComment"] = "false" var output *permissions.SecureWorkflowReponse var actionMap map[string]string - if test.fileName == "oneJob.yml" { + if test.fileName == "oneJob.yml" || test.fileName == "compositeAction.yml" { actionMap, err = maintainedactions.LoadMaintainedActions("maintainedactions/maintainedActions.json") if err != nil { t.Errorf("unable to load the file %s", err) diff --git a/testfiles/dockerfiles/input/Dockerfile-exempted b/testfiles/dockerfiles/input/Dockerfile-exempted new file mode 100644 index 000000000..e49b65311 --- /dev/null +++ b/testfiles/dockerfiles/input/Dockerfile-exempted @@ -0,0 +1,7 @@ +# Test file for exempted images +# python should NOT be pinned because it's in the exempted list +FROM python:3.7 + +RUN apt-get update && apt-get install -y vim + +WORKDIR /app diff --git a/testfiles/dockerfiles/input/Dockerfile-exempted-wildcard b/testfiles/dockerfiles/input/Dockerfile-exempted-wildcard new file mode 100644 index 000000000..f5f3bed9d --- /dev/null +++ b/testfiles/dockerfiles/input/Dockerfile-exempted-wildcard @@ -0,0 +1,15 @@ +# Test file for wildcard exemptions +# amazonlinux should NOT be pinned (matches amazon*) +# alpine should NOT be pinned (exact match) +# python SHOULD be pinned (not exempted) +FROM amazonlinux:2023 + +RUN yum install -y python3 + +FROM alpine:3.18 + +RUN apk add --no-cache bash + +FROM python:3.7 + +RUN pip install requests diff --git a/testfiles/dockerfiles/output/Dockerfile-exempted b/testfiles/dockerfiles/output/Dockerfile-exempted new file mode 100644 index 000000000..e49b65311 --- /dev/null +++ b/testfiles/dockerfiles/output/Dockerfile-exempted @@ -0,0 +1,7 @@ +# Test file for exempted images +# python should NOT be pinned because it's in the exempted list +FROM python:3.7 + +RUN apt-get update && apt-get install -y vim + +WORKDIR /app diff --git a/testfiles/dockerfiles/output/Dockerfile-exempted-wildcard b/testfiles/dockerfiles/output/Dockerfile-exempted-wildcard new file mode 100644 index 000000000..803ee6e98 --- /dev/null +++ b/testfiles/dockerfiles/output/Dockerfile-exempted-wildcard @@ -0,0 +1,15 @@ +# Test file for wildcard exemptions +# amazonlinux should NOT be pinned (matches amazon*) +# alpine should NOT be pinned (exact match) +# python SHOULD be pinned (not exempted) +FROM amazonlinux:2023 + +RUN yum install -y python3 + +FROM alpine:3.18 + +RUN apk add --no-cache bash + +FROM python:3.7@sha256:5fb6f4b9d73ddeb0e431c938bee25c69157a1e3c880a81ff72c43a8055628de5 + +RUN pip install requests diff --git a/testfiles/maintainedActions/input/compositeAction.yml b/testfiles/maintainedActions/input/compositeAction.yml new file mode 100644 index 000000000..5a0348cca --- /dev/null +++ b/testfiles/maintainedActions/input/compositeAction.yml @@ -0,0 +1,27 @@ +name: 'Test Composite Action' +description: 'Test composite action for maintained actions replacement' +branding: + icon: 'arrow-up' + color: 'blue' +inputs: + component: + description: 'Component Name' + required: true +runs: + using: 'composite' + steps: + - uses: amannn/action-semantic-pull-request@v5 + with: + types: feat,fix,chore + + - uses: fkirc/skip-duplicate-actions@v5 + with: + do_not_skip: '["release"]' + + - uses: chetan/git-restore-mtime-action@v1 + with: + pattern: '**/*' + + - name: Run custom script + run: echo "Running custom script" + shell: bash diff --git a/testfiles/maintainedActions/output/compositeAction.yml b/testfiles/maintainedActions/output/compositeAction.yml new file mode 100644 index 000000000..200af9d36 --- /dev/null +++ b/testfiles/maintainedActions/output/compositeAction.yml @@ -0,0 +1,27 @@ +name: 'Test Composite Action' +description: 'Test composite action for maintained actions replacement' +branding: + icon: 'arrow-up' + color: 'blue' +inputs: + component: + description: 'Component Name' + required: true +runs: + using: 'composite' + steps: + - uses: step-security/action-semantic-pull-request@v5 + with: + types: feat,fix,chore + + - uses: step-security/skip-duplicate-actions@v5 + with: + do_not_skip: '["release"]' + + - uses: step-security/git-restore-mtime-action@v2 + with: + pattern: '**/*' + + - name: Run custom script + run: echo "Running custom script" + shell: bash diff --git a/testfiles/secureworkflow/input/compositeAction.yml b/testfiles/secureworkflow/input/compositeAction.yml new file mode 100644 index 000000000..a1f580fed --- /dev/null +++ b/testfiles/secureworkflow/input/compositeAction.yml @@ -0,0 +1,25 @@ +name: 'Test Composite Action' +description: 'Test composite action for secure workflow' +branding: + icon: 'shield' + color: 'blue' +inputs: + token: + description: 'GitHub token' + required: true +runs: + using: 'composite' + steps: + - uses: actions/checkout@v1 + + - uses: amannn/action-semantic-pull-request@v5 + with: + types: feat,fix,chore + + - uses: fkirc/skip-duplicate-actions@v5 + with: + do_not_skip: '["release"]' + + - name: Run script + run: echo "Running script" + shell: bash diff --git a/testfiles/secureworkflow/output/compositeAction.yml b/testfiles/secureworkflow/output/compositeAction.yml new file mode 100644 index 000000000..964429bc8 --- /dev/null +++ b/testfiles/secureworkflow/output/compositeAction.yml @@ -0,0 +1,25 @@ +name: 'Test Composite Action' +description: 'Test composite action for secure workflow' +branding: + icon: 'shield' + color: 'blue' +inputs: + token: + description: 'GitHub token' + required: true +runs: + using: 'composite' + steps: + - uses: actions/checkout@v1 + + - uses: step-security/action-semantic-pull-request@a1b2c3d4e5f6g7h8i9j0k1l2m3n4o5p6q7r8s9t0 # v5.5.5 + with: + types: feat,fix,chore + + - uses: step-security/skip-duplicate-actions@b2c3d4e5f6g7h8i9j0k1l2m3n4o5p6q7r8s9t0a1 # v2.1.0 + with: + do_not_skip: '["release"]' + + - name: Run script + run: echo "Running script" + shell: bash