From dfbe68ac01d00b47859d97533005ac6cbae96d50 Mon Sep 17 00:00:00 2001 From: sailikhith-stepsecurity Date: Thu, 16 Jan 2025 17:56:32 +0530 Subject: [PATCH 1/7] add immutable action check --- .../workflow/pin/action_image_manifest.go | 110 ++++++++++++++++++ .../pin/action_image_manifest_test.go | 54 +++++++++ remediation/workflow/pin/pinactions.go | 2 +- 3 files changed, 165 insertions(+), 1 deletion(-) create mode 100644 remediation/workflow/pin/action_image_manifest.go create mode 100644 remediation/workflow/pin/action_image_manifest_test.go diff --git a/remediation/workflow/pin/action_image_manifest.go b/remediation/workflow/pin/action_image_manifest.go new file mode 100644 index 00000000..c628f4fe --- /dev/null +++ b/remediation/workflow/pin/action_image_manifest.go @@ -0,0 +1,110 @@ +package pin + +import ( + "encoding/json" + "fmt" + "regexp" + "strings" + + "github.com/google/go-containerregistry/pkg/name" + "github.com/google/go-containerregistry/pkg/v1/remote" + "github.com/sirupsen/logrus" +) + +var ( + githubImmutableActionArtifactType = "application/vnd.github.actions.package.v1+json" + tagRegex = regexp.MustCompile(`v[0-9]+\.[0-9]+\.[0-9]+$`) +) + +type ociManifest struct { + ArtifactType string `json:"artifactType"` +} + +// isImmutableAction checks if the action is an immutable action or not +// It queries the OCI manifest for the action and checks if the artifact type is "application/vnd.github.actions.package.v1+json" +// +// Example usage: +// +// # Immutable action (returns true) +// isImmutableAction("actions/checkout@v4.2.2") +// +// # Non-Immutable action (returns false) +// isImmutableAction("actions/checkout@v4.2.3") +// +// REF - https://github.com/actions/publish-immutable-action/issues/216#issuecomment-2549914784 +func IsImmutableAction(action string) bool { + + artifactType, err := getOCIImageArtifactTypeForGhAction(action) + if err != nil { + // log the error + logrus.WithFields(logrus.Fields{"action": action}).WithError(err).Error("error in getting OCI manifest for image") + return false + } + + if artifactType == githubImmutableActionArtifactType { + return true + } + return false + +} + +// getOCIImageArtifactTypeForGhAction retrieves the artifact type from a GitHub Action's OCI manifest. +// This function is used to determine if an action is immutable by checking its artifact type. +// +// Example usage: +// +// # Immutable action (returns "application/vnd.github.actions.package.v1+json", nil) +// artifactType, err := getOCIImageArtifactTypeForGhAction("actions/checkout@v4.2.2") +// +// Returns: +// - artifactType: The artifact type string from the OCI manifest +// - error: An error if the action format is invalid or if there's a problem retrieving the manifest +func getOCIImageArtifactTypeForGhAction(action string) (string, error) { + + // Split the action into parts (e.g., "actions/checkout@v2" -> ["actions/checkout", "v2"]) + parts := strings.Split(action, "@") + if len(parts) != 2 { + return "", fmt.Errorf("invalid action format") + } + + // convert v1.x.x to 1.x.x which is + // use regexp to match tag version format and replace v in prefix + // as immutable actions image tag is in format 1.x.x (without v prefix) + // REF - https://github.com/actions/publish-immutable-action/issues/216#issuecomment-2549914784 + if tagRegex.MatchString(parts[1]) { + // v1.x.x -> 1.x.x + parts[1] = strings.TrimPrefix(parts[1], "v") + } + + // Convert GitHub action to GHCR image reference using proper OCI reference format + image := fmt.Sprintf("ghcr.io/%s:%s", parts[0], parts[1]) + imageManifest, err := getOCIManifestForImage(image) + if err != nil { + return "", err + } + + var ociManifest ociManifest + err = json.Unmarshal([]byte(imageManifest), &ociManifest) + if err != nil { + return "", err + } + return ociManifest.ArtifactType, nil +} + +// getOCIManifestForImage retrieves the artifact type from the OCI image manifest +func getOCIManifestForImage(imageRef string) (string, error) { + + // Parse the image reference + ref, err := name.ParseReference(imageRef) + if err != nil { + return "", fmt.Errorf("error parsing reference: %v", err) + } + + // Get the image manifest + desc, err := remote.Get(ref) + if err != nil { + return "", fmt.Errorf("error getting manifest: %v", err) + } + + return string(desc.Manifest), nil +} diff --git a/remediation/workflow/pin/action_image_manifest_test.go b/remediation/workflow/pin/action_image_manifest_test.go new file mode 100644 index 00000000..51cd0ba3 --- /dev/null +++ b/remediation/workflow/pin/action_image_manifest_test.go @@ -0,0 +1,54 @@ +package pin + +import ( + "testing" +) + +func Test_isImmutableAction(t *testing.T) { + tests := []struct { + name string + action string + want bool + }{ + { + name: "immutable action - 1", + action: "actions/checkout@v4.2.2", + want: true, + }, + { + name: "immutable action - 2", + action: "step-security/wait-for-secrets@v1.2.0", + want: true, + }, + { + name: "non immutable action(valid action)", + action: "sailikhith-stepsecurity/hello-action@v1.0.2", + want: false, + }, + { + name: "non immutable action(invalid action)", + action: "sailikhith-stepsecurity/no-such-action@v1.0.2", + want: false, + }, + { + name: " action with release tag doesn't exist", + action: "actions/checkout@1.2.3", + want: false, + }, + { + name: "invalid action format", + action: "invalid-format", + want: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + + got := IsImmutableAction(tt.action) + if got != tt.want { + t.Errorf("isImmutableAction() = %v, want %v", got, tt.want) + } + }) + } +} diff --git a/remediation/workflow/pin/pinactions.go b/remediation/workflow/pin/pinactions.go index b8aef31c..db703a53 100644 --- a/remediation/workflow/pin/pinactions.go +++ b/remediation/workflow/pin/pinactions.go @@ -43,7 +43,7 @@ func PinAction(action, inputYaml string) (string, bool) { return inputYaml, updated // Cannot pin local actions and docker actions } - if isAbsolute(action) { + if isAbsolute(action) || IsImmutableAction(action) { return inputYaml, updated } leftOfAt := strings.Split(action, "@") From 0509869f7e9fdbfd824c4a6e9e16e413e854695d Mon Sep 17 00:00:00 2001 From: sailikhith-stepsecurity Date: Thu, 16 Jan 2025 18:16:00 +0530 Subject: [PATCH 2/7] fix test --- remediation/workflow/metadata/actionmetadata_test.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/remediation/workflow/metadata/actionmetadata_test.go b/remediation/workflow/metadata/actionmetadata_test.go index d3967e41..51e109f7 100644 --- a/remediation/workflow/metadata/actionmetadata_test.go +++ b/remediation/workflow/metadata/actionmetadata_test.go @@ -181,6 +181,13 @@ func TestKnowledgeBase(t *testing.T) { func doesActionRepoExist(filePath string) bool { splitOnSlash := strings.Split(filePath, "/") + + // Check if path has enough components + if len(splitOnSlash) < 6 { + log.Printf("error in doesActionRepoExist: invalid path format %s", filePath) + return false + } + owner := splitOnSlash[5] repo := splitOnSlash[6] From 6883158c260ff4589ab43799a6f2dce201d45cc4 Mon Sep 17 00:00:00 2001 From: sailikhith-stepsecurity Date: Thu, 16 Jan 2025 18:36:39 +0530 Subject: [PATCH 3/7] update kb by moving remove-disabled-formulae to remove-disabled-packages --- .../actions/remove-disabled-packages/action-security.yml | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 knowledge-base/actions/homebrew/actions/remove-disabled-packages/action-security.yml diff --git a/knowledge-base/actions/homebrew/actions/remove-disabled-packages/action-security.yml b/knowledge-base/actions/homebrew/actions/remove-disabled-packages/action-security.yml new file mode 100644 index 00000000..5255e85c --- /dev/null +++ b/knowledge-base/actions/homebrew/actions/remove-disabled-packages/action-security.yml @@ -0,0 +1,2 @@ +name: Remove disabled packages # Homebrew/actions/remove-disabled-packages +# GITHUB_TOKEN not used \ No newline at end of file From 421067f6c85695848b45b75bb66114bc58aee438 Mon Sep 17 00:00:00 2001 From: sailikhith-stepsecurity Date: Thu, 16 Jan 2025 18:55:55 +0530 Subject: [PATCH 4/7] removing unwanted change --- remediation/workflow/metadata/actionmetadata_test.go | 6 ------ 1 file changed, 6 deletions(-) diff --git a/remediation/workflow/metadata/actionmetadata_test.go b/remediation/workflow/metadata/actionmetadata_test.go index 51e109f7..2696baa7 100644 --- a/remediation/workflow/metadata/actionmetadata_test.go +++ b/remediation/workflow/metadata/actionmetadata_test.go @@ -182,12 +182,6 @@ func TestKnowledgeBase(t *testing.T) { func doesActionRepoExist(filePath string) bool { splitOnSlash := strings.Split(filePath, "/") - // Check if path has enough components - if len(splitOnSlash) < 6 { - log.Printf("error in doesActionRepoExist: invalid path format %s", filePath) - return false - } - owner := splitOnSlash[5] repo := splitOnSlash[6] From 1281489db29f26aa6ea219d0549fa1b269cbef14 Mon Sep 17 00:00:00 2001 From: sailikhith-stepsecurity Date: Sat, 18 Jan 2025 15:12:09 +0530 Subject: [PATCH 5/7] mocking http server for unit tests --- .../pin/action_image_manifest_test.go | 80 +++++++++++++++++++ .../immutableActionResponses/default.json | 8 ++ .../ghcr.io_actions_checkout:4.2.2.json | 38 +++++++++ ..._step-security_wait-for-secrets:1.2.0.json | 38 +++++++++ 4 files changed, 164 insertions(+) create mode 100644 testfiles/pinactions/immutableActionResponses/default.json create mode 100644 testfiles/pinactions/immutableActionResponses/ghcr.io_actions_checkout:4.2.2.json create mode 100644 testfiles/pinactions/immutableActionResponses/ghcr.io_step-security_wait-for-secrets:1.2.0.json diff --git a/remediation/workflow/pin/action_image_manifest_test.go b/remediation/workflow/pin/action_image_manifest_test.go index 51cd0ba3..1c3b5435 100644 --- a/remediation/workflow/pin/action_image_manifest_test.go +++ b/remediation/workflow/pin/action_image_manifest_test.go @@ -1,10 +1,75 @@ package pin import ( + "io/ioutil" + "net/http" + "net/http/httptest" + "net/url" + "strings" "testing" ) +var ( + testFilesDir = "../../../testfiles/pinactions/immutableActionResponses/" +) + +func createTestServer(t *testing.T) *httptest.Server { + return httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + + w.Header().Set("Content-Type", "application/json") + + // Mock manifest endpoints + switch r.URL.Path { + + case "/token": + // for immutable actions, since image will be present in registry...it returns 200 OK with token + // otherwise it returns 403 Forbidden + scope := r.URL.Query().Get("scope") + switch scope { + case "repository:actions/checkout:pull": + fallthrough + case "repository:step-security/wait-for-secrets:pull": + w.WriteHeader(http.StatusOK) + w.Write([]byte(`{"token": "test-token", "access_token": "test-token"}`)) + default: + w.WriteHeader(http.StatusForbidden) + w.Write([]byte(`{"errors": [{"code": "DENIED", "message": "requested access to the resource is denied"}]}`)) + } + + case "/v2/actions/checkout/manifests/4.2.2": + fallthrough + case "/v2/step-security/wait-for-secrets/manifests/1.2.0": + w.Write(readHttpResponseForAction(t, r.URL.Path)) + case "/v2/actions/checkout/manifests/1.2.3": // since this version doesn't exist + fallthrough + default: + w.WriteHeader(http.StatusNotFound) + w.Write(readHttpResponseForAction(t, "default")) + } + })) +} + func Test_isImmutableAction(t *testing.T) { + // Create test server that mocks GitHub Container Registry + server := createTestServer(t) + defer server.Close() + + // Create a custom client that redirects ghcr.io to our test server + originalClient := http.DefaultClient + http.DefaultClient = &http.Client{ + Transport: &http.Transport{ + Proxy: func(req *http.Request) (*url.URL, error) { + if strings.Contains(req.URL.Host, "ghcr.io") { + return url.Parse(server.URL) + } + return nil, nil + }, + }, + } + defer func() { + http.DefaultClient = originalClient + }() + tests := []struct { name string action string @@ -52,3 +117,18 @@ func Test_isImmutableAction(t *testing.T) { }) } } + +func readHttpResponseForAction(t *testing.T, actionPath string) []byte { + // remove v2 prefix from action path + actionPath = strings.TrimPrefix(actionPath, "v2/") + + fileName := strings.ReplaceAll(actionPath, "/", "-") + respFilePath := testFilesDir + fileName + + resp, err := ioutil.ReadFile(respFilePath) + if err != nil { + t.Fatalf("error reading test file") + } + + return resp +} diff --git a/testfiles/pinactions/immutableActionResponses/default.json b/testfiles/pinactions/immutableActionResponses/default.json new file mode 100644 index 00000000..d8e11ba4 --- /dev/null +++ b/testfiles/pinactions/immutableActionResponses/default.json @@ -0,0 +1,8 @@ +{ + "errors":[ + { + "code":"MANIFEST_UNKNOWN", + "message":"manifest unknown" + } + ] +} \ No newline at end of file diff --git a/testfiles/pinactions/immutableActionResponses/ghcr.io_actions_checkout:4.2.2.json b/testfiles/pinactions/immutableActionResponses/ghcr.io_actions_checkout:4.2.2.json new file mode 100644 index 00000000..d93e6678 --- /dev/null +++ b/testfiles/pinactions/immutableActionResponses/ghcr.io_actions_checkout:4.2.2.json @@ -0,0 +1,38 @@ +{ + "schemaVersion": 2, + "mediaType": "application/vnd.oci.image.manifest.v1+json", + "artifactType": "application/vnd.github.actions.package.v1+json", + "config": { + "mediaType": "application/vnd.oci.empty.v1+json", + "size": 2, + "digest": "sha256:44136fa355b3678a1146ad16f7e8649e94fb4fc21fe77e8310c060f61caaff8a" + }, + "layers": [ + { + "mediaType": "application/vnd.github.actions.package.layer.v1.tar+gzip", + "size": 424913, + "digest": "sha256:309d5e1a7604a5e688e2b55a6763e4109eb07349dca6d3c44e85c57ab2bb4f3f", + "annotations": { + "org.opencontainers.image.title": "actions-checkout_4.2.2.tar.gz" + } + }, + { + "mediaType": "application/vnd.github.actions.package.layer.v1.zip", + "size": 546845, + "digest": "sha256:e9808fe811a75b46234757f9566987635166bca838090fcbc8021a0d45c737b3", + "annotations": { + "org.opencontainers.image.title": "actions-checkout_4.2.2.zip" + } + } + ], + "annotations": { + "org.opencontainers.image.created": "2024-10-23T14:46:13.071Z", + "action.tar.gz.digest": "sha256:309d5e1a7604a5e688e2b55a6763e4109eb07349dca6d3c44e85c57ab2bb4f3f", + "action.zip.digest": "sha256:e9808fe811a75b46234757f9566987635166bca838090fcbc8021a0d45c737b3", + "com.github.package.type": "actions_oci_pkg", + "com.github.package.version": "4.2.2", + "com.github.source.repo.id": "197814629", + "com.github.source.repo.owner.id": "44036562", + "com.github.source.commit": "11bd71901bbe5b1630ceea73d27597364c9af683" + } +} \ No newline at end of file diff --git a/testfiles/pinactions/immutableActionResponses/ghcr.io_step-security_wait-for-secrets:1.2.0.json b/testfiles/pinactions/immutableActionResponses/ghcr.io_step-security_wait-for-secrets:1.2.0.json new file mode 100644 index 00000000..d85d3125 --- /dev/null +++ b/testfiles/pinactions/immutableActionResponses/ghcr.io_step-security_wait-for-secrets:1.2.0.json @@ -0,0 +1,38 @@ +{ + "schemaVersion": 2, + "mediaType": "application/vnd.oci.image.manifest.v1+json", + "artifactType": "application/vnd.github.actions.package.v1+json", + "config": { + "mediaType": "application/vnd.oci.empty.v1+json", + "size": 2, + "digest": "sha256:44136fa355b3678a1146ad16f7e8649e94fb4fc21fe77e8310c060f61caaff8a" + }, + "layers": [ + { + "mediaType": "application/vnd.github.actions.package.layer.v1.tar+gzip", + "size": 689381, + "digest": "sha256:6390cea2d46095ef08dd2746d4323b11b7d1190d7e9ad9ef4a23b8ee5481d295", + "annotations": { + "org.opencontainers.image.title": "step-security-wait-for-secrets_1.2.0.tar.gz" + } + }, + { + "mediaType": "application/vnd.github.actions.package.layer.v1.zip", + "size": 723541, + "digest": "sha256:56f5004c2b1bff0f148c3998aa0f5bd47a315a602428031b8ba72d881edfb429", + "annotations": { + "org.opencontainers.image.title": "step-security-wait-for-secrets_1.2.0.zip" + } + } + ], + "annotations": { + "org.opencontainers.image.created": "2024-10-24T05:13:19.501Z", + "action.tar.gz.digest": "sha256:6390cea2d46095ef08dd2746d4323b11b7d1190d7e9ad9ef4a23b8ee5481d295", + "action.zip.digest": "sha256:56f5004c2b1bff0f148c3998aa0f5bd47a315a602428031b8ba72d881edfb429", + "com.github.package.type": "actions_oci_pkg", + "com.github.package.version": "1.2.0", + "com.github.source.repo.id": "498456330", + "com.github.source.repo.owner.id": "88700172", + "com.github.source.commit": "5809f7d044804a5a1d43217fa8f3e855939fc9ef" + } +} \ No newline at end of file From 59ccd7533acd8cce1bd6a893d5a325ca333e1fdb Mon Sep 17 00:00:00 2001 From: sailikhith-stepsecurity Date: Mon, 20 Jan 2025 10:43:38 +0530 Subject: [PATCH 6/7] updating pinnig logic to use immutable version instead of hash --- .../workflow/pin/action_image_manifest.go | 8 +- .../pin/action_image_manifest_test.go | 63 +++++++++++----- remediation/workflow/pin/pinactions.go | 27 ++++++- remediation/workflow/pin/pinactions_test.go | 75 +++++++++++++++++++ ... => actions-checkout-manifests-4.2.2.json} | 0 ...ity-wait-for-secrets-manifests-1.2.0.json} | 0 .../pinactions/input/immutableaction-1.yml | 11 +++ .../pinactions/output/actionwithcomment.yml | 2 +- testfiles/pinactions/output/dockeraction.yml | 2 +- .../pinactions/output/immutableaction-1.yml | 11 +++ testfiles/pinactions/output/localaction.yml | 8 +- .../pinactions/output/multipleactions.yml | 2 +- testfiles/pinactions/output/multiplejobs.yml | 4 +- .../output/repeatedactionwithcomment.yml | 2 +- 14 files changed, 184 insertions(+), 31 deletions(-) rename testfiles/pinactions/immutableActionResponses/{ghcr.io_actions_checkout:4.2.2.json => actions-checkout-manifests-4.2.2.json} (100%) rename testfiles/pinactions/immutableActionResponses/{ghcr.io_step-security_wait-for-secrets:1.2.0.json => step-security-wait-for-secrets-manifests-1.2.0.json} (100%) create mode 100644 testfiles/pinactions/input/immutableaction-1.yml create mode 100644 testfiles/pinactions/output/immutableaction-1.yml diff --git a/remediation/workflow/pin/action_image_manifest.go b/remediation/workflow/pin/action_image_manifest.go index c628f4fe..f7c48e29 100644 --- a/remediation/workflow/pin/action_image_manifest.go +++ b/remediation/workflow/pin/action_image_manifest.go @@ -6,6 +6,8 @@ import ( "regexp" "strings" + "net/http" + "github.com/google/go-containerregistry/pkg/name" "github.com/google/go-containerregistry/pkg/v1/remote" "github.com/sirupsen/logrus" @@ -13,7 +15,7 @@ import ( var ( githubImmutableActionArtifactType = "application/vnd.github.actions.package.v1+json" - tagRegex = regexp.MustCompile(`v[0-9]+\.[0-9]+\.[0-9]+$`) + semanticTagRegex = regexp.MustCompile(`v[0-9]+\.[0-9]+\.[0-9]+$`) ) type ociManifest struct { @@ -71,7 +73,7 @@ func getOCIImageArtifactTypeForGhAction(action string) (string, error) { // use regexp to match tag version format and replace v in prefix // as immutable actions image tag is in format 1.x.x (without v prefix) // REF - https://github.com/actions/publish-immutable-action/issues/216#issuecomment-2549914784 - if tagRegex.MatchString(parts[1]) { + if semanticTagRegex.MatchString(parts[1]) { // v1.x.x -> 1.x.x parts[1] = strings.TrimPrefix(parts[1], "v") } @@ -101,7 +103,7 @@ func getOCIManifestForImage(imageRef string) (string, error) { } // Get the image manifest - desc, err := remote.Get(ref) + desc, err := remote.Get(ref, remote.WithTransport(http.DefaultTransport)) if err != nil { return "", fmt.Errorf("error getting manifest: %v", err) } diff --git a/remediation/workflow/pin/action_image_manifest_test.go b/remediation/workflow/pin/action_image_manifest_test.go index 1c3b5435..d16af7de 100644 --- a/remediation/workflow/pin/action_image_manifest_test.go +++ b/remediation/workflow/pin/action_image_manifest_test.go @@ -1,26 +1,45 @@ package pin import ( + "crypto/tls" "io/ioutil" "net/http" "net/http/httptest" - "net/url" + "path/filepath" "strings" "testing" ) -var ( - testFilesDir = "../../../testfiles/pinactions/immutableActionResponses/" -) +type customTransport struct { + base http.RoundTripper + baseURL string +} + +func (t *customTransport) RoundTrip(req *http.Request) (*http.Response, error) { + if strings.Contains(req.URL.Host, "ghcr.io") { + req2 := req.Clone(req.Context()) + req2.URL.Scheme = "https" + req2.URL.Host = strings.TrimPrefix(t.baseURL, "https://") + return t.base.RoundTrip(req2) + } + return t.base.RoundTrip(req) +} -func createTestServer(t *testing.T) *httptest.Server { - return httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { +func createGhesTestServer(t *testing.T) *httptest.Server { + return httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.Header().Set("Content-Type", "application/json") + if !strings.Contains(r.Host, "ghcr.io") { + w.WriteHeader(http.StatusNotFound) + return + } // Mock manifest endpoints switch r.URL.Path { + case "/v2/": // simulate ping request + w.WriteHeader(http.StatusOK) + case "/token": // for immutable actions, since image will be present in registry...it returns 200 OK with token // otherwise it returns 403 Forbidden @@ -29,6 +48,7 @@ func createTestServer(t *testing.T) *httptest.Server { case "repository:actions/checkout:pull": fallthrough case "repository:step-security/wait-for-secrets:pull": + w.WriteHeader(http.StatusOK) w.Write([]byte(`{"token": "test-token", "access_token": "test-token"}`)) default: @@ -38,6 +58,8 @@ func createTestServer(t *testing.T) *httptest.Server { case "/v2/actions/checkout/manifests/4.2.2": fallthrough + case "/v2/actions/checkout/manifests/1.2.0": + fallthrough case "/v2/step-security/wait-for-secrets/manifests/1.2.0": w.Write(readHttpResponseForAction(t, r.URL.Path)) case "/v2/actions/checkout/manifests/1.2.3": // since this version doesn't exist @@ -51,23 +73,29 @@ func createTestServer(t *testing.T) *httptest.Server { func Test_isImmutableAction(t *testing.T) { // Create test server that mocks GitHub Container Registry - server := createTestServer(t) + server := createGhesTestServer(t) defer server.Close() // Create a custom client that redirects ghcr.io to our test server originalClient := http.DefaultClient http.DefaultClient = &http.Client{ - Transport: &http.Transport{ - Proxy: func(req *http.Request) (*url.URL, error) { - if strings.Contains(req.URL.Host, "ghcr.io") { - return url.Parse(server.URL) - } - return nil, nil + Transport: &customTransport{ + base: &http.Transport{ + TLSClientConfig: &tls.Config{ + InsecureSkipVerify: true, + }, }, + baseURL: server.URL, }, } + + // update default transport + OriginalTransport := http.DefaultTransport + http.DefaultTransport = http.DefaultClient.Transport + defer func() { http.DefaultClient = originalClient + http.DefaultTransport = OriginalTransport }() tests := []struct { @@ -120,14 +148,15 @@ func Test_isImmutableAction(t *testing.T) { func readHttpResponseForAction(t *testing.T, actionPath string) []byte { // remove v2 prefix from action path - actionPath = strings.TrimPrefix(actionPath, "v2/") + actionPath = strings.TrimPrefix(actionPath, "/v2/") - fileName := strings.ReplaceAll(actionPath, "/", "-") - respFilePath := testFilesDir + fileName + fileName := strings.ReplaceAll(actionPath, "/", "-") + ".json" + testFilesDir := "../../../testfiles/pinactions/immutableActionResponses/" + respFilePath := filepath.Join(testFilesDir, fileName) resp, err := ioutil.ReadFile(respFilePath) if err != nil { - t.Fatalf("error reading test file") + t.Fatalf("error reading test file:%v", err) } return resp diff --git a/remediation/workflow/pin/pinactions.go b/remediation/workflow/pin/pinactions.go index db703a53..8f383755 100644 --- a/remediation/workflow/pin/pinactions.go +++ b/remediation/workflow/pin/pinactions.go @@ -4,6 +4,7 @@ import ( "context" "fmt" "os" + "regexp" "strings" "github.com/google/go-github/v40/github" @@ -74,8 +75,32 @@ func PinAction(action, inputYaml string) (string, bool) { } pinnedAction := fmt.Sprintf("%s@%s # %s", leftOfAt[0], commitSHA, tagOrBranch) + + // 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) { + pinnedAction = pinnedActionWithVersion + } + updated = !strings.EqualFold(action, pinnedAction) - inputYaml = strings.ReplaceAll(inputYaml, action, pinnedAction) + + // strings.ReplaceAll is not suitable here because it would incorrectly replace substrings + // For example, if we want to replace "actions/checkout@v1" to "actions/checkout@v1.2.3", it would also incorrectly match and replace in "actions/checkout@v1.2.3" + // making new string to "actions/checkout@v1.2.3.2.3" + // + // Instead, we use a regex pattern that ensures we only replace complete action references: + // Pattern: (@)($|\s|"|') + // - Group 1 (@): Captures the exact action reference + // - Group 2 ($|\s|"|'): Captures the delimiter that follows (end of line, whitespace, or quotes) + // + // Examples: + // - "actions/checkout@v1.2.3" - No match (no delimiter after v1) + // - "actions/checkout@v1 " - Matches (space delimiter) + // - "actions/checkout@v1"" - Matches (quote delimiter) + // - "actions/checkout@v1" - Matches (quote delimiter) + // - "actions/checkout@v1\n" - Matches (newline is considered whitespace \s) + actionRegex := regexp.MustCompile(`(` + regexp.QuoteMeta(action) + `)($|\s|"|')`) + inputYaml = actionRegex.ReplaceAllString(inputYaml, pinnedAction+"$2") yamlWithPreviousActionCommentsRemoved, wasModified := removePreviousActionComments(pinnedAction, inputYaml) if wasModified { return yamlWithPreviousActionCommentsRemoved, updated diff --git a/remediation/workflow/pin/pinactions_test.go b/remediation/workflow/pin/pinactions_test.go index a4872be7..ef20695d 100644 --- a/remediation/workflow/pin/pinactions_test.go +++ b/remediation/workflow/pin/pinactions_test.go @@ -3,7 +3,9 @@ package pin import ( "io/ioutil" "log" + "net/http" "path" + "strings" "testing" "github.com/jarcoal/httpmock" @@ -171,6 +173,78 @@ func TestPinActions(t *testing.T) { } ]`)) + // mock ping response + httpmock.RegisterResponder("GET", "https://ghcr.io/v2/", + httpmock.NewStringResponder(200, ``)) + + // Mock token endpoints + httpmock.RegisterResponder("GET", "https://ghcr.io/token", + func(req *http.Request) (*http.Response, error) { + scope := req.URL.Query().Get("scope") + switch scope { + // Following are the ones which simulate the image existance in ghcr + case "repository:actions/checkout:pull", + "repository:step-security/wait-for-secrets:pull", + "repository:actions/setup-node:pull", + "repository:peter-evans/close-issue:pull", + "repository:borales/actions-yarn:pull", + "repository:JS-DevTools/npm-publish:pull", + "repository:elgohr/Publish-Docker-Github-Action:pull", + "repository:brandedoutcast/publish-nuget:pull", + "repository:rohith/publish-nuget:pull": + return httpmock.NewJsonResponse(http.StatusOK, map[string]string{ + "token": "test-token", + "access_token": "test-token", + }) + default: + return httpmock.NewJsonResponse(http.StatusForbidden, map[string]interface{}{ + "errors": []map[string]string{ + { + "code": "DENIED", + "message": "requested access to the resource is denied", + }, + }, + }) + } + }) + + // Mock manifest endpoints for specific versions and commit hashes + manifestResponders := []string{ + // the following list will contain the list of actions with versions + // which are mocked to be immutable + "actions/checkout@v1.2.0", + } + + for _, action := range manifestResponders { + actionPath := strings.Split(action, "@")[0] + version := strings.TrimPrefix(strings.Split(action, "@")[1], "v") + // Mock manifest response so that we can treat action as immutable + httpmock.RegisterResponder("GET", "https://ghcr.io/v2/"+actionPath+"/manifests/"+version, + func(req *http.Request) (*http.Response, error) { + return httpmock.NewJsonResponse(http.StatusOK, map[string]interface{}{ + "schemaVersion": 2, + "mediaType": "application/vnd.github.actions.package.v1+json", + "artifactType": "application/vnd.github.actions.package.v1+json", + "config": map[string]interface{}{ + "mediaType": "application/vnd.github.actions.package.v1+json", + }, + }) + }) + } + + // Default manifest response for non-existent tags + httpmock.RegisterResponder("GET", `=~^https://ghcr\.io/v2/.*/manifests/.*`, + func(req *http.Request) (*http.Response, error) { + return httpmock.NewJsonResponse(http.StatusNotFound, map[string]interface{}{ + "errors": []map[string]string{ + { + "code": "MANIFEST_UNKNOWN", + "message": "manifest unknown", + }, + }, + }) + }) + tests := []struct { fileName string wantUpdated bool @@ -184,6 +258,7 @@ func TestPinActions(t *testing.T) { {fileName: "multipleactions.yml", wantUpdated: true}, {fileName: "actionwithcomment.yml", wantUpdated: true}, {fileName: "repeatedactionwithcomment.yml", wantUpdated: true}, + {fileName: "immutableaction-1.yml", wantUpdated: true}, } for _, tt := range tests { input, err := ioutil.ReadFile(path.Join(inputDirectory, tt.fileName)) diff --git a/testfiles/pinactions/immutableActionResponses/ghcr.io_actions_checkout:4.2.2.json b/testfiles/pinactions/immutableActionResponses/actions-checkout-manifests-4.2.2.json similarity index 100% rename from testfiles/pinactions/immutableActionResponses/ghcr.io_actions_checkout:4.2.2.json rename to testfiles/pinactions/immutableActionResponses/actions-checkout-manifests-4.2.2.json diff --git a/testfiles/pinactions/immutableActionResponses/ghcr.io_step-security_wait-for-secrets:1.2.0.json b/testfiles/pinactions/immutableActionResponses/step-security-wait-for-secrets-manifests-1.2.0.json similarity index 100% rename from testfiles/pinactions/immutableActionResponses/ghcr.io_step-security_wait-for-secrets:1.2.0.json rename to testfiles/pinactions/immutableActionResponses/step-security-wait-for-secrets-manifests-1.2.0.json diff --git a/testfiles/pinactions/input/immutableaction-1.yml b/testfiles/pinactions/input/immutableaction-1.yml new file mode 100644 index 00000000..41a6e0b9 --- /dev/null +++ b/testfiles/pinactions/input/immutableaction-1.yml @@ -0,0 +1,11 @@ +name: Integration Test Github +on: [push] +jobs: + build: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v1.2.0 + - uses: borales/actions-yarn@v2.3.0 + with: + auth-token: ${{ secrets.GITHUB_TOKEN }} + registry-url: npm.pkg.github.com diff --git a/testfiles/pinactions/output/actionwithcomment.yml b/testfiles/pinactions/output/actionwithcomment.yml index a1131c83..89388bb1 100644 --- a/testfiles/pinactions/output/actionwithcomment.yml +++ b/testfiles/pinactions/output/actionwithcomment.yml @@ -16,7 +16,7 @@ jobs: publish: runs-on: ubuntu-latest steps: - - uses: actions/checkout@544eadc6bf3d226fd7a7a9f0dc5b5bf7ca0675b9 # v1.2.0 + - uses: actions/checkout@v1.2.0 - uses: actions/setup-node@f1f314fca9dfce2769ece7d933488f076716723e # v1.4.6 with: node-version: 10 diff --git a/testfiles/pinactions/output/dockeraction.yml b/testfiles/pinactions/output/dockeraction.yml index 6e394864..1413f1e5 100644 --- a/testfiles/pinactions/output/dockeraction.yml +++ b/testfiles/pinactions/output/dockeraction.yml @@ -10,7 +10,7 @@ jobs: runs-on: ubuntu-latest steps: - name: Checkout - uses: actions/checkout@544eadc6bf3d226fd7a7a9f0dc5b5bf7ca0675b9 # v1.2.0 + uses: actions/checkout@v1.2.0 - name: Integration test uses: docker://ghcr.io/step-security/integration-test/int:latest env: diff --git a/testfiles/pinactions/output/immutableaction-1.yml b/testfiles/pinactions/output/immutableaction-1.yml new file mode 100644 index 00000000..a4a93961 --- /dev/null +++ b/testfiles/pinactions/output/immutableaction-1.yml @@ -0,0 +1,11 @@ +name: Integration Test Github +on: [push] +jobs: + build: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v1.2.0 + - uses: borales/actions-yarn@4965e1a0f0ae9c422a9a5748ebd1fb5e097d22b9 # v2.3.0 + with: + auth-token: ${{ secrets.GITHUB_TOKEN }} + registry-url: npm.pkg.github.com diff --git a/testfiles/pinactions/output/localaction.yml b/testfiles/pinactions/output/localaction.yml index c6d98688..f7910970 100644 --- a/testfiles/pinactions/output/localaction.yml +++ b/testfiles/pinactions/output/localaction.yml @@ -14,7 +14,7 @@ jobs: - uses: actions/setup-node@f1f314fca9dfce2769ece7d933488f076716723e # v1.4.6 with: node-version: 12.x - - uses: actions/checkout@544eadc6bf3d226fd7a7a9f0dc5b5bf7ca0675b9 # v1.2.0 + - uses: actions/checkout@v1.2.0 - run: npm ci - run: npm run build - run: npm run format-check @@ -32,7 +32,7 @@ jobs: steps: # Clone this repo - name: Checkout - uses: actions/checkout@544eadc6bf3d226fd7a7a9f0dc5b5bf7ca0675b9 # v1.2.0 + uses: actions/checkout@v1.2.0 # Basic checkout - name: Checkout basic @@ -150,7 +150,7 @@ jobs: steps: # Clone this repo - name: Checkout - uses: actions/checkout@544eadc6bf3d226fd7a7a9f0dc5b5bf7ca0675b9 # v1.2.0 + uses: actions/checkout@v1.2.0 # Basic checkout using git - name: Checkout basic @@ -182,7 +182,7 @@ jobs: steps: # Clone this repo - name: Checkout - uses: actions/checkout@544eadc6bf3d226fd7a7a9f0dc5b5bf7ca0675b9 # v1.2.0 + uses: actions/checkout@v1.2.0 # Basic checkout using git - name: Checkout basic diff --git a/testfiles/pinactions/output/multipleactions.yml b/testfiles/pinactions/output/multipleactions.yml index 9b9abf9c..1e4a7642 100644 --- a/testfiles/pinactions/output/multipleactions.yml +++ b/testfiles/pinactions/output/multipleactions.yml @@ -4,7 +4,7 @@ jobs: publish: runs-on: ubuntu-latest steps: - - uses: actions/checkout@544eadc6bf3d226fd7a7a9f0dc5b5bf7ca0675b9 # v1.2.0 + - uses: actions/checkout@v1.2.0 - uses: actions/setup-node@f1f314fca9dfce2769ece7d933488f076716723e # v1.4.6 with: node-version: 10 diff --git a/testfiles/pinactions/output/multiplejobs.yml b/testfiles/pinactions/output/multiplejobs.yml index 6d2d4721..435f8b62 100644 --- a/testfiles/pinactions/output/multiplejobs.yml +++ b/testfiles/pinactions/output/multiplejobs.yml @@ -8,7 +8,7 @@ jobs: name: build, pack & publish runs-on: ubuntu-latest steps: - - uses: actions/checkout@544eadc6bf3d226fd7a7a9f0dc5b5bf7ca0675b9 # v1.2.0 + - uses: actions/checkout@v1.2.0 # - name: Setup dotnet # uses: actions/setup-dotnet@v1 @@ -27,7 +27,7 @@ jobs: name: build, pack & publish runs-on: ubuntu-latest steps: - - uses: actions/checkout@544eadc6bf3d226fd7a7a9f0dc5b5bf7ca0675b9 # v1.2.0 + - uses: actions/checkout@v1.2.0 # - name: Setup dotnet # uses: actions/setup-dotnet@v1 diff --git a/testfiles/pinactions/output/repeatedactionwithcomment.yml b/testfiles/pinactions/output/repeatedactionwithcomment.yml index dbd50839..0cfd8a11 100644 --- a/testfiles/pinactions/output/repeatedactionwithcomment.yml +++ b/testfiles/pinactions/output/repeatedactionwithcomment.yml @@ -16,7 +16,7 @@ jobs: publish: runs-on: ubuntu-latest steps: - - uses: actions/checkout@544eadc6bf3d226fd7a7a9f0dc5b5bf7ca0675b9 # v1.2.0 + - uses: actions/checkout@v1.2.0 - uses: actions/setup-node@f1f314fca9dfce2769ece7d933488f076716723e # v1.4.6 with: node-version: 10 From 8e5d1fb6d6745e8b36b3bd279aa5510785658776 Mon Sep 17 00:00:00 2001 From: sailikhith-stepsecurity Date: Wed, 22 Jan 2025 11:41:39 +0530 Subject: [PATCH 7/7] updating action name while getting image manifest for bundled actions --- .../workflow/pin/action_image_manifest.go | 10 +++++++++- remediation/workflow/pin/pinactions_test.go | 19 ++++++++++++++++++- .../pinactions/input/immutableaction-1.yml | 3 ++- .../pinactions/output/immutableaction-1.yml | 1 + 4 files changed, 30 insertions(+), 3 deletions(-) diff --git a/remediation/workflow/pin/action_image_manifest.go b/remediation/workflow/pin/action_image_manifest.go index f7c48e29..16e3a1ad 100644 --- a/remediation/workflow/pin/action_image_manifest.go +++ b/remediation/workflow/pin/action_image_manifest.go @@ -69,6 +69,14 @@ func getOCIImageArtifactTypeForGhAction(action string) (string, error) { return "", fmt.Errorf("invalid action format") } + // For bundled actions like github/codeql-action/analyze@v3, + // we only need the repository part (github/codeql-action) to check for immutability + actionPath := parts[0] + if strings.Count(parts[0], "/") > 1 { + pathParts := strings.Split(parts[0], "/") + actionPath = strings.Join(pathParts[:2], "/") + } + // convert v1.x.x to 1.x.x which is // use regexp to match tag version format and replace v in prefix // as immutable actions image tag is in format 1.x.x (without v prefix) @@ -79,7 +87,7 @@ func getOCIImageArtifactTypeForGhAction(action string) (string, error) { } // Convert GitHub action to GHCR image reference using proper OCI reference format - image := fmt.Sprintf("ghcr.io/%s:%s", parts[0], parts[1]) + image := fmt.Sprintf("ghcr.io/%s:%s", actionPath, parts[1]) imageManifest, err := getOCIManifestForImage(image) if err != nil { return "", err diff --git a/remediation/workflow/pin/pinactions_test.go b/remediation/workflow/pin/pinactions_test.go index ef20695d..3e7c0ef8 100644 --- a/remediation/workflow/pin/pinactions_test.go +++ b/remediation/workflow/pin/pinactions_test.go @@ -173,6 +173,21 @@ func TestPinActions(t *testing.T) { } ]`)) + httpmock.RegisterResponder("GET", "https://api.github.com/repos/github/codeql-action/commits/v3", + httpmock.NewStringResponder(200, `d68b2d4edb4189fd2a5366ac14e72027bd4b37dd`)) + + httpmock.RegisterResponder("GET", "https://api.github.com/repos/github/codeql-action/git/matching-refs/tags/v3.", + 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, ``)) @@ -191,7 +206,8 @@ func TestPinActions(t *testing.T) { "repository:JS-DevTools/npm-publish:pull", "repository:elgohr/Publish-Docker-Github-Action:pull", "repository:brandedoutcast/publish-nuget:pull", - "repository:rohith/publish-nuget:pull": + "repository:rohith/publish-nuget:pull", + "repository:github/codeql-action:pull": return httpmock.NewJsonResponse(http.StatusOK, map[string]string{ "token": "test-token", "access_token": "test-token", @@ -213,6 +229,7 @@ func TestPinActions(t *testing.T) { // the following list will contain the list of actions with versions // which are mocked to be immutable "actions/checkout@v1.2.0", + "github/codeql-action@v3.28.2", } for _, action := range manifestResponders { diff --git a/testfiles/pinactions/input/immutableaction-1.yml b/testfiles/pinactions/input/immutableaction-1.yml index 41a6e0b9..3740d163 100644 --- a/testfiles/pinactions/input/immutableaction-1.yml +++ b/testfiles/pinactions/input/immutableaction-1.yml @@ -4,7 +4,8 @@ jobs: build: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v1.2.0 + - uses: actions/checkout@v1 + - uses: github/codeql-action/analyze@v3 - uses: borales/actions-yarn@v2.3.0 with: auth-token: ${{ secrets.GITHUB_TOKEN }} diff --git a/testfiles/pinactions/output/immutableaction-1.yml b/testfiles/pinactions/output/immutableaction-1.yml index a4a93961..b007a0e7 100644 --- a/testfiles/pinactions/output/immutableaction-1.yml +++ b/testfiles/pinactions/output/immutableaction-1.yml @@ -5,6 +5,7 @@ jobs: runs-on: ubuntu-latest steps: - uses: actions/checkout@v1.2.0 + - uses: github/codeql-action/analyze@v3.28.2 - uses: borales/actions-yarn@4965e1a0f0ae9c422a9a5748ebd1fb5e097d22b9 # v2.3.0 with: auth-token: ${{ secrets.GITHUB_TOKEN }}