Merge pull request #2522 from step-security/skip-adding-harden-runner

Add skipHardenRunnerForContainers option to exclude container jobs fr…

Author: varunsh-coder

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) {

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")
+	}
+}

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

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)
 		}

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"

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

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

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

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