diff --git a/remediation/workflow/hardenrunner/addaction.go b/remediation/workflow/hardenrunner/addaction.go index 553730817..2f93eed30 100644 --- a/remediation/workflow/hardenrunner/addaction.go +++ b/remediation/workflow/hardenrunner/addaction.go @@ -15,7 +15,7 @@ const ( HardenRunnerActionName = "Harden the runner (Audit all outbound calls)" ) -func AddAction(inputYaml, action string, pinActions, pinToImmutable bool) (string, bool, error) { +func AddAction(inputYaml, action string, pinActions, pinToImmutable bool, skipContainerJobs bool) (string, bool, error) { workflow := metadata.Workflow{} updated := false err := yaml.Unmarshal([]byte(inputYaml), &workflow) @@ -29,6 +29,10 @@ func AddAction(inputYaml, action string, pinActions, pinToImmutable bool) (strin if metadata.IsCallingReusableWorkflow(job) { continue } + // Skip adding action for jobs running in containers if skipContainerJobs is true + if skipContainerJobs && job.Container.Image != "" { + continue + } alreadyPresent := false for _, step := range job.Steps { if len(step.Uses) > 0 && strings.HasPrefix(step.Uses, HardenRunnerActionPath) { diff --git a/remediation/workflow/hardenrunner/addaction_test.go b/remediation/workflow/hardenrunner/addaction_test.go index 8c36cc844..3d2ee3e8c 100644 --- a/remediation/workflow/hardenrunner/addaction_test.go +++ b/remediation/workflow/hardenrunner/addaction_test.go @@ -33,7 +33,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, false) + got, gotUpdated, err := AddAction(string(input), tt.args.action, false, false, false) if gotUpdated != tt.wantUpdated { t.Errorf("AddAction() updated = %v, wantUpdated %v", gotUpdated, tt.wantUpdated) @@ -53,3 +53,26 @@ func TestAddAction(t *testing.T) { }) } } + +func TestAddActionWithContainer(t *testing.T) { + const inputDirectory = "../../../testfiles/addaction/input" + const outputDirectory = "../../../testfiles/addaction/output" + + // Test container job with skipContainerJobs = true + input, err := ioutil.ReadFile(path.Join(inputDirectory, "container-job.yml")) + if err != nil { + t.Fatalf("error reading test file") + } + + // Test: Skip container jobs when skipContainerJobs = true + got, gotUpdated, err := AddAction(string(input), "step-security/harden-runner@v2", false, false, true) + if err != nil { + t.Errorf("AddAction() with skipContainerJobs=true error = %v", err) + } + if gotUpdated { + t.Errorf("AddAction() with skipContainerJobs=true should not update container job") + } + if got != string(input) { + t.Errorf("AddAction() with skipContainerJobs=true should not modify the yaml") + } +} diff --git a/remediation/workflow/metadata/actionmetadata.go b/remediation/workflow/metadata/actionmetadata.go index 98cf4b6c0..506f0debb 100644 --- a/remediation/workflow/metadata/actionmetadata.go +++ b/remediation/workflow/metadata/actionmetadata.go @@ -31,10 +31,17 @@ type Job struct { Permissions Permissions `yaml:"permissions"` Uses string `yaml:"uses"` Env Env `yaml:"env"` + Container Container `yaml:"container"` // RunsOn []string `yaml:"runs-on"` Steps []Step `yaml:"steps"` } +type Container struct { + Image string `yaml:"image"` + Options string `yaml:"options"` + Env Env `yaml:"env"` +} + type Jobs map[string]Job type With map[string]string type Env map[string]string diff --git a/remediation/workflow/secureworkflow.go b/remediation/workflow/secureworkflow.go index 9c88674bc..22fb3c302 100644 --- a/remediation/workflow/secureworkflow.go +++ b/remediation/workflow/secureworkflow.go @@ -23,6 +23,7 @@ func SecureWorkflow(queryStringParams map[string]string, inputYaml string, svc d ignoreMissingKBs := false enableLogging := false addEmptyTopLevelPermissions := false + skipHardenRunnerForContainers := false exemptedActions, pinToImmutable, maintainedActionsMap := []string{}, false, map[string]string{} if len(params) > 0 { @@ -73,6 +74,10 @@ func SecureWorkflow(queryStringParams map[string]string, inputYaml string, svc d addEmptyTopLevelPermissions = true } + if queryStringParams["skipHardenRunnerForContainers"] == "true" { + skipHardenRunnerForContainers = true + } + if enableLogging { // Log query parameters paramsJSON, _ := json.MarshalIndent(queryStringParams, "", " ") @@ -157,7 +162,7 @@ func SecureWorkflow(queryStringParams map[string]string, inputYaml string, svc d log.Printf("Harden runner action is exempted from pinning") } } - secureWorkflowReponse.FinalOutput, addedHardenRunner, _ = hardenrunner.AddAction(secureWorkflowReponse.FinalOutput, HardenRunnerActionPathWithTag, pinHardenRunner, pinToImmutable) + secureWorkflowReponse.FinalOutput, addedHardenRunner, _ = hardenrunner.AddAction(secureWorkflowReponse.FinalOutput, HardenRunnerActionPathWithTag, pinHardenRunner, pinToImmutable, skipHardenRunnerForContainers) if enableLogging { log.Printf("Added harden runner: %v", addedHardenRunner) } diff --git a/remediation/workflow/secureworkflow_test.go b/remediation/workflow/secureworkflow_test.go index c8ff12107..bcb990993 100644 --- a/remediation/workflow/secureworkflow_test.go +++ b/remediation/workflow/secureworkflow_test.go @@ -278,6 +278,95 @@ func TestSecureWorkflow(t *testing.T) { } } +func TestSecureWorkflowContainerJob(t *testing.T) { + const inputDirectory = "../../testfiles/secureworkflow/input" + const outputDirectory = "../../testfiles/secureworkflow/output" + + httpmock.Activate() + defer httpmock.DeactivateAndReset() + + // Mock APIs for actions/checkout + httpmock.RegisterResponder("GET", "https://api.github.com/repos/actions/checkout/commits/v3", + httpmock.NewStringResponder(200, `c85c95e3d7251135ab7dc9ce3241c5835cc595a9`)) + + httpmock.RegisterResponder("GET", "https://api.github.com/repos/actions/checkout/git/matching-refs/tags/v3.", + httpmock.NewStringResponder(200, + `[ + { + "ref": "refs/tags/v3.5.3", + "object": { + "sha": "c85c95e3d7251135ab7dc9ce3241c5835cc595a9", + "type": "commit" + } + } + ]`), + ) + + // Mock APIs for step-security/harden-runner + httpmock.RegisterResponder("GET", "https://api.github.com/repos/step-security/harden-runner/commits/v2", + httpmock.NewStringResponder(200, `17d0e2bd7d51742c71671bd19fa12bdc9d40a3d6`)) + + httpmock.RegisterResponder("GET", "https://api.github.com/repos/step-security/harden-runner/git/matching-refs/tags/v2.", + httpmock.NewStringResponder(200, + `[ + { + "ref": "refs/tags/v2.8.1", + "object": { + "sha": "17d0e2bd7d51742c71671bd19fa12bdc9d40a3d6", + "type": "commit" + } + } + ]`), + ) + + var err error + var input []byte + input, err = ioutil.ReadFile(path.Join(inputDirectory, "container-job.yml")) + + if err != nil { + log.Fatal(err) + } + + os.Setenv("KBFolder", "../../knowledge-base/actions") + + // Test with skipHardenRunnerForContainers = true + queryParams := make(map[string]string) + queryParams["skipHardenRunnerForContainers"] = "true" + queryParams["addProjectComment"] = "false" + + output, err := SecureWorkflow(queryParams, string(input), &mockDynamoDBClient{}) + + if err != nil { + t.Errorf("Error not expected") + } + + // Verify that harden runner was not added + if output.AddedHardenRunner { + t.Errorf("Harden runner should not be added for container job with skipHardenRunnerForContainers=true") + } + + // Verify that the output matches expected output file + expectedOutput, err := ioutil.ReadFile(path.Join(outputDirectory, "container-job.yml")) + if err != nil { + log.Fatal(err) + } + + if output.FinalOutput != string(expectedOutput) { + t.Errorf("test failed container-job.yml did not match expected output\nExpected:\n%s\n\nGot:\n%s", + string(expectedOutput), output.FinalOutput) + } + + // Verify permissions were added + if !output.AddedPermissions { + t.Errorf("Permissions should be added even for container jobs") + } + + // Verify actions were pinned + if !output.PinnedActions { + t.Errorf("Actions should be pinned even for container jobs") + } +} + func TestSecureWorkflowEmptyPermissions(t *testing.T) { const inputDirectory = "../../testfiles/secureworkflow/input" const outputDirectory = "../../testfiles/secureworkflow/output" diff --git a/testfiles/addaction/input/container-job.yml b/testfiles/addaction/input/container-job.yml new file mode 100644 index 000000000..e13bcc714 --- /dev/null +++ b/testfiles/addaction/input/container-job.yml @@ -0,0 +1,18 @@ +name: "Container job workflow" + +on: + push: + + +jobs: + test: + runs-on: ubuntu-latest + container: + image: cgr.dev/chainguard/wolfi-base@sha256:91ed94ec4e72368a9b5113f2ffb1d8e783a91db489011a89d9fad3e3816a75ba + options: >- + --health-cmd pg_isready + --health-interval 10s + + steps: + - name: Checkout + uses: actions/checkout@v3 \ No newline at end of file diff --git a/testfiles/addaction/output/container-job.yml b/testfiles/addaction/output/container-job.yml new file mode 100644 index 000000000..e13bcc714 --- /dev/null +++ b/testfiles/addaction/output/container-job.yml @@ -0,0 +1,18 @@ +name: "Container job workflow" + +on: + push: + + +jobs: + test: + runs-on: ubuntu-latest + container: + image: cgr.dev/chainguard/wolfi-base@sha256:91ed94ec4e72368a9b5113f2ffb1d8e783a91db489011a89d9fad3e3816a75ba + options: >- + --health-cmd pg_isready + --health-interval 10s + + steps: + - name: Checkout + uses: actions/checkout@v3 \ No newline at end of file diff --git a/testfiles/secureworkflow/input/container-job.yml b/testfiles/secureworkflow/input/container-job.yml new file mode 100644 index 000000000..29bcc0f0b --- /dev/null +++ b/testfiles/secureworkflow/input/container-job.yml @@ -0,0 +1,20 @@ +name: "Container job workflow" + +on: + push: + + +jobs: + test: + runs-on: ubuntu-latest + container: + image: cgr.dev/chainguard/wolfi-base@sha256:91ed94ec4e72368a9b5113f2ffb1d8e783a91db489011a89d9fad3e3816a75ba + options: >- + --health-cmd pg_isready + --health-interval 10s + + steps: + - name: Checkout + uses: actions/checkout@v3 + - name: Run tests + run: npm test \ No newline at end of file diff --git a/testfiles/secureworkflow/output/container-job.yml b/testfiles/secureworkflow/output/container-job.yml new file mode 100644 index 000000000..9ec39b1be --- /dev/null +++ b/testfiles/secureworkflow/output/container-job.yml @@ -0,0 +1,23 @@ +name: "Container job workflow" + +on: + push: + + +permissions: + contents: read + +jobs: + test: + runs-on: ubuntu-latest + container: + image: cgr.dev/chainguard/wolfi-base@sha256:91ed94ec4e72368a9b5113f2ffb1d8e783a91db489011a89d9fad3e3816a75ba + options: >- + --health-cmd pg_isready + --health-interval 10s + + steps: + - name: Checkout + uses: actions/checkout@c85c95e3d7251135ab7dc9ce3241c5835cc595a9 # v3.5.3 + - name: Run tests + run: npm test \ No newline at end of file